CNTRLPLANE-3526: Add spec.monitoring API for metrics forwarding#8626
Merged
openshift-merge-bot[bot] merged 6 commits intoJun 25, 2026
Merged
Conversation
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@muraee: This pull request explicitly references no jira issue. 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. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR migrates metrics forwarding configuration from annotation-based to spec-based control. It adds MonitoringSpec, MetricsForwardingSpec, and enums (MetricsForwardingMode, MetricsSet), copies HostedCluster.Spec.Monitoring to HostedControlPlane.Spec.Monitoring with a backward-compatibility shim for the deprecated annotation, computes an effective metrics set for SRE reconciliation, updates controller predicates and gating to use the spec field, and updates unit and e2e tests to drive behavior via the new spec. Sequence Diagram(s)sequenceDiagram
participant User
participant HostedClusterController
participant HostedCluster
participant HostedControlPlane
participant ControlPlaneOperator
participant MetricsForwarder
User->>HostedCluster: set spec.monitoring.metricsForwarding.mode=Enabled
HostedClusterController->>HostedCluster: read Spec.Monitoring
HostedClusterController->>HostedControlPlane: copy Spec.Monitoring (apply deprecated-annotation shim if unset)
ControlPlaneOperator->>HostedControlPlane: read Spec.Monitoring
ControlPlaneOperator->>MetricsForwarder: enable/disable based on MetricsForwarding.Mode and MetricsSet
MetricsForwarder-->>ControlPlaneOperator: status
Suggested reviewers
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
muraee
changed the title
|
@muraee: This pull request references CNTRLPLANE-3526 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 story to target the "5.0.0" version, but no target version was set. 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. |
openshift-ci
Bot
added
area/api
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@api/hypershift/v1beta1/hosted_controlplane.go`:
- Around line 190-195: The Monitoring field in HostedControlPlane currently has
an inconsistent JSON tag; locate the Monitoring declaration (Monitoring
MonitoringSpec) and replace the tag string that contains both
"omitempty,omitzero" with the single "omitzero" form (i.e.,
json:"monitoring,omitzero") so it matches the style used by HostedClusterSpec
and other fields like AutoNode.
In
`@control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go`:
- Around line 1189-1192: The code reads
hcp.Spec.Monitoring.MetricsForwarding.MetricsSet without nil checks which can
panic; update the logic around effectiveMetricsSet (and keep r.MetricsSet
fallback) to first verify hcp.Spec.Monitoring != nil and
hcp.Spec.Monitoring.MetricsForwarding != nil before accessing MetricsSet, and
only override effectiveMetricsSet when those pointers are non-nil and MetricsSet
is non-empty (preserve existing behavior of using metrics.MetricsSet(...) when
present).
In
`@control-plane-operator/controllers/hostedcontrolplane/v2/endpoint_resolver/component.go`:
- Around line 46-49: The predicate function dereferences
cpContext.HCP.Spec.Monitoring.MetricsForwarding.Mode without nil checks which
can panic when Monitoring or MetricsForwarding are nil; update
predicate(cpContext component.WorkloadContext) to first check cpContext.HCP,
cpContext.HCP.Spec, cpContext.HCP.Spec.Monitoring and
cpContext.HCP.Spec.Monitoring.MetricsForwarding for nil before reading Mode and
combine that guarded check with the existing DisableMonitoringServices
annotation check (hyperv1.DisableMonitoringServices) so the function returns
false (no metrics forwarding) when any of the intermediate structs are nil and
only true when Mode == hyperv1.MetricsForwardingModeEnabled and monitoring is
not disabled.
In
`@control-plane-operator/controllers/hostedcontrolplane/v2/metrics_proxy/component.go`:
- Around line 69-72: The predicate function reads
cpContext.HCP.Spec.Monitoring.MetricsForwarding.Mode without nil guards; add
checks in predicate to ensure cpContext.HCP, cpContext.HCP.Spec, and
cpContext.HCP.Spec.Monitoring are non-nil (and that Monitoring.MetricsForwarding
is present) before accessing Mode, and return false (no reconcile) if any are
nil; keep the existing DisableMonitoringServices annotation check
(hyperv1.DisableMonitoringServices) and only evaluate Mode when the monitoring
structs exist to avoid nil-pointer panics.
In `@hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go`:
- Around line 2508-2511: The current fallback flips any non-Enabled mode
(including an explicit Disabled) to Enabled when the deprecated annotation
exists; change the condition to only apply the annotation fallback when the HCP
mode is not explicitly set (e.g., empty/unspecified) rather than any mode other
than Enabled. Concretely, update the check around
hcp.Spec.Monitoring.MetricsForwarding.Mode so it only sets
hyperv1.MetricsForwardingModeEnabled from the deprecated
hcluster.Annotations[hyperv1.EnableMetricsForwarding] when the existing mode is
the unset/zero value (not when it equals hyperv1.MetricsForwardingModeDisabled
or any explicit value).
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9bad19ca-0e30-40f1-88ee-d8aca6240665
⛔ Files ignored due to path filters (44)
api/hypershift/v1beta1/zz_generated.deepcopy.gois excluded by!**/zz_generated*.go,!**/zz_generated*api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ClusterUpdateAcceptRisks.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ClusterVersionOperatorConfiguration.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDC.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDCWithUIDAndExtraClaimMappings.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDCWithUpstreamParity.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/GCPPlatform.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HCPEtcdBackup.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ImageStreamImportMode.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/KMSEncryptionProvider.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/OpenStack.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/TLSAdherence.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ClusterUpdateAcceptRisks.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ClusterVersionOperatorConfiguration.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDC.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDCWithUIDAndExtraClaimMappings.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDCWithUpstreamParity.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/GCPPlatform.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/HCPEtcdBackup.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ImageStreamImportMode.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/KMSEncryptionProvider.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/OpenStack.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/TLSAdherence.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**client/applyconfiguration/hypershift/v1beta1/hostedclusterspec.gois excluded by!client/**client/applyconfiguration/hypershift/v1beta1/hostedcontrolplanespec.gois excluded by!client/**client/applyconfiguration/hypershift/v1beta1/metricsforwardingspec.gois excluded by!client/**client/applyconfiguration/hypershift/v1beta1/monitoringspec.gois excluded by!client/**client/applyconfiguration/utils.gois excluded by!client/**cmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.monitoring.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamldocs/content/reference/aggregated-docs.mdis excluded by!docs/content/reference/aggregated-docs.mddocs/content/reference/api.mdis excluded by!docs/content/reference/api.mdvendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**,!**/zz_generated*.go,!**/zz_generated*
📒 Files selected for processing (11)
api/hypershift/v1beta1/hosted_controlplane.goapi/hypershift/v1beta1/hostedcluster_types.gocontrol-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/endpoint_resolver/component.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/endpoint_resolver/component_test.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/metrics_proxy/component.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.gohypershift-operator/controllers/hostedcluster/hostedcluster_controller.gotest/e2e/util/util_metrics_proxy.gotest/e2e/v2/tests/hosted_cluster_metrics_test.go
csrwng
reviewed
May 28, 2026
muraee
force-pushed
the
metrics-forwarding-api
branch
from
996709e to
89814ff
Compare
Contributor
There was a problem hiding this comment.
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
`@control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go`:
- Around line 951-952: The code directly reads
hcp.Spec.Monitoring.MetricsForwarding.Mode which can panic when Monitoring or
MetricsForwarding is nil; update the conditional to first nil-check
hcp.Spec.Monitoring and hcp.Spec.Monitoring.MetricsForwarding and only compare
Mode to hyperv1.MetricsForwardingModeEnabled when both are non-nil, otherwise
treat it as not enabled and call return k8sutil.DeleteAllIfNeeded(ctx, r.client,
deployment, cm, servingCA, podMonitor); ensure you reference the same symbols
(hcp, Spec, Monitoring, MetricsForwarding, Mode,
hyperv1.MetricsForwardingModeEnabled) so the branch exactly mirrors the intended
behavior.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9bc66ee1-87cc-4ca1-8f51-fda6d9f9d236
⛔ Files ignored due to path filters (44)
api/hypershift/v1beta1/zz_generated.deepcopy.gois excluded by!**/zz_generated*.go,!**/zz_generated*api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ClusterUpdateAcceptRisks.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ClusterVersionOperatorConfiguration.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDC.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDCWithUIDAndExtraClaimMappings.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDCWithUpstreamParity.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/GCPPlatform.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HCPEtcdBackup.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ImageStreamImportMode.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/KMSEncryptionProvider.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/OpenStack.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/TLSAdherence.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ClusterUpdateAcceptRisks.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ClusterVersionOperatorConfiguration.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDC.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDCWithUIDAndExtraClaimMappings.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDCWithUpstreamParity.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/GCPPlatform.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/HCPEtcdBackup.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ImageStreamImportMode.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/KMSEncryptionProvider.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/OpenStack.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/TLSAdherence.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**client/applyconfiguration/hypershift/v1beta1/hostedclusterspec.gois excluded by!client/**client/applyconfiguration/hypershift/v1beta1/hostedcontrolplanespec.gois excluded by!client/**client/applyconfiguration/hypershift/v1beta1/metricsforwardingspec.gois excluded by!client/**client/applyconfiguration/hypershift/v1beta1/monitoringspec.gois excluded by!client/**client/applyconfiguration/utils.gois excluded by!client/**cmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.monitoring.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamldocs/content/reference/aggregated-docs.mdis excluded by!docs/content/reference/aggregated-docs.mddocs/content/reference/api.mdis excluded by!docs/content/reference/api.mdvendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**,!**/zz_generated*.go,!**/zz_generated*
📒 Files selected for processing (11)
api/hypershift/v1beta1/hosted_controlplane.goapi/hypershift/v1beta1/hostedcluster_types.gocontrol-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/endpoint_resolver/component.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/endpoint_resolver/component_test.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/metrics_proxy/component.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.gohypershift-operator/controllers/hostedcluster/hostedcluster_controller.gotest/e2e/util/util_metrics_proxy.gotest/e2e/v2/tests/hosted_cluster_metrics_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- control-plane-operator/controllers/hostedcontrolplane/v2/endpoint_resolver/component.go
- test/e2e/v2/tests/hosted_cluster_metrics_test.go
- api/hypershift/v1beta1/hosted_controlplane.go
- control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
- test/e2e/util/util_metrics_proxy.go
- control-plane-operator/controllers/hostedcontrolplane/v2/endpoint_resolver/component_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8626 +/- ##
==========================================
+ Coverage 42.50% 42.60% +0.10%
==========================================
Files 768 768
Lines 95272 95318 +46
==========================================
+ Hits 40498 40614 +116
+ Misses 51971 51892 -79
- Partials 2803 2812 +9
... and 3 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
5 tasks
Allow the forwarded metrics set (guest-side, via metrics-proxy) to be configured independently from the MC-side ServiceMonitor/PodMonitor relabel configs. When MetricsForwardingSpec.MetricsSet is not set, it falls back to MonitoringSpec.MetricsSet (then to the global METRICS_SET env var). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
muraee
force-pushed
the
metrics-forwarding-api
branch
from
f5670b8 to
0c33a77
Compare
- Broaden SRE ConfigMap watch to enqueue HostedClusters with per-cluster metricsSet SRE override, not just when operator global is SRE - Extract effectiveMetricsSet() helper to deduplicate override resolution - Fix comment to say "None" instead of "Disabled" matching the enum - Fix test names to use Forward/None instead of Enabled/Disabled - Add positive test for Mode=Forward verifying resources are preserved - Add envtest for required mode field on MetricsForwardingSpec - Add godoc to exported Predicate function - Add TestMetricsSetConstantsInSync asserting hyperv1/metrics type parity Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
muraee
force-pushed
the
metrics-forwarding-api
branch
from
0c33a77 to
ef52175
Compare
Contributor
|
/lgtm |
Contributor
|
Scheduling tests matching the |
Older CPOs (4.22) only read the EnableMetricsForwarding annotation, not the new spec.monitoring.metricsForwarding field. When the HO sets the spec on the HCP without also setting the annotation, a 4.22 CPO never deploys metrics forwarding components, breaking 4.22 e2e tests and real-world N-1 version skew scenarios. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
muraee
force-pushed
the
metrics-forwarding-api
branch
from
eadd49d to
170c140
Compare
Contributor
|
/lgtm |
Contributor
|
Scheduling tests matching the |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
Contributor
Author
|
/retest-required |
Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryBoth failures are in Karpenter-related tests that are unrelated to PR #8626 (CNTRLPLANE-3526: Add spec.monitoring API for metrics forwarding). The PR adds a Root CauseJob 1 ( Job 2 ( Both failures reproduce on other PRs with completely different code changes, confirming they are pre-existing infrastructure/environment flakiness. Recommendations
Evidence
|
Contributor
Author
|
/retest-required |
Contributor
Author
|
/verified by e2e |
openshift-ci-robot
added
the
verified
|
@muraee: This PR has been marked as verified by 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. |
Contributor
|
@muraee: all tests passed! 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. |
Contributor
Author
|
/jira backport release-4.22 |
|
@muraee: The following backport issues have been created: Queuing cherrypicks to the requested branches to be created after this PR merges: 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. |
|
@openshift-ci-robot: #8626 failed to apply on top of branch "release-4.22": 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 kubernetes-sigs/prow repository. |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
spec.monitoring.metricsForwardingAPI on HostedCluster and HostedControlPlane, replacing the annotation-basedhypershift.openshift.io/enable-metrics-forwardingmechanismmetricsSetfield (Telemetry/SRE/All) that overrides the globalMETRICS_SETenv var on the HyperShift OperatorTest plan
TestPredicate)TestReconcileMetricsForwarder)make verifypasses (0 lint issues)make api-lint-fixpasses (0 issues)spec.monitoring.metricsForwarding.mode: Enabled🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests