search

kubernetes-sigs / kubebuilder

Kubebuilder - SDK for building Kubernetes APIs using CRDs
http://book.kubebuilder.io
Apache License 2.0
7.9k stars 1.45k forks source link

kubebuilder make install fails #3460

Closed rbroggi closed 1 year ago

rbroggi commented 1 year ago

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

Extra Labels

/kind documentation

k8s-ci-robot commented 1 year ago

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

In response to [this](https://github.com/kubernetes-sigs/kubebuilder/issues/3460): >### What broke? What's expected? > >Following the kubebuilder book, the [make install](https://github.com/kubernetes-sigs/kubebuilder/blob/master/docs/book/src/cronjob-tutorial/running.md?plain=1#L16) step fails with the error message: > >```txt >The CustomResourceDefinition "cronjobs.batch.tutorial.kubebuilder.io" is invalid: metadata.annotations: Too long: must have at most 262144 bytes >``` > >As described in [this post](https://medium.com/pareture/kubectl-install-crd-failed-annotations-too-long-2ebc91b40c7d) 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](https://book.kubebuilder.io/) up to the `make install` in [this book section](https://book.kubebuilder.io/cronjob-tutorial/running.html). > > > >### 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 > >```yaml >- 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](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
camilamacedo86 commented 1 year ago

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: https://github.com/kubernetes-sigs/kubebuilder/blob/66f3fbc7ae349e8ee31f1c6eb8d55d5b1c78ce52/docs/book/src/multiversion-tutorial/testdata/project/Makefile#L41-L47

Would you like to contribute with this one?

RafalSkolasinski commented 1 year ago

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

https://github.com/kubernetes-sigs/kubebuilder/blob/66f3fbc7ae349e8ee31f1c6eb8d55d5b1c78ce52/docs/book/src/cronjob-tutorial/testdata/project/Makefile#L47-L49

camilamacedo86 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

I can try and do that...

RafalSkolasinski commented 1 year ago

Maybe also worth adding it to FAQ?

rbroggi commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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: https://github.com/kubernetes-sigs/kubebuilder/issues/1140#issuecomment-1299307417

Therefore, what we can do here is:

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 commented 1 year ago
  • 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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

I would like to take this up /assign

Sajiyah-Salat commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.