Open
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@hongkailiu: This pull request references OTA-1548 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 "4.22.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. |
WalkthroughAdds a new Cobra subcommand to manage ClusterVersion.spec.desiredUpdate.acceptRisks, wires it behind a feature gate, propagates AcceptRisks handling into upgrade and cancel flows, bumps two dependencies, adds unit tests for risk computation, and makes small CLI/describer test adjustments. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "oc upgrade accept\n(Cobra cmd)"
participant Client as "ClusterVersion client"
participant API as "Kubernetes API / ClusterVersion"
User->>CLI: invoke command (tokens, --replace/--clear)
CLI->>CLI: parse flags, validate args
CLI->>Client: GET ClusterVersion
Client->>API: request cv
API-->>Client: return cv (includes spec.desiredUpdate.acceptRisks)
Client-->>CLI: return cv
CLI->>CLI: compute new acceptRisks (getAcceptRisks)
alt changes required
CLI->>Client: PATCH ClusterVersion (JSON Merge Patch)
Client->>API: apply patch
API-->>Client: patched cv
Client-->>CLI: success
CLI->>User: print updated acceptRisks
else no changes
CLI->>User: print "no changes"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
hongkailiu
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
openshift-ci
bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
pkg/cli/admin/upgrade/accept/accept.go (1)
134-136: Placeholder logic pending API update.The hardcoded fake risks bypass actual ClusterVersion data. Ensure this is tracked for completion once the
o/apidependency is updated.Would you like me to open an issue to track this TODO?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
pkg/cli/admin/upgrade/accept/accept.gopkg/cli/admin/upgrade/upgrade.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
pkg/cli/admin/upgrade/accept/accept.gopkg/cli/admin/upgrade/upgrade.go
🧬 Code graph analysis (2)
pkg/cli/admin/upgrade/accept/accept.go (1)
pkg/cli/admin/upgrade/upgrade.go (1)
New(56-132)
pkg/cli/admin/upgrade/upgrade.go (1)
pkg/cli/admin/upgrade/accept/accept.go (1)
New(29-57)
🔇 Additional comments (1)
pkg/cli/admin/upgrade/upgrade.go (1)
28-28: LGTM!The import and feature gate wiring follow the established pattern used for the
statusandrollbacksubcommands.Also applies to: 126-128
hongkailiu
force-pushed
the
accept-cmd
branch
2 times, most recently
from
b4dc31c to
1efbc2c
Compare
hongkailiu
force-pushed
the
accept-cmd
branch
from
1efbc2c to
7e52894
Compare
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: hongkailiu 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 |
openshift-ci
bot
removed
the
approved
hongkailiu
force-pushed
the
accept-cmd
branch
from
7e52894 to
62e2d9a
Compare
Member
Author
|
I will wait a bit on |
hongkailiu
force-pushed
the
accept-cmd
branch
6 times, most recently
from
87f10fc to
583aa51
Compare
Member
Author
|
Cluster bot: Testing results with 583aa51: So we showed that the oc/pkg/cli/admin/upgrade/upgrade.go Line 688 in 90d7ae6 |
wking
reviewed
Jan 30, 2026
wking
reviewed
Jan 30, 2026
wking
reviewed
Jan 30, 2026
wking
reviewed
Jan 30, 2026
wking
reviewed
Jan 30, 2026
wking
reviewed
Jan 30, 2026
Member
Author
|
Tested with 254493c |
|
I just noticed an issue that we do not show help info for Right now there are descriptions for |
|
@hongkailiu I mean we need a command help info, like what we have for |
wking
reviewed
Mar 2, 2026
|
@hongkailiu: This pull request references OTA-1548 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 "4.22.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. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
pkg/cli/admin/upgrade/upgrade.go (1)
236-236: Use existing request context for the clear patch call.At Line [236], using
context.TODO()bypasses thectxalready established in this method. Prefer passingctxfor cancellation/deadline propagation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/upgrade.go` at line 236, The Patch call using context.TODO() should use the existing request context for cancellation/deadline propagation: replace the context.TODO() argument in o.Client.ConfigV1().ClusterVersions().Patch(...) with the method's ctx variable so the Patch operation honors the caller's context.pkg/cli/admin/upgrade/accept/accept.go (1)
57-57: Remove commented-out command config.At Line [57], stale commented code (
//Hidden: true,) is confusing now that visibility is intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/accept/accept.go` at line 57, Remove the stale commented-out command configuration line "//Hidden: true," from the accept command definition to avoid confusion; locate the accept command (e.g., the acceptCmd or NewAcceptCmd command/constructor in accept.go) and delete that commented line so the command configuration contains only active fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/cli/admin/upgrade/accept/accept_test.go`:
- Around line 55-56: The test's diff argument order is inverted: change the
cmp.Diff call in the accept_test.go assertion from cmp.Diff(actual,
testCase.expected) to cmp.Diff(testCase.expected, actual) so the produced diff
matches the "(-want +got)" label; this affects the assertion around
getAcceptRisks() where variables actual and testCase.expected are compared using
cmp.Diff.
In `@pkg/cli/admin/upgrade/accept/accept.go`:
- Line 49: The long help text for the accept command contains a typo: change the
phrase "excepted risks" to "accepted risks" in the long description string (the
Long/LongHelp text associated with the accept command, e.g., in the acceptCmd
declaration or its help variable in accept.go) so the user-facing help displays
"accepted risks".
- Around line 155-156: The call to patchDesiredUpdate currently uses
context.TODO(), which prevents Run(ctx) cancellation/timeouts from propagating;
change the call to pass the incoming context (ctx) instead. Locate the
invocation of patchDesiredUpdate in the accept flow (the call using
patchDesiredUpdate(context.TODO(), acceptRisks,
o.Client.ConfigV1().ClusterVersions(), "version")) and replace the placeholder
context with the Run(ctx) parameter so patchDesiredUpdate receives the caller's
context; ensure any surrounding functions (e.g., Run(ctx)) keep ctx in scope and
thread it through to this call.
In `@pkg/cli/admin/upgrade/upgrade.go`:
- Around line 269-271: The code dereferences cv.Spec.DesiredUpdate.AcceptRisks
without guarding for nil; update the logic around the assignment to
update.AcceptRisks so you first check that cv.Spec != nil and
cv.Spec.DesiredUpdate != nil before reading AcceptRisks (and fall back to a safe
default, e.g. false, when DesiredUpdate is nil). Apply the same nil-check fix at
the other occurrence referencing cv.Spec.DesiredUpdate.AcceptRisks (the second
instance noted) so both reads are protected and cannot panic.
---
Nitpick comments:
In `@pkg/cli/admin/upgrade/accept/accept.go`:
- Line 57: Remove the stale commented-out command configuration line "//Hidden:
true," from the accept command definition to avoid confusion; locate the accept
command (e.g., the acceptCmd or NewAcceptCmd command/constructor in accept.go)
and delete that commented line so the command configuration contains only active
fields.
In `@pkg/cli/admin/upgrade/upgrade.go`:
- Line 236: The Patch call using context.TODO() should use the existing request
context for cancellation/deadline propagation: replace the context.TODO()
argument in o.Client.ConfigV1().ClusterVersions().Patch(...) with the method's
ctx variable so the Patch operation honors the caller's context.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
go.modpkg/cli/admin/upgrade/accept/accept.gopkg/cli/admin/upgrade/accept/accept_test.gopkg/cli/admin/upgrade/rollback/rollback.gopkg/cli/admin/upgrade/upgrade.gopkg/helpers/describe/describer_test.go
Comment on lines
+55
to
+56
| if diff := cmp.Diff(actual, testCase.expected); diff != "" { | ||
| t.Errorf("getAcceptRisks() mismatch (-want +got):\n%s", diff) |
There was a problem hiding this comment.
Fix cmp.Diff argument order in failure output.
At Line [55], cmp.Diff(actual, testCase.expected) conflicts with the (-want +got) label and inverts diff semantics.
Proposed fix
- if diff := cmp.Diff(actual, testCase.expected); diff != "" {
+ if diff := cmp.Diff(testCase.expected, actual); diff != "" {
t.Errorf("getAcceptRisks() mismatch (-want +got):\n%s", diff)
}📝 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.
Suggested change
| if diff := cmp.Diff(actual, testCase.expected); diff != "" { | |
| t.Errorf("getAcceptRisks() mismatch (-want +got):\n%s", diff) | |
| if diff := cmp.Diff(testCase.expected, actual); diff != "" { | |
| t.Errorf("getAcceptRisks() mismatch (-want +got):\n%s", diff) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/cli/admin/upgrade/accept/accept_test.go` around lines 55 - 56, The test's
diff argument order is inverted: change the cmp.Diff call in the accept_test.go
assertion from cmp.Diff(actual, testCase.expected) to
cmp.Diff(testCase.expected, actual) so the produced diff matches the "(-want
+got)" label; this affects the assertion around getAcceptRisks() where variables
actual and testCase.expected are compared using cmp.Diff.
| removal if it exists and no-ops otherwise. If --replace is specified, the prefix "-" on the risks | ||
| is not allowed. | ||
|
|
||
| Passing --clear removes all existing excepted risks. |
There was a problem hiding this comment.
User-facing typo in long help text.
At Line [49], “excepted risks” should be “accepted risks”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/cli/admin/upgrade/accept/accept.go` at line 49, The long help text for
the accept command contains a typo: change the phrase "excepted risks" to
"accepted risks" in the long description string (the Long/LongHelp text
associated with the accept command, e.g., in the acceptCmd declaration or its
help variable in accept.go) so the user-facing help displays "accepted risks".
Comment on lines
+155
to
+156
| if err := patchDesiredUpdate(context.TODO(), acceptRisks, o.Client.ConfigV1().ClusterVersions(), "version"); err != nil { | ||
| return err |
There was a problem hiding this comment.
Use the passed context for patch calls.
At Line [155], context.TODO() ignores the caller context from Run(ctx), so cancellation/timeouts won’t propagate to the API call.
Proposed fix
- if err := patchDesiredUpdate(context.TODO(), acceptRisks, o.Client.ConfigV1().ClusterVersions(), "version"); err != nil {
+ if err := patchDesiredUpdate(ctx, acceptRisks, o.Client.ConfigV1().ClusterVersions(), "version"); err != nil {
return err
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/cli/admin/upgrade/accept/accept.go` around lines 155 - 156, The call to
patchDesiredUpdate currently uses context.TODO(), which prevents Run(ctx)
cancellation/timeouts from propagating; change the call to pass the incoming
context (ctx) instead. Locate the invocation of patchDesiredUpdate in the accept
flow (the call using patchDesiredUpdate(context.TODO(), acceptRisks,
o.Client.ConfigV1().ClusterVersions(), "version")) and replace the placeholder
context with the Run(ctx) parameter so patchDesiredUpdate receives the caller's
context; ensure any surrounding functions (e.g., Run(ctx)) keep ctx in scope and
thread it through to this call.
Comment on lines
+269
to
+271
| if update != nil { | ||
| update.AcceptRisks = cv.Spec.DesiredUpdate.AcceptRisks | ||
| } |
There was a problem hiding this comment.
Guard cv.Spec.DesiredUpdate before reading AcceptRisks.
At Line [270] and Line [411], dereferencing cv.Spec.DesiredUpdate.AcceptRisks can panic when cv.Spec.DesiredUpdate == nil (valid state for clusters with no desired update set).
Proposed fix
- if update != nil {
- update.AcceptRisks = cv.Spec.DesiredUpdate.AcceptRisks
- }
+ if cv.Spec.DesiredUpdate != nil {
+ update.AcceptRisks = append([]configv1.AcceptRisk(nil), cv.Spec.DesiredUpdate.AcceptRisks...)
+ }
...
- update.AcceptRisks = cv.Spec.DesiredUpdate.AcceptRisks
+ if cv.Spec.DesiredUpdate != nil {
+ update.AcceptRisks = append([]configv1.AcceptRisk(nil), cv.Spec.DesiredUpdate.AcceptRisks...)
+ }Also applies to: 411-411
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/cli/admin/upgrade/upgrade.go` around lines 269 - 271, The code
dereferences cv.Spec.DesiredUpdate.AcceptRisks without guarding for nil; update
the logic around the assignment to update.AcceptRisks so you first check that
cv.Spec != nil and cv.Spec.DesiredUpdate != nil before reading AcceptRisks (and
fall back to a safe default, e.g. false, when DesiredUpdate is nil). Apply the
same nil-check fix at the other occurrence referencing
cv.Spec.DesiredUpdate.AcceptRisks (the second instance noted) so both reads are
protected and cannot panic.
Member
Author
$ git --no-pager log --pretty=oneline -1
506aaa41ab10afc01415cf65a4d5e1250953d4c8 (HEAD -> accept-cmd) Stop hiding rollback cmd
$ OC_ENABLE_CMD_UPGRADE_ACCEPT_RISKS=true OC_ENABLE_CMD_UPGRADE_ROLLBACK=true ./oc adm upgrade -h | grep 'Available Commands:' -A 7
Available Commands:
accept Accept risks exposed to conditional updates.
channel Set or clear the update channel
recommend Displays cluster update recommendations.
rollback Rollback the cluster to the previous release.
status Display the status of the current cluster version update or multi-arch migration
Options:
$ OC_ENABLE_CMD_UPGRADE_ROLLBACK=true ./oc adm upgrade -h | grep 'Available Commands:' -A 7
Available Commands:
channel Set or clear the update channel
recommend Displays cluster update recommendations.
rollback Rollback the cluster to the previous release.
status Display the status of the current cluster version update or multi-arch migration
Options:
--allow-explicit-upgrade=false:
$ OC_ENABLE_CMD_UPGRADE_ACCEPT_RISKS=true ./oc adm upgrade -h | grep 'Available Commands:' -A 7
Available Commands:
accept Accept risks exposed to conditional updates.
channel Set or clear the update channel
recommend Displays cluster update recommendations.
status Display the status of the current cluster version update or multi-arch migration
Options:
--allow-explicit-upgrade=false:
$ ./oc adm upgrade -h | grep 'Available Commands:' -A 7
Available Commands:
channel Set or clear the update channel
recommend Displays cluster update recommendations.
status Display the status of the current cluster version update or multi-arch migration
Options:
--allow-explicit-upgrade=false:
Upgrade even if the upgrade target is not listed in the available versions list.
Also did a search: Probable they are $ rg 'Hidden:' -g '!vendor/'
pkg/cli/serviceaccounts/newtoken.go
83: Hidden: true,
pkg/cli/serviceaccounts/create_kubeconfig.go
68: Hidden: true,
pkg/cli/serviceaccounts/subcommand.go
26: Hidden: true,
pkg/cli/serviceaccounts/gettoken.go
63: Hidden: true,
|
|
Some invalid behavior need your confirm: |
Member
Author
You found a design flaw. We should use |
|
@hongkailiu: This pull request references OTA-1548 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 "4.22.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. |
Member
Author
$ git --no-pager log --pretty=oneline -1
ddb58895eb8d4ee9fc6edfa6a5b492139e51e0ec (HEAD -> accept-cmd) Use risk- for removal instead of -risk
$ CGO_CFLAGS="-I/opt/homebrew/opt/heimdal/include" make oc
$ OC_ENABLE_CMD_UPGRADE_ACCEPT_RISKS=true ./oc adm upgrade accept riskA,riskB
info: Accept risks are [riskA, riskB]
$ OC_ENABLE_CMD_UPGRADE_ACCEPT_RISKS=true ./oc adm upgrade accept --clear
info: Accept risks are []
$ OC_ENABLE_CMD_UPGRADE_ACCEPT_RISKS=true ./oc adm upgrade accept --clear
info: Accept risks are not changed
$ OC_ENABLE_CMD_UPGRADE_ACCEPT_RISKS=true ./oc adm upgrade accept riskA,riskB
info: Accept risks are [riskA, riskB]
$ OC_ENABLE_CMD_UPGRADE_ACCEPT_RISKS=true ./oc adm upgrade accept riskA,riskB,riskC
info: Accept risks are [riskA, riskB, riskC]
$ OC_ENABLE_CMD_UPGRADE_ACCEPT_RISKS=true ./oc adm upgrade accept riskA,riskB --replace
info: Accept risks are [riskA, riskB]
$ oc get clusterversion version -o yaml | yq -y .spec
channel: simple
clusterID: 250943dd-e3b5-4010-b554-bf67572256d4
desiredUpdate:
acceptRisks:
- name: riskA
- name: riskB
architecture: ''
force: false
image: quay.io/openshift-release-dev/ocp-release@sha256:00000000000000000000000000000000000000000000000000000000003d5ef1
version: 4.22.1
overrides:
- group: config.openshift.io
kind: ClusterImagePolicy
name: openshift
namespace: ''
unmanaged: true
upstream: https://fauxinnati-fauxinnati.apps.ota-stage.q2z4.p1.openshiftapps.com/api/upgrades_info/graph
$ OC_ENABLE_CMD_UPGRADE_ACCEPT_RISKS=true ./oc adm upgrade accept riskA,riskB-
info: Accept risks are [riskA]
$ oc get clusterversion version -o yaml | yq -y .spec
channel: simple
clusterID: 250943dd-e3b5-4010-b554-bf67572256d4
desiredUpdate:
acceptRisks:
- name: riskA
architecture: ''
force: false
image: quay.io/openshift-release-dev/ocp-release@sha256:00000000000000000000000000000000000000000000000000000000003d5ef1
version: 4.22.1
overrides:
- group: config.openshift.io
kind: ClusterImagePolicy
name: openshift
namespace: ''
unmanaged: true
upstream: https://fauxinnati-fauxinnati.apps.ota-stage.q2z4.p1.openshiftapps.com/api/upgrades_info/graph |
With OC_ENABLE_CMD_UPGRADE_ACCEPT_RISKS=true, a new command `oc adm upgrade accept` is enabled. It accepts comma-separated risks exposed to an OpenShift release [1]. The risks are stored in `clusterversion/version`'s `.specs.desiredUpdate.acceptRisks`. [1]. https://docs.redhat.com/en/documentation/openshift_container_platform/4.18/html-single/updating_clusters/index#understanding-clusterversion-conditiontypes_understanding-openshift-updates
There was a problem hiding this comment.
♻️ Duplicate comments (2)
pkg/cli/admin/upgrade/accept/accept.go (2)
154-154:⚠️ Potential issue | 🟠 MajorPropagate
Run(ctx)context to the patch request.Line [154] uses
context.TODO(), so command cancellation/timeouts won’t reach the API patch call.✅ Proposed fix
- if err := patchDesiredUpdate(context.TODO(), acceptRisks, o.Client.ConfigV1().ClusterVersions(), "version"); err != nil { + if err := patchDesiredUpdate(ctx, acceptRisks, o.Client.ConfigV1().ClusterVersions(), "version"); err != nil { return err }#!/bin/bash # Verify there are no remaining patch calls that drop caller context in this file rg -nP 'patchDesiredUpdate\(context\.TODO\(' pkg/cli/admin/upgrade/accept/accept.go🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/accept/accept.go` at line 154, The patch call uses context.TODO() which drops the caller's Run(ctx) cancellation/timeout; change the call to pass the incoming context (use the Run(ctx) parameter) instead of context.TODO() so the API patch request inherits caller cancellation. Update the invocation of patchDesiredUpdate(...) in accept.go to use the ctx variable from Run(ctx) and ensure any helper wrappers (patchDesiredUpdate) accept and forward that context if not already.
49-49:⚠️ Potential issue | 🟡 MinorFix user-facing typo in help text.
Line [49] says “excepted risks”; this should be “accepted risks”.
✏️ Proposed fix
- Passing --clear removes all existing excepted risks. + Passing --clear removes all existing accepted risks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/accept/accept.go` at line 49, Update the user-facing help text for the --clear flag: find the help/usage string that currently reads "Passing --clear removes all existing excepted risks." (the --clear flag's description in the accept command) and change "excepted risks" to "accepted risks" so it reads "Passing --clear removes all existing accepted risks." Ensure only the typo in the flag description is changed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/cli/admin/upgrade/accept/accept.go`:
- Line 154: The patch call uses context.TODO() which drops the caller's Run(ctx)
cancellation/timeout; change the call to pass the incoming context (use the
Run(ctx) parameter) instead of context.TODO() so the API patch request inherits
caller cancellation. Update the invocation of patchDesiredUpdate(...) in
accept.go to use the ctx variable from Run(ctx) and ensure any helper wrappers
(patchDesiredUpdate) accept and forward that context if not already.
- Line 49: Update the user-facing help text for the --clear flag: find the
help/usage string that currently reads "Passing --clear removes all existing
excepted risks." (the --clear flag's description in the accept command) and
change "excepted risks" to "accepted risks" so it reads "Passing --clear removes
all existing accepted risks." Ensure only the typo in the flag description is
changed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7975d16a-f314-43b5-908f-12d70d662137
📒 Files selected for processing (2)
pkg/cli/admin/upgrade/accept/accept.gopkg/cli/admin/upgrade/rollback/rollback.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/cli/admin/upgrade/rollback/rollback.go
hongkailiu
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
@hongkailiu: This pull request references OTA-1548 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 "4.22.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. |
There was a problem hiding this comment.
♻️ Duplicate comments (3)
pkg/cli/admin/upgrade/upgrade.go (1)
269-271:⚠️ Potential issue | 🔴 CriticalGuard
cv.Spec.DesiredUpdatebefore readingAcceptRisks(panic risk).Both reads dereference
cv.Spec.DesiredUpdatewithout a nil check. Clusters with no desired update can hit a nil-pointer panic.Proposed fix
- if update != nil { - update.AcceptRisks = cv.Spec.DesiredUpdate.AcceptRisks - } + if cv.Spec.DesiredUpdate != nil { + update.AcceptRisks = append([]configv1.AcceptRisk(nil), cv.Spec.DesiredUpdate.AcceptRisks...) + } ... - update.AcceptRisks = cv.Spec.DesiredUpdate.AcceptRisks + if cv.Spec.DesiredUpdate != nil { + update.AcceptRisks = append([]configv1.AcceptRisk(nil), cv.Spec.DesiredUpdate.AcceptRisks...) + }Also applies to: 411-411
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/upgrade.go` around lines 269 - 271, The code reads cv.Spec.DesiredUpdate.AcceptRisks without guarding cv.Spec.DesiredUpdate which can cause a nil-pointer panic; update the logic around the assignment to first check that cv.Spec != nil and cv.Spec.DesiredUpdate != nil before reading AcceptRisks (e.g., only set update.AcceptRisks = cv.Spec.DesiredUpdate.AcceptRisks when cv.Spec.DesiredUpdate != nil), and apply the same nil-check pattern to the other occurrence that reads cv.Spec.DesiredUpdate.AcceptRisks (the second spot noted in the review).pkg/cli/admin/upgrade/accept/accept.go (2)
154-155:⚠️ Potential issue | 🟠 MajorUse
ctxin patch call instead ofcontext.TODO().
Run(ctx)receives caller context, but the patch call discards it. This prevents cancellation/timeout propagation.Proposed fix
- if err := patchDesiredUpdate(context.TODO(), acceptRisks, o.Client.ConfigV1().ClusterVersions(), "version"); err != nil { + if err := patchDesiredUpdate(ctx, acceptRisks, o.Client.ConfigV1().ClusterVersions(), "version"); err != nil { return err }#!/bin/bash # Verify whether patch call in accept.Run uses context.TODO instead of ctx. rg -n --type=go 'patchDesiredUpdate\(context\.TODO\(\)' pkg/cli/admin/upgrade/accept/accept.go rg -n --type=go 'func \(o \*options\) Run\(ctx context\.Context\)' pkg/cli/admin/upgrade/accept/accept.goExpected result:
- First command returns a match (current issue present).
- Second command confirms
Run(ctx)exists, soctxshould be threaded through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/accept/accept.go` around lines 154 - 155, The call to patchDesiredUpdate is using context.TODO() which discards the caller context; change the call in the Run(ctx context.Context) method to pass the received ctx instead of context.TODO() so cancellation/timeouts propagate (update the call to patchDesiredUpdate(ctx, acceptRisks, o.Client.ConfigV1().ClusterVersions(), "version")). Locate the call to patchDesiredUpdate in the options.Run (or accept.Run) function and replace context.TODO() with the ctx parameter.
49-49:⚠️ Potential issue | 🟡 MinorTypo in user-facing help text.
“excepted risks” should be “accepted risks”.
Proposed fix
- Passing --clear removes all existing excepted risks. + Passing --clear removes all existing accepted risks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/accept/accept.go` at line 49, There's a typo in the user-facing help text in accept.go: change the phrase "excepted risks" to "accepted risks" in the help/description string associated with the --clear flag (look for the string "Passing --clear removes all existing excepted risks" inside accept.go, likely in the command description or flag help for the Accept command or NewAcceptCmd); update that literal to "Passing --clear removes all existing accepted risks".
🧹 Nitpick comments (1)
pkg/helpers/describe/describer_test.go (1)
136-136: Please document why this group-version is exempted.Adding a new entry to
MissingDescriberGroupCoverageExceptionsbroadens the test allowlist. Add a short inline reason (or issue link) next to this entry so future reviewers know this is intentional.Proposed tweak
- {Group: "apiextensions.openshift.io", Version: "v1alpha1"}, + {Group: "apiextensions.openshift.io", Version: "v1alpha1"}, // intentional temporary exception; see <issue/link>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/helpers/describe/describer_test.go` at line 136, The test allowlist entry {Group: "apiextensions.openshift.io", Version: "v1alpha1"} in MissingDescriberGroupCoverageExceptions needs an inline comment explaining why it's exempted; update the test by adding a brief justification (e.g., "exempt due to X reason" or an issue/PR link) directly next to that entry so future reviewers understand the intentional omission and can reassess it later.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/cli/admin/upgrade/accept/accept.go`:
- Around line 154-155: The call to patchDesiredUpdate is using context.TODO()
which discards the caller context; change the call in the Run(ctx
context.Context) method to pass the received ctx instead of context.TODO() so
cancellation/timeouts propagate (update the call to patchDesiredUpdate(ctx,
acceptRisks, o.Client.ConfigV1().ClusterVersions(), "version")). Locate the call
to patchDesiredUpdate in the options.Run (or accept.Run) function and replace
context.TODO() with the ctx parameter.
- Line 49: There's a typo in the user-facing help text in accept.go: change the
phrase "excepted risks" to "accepted risks" in the help/description string
associated with the --clear flag (look for the string "Passing --clear removes
all existing excepted risks" inside accept.go, likely in the command description
or flag help for the Accept command or NewAcceptCmd); update that literal to
"Passing --clear removes all existing accepted risks".
In `@pkg/cli/admin/upgrade/upgrade.go`:
- Around line 269-271: The code reads cv.Spec.DesiredUpdate.AcceptRisks without
guarding cv.Spec.DesiredUpdate which can cause a nil-pointer panic; update the
logic around the assignment to first check that cv.Spec != nil and
cv.Spec.DesiredUpdate != nil before reading AcceptRisks (e.g., only set
update.AcceptRisks = cv.Spec.DesiredUpdate.AcceptRisks when
cv.Spec.DesiredUpdate != nil), and apply the same nil-check pattern to the other
occurrence that reads cv.Spec.DesiredUpdate.AcceptRisks (the second spot noted
in the review).
---
Nitpick comments:
In `@pkg/helpers/describe/describer_test.go`:
- Line 136: The test allowlist entry {Group: "apiextensions.openshift.io",
Version: "v1alpha1"} in MissingDescriberGroupCoverageExceptions needs an inline
comment explaining why it's exempted; update the test by adding a brief
justification (e.g., "exempt due to X reason" or an issue/PR link) directly next
to that entry so future reviewers understand the intentional omission and can
reassess it later.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 76cca185-dc7d-432e-bb5c-1d7c68b00c93
⛔ Files ignored due to path filters (105)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/.ci-operator.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.coderabbit.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/AGENTS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/Dockerfile.ocpis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/types_compatibilityrequirement.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_feature.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_scheduling.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/console/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/features.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machine/v1beta1/types_awsprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machine/v1beta1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/machine/v1beta1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/machineconfiguration/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machineconfiguration/v1/types_machineconfignode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machineconfiguration/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/machineconfiguration/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/machineconfiguration/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/machineconfiguration/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machineconfiguration/v1alpha1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machineconfiguration/v1alpha1/types_internalreleaseimage.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machineconfiguration/v1alpha1/types_osimagestream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machineconfiguration/v1alpha1/types_pinnedimageset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machineconfiguration/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/machineconfiguration/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/azureplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterimagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpserviceendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicyfulciocawithrekorrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicypkirootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicypublickeyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagesigstoreverificationpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openstackplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ovirtplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vsphereplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clusterimagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicyfulciocawithrekorrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicypkirootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicypublickeyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagesigstoreverificationpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/machineconfignodestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/machineconfignodestatusinternalreleaseimage.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/machineconfignodestatusinternalreleaseimageref.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/machineconfigpoolspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/machineconfigpoolstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/osimagestreamreference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/internalreleaseimage.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/internalreleaseimagebundlestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/internalreleaseimageref.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/internalreleaseimagespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/internalreleaseimagestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/osimagestream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/osimagestreamset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/osimagestreamstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/pinnedimageref.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1alpha1/generated_expansion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1alpha1/internalreleaseimage.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1alpha1/machineconfiguration_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1alpha1/osimagestream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ingresscontrollerspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ingresscontrollertuningoptions.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (6)
go.modpkg/cli/admin/upgrade/accept/accept.gopkg/cli/admin/upgrade/accept/accept_test.gopkg/cli/admin/upgrade/rollback/rollback.gopkg/cli/admin/upgrade/upgrade.gopkg/helpers/describe/describer_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- go.mod
- pkg/cli/admin/upgrade/accept/accept_test.go
Contributor
|
@hongkailiu: 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
4 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.
With OC_ENABLE_CMD_UPGRADE_ACCEPT_RISKS=true, a new command
oc adm upgrade acceptis enabled. It accepts comma-separated risks exposed to an OpenShift release [1].The risks are stored in
clusterversion/version's.specs.desiredUpdate.acceptRisks.[1]. https://docs.redhat.com/en/documentation/openshift_container_platform/4.18/html-single/updating_clusters/index#understanding-clusterversion-conditiontypes_understanding-openshift-updates
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
Style