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 webhook is misbehaving on kubectl patch #6443

Closed
vdemeester opened this issue Mar 28, 2023 · 3 comments · Fixed by #6450
Closed

[v0.44.x] Conversion webhook is misbehaving on kubectl patch #6443

vdemeester opened this issue Mar 28, 2023 · 3 comments · Fixed by #6450
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now.

Comments

@vdemeester
Copy link
Member

Expected Behavior

When doing a kubectl patch to add an annotation on a v1 type (like PipelineRun), I expect it to keep the existing annotations.

Actual Behavior

When doing a kubectl patch to add an annotation on a v1 type (like PipelineRun), the existing annotations are gone, which is a big problem when using embedded-status: both, as tekton.dev/v1beta1Taskruns being gone, we are loosing some data.

Steps to Reproduce the Problem

  1. Deploy v0.44.x on a cluster
  2. Edit the feature-flags configmap to set embedded-status: both
  3. Execute a PipelineRun, kubectl create -f ./examples/v1beta1/pipelineruns/pipelinerun.yaml
  4. Do k get v1beta1.pipelinerun.tekton.dev/demo-pipeline-run-1 -o yaml once finished, and look at the status.taskRuns being populated
  5. Patch the PipelineRun using kubectl patch pr demo-pipeline-run-1 --type='json' -p='[{"op": "add", "path": "/metadata/annotations/foo", "value":"bar"}]'
  6. Get the v1beta1 PipelineRun using k get v1beta1.pipelinerun.tekton.dev/demo-pipeline-run-1 -o yaml and see that we do not have the status.taskRuns anymore

Additional Info

  • Tekton Pipeline version:

    Output of tkn version or kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'

v0.44.x
@vdemeester vdemeester added the kind/bug Categorizes issue or PR as related to a bug. label Mar 28, 2023
@vdemeester
Copy link
Member Author

/assign

@vdemeester vdemeester added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Mar 28, 2023
@vdemeester
Copy link
Member Author

Quick note : the same behavior applies when trying to patch labels or anything really (for example, the timeout field) — using kubectl path or kubectl edit.

@vdemeester
Copy link
Member Author

Alright, I did figure something out it seems. I did add a log to print what "config" it sees for embedded-status. And it turns out, even if the value in the configmap is both, the one that the webhook prints is minimal.

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. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant