NO-ISSUE: recover the context in upgrade

[hongkailiu]

This was missed when I worked on [1].

[1]. https://github.com/openshift/oc/pull/2170/changes#r2997204077

[openshift-ci-robot]

@hongkailiu: This pull request explicitly references no jira issue.

Details

In response to this:

This was missed when I worked on [1].

[1]. https://github.com/openshift/oc/pull/2170/changes#r2997204077

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.

[coderabbitai]

Walkthrough

The upgrade command's Run method signature was refactored to remove its context parameter, with the implementation using context.TODO() internally instead of relying on the caller-provided context for initial and subsequent operations.

Changes

Cohort / File(s)	Summary
Upgrade Command Refactoring pkg/cli/admin/upgrade/upgrade.go	Modified Options.Run() signature to remove context parameter; replaced caller-provided context with context.TODO() for ClusterVersions().Get and PATCH operations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

pkg/cli/admin/upgrade/upgrade.go (1)

207-217: Consider propagating cmd.Context() instead of using context.TODO().

Using context.TODO() throughout means the upgrade operations lose cancellation support. If a user presses Ctrl+C during an upgrade request, the in-flight API calls will continue rather than being cancelled.

The idiomatic approach would be to either keep the context parameter:

func (o *Options) Run(ctx context.Context) error {

Or retrieve it within the Cobra Run function and pass it:

Run: func(cmd *cobra.Command, args []string) {
    kcmdutil.CheckErr(o.Complete(f, cmd, args))
    kcmdutil.CheckErr(o.Run(cmd.Context()))
},

If using context.TODO() is an intentional pattern in this codebase, please disregard.

🤖 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 207 - 217, The Run method on
Options currently uses context.TODO() (see Options.Run and the call to
o.Client.ConfigV1().ClusterVersions().Get), which prevents cancellation; change
the signature to accept a context (func (o *Options) Run(ctx context.Context)
error), replace all uses of context.TODO() inside this method with the passed
ctx (including the ClusterVersions().Get call and any subsequent API calls), and
update the cobra command invocations that call Options.Run to pass cmd.Context()
(e.g., in the Run: func(cmd *cobra.Command, args []string) { ...;
o.Run(cmd.Context()) }). Ensure all callers of Options.Run are updated
accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/cli/admin/upgrade/upgrade.go`:
- Around line 207-217: The Run method on Options currently uses context.TODO()
(see Options.Run and the call to o.Client.ConfigV1().ClusterVersions().Get),
which prevents cancellation; change the signature to accept a context (func (o
*Options) Run(ctx context.Context) error), replace all uses of context.TODO()
inside this method with the passed ctx (including the ClusterVersions().Get call
and any subsequent API calls), and update the cobra command invocations that
call Options.Run to pass cmd.Context() (e.g., in the Run: func(cmd
*cobra.Command, args []string) { ...; o.Run(cmd.Context()) }). Ensure all
callers of Options.Run are updated accordingly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 551b62f0-283d-4024-ad5d-c9df9b5cc501

📥 Commits

Reviewing files that changed from the base of the PR and between 12f8fce and b233283.

📒 Files selected for processing (1)

• pkg/cli/admin/upgrade/upgrade.go

[hongkailiu]

/retest

[openshift-ci]

@hongkailiu: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[ardaguclu]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ardaguclu, hongkailiu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• pkg/cli/admin/upgrade/OWNERS [ardaguclu,hongkailiu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[hongkailiu]

Recover the code to what it was.
Risk is low.

/verified by @hongkailiu

[openshift-ci-robot]

@hongkailiu: This PR has been marked as verified by @hongkailiu.

Details

In response to this:

Recover the code to what it was.
Risk is low.

/verified by @hongkailiu

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.