Skip to content

Commit a62d3bf

Browse files
joelanfordjoelanford
authored
remove ForceSemverUpgradeConstraints feature gate and implementation (#1638)
Signed-off-by: Joe Lanford <[email protected]>
1 parent 594cba3 commit a62d3bf

File tree

4 files changed

+4
-279
lines changed

4 files changed

+4
-279
lines changed

‎internal/catalogmetadata/filter/successors.go

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,9 @@ import (
99
"github.com/operator-framework/operator-registry/alpha/declcfg"
1010

1111
ocv1 "github.com/operator-framework/operator-controller/api/v1"
12-
"github.com/operator-framework/operator-controller/internal/features"
1312
)
1413

1514
func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (Predicate[declcfg.Bundle], error) {
16-
var successors successorsPredicateFunc = legacySuccessor
17-
if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) {
18-
successors = semverSuccessor
19-
}
20-
2115
installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version)
2216
if err != nil {
2317
return nil, fmt.Errorf("parsing installed bundle %q version %q: %w", installedBundle.Name, installedBundle.Version, err)
@@ -28,7 +22,7 @@ func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Chann
2822
return nil, fmt.Errorf("parsing installed version constraint %q: %w", installedBundleVersion.String(), err)
2923
}
3024

31-
successorsPredicate, err := successors(installedBundle, channels...)
25+
successorsPredicate, err := legacySuccessor(installedBundle, channels...)
3226
if err != nil {
3327
return nil, fmt.Errorf("getting successorsPredicate: %w", err)
3428
}
@@ -40,10 +34,6 @@ func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Chann
4034
), nil
4135
}
4236

43-
// successorsPredicateFunc returns a predicate to find successors
44-
// for a bundle. Predicate must not include the current version.
45-
type successorsPredicateFunc func(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (Predicate[declcfg.Bundle], error)
46-
4737
func legacySuccessor(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (Predicate[declcfg.Bundle], error) {
4838
installedBundleVersion, err := bsemver.Parse(installedBundle.Version)
4939
if err != nil {
@@ -84,28 +74,3 @@ func legacySuccessor(installedBundle ocv1.BundleMetadata, channels ...declcfg.Ch
8474
return false
8575
}, nil
8676
}
87-
88-
// semverSuccessor returns a predicate to find successors based on Semver.
89-
// Successors will not include versions outside the major version of the
90-
// installed bundle as major version is intended to indicate breaking changes.
91-
//
92-
// NOTE: semverSuccessor does not consider channels since there is no information
93-
// in a channel entry that is necessary to determine if a bundle is a successor.
94-
// A semver range check is the only necessary element. If filtering by channel
95-
// membership is necessary, an additional filter for that purpose should be applied.
96-
func semverSuccessor(installedBundle ocv1.BundleMetadata, _ ...declcfg.Channel) (Predicate[declcfg.Bundle], error) {
97-
currentVersion, err := mmsemver.NewVersion(installedBundle.Version)
98-
if err != nil {
99-
return nil, err
100-
}
101-
102-
// Based on current version create a caret range comparison constraint
103-
// to allow only minor and patch version as successors and exclude current version.
104-
constraintStr := fmt.Sprintf("^%[1]s, != %[1]s", currentVersion.String())
105-
wantedVersionRangeConstraint, err := mmsemver.NewConstraint(constraintStr)
106-
if err != nil {
107-
return nil, err
108-
}
109-
110-
return InMastermindsSemverRange(wantedVersionRangeConstraint), nil
111-
}

‎internal/catalogmetadata/filter/successors_test.go

Lines changed: 1 addition & 175 deletions
Original file line numberDiff line numberDiff line change
@@ -9,190 +9,16 @@ import (
99
"github.com/google/go-cmp/cmp/cmpopts"
1010
"github.com/stretchr/testify/assert"
1111
"github.com/stretchr/testify/require"
12-
featuregatetesting "k8s.io/component-base/featuregate/testing"
1312

1413
"github.com/operator-framework/operator-registry/alpha/declcfg"
1514
"github.com/operator-framework/operator-registry/alpha/property"
1615

1716
ocv1 "github.com/operator-framework/operator-controller/api/v1"
1817
"github.com/operator-framework/operator-controller/internal/bundleutil"
1918
"github.com/operator-framework/operator-controller/internal/catalogmetadata/compare"
20-
"github.com/operator-framework/operator-controller/internal/features"
2119
)
2220

23-
func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsEnabled(t *testing.T) {
24-
featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)
25-
26-
const testPackageName = "test-package"
27-
channelSet := map[string]declcfg.Channel{
28-
testPackageName: {
29-
Package: testPackageName,
30-
Name: "stable",
31-
},
32-
}
33-
34-
bundleSet := map[string]declcfg.Bundle{
35-
// Major version zero is for initial development and
36-
// has different update behaviour than versions >= 1.0.0:
37-
// - In versions 0.0.y updates are not allowed when using semver constraints
38-
// - In versions 0.x.y only patch updates are allowed (>= 0.x.y and < 0.x+1.0)
39-
// This means that we need in test data bundles that cover these three version ranges.
40-
"test-package.v0.0.1": {
41-
Name: "test-package.v0.0.1",
42-
Package: testPackageName,
43-
Image: "registry.io/repo/[email protected]",
44-
Properties: []property.Property{
45-
property.MustBuildPackage(testPackageName, "0.0.1"),
46-
},
47-
},
48-
"test-package.v0.0.2": {
49-
Name: "test-package.v0.0.2",
50-
Package: testPackageName,
51-
Image: "registry.io/repo/[email protected]",
52-
Properties: []property.Property{
53-
property.MustBuildPackage(testPackageName, "0.0.2"),
54-
},
55-
},
56-
"test-package.v0.1.0": {
57-
Name: "test-package.v0.1.0",
58-
Package: testPackageName,
59-
Image: "registry.io/repo/[email protected]",
60-
Properties: []property.Property{
61-
property.MustBuildPackage(testPackageName, "0.1.0"),
62-
},
63-
},
64-
"test-package.v0.1.1": {
65-
Name: "test-package.v0.1.1",
66-
Package: testPackageName,
67-
Image: "registry.io/repo/[email protected]",
68-
Properties: []property.Property{
69-
property.MustBuildPackage(testPackageName, "0.1.1"),
70-
},
71-
},
72-
"test-package.v0.1.2": {
73-
Name: "test-package.v0.1.2",
74-
Package: testPackageName,
75-
Image: "registry.io/repo/[email protected]",
76-
Properties: []property.Property{
77-
property.MustBuildPackage(testPackageName, "0.1.2"),
78-
},
79-
},
80-
"test-package.v0.2.0": {
81-
Name: "test-package.v0.2.0",
82-
Package: testPackageName,
83-
Image: "registry.io/repo/[email protected]",
84-
Properties: []property.Property{
85-
property.MustBuildPackage(testPackageName, "0.2.0"),
86-
},
87-
},
88-
"test-package.v2.0.0": {
89-
Name: "test-package.v2.0.0",
90-
Package: testPackageName,
91-
Image: "registry.io/repo/[email protected]",
92-
Properties: []property.Property{
93-
property.MustBuildPackage(testPackageName, "2.0.0"),
94-
},
95-
},
96-
"test-package.v2.1.0": {
97-
Name: "test-package.v2.1.0",
98-
Package: testPackageName,
99-
Image: "registry.io/repo/[email protected]",
100-
Properties: []property.Property{
101-
property.MustBuildPackage(testPackageName, "2.1.0"),
102-
},
103-
},
104-
"test-package.v2.2.0": {
105-
Name: "test-package.v2.2.0",
106-
Package: testPackageName,
107-
Image: "registry.io/repo/[email protected]",
108-
Properties: []property.Property{
109-
property.MustBuildPackage(testPackageName, "2.2.0"),
110-
},
111-
},
112-
// We need a bundle with a different major version to ensure
113-
// that we do not allow upgrades from one major version to another
114-
"test-package.v3.0.0": {
115-
Name: "test-package.v3.0.0",
116-
Package: testPackageName,
117-
Image: "registry.io/repo/[email protected]",
118-
Properties: []property.Property{
119-
property.MustBuildPackage(testPackageName, "3.0.0"),
120-
},
121-
},
122-
}
123-
124-
for _, b := range bundleSet {
125-
ch := channelSet[b.Package]
126-
ch.Entries = append(ch.Entries, declcfg.ChannelEntry{Name: b.Name})
127-
channelSet[b.Package] = ch
128-
}
129-
130-
for _, tt := range []struct {
131-
name string
132-
installedBundle ocv1.BundleMetadata
133-
expectedResult []declcfg.Bundle
134-
}{
135-
{
136-
name: "with non-zero major version",
137-
installedBundle: bundleutil.MetadataFor("test-package.v2.0.0", bsemver.MustParse("2.0.0")),
138-
expectedResult: []declcfg.Bundle{
139-
// Updates are allowed within the major version
140-
bundleSet["test-package.v2.2.0"],
141-
bundleSet["test-package.v2.1.0"],
142-
bundleSet["test-package.v2.0.0"],
143-
},
144-
},
145-
{
146-
name: "with zero major and zero minor version",
147-
installedBundle: bundleutil.MetadataFor("test-package.v0.0.1", bsemver.MustParse("0.0.1")),
148-
expectedResult: []declcfg.Bundle{
149-
// No updates are allowed in major version zero when minor version is also zero
150-
bundleSet["test-package.v0.0.1"],
151-
},
152-
},
153-
{
154-
name: "with zero major and non-zero minor version",
155-
installedBundle: bundleutil.MetadataFor("test-package.v0.1.0", bsemver.MustParse("0.1.0")),
156-
expectedResult: []declcfg.Bundle{
157-
// Patch version updates are allowed within the minor version
158-
bundleSet["test-package.v0.1.2"],
159-
bundleSet["test-package.v0.1.1"],
160-
bundleSet["test-package.v0.1.0"],
161-
},
162-
},
163-
{
164-
name: "installed bundle not found",
165-
installedBundle: ocv1.BundleMetadata{
166-
Name: "test-package.v9.0.0",
167-
Version: "9.0.0",
168-
},
169-
expectedResult: []declcfg.Bundle{},
170-
},
171-
} {
172-
t.Run(tt.name, func(t *testing.T) {
173-
successors, err := SuccessorsOf(tt.installedBundle, channelSet[testPackageName])
174-
require.NoError(t, err)
175-
176-
allBundles := make([]declcfg.Bundle, 0, len(bundleSet))
177-
for _, bundle := range bundleSet {
178-
allBundles = append(allBundles, bundle)
179-
}
180-
result := Filter(allBundles, successors)
181-
182-
// sort before comparison for stable order
183-
slices.SortFunc(result, compare.ByVersion)
184-
185-
gocmpopts := []cmp.Option{
186-
cmpopts.IgnoreUnexported(declcfg.Bundle{}),
187-
}
188-
require.Empty(t, cmp.Diff(result, tt.expectedResult, gocmpopts...))
189-
})
190-
}
191-
}
192-
193-
func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing.T) {
194-
featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)
195-
21+
func TestSuccessorsPredicate(t *testing.T) {
19622
const testPackageName = "test-package"
19723
channelSet := map[string]declcfg.Channel{
19824
testPackageName: {

‎internal/features/features.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,13 @@ import (
66
)
77

88
const (
9-
// Add new feature gates constants (strings)
10-
// Ex: SomeFeature featuregate.Feature = "SomeFeature"
11-
12-
ForceSemverUpgradeConstraints featuregate.Feature = "ForceSemverUpgradeConstraints"
9+
// Add new feature gates constants (strings)
10+
// Ex: SomeFeature featuregate.Feature = "SomeFeature"
1311
)
1412

1513
var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
1614
// Add new feature gate definitions
1715
// Ex: SomeFeature: {...}
18-
19-
ForceSemverUpgradeConstraints: {Default: false, PreRelease: featuregate.Alpha},
2016
}
2117

2218
var OperatorControllerFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate()

‎internal/resolve/catalog_test.go

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1313
"k8s.io/apimachinery/pkg/labels"
1414
"k8s.io/apimachinery/pkg/util/rand"
15-
featuregatetesting "k8s.io/component-base/featuregate/testing"
1615
"k8s.io/utils/ptr"
1716
"sigs.k8s.io/controller-runtime/pkg/client"
1817

@@ -21,7 +20,6 @@ import (
2120

2221
ocv1 "github.com/operator-framework/operator-controller/api/v1"
2322
catalogd "github.com/operator-framework/operator-controller/catalogd/api/v1"
24-
"github.com/operator-framework/operator-controller/internal/features"
2523
)
2624

2725
func TestInvalidClusterExtensionVersionRange(t *testing.T) {
@@ -426,7 +424,6 @@ func TestPackageVariationsBetweenCatalogs(t *testing.T) {
426424
}
427425

428426
func TestUpgradeFoundLegacy(t *testing.T) {
429-
featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)
430427
pkgName := randPkg()
431428
w := staticCatalogWalker{
432429
"a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) {
@@ -454,7 +451,6 @@ func TestUpgradeFoundLegacy(t *testing.T) {
454451
}
455452

456453
func TestUpgradeNotFoundLegacy(t *testing.T) {
457-
featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)
458454
pkgName := randPkg()
459455
w := staticCatalogWalker{
460456
"a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) {
@@ -478,62 +474,6 @@ func TestUpgradeNotFoundLegacy(t *testing.T) {
478474
assert.EqualError(t, err, fmt.Sprintf(`error upgrading from currently installed version "0.1.0": no bundles found for package %q matching version "<1.0.0 >=2.0.0"`, pkgName))
479475
}
480476

481-
func TestUpgradeFoundSemver(t *testing.T) {
482-
featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)
483-
pkgName := randPkg()
484-
w := staticCatalogWalker{
485-
"a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) {
486-
return &declcfg.DeclarativeConfig{}, nil, nil
487-
},
488-
"b": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) {
489-
return &declcfg.DeclarativeConfig{}, nil, nil
490-
},
491-
"c": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) {
492-
return genPackage(pkgName), nil, nil
493-
},
494-
}
495-
r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs}
496-
ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided)
497-
installedBundle := &ocv1.BundleMetadata{
498-
Name: bundleName(pkgName, "1.0.0"),
499-
Version: "1.0.0",
500-
}
501-
// there is a legacy upgrade edge from 1.0.0 to 2.0.0, but we are using semver semantics here.
502-
// therefore:
503-
// 1.0.0 => 1.0.2 is what we expect
504-
gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, installedBundle)
505-
require.NoError(t, err)
506-
assert.Equal(t, genBundle(pkgName, "1.0.2"), *gotBundle)
507-
assert.Equal(t, bsemver.MustParse("1.0.2"), *gotVersion)
508-
assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation)
509-
}
510-
511-
func TestUpgradeNotFoundSemver(t *testing.T) {
512-
featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)
513-
pkgName := randPkg()
514-
w := staticCatalogWalker{
515-
"a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) {
516-
return &declcfg.DeclarativeConfig{}, nil, nil
517-
},
518-
"b": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) {
519-
return &declcfg.DeclarativeConfig{}, nil, nil
520-
},
521-
"c": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) {
522-
return genPackage(pkgName), nil, nil
523-
},
524-
}
525-
r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs}
526-
ce := buildFooClusterExtension(pkgName, []string{}, "!=0.1.0", ocv1.UpgradeConstraintPolicyCatalogProvided)
527-
installedBundle := &ocv1.BundleMetadata{
528-
Name: bundleName(pkgName, "0.1.0"),
529-
Version: "0.1.0",
530-
}
531-
// there are legacy upgrade edges from 0.1.0 to 1.0.x, but we are using semver semantics here.
532-
// therefore, we expect to fail because there are no semver-compatible upgrade edges from 0.1.0.
533-
_, _, _, err := r.Resolve(context.Background(), ce, installedBundle)
534-
assert.EqualError(t, err, fmt.Sprintf(`error upgrading from currently installed version "0.1.0": no bundles found for package %q matching version "!=0.1.0"`, pkgName))
535-
}
536-
537477
func TestDowngradeFound(t *testing.T) {
538478
pkgName := randPkg()
539479
w := staticCatalogWalker{
@@ -838,7 +778,6 @@ func TestInvalidClusterExtensionCatalogMatchLabelsValue(t *testing.T) {
838778
}
839779

840780
func TestClusterExtensionMatchLabel(t *testing.T) {
841-
featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)
842781
pkgName := randPkg()
843782
w := staticCatalogWalker{
844783
"a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) {
@@ -859,7 +798,6 @@ func TestClusterExtensionMatchLabel(t *testing.T) {
859798
}
860799

861800
func TestClusterExtensionNoMatchLabel(t *testing.T) {
862-
featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)
863801
pkgName := randPkg()
864802
w := staticCatalogWalker{
865803
"a": func() (*declcfg.DeclarativeConfig, *catalogd.ClusterCatalogSpec, error) {

0 commit comments

Comments
 (0)