New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pkg/cli/admin/upgrade/channel: Add 'oc adm upgrade channel ...' #576
Conversation
c09c871
to
170242c
Compare
@wking: The following tests failed, say
Full PR test history. Your PR dashboard. 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/test-infra repository. I understand the commands that are listed here. |
o := NewOptions(streams) | ||
cmd := &cobra.Command{ | ||
Use: "channel CHANNEL", | ||
Short: "Set or clear the update channel", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oc adm upgrade channel candidate-4.6
Is not intuitive IMO. We need a verb or flag to make it intuitive e.g. oc adm upgrade channel --to candidate-4.6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to compile this PR to test the user experience but getting in to some error because I am using go 1.15.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased onto master, and it compiles in the CI presubmits. What error were you seeing?
/remove-lifecycle stale
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can compile the PR successfully now.
@wking: The following tests failed, say
Full PR test history. Your PR dashboard. 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/test-infra repository. I understand the commands that are listed here. |
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
170242c
to
7d8908b
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wking The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Give folks some context about where the recommended updates, or lack thereof, are coming from. We don't need to print anything in the channel-unset case, because that's already covered by the RetrievedUpdates condition handling: $ ~/src/openshift/oc/oc adm upgrade Cluster version is 4.6.0-fc.3 warning: Cannot display available updates: Reason: NoChannel Message: The update channel has not been configured. When a channel is configured: $ oc patch clusterversion version --type json -p '[{"op": "add", "path": "/spec/channel", "value": "candidate-4.6"}]' output looks like: $ oc adm upgrade Cluster version is 4.6.0-fc.3 Upstream: https://api.openshift.com/api/upgrades_info/v1/graph Channel: candidate-4.6 (available channels: candidate-4.6) Updates: VERSION IMAGE 4.6.0-fc.4 quay.io/openshift-release-dev/ocp-release@sha256:960ec73733150827076cbb5fa2c1f5aaa9a94bfbce1b4897e46432a56ac976c1 4.6.0-fc.5 quay.io/openshift-release-dev/ocp-release@sha256:5883d0db15939484bd477147e6949c53fbc6f551ec20a0f1106b8a3acfb86ef8
A new subcommand for conveniently managing channels. Example workflow, starting in a channel: $ oc adm upgrade Cluster version is 4.6.0-fc.3 Upstream: https://api.openshift.com/api/upgrades_info/v1/graph Channel: candidate-4.6 (available channels: candidate-4.6) Updates: VERSION IMAGE 4.6.0-fc.4 quay.io/openshift-release-dev/ocp-release@sha256:960ec73733150827076cbb5fa2c1f5aaa9a94bfbce1b4897e46432a56ac976c1 4.6.0-fc.5 quay.io/openshift-release-dev/ocp-release@sha256:5883d0db15939484bd477147e6949c53fbc6f551ec20a0f1106b8a3acfb86ef8 Trying to change to the same channel is a no-op: $ oc adm upgrade channel candidate-4.6 info: Cluster is already in candidate-4.6 Trying to change to an unrecognized channel gets a warning: $ oc adm upgrade channel does-not-exist error: the requested channel "does-not-exist" is not one of the available channels (candidate-4.6), you must pass --allow-explicit-channel to continue $ oc adm upgrade channel --allow-explicit-channel does-not-exist warning: The requested channel "does-not-exist" is not one of the available channels (candidate-4.6). You have used --allow-explicit-channel to proceed anyway. $ oc adm upgrade Cluster version is 4.6.0-fc.3 Channel: does-not-exist warning: Cannot display available updates: Reason: VersionNotFound Message: Unable to retrieve available updates: currently reconciling cluster version 4.6.0-fc.3 not found in the "does-not-exist" channel When we have no known channels, changing requires no override: $ oc adm upgrade channel does-not-exist-either warning: No channels known to be compatible with the current version "4.6.0-fc.3"; unable to validate "does-not-exist-either". $ oc adm upgrade channel candidate-4.6 warning: No channels known to be compatible with the current version "4.6.0-fc.3"; unable to validate "candidate-4.6". Clearing a known channel needs an explicit override: $ oc adm upgrade channel error: the requested channel "" is not one of the available channels (candidate-4.6), you must pass --allow-explicit-channel to continue $ oc adm upgrade channel --allow-explicit-channel warning: Clearing channel "candidate-4.6"; cluster will no longer request available update recommendations. $ oc adm upgrade Cluster version is 4.6.0-fc.3 warning: Cannot display available updates: Reason: NoChannel Message: The update channel has not been configured. Trying to re-clear the channel is a no-op: $ oc adm upgrade channel info: Cluster channel is already clear And you can set any channel from a cleared channel without an override, because this is another case where we have no idea what the valid choices are: $ oc adm upgrade channel candidate-4.6 warning: No channels known to be compatible with the current version "4.6.0-fc.3"; unable to validate "candidate-4.6". $ oc adm upgrade Cluster version is 4.6.0-fc.3 Upstream: https://api.openshift.com/api/upgrades_info/v1/graph Channel: candidate-4.6 (available channels: candidate-4.6) Updates: VERSION IMAGE 4.6.0-fc.4 quay.io/openshift-release-dev/ocp-release@sha256:960ec73733150827076cbb5fa2c1f5aaa9a94bfbce1b4897e46432a56ac976c1 4.6.0-fc.5 quay.io/openshift-release-dev/ocp-release@sha256:5883d0db15939484bd477147e6949c53fbc6f551ec20a0f1106b8a3acfb86ef8 Clearing from an unknown channel does not require an override either: $ oc adm upgrade channel --allow-explicit-channel does-not-exist warning: The requested channel "does-not-exist" is not one of the available channels (candidate-4.6). You have used --allow-explicit-channel to proceed anyway. $ oc adm upgrade channel warning: Clearing channel "does-not-exist"; cluster will no longer request available update recommendations. Completions updated with: $ hack/update-generated-completions.sh
/refresh |
/retest |
@@ -328,6 +328,19 @@ func (o *Options) Run() error { | |||
fmt.Fprintf(o.Out, "Upgradeable=False\n\n Reason: %s\n Message: %s\n\n", c.Reason, c.Message) | |||
} | |||
|
|||
if cv.Spec.Channel != "" { | |||
if cv.Spec.Upstream == "" { | |||
fmt.Fprint(o.Out, "Upstream is unset, so the cluster will use an appropriate default.\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/an appropriate default/ the default/ . Because we do not have multiple upstream options.
I could reproduce this twice i.e. I am getting
|
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
/retest |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: LalatenduMohanty, soltysh, wking The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/override ci/prow/e2e-aws-upgrade |
@sdodson: Overrode contexts on behalf of sdodson: ci/prow/e2e-aws-upgrade In 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 kubernetes/test-infra repository. |
@wking: The following test failed, say
Full PR test history. Your PR dashboard. 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/test-infra repository. I understand the commands that are listed here. |
/override ci/prow/e2e-aws |
@sdodson: Overrode contexts on behalf of sdodson: ci/prow/e2e-aws In 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 kubernetes/test-infra repository. |
I would prefer to configure this by clearing the 'upstream' setting, which seems more intuitive for "there is no upstream" to me. But sadly, it seems that the CVO has been falling back to a default URI when the ClusterVersion upstream is empty since way back [1,2], and that this behavior is enshrined in the API [3]. Although the channel docs also talk about defaults [4], the only channel defaulting in the CVO is when creating a ClusterVersion object after the in-cluster copy was (accidentally?) deleted [5]. So maybe we could talk folks into adjusting the CVO logic to return NoUpstream in the empty-upstream case, but at the moment, clearing the channel is the best approach for "the CVO keeps complaining that it can't hit the upstream, and I want ot quiet it down [6]". The 'oc adm upgrade channel' command just landed for 4.9 in [7]. [1]: https://github.com/openshift/cluster-version-operator/blame/2c4931dc283c551938be1a00fee290de0b79d99c/pkg/cvo/availableupdates.go#L27-L31 [2]: openshift/cluster-version-operator@ab4d84a#diff-78f2af341fa49292dd6930f378018867R24 [3]: https://github.com/openshift/api/blame/0422dc17083e9e8df18d029f3f34322e96e9c326/config/v1/types_cluster_version.go#L56-L57 [4]: https://github.com/openshift/api/blame/0422dc17083e9e8df18d029f3f34322e96e9c326/config/v1/types_cluster_version.go#L62-L63 [5]: https://github.com/openshift/cluster-version-operator/blob/2c4931dc283c551938be1a00fee290de0b79d99c/pkg/cvo/cvo.go#L602 [6]: https://bugzilla.redhat.com/show_bug.cgi?id=1827378 [7]: openshift/oc#576
I would prefer to configure this by clearing the 'upstream' setting, which seems more intuitive for "there is no upstream" to me. But sadly, it seems that the CVO has been falling back to a default URI when the ClusterVersion upstream is empty since way back [1,2], and that this behavior is enshrined in the API [3]. Although the channel docs also talk about defaults [4], the only channel defaulting in the CVO is when creating a ClusterVersion object after the in-cluster copy was (accidentally?) deleted [5]. So maybe we could talk folks into adjusting the CVO logic to return NoUpstream in the empty-upstream case, but at the moment, clearing the channel is the best approach for "the CVO keeps complaining that it can't hit the upstream, and I want ot quiet it down [6]". The 'oc adm upgrade channel' command just landed for 4.9 in [7]. [1]: https://github.com/openshift/cluster-version-operator/blame/2c4931dc283c551938be1a00fee290de0b79d99c/pkg/cvo/availableupdates.go#L27-L31 [2]: openshift/cluster-version-operator@ab4d84a#diff-78f2af341fa49292dd6930f378018867R24 [3]: https://github.com/openshift/api/blame/0422dc17083e9e8df18d029f3f34322e96e9c326/config/v1/types_cluster_version.go#L56-L57 [4]: https://github.com/openshift/api/blame/0422dc17083e9e8df18d029f3f34322e96e9c326/config/v1/types_cluster_version.go#L62-L63 [5]: https://github.com/openshift/cluster-version-operator/blob/2c4931dc283c551938be1a00fee290de0b79d99c/pkg/cvo/cvo.go#L602 [6]: https://bugzilla.redhat.com/show_bug.cgi?id=1827378 [7]: openshift/oc#576
I would prefer to configure this by clearing the 'upstream' setting, which seems more intuitive for "there is no upstream" to me. But sadly, it seems that the CVO has been falling back to a default URI when the ClusterVersion upstream is empty since way back [1,2], and that this behavior is enshrined in the API [3]. Although the channel docs also talk about defaults [4], the only channel defaulting in the CVO is when creating a ClusterVersion object after the in-cluster copy was (accidentally?) deleted [5]. So maybe we could talk folks into adjusting the CVO logic to return NoUpstream in the empty-upstream case, but at the moment, clearing the channel is the best approach for "the CVO keeps complaining that it can't hit the upstream, and I want ot quiet it down [6]". The 'oc adm upgrade channel' command just landed for 4.9 in [7]. [1]: https://github.com/openshift/cluster-version-operator/blame/2c4931dc283c551938be1a00fee290de0b79d99c/pkg/cvo/availableupdates.go#L27-L31 [2]: openshift/cluster-version-operator@ab4d84a#diff-78f2af341fa49292dd6930f378018867R24 [3]: https://github.com/openshift/api/blame/0422dc17083e9e8df18d029f3f34322e96e9c326/config/v1/types_cluster_version.go#L56-L57 [4]: https://github.com/openshift/api/blame/0422dc17083e9e8df18d029f3f34322e96e9c326/config/v1/types_cluster_version.go#L62-L63 [5]: https://github.com/openshift/cluster-version-operator/blob/2c4931dc283c551938be1a00fee290de0b79d99c/pkg/cvo/cvo.go#L602 [6]: https://bugzilla.redhat.com/show_bug.cgi?id=1827378 [7]: openshift/oc#576
Making it easier for folks to find the 'oc adm upgrade channel' subcommand from 4b8f0fd (pkg/cli/admin/upgrade/channel: Add 'oc adm upgrade channel ...', 2022-03-16, openshift#576), when they look at 'oc adm --help'.
Making it easier for folks to find the 'oc adm upgrade channel' subcommand from 4b8f0fd (pkg/cli/admin/upgrade/channel: Add 'oc adm upgrade channel ...', 2022-03-16, openshift#576), when they look at 'oc adm --help'.
A new subcommand for conveniently managing channels. Example workflow, starting in a channel:
Trying to change to the same channel is a no-op:
Trying to change to an unrecognized channel gets a warning:
When we have no known channels, changing requires no override:
Clearing a known channel needs an explicit override:
Trying to re-clear the channel is a no-op:
And you can set any channel from a cleared channel without an override, because this is another case where we have no idea what the valid choices are:
Clearing from an unknown channel does not require an override either: