Skip to content

Commit 5cd5656

Browse files
fix(controller): tolerate delayed webhook certificate startup
Keep process liveness independent of certificate readiness and allow a bounded startup window in the Helm chart. Preserve certificate and webhook gating for readiness. Add simple unit coverage for disabled webhooks, pending certificates, webhook startup, and readiness. Do not add E2E coverage or readiness diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c077ebb-9cd5-4c85-a49d-0178a7f67a57
1 parent 5285753 commit 5cd5656

4 files changed

Lines changed: 129 additions & 13 deletions

File tree

‎src/subsystems/cache-controller/charts/dacs/templates/manager/manager.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@ spec:
9999
port: 8081
100100
initialDelaySeconds: 5
101101
periodSeconds: 10
102+
startupProbe:
103+
httpGet:
104+
path: /readyz
105+
port: 8081
106+
failureThreshold: 60
107+
periodSeconds: 5
102108
resources:
103109
{{- if .Values.manager.resources }}
104110
{{- toYaml .Values.manager.resources | nindent 20 }}

‎src/subsystems/cache-controller/cmd/main.go‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -402,30 +402,48 @@ func main() {
402402

403403
// +kubebuilder:scaffold:builder
404404

405-
// Setup health checks with cert readiness awareness
405+
if err := setupHealthChecks(mgr, enableWebhooks, certSetupFinished, func(req *http.Request) error {
406+
return mgr.GetWebhookServer().StartedChecker()(req)
407+
}); err != nil {
408+
logAndExit(err, "unable to set up health checks")
409+
}
410+
411+
setupLog.Info("starting manager")
412+
if err := mgr.Start(ctx); err != nil {
413+
logAndExit(err, "problem running manager")
414+
}
415+
}
416+
417+
type healthCheckRegistrar interface {
418+
AddHealthzCheck(string, healthz.Checker) error
419+
AddReadyzCheck(string, healthz.Checker) error
420+
}
421+
422+
func setupHealthChecks(
423+
mgr healthCheckRegistrar,
424+
enableWebhooks bool,
425+
certSetupFinished <-chan struct{},
426+
webhookStarted healthz.Checker,
427+
) error {
406428
checker := healthz.Ping
407429
if enableWebhooks {
408430
checker = func(req *http.Request) error {
409431
select {
410432
case <-certSetupFinished:
411-
return mgr.GetWebhookServer().StartedChecker()(req)
433+
return webhookStarted(req)
412434
default:
413435
return fmt.Errorf("certs are not ready yet")
414436
}
415437
}
416438
}
417439

418-
if err := mgr.AddHealthzCheck("healthz", checker); err != nil {
419-
logAndExit(err, "unable to set up health check")
440+
if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
441+
return fmt.Errorf("registering liveness check: %w", err)
420442
}
421443
if err := mgr.AddReadyzCheck("readyz", checker); err != nil {
422-
logAndExit(err, "unable to set up ready check")
423-
}
424-
425-
setupLog.Info("starting manager")
426-
if err := mgr.Start(ctx); err != nil {
427-
logAndExit(err, "problem running manager")
444+
return fmt.Errorf("registering readiness check: %w", err)
428445
}
446+
return nil
429447
}
430448

431449
// logAndExit logs the error and exits the program with a non-zero status code.

‎src/subsystems/cache-controller/cmd/main_test.go‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,15 @@
1515
package main
1616

1717
import (
18+
"errors"
19+
"net/http"
20+
"net/http/httptest"
1821
"testing"
1922

2023
corev1 "k8s.io/api/core/v1"
2124
"k8s.io/apimachinery/pkg/api/resource"
2225
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
"sigs.k8s.io/controller-runtime/pkg/healthz"
2327

2428
"go.uber.org/zap/zapcore"
2529
"gotest.tools/v3/assert"
@@ -101,3 +105,69 @@ func TestRemoveUnusedNodeFields_NonNodeInput(t *testing.T) {
101105
assert.NilError(t, err)
102106
assert.Equal(t, any(cm), out, "expected pass-through for non-Node input")
103107
}
108+
109+
type healthCheckRecorder struct {
110+
checks map[string]healthz.Checker
111+
}
112+
113+
func (r *healthCheckRecorder) AddHealthzCheck(name string, checker healthz.Checker) error {
114+
r.checks[name] = checker
115+
return nil
116+
}
117+
118+
func (r *healthCheckRecorder) AddReadyzCheck(name string, checker healthz.Checker) error {
119+
r.checks[name] = checker
120+
return nil
121+
}
122+
123+
func TestSetupHealthChecks(t *testing.T) {
124+
tests := []struct {
125+
name string
126+
enableWebhooks bool
127+
certsReady bool
128+
webhookErr error
129+
wantErr string
130+
}{
131+
{name: "webhooks disabled"},
132+
{name: "certificates pending", enableWebhooks: true, wantErr: "certs are not ready yet"},
133+
{
134+
name: "webhook starting",
135+
enableWebhooks: true,
136+
certsReady: true,
137+
webhookErr: errors.New("webhook server has not started"),
138+
wantErr: "webhook server has not started",
139+
},
140+
{name: "webhook ready", enableWebhooks: true, certsReady: true},
141+
}
142+
for _, tt := range tests {
143+
t.Run(tt.name, func(t *testing.T) {
144+
certsReady := make(chan struct{})
145+
if tt.certsReady {
146+
close(certsReady)
147+
}
148+
recorder := &healthCheckRecorder{checks: make(map[string]healthz.Checker)}
149+
webhookCalls := 0
150+
err := setupHealthChecks(recorder, tt.enableWebhooks, certsReady, func(*http.Request) error {
151+
webhookCalls++
152+
return tt.webhookErr
153+
})
154+
assert.NilError(t, err)
155+
assert.Assert(t, recorder.checks["healthz"] != nil, "liveness check must be registered")
156+
assert.Assert(t, recorder.checks["readyz"] != nil, "readiness check must be registered")
157+
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/healthz", nil)
158+
assert.NilError(t, recorder.checks["healthz"](req), "liveness must not depend on certificate or webhook readiness")
159+
req = httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/readyz", nil)
160+
err = recorder.checks["readyz"](req)
161+
if tt.wantErr == "" {
162+
assert.NilError(t, err)
163+
} else {
164+
assert.Error(t, err, tt.wantErr)
165+
}
166+
wantWebhookCalls := 0
167+
if tt.enableWebhooks && tt.certsReady {
168+
wantWebhookCalls = 1
169+
}
170+
assert.Equal(t, webhookCalls, wantWebhookCalls, "only check webhook startup after certificates are ready")
171+
})
172+
}
173+
}

‎src/subsystems/cache-controller/docs/cert-controller.md‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,40 @@ This allows the Kubernetes API server to trust the webhook's TLS certificate.
3434
- **Path:** `/tmp/k8s-webhook-server/serving-certs`
3535
- **Purpose:** The webhook server reads certificates from this directory
3636

37-
The rotator writes certificates here so the controller-runtime webhook server can use them.
37+
Kubelet projects the certificate Secret into this directory so the
38+
controller-runtime webhook server can use it. Secret updates are asynchronous;
39+
generating certificates does not make the mounted files available immediately.
3840

3941
## How It Works
4042

4143
1. Controller starts -> cert rotator initializes
4244
2. Rotator generates self-signed CA + server certificate
43-
3. Certificates stored in Secret and written to filesystem
45+
3. Certificates stored in Secret and projected into the pod by kubelet
4446
4. CA bundle injected into webhook configurations
4547
5. Webhooks registered (only after certs are ready)
4648
6. Rotator monitors expiry and auto-renews
4749

50+
## Health Probes
51+
52+
`/healthz` checks process liveness independently of certificate initialization.
53+
`/readyz` succeeds only after certificate setup completes and the webhook server
54+
starts. With webhooks disabled, readiness does not depend on certificates.
55+
56+
The Helm chart's startup probe checks `/readyz` every five seconds and allows
57+
60 failures, giving initialization approximately five minutes before Kubernetes
58+
restarts the container. Liveness and readiness probes begin after startup
59+
succeeds. Keep `EnableReadinessCheck` enabled in the cert rotator so webhook
60+
configuration reconciliation waits for mounted certificates.
61+
62+
Unit tests cover the registered liveness and readiness checks, certificate
63+
initialization, and webhook startup. Run them from
64+
`src/subsystems/cache-controller`:
65+
66+
```bash
67+
CACHE_CONTROLLER_CONFIG_PATH="$PWD/internal/config/cache-controller-config.yaml" \
68+
go test ./cmd
69+
```
70+
4871
## RBAC Requirements
4972

5073
| File | Purpose |
@@ -58,4 +81,3 @@ Check certificate secret:
5881
```
5982
kubectl get secret cache-controller-webhook-server-cert -n cache-controller-system
6083
```
61-

0 commit comments

Comments
 (0)