NE-2097: Avoid installing past desired version

This commit introduces a sematic version check for the CSV of next
installplan. If the next installplan has a version which is sematically
past the desired it's not considered for the upgrade.

Author: alebedev87

go.mod

@@ -15,6 +15,7 @@ require (
 	github.com/IBM/go-sdk-core/v5 v5.17.4
 	github.com/IBM/networking-go-sdk v0.26.0
 	github.com/aws/aws-sdk-go v1.38.49
+	github.com/coreos/go-semver v0.3.1
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
 	github.com/florianl/go-nfqueue v1.3.2
 	github.com/go-logr/logr v1.4.2

go.sum

@@ -80,6 +80,8 @@ github.com/cncf/xds/go v0.0.0-20210922020428-25de7278fc84/go.mod h1:eXthEFrGJvWH
 github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
 github.com/cncf/xds/go v0.0.0-20240905190251-b4127c9b8d78 h1:QVw89YDxXxEe+l8gU8ETbOasdwEV+avkR75ZzsVV9WI=
 github.com/cncf/xds/go v0.0.0-20240905190251-b4127c9b8d78/go.mod h1:W+zGtBO5Y1IgJhy4+A9GOqVhqLpfZi+vwmdNXUehLA8=
+github.com/coreos/go-semver v0.3.1 h1:yi21YpKnrx1gt5R+la8n5WgS0kCrsPp33dmEyHReZr4=
+github.com/coreos/go-semver v0.3.1/go.mod h1:irMmmIw/7yzSRPWryHsK7EYSg09caPQL03VsM8rvUec=
 github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

pkg/operator/controller/gatewayclass/subscription.go

@@ -3,7 +3,9 @@ package gatewayclass
 import (
 	"context"
 	"fmt"
+	"regexp"
 
+	"github.com/coreos/go-semver/semver"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -31,6 +33,11 @@ const (
 	WorkloadPartitioningManagementPreferredScheduling = `{"effect": "PreferredDuringScheduling"}`
 )
 
+var (
+	// csvSemVerRegexp is a RegExp to extract semantic version from OLM CSV name.
+	csvSemVerRegexp = regexp.MustCompile(`v(\d+\.\d+\.\d+)`)
+)
+
 // ensureServiceMeshOperatorSubscription attempts to ensure that a subscription
 // for servicemeshoperator is present and returns a Boolean indicating whether
 // it exists, the subscription if it exists, and an error value.
@@ -193,6 +200,8 @@ func (r *reconciler) currentInstallPlan(ctx context.Context) (bool, *operatorsv1
 	}
 	var currentInstallPlan, nextInstallPlan *operatorsv1alpha1.InstallPlan
 	multipleInstallPlans := false
+	desiredCSVSemVer, currentCSVSemVer := extractSemVerFromCSV(subscription.Spec.StartingCSV), extractSemVerFromCSV(subscription.Status.CurrentCSV)
+
 	for _, installPlan := range installPlans.Items {
 		if len(installPlan.OwnerReferences) == 0 || len(installPlan.Spec.ClusterServiceVersionNames) == 0 {
 			continue
@@ -249,9 +258,16 @@ func (r *reconciler) currentInstallPlan(ctx context.Context) (bool, *operatorsv1
 			// The first condition (below) prevents setting the next InstallPlan while a replacement
 			// or installation is ongoing, or when the end of the upgrade graph is reached.
 			if subscription.Status.InstalledCSV != subscription.Status.CurrentCSV && csvName == subscription.Status.CurrentCSV {
-				if nextInstallPlan == nil {
-					nextInstallPlan = &installPlan
-					break
+				// CurrentCSV should not be greater (semver-wise) than desiredCSV to avoid upgrading past the desired version,
+				// in case the desired one was skipped.
+				// Note: since we use the "stable" channel, the upgrade graph allows transitions between minor releases.
+				// For example, after installing 3.0.3, we may see 3.1.0 in currentCSV.
+				if currentCSVSemVer != nil && desiredCSVSemVer != nil && if currentCSVSemVer.Compare(*desiredCSVSemVer) != 1 {
+						if nextInstallPlan == nil {
+							nextInstallPlan = &installPlan
+							break
+						}
+					}
 				}
 			}
 		}
@@ -311,3 +327,15 @@ func installPlanChanged(current, expected *operatorsv1alpha1.InstallPlan) (bool,
 
 	return true, updated
 }
+
+// extractSemVerFromCSV exctracts the semantic version from an OLM CSV name.
+// Returns a nil pointer if it fails to parse the CSV name.
+func extractSemVerFromCSV(csv string) *semver.Version {
+	match := csvSemVerRegexp.FindStringSubmatch(csv)
+	if len(match) > 1 {
+		if v, err := semver.NewVersion(match[1]); err == nil {
+			return v
+		}
+	}
+	return nil
+}

pkg/operator/controller/gatewayclass/subscription_test.go

@@ -986,6 +986,77 @@ func Test_ensureServiceMeshOperatorInstallPlan(t *testing.T) {
 			},
 			expectUpdate: []client.Object{},
 		},
+		{
+			name:    "Upgrade to non-existing desired version",
+			channel: "stable",
+			version: "servicemeshoperator.v1.0.2",
+			existingObjects: []runtime.Object{
+				&operatorsv1alpha1.Subscription{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: operatorcontroller.ServiceMeshOperatorSubscriptionName().Namespace,
+						Name:      operatorcontroller.ServiceMeshOperatorSubscriptionName().Name,
+						UID:       "foobar",
+					},
+					Spec: &operatorsv1alpha1.SubscriptionSpec{
+						Channel:                "stable",
+						InstallPlanApproval:    operatorsv1alpha1.ApprovalManual,
+						Package:                "servicemeshoperator",
+						CatalogSource:          "redhat-operators",
+						CatalogSourceNamespace: "openshift-marketplace",
+						StartingCSV:            "servicemeshoperator.v1.0.2",
+					},
+					Status: operatorsv1alpha1.SubscriptionStatus{
+						InstalledCSV: "servicemeshoperator.v1.0.1",
+						CurrentCSV:   "servicemeshoperator.v1.0.3", // 1.0.2 was skipped from upgrade graph
+					},
+				},
+				// Current
+				&operatorsv1alpha1.InstallPlan{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "Install-bar",
+						Namespace: operatorcontroller.OpenshiftOperatorNamespace,
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								UID: "foobar",
+							},
+						},
+					},
+					Spec: operatorsv1alpha1.InstallPlanSpec{
+						ClusterServiceVersionNames: []string{
+							"servicemeshoperator.v1.0.1",
+						},
+						Approval: operatorsv1alpha1.ApprovalManual,
+						Approved: true,
+					},
+					Status: operatorsv1alpha1.InstallPlanStatus{
+						Phase: operatorsv1alpha1.InstallPlanPhaseComplete,
+					},
+				},
+				// N+2 (and expected is in between but skipped).
+				&operatorsv1alpha1.InstallPlan{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "Install-baz",
+						Namespace: operatorcontroller.OpenshiftOperatorNamespace,
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								UID: "foobar",
+							},
+						},
+					},
+					Spec: operatorsv1alpha1.InstallPlanSpec{
+						ClusterServiceVersionNames: []string{
+							"servicemeshoperator.v1.0.3",
+						},
+						Approval: operatorsv1alpha1.ApprovalManual,
+						Approved: false,
+					},
+					Status: operatorsv1alpha1.InstallPlanStatus{
+						Phase: operatorsv1alpha1.InstallPlanPhaseRequiresApproval,
+					},
+				},
+			},
+			expectUpdate: []client.Object{},
+		},
 	}
 
 	scheme := runtime.NewScheme()

vendor/modules.txt

@@ -176,6 +176,9 @@ github.com/cespare/xxhash/v2
 ## explicit; go 1.19
 github.com/cncf/xds/go/xds/data/orca/v3
 github.com/cncf/xds/go/xds/service/orca/v3
+# github.com/coreos/go-semver v0.3.1
+## explicit; go 1.8
+github.com/coreos/go-semver/semver
 # github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
 ## explicit
 github.com/davecgh/go-spew/spew