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

kubebuilder make install fails #3460

Closed
rbroggi opened this issue Jun 16, 2023 · 17 comments · Fixed by #3543
Closed

kubebuilder make install fails #3460

rbroggi opened this issue Jun 16, 2023 · 17 comments · Fixed by #3543
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation.

Comments

@rbroggi
Copy link

rbroggi commented Jun 16, 2023

What broke? What's expected?

Following the kubebuilder book, the make install step fails with the error message:

The CustomResourceDefinition "cronjobs.batch.tutorial.kubebuilder.io" is invalid: metadata.annotations: Too long: must have at most 262144 bytes

As described in this post the issue derives from the auto-generated annotation when running kubectl apply.
Running the same command with kubectl create or kubectl replace works.

I was wondering how to correct this issue. Possible approaches:

  1. easy, workaround: insert a warning into the book stating this problem and telling the user to edit the make install;
  2. easy, workaround, fragile: make the install target slightly more intelligent and dynamically choose apply, create or replace depending on whether the CRDs exist and on the possible error;
  3. harder: solve the problem on kubectl apply - here a further discussion needs to be triggered on whether we even want to solve this issue and what impacts that could generate (not sure it would be backwards compatible)
  4. within kubectl apply make a more detailed error and suggest usage of create or replace commands instead.

Reproducing this issue

Follow the kubebuilder book up to the make install in this book section.

KubeBuilder (CLI) Version

Version: main.version{KubeBuilderVersion:"v3.10.0-110-gd9c14df9", KubernetesVendor:"unknown", GitCommit:"d9c14df9d84fafd08c9ec88441e9ce218ad6effa", BuildDate:"2023-06-16T09:47:40Z", GoOs:"darwin", GoArch:"arm64"}

PROJECT version

version: "3"

Plugin versions

- go.kubebuilder.io/v4

Other versions

  • go version go1.20.4 darwin/arm64

  • sigs.k8s.io/controller-runtime v0.15.0

Extra Labels

/kind documentation

@rbroggi rbroggi added the kind/bug Categorizes issue or PR as related to a bug. label Jun 16, 2023
@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 16, 2023
@k8s-ci-robot
Copy link
Contributor

@rbroggi: The label(s) kind/documentation,, kind//kind cannot be applied, because the repository doesn't have them.

In response to this:

What broke? What's expected?

Following the kubebuilder book, the make install step fails with the error message:

The CustomResourceDefinition "cronjobs.batch.tutorial.kubebuilder.io" is invalid: metadata.annotations: Too long: must have at most 262144 bytes

As described in this post the issue derives from the auto-generated annotation when running kubectl apply.
Running the same command with kubectl create or kubectl replace works.

I was wondering how to correct this issue. Possible approaches:

  1. easy, workaround: insert a warning into the book stating this problem and telling the user to edit the make install;
  2. easy, workaround, fragile: make the install target slightly more intelligent and dynamically choose apply, create or replace depending on whether the CRDs exist and on the possible error;
  3. harder: solve the problem on kubectl apply - here a further discussion needs to be triggered on whether we even want to solve this issue and what impacts that could generate (not sure it would be backwards compatible)
  4. within kubectl apply make a more detailed error and suggest usage of create or replace commands instead.

Reproducing this issue

Follow the kubebuilder book up to the make install in this book section.

KubeBuilder (CLI) Version

Version: main.version{KubeBuilderVersion:"v3.10.0-110-gd9c14df9", KubernetesVendor:"unknown", GitCommit:"d9c14df9d84fafd08c9ec88441e9ce218ad6effa", BuildDate:"2023-06-16T09:47:40Z", GoOs:"darwin", GoArch:"arm64"}

PROJECT version

version: "3"

Plugin versions

- go.kubebuilder.io/v4

Other versions

  • go version go1.20.4 darwin/arm64

  • sigs.k8s.io/controller-runtime v0.15.0

Extra Labels

/kind documentation, /kind feature

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.

@rbroggi rbroggi closed this as completed Jun 16, 2023
@rbroggi rbroggi reopened this Jun 16, 2023
@camilamacedo86
Copy link
Member

HI @rbroggi,

If you look at the code generate for the sample of the tutorial the target to generate the manifest has a customization to avoid this scenario. I think we could add a note in the tutorial explaining it.

See:

.PHONY: manifests
manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
# Note that the option maxDescLen=0 was added in the default scaffold in order to sort out the issue
# Too long: must have at most 262144 bytes. By using kubectl apply to create / update resources an annotation
# is created by K8s API to store the latest version of the resource ( kubectl.kubernetes.io/last-applied-configuration).
# However, it has a size limit and if the CRD is too big with so many long descriptions as this one it will cause the failure.
$(CONTROLLER_GEN) rbac:roleName=manager-role crd:maxDescLen=0 webhook paths="./..." output:crd:artifacts:config=config/crd/bases

Would you like to contribute with this one?

@camilamacedo86 camilamacedo86 added kind/documentation Categorizes issue or PR as related to documentation. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. and removed kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. labels Jul 3, 2023
@RafalSkolasinski
Copy link

This is missing from cronjob main tutorial though and is also nor is it I believe mentioned in the book itself so far.

.PHONY: manifests
manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases

@camilamacedo86
Copy link
Member

Hi @RafalSkolasinski,

This is missing from cronjob main tutorial though and is also nor is it I believe mentioned in the book itself so far.

The code above is from the sample of the cronjob itlsef.
What seems missing is to add it to the docs via a note: (give a look in the class:note annotations in the docs)
So, would you like to contribute by adding this one?

@RafalSkolasinski
Copy link

So would you see it as step instructing the reader of kubebuilder book to go and edit the Makefile just after it was generated by Kubebuilder to make sure it includes the crd:maxDescLen=0?

@rbroggi
Copy link
Author

rbroggi commented Jul 3, 2023

I can try and do that...

@RafalSkolasinski
Copy link

Maybe also worth adding it to FAQ?

@rbroggi
Copy link
Author

rbroggi commented Jul 3, 2023

To summarise:

  1. The flag is not present in all Makefiles of the cronjob tutorial;
  2. It doesn't seem to be present in any other instance of the manifests make target which means that the issue will still happen on generated scaffolds, right?

@derecknowayback
Copy link

derecknowayback commented Jul 6, 2023

I met this problem too, and I tried to add crd:maxDescLen=0 but it didn't work. So could you tell me what is the solution? Or did I miss something else?

@camilamacedo86
Copy link
Member

camilamacedo86 commented Jul 7, 2023

So would you see it as step instructing the reader of kubebuilder book to go and edit the Makefile just after it was generated by Kubebuilder to make sure it includes the crd:maxDescLen=0?

See the comment with the explanation:

# Note that the option maxDescLen=0 was added in the default scaffold in order to sort out the issue 
     # Too long: must have at most 262144 bytes. By using kubectl apply to create / update resources an annotation 
     # is created by K8s API to store the latest version of the resource ( kubectl.kubernetes.io/last-applied-configuration). 
     # However, it has a size limit and if the CRD is too big with so many long descriptions as this one it will cause the failure. 

K8s has a limitation regards the size of CRDs. Please see: #1140 (comment)

Therefore, what we can do here is:

  • a) Create a NOTE explaining the k8s size limitation
  • b) Explain that the workaround is to not kubectl apply when installing OR to set maxDescLen on controller-tools when generating the CRD
  • c) Explain that controller-gen introduce maxDescLen to work around this problem until we get rid of client-side apply
  • d) See the PR ✨ Generation of typed apply clients controller-tools#536

When I say create a note? See the docs and look for <aside class="note">.
Therefore we could add this info via a NOTE

<aside class="note">

<h1>To workaround the error `Too long: must have at most 262144 bytes`</h1>

Add: Why the error occur (k8s limitation)
Add: The workaround (use apply or maxDescLen)

</aside>

I met this problem too, and I tried to add crd:maxDescLen=0 but it didn't work. So could you tell me what is the solution? Or did I miss something else?

@derecknowayback:

You need to add the option and then run make manifests to generate the CRD with.
If remove the description of the CRDs did not sort out the problem and it is bigger than 1 GB then you need to check why to solve it.

@rbroggi
Copy link
Author

rbroggi commented Jul 7, 2023

  • xplain that the workaround is to not kubectl apply when installing OR to set maxDescLen on controller-tools when generating the CRD

Hey @camilamacedo86 , thank you very much for your detailed explanation!

I will try to follow up with that. Something I would like to disambiguate is:

  1. Why the option above was put only in one Makefile:
❯ rg -l 'crd:maxDescLen=0'
docs/book/src/multiversion-tutorial/testdata/project/Makefile
❯ rg -l "^manifests:"
pkg/plugins/golang/v3/commons.go
pkg/plugins/golang/declarative/v1/scaffolds/internal/templates/channel.go
pkg/plugins/golang/v3/scaffolds/internal/templates/makefile.go
pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go
pkg/plugins/golang/v2/scaffolds/internal/templates/makefile.go
docs/book/src/reference/generating-crd.md
docs/book/src/component-config-tutorial/testdata/project/Makefile
docs/book/src/reference/makefile-helpers.md
docs/book/src/cronjob-tutorial/testdata/project/Makefile
testdata/project-v4-with-deploy-image/Makefile
testdata/project-v4/Makefile
testdata/project-v2/Makefile
testdata/project-v3/Makefile
testdata/project-v4-multigroup/Makefile
testdata/project-v4-declarative-v1/Makefile
testdata/project-v4-declarative-v1/channels/stable
testdata/project-v4-with-grafana/Makefile
docs/book/src/multiversion-tutorial/testdata/project/Makefile

Should we also put the options in all the Makefiles on top of the observation that you suggested above?

Thx 😄

@camilamacedo86
Copy link
Member

Hi @rbroggi,

I think we should only add a note info to explain when/why and how workaround the scenario. We should not add the option crd:maxDescLen=0 in the default scaffold because it is an workaround. See that not all scenarios will face that. The error only occurs if you create a CRD where it has more than the limit size defined in k8s api which usually is possible to be sorted out if you remove the descriptions from it to reduce the size.

So, as you can see it is not something that must or should be done by default.

I would either recommend you think a lot about the design of your APIs/CRDs and personally try to follow the DDD principle so that you have a better maintainability, re-usage of your solution and also you much avoid this issue as well.

@Sajiyah-Salat
Copy link
Contributor

Sajiyah-Salat commented Aug 7, 2023

Hey @camilamacedo86 is the solution you suggested above still valid?
should I work on this?
Also from suggestion looks like it will be a documentation contribution, right?
should we add a new doc page or just add a comment in code itself for workaround.

@Sajiyah-Salat
Copy link
Contributor

I would like to take this up
/assign

@Sajiyah-Salat
Copy link
Contributor

Hello @rbroggi As I will start working on this. Would you like to add anything? The plan is to add note as suggested by @camilamacedo86 in relatable doc page.

@Sajiyah-Salat
Copy link
Contributor

Hello @rbroggi @camilamacedo86 which would be the place to add this note.
Should we add it under make install or in a faq section.

@rbroggi
Copy link
Author

rbroggi commented Aug 19, 2023

Hello @rbroggi @camilamacedo86 which would be the place to add this note.
Should we add it under make install or in a faq section.

I'm guessing that what @camilamacedo86 had in mind was to have it close to the sections containing the shell commands where the simple kubectl apply embedded in the makefile fails.

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. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants