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

crd: reduce CRD size #498

Merged

Conversation

AlexanderYastrebov
Copy link
Member

@AlexanderYastrebov AlexanderYastrebov commented Jun 13, 2023

Why

When CRD is applied via kubectl apply -f docs/stackset_crd.yaml (aka client-side apply) kubectl stores
CRD content into the kubectl.kubernetes.io/last-applied-configuration annotation which has
size limit of 262144 bytes.
If the size of the annotation exceeds the limit, kubectl will fail with the following error:

The CustomResourceDefinition "stacksets.zalando.org" is invalid: metadata.annotations: Too long: must have at most 262144 bytes

See kubernetes/kubectl#712

The CRD contains a lot of descriptions for k8s.io types and controller-gen
does not allow to skip descriptions per field or per package,
see kubernetes-sigs/controller-tools#441

How

It removes descriptions starting at the deepest level of the yaml tree
until the size of the yaml converted to json is less than the maximum allowed annotation size.

@@ -1,4 +1,3 @@
---
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we need this and why controller-gen adds it, perhaps to enable concatenation via cat foo.yaml bar.yaml > baz.yaml

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to now have it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean not to have it?

Comment on lines +79 to +76
size -= len(d.value)
delete(d.property, "description")
Copy link
Member Author

@AlexanderYastrebov AlexanderYastrebov Jun 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we can do more sophisticated logic if necessary, e.g. trim description to a single sentence like in zalando-incubator/kubernetes-on-aws#5980

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this is not exactly precise as removing description also reduces json size by the len("description") plus quotes and colon. Strict implementation should recalculate actual size after removal by serializing into json.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting :)

@szuecs
Copy link
Member

szuecs commented Jun 13, 2023

Oh, oh:

When CRDs get too large, it becomes impossible to manage them with tools like Helm which store a compressed manifest in a Secret, which has a 1 MB limit.

@RomanZavodskikh
Copy link

In theory, if the limit of 262144 bytes would be increased, will we have a possibility to make descriptions full again?

@mikkeloscar
Copy link
Contributor

👍

@AlexanderYastrebov
Copy link
Member Author

@RomanZavodskikh

In theory, if the limit of 262144 bytes would be increased, will we have a possibility to make descriptions full again?

Yes.

Another options are: use server-side apply that does not need annotation or create/update CRDs via API instead of using kubectl apply. Both require changes to https://github.com/zalando-incubator/cluster-lifecycle-manager/

Also StackSet embeds descriptions for for k8s.io types which may grow over time and CRD may theoretically hit the limit of k8s object size (~1.5Mb).

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
@mikkeloscar
Copy link
Contributor

👍

1 similar comment
@AlexanderYastrebov
Copy link
Member Author

👍

@katyanna katyanna merged commit 6adce5e into zalando-incubator:master Jun 15, 2023
@AlexanderYastrebov AlexanderYastrebov deleted the hack/reduce-crd-size branch June 15, 2023 15:13
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Jun 16, 2023
See zalando-incubator/stackset-controller#498

Followup on #5980

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Jun 16, 2023
Stack and StackSet CRDs were update to not exceed annotation size limit,
see zalando-incubator/stackset-controller#498

Note that CRDs are not verbatim copies but contain conditional blocks.
Templates were commented to make CRDs valid yaml.

Followup on #5980

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Jun 16, 2023
Stack and StackSet CRDs were updated to not exceed annotation size limit,
see zalando-incubator/stackset-controller#498

Note that CRDs are not verbatim copies but contain conditional blocks.
Templates were commented to make CRDs valid yaml.

Followup on #5980

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Jun 19, 2023
Stack and StackSet CRDs were updated to not exceed annotation size limit,
see zalando-incubator/stackset-controller#498

Note that CRDs are not verbatim copies but contain conditional blocks.
Templates were commented to make CRDs valid yaml.

Followup on #5980

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Jun 19, 2023
Stack and StackSet CRDs were updated to not exceed annotation size limit,
see zalando-incubator/stackset-controller#498

Note that CRDs are not verbatim copies but contain conditional blocks.
Templates were commented to make CRDs valid yaml.

Followup on #5980

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants