diff --git a/pkg/cvo/status_history.go b/pkg/cvo/status_history.go index f5cea9dc8..9319744c3 100644 --- a/pkg/cvo/status_history.go +++ b/pkg/cvo/status_history.go @@ -215,21 +215,3 @@ func getEffectiveMicro(version string) string { } return splits[2] } - -// getCurrentVersion determines and returns the cluster's current version by iterating through the -// provided update history until it finds the first version with update State of Completed. If a -// Completed version is not found the version of the oldest history entry, which is the originally -// installed version, is returned. If history is empty the empty string is returned. -func getCurrentVersion(history []configv1.UpdateHistory) string { - for _, h := range history { - if h.State == configv1.CompletedUpdate { - klog.V(2).Infof("Cluster current version=%s", h.Version) - return h.Version - } - } - // Empty history should only occur if method is called early in startup before history is populated. - if len(history) != 0 { - return history[len(history)-1].Version - } - return "" -} diff --git a/pkg/cvo/upgradeable.go b/pkg/cvo/upgradeable.go index 5b5242c00..04bf98e7f 100644 --- a/pkg/cvo/upgradeable.go +++ b/pkg/cvo/upgradeable.go @@ -23,6 +23,7 @@ import ( "github.com/openshift/cluster-version-operator/lib/resourcedelete" "github.com/openshift/cluster-version-operator/lib/resourcemerge" "github.com/openshift/cluster-version-operator/pkg/internal" + "github.com/openshift/cluster-version-operator/pkg/payload/precondition/clusterversion" ) const ( @@ -365,7 +366,7 @@ func (check *clusterAdminAcksCompletedUpgradeable) Check() *configv1.ClusterOper Message: message, } } - currentVersion := getCurrentVersion(cv.Status.History) + currentVersion := clusterversion.GetCurrentVersion(cv.Status.History) // This can occur in early start up when the configmap is first added and version history // has not yet been populated. diff --git a/pkg/payload/precondition/clusterversion/upgradeable.go b/pkg/payload/precondition/clusterversion/upgradeable.go index 62d66d1c9..29c890844 100644 --- a/pkg/payload/precondition/clusterversion/upgradeable.go +++ b/pkg/payload/precondition/clusterversion/upgradeable.go @@ -2,6 +2,7 @@ package clusterversion import ( "context" + "fmt" "time" "github.com/blang/semver/v4" @@ -95,7 +96,8 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele } klog.V(4).Infof("The current version is %s parsed from %s and the target version is %s parsed from %s", currentVersion.String(), cv.Status.Desired.Version, targetVersion.String(), releaseContext.DesiredVersion) - if targetVersion.LTE(currentVersion) || (targetVersion.Major == currentVersion.Major && targetVersion.Minor == currentVersion.Minor) { + patchOnly := targetVersion.Major == currentVersion.Major && targetVersion.Minor == currentVersion.Minor + if targetVersion.LTE(currentVersion) || patchOnly { // When Upgradeable==False, a patch level update with the same minor level is allowed unless overrides are set // This Upgradeable precondition is only concerned about moving forward, i.e., do not care about downgrade which is taken care of by the Rollback precondition if condition := ClusterVersionOverridesCondition(cv); condition != nil { @@ -106,6 +108,15 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele Name: pf.Name(), } } else { + if completedVersion := minorUpdateFrom(cv.Status, currentVersion); completedVersion != "" && patchOnly { + // This is to generate an accepted risk for the accepting case 4.y.z -> 4.y+1.z' -> 4.y+1.z'' + return &precondition.Error{ + Reason: "MinorVersionClusterUpdateInProgress", + Message: fmt.Sprintf("Retarget to %s while a minor level update from %s to %s is in progress", targetVersion, completedVersion, currentVersion), + Name: pf.Name(), + NonBlockingWarning: true, + } + } klog.V(2).Infof("Precondition %q passed on update to %s", pf.Name(), targetVersion.String()) return nil } @@ -119,5 +130,43 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele } } +// minorUpdateFrom returns the version that was installed completed if a minor level upgrade is in progress +// and the empty string otherwise +func minorUpdateFrom(status configv1.ClusterVersionStatus, currentVersion semver.Version) string { + completedVersion := GetCurrentVersion(status.History) + if completedVersion == "" { + return "" + } + v, err := semver.Parse(completedVersion) + if err != nil { + return "" + } + if cond := resourcemerge.FindOperatorStatusCondition(status.Conditions, configv1.OperatorProgressing); cond != nil && + cond.Status == configv1.ConditionTrue && + v.Major == currentVersion.Major && + v.Minor < currentVersion.Minor { + return completedVersion + } + return "" +} + // Name returns Name for the precondition. func (pf *Upgradeable) Name() string { return "ClusterVersionUpgradeable" } + +// GetCurrentVersion determines and returns the cluster's current version by iterating through the +// provided update history until it finds the first version with update State of Completed. If a +// Completed version is not found the version of the oldest history entry, which is the originally +// installed version, is returned. If history is empty the empty string is returned. +func GetCurrentVersion(history []configv1.UpdateHistory) string { + for _, h := range history { + if h.State == configv1.CompletedUpdate { + klog.V(2).Infof("Cluster current version=%s", h.Version) + return h.Version + } + } + // Empty history should only occur if method is called early in startup before history is populated. + if len(history) != 0 { + return history[len(history)-1].Version + } + return "" +} diff --git a/pkg/payload/precondition/clusterversion/upgradeable_test.go b/pkg/payload/precondition/clusterversion/upgradeable_test.go index bdf0b460c..e888f6bef 100644 --- a/pkg/payload/precondition/clusterversion/upgradeable_test.go +++ b/pkg/payload/precondition/clusterversion/upgradeable_test.go @@ -2,6 +2,7 @@ package clusterversion import ( "context" + "errors" "fmt" "testing" @@ -23,11 +24,12 @@ func TestUpgradeableRun(t *testing.T) { } tests := []struct { - name string - upgradeable *configv1.ConditionStatus - currVersion string - desiredVersion string - expected *precondition.Error + name string + upgradeable *configv1.ConditionStatus + completedVersion string + currVersion string + desiredVersion string + expected *precondition.Error }{ { name: "first", @@ -125,6 +127,26 @@ func TestUpgradeableRun(t *testing.T) { currVersion: "4.17.0-0.test-2024-10-07-173400-ci-ln-pwn9ngk-latest", desiredVersion: "4.17.0-rc.6", }, + { + name: "move-y-then-z, with false condition", + upgradeable: ptr(configv1.ConditionFalse), + completedVersion: "4.1.1", + currVersion: "4.2.1", + desiredVersion: "4.2.3", + expected: &precondition.Error{ + NonBlockingWarning: true, + Name: "ClusterVersionUpgradeable", + Message: "Retarget to 4.2.3 while a minor level update from 4.1.1 to 4.2.1 is in progress", + Reason: "MinorVersionClusterUpdateInProgress", + }, + }, + { + name: "move-z-then-z, with false condition", + upgradeable: ptr(configv1.ConditionFalse), + completedVersion: "4.2.1", + currVersion: "4.2.2", + desiredVersion: "4.2.3", + }, } for _, tc := range tests { @@ -134,8 +156,13 @@ func TestUpgradeableRun(t *testing.T) { Spec: configv1.ClusterVersionSpec{}, Status: configv1.ClusterVersionStatus{ Desired: configv1.Release{Version: tc.currVersion}, + History: []configv1.UpdateHistory{{State: configv1.CompletedUpdate, Version: tc.completedVersion}}, }, } + if tc.completedVersion != "" && tc.completedVersion != tc.currVersion { + clusterVersion.Status.Conditions = append(clusterVersion.Status.Conditions, + configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue}) + } if tc.upgradeable != nil { clusterVersion.Status.Conditions = append(clusterVersion.Status.Conditions, configv1.ClusterOperatorStatusCondition{ Type: configv1.OperatorUpgradeable, @@ -154,7 +181,8 @@ func TestUpgradeableRun(t *testing.T) { if (tc.expected == nil && err != nil) || (tc.expected != nil && err == nil) { t.Errorf("%s: expected %v but got %v", tc.name, tc.expected, err) } else if tc.expected != nil && err != nil { - if pError, ok := err.(*precondition.Error); !ok { + var pError *precondition.Error + if !errors.As(err, &pError) { t.Errorf("Failed to convert to err: %v", err) } else if diff := cmp.Diff(tc.expected, pError, cmpopts.IgnoreFields(precondition.Error{}, "Nested")); diff != "" { t.Errorf("%s differs from expected:\n%s", tc.name, diff)