Skip to content

Conversation

@prad9192
Copy link

@prad9192 prad9192 commented Nov 7, 2025

What

Changed StopSidecars() to use Patch() instead of Update() when stopping sidecar containers by replacing their images with the nop image.

Why

The original implementation used Update(), which requires an exact resourceVersion match. This causes 409 conflicts when the kubelet updates the pod status between our GET and UPDATE calls (which happens frequently as containers terminate).

This causes the below errors on the pod.

error stopping sidecars of Pod: Operation cannot be fulfilled on pods "...": 
the object has been modified; please apply your changes to the latest version

And the task runs fail with TaskRunResolutionFailed

image

Even though the task succeeded and sidecars eventually stop, the TaskRun gets marked as failed due to this race condition.
The fix uses JSON Patch (same pattern as UpdateReady() and CancelPod() in this file), which only patches the specific container image fields

Release Notes

Does this PR introduce a user-facing change?

Fixed race condition causing TaskRuns to fail with 409 conflict error when stopping sidecars. 
StopSidecars now uses Patch instead of Update to avoid conflicts with concurrent kubelet pod status updates.

@tekton-robot
Copy link
Collaborator

@prad9192: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tekton-robot tekton-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Nov 7, 2025
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 7, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: prad9192 / name: Pradeep Ashwathanarayan (434ad9a)

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign vdemeester after the PR has been reviewed.
You can assign the PR to them by writing /assign @vdemeester in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 7, 2025
@prad9192 prad9192 force-pushed the tekton-stop-sidecar-patch-instead-of-update branch from 934d222 to 434ad9a Compare November 7, 2025 01:22
@prad9192
Copy link
Author

prad9192 commented Nov 9, 2025

/retest

@waveywaves waveywaves added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants