Update Kubeflow Pipeline ECR image and Helm charts #626
Conversation
There was a problem hiding this comment.
We also need to check the latest sync.py script from RC2 branch manifests and update that
Take a look at this PR - #456 and see what changed in awsconfigs/apps/pipeline/s3/sync.py file from upstream https://github.com/kubeflow/manifests/blob/v1.6.1/apps/pipeline/upstream/base/installs/multi-user/pipelines-profile-controller/sync.py
...w-pipelines/rds-only/crds/clusterworkflowtemplates.argoproj.io-CustomResourceDefinition.yaml
Outdated
Show resolved
Hide resolved
| @@ -29,14 +29,13 @@ spec: | |||
| - env: | |||
| - name: NAMESPACE | |||
| value: '' | |||
| valueFrom: null | |||
There was a problem hiding this comment.
can you confirm this was a bug previously?
There was a problem hiding this comment.
kustomize version issue. doesn't affect behavior, it;s null
...elines/rds-s3-static/crds/clusterworkflowtemplates.argoproj.io-CustomResourceDefinition.yaml
Outdated
Show resolved
Hide resolved
| autoUpdatePipelineDefaultVersion: 'true' | ||
| bucketName: '{{ .Values.s3.bucketName }}' | ||
| cacheDb: cachedb | ||
| cacheImage: gcr.io/google-containers/busybox | ||
| cacheNodeRestrictions: 'false' | ||
| cronScheduleTimezone: UTC | ||
| dbHost: testa.c6uzclpednto.ca-central-1.rds.amazonaws.com |
There was a problem hiding this comment.
why didnt the unit test catch this?
There was a problem hiding this comment.
this was from some of Ryan's previous IRSA chart, he accidentally left hard-coded values
There was a problem hiding this comment.
sure, shouldnt the helmify unit test fail after generation?
...low-pipelines/rds-s3/crds/clusterworkflowtemplates.argoproj.io-CustomResourceDefinition.yaml
Outdated
Show resolved
Hide resolved
| labels: | ||
| app: kubeflow-pipelines-profile-controller | ||
| app.kubernetes.io/component: ml-pipeline | ||
| app.kubernetes.io/name: kubeflow-pipelines | ||
| application-crd-id: kubeflow-pipelines | ||
| name: kubeflow-pipelines-profile-controller-code-dhffkh7f55 | ||
| name: kubeflow-pipelines-profile-controller-code |
There was a problem hiding this comment.
how come the suffix is getting removed here, but not in charts/apps/kubeflow-pipelines/rds-s3/templates/ConfigMap/kubeflow-pipelines-profile-controller-env-5252m69c4c-kubeflow-ConfigMap.yaml
There was a problem hiding this comment.
kubernetes-sigs/kustomize#5047 again seems an issue from kustomize version
There was a problem hiding this comment.
I think this bug is not reproducible in this PR atleast because is it happening only in rds-s3 and not in vanilla or rds-only. can you check?
There was a problem hiding this comment.
you are right, both rds-only and vanilla have the hash-suffix
**Description of your changes:** - update awsconfig and helm chart to use the newly built pipeline image (https://gallery.ecr.aws/kubeflow-on-aws/ml-pipeline/api-server) **Testing:** - [ ] Unit tests pass - [x] e2e tests pass - Details about new tests (If this PR adds a new feature) - Details about any manual tests performed
Description of your changes:
Testing: