OTA-1959: Case 2 - oc adm upgrade recommend correctly filters out accepted risks from both sources - #2355
Open
nbottari9 wants to merge 1 commit into
Open
Conversation
… --version [VERSION]` correctly filters out both graph- and alert-sourced risks when they're accepted. Signed-off-by: Nicholas Bottari <nbottari9@gmail.com>
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@nbottari9: This pull request references OTA-1959 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. |
WalkthroughChangesAccepted upgrade risks
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 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 |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nbottari9 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
`@pkg/cli/admin/upgrade/recommend/examples/5.0.0-accepted-graph-and-alert-risks.json`:
- Line 1: Rename the fixture file so its basename is
5.0.0-accepted-graph-and-alert-risks-alerts.json, matching the alert fixture
path derived by options.Complete. Preserve the existing JSON contents unchanged.
🪄 Autofix
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: Pro Plus
Run ID: e7f03290-e227-412f-b0a5-1ec374857f58
📒 Files selected for processing (8)
pkg/cli/admin/upgrade/recommend/examples/5.0.0-accepted-graph-and-alert-risks-cv.yamlpkg/cli/admin/upgrade/recommend/examples/5.0.0-accepted-graph-and-alert-risks-featuregate.yamlpkg/cli/admin/upgrade/recommend/examples/5.0.0-accepted-graph-and-alert-risks-infrastructure.yamlpkg/cli/admin/upgrade/recommend/examples/5.0.0-accepted-graph-and-alert-risks.jsonpkg/cli/admin/upgrade/recommend/examples/5.0.0-accepted-graph-and-alert-risks.outputpkg/cli/admin/upgrade/recommend/examples/5.0.0-accepted-graph-and-alert-risks.show-outdated-releases-outputpkg/cli/admin/upgrade/recommend/examples/5.0.0-accepted-graph-and-alert-risks.version-5.0.0-ec.5-outputpkg/cli/admin/upgrade/recommend/examples_test.go
| @@ -0,0 +1 @@ | |||
| {"status":"success","data":{"alerts":[{"labels":{"alertname":"OpenShiftUpdateRiskMightApply","namespace":"openshift-cluster-version","reason":"Alert:firing","risk":"TestAlert","severity":"warning"},"annotations":{"description":"The conditional update risk TestAlert might apply to the cluster because of Alert:firing, and the cluster update to a version exposed to the risk is not recommended. For more information refer to 'oc adm upgrade'.","runbook_url":"https://github.com/openshift/runbooks/blob/master/alerts/cluster-version-operator/OpenShiftUpdateRiskMightApply.md","summary":"The cluster might have been exposed to the conditional update risk for 15 minutes."},"state":"pending","activeAt":"2026-08-05T16:37:45.987948515Z","value":"1e+00","partialResponseStrategy":"WARN"},{"labels":{"alertname":"OpenShiftUpdateRiskMightApply","namespace":"openshift-cluster-version","reason":"Match","risk":"SyntheticRiskA","severity":"warning"},"annotations":{"description":"The conditional update risk SyntheticRiskA might apply to the cluster because of Match, and the cluster update to a version exposed to the risk is not recommended. For more information refer to 'oc adm upgrade'.","runbook_url":"https://github.com/openshift/runbooks/blob/master/alerts/cluster-version-operator/OpenShiftUpdateRiskMightApply.md","summary":"The cluster might have been exposed to the conditional update risk for 15 minutes."},"state":"pending","activeAt":"2026-08-05T16:34:45.987948515Z","value":"1e+00","partialResponseStrategy":"WARN"},{"labels":{"alertname":"OpenShiftUpdateRiskMightApply","namespace":"openshift-cluster-version","reason":"Match","risk":"SyntheticRiskB","severity":"warning"},"annotations":{"description":"The conditional update risk SyntheticRiskB might apply to the cluster because of Match, and the cluster update to a version exposed to the risk is not recommended. For more information refer to 'oc adm upgrade'.","runbook_url":"https://github.com/openshift/runbooks/blob/master/alerts/cluster-version-operator/OpenShiftUpdateRiskMightApply.md","summary":"The cluster might have been exposed to the conditional update risk for 15 minutes."},"state":"pending","activeAt":"2026-08-05T16:34:45.987948515Z","value":"1e+00","partialResponseStrategy":"WARN"},{"labels":{"alertname":"OpenShiftUpdateRiskMightApply","namespace":"openshift-cluster-version","reason":"Match","risk":"SyntheticRiskC","severity":"warning"},"annotations":{"description":"The conditional update risk SyntheticRiskC might apply to the cluster because of Match, and the cluster update to a version exposed to the risk is not recommended. For more information refer to 'oc adm upgrade'.","runbook_url":"https://github.com/openshift/runbooks/blob/master/alerts/cluster-version-operator/OpenShiftUpdateRiskMightApply.md","summary":"The cluster might have been exposed to the conditional update risk for 15 minutes."},"state":"pending","activeAt":"2026-08-05T16:34:45.987948515Z","value":"1e+00","partialResponseStrategy":"WARN"},{"labels":{"alertname":"ClusterNotUpgradeable","condition":"Upgradeable","endpoint":"metrics","name":"version","namespace":"openshift-cluster-version","severity":"info"},"annotations":{"description":"In most cases, you will still be able to apply patch releases. Reason MultipleReasons. For more information refer to 'oc adm upgrade' or https://console-openshift-console.apps.ci-ln-6tjdy82-72292.gcp-2.ci.openshift.org/settings/cluster/.","summary":"One or more cluster operators have been blocking minor or major version cluster updates for at least an hour."},"state":"pending","activeAt":"2026-08-05T15:56:40.669438478Z","value":"0e+00","partialResponseStrategy":"WARN"},{"labels":{"alertname":"UpdateAvailable","channel":"risks-always","namespace":"openshift-cluster-version","severity":"info","upstream":"https://fauxinnati-fauxinnati.apps.ota-stage.q2z4.p1.openshiftapps.com/api/upgrades_info/graph?channel=risks-always\u0026version=5.0.0-ec.2\u0026arch=amd64"},"annotations":{"description":"For more information refer to 'oc adm upgrade' or https://console-openshift-console.apps.ci-ln-6tjdy82-72292.gcp-2.ci.openshift.org/settings/cluster/.","summary":"Your upstream update recommendation service recommends you update your cluster."},"state":"firing","activeAt":"2026-08-05T16:40:10.741596918Z","value":"2e+00","partialResponseStrategy":"WARN"},{"labels":{"alertname":"InsightsRecommendationActive","container":"insights-operator","description":"Enabling the **TechPreviewNoUpgrade** feature set on your cluster\ncan not be undone and prevents minor version updates. Please do\nnot enable this feature set on production clusters.\n","endpoint":"https","info_link":"https://console.redhat.com/openshift/insights/advisor/clusters/d275ee16-3918-407d-8d56-981eb2ce7ae2?first=ccx_rules_ocp.external.rules.upgrade_is_blocked_due_to_tpfg%7CTECH_PREVIEW_NO_UPGRADE_FEATURE_SET_IS_ENABLED","instance":"10.128.0.14:8443","job":"metrics","namespace":"openshift-insights","pod":"insights-operator-584747df49-dznkz","service":"metrics","severity":"info","total_risk":"Important"},"annotations":{"description":"Insights recommendation \"Enabling the **TechPreviewNoUpgrade** feature set on your cluster\ncan not be undone and prevents minor version updates. Please do\nnot enable this feature set on production clusters.\n\" with total risk \"Important\" was detected on the cluster. More information is available at https://console.redhat.com/openshift/insights/advisor/clusters/d275ee16-3918-407d-8d56-981eb2ce7ae2?first=ccx_rules_ocp.external.rules.upgrade_is_blocked_due_to_tpfg%7CTECH_PREVIEW_NO_UPGRADE_FEATURE_SET_IS_ENABLED.","summary":"An Insights recommendation is active for this cluster."},"state":"firing","activeAt":"2026-08-05T15:56:45.536087262Z","value":"1e+00","partialResponseStrategy":"WARN"},{"labels":{"alertname":"TechPreviewNoUpgrade","container":"kube-apiserver-operator","endpoint":"https","instance":"10.128.0.34:8443","job":"kube-apiserver-operator","name":"TechPreviewNoUpgrade","namespace":"openshift-kube-apiserver-operator","pod":"kube-apiserver-operator-7d998989dd-x9q7g","service":"metrics","severity":"warning"},"annotations":{"description":"Cluster has enabled Technology Preview features that cannot be undone and will prevent upgrades. The TechPreviewNoUpgrade feature set is not recommended on production clusters.","summary":"Cluster has enabled tech preview features that will prevent upgrades."},"state":"firing","activeAt":"2026-08-05T15:57:01.406855878Z","value":"0e+00","partialResponseStrategy":"WARN"},{"labels":{"alertname":"Watchdog","namespace":"openshift-monitoring","severity":"none"},"annotations":{"description":"This is an alert meant to ensure that the entire alerting pipeline is functional.\nThis alert is always firing, therefore it should always be firing in Alertmanager\nand always fire against a receiver. There are integrations with various notification\nmechanisms that send a notification when this alert is not firing. For example the\n\"DeadMansSnitch\" integration in PagerDuty.\n","summary":"An alert that should always be firing to certify that Alertmanager is working properly."},"state":"firing","activeAt":"2026-08-05T15:56:06.311672714Z","value":"1e+00","partialResponseStrategy":"WARN"},{"labels":{"alertname":"TargetDown","job":"check-endpoints","namespace":"openshift-apiserver","service":"check-endpoints","severity":"warning"},"annotations":{"description":"100% of the check-endpoints/check-endpoints targets in openshift-apiserver namespace have been unreachable for more than 15 minutes. This may be a symptom of network connectivity issues, down nodes, or failures within these components. Assess the health of the infrastructure and nodes running these targets and then contact support.","runbook_url":"https://github.com/openshift/runbooks/blob/master/alerts/cluster-monitoring-operator/TargetDown.md","summary":"Some targets were not reachable from the monitoring server for an extended period of time."},"state":"firing","activeAt":"2026-08-05T15:56:40.936046152Z","value":"1e+02","partialResponseStrategy":"WARN"},{"labels":{"alertname":"AlertmanagerReceiversNotConfigured","namespace":"openshift-monitoring","severity":"warning"},"annotations":{"description":"Alerts are not configured to be sent to a notification system, meaning that you may not be notified in a timely fashion when important failures occur. Check the OpenShift documentation to learn how to configure notifications with Alertmanager.","summary":"Receivers (notification integrations) are not configured on Alertmanager"},"state":"firing","activeAt":"2026-08-05T15:56:15.295812914Z","value":"0e+00","partialResponseStrategy":"WARN"},{"labels":{"alertname":"TestAlert","openShiftUpdatePrecheck":"true","severity":"warning"},"annotations":{"description":"Test alert for updates","summary":"Test alert for updates"},"state":"firing","activeAt":"2026-08-05T16:35:19.532828786Z","value":"1e+00","partialResponseStrategy":"WARN"}]}} No newline at end of file | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Rename this file to the required alert fixture name.
options.Complete derives examples/5.0.0-accepted-graph-and-alert-risks-alerts.json from the ClusterVersion fixture path. The current filename does not match that path. Every test variant fails during Complete before output comparison.
Proposed rename
- pkg/cli/admin/upgrade/recommend/examples/5.0.0-accepted-graph-and-alert-risks.json
+ pkg/cli/admin/upgrade/recommend/examples/5.0.0-accepted-graph-and-alert-risks-alerts.json🤖 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
`@pkg/cli/admin/upgrade/recommend/examples/5.0.0-accepted-graph-and-alert-risks.json`
at line 1, Rename the fixture file so its basename is
5.0.0-accepted-graph-and-alert-risks-alerts.json, matching the alert fixture
path derived by options.Complete. Preserve the existing JSON contents unchanged.
Contributor
Author
|
/hold did not have the correct name for alerts file, so it was not being factored in |
openshift-ci
Bot
added
the
do-not-merge/hold
Contributor
|
@nbottari9: 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. |
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
2 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.
test-fixtures: add new test fixture to test
oc adm upgrade recommend --version [VERSION]correctly filters out both graph- and alert-sourced risks when they're accepted.See Case 1 (#2349)
Summary by CodeRabbit
New Features
Tests
5.0.0-ec.5and behavior when accepted risks are present.