Skip to content

perf: avoid unnecessary apiserver calls in TaskRun done path (stopSidecars on every resync) #9755

@vdemeester

Description

@vdemeester

Problem

When reconciling a completed TaskRun, the done path in pkg/reconciler/taskrun/taskrun.go calls stopSidecars on every resync. This function makes a live GET call to the apiserver (kubeclient.CoreV1().Pods().Get()) to fetch the pod and check whether sidecars are still running — even when they've already been stopped on a previous reconcile.

At scale with many completed TaskRuns retained (e.g. for dashboard visibility), this generates unnecessary apiserver load on every resync cycle.

Additionally, when EnableKubernetesSidecar is enabled, the done path also calls Discovery().ServerVersion() on every resync to determine whether native sidecars are supported.

Per-resync cost for a completed TaskRun (current)

Operation API call? Notes
Discovery().ServerVersion() Yes (may be cached by discovery client) Only when EnableKubernetesSidecar=true
stopSidecarsPods().Get() Yes (live apiserver call, not through informer) Fetches pod to check if still running
stopSidecarsPods().Patch() Conditional Only if sidecars still running

Proposed Fix

Before calling stopSidecars, check whether sidecars have already been stopped using the TaskRun's own status (no API call needed):

  • If tr.Status.Sidecars is empty → no sidecars to stop, skip entirely
  • If all entries in tr.Status.Sidecars have Terminated != nil → sidecars already stopped, skip
  • Otherwise → call stopSidecars as usual

This would eliminate the live Pods().Get() call on every resync of a completed TaskRun whose sidecars have already been stopped.

Subtlety

Unlike PipelineRun cleanup (which is purely idempotent), stopSidecars may need multiple attempts if the sidecar stop fails with a concurrent modification error (it requeues after 1s). So we can't remove it from the done path entirely — we just need to guard it with a status check.

Context

This is a follow-up to #9706 which applied similar optimizations to the PipelineRun reconciler (moving IsDone() earlier, eager cleanup, skipping labels/annotations updates). The SetDefaults removal in the TaskRun done path was already included in that PR.

/kind cleanup

Metadata

Metadata

Assignees

Labels

help wantedDenotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.kind/cleanupCategorizes issue or PR as related to cleaning up code, process, or technical debt.meaty-juicy-coding-workThis task is mostly about implementation!!! And docs and tests of course but that's a given

Type

Projects

Status

In Progress

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions