fix(openshift): drop z from next calculated y-stream (#2324)

When determining operator compatibility, drop z and build
versions in the calculation of the next Y-stream (minor)
release of OpenShift.

e.g. If the current version is v4.9.5+build, the next Y-stream is
calculated as v4.10.0.

If a pre-release is included, drop it as well but don't increment
the minor version.

e.g. If the current version is v4.10.0-rc, the next Y-stream is
calculated as v4.10.0.

Before this change, the next Y-stream was the result of simply
iterating the cluster's minor version. When the cluster was at
a patch version greater than that specified by an operator, upgrades
would be erroneously blocked.

e.g. If the current version was v4.9.5, the next version would be
calculated as v4.10.5, which would block upgrades on operators with
max versions set to v4.10 -- or more explicitly [v4.10.0, v4.10.5) (')'
is read "exclusive").

Signed-off-by: Nick Hale <[email protected]>

Author: njhale

pkg/controller/operators/openshift/helpers.go

@@ -125,16 +125,20 @@ func transientErrors(err error) error {
 }
 
 func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error) {
-	next, err := desiredRelease(ctx, cli)
+	desired, err := desiredRelease(ctx, cli)
 	if err != nil {
 		return nil, err
 	}
 
-	if next == nil {
+	if desired == nil {
 		// Note: This shouldn't happen
-		return nil, fmt.Errorf("Failed to determine next OpenShift Y-stream release")
+		return nil, fmt.Errorf("Failed to determine current OpenShift Y-stream release")
+	}
+
+	next, err := nextY(*desired)
+	if err != nil {
+		return nil, err
 	}
-	next.Minor++
 
 	csvList := &operatorsv1alpha1.ClusterServiceVersionList{}
 	if err := cli.List(ctx, csvList); err != nil {
@@ -158,7 +162,7 @@ func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error
 			continue
 		}
 
-		if max == nil || max.GTE(*next) {
+		if max == nil || max.GTE(next) {
 			continue
 		}
 		s.maxOpenShiftVersion = max.String()
@@ -189,6 +193,20 @@ func desiredRelease(ctx context.Context, cli client.Client) (*semver.Version, er
 	return &desired, nil
 }
 
+func nextY(v semver.Version) (semver.Version, error) {
+	v.Build = nil // Builds are irrelevant
+
+	if len(v.Pre) > 0 {
+		// Dropping pre-releases is equivalent to incrementing Y
+		v.Pre = nil
+		v.Patch = 0
+
+		return v, nil
+	}
+
+	return v, v.IncrementMinor() // Sets Y=Y+1 and Z=0
+}
+
 const (
 	MaxOpenShiftVersionProperty = "olm.maxOpenShiftVersion"
 )

pkg/controller/operators/openshift/helpers_test.go

@@ -251,6 +251,11 @@ func TestIncompatibleOperators(t *testing.T) {
 				{
 					name:                "chestnut",
 					namespace:           "default",
+					maxOpenShiftVersion: "1.2.0-pre+build",
+				},
+				{
+					name:                "drupe",
+					namespace:           "default",
 					maxOpenShiftVersion: "2.0.0",
 				},
 			},
@@ -290,6 +295,11 @@ func TestIncompatibleOperators(t *testing.T) {
 				{
 					name:                "drupe",
 					namespace:           "default",
+					maxOpenShiftVersion: "1.1.0-pre+build",
+				},
+				{
+					name:                "european-hazelnut",
+					namespace:           "default",
 					maxOpenShiftVersion: "0.1.0",
 				},
 			},
@@ -314,6 +324,11 @@ func TestIncompatibleOperators(t *testing.T) {
 					{
 						name:                "drupe",
 						namespace:           "default",
+						maxOpenShiftVersion: "1.1.0-pre+build",
+					},
+					{
+						name:                "european-hazelnut",
+						namespace:           "default",
 						maxOpenShiftVersion: "0.1.0",
 					},
 				},
@@ -413,14 +428,14 @@ func TestIncompatibleOperators(t *testing.T) {
 			},
 		},
 		{
-			description: "Compatible/EmptyVersion",
+			description: "EmptyVersion",
 			cv: configv1.ClusterVersion{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "version",
 				},
 				Status: configv1.ClusterVersionStatus{
 					Desired: configv1.Update{
-						Version: "",
+						Version: "", // This should result in an transient error
 					},
 				},
 			},
@@ -441,6 +456,70 @@ func TestIncompatibleOperators(t *testing.T) {
 				incompatible: nil,
 			},
 		},
+		{
+			description: "ClusterZ",
+			cv: configv1.ClusterVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "version",
+				},
+				Status: configv1.ClusterVersionStatus{
+					Desired: configv1.Update{
+						Version: "1.0.1", // Next Y-stream is 1.1.0, NOT 1.1.1
+					},
+				},
+			},
+			in: skews{
+				{
+					name:                "almond",
+					namespace:           "default",
+					maxOpenShiftVersion: "1.1.2",
+				},
+				{
+					name:                "beech",
+					namespace:           "default",
+					maxOpenShiftVersion: "1.1",
+				},
+			},
+			expect: expect{
+				err:          false,
+				incompatible: nil,
+			},
+		},
+		{
+			description: "ClusterPre",
+			cv: configv1.ClusterVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "version",
+				},
+				Status: configv1.ClusterVersionStatus{
+					Desired: configv1.Update{
+						Version: "1.1.0-pre", // Next Y-stream is 1.1.0, NOT 1.2.0
+					},
+				},
+			},
+			in: skews{
+				{
+					name:                "almond",
+					namespace:           "default",
+					maxOpenShiftVersion: "1.1.0",
+				},
+				{
+					name:                "beech",
+					namespace:           "default",
+					maxOpenShiftVersion: "1.1.0-pre",
+				},
+			},
+			expect: expect{
+				err: false,
+				incompatible: skews{
+					{
+						name:                "beech",
+						namespace:           "default",
+						maxOpenShiftVersion: "1.1.0-pre",
+					},
+				},
+			},
+		},
 	} {
 		t.Run(tt.description, func(t *testing.T) {
 			objs := []client.Object{tt.cv.DeepCopy()}