Skip to content

Bug 2075647: pkg/cli/admin/upgrade: Use PATCH instead of POST for spec updates #1111

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 3 commits into from
Apr 18, 2022
Merged

Bug 2075647: pkg/cli/admin/upgrade: Use PATCH instead of POST for spec updates #1111

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dd48aa4
pkg/cli/admin/upgrade: --to-image --allow-explicit-upgrade and known …
wking Apr 14, 2022
27c7bdb
pkg/cli/admin/upgrade: Allow --to-lastest and --allow-not-recommended…
wking Apr 14, 2022
123fa58
pkg/cli/admin/upgrade: Use PATCH instead of POST for spec updates
wking Apr 14, 2022
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
103 changes: 46 additions & 57 deletions pkg/cli/admin/upgrade/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package upgrade
import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -194,47 +195,9 @@ func (o *Options) Run() error {
}
return nil

case o.ToLatestAvailable:
if len(cv.Status.AvailableUpdates) == 0 {
fmt.Fprintf(o.Out, "info: Cluster is already at the latest available version %s\n", cv.Status.Desired.Version)
return nil
}

if err := checkForUpgrade(cv); err != nil {
if !o.AllowUpgradeWithWarnings {
return fmt.Errorf("%s\n\nIf you want to upgrade anyway, use --allow-upgrade-with-warnings.", err)
}
fmt.Fprintf(o.ErrOut, "warning: --allow-upgrade-with-warnings is bypassing: %s", err)
}

sortReleasesBySemanticVersions(cv.Status.AvailableUpdates)

update := cv.Status.AvailableUpdates[0]
desiredUpdate := &configv1.Update{
Version: update.Version,
Image: update.Image,
}
if o.Force {
desiredUpdate.Force = true
fmt.Fprintln(o.ErrOut, "warning: --force overrides cluster verification of your supplied release image and waives any update precondition failures.")
}

cv.Spec.DesiredUpdate = desiredUpdate
_, err := o.Client.ConfigV1().ClusterVersions().Update(context.TODO(), cv, metav1.UpdateOptions{})
if err != nil {
return fmt.Errorf("Unable to upgrade to latest version %s: %v", update.Version, err)
}

if len(update.Version) > 0 {
fmt.Fprintf(o.Out, "Updating to latest version %s\n", update.Version)
} else {
fmt.Fprintf(o.Out, "Updating to latest release image %s\n", update.Image)
}

return nil

case len(o.To) > 0, len(o.ToImage) > 0:
case o.ToLatestAvailable, len(o.To) > 0, len(o.ToImage) > 0:
var update *configv1.Update

if len(o.To) > 0 && o.To == cv.Status.Desired.Version {
fmt.Fprintf(o.Out, "info: Cluster is already at version %s\n", o.To)
return nil
Expand All @@ -245,7 +208,8 @@ func (o *Options) Run() error {
return nil
}

possibleUpgradeTargets := make([]string, 0, len(cv.Status.AvailableUpdates)+len(cv.Status.ConditionalUpdates))
possibleUpgradeTargets := make([]configv1.Release, 0, len(cv.Status.AvailableUpdates)+len(cv.Status.ConditionalUpdates))
recommendedConditions := make(map[string]metav1.Condition, len(cv.Status.ConditionalUpdates))

// check for recommended updates
for _, available := range cv.Status.AvailableUpdates {
Expand All @@ -258,7 +222,7 @@ func (o *Options) Run() error {
} else if err != nil {
fmt.Fprintf(o.ErrOut, "warning: unable to calculate match for the update target in available updates: %v\n", err)
}
possibleUpgradeTargets = append(possibleUpgradeTargets, available.Version)
possibleUpgradeTargets = append(possibleUpgradeTargets, available)
}

if update == nil {
Expand All @@ -282,42 +246,65 @@ func (o *Options) Run() error {
fmt.Fprintf(o.ErrOut, "warning: unable to calculate match for the update target in available conditional updates: %v\n", err)
}
if o.AllowNotRecommended {
possibleUpgradeTargets = append(possibleUpgradeTargets, upgrade.Release.Version)
possibleUpgradeTargets = append(possibleUpgradeTargets, upgrade.Release)
recommendedConditions[upgrade.Release.Image] = *c
}
}
}
}

if o.ToImage != "" {
if o.AllowExplicitUpgrade {
update = &configv1.Update{
Version: "",
Image: o.ToImage,
}
fmt.Fprintln(o.ErrOut, "warning: The requested upgrade image is not one of the available updates."+
"You have used --allow-explicit-upgrade for the update to proceed anyway")
if update == nil && o.ToImage != "" && o.AllowExplicitUpgrade {
update = &configv1.Update{
Version: "",
Image: o.ToImage,
}
fmt.Fprintln(o.ErrOut, "warning: The requested upgrade image is not one of the available updates."+
"You have used --allow-explicit-upgrade for the update to proceed anyway")
}

sortReleasesBySemanticVersions(possibleUpgradeTargets)

if o.ToLatestAvailable && len(possibleUpgradeTargets) > 0 {
update = &configv1.Update{
Version: possibleUpgradeTargets[0].Version,
Image: possibleUpgradeTargets[0].Image,
}

if c, ok := recommendedConditions[update.Image]; ok {
fmt.Fprintf(o.ErrOut, "warning: with --allow-not-recommended you have accepted the risks with %s and bypassed %s=%s %s: %s\n",
update.Version, c.Type, c.Status, c.Reason, c.Message)
}
}

if update == nil {
sort.Strings(possibleUpgradeTargets)
c := findClusterOperatorStatusCondition(cv.Status.Conditions, configv1.RetrievedUpdates)

toImageOption := "--to-image"
if o.ToImage != "" {
toImageOption = "--allow-explicit-upgrade"
}
nextStep := fmt.Sprintf("%s to continue with the update", toImageOption)
article := "the" // --to and --to-image have a specific target in mind
if o.ToLatestAvailable {
article = "an" // --to-latest does not have a specific target in mind
}
nextStep := fmt.Sprintf("%s to continue with %s update", toImageOption, article)

switch {
case c != nil && c.Status != configv1.ConditionTrue:
return fmt.Errorf("cannot refresh available updates:\n Reason: %s\n Message: %s\n\nspecify %s.", c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "), nextStep)
case len(possibleUpgradeTargets) == 0 && o.AllowNotRecommended:
return fmt.Errorf("no recommended or conditional updates, specify %s or wait for new updates to be available.", nextStep)
case len(possibleUpgradeTargets) == 0 && o.ToLatestAvailable:
fmt.Fprintf(o.Out, "info: Cluster is already at the latest available version %s\n", cv.Status.Desired.Version)
return nil
case len(possibleUpgradeTargets) == 0:
return fmt.Errorf("no recommended updates, specify %s or wait for new updates to be available.", nextStep)
case len(possibleUpgradeTargets) > 0:
return fmt.Errorf("the update is not one of the possible targets: %s. specify %s.", strings.Join(possibleUpgradeTargets, ", "), nextStep)
versions := make([]string, 0, len(possibleUpgradeTargets))
for _, release := range possibleUpgradeTargets {
versions = append(versions, release.Version)
}
return fmt.Errorf("the update is not one of the possible targets: %s. specify %s.", strings.Join(versions, ", "), nextStep)
default:
return errors.New("unable to calculate a target update")
}
Expand All @@ -335,10 +322,12 @@ func (o *Options) Run() error {
fmt.Fprintf(o.ErrOut, "warning: --allow-upgrade-with-warnings is bypassing: %s", err)
}

cv.Spec.DesiredUpdate = update

_, err := o.Client.ConfigV1().ClusterVersions().Update(context.TODO(), cv, metav1.UpdateOptions{})
updateJSON, err := json.Marshal(update)
if err != nil {
return fmt.Errorf("marshal ClusterVersion patch: %v", err)
}
patch := []byte(fmt.Sprintf(`{"spec":{"desiredUpdate": %s}}`, updateJSON))
if _, err := o.Client.ConfigV1().ClusterVersions().Patch(context.TODO(), cv.Name, types.MergePatchType, patch, metav1.PatchOptions{}); err != nil {
return fmt.Errorf("Unable to upgrade: %v", err)
}

Expand Down