Fix: add TLS connectivity probe to webhook readiness checks by pdettori · Pull Request #421 · rossoctl/operator

pdettori · 2026-06-10T03:11:22Z

Summary

Adds webhookServer.StartedChecker() as a readyz check in cmd/main.go so the pod Ready condition gates on the webhook TLS server actually listening on :9443
Adds ProbeWebhookReady() utility in test/utils/utils.go that uses kubectl wait --for=condition=Ready on controller pods
Adds the readiness probe as a second phase after the existing endpoint IP check at all 7 locations in test/e2e/e2e_test.go
Fixes the race condition where the endpoint IP populates before the webhook TLS server is ready, causing pod creation failures

Root Cause

After a controller restart, the Kubernetes Endpoints object gets the pod IP immediately, but the webhook's TLS server needs additional time to initialize (read certs, bind port 9443). The existing readyz check was healthz.Ping (a no-op), so the pod reported Ready before the webhook was accepting connections. The E2E tests only verified the endpoint IP was present, not that TLS was ready.

Fix

Runtime fix: Add mgr.AddReadyzCheck("webhook", webhookServer.StartedChecker()) — the controller-runtime webhook server exposes StartedChecker() which returns healthy only after the TLS listener is bound.
Test fix: ProbeWebhookReady() uses kubectl wait --for=condition=Ready which now reflects true webhook readiness, and handles rolling restarts (multiple pods) correctly.

Test Plan

CI E2E tests pass — specifically the "Skill Discovery > Feature gate enabled > should populate linkedSkills from annotation" test that was consistently failing
No regression in other test blocks that also use the readiness probe

Fixes #420

Assisted-By: Claude Code

The webhook endpoint IP check passes before the TLS server is ready, causing pod creation failures when Kubernetes calls the mutating webhook. Add a curl-based TLS probe after the endpoint IP check at all 7 locations where webhook readiness is verified. Fixes: rossoctl#420 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>

The curl probe pod was rejected by the restricted PodSecurity policy in CI. Add required securityContext fields (runAsNonRoot, capabilities drop ALL, seccompProfile RuntimeDefault) via --overrides. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>

The webhook server only handles admission POST requests and does not respond to HTTP GET. Using --connect-only verifies the TLS handshake succeeds without waiting for an HTTP response, avoiding a 5s timeout on every probe attempt. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>

The curl probe pod approach fails in CI due to image pull delays and kubectl run timeout issues. Replace with a simple check that the controller pod is fully Ready — the manager's readiness probe on port 8081 gates on complete initialization including webhook server. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>

pdettori · 2026-06-10T11:36:36Z

CI Status

This PR fixes the webhook readiness race condition (problem 1 from #420). After 4 iterations:

Commit	Fix
`95057f1`	Initial: curl probe for TLS connectivity
`68f9a29`	Add PodSecurity context (restricted policy)
`3efc274`	Use `--connect-only` (webhook doesn't serve HTTP GET)
`1564816`	Replace curl pod with controller pod readiness check

The final approach checks that the controller pod is fully Ready (readiness probe passes on port 8081), which gates on complete manager initialization including the webhook server.

Remaining E2E failure

The Skill Discovery linkedSkills test still fails, but this is a separate pre-existing issue — the same failure occurs on main branch (runs 27249800636, 27249348694, 27249047484 all fail).

Root cause: The controller hits an optimistic concurrency conflict when updating .status.linkedSkills and does not retry. This is tracked in #422.

What this PR fixes

The webhook "connection refused" race after controller restart (all 7 locations)
Prevents the cascading BeforeAll failures that were blocking all E2E tests

What still needs fixing (separate PR)

Fix: reconciler conflict error prevents linkedSkills status update #422: Reconciler needs retry.RetryOnConflict for status updates

huang195

Review Summary

Right instinct, Paolo — gating the tests on real webhook readiness instead of just the endpoint IP is exactly the fix #420 needs. But the implementation rests on a premise that doesn't hold for this controller: ProbeWebhookReady checks the pod's Ready condition, and that condition is wired to readyz = healthz.Ping (cmd/main.go:687) — a no-op that flips True before the webhook TLS server is listening on :9443. So the probe reports ready too early and the underlying race survives, which is consistent with E2E still failing in CI (in the failed run the probe passed in ~20s, then the suite failed anyway). The real lever is one line in main.go: add webhookServer.StartedChecker() as a readyz check (the server at main.go:279 exposes it), then this util does what it claims.

Two more things:

Stale PR description — the body describes ProbeWebhookTLS() running a temporary curl pod with --rm, but the merged approach (final commit "replace curl probe with pod readiness check") is ProbeWebhookReady doing a kubectl get pods readiness check. The Test Plan's "curl probe pod cleans up correctly (--rm)" checkbox no longer applies — worth updating so reviewers aren't chasing the old design.
E2E (fail) is the suite this PR exists to fix; it should go green before merge.

Areas reviewed: Go (e2e test + utils) · Commits: 4, all signed-off ✅ · CI: E2E failing (rest green, incl. pr-title ✅)

Tighten the readiness gate (StartedChecker) and refresh the PR body, and this lands. 🍝

Assisted-By: Claude Code

huang195 · 2026-06-10T12:36:03Z

 }
+
+// ProbeWebhookReady checks that the controller pod is fully Ready (all
+// containers passing readiness probes). The manager's readiness probe gates


must-fix — This comment ("the manager's readiness probe gates on full initialization including the webhook server startup") isn't accurate for this controller. In cmd/main.go:687 the readyz check is mgr.AddReadyzCheck("readyz", healthz.Ping), a no-op that returns OK as soon as the health endpoint is up. It does not gate on the webhook TLS server binding :9443. So the pod reports Ready before the webhook is accepting connections — the exact race this PR targets is not actually closed, which matches E2E still failing.

The clean fix is in main.go:

if err := mgr.AddReadyzCheck("webhook", webhookServer.StartedChecker()); err != nil { ... }

The webhook server created at main.go:279 (webhook.NewServer(...)) exposes StartedChecker(). Once readyz actually gates on webhook startup, the pod Ready condition becomes meaningful and ProbeWebhookReady does what it claims. Alternatively, restore a real TLS connectivity probe (as the PR body still describes).

Fixed in ce59ce4. Added webhookServer.StartedChecker() as a readyz check in main.go:691, so pod Ready now gates on the webhook TLS server actually listening on :9443. Updated the comment accordingly.

huang195 · 2026-06-10T12:36:03Z

+	cmd := exec.Command("kubectl", "get", "pods",
+		"-l", "control-plane=controller-manager",
+		"-n", namespace,
+		"-o", "jsonpath={.items[0].status.conditions[?(@.type==\"Ready\")].status}")


suggestion — .items[0] is fragile for the restart case (e2e_test.go:2159, "after restart"). During a rolling restart two pods coexist briefly and item ordering is nondeterministic, so this can read the old still-Ready pod and pass prematurely. Consider kubectl wait --for=condition=Ready pod -l control-plane=controller-manager (waits on all matching pods), or filter to the newest/running pod before reading the condition.

Fixed in ce59ce4. Replaced the .items[0] jsonpath approach with kubectl wait --for=condition=Ready pod -l control-plane=controller-manager which waits on all matching pods and handles the rolling restart case correctly.

Add webhookServer.StartedChecker() as a readyz check so the pod Ready condition reflects actual webhook TLS availability on :9443, not just the health endpoint being up. Replace the fragile .items[0] jsonpath probe with kubectl wait which handles rolling restarts correctly. Addresses review feedback from rossoctl#421. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>

huang195

Review Summary (re-review)

Bravo, Paolo — this is the real fix now. Gating readyz on webhookServer.StartedChecker() (cmd/main.go:691) means the pod's Ready condition genuinely reflects the webhook TLS server accepting connections on :9443, so ProbeWebhookReady now does exactly what its (updated, accurate) comment claims. And switching the util to kubectl wait --for=condition=Ready pod -l control-plane=controller-manager resolves the .items[0] restart-case fragility cleanly — it waits on all matching pods, so the brief two-pod window during a rolling restart self-heals. Molto bene.

One leftover, non-blocking: the PR description still narrates the old ProbeWebhookTLS() curl-pod approach (and the Test Plan's --rm cleanup checkbox), which no longer matches the kubectl wait implementation — worth a refresh so it reads true.

E2E is still running (pending) — since closing this race is the whole point, that green is the real confirmation; worth a glance before merge.

Areas reviewed: Go (main.go + e2e test + utils) · Commits: 5, all signed-off ✅ · CI: all green except E2E (pending)

No blockers — approving. 🍝

Assisted-By: Claude Code

pdettori · 2026-06-10T14:19:35Z

Needs to merge before #423 even if CI not passing as we need also #423 to pass CI.

(cherry picked from commit f13eba2) Signed-off-by: cwiklik <cwiklikj@gmail.com>

pdettori requested a review from a team as a code owner June 10, 2026 03:11

pdettori changed the title ~~fix(e2e): add TLS connectivity probe to webhook readiness checks~~ Fix: add TLS connectivity probe to webhook readiness checks Jun 10, 2026

pdettori added 3 commits June 9, 2026 23:34

pdettori mentioned this pull request Jun 10, 2026

Fix: reconciler conflict error prevents linkedSkills status update #422

Closed

huang195 requested changes Jun 10, 2026

View reviewed changes

huang195 approved these changes Jun 10, 2026

View reviewed changes

pdettori merged commit f13eba2 into rossoctl:main Jun 10, 2026
16 of 17 checks passed

pdettori deleted the fix/webhook-readiness-420 branch June 10, 2026 14:19

This was referenced Jun 15, 2026

Weekly Report 2026-06-15 rossoctl/rossoctl#1971

Closed

Weekly Report 2026-06-16 (test) rossoctl/rossoctl#1989

Closed

cwiklik pushed a commit that referenced this pull request Jun 16, 2026

fix: add TLS connectivity probe to webhook readiness checks (#421)

5ddfbae

(cherry picked from commit f13eba2) Signed-off-by: cwiklik <cwiklikj@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix: add TLS connectivity probe to webhook readiness checks#421

Fix: add TLS connectivity probe to webhook readiness checks#421
pdettori merged 5 commits into
rossoctl:mainfrom
pdettori:fix/webhook-readiness-420

pdettori commented Jun 10, 2026 •

edited

Loading

Uh oh!

pdettori commented Jun 10, 2026

Uh oh!

huang195 left a comment

Uh oh!

huang195 Jun 10, 2026

Uh oh!

pdettori Jun 10, 2026

Uh oh!

huang195 Jun 10, 2026

Uh oh!

pdettori Jun 10, 2026

Uh oh!

huang195 left a comment

Uh oh!

pdettori commented Jun 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pdettori commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Fix

Test Plan

Uh oh!

pdettori commented Jun 10, 2026

CI Status

Remaining E2E failure

What this PR fixes

What still needs fixing (separate PR)

Uh oh!

huang195 left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

huang195 Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

pdettori Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

huang195 Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

pdettori Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

huang195 left a comment

Choose a reason for hiding this comment

Review Summary (re-review)

Uh oh!

pdettori commented Jun 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pdettori commented Jun 10, 2026 •

edited

Loading