Fix pipeline cannot run bug when using marketplace managed storage (#…

…2341) * Make gcs bucket name configurable + fix marketplace managed storage does not run pipelines successfully * gcsBucketName is computed from cloudsqlInstanceConnectionName + avoid code duplication
kubeflow · Oct 16, 2019 · 49a50df · 49a50df
1 parent bb48749
commit 49a50df
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 6 deletions.
diff --git a/manifests/gcp_marketplace/chart/kubeflow-pipelines/templates/argo.yaml b/manifests/gcp_marketplace/chart/kubeflow-pipelines/templates/argo.yaml
@@ -93,7 +93,7 @@ data:
     artifactRepository:
     {
         s3: {
-            bucket: mlpipeline,
+            bucket: '{{ if .Values.managedstorage.enabled }}{{ tpl .Values.managedstorage.gcsBucketName . }}{{ else }}mlpipeline{{ end }}',
             keyPrefix: artifacts,
             endpoint: minio-service.{{ .Release.Namespace }}:9000,
             insecure: true,

diff --git a/manifests/gcp_marketplace/chart/kubeflow-pipelines/templates/pipeline.yaml b/manifests/gcp_marketplace/chart/kubeflow-pipelines/templates/pipeline.yaml
@@ -624,11 +624,7 @@ spec:
             # Following environment variables are only needed when using Cloud SQL and GCS.
             {{ if .Values.managedstorage.enabled }}
             - name: OBJECTSTORECONFIG_BUCKETNAME
-              {{ if .Values.managedstorage.databaseNamePrefix }}
-              value: '{{ printf "%s-%s" .Values.managedstorage.cloudsqlInstanceConnectionName .Values.managedstorage.databaseNamePrefix | replace ":" "-" | trunc 50 }}'
-              {{ else }}
-              value: '{{ printf "%s-%s" .Values.managedstorage.cloudsqlInstanceConnectionName .Release.Name | replace ":" "-" | trunc 50 }}'
-              {{ end }}
+              value: '{{ tpl .Values.managedstorage.gcsBucketName . }}'
             - name: DBCONFIG_DBNAME
               {{ if .Values.managedstorage.databaseNamePrefix }}
               value: '{{ .Values.managedstorage.databaseNamePrefix }}_pipeline'

diff --git a/manifests/gcp_marketplace/chart/kubeflow-pipelines/values.yaml b/manifests/gcp_marketplace/chart/kubeflow-pipelines/values.yaml
@@ -20,6 +20,12 @@ serviceAccountCredential: ""
 managedstorage:
   enabled: false
   cloudsqlInstanceConnectionName: null
+  # gcsBucketName should be determined by cloudsqlInstanceConnectionName to make
+  # sure user is always using a valid pair of connection name + gcs bucket name.
+  #
+  # gcsBucketName is used in two places, so I wrote a template string here that
+  # can be evaluated in each place.
+  gcsBucketName: '{{ if .Values.managedstorage.databaseNamePrefix }}{{ printf "%s-%s" .Values.managedstorage.cloudsqlInstanceConnectionName .Values.managedstorage.databaseNamePrefix | replace ":" "-" | trunc 50 }}{{ else }}{{ printf "%s-%s" .Values.managedstorage.cloudsqlInstanceConnectionName .Release.Name | replace ":" "-" | trunc 50 }}{{ end }}'
   databaseNamePrefix: null
   dbUsername: 'root'
   dbPassword: ''