Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add initial SemVer upgrade support #444

Merged
merged 2 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?=
m1kola marked this conversation as resolved.
Show resolved Hide resolved

OPERATOR_CONTROLLER_NAMESPACE ?= operator-controller-system
KIND_CLUSTER_NAME ?= operator-controller
Expand Down Expand Up @@ -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)

Expand Down
34 changes: 33 additions & 1 deletion internal/resolution/variablesources/installed_package.go
m1kola marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import (
"fmt"
"sort"

mmsemver "github.com/Masterminds/semver/v3"
m1kola marked this conversation as resolved.
Show resolved Hide resolved
"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{}
Expand Down Expand Up @@ -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
}

Expand All @@ -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())
awgreene marked this conversation as resolved.
Show resolved Hide resolved
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
}
21 changes: 21 additions & 0 deletions internal/resolution/variablesources/installed_package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
awgreene marked this conversation as resolved.
Show resolved Hide resolved
})

t.Run("with ForceSemverUpgradeConstraints feature gate disabled", func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)()

Expand Down
91 changes: 50 additions & 41 deletions test/e2e/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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() {
m1kola marked this conversation as resolved.
Show resolved Hide resolved
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.
awgreene marked this conversation as resolved.
Show resolved Hide resolved
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"))
awgreene marked this conversation as resolved.
Show resolved Hide resolved
}).Should(Succeed())
})
})

AfterEach(func() {
Expand Down
48 changes: 48 additions & 0 deletions testdata/catalogs/test-catalog/catalog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down