Skip to content

OCPBUGS-7780: pkg/cli/admin/upgrade/channel: Use PATCH instead of POST for spec updates #1346

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 1 commit into from
Feb 23, 2023
Merged

OCPBUGS-7780: pkg/cli/admin/upgrade/channel: Use PATCH instead of POST for spec updates #1346

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
pkg/cli/admin/upgrade/channel: Use PATCH instead of POST for spec upd… 
…ates

Like 123fa58 (pkg/cli/admin/upgrade: Use PATCH instead of POST for
spec updates, 2022-04-14, #1111), but for the 'channel' subcommand.
123fa58 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 578fa80    pkg/cli/admin/upgrade/OWNERS
  100644 blob e2418c1    pkg/cli/admin/upgrade/upgrade.go
  100644 blob 581af58    pkg/cli/admin/upgrade/upgrade_test.go
  $ git ls-tree -r origin/release-4.9 -- pkg/cli/admin/upgrade
  100644 blob 578fa80    pkg/cli/admin/upgrade/OWNERS
  100644 blob b427176    pkg/cli/admin/upgrade/channel/channel.go
  100644 blob 2bbf9da    pkg/cli/admin/upgrade/upgrade.go
  100644 blob 581af58    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.
  • Loading branch information
@wking
wking committed Feb 20, 2023
commit ed394856a5c5309b9f58b0b1aa532ba918047329
6 changes: 3 additions & 3 deletions pkg/cli/admin/upgrade/channel/channel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

Expand Down