Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove use of deprecated patchesStrategicMerge #3539

Closed
lentzi90 opened this issue Aug 17, 2023 · 13 comments · Fixed by #3661
Closed

Remove use of deprecated patchesStrategicMerge #3539

lentzi90 opened this issue Aug 17, 2023 · 13 comments · Fixed by #3661
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@lentzi90
Copy link

What broke? What's expected?

Most of the deprecated kustomize syntax has already been removed, but I came across one instance of patchesStrategicMerge still lingering:


This should be changed to patches.

Reproducing this issue

mkdir -p ~/projects/guestbook
cd ~/projects/guestbook
kubebuilder init --domain my.domain --repo my.domain/guestbook
kubebuilder create api --group webapp --version v1 --kind Guestbook --resource --controller

Now check config/default/kustomization.yaml:

grep "patchesStrategicMerge" config/default/kustomization.yaml

KubeBuilder (CLI) Version

Version: main.version{KubeBuilderVersion:"3.11.1", KubernetesVendor:"1.27.1", GitCommit:"1dc8ed95f7cc55fef3151f749d3d541bec3423c9", BuildDate:"2023-07-03T13:10:56Z", GoOs:"linux", GoArch:"amd64"}

PROJECT version

3

Plugin versions

layout:
- go.kubebuilder.io/v4

Other versions

No response

Extra Labels

/kind deprecation

@lentzi90 lentzi90 added the kind/bug Categorizes issue or PR as related to a bug. label Aug 17, 2023
@k8s-ci-robot k8s-ci-robot added the kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. label Aug 17, 2023
@typeid
Copy link
Contributor

typeid commented Aug 17, 2023

There seem to be a few more occurrences:

./docs/book/src/component-config-tutorial/api-changes.md:121:Update the file `default/kustomization.yaml` by adding under the [`patchesStrategicMerge:` key](https://kubectl.docs.kubernetes.io/references/kustomize/builtins/#_patchesstrategicmerge_) the following patch:
./docs/book/src/component-config-tutorial/api-changes.md:124:patchesStrategicMerge:
./docs/book/src/component-config-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/src/cronjob-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/src/multiversion-tutorial/testdata/project/config/crd/kustomization.yaml:8:patchesStrategicMerge:
./docs/book/src/multiversion-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/book/cronjob-tutorial/running-webhook.html:230:patchesStrategicMerge:
./docs/book/book/cronjob-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/book/multiversion-tutorial/testdata/project/config/crd/kustomization.yaml:8:patchesStrategicMerge:
./docs/book/book/multiversion-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/book/component-config-tutorial/api-changes.html:265:<p>Update the file <code>default/kustomization.yaml</code> by adding under the <a href="https://kubectl.docs.kubernetes.io/references/kustomize/builtins/#_patchesstrategicmerge_"><code>patchesStrategicMerge:</code> key</a> the following patch:</p>
./docs/book/book/component-config-tutorial/api-changes.html:266:<pre><code class="language-yaml">patchesStrategicMerge:
./docs/book/book/component-config-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/book/print.html:2530:patchesStrategicMerge:
./docs/book/book/print.html:4291:<p>Update the file <code>default/kustomization.yaml</code> by adding under the <a href="https://kubectl.docs.kubernetes.io/references/kustomize/builtins/#_patchesstrategicmerge_"><code>patchesStrategicMerge:</code> key</a> the following patch:</p>
./docs/book/book/print.html:4292:<pre><code class="language-yaml">patchesStrategicMerge:
./pkg/plugins/common/kustomize/v1/scaffolds/internal/templates/config/crd/kustomization.go:113:patchesStrategicMerge:
./pkg/plugins/common/kustomize/v1/scaffolds/internal/templates/config/kdefault/kustomization.go:73:patchesStrategicMerge:
./pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/kdefault/kustomization.go:75:patchesStrategicMerge:
./pkg/plugins/golang/v2/scaffolds/internal/templates/config/crd/kustomization.go:113:patchesStrategicMerge:
./pkg/plugins/golang/v2/scaffolds/internal/templates/config/kdefault/kustomization.go:83:patchesStrategicMerge:
./testdata/project-v2/config/default/kustomization.yaml:27:patchesStrategicMerge:
./testdata/project-v2/config/crd/kustomization.yaml:10:patchesStrategicMerge:
./testdata/project-v3/config/crd/kustomization.yaml:10:patchesStrategicMerge:
./testdata/project-v3/config/default/kustomization.yaml:27:patchesStrategicMerge:
./testdata/project-v4-declarative-v1/config/default/kustomization.yaml:29:patchesStrategicMerge:
./testdata/project-v4-multigroup/config/default/kustomization.yaml:29:patchesStrategicMerge:
./testdata/project-v4-with-deploy-image/config/default/kustomization.yaml:29:patchesStrategicMerge:
./testdata/project-v4-with-grafana/config/default/kustomization.yaml:29:patchesStrategicMerge:
./testdata/project-v4/config/default/kustomization.yaml:29:patchesStrategicMerge:

We could probably look into removing all of those.

@camilamacedo86
Copy link
Member

We should only remove from kustomize/v2 plugin and not v1 since, the v1 is deprecated. Therefore, the projects scaffolds within go/v2 and go/3 layout, which either are deprecated, will not be changed.

So, the change must be done in: ./pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/kdefault/kustomization.go:75:patchesStrategicMerge:

And then, we run make generate to update the docs and samples accordingly.

Would you like to contribute with this one @lentzi90?

@camilamacedo86 camilamacedo86 added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Aug 17, 2023
@ashutosh887
Copy link
Contributor

ashutosh887 commented Aug 17, 2023

I'd like to pick this if not @lentzi90

cc: @camilamacedo86

@Sajiyah-Salat
Copy link
Contributor

/assign

@ashutosh887
Copy link
Contributor

/assign

@lentzi90
Copy link
Author

Feel free to take it!
After I created the issue I have found an issue in kustomize, which is probably the reason this wasn't fixed before. See kubernetes-sigs/kustomize#5049.
It seems to be moving along nicely, so perhaps we could just wait for it to be resolved to avoid unnecessary workarounds?

The gist is that pathesStrategicMerge can be easily changed into patches like this:

# Before
pathesStrategicMerge:
- patch_file.yaml

# After
patches:
- path: patch_file.yaml

However, there is one thing that patches cannot do (yet). It does not accept a patch file with multiple patches in it.
This is the case we have for the webhookcainjection_patch.yaml.

It is possible to work around by splitting the patch into multiple files, but as mentioned, I think we could just wait for the issue to be fixed in kustomize before we get rid of patchesStrategicMerge.

@Sajiyah-Salat
Copy link
Contributor

They are working on patches and it will accept multiple patches in future. Should we just wait for now?
cc: @camilamacedo86 @varshaprasad96

@lentzi90
Copy link
Author

The issue in Kustomize has been fixed 🎉
I guess now we need to wait for a release and bump the version somewhere in kubebuilder to make it work

@Sajiyah-Salat
Copy link
Contributor

After the release we just need to run kustomize edit fix to fix the issue as described in description of the above issue mentioned. right?

@lentzi90
Copy link
Author

I think it may be better to do it manually. At least I had some issues with kustomize edit fix before. It can do strange things 😅 It should be pretty easy to do manually also, all that is needed is this:

# Before
pathesStrategicMerge:
- patch_file.yaml

# After
patches:
- path: patch_file.yaml

@Sajiyah-Salat
Copy link
Contributor

Sajiyah-Salat commented Sep 20, 2023

@camilamacedo86 okay so we can do it now as well. right? I can open a pr then. is it fine?

@lentzi90
Copy link
Author

I'm not sure how kubebuilder handles kustomize. Maybe @camilamacedo86 knows?

@camilamacedo86
Copy link
Member

camilamacedo86 commented Oct 1, 2023

Hi @Sajiyah-Salat

@camilamacedo86 okay so we can do it now as well. right? I can open a pr then. is it fine?

Yes, you can open PRs to solve any issue and as please you.
Thank you 🥇

Hi @lentzi90 ,

I'm not sure how kubebuilder handles kustomize. Maybe @camilamacedo86 knows?

  • The changes should be done in the plugin kustomize/v2 only : kustomize/v2

  • kustomize plugin is the plugin responsible for scaffolding everything under the config dir.

  • When we run kubebuilder init|edit|create we are calling by default the init|edit|create implementation of the kustomize/v2 and that of the golang/v4.

  • Look at the templates, we must make the changes on the templates.

  • Therefore, if we use kustomize edit fix we need to compare with a version of the same project without the fix, check the changes (e.g. compare the same project under the sample project-v4 with the fix and without the fix) and then update the templates, see: kustomize/v2 templates

  • After updating the templates run make generate to get the samples under testdata (testdata) and docs with your change.

I hope that it answer your questions
Thank you for your collaboration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
6 participants