helm: add support for kube-version and add cli args for both kube-version and api-versions #5270
Conversation
|
This PR has multiple commits, and the default merge method is: merge. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/M
MrFreezeex
force-pushed
the
helm-kube-api
branch
2 times, most recently
from
5f25018 to
06c57a4
Compare
|
Hi @koba1t, sorry for the ping. Since I saw you reviewing the helm OCI PR and as this is also a helm related PR, do you think it would it be possible for you to take a look/review this PR as well whenever you can? 🙏 |
|
Hi @cailynse, sorry for the ping. But can you have a look at this, it's a key piece that's blocking the use of modern helm charts. |
|
/assign @koba1t |
|
Hi @MrFreezeex Looks almost good, but I think more test cases. |
Hey, thanks for the review! Although I am not sure I understood what test you meant. Do you mean like a test when you pass both the CLI option to kustomize and |
Hmm, I think we should add a test that covers such cases. But I want to add any tests to check when the |
Sure I can take a look 👍
Hmm well I can do that but such test would be testing directly helm rather than a kustomize feature. In the tests of kustomize I currently test that if you pass |
Signed-off-by: Arthur Outhenin-Chalandre <[email protected]>
It makes sense to add that as a CLI args since you could use one single kustomization file/helm chart for multiple clusters. Also it's easier to have those on the CLI if the user has some kind of tooling that will end up calling kustomize and that could pass those (i.e.: ArgoCD is doing that for Helm so it could do that for Kustomize as well that will end up calling Helm as well). Signed-off-by: Arthur Outhenin-Chalandre <[email protected]>
|
Actually there is one for apiversions as well even though it doesn't test much, will check how to do that 👍 |
MrFreezeex
force-pushed
the
helm-kube-api
branch
from
06c57a4 to
8ba0557
Compare
k8s-ci-robot
added
size/L
|
I added a new test with two different kubeVersion that make some change to the ingress group ( |
MrFreezeex
force-pushed
the
helm-kube-api
branch
from
8ba0557 to
e63035a
Compare
There was a problem hiding this comment.
Thanks @MrFreezeex
I added one minor suggestion for a test case.
Please check this.
|
|
||
| th.WriteK(th.GetRoot(), ` | ||
| namespace: default | ||
| helmCharts: |
There was a problem hiding this comment.
Looks like two helms generating one time.
So, I think I prefer generating one helmCharts two times in this case due to eliminate side effects.
There was a problem hiding this comment.
Sure, I think I did what you wanted! Let me know 👍
Signed-off-by: Arthur Outhenin-Chalandre <[email protected]>
MrFreezeex
force-pushed
the
helm-kube-api
branch
from
e63035a to
dc29923
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: koba1t, MrFreezeex 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 |
k8s-ci-robot
added
the
approved
Add basic --kube-version flag support and test and add Kustomize CLI args for both api-versions and kube-version
CLI args are useful since you could use one single kustomization file/helm chart for multiple clusters (which have different versions). Also it's easier to have those on the CLI if the user has some kind of tooling that will end up calling kustomize and that could pass those to Helm in the end (i.e.: ArgoCD is doing that for Helm so it could do that for Kustomize as well that will end up calling Helm).
Fixes #5112