Skip to content

Commit

Permalink
update kfp-api's apiserver configuration (#375)
Browse files Browse the repository at this point in the history
* update kfp-api's apiserver configuration

This:
* removes deprecated `DBCONFIG_USER`, etc, environment variables (they have been replaced by variables of name `DBCONFIG_[driver]CONFIG_*`)
* adds `OBJECTSTORECONFIG_HOST`, `_PORT`, and `_REGION`, which previously were required.  Although currently they seem to be ignored due to kubeflow/pipelines#9689 - but in theory they'll matter again?  Not sure exactly the scope of that issue.
  • Loading branch information
ca-scribner authored Nov 16, 2023
1 parent e204488 commit acdf20a
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 10 deletions.
20 changes: 15 additions & 5 deletions charms/kfp-api/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ def _generate_environment(self) -> dict:
raise error

env_vars = {
# Configurations that are also defined in the upstream manifests
"AUTO_UPDATE_PIPELINE_DEFAULT_VERSION": self.model.config[
"auto-update-default-version"
],
Expand All @@ -231,11 +232,6 @@ def _generate_environment(self) -> dict:
"POD_NAMESPACE": self.model.name,
"OBJECTSTORECONFIG_SECURE": "false",
"OBJECTSTORECONFIG_BUCKETNAME": self.model.config["object-store-bucket-name"],
"DBCONFIG_USER": db_data["db_username"],
"DBCONFIG_PASSWORD": db_data["db_password"],
"DBCONFIG_DBNAME": db_data["db_name"],
"DBCONFIG_HOST": db_data["db_host"],
"DBCONFIG_PORT": db_data["db_port"],
"DBCONFIG_CONMAXLIFETIME": "120s",
"DB_DRIVER_NAME": "mysql",
"DBCONFIG_MYSQLCONFIG_USER": db_data["db_username"],
Expand All @@ -252,6 +248,20 @@ def _generate_environment(self) -> dict:
"ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_HOST": viz_data["service-name"],
"ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_PORT": viz_data["service-port"],
"CACHE_IMAGE": self.model.config["cache-image"],
# Configurations charmed-kubeflow adds to those of upstream
"ARCHIVE_CONFIG_LOG_FILE_NAME": self.model.config["log-archive-filename"],
"ARCHIVE_CONFIG_LOG_PATH_PREFIX": self.model.config["log-archive-prefix"],
# OBJECTSTORECONFIG_HOST and _PORT set the object storage configurations,
# taking precedence over configuration in the config.json or
# MINIO_SERVICE_SERVICE_* environment variables.
# NOTE: While OBJECTSTORECONFIG_HOST and _PORT control the object store
# that the apiserver connects to, other parts of kfp currently cannot use
# object stores with arbitrary names. See
# https://github.com/kubeflow/pipelines/issues/9689 and
# https://github.com/canonical/minio-operator/pull/151 for more details.
"OBJECTSTORECONFIG_HOST": f"{object_storage['service']}.{object_storage['namespace']}",
"OBJECTSTORECONFIG_PORT": str(object_storage["port"]),
"OBJECTSTORECONFIG_REGION": "",
}

return env_vars
Expand Down
16 changes: 11 additions & 5 deletions charms/kfp-api/tests/unit/test_operator.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,11 +446,6 @@ def test_install_with_all_inputs_and_pebble(
"POD_NAMESPACE": harness.charm.model.name,
"OBJECTSTORECONFIG_SECURE": "false",
"OBJECTSTORECONFIG_BUCKETNAME": harness.charm.config["object-store-bucket-name"],
"DBCONFIG_USER": "root",
"DBCONFIG_PASSWORD": mysql_data["root_password"],
"DBCONFIG_DBNAME": mysql_data["database"],
"DBCONFIG_HOST": mysql_data["host"],
"DBCONFIG_PORT": mysql_data["port"],
"DBCONFIG_CONMAXLIFETIME": "120s",
"DB_DRIVER_NAME": "mysql",
"DBCONFIG_MYSQLCONFIG_USER": "root",
Expand All @@ -467,6 +462,17 @@ def test_install_with_all_inputs_and_pebble(
"ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_HOST": kfp_viz_data["service-name"],
"ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_PORT": kfp_viz_data["service-port"],
"CACHE_IMAGE": harness.charm.config["cache-image"],
"ARCHIVE_CONFIG_LOG_FILE_NAME": harness.charm.config["log-archive-filename"],
"ARCHIVE_CONFIG_LOG_PATH_PREFIX": harness.charm.config["log-archive-prefix"],
# OBJECTSTORECONFIG_HOST and _PORT currently have no effect due to
# https://github.com/kubeflow/pipelines/issues/9689, described more in
# https://github.com/canonical/minio-operator/pull/151
# They're included here so that when the upstream issue is fixed we don't break
"OBJECTSTORECONFIG_HOST": (
f"{objectstorage_data['service']}.{objectstorage_data['namespace']}"
),
"OBJECTSTORECONFIG_PORT": str(objectstorage_data["port"]),
"OBJECTSTORECONFIG_REGION": "",
}
test_env = pebble_plan_info["services"][KFP_API_SERVICE_NAME]["environment"]

Expand Down

0 comments on commit acdf20a

Please sign in to comment.