Revert "Drop the guard on UpgradeInProgress"

This reverts commit 362e9ca.

pkg/payload/precondition/clusterversion/upgradeable.go

@@ -2,6 +2,7 @@ package clusterversion
 
 import (
 	"context"
+	"fmt"
 	"strconv"
 	"strings"
 	"time"
@@ -65,6 +66,32 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele
 		}
 	}
 
+	if cond := resourcemerge.FindOperatorStatusCondition(cv.Status.Conditions, UpgradeInProgress); cond != nil && cond.Status == configv1.ConditionTrue {
+		desiredMinor := GetEffectiveMinor(releaseContext.DesiredVersion)
+		currentVersion := GetCurrentVersion(cv.Status.History)
+		if minorInProgress := GetEffectiveMinor(cv.Status.Desired.Version); minorVersionUpgrade(minorInProgress, desiredMinor) {
+			return &precondition.Error{
+				Reason:  "UpgradeInProgress",
+				Message: fmt.Sprintf("The minor level upgrade to %s is not recommended until the existing upgrade from %s to %s completes.", releaseContext.DesiredVersion, currentVersion, cv.Status.Desired.Version),
+				Name:    pf.Name(),
+			}
+		} else {
+			currentMinor := GetEffectiveMinor(currentVersion)
+			desiredMinor := GetEffectiveMinor(cv.Status.Desired.Version)
+			if releaseContext.DesiredVersion != "" && minorVersionUpgrade(currentMinor, desiredMinor) {
+				return &precondition.Error{
+					Reason:  "MinorVersionClusterUpgradeInProgress",
+					Message: fmt.Sprintf("The upgrade is retargeted to %s from the existing minor level upgrade from %s to %s.", releaseContext.DesiredVersion, currentVersion, cv.Status.Desired.Version),
+					Name:    pf.Name(),
+					// This is to generate a message in the accepted risks
+					// for the unblocking case 4.y.z -> 4.y+1.z' -> 4.y+1.z''
+					NonBlockingWarning: true,
+				}
+
+			}
+		}
+	}
+
 	// if we are upgradeable==true we can always upgrade
 	up := resourcemerge.FindOperatorStatusCondition(cv.Status.Conditions, configv1.OperatorUpgradeable)
 	if up == nil {

pkg/payload/precondition/clusterversion/upgradeable_test.go

@@ -60,11 +60,13 @@ func TestUpgradeableRun(t *testing.T) {
 	}
 
 	tests := []struct {
-		name           string
-		upgradeable    *configv1.ConditionStatus
-		currVersion    string
-		desiredVersion string
-		expected       string
+		name                     string
+		upgradeable              *configv1.ConditionStatus
+		currVersion              string
+		desiredVersion           string
+		versionPartiallyUpgraded string
+		NonBlockingWarning       bool
+		expected                 string
 	}{
 		{
 			name:           "first",
@@ -105,6 +107,28 @@ func TestUpgradeableRun(t *testing.T) {
 			desiredVersion: "4.1.4",
 			expected:       "",
 		},
+		{
+			name:                     "move-(y+1) while move-y is in progress",
+			currVersion:              "4.6.3",
+			versionPartiallyUpgraded: "4.7.2",
+			desiredVersion:           "4.8.1",
+			expected:                 "The minor level upgrade to 4.8.1 is not recommended until the existing upgrade from 4.6.3 to 4.7.2 completes.",
+		},
+		{
+			name:                     "move-(y+1) while move-z is in progress",
+			currVersion:              "4.14.15",
+			versionPartiallyUpgraded: "4.14.35",
+			desiredVersion:           "4.15.29",
+			expected:                 "The minor level upgrade to 4.15.29 is not recommended until the existing upgrade from 4.14.15 to 4.14.35 completes.",
+		},
+		{
+			name:                     "move-y with z while move-y is in progress",
+			currVersion:              "4.6.3",
+			versionPartiallyUpgraded: "4.7.2",
+			desiredVersion:           "4.7.3",
+			NonBlockingWarning:       true,
+			expected:                 "The upgrade is retargeted to 4.7.3 from the existing minor level upgrade from 4.6.3 to 4.7.2.",
+		},
 	}
 
 	for _, tc := range tests {
@@ -114,24 +138,52 @@ func TestUpgradeableRun(t *testing.T) {
 				Spec:       configv1.ClusterVersionSpec{},
 				Status: configv1.ClusterVersionStatus{
 					History: []configv1.UpdateHistory{},
+					Desired: configv1.Release{Version: tc.versionPartiallyUpgraded},
 				},
 			}
 			if len(tc.currVersion) > 0 {
 				clusterVersion.Status.History = append(clusterVersion.Status.History, configv1.UpdateHistory{Version: tc.currVersion, State: configv1.CompletedUpdate})
 			}
+			if tc.versionPartiallyUpgraded != "" {
+				clusterVersion.Status.History = append(clusterVersion.Status.History, configv1.UpdateHistory{Version: tc.versionPartiallyUpgraded, State: configv1.PartialUpdate})
+			}
 			if tc.upgradeable != nil {
 				clusterVersion.Status.Conditions = append(clusterVersion.Status.Conditions, configv1.ClusterOperatorStatusCondition{
 					Type:    configv1.OperatorUpgradeable,
 					Status:  *tc.upgradeable,
 					Message: fmt.Sprintf("set to %v", *tc.upgradeable),
 				})
 			}
+
+			if tc.versionPartiallyUpgraded != "" {
+				clusterVersion.Status.Conditions = append(clusterVersion.Status.Conditions, configv1.ClusterOperatorStatusCondition{
+					Type:    UpgradeInProgress,
+					Status:  configv1.ConditionTrue,
+					Message: "some-message",
+					Reason:  "some-reason",
+				})
+			} else {
+				clusterVersion.Status.Conditions = append(clusterVersion.Status.Conditions, configv1.ClusterOperatorStatusCondition{
+					Type:    UpgradeInProgress,
+					Status:  configv1.ConditionFalse,
+					Message: "message-bar",
+					Reason:  "reason-bar",
+				})
+			}
 			cvLister := fakeClusterVersionLister(t, clusterVersion)
 			instance := NewUpgradeable(cvLister)
 
 			err := instance.Run(ctx, precondition.ReleaseContext{
 				DesiredVersion: tc.desiredVersion,
 			})
+			if tc.NonBlockingWarning {
+				pError, ok := err.(*precondition.Error)
+				if !ok {
+					t.Errorf("Failed to convert to err: %v", err)
+				} else if pError.NonBlockingWarning != true {
+					t.Error("NonBlockingWarning should be true")
+				}
+			}
 			switch {
 			case err != nil && len(tc.expected) == 0:
 				t.Error(err)