CMP-4601: metrics: wait for the serving cert before serving TLS - #1350
CMP-4601: metrics: wait for the serving cert before serving TLS#1350Vincent056 wants to merge 1 commit into
Conversation
The HTTPS metrics server calls ListenAndServeTLS once and deliberately swallows the error so a metrics failure never takes down the operator. But the serving cert is minted asynchronously by the service-ca operator after the metrics Service is created, so on a fresh deployment the operator can start before the secret volume is populated - and when it loses that race the :8585 endpoint stays connection-refused for the life of the pod, with nothing to self-heal (no retry, no restart). Seen in the wild in the e2e-aws-parallel run for ComplianceAsCode#1255 (2026-08-18): 'Metrics service failed: open /var/run/secrets/serving-cert/tls.crt: no such file or directory' at pod start, and every metrics assertion (TestResultServerHTTPVersion, TestServiceMonitoringMetricsTarget, TestSingleScanSucceeds) failing with curl exit 7. Wait for the cert and key files to appear (bounded, context-aware) before starting the TLS listener, keeping the existing behavior of never exiting the operator on metrics failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@Vincent056: This pull request references CMP-4601 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target either version "5.1.0." or "openshift-5.1.0.", but it targets "compliance-operator-1.10.0" instead. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
|
🤖 To deploy this PR, run the following command: |
|
/retest-required |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: taimurhafeez, Vincent056 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
Pre-Merge verification passed on 4.22. |
|
@Vincent056: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
CI failure on |
4 participants
Status: under test — please hold review of correctness claims until validation completes. We are actively validating this against live clusters and e2e (tracking in CMP-4601); I'll drop the draft state and comment here once we have a validated run.
Problem
Metrics.StartcallsListenAndServeTLSexactly once and deliberately swallows the error (a metrics failure must never take down the operator). The serving cert is minted asynchronously by the service-ca operator after the metrics Service is created — so on a fresh deployment the operator can start before the secret volume is populated. Losing that race leavesmetrics.<ns>.svc:8585/metrics-coconnection-refused for the life of the pod: no retry, no restart (liveness only checks healthz), no self-heal.Caught live in #1255's e2e-aws-parallel run: operator logged
Metrics service failed: open /var/run/secrets/serving-cert/tls.crt: no such file or directoryat pod start, andTestResultServerHTTPVersion,TestServiceMonitoringMetricsTarget,TestSingleScanSucceedsall failed with curl exit 7 against :8585 while Prometheus scraped :8383 fine.Fix
Wait (bounded 5 min, 5 s poll, context-aware) for the cert and key files to exist before starting the TLS listener. On final failure the behavior is unchanged: log and keep the operator alive. Unit-tested (late-minted cert recovery, timeout, context cancellation); package coverage 60.7 → 62.2 (baseline updated).
Out of scope, noted in CMP-4601
GetCertificate-based reload would fix that)🤖 Generated with Claude Code