CNTRLPLANE-3584: Enable resource metrics for kube-scheduler endpoints - #8680
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
@dhgautam99: This pull request references CNTRLPLANE-3584 which is a valid 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. |
📝 WalkthroughWalkthroughThis pull request enables Prometheus to scrape a second endpoint ( 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
area/control-plane-operator
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8680 +/- ##
==========================================
+ Coverage 41.66% 41.68% +0.02%
==========================================
Files 758 758
Lines 93929 93983 +54
==========================================
+ Hits 39135 39177 +42
- Misses 52046 52058 +12
Partials 2748 2748
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
control-plane-operator/hostedclusterconfigoperator/controllers/resources/rbac/reconcile.go (1)
294-298: ⚡ Quick winUse manifest-derived role name to prevent drift.
Line 297 hardcodes
"hypershift-metrics-resources-reader"instead of referencing the manifest constructor. If either side changes, role binding will silently point to the wrong role.💡 Proposed fix
r.RoleRef = rbacv1.RoleRef{ APIGroup: rbacv1.SchemeGroupVersion.Group, Kind: "ClusterRole", - Name: "hypershift-metrics-resources-reader", + Name: hccomanifests.MetricsResourcesClusterRole().Name, }🤖 Prompt for 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. In `@control-plane-operator/hostedclusterconfigoperator/controllers/resources/rbac/reconcile.go` around lines 294 - 298, Replace the hardcoded RoleRef.Name string in the r.RoleRef assignment with the canonical name produced by the manifest constructor that builds the corresponding ClusterRole (the same code that currently constructs "hypershift-metrics-resources-reader"); locate that manifest constructor or exported name/constant used when creating the ClusterRole and use it to set r.RoleRef.Name in the r.RoleRef assignment so the RoleBinding always references the manifest-derived role name and cannot drift.
🤖 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/controllers/hostedcontrolplane/v2/kube_scheduler/servicemonitor.go`:
- Around line 11-16: The adaptServiceMonitor function currently indexes
sm.Spec.Endpoints[0] without verifying endpoints exist, which can panic; update
adaptServiceMonitor to first check len(sm.Spec.Endpoints) > 0 and return a clear
error (or create/append a default endpoint if desired) when it is zero, then
only set MetricRelabelConfigs and call util.ApplyClusterIDLabel on
sm.Spec.Endpoints[0]; reference the adaptServiceMonitor function,
sm.Spec.Endpoints, MetricRelabelConfigs, and util.ApplyClusterIDLabel when
making the change.
In `@support/metrics/sets.go`:
- Around line 229-237: The Telemetry branch in SchedulerRelabelConfigs
incorrectly returns sreMetricsSetConfig.KubeScheduler causing Telemetry to
depend on the SRE config; change the MetricsSetTelemetry case to return the
Telemetry-specific relabel config (e.g., telemetryMetricsSetConfig.KubeScheduler
or a dedicated telemetry config variable) instead of
sreMetricsSetConfig.KubeScheduler, ensuring MetricsSetSRE still returns
sreMetricsSetConfig.KubeScheduler and the default returns nil.
---
Nitpick comments:
In
`@control-plane-operator/hostedclusterconfigoperator/controllers/resources/rbac/reconcile.go`:
- Around line 294-298: Replace the hardcoded RoleRef.Name string in the
r.RoleRef assignment with the canonical name produced by the manifest
constructor that builds the corresponding ClusterRole (the same code that
currently constructs "hypershift-metrics-resources-reader"); locate that
manifest constructor or exported name/constant used when creating the
ClusterRole and use it to set r.RoleRef.Name in the r.RoleRef assignment so the
RoleBinding always references the manifest-derived role name and cannot drift.
🪄 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: 7d14cef1-a721-4b03-b73a-d3d4e5886071
⛔ Files ignored due to path filters (20)
control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/AROSwift/zz_fixture_TestControlPlaneComponents_kube_scheduler_controlplanecomponent.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/AROSwift/zz_fixture_TestControlPlaneComponents_kube_scheduler_deployment.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/AROSwift/zz_fixture_TestControlPlaneComponents_kube_scheduler_service.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/AROSwift/zz_fixture_TestControlPlaneComponents_kube_scheduler_servicemonitor.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/GCP/zz_fixture_TestControlPlaneComponents_kube_scheduler_controlplanecomponent.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/GCP/zz_fixture_TestControlPlaneComponents_kube_scheduler_deployment.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/GCP/zz_fixture_TestControlPlaneComponents_kube_scheduler_service.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/GCP/zz_fixture_TestControlPlaneComponents_kube_scheduler_servicemonitor.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/IBMCloud/zz_fixture_TestControlPlaneComponents_kube_scheduler_controlplanecomponent.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/IBMCloud/zz_fixture_TestControlPlaneComponents_kube_scheduler_deployment.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/IBMCloud/zz_fixture_TestControlPlaneComponents_kube_scheduler_service.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/IBMCloud/zz_fixture_TestControlPlaneComponents_kube_scheduler_servicemonitor.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_kube_scheduler_controlplanecomponent.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_kube_scheduler_deployment.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_kube_scheduler_service.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_kube_scheduler_servicemonitor.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/zz_fixture_TestControlPlaneComponents_kube_scheduler_controlplanecomponent.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/zz_fixture_TestControlPlaneComponents_kube_scheduler_deployment.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/zz_fixture_TestControlPlaneComponents_kube_scheduler_service.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/zz_fixture_TestControlPlaneComponents_kube_scheduler_servicemonitor.yamlis excluded by!**/testdata/**
📒 Files selected for processing (15)
control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.gocontrol-plane-operator/controllers/hostedcontrolplane/manifests/pki.gocontrol-plane-operator/controllers/hostedcontrolplane/pki/scheduler.gocontrol-plane-operator/controllers/hostedcontrolplane/pki/scheduler_test.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/assets/kube-scheduler/deployment.yamlcontrol-plane-operator/controllers/hostedcontrolplane/v2/assets/kube-scheduler/service.yamlcontrol-plane-operator/controllers/hostedcontrolplane/v2/assets/kube-scheduler/servicemonitor.yamlcontrol-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/component.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/component_test.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/servicemonitor.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/servicemonitor_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/rbac.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/rbac/reconcile.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.gosupport/metrics/sets.go
| func adaptServiceMonitor(cpContext component.WorkloadContext, sm *prometheusoperatorv1.ServiceMonitor) error { | ||
| sm.Spec.NamespaceSelector = prometheusoperatorv1.NamespaceSelector{ | ||
| MatchNames: []string{sm.Namespace}, | ||
| } | ||
| sm.Spec.Endpoints[0].MetricRelabelConfigs = metrics.SchedulerRelabelConfigs(cpContext.MetricsSet) | ||
| util.ApplyClusterIDLabel(&sm.Spec.Endpoints[0], cpContext.HCP.Spec.ClusterID) |
There was a problem hiding this comment.
Guard endpoint indexing to avoid reconcile panic.
Line 15 and Line 16 dereference sm.Spec.Endpoints[0] without validating length. A zero-endpoint manifest will panic the controller instead of returning a reconcile error.
💡 Proposed fix
import (
+ "fmt"
+
component "github.com/openshift/hypershift/support/controlplane-component"
"github.com/openshift/hypershift/support/metrics"
"github.com/openshift/hypershift/support/util"
@@
func adaptServiceMonitor(cpContext component.WorkloadContext, sm *prometheusoperatorv1.ServiceMonitor) error {
sm.Spec.NamespaceSelector = prometheusoperatorv1.NamespaceSelector{
MatchNames: []string{sm.Namespace},
}
+ if len(sm.Spec.Endpoints) == 0 {
+ return fmt.Errorf("kube-scheduler ServiceMonitor must define at least one endpoint")
+ }
sm.Spec.Endpoints[0].MetricRelabelConfigs = metrics.SchedulerRelabelConfigs(cpContext.MetricsSet)
util.ApplyClusterIDLabel(&sm.Spec.Endpoints[0], cpContext.HCP.Spec.ClusterID)As per coding guidelines: "Avoid panics in Go except in truly unrecoverable cases."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func adaptServiceMonitor(cpContext component.WorkloadContext, sm *prometheusoperatorv1.ServiceMonitor) error { | |
| sm.Spec.NamespaceSelector = prometheusoperatorv1.NamespaceSelector{ | |
| MatchNames: []string{sm.Namespace}, | |
| } | |
| sm.Spec.Endpoints[0].MetricRelabelConfigs = metrics.SchedulerRelabelConfigs(cpContext.MetricsSet) | |
| util.ApplyClusterIDLabel(&sm.Spec.Endpoints[0], cpContext.HCP.Spec.ClusterID) | |
| import ( | |
| "fmt" | |
| component "github.com/openshift/hypershift/support/controlplane-component" | |
| "github.com/openshift/hypershift/support/metrics" | |
| "github.com/openshift/hypershift/support/util" | |
| prometheusoperatorv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" | |
| ) | |
| func adaptServiceMonitor(cpContext component.WorkloadContext, sm *prometheusoperatorv1.ServiceMonitor) error { | |
| sm.Spec.NamespaceSelector = prometheusoperatorv1.NamespaceSelector{ | |
| MatchNames: []string{sm.Namespace}, | |
| } | |
| if len(sm.Spec.Endpoints) == 0 { | |
| return fmt.Errorf("kube-scheduler ServiceMonitor must define at least one endpoint") | |
| } | |
| sm.Spec.Endpoints[0].MetricRelabelConfigs = metrics.SchedulerRelabelConfigs(cpContext.MetricsSet) | |
| util.ApplyClusterIDLabel(&sm.Spec.Endpoints[0], cpContext.HCP.Spec.ClusterID) | |
| } |
🤖 Prompt for 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.
In
`@control-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/servicemonitor.go`
around lines 11 - 16, The adaptServiceMonitor function currently indexes
sm.Spec.Endpoints[0] without verifying endpoints exist, which can panic; update
adaptServiceMonitor to first check len(sm.Spec.Endpoints) > 0 and return a clear
error (or create/append a default endpoint if desired) when it is zero, then
only set MetricRelabelConfigs and call util.ApplyClusterIDLabel on
sm.Spec.Endpoints[0]; reference the adaptServiceMonitor function,
sm.Spec.Endpoints, MetricRelabelConfigs, and util.ApplyClusterIDLabel when
making the change.
2f742aa to
2f9a908
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
control-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/servicemonitor.go (1)
16-20:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGuard endpoint indexing before mutating relabel configs.
Line 16 and Line 19 index
sm.Spec.Endpoints[0]/[1]without checking length; a malformed or future-changed manifest will panic reconciliation.Proposed fix
import ( + "fmt" + component "github.com/openshift/hypershift/support/controlplane-component" "github.com/openshift/hypershift/support/metrics" "github.com/openshift/hypershift/support/util" @@ func adaptServiceMonitor(cpContext component.WorkloadContext, sm *prometheusoperatorv1.ServiceMonitor) error { sm.Spec.NamespaceSelector = prometheusoperatorv1.NamespaceSelector{ MatchNames: []string{sm.Namespace}, } + if len(sm.Spec.Endpoints) < 2 { + return fmt.Errorf("kube-scheduler ServiceMonitor must define at least 2 endpoints, got %d", len(sm.Spec.Endpoints)) + } sm.Spec.Endpoints[0].MetricRelabelConfigs = metrics.SchedulerRelabelConfigs(cpContext.MetricsSet) util.ApplyClusterIDLabel(&sm.Spec.Endpoints[0], cpContext.HCP.Spec.ClusterID) sm.Spec.Endpoints[1].MetricRelabelConfigs = metrics.SchedulerResourceRelabelConfigs(cpContext.MetricsSet) util.ApplyClusterIDLabel(&sm.Spec.Endpoints[1], cpContext.HCP.Spec.ClusterID)As per coding guidelines: “Avoid panics in Go except in truly unrecoverable cases.”
🤖 Prompt for 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. In `@control-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/servicemonitor.go` around lines 16 - 20, The code mutates sm.Spec.Endpoints[0] and [1] without checking that sm.Spec.Endpoints has at least two elements, which can panic; before assigning MetricRelabelConfigs or calling util.ApplyClusterIDLabel, guard with length checks (e.g., if len(sm.Spec.Endpoints) > 0 { ... } and if len(sm.Spec.Endpoints) > 1 { ... }) or ensure the slice is initialized/populated with the expected endpoints, and then call metrics.SchedulerRelabelConfigs(cpContext.MetricsSet), metrics.SchedulerResourceRelabelConfigs(cpContext.MetricsSet) and util.ApplyClusterIDLabel(... cpContext.HCP.Spec.ClusterID) only after confirming the corresponding endpoint exists.Source: Coding guidelines
🤖 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.
Duplicate comments:
In
`@control-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/servicemonitor.go`:
- Around line 16-20: The code mutates sm.Spec.Endpoints[0] and [1] without
checking that sm.Spec.Endpoints has at least two elements, which can panic;
before assigning MetricRelabelConfigs or calling util.ApplyClusterIDLabel, guard
with length checks (e.g., if len(sm.Spec.Endpoints) > 0 { ... } and if
len(sm.Spec.Endpoints) > 1 { ... }) or ensure the slice is initialized/populated
with the expected endpoints, and then call
metrics.SchedulerRelabelConfigs(cpContext.MetricsSet),
metrics.SchedulerResourceRelabelConfigs(cpContext.MetricsSet) and
util.ApplyClusterIDLabel(... cpContext.HCP.Spec.ClusterID) only after confirming
the corresponding endpoint exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7ddbd74d-62f2-46e8-8921-746d5e7e7fe5
📒 Files selected for processing (7)
control-plane-operator/controllers/hostedcontrolplane/v2/assets/kube-scheduler/servicemonitor.yamlcontrol-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/servicemonitor.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/servicemonitor_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/rbac.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/rbac/reconcile.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.gosupport/metrics/sets.go
🚧 Files skipped from review as they are similar to previous changes (3)
- control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go
- control-plane-operator/hostedclusterconfigoperator/controllers/resources/rbac/reconcile.go
- control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/rbac.go
The built-in system:monitoring ClusterRole only grants access to /metrics. This adds a ClusterRole and ClusterRoleBinding in the guest cluster to allow the metrics client to scrape /metrics/resources on kube-scheduler, which exposes kube_pod_resource_request and kube_pod_resource_limit metrics.
2f9a908 to
7fc8f82
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
support/metrics/sets_test.go (1)
12-38: ⚡ Quick winStrengthen SRE/All assertions to validate the actual contract.
The length-only check (
len(got)) does not verify the “should return nil” behavior forMetricsSetAll, and hardcoding SRE as0is weaker than asserting against the configured SRE slice. Please assertnilforMetricsSetAlland compare SRE tosreMetricsSetConfig.KubeSchedulerResourceMetrics.🤖 Prompt for 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. In `@support/metrics/sets_test.go` around lines 12 - 38, The test in the loop over the test cases is performing weak assertions by only comparing the length of the returned slice. For the MetricsSetAll case, instead of checking len(got) != 0, explicitly assert that got is nil to properly validate the "should return nil" contract. For the MetricsSetSRE case, replace the hardcoded want value of 0 with a comparison against the actual configured SRE slice sreMetricsSetConfig.KubeSchedulerResourceMetrics, and update the test assertion logic to compare the returned slice directly against the configured value rather than just checking its length. This ensures the SchedulerResourceRelabelConfigs function returns the correct configured values for each metrics set.
🤖 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.
Nitpick comments:
In `@support/metrics/sets_test.go`:
- Around line 12-38: The test in the loop over the test cases is performing weak
assertions by only comparing the length of the returned slice. For the
MetricsSetAll case, instead of checking len(got) != 0, explicitly assert that
got is nil to properly validate the "should return nil" contract. For the
MetricsSetSRE case, replace the hardcoded want value of 0 with a comparison
against the actual configured SRE slice
sreMetricsSetConfig.KubeSchedulerResourceMetrics, and update the test assertion
logic to compare the returned slice directly against the configured value rather
than just checking its length. This ensures the SchedulerResourceRelabelConfigs
function returns the correct configured values for each metrics set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 215f91a8-b71c-4e70-843b-c1ab7feb7838
⛔ Files ignored due to path filters (5)
control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/AROSwift/zz_fixture_TestControlPlaneComponents_kube_scheduler_servicemonitor.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/GCP/zz_fixture_TestControlPlaneComponents_kube_scheduler_servicemonitor.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/IBMCloud/zz_fixture_TestControlPlaneComponents_kube_scheduler_servicemonitor.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_kube_scheduler_servicemonitor.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-scheduler/zz_fixture_TestControlPlaneComponents_kube_scheduler_servicemonitor.yamlis excluded by!**/testdata/**
📒 Files selected for processing (9)
control-plane-operator/controllers/hostedcontrolplane/v2/assets/kube-scheduler/servicemonitor.yamlcontrol-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/servicemonitor.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/servicemonitor_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/rbac.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/rbac/reconcile.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/rbac/reconcile_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.gosupport/metrics/sets.gosupport/metrics/sets_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- support/metrics/sets.go
- control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/rbac.go
- control-plane-operator/controllers/hostedcontrolplane/v2/assets/kube-scheduler/servicemonitor.yaml
- control-plane-operator/hostedclusterconfigoperator/controllers/resources/rbac/reconcile.go
- control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
dhgautam99
changed the title
|
/approve |
openshift-ci
Bot
added
the
approved
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, dhgautam99, muraee 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 |
|
/test e2e-aks |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
/test e2e-aks |
|
/test e2e-kubevirt-aws-ovn-reduced |
|
Now I have the full picture. Let me produce the final report. Timeline:
The Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThis job was manually aborted by the PR author Root CauseThe PR author
At the time of the abort, the CI pipeline was still in the image build phase — specifically, the No multi-stage test steps (pre/test/post phases) were ever executed. No cluster was provisioned. No tests ran. The failure is purely due to the user-initiated abort. Recommendations
Evidence
|
|
/test e2e-kubevirt-aws-ovn-reduced |
|
/verified by @vsolanki12 below endpoint didn't exist and /metrics/resources was not scraping: After fix: Servicemonitor is getting successfully recreated, if it is deleted by any user. |
openshift-ci-robot
added
the
verified
|
@vsolanki12: 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. |
|
@dhgautam99: 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. |
6 participants
What this PR does / why we need it:
Adds guest-cluster RBAC to allow the
metrics-clientcertificate identity (system:serviceaccount:hypershift:prometheus) to access the kube-scheduler/metrics/resourcesendpoint.The built-in
system:monitoringClusterRole only grants GET on/metricsand/metrics/slis. The/metrics/resourcesendpoint (KEP-1748, exposingkube_pod_resource_requestandkube_pod_resource_limit) requires a separate authorization grant. Without this, prometheus-user-workload on the management cluster receives a 403 Forbidden when scraping/metrics/resourcesvia the ServiceMonitor.This PR creates a new
hypershift-metrics-resources-readerClusterRole and ClusterRoleBinding in the guest cluster via HCCO, granting GET on/metrics/resourcesto the metrics-client identity.Which issue(s) this PR fixes:
Fixes CNTRLPLANE-3584
Depends on #8489
Special notes for your reviewer:
system:monitoringClusterRoleBinding--authorization-kubeconfigpointing to the guest cluster KAS, so RBAC checks for metrics endpoints happen on the guest cluster even though the scheduler runs on the management cluster/metrics/resourcesendpoint only returns data on the leader scheduler podChecklist:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
/metrics/resources, with TLS and cluster ID labeling applied to both endpoints.Chores
/metrics/resourcesscrape target.Tests