check `initContainers` for the kubernetes sidecar implementation #8986

pritidesai · 2025-08-21T15:53:35Z

Changes

This code checks if all containers in a Pod, including init containers, have completed their execution before marking a TaskRun as complete when kubernetes native sidecar is enabled. It's part of the TaskRun status determination logic.

/kind bug

Closes #8987

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
Has Tests included if any functionality added or changed
pre-commit Passed
Follows the commit message standard
Meets the Tekton contributor standards (including functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Updated the sidecar implementation to check the completion status of initContainers before marking the taskRun complete.

tekton-robot · 2025-08-21T16:06:01Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/pod/status.go	92.2%	92.4%	0.2

pritidesai · 2025-08-21T19:05:35Z

/retest

pritidesai · 2025-08-21T20:22:32Z

With these changes:

kubectl get tr  dind-sidecar-cddsd -o json | jq .status.sidecars
[
  {
    "container": "sidecar-server",
    "imageID": "docker.io/library/docker@sha256:74e78208fc18da48ddf8b569abe21563730845c312130bd0f0b059746a7e10f5",
    "name": "server",
    "terminated": {
      "containerID": "containerd://aa7e848e7949b614ad853f4fbac749be579b0f3521f8c3b6d107b4d780eeac82",
      "exitCode": 0,
      "finishedAt": "2025-08-21T20:21:39Z",
      "reason": "Completed",
      "startedAt": "2025-08-21T20:20:40Z"
    }
  }
]

tekton-robot · 2025-08-21T20:26:25Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/pod/status.go	92.2%	92.4%	0.2

pritidesai · 2025-08-22T00:01:24Z

/retest

pritidesai · 2025-08-22T00:04:33Z

/cc @kgcarr @say5

Tagging contributors who have previously worked on this feature for visibility and input.

tekton-robot · 2025-08-22T00:04:36Z

@pritidesai: GitHub didn't allow me to request PR reviews from the following users: kgcarr, say5.

Note that only tektoncd members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @kgcarr @say5

Tagging contributors who have previously worked on this feature for visibility and input.

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.

waveywaves

This looks great. The feature is very useful, and it makes sense to take the initContainer statuses into account when we think of the overall completion status. This implementation is closely tied to the sidecar feature right now, it might be better to have this decoupled into another config setting and a annotation which would help users disable this feature if necessary.

cc @twoGiants @vdemeester

pkg/pod/status.go

pkg/pod/status_test.go

AlanGreene · 2025-08-26T11:24:49Z

it might be better to have this decoupled into another config setting and a annotation which would help users disable this feature if necessary.

I'm not sure I agree, this is a bug fix for the existing feature rather than adding a new feature.

Without this check, the TaskRun status is incorrect / incomplete as it does not reflect an accurate status for the sidecars.

This code checks if all containers in a Pod, including init containers, have completed their execution before marking a TaskRun as complete when kubernetes native sidecar is enabled. It's part of the TaskRun status determination logic. Signed-off-by: Priti Desai <pdesai@us.ibm.com>

tekton-robot · 2025-08-26T16:45:21Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/pod/status.go	92.2%	92.4%	0.2

pritidesai · 2025-08-26T17:12:27Z

/retest

pritidesai · 2025-08-26T18:12:15Z

@waveywaves, this is ready for review - let me know if you have any feedback, appreciate your time!

afrittoli

Thanks, this looks good to me
/approve

afrittoli · 2025-08-26T21:17:02Z

pkg/pod/status.go

+	// When EnableKubernetesSidecar is true, we need to ensure all init containers
+	// are completed before considering the taskRun complete, in addition to the regular containers.
+	// This is because sidecars in Kubernetes can keep running after the main containers complete.
+	if config.FromContextOrDefaults(ctx).FeatureFlags.EnableKubernetesSidecar {


NIT: I wonder if this could be checked in all cases (even if sidecar is not enabled).
That said, having the check helps reducing the scope of the patch and makes it a better candidate for backporting.

tekton-robot · 2025-08-26T21:17:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [afrittoli]

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

waveywaves · 2025-08-27T14:41:03Z

Without this check, the TaskRun status is incorrect/incomplete as it does not reflect an accurate status for the sidecars.

@AlanGreene makes sense, was thinking about this in a more holistic way, where this initContainer completion is something we should add apart from just checking it for the sidecar. @afrittoli also hinted at this here. I will capture this in a separate issue for now.

pritidesai · 2025-08-27T18:05:40Z

Without this check, the TaskRun status is incorrect/incomplete as it does not reflect an accurate status for the sidecars.

@AlanGreene makes sense, was thinking about this in a more holistic way, where this initContainer completion is something we should add apart from just checking it for the sidecar. @afrittoli also hinted at this here. I will capture this in a separate issue for now.

yes, lets create a separate issue. All tekton internal processing runs as initContainers when Kubernetes native sidecars are not enabled. This setup has been working as intended since its implementation, and we haven't received any reports indicating issues with its design in this context.

Traditionally, we have determined the taskRun status based on steps and this PR does not change that behaviour. With the new issue created, we can consider including the sidecar's status in the taskRun.

pritidesai · 2025-08-28T14:14:52Z

@vdemeester @waveywaves I will appreciate it if you could review the changes in this PR, looking for one more review, thanks!

waveywaves · 2025-08-29T15:44:51Z

/lgtm

this looks good, I am still working on a issue for this, shall create it soon

pritidesai · 2025-08-29T15:51:09Z

/cherry-pick release-v1.3.x

pritidesai · 2025-08-29T16:20:27Z

/cherrypick release-v1.3.x

github-project-automation bot added this to Tekton Community Roadmap Aug 21, 2025

github-project-automation bot moved this to Todo in Tekton Community Roadmap Aug 21, 2025

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. labels Aug 21, 2025

tekton-robot requested review from afrittoli and twoGiants August 21, 2025 15:53

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 21, 2025

pritidesai force-pushed the native-sidecar-in-tr-status-1 branch from 5d140c4 to 1aed4fc Compare August 21, 2025 20:15

waveywaves self-assigned this Aug 22, 2025

waveywaves reviewed Aug 26, 2025

View reviewed changes

pkg/pod/status.go Show resolved Hide resolved

pkg/pod/status_test.go Show resolved Hide resolved

pritidesai force-pushed the native-sidecar-in-tr-status-1 branch from 1aed4fc to dc151a7 Compare August 26, 2025 16:29

afrittoli reviewed Aug 26, 2025

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 26, 2025

vdemeester self-assigned this Aug 27, 2025

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 29, 2025

tekton-robot merged commit 0cb253c into tektoncd:main Aug 29, 2025
42 of 48 checks passed

github-project-automation bot moved this from Todo to Done in Tekton Community Roadmap Aug 29, 2025

pritidesai mentioned this pull request Aug 29, 2025

[release-v1.3.x] check initContainers for kubernetes sidecar implementation #8997

Merged

8 tasks

Uh oh!

check initContainers for the kubernetes sidecar implementation #8986

check initContainers for the kubernetes sidecar implementation #8986

Uh oh!

Conversation

pritidesai commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Submitter Checklist

Release Notes

Uh oh!

tekton-robot commented Aug 21, 2025

Uh oh!

pritidesai commented Aug 21, 2025

Uh oh!

pritidesai commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tekton-robot commented Aug 21, 2025

Uh oh!

pritidesai commented Aug 22, 2025

Uh oh!

pritidesai commented Aug 22, 2025

Uh oh!

tekton-robot commented Aug 22, 2025

Uh oh!

waveywaves left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AlanGreene commented Aug 26, 2025

Uh oh!

tekton-robot commented Aug 26, 2025

Uh oh!

pritidesai commented Aug 26, 2025

Uh oh!

pritidesai commented Aug 26, 2025

Uh oh!

afrittoli left a comment

Choose a reason for hiding this comment

Uh oh!

afrittoli Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

tekton-robot commented Aug 26, 2025

Uh oh!

waveywaves commented Aug 27, 2025

Uh oh!

pritidesai commented Aug 27, 2025

Uh oh!

pritidesai commented Aug 28, 2025

Uh oh!

waveywaves commented Aug 29, 2025

Uh oh!

Uh oh!

pritidesai commented Aug 29, 2025

Uh oh!

pritidesai commented Aug 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

check `initContainers` for the kubernetes sidecar implementation #8986

check `initContainers` for the kubernetes sidecar implementation #8986

pritidesai commented Aug 21, 2025 •

edited

Loading

pritidesai commented Aug 21, 2025 •

edited

Loading