feat(deployment): add option to deploy mysql in KFP standalone #9855

Linchin · 2023-08-12T00:46:56Z

Description of your changes:

Update the manifests so users can choose to deploy KFP with MySQL or PostgreSQL
Note: the PostgreSQL will not work for now, due to backend support is not completed
Note: added a set of env vars for MySQL, and another set of env vars for PostgreSQL. Keeping the current env vars so the deployment doesn't break due to this PR. Will need to remove them after releasing and fully support the new SQL settings.

After merging this PR, users who deploy KFP standalone with MySQL will use the same commands as before:

export PIPELINE_VERSION=2.0.0
kubectl apply -k "github.com/kubeflow/pipelines/manifests/kustomize/cluster-scoped-resources?ref=$PIPELINE_VERSION"
kubectl wait --for condition=established --timeout=60s crd/applications.app.k8s.io
kubectl apply -k "github.com/kubeflow/pipelines/manifests/kustomize/env/dev?ref=$PIPELINE_VERSION"

With PostgreSQL, the command is slightly different:

export PIPELINE_VERSION=2.0.0
kubectl apply -k "github.com/kubeflow/pipelines/manifests/kustomize/cluster-scoped-resources?ref=$PIPELINE_VERSION"
kubectl wait --for condition=established --timeout=60s crd/applications.app.k8s.io
kubectl apply -k "github.com/kubeflow/pipelines/manifests/kustomize/env/dev/postgresql?ref=$PIPELINE_VERSION"

Using this method instead of letting user change their config in pipeline-install-config.yaml, because Kustomize does not allow conditional deployment (as far as I can find).

To-Do:

Verify that the manifests with MySQL deploys successfully
Update the documentation
(after release) remove the old environment variables in ml-pipeline-apiserver-deployment.yaml

Checklist:

The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.

Linchin · 2023-08-14T18:05:37Z

part of #9813

Linchin · 2023-08-16T20:19:46Z

/test kubeflow-pipeline-e2e-test

zijianjoy · 2023-08-17T16:10:42Z

manifests/kustomize/base/installs/generic/postgres/pipeline-install-config.yaml

+  dbHost: postgres
+  dbPort: "5432"


Can we avoid reusing the same variable name for both mysql config and postgresql config?

In the installs/generic/pipeline-install-config.yaml file, you defined mysqlHost, so we can use the same approach to name postgresqlConfigDbName or something similar.

Thank you for catching this. I should have added the postgres counterpart for the db config. The original parameters dbType and dbHost are kept for now because the v2 images these manifests use still call for them. They should be removed after we release the new images.

zijianjoy · 2023-08-17T16:56:43Z

manifests/kustomize/base/installs/generic/pipeline-install-config.yaml

+  dbHost: mysql  # relic to be removed after release
+  dbPort: "3306" # relic to be removed after release
+  dbType: mysql
+  mysqlHost: mysql


NIT: Do we want to name the new mysqlPort key to be something similar to the definition in

pipelines/backend/src/apiserver/config/config.json

Line 5 in e88e7a6

"DBName": "mlpipeline",

?

Sorry, I'm not sure understand. Did you mean to name it something like DBConfig.MySQLConfig.Port?

That is right, because it is a configmap, you can call it like dbconfig_mysqlconfig_port.

I'm not sure about this. The rest of the variable names are not consistent with their counterparts in config.json either. If we change all of them, this might lead to more backward compatibility issues.

zijianjoy · 2023-08-17T16:59:32Z

manifests/kustomize/base/pipeline/ml-pipeline-apiserver-deployment.yaml

+            secretKeyRef:
+              name: postgres-secret
+              key: username
+              optional: true


If postgresql is chosen as the driver DB, the flags here are not optional, but the mysql flags are optional.

Maybe consider using a patch to include postgresql flag in the platform-agnostic-postgresql overlay?

Thank you, I didn't think of this idea. I have changed the code so the mysql parameters are only required in default deployment, and postgres parameters are only required when postgres is used. I will commit after the local test passes.

manifests/kustomize/env/dev/postgresql/kustomization.yaml

Linchin · 2023-08-17T20:03:58Z

/test kubeflow-pipeline-e2e-test

Linchin · 2023-08-17T23:23:36Z

/test kubeflow-pipeline-e2e-test

zijianjoy · 2023-08-18T00:09:22Z

manifests/kustomize/base/installs/generic/pipeline-install-config.yaml

@@ -11,7 +11,7 @@ data:
    until the changes take effect. A quick way to restart all deployments in a
    namespace: `kubectl rollout restart deployment -n <your-namespace>`.
  appName: pipeline
-  appVersion: 2.0.0
+  appVersion: 2.0.1


It seems that we should still use 2.0.0 for the consistency across repo.

This was changed due to version 2.0.1 was released yesterday, see #9899.

zijianjoy · 2023-08-18T00:17:31Z

manifests/kustomize/base/pipeline/postgres/kustomization.yaml

+bases:
+  - ../metadata-writer
+resources:
+  - ml-pipeline-apiserver-deployment.yaml


Using patches in kustomization.yaml can make this file much simpler: https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/patches/

apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization bases: - ../metadata-writer - ../ patches: - ml-pipeline-apiserver-deployment-patch.yaml

ml-pipeline-apiserver-deployment-patch.yaml

apiVersion: apps/v1 kind: Deployment metadata: name: ml-pipeline spec: template: spec: containers: - name: DB_DRIVER_NAME valueFrom: configMapKeyRef: name: pipeline-install-config key: dbType # PostgreSQL Config - name: DBCONFIG_POSTGRESQLCONFIG_USER valueFrom: secretKeyRef: name: postgres-secret key: username - name: DBCONFIG_POSTGRESQLCONFIG_PASSWORD valueFrom: secretKeyRef: name: postgres-secret key: password - name: DBCONFIG_POSTGRESQLCONFIG_DBNAME valueFrom: configMapKeyRef: name: pipeline-install-config key: pipelineDb - name: DBCONFIG_POSTGRESQLCONFIG_HOST valueFrom: configMapKeyRef: name: pipeline-install-config key: postgresHost - name: DBCONFIG_POSTGRESQLCONFIG_PORT valueFrom: configMapKeyRef: name: pipeline-install-config key: postgresPort # end of PostgreSQL variables name: ml-pipeline-api-server

They are still slightly different, because ml-pipeline-apiserver-deployment.yaml in the parent folder also includes some MySQL specific parameters, which are not needed in a PostgreSQL deployment.

zijianjoy

/lgtm

Awesome work Lingqing! Pending for test to pass and remaining comments to address.

zijianjoy · 2023-08-21T19:03:30Z

/lgtm
/approve

Thank you so much Lingqing!

google-oss-prow · 2023-08-21T19:03:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zijianjoy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~manifests/kustomize/OWNERS~~ [zijianjoy]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…low#9855) * add option to deploy mysql * fix deployment errors * remove temp code in development * keep mysql deployment path same as before * change the generic folder * manifest error * revert default cache path * address comments

add option to deploy mysql

236c602

Linchin added the do-not-merge/work-in-progress label Aug 12, 2023

Linchin requested review from chensun and zijianjoy August 12, 2023 00:46

Linchin assigned chensun and zijianjoy Aug 12, 2023

google-oss-prow bot removed the do-not-merge/work-in-progress label Aug 12, 2023

google-oss-prow bot requested a review from gkcalat August 12, 2023 00:47

google-oss-prow bot added the size/L label Aug 12, 2023

Linchin added 6 commits August 15, 2023 20:26

fix deployment errors

909a948

remove temp code in development

5756c2b

keep mysql deployment path same as before

5f9f11f

change the generic folder

a131737

manifest error

ae5ebd1

revert default cache path

3f2acb5

zijianjoy reviewed Aug 17, 2023

View reviewed changes

address comments

374c442

google-oss-prow bot added size/XL and removed size/L labels Aug 17, 2023

Linchin requested a review from zijianjoy August 17, 2023 22:05

Merge branch 'master' into postgres-manifest

621c4c8

zijianjoy reviewed Aug 18, 2023

View reviewed changes

google-oss-prow bot added the lgtm label Aug 18, 2023

google-oss-prow bot added the approved label Aug 21, 2023

google-oss-prow bot merged commit b086020 into kubeflow:master Aug 21, 2023

Linchin mentioned this pull request Aug 21, 2023

Standalone KFP manifests for postgres #9860

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(deployment): add option to deploy mysql in KFP standalone #9855

feat(deployment): add option to deploy mysql in KFP standalone #9855

Linchin commented Aug 12, 2023 •

edited

Loading

Linchin commented Aug 14, 2023

Linchin commented Aug 16, 2023

zijianjoy Aug 17, 2023

Linchin Aug 17, 2023

zijianjoy Aug 17, 2023

Linchin Aug 17, 2023

zijianjoy Aug 18, 2023

Linchin Aug 18, 2023

zijianjoy Aug 17, 2023

Linchin Aug 17, 2023

Linchin commented Aug 17, 2023

Linchin commented Aug 17, 2023

zijianjoy Aug 18, 2023

Linchin Aug 18, 2023

zijianjoy Aug 18, 2023

Linchin Aug 18, 2023 •

edited

Loading

zijianjoy left a comment

zijianjoy commented Aug 21, 2023

google-oss-prow bot commented Aug 21, 2023

feat(deployment): add option to deploy mysql in KFP standalone #9855

feat(deployment): add option to deploy mysql in KFP standalone #9855

Conversation

Linchin commented Aug 12, 2023 • edited Loading

Linchin commented Aug 14, 2023

Linchin commented Aug 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Linchin commented Aug 17, 2023

Linchin commented Aug 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Linchin Aug 18, 2023 • edited Loading

Choose a reason for hiding this comment

zijianjoy left a comment

Choose a reason for hiding this comment

zijianjoy commented Aug 21, 2023

google-oss-prow bot commented Aug 21, 2023

Linchin commented Aug 12, 2023 •

edited

Loading

Linchin Aug 18, 2023 •

edited

Loading