Add initial SemVer upgrade support (#444)

* Add initial SemVer upgrade support OLM will now use SemVer to determine next upgrade. In this iteration we only support upgrade within the same major versions: e.g 1.0.1 can be upgraded to 1.0.2 or 1.3.2, but not 2.0.0. Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com> * Update E2E * Add tests for semver upgrades * Label semver and legacy tests so it is possible to filter one or the other out * Update Makefile to no run legacy tests by default to match default deployment of operator-controller Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com> --------- Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
operator-framework · Oct 19, 2023 · fd02095 · fd02095
1 parent c9d387e
commit fd02095
Show file tree

Hide file tree

Showing 5 changed files with 162 additions and 44 deletions.
diff --git a/Makefile b/Makefile
@@ -20,8 +20,8 @@ export XDG_DATA_HOME ?= /tmp/.local/share
 include .bingo/Variables.mk
 
 # ARTIFACT_PATH is the absolute path to the directory where the operator-controller e2e tests will store the artifacts
-# for example: ARTIFACT_PATH=/tmp/artifacts make test 
-export ARTIFACT_PATH ?= 
+# for example: ARTIFACT_PATH=/tmp/artifacts make test
+export ARTIFACT_PATH ?=
 
 OPERATOR_CONTROLLER_NAMESPACE ?= operator-controller-system
 KIND_CLUSTER_NAME ?= operator-controller
@@ -156,11 +156,19 @@ kind-load-test-artifacts: $(KIND) #EXHELP Load the e2e testdata container images
 	$(CONTAINER_RUNTIME) build $(TESTDATA_DIR)/bundles/registry-v1/prometheus-operator.v0.37.0 -t localhost/testdata/bundles/registry-v1/prometheus-operator:v0.37.0
 	$(CONTAINER_RUNTIME) build $(TESTDATA_DIR)/bundles/registry-v1/prometheus-operator.v0.47.0 -t localhost/testdata/bundles/registry-v1/prometheus-operator:v0.47.0
 	$(CONTAINER_RUNTIME) build $(TESTDATA_DIR)/bundles/registry-v1/prometheus-operator.v0.65.1 -t localhost/testdata/bundles/registry-v1/prometheus-operator:v0.65.1
+	$(CONTAINER_RUNTIME) tag localhost/testdata/bundles/registry-v1/prometheus-operator:v0.65.1 localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0
+	$(CONTAINER_RUNTIME) tag localhost/testdata/bundles/registry-v1/prometheus-operator:v0.65.1 localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.1
+	$(CONTAINER_RUNTIME) tag localhost/testdata/bundles/registry-v1/prometheus-operator:v0.65.1 localhost/testdata/bundles/registry-v1/prometheus-operator:v1.2.0
+	$(CONTAINER_RUNTIME) tag localhost/testdata/bundles/registry-v1/prometheus-operator:v0.65.1 localhost/testdata/bundles/registry-v1/prometheus-operator:v2.0.0
 	$(CONTAINER_RUNTIME) build $(TESTDATA_DIR)/bundles/plain-v0/plain.v0.1.0 -t localhost/testdata/bundles/plain-v0/plain:v0.1.0
 	$(CONTAINER_RUNTIME) build $(TESTDATA_DIR)/catalogs -f $(TESTDATA_DIR)/catalogs/test-catalog.Dockerfile -t localhost/testdata/catalogs/test-catalog:e2e
 	$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v0.37.0 --name $(KIND_CLUSTER_NAME)
 	$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v0.47.0 --name $(KIND_CLUSTER_NAME)
 	$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v0.65.1 --name $(KIND_CLUSTER_NAME)
+	$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0 --name $(KIND_CLUSTER_NAME)
+	$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.1 --name $(KIND_CLUSTER_NAME)
+	$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v1.2.0 --name $(KIND_CLUSTER_NAME)
+	$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v2.0.0 --name $(KIND_CLUSTER_NAME)
 	$(KIND) load docker-image localhost/testdata/bundles/plain-v0/plain:v0.1.0 --name $(KIND_CLUSTER_NAME)
 	$(KIND) load docker-image localhost/testdata/catalogs/test-catalog:e2e --name $(KIND_CLUSTER_NAME)
 

diff --git a/internal/resolution/variablesources/installed_package.go b/internal/resolution/variablesources/installed_package.go
@@ -5,13 +5,15 @@ import (
 	"fmt"
 	"sort"
 
+	mmsemver "github.com/Masterminds/semver/v3"
 	"github.com/operator-framework/deppy/pkg/deppy"
 	"github.com/operator-framework/deppy/pkg/deppy/input"
 
 	"github.com/operator-framework/operator-controller/internal/catalogmetadata"
 	catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
 	catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
 	"github.com/operator-framework/operator-controller/internal/resolution/variables"
+	"github.com/operator-framework/operator-controller/pkg/features"
 )
 
 var _ input.VariableSource = &InstalledPackageVariableSource{}
@@ -59,10 +61,15 @@ func (r *InstalledPackageVariableSource) notFoundError() error {
 }
 
 func NewInstalledPackageVariableSource(catalogClient BundleProvider, bundleImage string) (*InstalledPackageVariableSource, error) {
+	successors := legacySemanticsSuccessors
+	if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) {
+		successors = semverSuccessors
+	}
+
 	return &InstalledPackageVariableSource{
 		catalogClient: catalogClient,
 		bundleImage:   bundleImage,
-		successors:    legacySemanticsSuccessors,
+		successors:    successors,
 	}, nil
 }
 
@@ -83,3 +90,28 @@ func legacySemanticsSuccessors(allBundles []*catalogmetadata.Bundle, installedBu
 
 	return upgradeEdges, nil
 }
+
+// semverSuccessors returns successors based on Semver.
+// Successors will not include versions outside the major version of the
+// installed bundle as major version is intended to indicate breaking changes.
+func semverSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
+	currentVersion, err := installedBundle.Version()
+	if err != nil {
+		return nil, err
+	}
+
+	// Based on current version create a caret range comparison constraint
+	// to allow only minor and patch version as successors and exclude current version.
+	constraintStr := fmt.Sprintf("^%s, != %s", currentVersion.String(), currentVersion.String())
+	wantedVersionRangeConstraint, err := mmsemver.NewConstraint(constraintStr)
+	if err != nil {
+		return nil, err
+	}
+
+	upgradeEdges := catalogfilter.Filter(allBundles, catalogfilter.InMastermindsSemverRange(wantedVersionRangeConstraint))
+	sort.SliceStable(upgradeEdges, func(i, j int) bool {
+		return catalogsort.ByVersion(upgradeEdges[i], upgradeEdges[j])
+	})
+
+	return upgradeEdges, nil
+}
diff --git a/internal/resolution/variablesources/installed_package_test.go b/internal/resolution/variablesources/installed_package_test.go
@@ -121,6 +121,27 @@ func TestInstalledPackageVariableSource(t *testing.T) {
 	const bundleImage = "registry.io/repo/test-package@v2.0.0"
 	fakeCatalogClient := testutil.NewFakeCatalogClient(bundleList)
 
+	t.Run("with ForceSemverUpgradeConstraints feature gate enabled", func(t *testing.T) {
+		defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)()
+
+		ipvs, err := variablesources.NewInstalledPackageVariableSource(&fakeCatalogClient, bundleImage)
+		require.NoError(t, err)
+
+		variables, err := ipvs.GetVariables(context.TODO())
+		require.NoError(t, err)
+		require.Len(t, variables, 1)
+		packageVariable, ok := variables[0].(*olmvariables.InstalledPackageVariable)
+		assert.True(t, ok)
+		assert.Equal(t, deppy.IdentifierFromString("installed package test-package"), packageVariable.Identifier())
+
+		// ensure bundles are in version order (high to low)
+		bundles := packageVariable.Bundles()
+		require.Len(t, bundles, 3)
+		assert.Equal(t, "test-package.v2.2.0", packageVariable.Bundles()[0].Name)
+		assert.Equal(t, "test-package.v2.1.0", packageVariable.Bundles()[1].Name)
+		assert.Equal(t, "test-package.v2.0.0", packageVariable.Bundles()[2].Name)
+	})
+
 	t.Run("with ForceSemverUpgradeConstraints feature gate disabled", func(t *testing.T) {
 		defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)()
 

diff --git a/test/e2e/install_test.go b/test/e2e/install_test.go
@@ -67,7 +67,7 @@ var _ = Describe("Operator Install", func() {
 					g.Expect(cond.Status).To(Equal(metav1.ConditionTrue))
 					g.Expect(cond.Reason).To(Equal(operatorv1alpha1.ReasonSuccess))
 					g.Expect(cond.Message).To(ContainSubstring("resolved to"))
-					g.Expect(operator.Status.ResolvedBundleResource).To(Equal("localhost/testdata/bundles/registry-v1/prometheus-operator:v0.65.1"))
+					g.Expect(operator.Status.ResolvedBundleResource).To(Equal("localhost/testdata/bundles/registry-v1/prometheus-operator:v2.0.0"))
 				}).Should(Succeed())
 
 				By("eventually installing the package successfully")
@@ -180,48 +180,57 @@ var _ = Describe("Operator Install", func() {
 			}).Should(Succeed())
 		})
 
-		It("handles upgrade edges correctly", func() {
-			By("creating a valid Operator resource")
-			operator.Spec = operatorv1alpha1.OperatorSpec{
-				PackageName: "prometheus",
-				Version:     "0.37.0",
-			}
-			Expect(c.Create(ctx, operator)).To(Succeed())
-			By("eventually reporting a successful resolution")
-			Eventually(func(g Gomega) {
-				g.Expect(c.Get(ctx, types.NamespacedName{Name: operator.Name}, operator)).To(Succeed())
-				cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorv1alpha1.TypeResolved)
-				g.Expect(cond).ToNot(BeNil())
-				g.Expect(cond.Reason).To(Equal(operatorv1alpha1.ReasonSuccess))
-				g.Expect(cond.Message).To(ContainSubstring("resolved to"))
-				g.Expect(operator.Status.ResolvedBundleResource).ToNot(BeEmpty())
-			}).Should(Succeed())
+		When("resolving upgrade edges", func() {
+			BeforeEach(func() {
+				By("creating an Operator at a specified version")
+				operator.Spec = operatorv1alpha1.OperatorSpec{
+					PackageName: "prometheus",
+					Version:     "1.0.0",
+				}
+				Expect(c.Create(ctx, operator)).To(Succeed())
+				By("eventually reporting a successful resolution")
+				Eventually(func(g Gomega) {
+					g.Expect(c.Get(ctx, types.NamespacedName{Name: operator.Name}, operator)).To(Succeed())
+					cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorv1alpha1.TypeResolved)
+					g.Expect(cond).ToNot(BeNil())
+					g.Expect(cond.Reason).To(Equal(operatorv1alpha1.ReasonSuccess))
+					g.Expect(cond.Message).To(ContainSubstring("resolved to"))
+					g.Expect(operator.Status.ResolvedBundleResource).To(Equal("localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0"))
+				}).Should(Succeed())
+			})
 
-			By("updating the Operator resource to a non-successor version")
-			operator.Spec.Version = "0.65.1" // current (0.37.0) and successor (0.47.0) are the only values that would be SAT.
-			Expect(c.Update(ctx, operator)).To(Succeed())
-			By("eventually reporting an unsatisfiable resolution")
-			Eventually(func(g Gomega) {
-				g.Expect(c.Get(ctx, types.NamespacedName{Name: operator.Name}, operator)).To(Succeed())
-				cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorv1alpha1.TypeResolved)
-				g.Expect(cond).ToNot(BeNil())
-				g.Expect(cond.Reason).To(Equal(operatorv1alpha1.ReasonResolutionFailed))
-				g.Expect(cond.Message).To(MatchRegexp(`^constraints not satisfiable:.*; installed package prometheus requires at least one of.*0.47.0[^,]*,[^,]*0.37.0[^;]*;.*`))
-				g.Expect(operator.Status.ResolvedBundleResource).To(BeEmpty())
-			}).Should(Succeed())
+			It("does not allow to upgrade the Operator to a non-successor version", func() {
+				By("updating the Operator resource to a non-successor version")
+				// Semver only allows upgrades within major version at the moment.
+				operator.Spec.Version = "2.0.0"
+				Expect(c.Update(ctx, operator)).To(Succeed())
+				By("eventually reporting an unsatisfiable resolution")
+				Eventually(func(g Gomega) {
+					g.Expect(c.Get(ctx, types.NamespacedName{Name: operator.Name}, operator)).To(Succeed())
+					cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorv1alpha1.TypeResolved)
+					g.Expect(cond).ToNot(BeNil())
+					g.Expect(cond.Reason).To(Equal(operatorv1alpha1.ReasonResolutionFailed))
+					g.Expect(cond.Message).To(ContainSubstring("constraints not satisfiable"))
+					g.Expect(cond.Message).To(ContainSubstring("installed package prometheus requires at least one of test-catalog-prometheus-prometheus-operator.1.2.0, test-catalog-prometheus-prometheus-operator.1.0.1, test-catalog-prometheus-prometheus-operator.1.0.0;"))
+					g.Expect(operator.Status.ResolvedBundleResource).To(BeEmpty())
+				}).Should(Succeed())
+			})
 
-			By("updating the Operator resource to a valid upgrade edge")
-			operator.Spec.Version = "0.47.0"
-			Expect(c.Update(ctx, operator)).To(Succeed())
-			By("eventually reporting a successful resolution and bundle path")
-			Eventually(func(g Gomega) {
-				g.Expect(c.Get(ctx, types.NamespacedName{Name: operator.Name}, operator)).To(Succeed())
-				cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorv1alpha1.TypeResolved)
-				g.Expect(cond).ToNot(BeNil())
-				g.Expect(cond.Reason).To(Equal(operatorv1alpha1.ReasonSuccess))
-				g.Expect(cond.Message).To(ContainSubstring("resolved to"))
-				g.Expect(operator.Status.ResolvedBundleResource).ToNot(BeEmpty())
-			}).Should(Succeed())
+			It("does allow to upgrade the Operator to any of the successor versions within non-zero major version", func() {
+				By("updating the Operator resource by skipping versions")
+				// Test catalog has versions between the initial version and new version
+				operator.Spec.Version = "1.2.0"
+				Expect(c.Update(ctx, operator)).To(Succeed())
+				By("eventually reporting a successful resolution and bundle path")
+				Eventually(func(g Gomega) {
+					g.Expect(c.Get(ctx, types.NamespacedName{Name: operator.Name}, operator)).To(Succeed())
+					cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorv1alpha1.TypeResolved)
+					g.Expect(cond).ToNot(BeNil())
+					g.Expect(cond.Reason).To(Equal(operatorv1alpha1.ReasonSuccess))
+					g.Expect(cond.Message).To(ContainSubstring("resolved to"))
+					g.Expect(operator.Status.ResolvedBundleResource).To(Equal("localhost/testdata/bundles/registry-v1/prometheus-operator:v1.2.0"))
+				}).Should(Succeed())
+			})
 		})
 
 		AfterEach(func() {

diff --git a/testdata/catalogs/test-catalog/catalog.yaml b/testdata/catalogs/test-catalog/catalog.yaml
@@ -18,6 +18,14 @@ entries:
     replaces: prometheus-operator.0.37.0
   - name: prometheus-operator.0.65.1
     replaces: prometheus-operator.0.47.0
+  - name: prometheus-operator.1.0.0
+    replaces: prometheus-operator.0.65.1
+  - name: prometheus-operator.1.0.1
+    replaces: prometheus-operator.1.0.0
+  - name: prometheus-operator.1.2.0
+    replaces: prometheus-operator.1.0.1
+  - name: prometheus-operator.2.0.0
+    replaces: prometheus-operator.1.2.0
 ---
 schema: olm.bundle
 name: prometheus-operator.0.37.0
@@ -49,6 +57,46 @@ properties:
       packageName: prometheus
       version: 0.65.1
 ---
+schema: olm.bundle
+name: prometheus-operator.1.0.0
+package: prometheus
+image: localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0
+properties:
+  - type: olm.package
+    value:
+      packageName: prometheus
+      version: 1.0.0
+---
+schema: olm.bundle
+name: prometheus-operator.1.0.1
+package: prometheus
+image: localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.1
+properties:
+  - type: olm.package
+    value:
+      packageName: prometheus
+      version: 1.0.1
+---
+schema: olm.bundle
+name: prometheus-operator.1.2.0
+package: prometheus
+image: localhost/testdata/bundles/registry-v1/prometheus-operator:v1.2.0
+properties:
+  - type: olm.package
+    value:
+      packageName: prometheus
+      version: 1.2.0
+---
+schema: olm.bundle
+name: prometheus-operator.2.0.0
+package: prometheus
+image: localhost/testdata/bundles/registry-v1/prometheus-operator:v2.0.0
+properties:
+  - type: olm.package
+    value:
+      packageName: prometheus
+      version: 2.0.0
+---
 schema: olm.package
 name: plain
 defaultChannel: beta