From 79a5b1637556dbadb39b68444a2cf760d0b6e7ba Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Thu, 2 Nov 2023 14:56:35 -0400 Subject: [PATCH 1/6] 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 https://github.com/kubeflow/pipelines/issues/9689 - but in theory they'll matter again? Not sure exactly the scope of that issue. --- charms/kfp-api/src/charm.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/charms/kfp-api/src/charm.py b/charms/kfp-api/src/charm.py index 6a5eb73c..f99cddfe 100755 --- a/charms/kfp-api/src/charm.py +++ b/charms/kfp-api/src/charm.py @@ -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" ], @@ -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"], @@ -252,6 +248,17 @@ 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 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"{object_storage['service']}.{object_storage['namespace']}", + "OBJECTSTORECONFIG_PORT": str(object_storage["port"]), + "OBJECTSTORECONFIG_REGION": "", } return env_vars From 7e6745ba3e1aeda82ab48bc434f1dc8ec2e7c15d Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Fri, 3 Nov 2023 11:08:27 -0400 Subject: [PATCH 2/6] fix formatting --- charms/kfp-api/src/charm.py | 1 - 1 file changed, 1 deletion(-) diff --git a/charms/kfp-api/src/charm.py b/charms/kfp-api/src/charm.py index f99cddfe..a6f46e6a 100755 --- a/charms/kfp-api/src/charm.py +++ b/charms/kfp-api/src/charm.py @@ -248,7 +248,6 @@ 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"], From 118f0822cadb7205b9e816a3615245ecc6cd518a Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Fri, 3 Nov 2023 13:18:23 -0400 Subject: [PATCH 3/6] update tests to match previous changes --- charms/kfp-api/tests/unit/test_operator.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/charms/kfp-api/tests/unit/test_operator.py b/charms/kfp-api/tests/unit/test_operator.py index 75295589..3662a6f8 100644 --- a/charms/kfp-api/tests/unit/test_operator.py +++ b/charms/kfp-api/tests/unit/test_operator.py @@ -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", @@ -467,6 +462,15 @@ 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"] From a127670903b902a46e5ed9af3deb6a5a854eca17 Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Wed, 15 Nov 2023 16:07:42 -0500 Subject: [PATCH 4/6] fix: linting --- charms/kfp-api/tests/unit/test_operator.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/charms/kfp-api/tests/unit/test_operator.py b/charms/kfp-api/tests/unit/test_operator.py index 3662a6f8..d5b691f4 100644 --- a/charms/kfp-api/tests/unit/test_operator.py +++ b/charms/kfp-api/tests/unit/test_operator.py @@ -468,7 +468,9 @@ def test_install_with_all_inputs_and_pebble( # 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_HOST": ( + f"{objectstorage_data['service']}.{objectstorage_data['namespace']}" + ), "OBJECTSTORECONFIG_PORT": str(objectstorage_data["port"]), "OBJECTSTORECONFIG_REGION": "", } From c9d06f5799a4609df16ae53c524cf96b81bc35e9 Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Thu, 16 Nov 2023 09:44:02 -0500 Subject: [PATCH 5/6] update comments in kfp-api charm.py for OBJECTSTORECONFIG --- charms/kfp-api/src/charm.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/charms/kfp-api/src/charm.py b/charms/kfp-api/src/charm.py index a6f46e6a..1b52b06b 100755 --- a/charms/kfp-api/src/charm.py +++ b/charms/kfp-api/src/charm.py @@ -251,10 +251,14 @@ def _generate_environment(self) -> dict: # 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 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 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": "", From 4ef98aaf19abb476574c44bccd08f92c9c1ad44c Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Thu, 16 Nov 2023 09:47:46 -0500 Subject: [PATCH 6/6] fix linting --- charms/kfp-api/src/charm.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/charms/kfp-api/src/charm.py b/charms/kfp-api/src/charm.py index 1b52b06b..15da2061 100755 --- a/charms/kfp-api/src/charm.py +++ b/charms/kfp-api/src/charm.py @@ -252,11 +252,11 @@ def _generate_environment(self) -> dict: "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. + # 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 + # 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']}",