kubernetes-sigs
🌱 Kustomize: Update deprecated syntax
What this PR does / why we need it:
This commit updates the following:
- patchesStrategicMerge -> patches
- patchesJson6902 -> patches
- vars and varReference -> replacements
- bases -> resources
Most of this is straight forward, but the vars -> replacements change is a bit complicated. I have taken inspiration from kubebuilder for how to do the change. In particular I changed the name of the secret that holds the certificate to be static. Previously it was set partially from a variable. I believe it would be unnecessarily complicated to keep this behavior and that a static name does not take away much.
Here is a link to some relevant code in kubebuilder that I used as inspiration: https://github.com/kubernetes-sigs/kubebuilder/blob/68abac1dfb5550f7159dec4ce600cd9b6db925bb/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/kdefault/kustomization.go#L97
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Related to #8457.
/area dependency
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign sbueringer for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Thanks so much for working on this!
Do you want to also include the Kustomize bump in the Makefile so we can close off that issue?
Sure, I can do that. I was not sure about the first part of the issue though. Something about creationTimestamp, is that done?
Hmm this could prove a bit tricky.
Error: trouble configuring builtin PatchTransformer with config: `
path: cluster-ipv6.yaml
`: unable to parse SM or JSON patch from [---
apiVersion: cluster.x-k8s.io/v1beta1
kind: Cluster
metadata:
name: '${CLUSTER_NAME}'
I guess the envsubst variables are messing up the kustomize patch. Will need to investigate.
Sure, I can do that. I was not sure about the first part of the issue though. Something about creationTimestamp, is that done?
Yeah - the creationTimestamp issue was a bug which was fixed in controller tools - ref https://github.com/kubernetes-sigs/controller-tools/pull/800
The test errors seem to be because of https://github.com/kubernetes-sigs/kustomize/issues/5049. It is possible to work around them by splitting the strategic merge patches into multiple files, but I think the better approach is to just wait for this issue to be resolved. There is work on-going to fix it and if that lands we will not need any additional changes I hope.
/hold
Thanks for digging into the errors! Let's track the status of that issue - if it's fixed relatively soon we can update. . AFAIK we only kustomize for generating test yamls so I'm not sure if we have any real urgency for bumping the version for CAPI.
PR needs rebase.
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.
Hey, thanks for this update. I bumped into this PR yesterday.
The issue mentioned above is fixed upstream. I checked out your branch locally and even with the latest kustomize version (v5.0.3), I'm getting the parsing error mentioned above.
It is possible to work around them by splitting the strategic merge patches into multiple files.
I think this still holds true. When I comment out a patch from the files where we have more than one patches, it works.
I guess you mean v5.3.0? That is strange :thinking: I will try to get some time to check it this week
I guess you mean v5.3.0? That is strange 🤔
Yes, that was a typo. :(
Let me know (slack or here) if I can be helpful. Happy to help in upgrading this.
@lentzi90: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| pull-cluster-api-e2e-informing-main | e502aeceba3314c75a4318034dd8e270432d7708 | link | false | /test pull-cluster-api-e2e-informing-main |
| pull-cluster-api-e2e-main | e502aeceba3314c75a4318034dd8e270432d7708 | link | true | /test pull-cluster-api-e2e-main |
| pull-cluster-api-e2e-blocking-main | e502aeceba3314c75a4318034dd8e270432d7708 | link | true | /test pull-cluster-api-e2e-blocking-main |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
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. I understand the commands that are listed here.
@lentzi90 , kindly want to ask if you have time to get back on this or should someone else takeover :-)
Sorry, I have not had time to get back to this. @peppi-lotta volunteered to take over though :tada: Hopefully she will be able take this forward next week :slightly_smiling_face:
@lentzi90: GitHub didn't allow me to assign the following users: peppi-lotta.
Note that only kubernetes-sigs members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide
In response to this:
/assign @peppi-lotta
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.
Let's close this in favor of https://github.com/kubernetes-sigs/cluster-api/pull/10294