From bf11c93300d4f62a7b5002b26a37656a1604220c Mon Sep 17 00:00:00 2001 From: Nick Hale Date: Wed, 11 Aug 2021 13:01:43 -0400 Subject: [PATCH] fix(openshift): drop z from next calculated y-stream 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 --- pkg/controller/operators/openshift/helpers.go | 28 +++++-- .../operators/openshift/helpers_test.go | 83 ++++++++++++++++++- 2 files changed, 104 insertions(+), 7 deletions(-) diff --git a/pkg/controller/operators/openshift/helpers.go b/pkg/controller/operators/openshift/helpers.go index 3c6159518d..112bbd42cf 100644 --- a/pkg/controller/operators/openshift/helpers.go +++ b/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" ) diff --git a/pkg/controller/operators/openshift/helpers_test.go b/pkg/controller/operators/openshift/helpers_test.go index 1f4b57120b..69e8f3a832 100644 --- a/pkg/controller/operators/openshift/helpers_test.go +++ b/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()}