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

[v0.44.x] conversion can fail if the PipelineRun is too big with embedded-status set to both (or full) #6561

Closed
vdemeester opened this issue Apr 19, 2023 · 6 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@vdemeester
Copy link
Member

Expected Behavior

Using pipeline 0.44 and embedded-status set to both or full, it should be possible to list PipelineRun with v1 and not runing into issues.

Actual Behavior

Depending on the size of the PipelineRun (and mainly taskruns.status), it can/will fail.

tkn pr ls                                                                                              
Failed to list objects from pipelines-ci namespace 
Error: failed to list PipelineRuns from namespace pipelines-ci: conversion webhook for tekton.dev/v1beta1, Kind=PipelineRun returned invalid metadata: metadata.annotation: Too long: must have at most 262144 bytes

This shows it fail using tkn (which uses v1 API in that case) but it would fail using kubectl get as well, as the problem is happening in the conversion webhook.

Steps to Reproduce the Problem

  1. Install v0.44.x and set embedded-status to both (or full)
  2. Create a big PipelineRun and wait for it to finish
  3. kubectl get pipelinerun my-pipelinerun to see it fail
  4. kubectl get pipelinerun.v1beta1.tekton.dev my-pipelinerun would succeed

Additional Info

  • Kubernetes version:

Any that runs Pipeline 0.44.x

  • Tekton Pipeline version:
v0.44.x

Reasons and possible solutions

This happens because we are using annotations to convert from v1beta1 to v1 for fields that do not exists. We can run into a case where the size of the field "marshalled" in that annotation will be too big to be "stored" in an annotation.

Possible fix: compress and encode in base64 the marshalled field in the annotations to make them take less places. It should be "fine" to not support the current way as, the source-of-truth (aka the store version) is v1beta1 and still has the field.

/cc @JeromeJu @lbernick

@vdemeester vdemeester added the kind/bug Categorizes issue or PR as related to a bug. label Apr 19, 2023
@vdemeester vdemeester added this to the Pipelines v0.48 milestone May 2, 2023
@vdemeester vdemeester removed this from the Pipelines v0.48 milestone May 2, 2023
@pritidesai
Copy link
Member

/assign @vdemeester
/assign @JeromeJu

@concaf
Copy link
Contributor

concaf commented May 4, 2023

/assign @concaf
spoke with @vdemeester offline to collaborate on this 😇

@concaf
Copy link
Contributor

concaf commented Jul 26, 2023

👋🏼 something to keep in mind is that #6968 mitigates this issue to an extent, however due to annotations size limit (all annotations keys and values added together) of 256 kB, we cannot guarantee that compression will fix this entirely.

how do we want to move forward in this case? should we explore storing this somewhere else other than annotations?

@vdemeester
Copy link
Member Author

cc @tektoncd/core-maintainers

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 7, 2023
@vdemeester vdemeester removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 19, 2024
@vdemeester
Copy link
Member Author

I am going to go ahead and close this. 0.44.x is an old version, and the issue discussed shouldn't arise anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

5 participants