Ignore context-canceled errors when logging drift detector shutdown failures

[pujitha24]

What this PR does:

Stops the kubernetes drift detector from logging failed to check application and failed to clean partially cloned repository at Error level when the failure is caused by the detector's context already being canceled (i.e. piped is shutting down). The underlying operations (checkApplication, gitRepo.CleanPath) still run exactly as before; only the resulting error is now suppressed when ctx.Err() != nil.

Why we need it:

When a piped agent shuts down, its context is canceled while the periodic drift detector may still be mid-flight. This causes in-flight operations to fail (e.g. kustomize being killed with signal: killed, or git commands failing with context canceled), and those expected, benign failures were being logged as Error, producing log spam / false alarms right at shutdown.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

• How are users affected by this change: No behavior change for users; this only removes two spurious Error-level log lines that piped previously emitted during a normal graceful shutdown. Cleanup and manifest-check operations still execute exactly as before.
• Is this breaking change: No
• How to migrate (if breaking change): N/A

Screenshots/Videos (for documentation or website changes):
N/A (no docs/.md changes)

---

Motivation: #5338 reports two spurious Error logs during piped shutdown: failed to clean partially cloned repository (from gitRepo.CleanPath) and failed to check application: ... (from checkApplication, e.g. via kustomize being killed with signal: killed, or a git command returning context canceled). A maintainer confirmed cleanup should still run, but its error (and the checkApplication error) can be ignored when caused by the context being canceled.

Approach: In pkg/app/piped/driftdetector/kubernetes/detector.go's check method, both call sites still invoke their operations unconditionally, but now only log the resulting error at Error level when ctx.Err() == nil. ctx.Err() != nil is checked directly (rather than errors.Is(err, context.Canceled)) because a process killed via exec.CommandContext on context cancellation (e.g. kustomize) returns signal: killed, which does not wrap context.Canceled in Go's os/exec implementation — checking the context directly reliably covers both error shapes reported in the issue.

Validation: Added TestCheck_ContextCanceled in pkg/app/piped/driftdetector/kubernetes/detector_test.go, which drives detector.check with a mocked git.Repo (gomock) whose CleanPath call fails and whose repository path lacks an application config file (making checkApplication fail quickly, independent of context state). With a non-canceled context both failures are logged as Error (2 log entries); with an already-canceled context, zero Error logs are produced, while the mocked CleanPath/Pull/GetLatestCommit calls are still asserted to have run. Verified this test fails (2 unwanted Error logs) against the old code and passes after the fix.

Commands run and their results:

• go test ./pkg/app/piped/driftdetector/kubernetes/... -run TestCheck_ContextCanceled -v — PASS (both subtests)
• go test ./pkg/app/piped/driftdetector/... -v — PASS (all packages, including previously-existing tests in ecs/lambda)
• go build ./pkg/app/piped/driftdetector/... — success
• go vet ./pkg/app/piped/driftdetector/... — clean
• golangci-lint run --config .golangci.yml ./pkg/app/piped/driftdetector/... — 0 issues

Not run: no dev-stack/e2e piped shutdown reproduction (this is a logging-only change in one function, fully covered by the targeted unit test above).

Note: master's own CI (go-test-completed) is currently green, so this PR is branched from a healthy base.

Scope: only pkg/app/piped/driftdetector/kubernetes/detector.go is changed, since it is the exact file the issue and the maintainer's comment point to, and it's the only drift detector that performs the CleanPath cleanup call. The terraform/ecs/lambda/cloudrun detectors have a similar checkApplication error-log line but weren't reported in this issue and are left untouched to keep this change minimal.

---

AI assistance: this change was drafted with Claude Code.

Fixes #5338

[netlify]

✅ Deploy Preview for pipecd-site canceled.

Name	Link
🔨 Latest commit	0abd2e4
🔍 Latest deploy log	https://app.netlify.com/projects/pipecd-site/deploys/6a9ddc0262c98400089df4e9

[Copilot]

🟢 Approval recommended

The change is narrow (logging-only), aligns with the stated shutdown-cancellation behavior, and is covered by a targeted unit test.

Pull request overview

Reduces shutdown-time log noise in the legacy piped Kubernetes drift detector by suppressing expected error logs when the drift detector’s context has already been canceled, and adds a unit test to validate the behavior.

Changes:

• Suppress Error-level logs from checkApplication and gitRepo.CleanPath when ctx.Err() != nil (shutdown/cancellation scenario).
• Add a unit test covering both canceled vs non-canceled context cases and asserting error-log emission behavior.

File summaries

File	Description
pkg/app/piped/driftdetector/kubernetes/detector.go	Gate two existing error logs on ctx.Err() == nil to avoid noisy shutdown-time cancellation errors.
pkg/app/piped/driftdetector/kubernetes/detector_test.go	Adds TestCheck_ContextCanceled to verify error logs are suppressed when the context is already canceled.

Review details

Suppressed comments (2)

pkg/app/piped/driftdetector/kubernetes/detector.go:188

• This comment also implies the error is "caused by" context cancellation, but the condition only checks ctx.Err(). Consider rewording to match the actual behavior (suppress errors when the context is canceled).

				// This clean is only partial; the entire cleanup is performed elsewhere,
				// so ignore the error when it's caused by the context being canceled.

pkg/app/piped/driftdetector/kubernetes/detector_test.go:123

• Wire the no-op reporter into the detector under test so the test doesn’t depend on an early error return to avoid calling a nil reporter.

			d := &detector{
				provider:          config.PipedPlatformProvider{Name: "kubernetes-default"},
				appLister:         &fakeAppLister{apps: []*model.Application{app}},
				stateGetter:       fakeStateGetter{},
				appManifestsCache: memorycache.NewCache(),
				config:            &config.PipedSpec{},
				logger:            logger,
				gitRepos:          map[string]git.Repo{"repo-1": repo},
				syncStates:        make(map[string]model.ApplicationSyncState),
			}

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[pujitha24]

Good catches — reworded both comments (checkApplication and CleanPath sites) so they no longer imply the error was caused by cancellation, since the code actually suppresses any error once ctx.Err() != nil. Also wired a no-op fakeReporter into the detector under test so it's not relying on checkApplication's early failure to avoid a nil reporter panic.