[Manifest] Apply kustomize best practices to standalone manifest #3978
Conversation
Bobgy
added
kind/misc
|
This change is |
| value: $(BUCKET_NAME) | ||
| valueFrom: | ||
| configMapKeyRef: | ||
| name: pipeline-install-config |
There was a problem hiding this comment.
I saw in real deployment, the configmap name is pipeline-install-config-xxx. It has a strange post-fix, does it matter?
There was a problem hiding this comment.
No , it won't cause any problems. kustomize adds the suffix automatically to both configmap name and references to it. So that it can support configmap rolling update (updating configmap requires all dependent pods to restart to pick up new changes -- therefore, by adding a suffix, when configmap content changes, all deployments referring to it will use a new configmap name, so they will restart those pods with new configmap.)
configMapKeyRef from a configmap will be preserved in deployment yaml in Kubernetes, that's more kubernetes native way of adding config to a deployment.
This is suggested in https://github.com/kubeflow/manifests/blob/master/docs/KustomizeBestPractices.md#command-line-substitution.
| # NOTE: var name must be unique globally to allow composition of multiple kustomize | ||
| # packages. Therefore, we added prefix `kfp-cluster-scoped-` to distinguish it from | ||
| # others. | ||
| - name: kfp-cluster-scoped-namespace |
There was a problem hiding this comment.
wondering whether we just use "kfp-" prefix as it can already solve the concern of global conflict with other components. "kfp-cluster-sopced-namespace" would let me think on whether there is another namespace
There was a problem hiding this comment.
Agreed, any other suggestions?
I wasn't using kfp- because I want to be able to compose cluster-scoped package together with kfp namespace scoped package.
Configuration for a var can only appear once between both packages.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rmgogogo 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:
Approvers can indicate their approval by writing |
|
Getting one strange test failure /retest |
|
/retest |
|
/assign @jlewi I plan to use this as upstream for kf 1.1 very soon |
* Use configMapKeyRef for env vars * Allow easy customization of cluster-scoped resources namespace * clean up * Clean up * Simplify var replacement with direct configmap value ref * clean up params.env
…eflow#3978) * Use configMapKeyRef for env vars * Allow easy customization of cluster-scoped resources namespace * clean up * Clean up * Simplify var replacement with direct configmap value ref * clean up params.env
…eflow#3978) * Use configMapKeyRef for env vars * Allow easy customization of cluster-scoped resources namespace * clean up * Clean up * Simplify var replacement with direct configmap value ref * clean up params.env
This is part of KFP and KF 1.1 integration.
Kubeflow kustomize best practices doc: https://github.com/kubeflow/manifests/blob/master/docs/KustomizeBestPractices.md
Changes:
kfp-to make sure they don't conflict with kubeflowkustomization.yaml's namespace field for customization.