OCPBUGS-100302: fix(metrics): only report limited support when label … - #9185
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@holysoles: This pull request references Jira Issue OCPBUGS-100302, which is invalid:
Comment 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. |
📝 WalkthroughWalkthroughThe limited-support metric now reports enabled only when Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hypershift-operator/controllers/hostedcluster/metrics/metrics.go (1)
418-420: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression case for an explicit
"false"label.The implementation correctly distinguishes
"true"from other values, butmetrics_test.gocurrently covers only"true"and an absent label. Add"false"with an expected metric value of0to protect the OCPBUGS-100302 fix from regressing to key-presence-only logic.🤖 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 `@hypershift-operator/controllers/hostedcluster/metrics/metrics.go` around lines 418 - 420, Add a regression case in metrics_test.go for a present hyperv1.LimitedSupportLabel set explicitly to "false", and assert the resulting metric value is 0. Keep the existing "true" and absent-label cases unchanged so the test verifies value-based handling rather than key presence.
🤖 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 `@hypershift-operator/controllers/hostedcluster/metrics/metrics.go`:
- Around line 418-420: Add a regression case in metrics_test.go for a present
hyperv1.LimitedSupportLabel set explicitly to "false", and assert the resulting
metric value is 0. Keep the existing "true" and absent-label cases unchanged so
the test verifies value-based handling rather than key presence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3899e0ec-c3aa-434e-921c-e83a0f77c836
📒 Files selected for processing (1)
hypershift-operator/controllers/hostedcluster/metrics/metrics.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9185 +/- ##
=======================================
Coverage 44.77% 44.77%
=======================================
Files 775 775
Lines 97210 97210
=======================================
Hits 43528 43528
Misses 50681 50681
Partials 3001 3001
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
/jira refresh |
|
@holysoles: This pull request references Jira Issue OCPBUGS-100302, which is invalid:
Comment 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. |
|
/jira refresh |
|
@holysoles: This pull request references Jira Issue OCPBUGS-100302, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
d3fa737 to
982db0f
Compare
|
@holysoles: This pull request references Jira Issue OCPBUGS-100302, which is valid. 3 validation(s) were run on this bug
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. |
CS sets api.openshift.com/limited-support to false rather than deleting it when exiting limited support, because HC-to-HCP label sync is additive-only (OCPBUGS-85584). The metric collector only checked key presence, so clusters that entered limited support permanently reported 1.0 even after support was restored. Check the value is true before reporting the metric as enabled.
982db0f to
d28b683
Compare
|
Scheduling tests matching the |
|
/retest |
|
/retest-required |
Test Resultse2e-aws
e2e-aks
|
|
/retest-required |
1 similar comment
|
/retest-required |
|
/approve |
|
/retest-required |
|
/verified by @holysoles via unit tests |
|
@joshbranham: 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: holysoles, ironcladlou 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 |
|
@holysoles: 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. |
|
@holysoles: Jira Issue Verification Checks: Jira Issue OCPBUGS-100302 Jira Issue OCPBUGS-100302 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
|
Fix included in release 5.0.0-0.nightly-2026-08-04-023110 |
What this PR does / why we need it:
The
hypershift_cluster_limited_support_enabledmetric currently reports1if theapi.openshift.com/limited-supportlabel is present, no matter the value.clusters-service intentionally sets the label as
falserather than deleting it when a cluster exists limited support, because the HC-to-HCP label sync is additive-only (OCPBUGS-85584).This means clusters that enter limited support continue reporting
hypershift_cluster_limited_support_enabled = 1even when they exit limited support. This affects our ability to correctly silence alerts.Which issue(s) this PR fixes:
Fixes OCPBUGS-100302
Special notes for your reviewer:
Checklist:
Summary by CodeRabbit
true.