fix(errors): retry transient "failed calling webhook" apiserver 500s

[HsiuChuanHsu]

Fixes #14220

Motivation

A webhook can be briefly unreachable during normal operation. While it is unreachable, the kube-apiserver
rejects the request with an HTTP 500 InternalError:

Internal error occurred: failed calling webhook "..."

An earlier attempt (PR #16038) treated all 500s as transient which was not that correct. Blindly retrying them would mask real failures and pile load onto an already-unhealthy apiserver.
So this task is to retry only this known-transient case while leaving every other 500 classified as permanent.

Modifications

Added a single targeted classifier in util/errors/errors.go, same style as the existing isResourceQuotaTimeoutErr:

func isTransientWebhookErr(err error) bool {
    return apierr.IsInternalError(err) && strings.Contains(err.Error(), "failed calling webhook")
}

And added into isTransientErr` alongside the other transient checks.

Verification

Documentation

AI

Claude Sonnet 4.8

• Debug core logic.
• Refine comment wording
• Unit tests

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling for temporary webhook failures so affected operations can be retried appropriately.
  • Preserved existing behavior for unrelated internal errors and requests explicitly denied by webhooks.

• Tests

  • Added coverage for transient webhook timeouts, unrelated errors, and denied requests.

[coderabbitai]

📝 Walkthrough

Walkthrough

The transient error classifier now treats internal apiserver errors containing "failed calling webhook" as transient. Tests cover webhook timeouts, unrelated internal errors, and admission-denial errors.

Changes

Transient webhook error handling

Layer / File(s)	Summary
Webhook error classification and coverage util/errors/errors.go, util/errors/errors_test.go	isTransientErr now uses isTransientWebhookErr to classify matching internal apiserver webhook failures. Tests cover transient webhook timeouts, unrelated internal errors, and admission-denial errors.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• argoproj/argo-workflows#16484: Modifies isTransientErr and its tests for additional transient errors.
• argoproj/argo-workflows#16485: Extends isTransientErr tests for additional error-string patterns.
• argoproj/argo-workflows#16487: Adds apiserver and gRPC error classifications with corresponding tests.

Suggested labels: cherry-pick/3.7, cherry-pick/4.0

Suggested reviewers: shuangkun, joibel

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the targeted retry fix for transient apiserver webhook errors.
Description check	✅ Passed	The description includes the issue, motivation, modifications, and AI disclosure, but leaves verification and documentation sections incomplete.
Linked Issues check	✅ Passed	The change addresses issue #14220 by retrying internal apiserver errors caused by transient webhook failures while excluding unrelated 500 errors.
Out of Scope Changes check	✅ Passed	The code and test changes are limited to transient webhook error classification and directly support issue #14220.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@util/errors/errors.go`:
- Around line 92-95: Restrict isTransientWebhookErr in util/errors/errors.go to
classify only explicitly retryable webhook causes rather than relying on the
shared “failed calling webhook” text; preserve structured failure information if
needed before classification. Add a regression case in
util/errors/errors_test.go covering a non-transient failed-calling-webhook error
and verify it is not classified as transient.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 636a3860-24c3-4f63-96bf-c867bfc2db17

📥 Commits

Reviewing files that changed from the base of the PR and between e0b6328 and 50f2422.

📒 Files selected for processing (2)

• util/errors/errors.go
• util/errors/errors_test.go

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Keep webhook retries within the intended transient boundary.

The classifier treats the shared failed calling webhook wrapper as sufficient evidence of a transient failure. Kubernetes uses this path for additional webhook-call failures, including client construction and response errors. (raw.githubusercontent.com)

• util/errors/errors.go#L92-L95: Match only retryable causes, or preserve structured failure information before classification.
• util/errors/errors_test.go#L78-L82: Add a non-transient failed calling webhook regression case.

📍 Affects 2 files

• util/errors/errors.go#L92-L95 (this comment)
• util/errors/errors_test.go#L78-L82

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@util/errors/errors.go` around lines 92 - 95, Restrict isTransientWebhookErr
in util/errors/errors.go to classify only explicitly retryable webhook causes
rather than relying on the shared “failed calling webhook” text; preserve
structured failure information if needed before classification. Add a regression
case in util/errors/errors_test.go covering a non-transient
failed-calling-webhook error and verify it is not classified as transient.