OCPBUGS-112272: node ServiceMonitor serverName uses guest namespace on HyperShift - #603
OCPBUGS-112272: node ServiceMonitor serverName uses guest namespace on HyperShift#603bryan-cox wants to merge 1 commit into
Conversation
…n HyperShift
The node metrics ServiceMonitor's TLS serverName was built from ${NAMESPACE}
(control-plane namespace) instead of the guest namespace. On HyperShift these
namespaces differ, so serverName never matched the serving certificate's SANs,
every node metrics scrape failed certificate verification, and TargetDown fired
continuously. On standalone the two namespaces are identical, so the defect was
invisible there.
The shared patch common/metrics/service_monitor_add_port.yaml.patch hardcoded
${NAMESPACE}, which is only correct for the controller monitor. The node metrics
Service runs guest-side and its serving certificate is issued for the guest
namespace.
Introduce a ${MONITOR_NAMESPACE} template variable in the patch and resolve it
per service prefix in the generator: ${NODE_NAMESPACE} for the node monitor and
${NAMESPACE} for the controller monitor. Standalone output is functionally
unchanged since both variables resolve to openshift-cluster-csi-drivers.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
the
jira/valid-reference
|
Skipping CI for Draft Pull Request. |
openshift-ci-robot
added
the
jira/valid-bug
|
@bryan-cox: This pull request references Jira Issue OCPBUGS-112272, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe ServiceMonitor TLS 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/lgtm |
|
Scheduling required tests: Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, jsafrane 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 |
openshift-ci
Bot
added
the
approved
|
@bryan-cox: The following tests 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. |
4 participants
What
On HyperShift, the node metrics ServiceMonitor that csi-operator creates in the guest cluster set its TLS
serverNamefrom the control-plane namespace (${NAMESPACE}) instead of the guest namespace. The name never matched the service serving certificate, so every node metrics scrape failed certificate verification, the node CSI driver metrics target was permanently down, andTargetDownfired continuously.Impact is limited to observability — CSI provisioning and attach/detach are unaffected — but node CSI metrics are never collected and the permanently-failing target produces continuous alert noise.
Root cause
The shared patch
assets/common/metrics/service_monitor_add_port.yaml.patchhardcoded${NAMESPACE}inserverName. That patch is applied bygenerateDriverMetricsService()for both the controller monitor (where${NAMESPACE}is correct, since the controller runs control-plane side) and the node monitor (where it is wrong, since the node DaemonSet and its metrics Service run guest side).In
pkg/driver/common/operator/replacer.gothe two namespace variables resolve to different values under HyperShift:${NAMESPACE}→ control-plane namespace${NODE_NAMESPACE}→ guest namespaceOn standalone both resolve to
openshift-cluster-csi-drivers, which is why the defect was invisible there.Fix
${MONITOR_NAMESPACE}template variable in the shared ServiceMonitor patch.monitorNamespaceVariable):${NODE_NAMESPACE}for the node monitor,${NAMESPACE}for the controller monitor. Wired into bothgenerateDriverMetricsServiceandgenerateSidecarMetricsServices.make update.Scope / safety
${NAMESPACE}and${NODE_NAMESPACE}both resolve toopenshift-cluster-csi-drivers, so the renderedserverNameis byte-for-byte identical to before — only the template variable changed.${NAMESPACE}).Testing
pkg/generator/asset_generator_test.go) asserting the node monitor resolves to${NODE_NAMESPACE}and the controller monitor to${NAMESPACE}, using the real embedded assets.make updateis idempotent (no unintended diffs),go build ./...clean,go test ./pkg/...all pass.Acceptance criteria
serverNameresolves to the guest namespace, matching the serving cert SANs.serverNamefrom${NODE_NAMESPACE}.${NAMESPACE}).