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

Know what changes in the scaffolds for Golang operators or SDK bundle generate command are required to use kustomize v4 (make bundle does not work with) #5898

Closed
camilamacedo86 opened this issue Jun 23, 2022 · 6 comments
Assignees
Labels
language/go Issue is related to a Go operator project needs discussion priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Jun 23, 2022

Investigation

Describe the problem you need a feature to resolve.

For we are able to support Apple Silicon we need to have the kustomize bin v3 for darwin/arm64 (kubernetes-sigs/kustomize#4612) which seems that will not be provided because v3 is deprecated more than one year ago OR are able to move forward to use kustomize v4.

According to the kustomize release notes, https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv4.0.0 would be possible to use the kustomize v4 with the current scaffold (only the use of special URLs in the resource would be impacted but we do not use it). The other changes introduced with v4 would be a problem only if we begin to change the scaffold (for example move from vars to replacement and then we would begin to generate a new syntax by calling create api/webhooks)

However, the documentation is not very clear about what exactly stops working when we move from v3 to v4. Or how to migrate from v3 to v4.

Therefore, by trying to check SDK with kustomize v4 it breaks the Golang/v3 scaffold when Webhooks are scaffold.
The kustomize syntax used to remove the cert-manager from the CSV fails:

$ make bundle
/Users/camilamacedo86/go/src/github.com/operator-framework/operator-sdk/testdata/go/v3/memcached-operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate kustomize manifests --interactive=false -q
cd config/manager && /Users/camilamacedo86/go/src/github.com/operator-framework/operator-sdk/testdata/go/v3/memcached-operator/bin/kustomize edit set image controller=controller:latest
/Users/camilamacedo86/go/src/github.com/operator-framework/operator-sdk/testdata/go/v3/memcached-operator/bin/kustomize build config/manifests | operator-sdk generate bundle -q --overwrite --version 0.0.1  
Error: remove operation does not apply: doc is missing path: "/spec/template/spec/containers/1/volumeMounts/0": missing value

Describe the solution you'd like.

Know what changes in the scaffolds for Golang-based language are required to use kustomize v4.

How to reproduce the scenario:

a) Update the Makefile of the Memcached sample

KUSTOMIZE_VERSION ?= v3.8.7
to use kustomize 4.5.5

b) Then, run make bundle

You will check the error:

Error: remove operation does not apply: doc is missing path: "/spec/template/spec/containers/1/volumeMounts/0": missing valueINFO[0000] Creating bundle.Dockerfile                   INFO[0000] Creating bundle/metadata/annotations.yaml    INFO[0000] Bundle metadata generated suceessfully       make: *** [Makefile:189: bundle] Error 1

/language go

@openshift-ci openshift-ci bot added the language/go Issue is related to a Go operator project label Jun 23, 2022
@camilamacedo86 camilamacedo86 changed the title Support Apple Silicon with kustomize v4 Know what changes in the scaffolds for Golang-based language are required to use kustomize v4 (make bundle does not work with) Jun 23, 2022
@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Jun 23, 2022

@jberkhahn jberkhahn added needs discussion priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jun 27, 2022
@jberkhahn jberkhahn added this to the v1.24.0 milestone Jun 27, 2022
@everettraven
Copy link
Contributor

@camilamacedo86 During the triage meeting we felt it would be good to add needs discussion so that this issue can be used to discuss what changes we will need to make throughout Operator SDK to support Kustomize v4. We felt it would allow getting as much input as possible and deciding what the next steps that need to be taken are. I am planning to spend some time looking into this and putting together my thoughts in a comment later.

@camilamacedo86 camilamacedo86 changed the title Know what changes in the scaffolds for Golang-based language are required to use kustomize v4 (make bundle does not work with) Know what changes in the scaffolds for Golang operators or SDK bundle generate command are required to use kustomize v4 (make bundle does not work with) Jun 27, 2022
@camilamacedo86
Copy link
Contributor Author

Hi @everettraven,

Thank you for sharing it with me. That is great you will look at that.

I just would like to share that the idea of this task is we identify what/why the make bundle when webhooks are scaffold with kustomize v4 does not work first then we can move with the other steps.

Se can use the CLI API to know what plugin key was used to do the scaffold and then perform the code that works with. (For example, as we do with the legacy layout go/v2, see: here). We will probably need to use it in the helper's plugin.

@everettraven
Copy link
Contributor

I spent a bit of time this morning looking into this a little bit more. From what I can tell, this seems to be related to the ordering in which the patches are occurring when using kustomize. I had a bit of a start to a solution but it seems like there needs to be some changes in the way the patches are created during the scaffolding process of the kustomize/v1 plugin. It seems like patches are applied in the following order: patchesJson6902, patchesStrategicMerge, patches More info

I need to do a little bit more digging when I have a bit more time, but from what I can tell so far this may need to be a change to the kustomize/v1 plugin upstream in Kubebuilder.

I will post more updates here as I find out more.

@camilamacedo86
Copy link
Contributor Author

I could analyse this one and find why the make bundle is not working with kustomize v4x:

By running kustomize build config/default with kustomize version 3.8.1 and 4.5.5 we can check that the containers on the Deployment are sorted in another order. Then, note that to remove the cert volumes when we build the CSV because it does not work with OLM we do the patch:

# [WEBHOOK] To enable webhooks, uncomment all the sections with [WEBHOOK] prefix.
# Do NOT uncomment sections with prefix [CERTMANAGER], as OLM does not support cert-manager.
# These patches remove the unnecessary "cert" volume and its manager container volumeMount.
patchesJson6902:
- target:
group: apps
version: v1
kind: Deployment
name: controller-manager
namespace: system
patch: |-
# Remove the manager container's "cert" volumeMount, since OLM will create and mount a set of certs.
# Update the indices in this path if adding or removing containers/volumeMounts in the manager's Deployment.
- op: remove
path: /spec/template/spec/containers/1/volumeMounts/0
# Remove the "cert" volume, since OLM will create and mount a set of certs.
# Update the indices in this path if adding or removing volumes in the manager's Deployment.
- op: remove
path: /spec/template/spec/volumes/0

However, the second container with 4.5.5 is thekube-rbac-proxy and is no longer the manager. So the target /spec/template/spec/containers/1/volumeMounts/0 is not found. The target with Kustomize v4 will be /spec/template/spec/containers/0/volumeMounts/0.

⚠️ Untile we merge the PR to fix it change the target /spec/template/spec/containers/1/volumeMounts/0 in the config/manifests scaffold will NOT work. The make bundle target calls the command operator-sdk generate kustomize manifests and the "subcommand" kustomize is overwriting the file content.

NOTE It is not a breaking change described in the Kustomize Release Notes. They do not have a migration guide from v3 to v4 but I could not find it on docs either.

c/c @everettraven

@asmacdo asmacdo modified the milestones: v1.24.0, v1.25.0 Sep 21, 2022
@everettraven
Copy link
Contributor

Verified that this works as expected after the PR @camilamacedo86 mentioned in the above comment

The default go/v3 plugin still uses Kustomize v3.8.7 while the new go/v4-alpha plugin uses Kustomize v4.5.5. I tested creating the memcached-operator with both plugins and running make bundle. Both the go/v3 and go/v4-alpha plugin resulted in successful bundle generation.

Closing since this is now resolved. Thanks @camilamacedo86 for resolving this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language/go Issue is related to a Go operator project needs discussion priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

4 participants