From ed394856a5c5309b9f58b0b1aa532ba918047329 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 20 Feb 2023 09:38:00 -0800 Subject: [PATCH] pkg/cli/admin/upgrade/channel: Use PATCH instead of POST for spec updates Like 123fa58184 (pkg/cli/admin/upgrade: Use PATCH instead of POST for spec updates, 2022-04-14, #1111), but for the 'channel' subcommand. 123fa58184 described the general risk of using POST/Update when the local 'oc' client may not be aware of some new spec properties. For the 'channel' subcommand specifically, it was added in 4.9: $ git ls-tree -r origin/release-4.8 -- pkg/cli/admin/upgrade 100644 blob 578fa80a8162d0cdfc6cb277e7b09c3bbad81444 pkg/cli/admin/upgrade/OWNERS 100644 blob e2418c1a0b931f4d444154a6b180855431bba72d pkg/cli/admin/upgrade/upgrade.go 100644 blob 581af58611b0b4c11d5a6a2fbb9e14796e869ce4 pkg/cli/admin/upgrade/upgrade_test.go $ git ls-tree -r origin/release-4.9 -- pkg/cli/admin/upgrade 100644 blob 578fa80a8162d0cdfc6cb277e7b09c3bbad81444 pkg/cli/admin/upgrade/OWNERS 100644 blob b427176401a5ec5973468dc569442fed04701744 pkg/cli/admin/upgrade/channel/channel.go 100644 blob 2bbf9da0afe7cfca1cc5d7317e6b99dc374e5328 pkg/cli/admin/upgrade/upgrade.go 100644 blob 581af58611b0b4c11d5a6a2fbb9e14796e869ce4 pkg/cli/admin/upgrade/upgrade_test.go And between 4.9 and the present, ClusterVersion spec grew the 'capabilities' property: $ git diff origin/release-4.9..origin/master -- vendor/github.com/openshift/api/config/v1/types_cluster_version.go | grep -v // | grep -A10 Spec @@ -45,8 +45,17 @@ type ClusterVersionSpec struct { @@ -68,6 +77,12 @@ type ClusterVersionSpec struct { Channel string `json:"channel,omitempty"` + Capabilities *ClusterVersionCapabilitiesSpec `json:"capabilities,omitempty"` + @@ -113,6 +128,9 @@ type ClusterVersionStatus struct { ... That property landed in 4.11: $ git --no-pager grep -c ClusterVersionCapabilitiesSpec origin/release-4.10 vendor/github.com/openshift/api/config/v1/types_cluster_version.go ...no hits... $ git --no-pager grep -c ClusterVersionCapabilitiesSpec origin/release-4.11 vendor/github.com/openshift/api/config/v1/types_cluster_version.go origin/release-4.11:vendor/github.com/openshift/api/config/v1/types_cluster_version.go:3 So I suspect 4.9 and 4.10 oc calls to 'oc adm upgrade channel ...' for 4.11+ clusters would clear spec.capabilities. Not all that many clusters try to restrict capabilities, but folks will need to bump their channel for at least every other minor (if their using EUS channels), and while we recommend folks use an oc from the 4.y they're heading towards, we don't have anything in place to enforce that. --- pkg/cli/admin/upgrade/channel/channel.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cli/admin/upgrade/channel/channel.go b/pkg/cli/admin/upgrade/channel/channel.go index b427176401..37d041c779 100644 --- a/pkg/cli/admin/upgrade/channel/channel.go +++ b/pkg/cli/admin/upgrade/channel/channel.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/cli-runtime/pkg/genericclioptions" kcmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubectl/pkg/util/templates" @@ -129,9 +130,8 @@ func (o *Options) Run() error { fmt.Fprintf(o.ErrOut, "warning: Clearing channel %q; cluster will no longer request available update recommendations.\n", cv.Spec.Channel) } - cv.Spec.Channel = o.Channel - - if _, err = o.Client.ConfigV1().ClusterVersions().Update(ctx, cv, metav1.UpdateOptions{}); err != nil { + patch := []byte(fmt.Sprintf(`{"spec":{"channel": %q}}`, o.Channel)) + if _, err := o.Client.ConfigV1().ClusterVersions().Patch(ctx, cv.ObjectMeta.Name, types.MergePatchType, patch, metav1.PatchOptions{}); err != nil { return fmt.Errorf("unable to set channel: %w", err) }