Skip to content

Update metadata:annotations: Too long error string detection #6480

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 3, 2025
Merged

Update metadata:annotations: Too long error string detection #6480

merged 2 commits into from
Jan 3, 2025

Conversation

andrew-farries
Copy link
Contributor

Update the regular expression used to match the Kubernetes metadata.annotations: Too long API error returned when a resource annotation exceeds a maximum size.

The error message changed in Kubernetes v1.32: kubernetes/kubernetes#128553

The change of error string caused Tilt's logic to fall back to create or replace the resource to fail:

tilt/internal/k8s/client.go

Lines 574 to 583 in 875606f

result, err := k.applyEntity(ctx, entity)
if err != nil {
msg, match := maybeTooLargeError(err)
if match {
fallback = true
logger.Get(ctx).Infof("Updating %q failed: %s", entity.Name(), msg)
logger.Get(ctx).Infof("Attempting to create or replace")
result, err = k.createOrReplaceEntity(ctx, entity)
}
}

Updating the regular expression ensures that resources with annotations exceeding the maximum size can still be applied in Kubernetes >= v1.32.

Verification

Take a large CRD, such as the one in this Gist and save it as install.yaml.

Create the following Tiltfile:

k8s_yaml('install.yaml')

Run tilt up against a Kubernetes v1.32 cluster, such as one created by kind v0.26.0.

On master tilt up fails:

uncategorized │ Initial Build
uncategorized │ STEP 1/1 — Deploying
uncategorized │      Applying YAML to cluster
uncategorized │      Tried to apply objects to cluster:
uncategorized │        → envoyproxies.gateway.envoyproxy.io:customresourcedefinition
uncategorized │
uncategorized │ ERROR: Build Failed: CustomResourceDefinition.apiextensions.k8s.io "envoyproxies.gateway.envoyproxy.io" is invalid: metadata.annotations: Too long: may not be more than 262144 bytes

With the changes from this PR tilt up succeeds:

uncategorized │ Initial Build
uncategorized │ STEP 1/1 — Deploying
uncategorized │      Applying YAML to cluster
uncategorized │      Updating "envoyproxies.gateway.envoyproxy.io" failed: CustomResourceDefinition.apiextensions.k8s.io "envoyproxies.gateway.envoyproxy.io" is invalid: metadata.annotations: Too long: may not be more than 262144 bytes
uncategorized │      Attempting to create or replace
uncategorized │      Updating "envoyproxies.gateway.envoyproxy.io" succeeded!

@andrew-farries
Update MetadataAnnotationsTooLongRe
42c7e67 
Update the regexp to match both pre and post Kubernetes 1.32 error
messages.

Signed-off-by: Andrew Farries <andrew.farries@xata.io>
@andrew-farries
Update TestUpsertAnnotationTooLong
de5a8cc 
Add both error messages, pre and post Kubernetes v1.32.

Signed-off-by: Andrew Farries <andrew.farries@xata.io>
@andrew-farries andrew-farries force-pushed the update-too-long-detection branch from c8afa54 to de5a8cc Compare January 3, 2025 11:09
@andrew-farries
Copy link
Contributor Author

Rebased to add Signed-off-by to each commit.

nicks
nicks approved these changes Jan 3, 2025
View reviewed changes
Copy link
Member

@nicks nicks left a comment

Choose a reason for hiding this comment

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

LGTM!

Nice find!

@nicks nicks merged commit ab51f91 into tilt-dev:master Jan 3, 2025
8 checks passed
@nicks
Copy link
Member

nicks commented Jan 3, 2025

i'm going to try to do some more k8s 1.32 upgrades today then push this out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicks nicks nicks approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andrew-farries @nicks