Skip to content

Commit f1d4d8e

Browse files
author
Mikalai Radchuk
committed
Respect .spec.upgradeConstraintPolicy
This commit makes OLM respect `.spec.upgradeConstraintPolicy` set on an `Operator` CR when chosing upgrade edges. Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
1 parent cd784a0 commit f1d4d8e

File tree

5 files changed

+197
-22
lines changed

5 files changed

+197
-22
lines changed

internal/controllers/variable_source.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func (v *VariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, er
7171
return variablesources.NewOperatorVariableSource(operatorList.Items, allBundles, inputVariableSource), nil
7272
},
7373
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
74-
return variablesources.NewBundleDeploymentVariableSource(bundleDeploymentList.Items, allBundles, inputVariableSource), nil
74+
return variablesources.NewBundleDeploymentVariableSource(operatorList.Items, bundleDeploymentList.Items, allBundles, inputVariableSource), nil
7575
},
7676
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
7777
return variablesources.NewBundlesAndDepsVariableSource(allBundles, inputVariableSource), nil

internal/resolution/variablesources/bundle_deployment.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,22 @@ import (
77
"github.com/operator-framework/deppy/pkg/deppy/input"
88
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
99

10+
operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
1011
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
1112
)
1213

1314
var _ input.VariableSource = &BundleDeploymentVariableSource{}
1415

1516
type BundleDeploymentVariableSource struct {
17+
operators []operatorsv1alpha1.Operator
1618
bundleDeployments []rukpakv1alpha1.BundleDeployment
1719
allBundles []*catalogmetadata.Bundle
1820
inputVariableSource input.VariableSource
1921
}
2022

21-
func NewBundleDeploymentVariableSource(bundleDeployments []rukpakv1alpha1.BundleDeployment, allBundles []*catalogmetadata.Bundle, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource {
23+
func NewBundleDeploymentVariableSource(operators []operatorsv1alpha1.Operator, bundleDeployments []rukpakv1alpha1.BundleDeployment, allBundles []*catalogmetadata.Bundle, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource {
2224
return &BundleDeploymentVariableSource{
25+
operators: operators,
2326
bundleDeployments: bundleDeployments,
2427
allBundles: allBundles,
2528
inputVariableSource: inputVariableSource,
@@ -37,7 +40,7 @@ func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context) ([]de
3740
return nil, err
3841
}
3942

40-
installedPackages, err := MakeInstalledPackageVariables(o.allBundles, o.bundleDeployments)
43+
installedPackages, err := MakeInstalledPackageVariables(o.allBundles, o.operators, o.bundleDeployments)
4144
if err != nil {
4245
return nil, err
4346
}

internal/resolution/variablesources/bundle_deployment_test.go

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,32 @@ import (
1010
"github.com/operator-framework/operator-registry/alpha/declcfg"
1111
"github.com/operator-framework/operator-registry/alpha/property"
1212

13+
operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
1314
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
1415
olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
1516
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
1617

1718
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
19+
"k8s.io/utils/pointer"
1820

1921
"github.com/operator-framework/deppy/pkg/deppy"
2022
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
2123
)
2224

23-
func bundleDeployment(name, image string) rukpakv1alpha1.BundleDeployment {
24-
return rukpakv1alpha1.BundleDeployment{
25+
func fakeOperator(name, packageName string, upgradeConstraintPolicy operatorsv1alpha1.UpgradeConstraintPolicy) operatorsv1alpha1.Operator {
26+
return operatorsv1alpha1.Operator{
27+
ObjectMeta: metav1.ObjectMeta{
28+
Name: name,
29+
},
30+
Spec: operatorsv1alpha1.OperatorSpec{
31+
PackageName: packageName,
32+
UpgradeConstraintPolicy: upgradeConstraintPolicy,
33+
},
34+
}
35+
}
36+
37+
func bundleDeployment(name, image string, owner *operatorsv1alpha1.Operator) rukpakv1alpha1.BundleDeployment {
38+
bd := rukpakv1alpha1.BundleDeployment{
2539
ObjectMeta: metav1.ObjectMeta{
2640
Name: name,
2741
},
@@ -39,6 +53,21 @@ func bundleDeployment(name, image string) rukpakv1alpha1.BundleDeployment {
3953
},
4054
},
4155
}
56+
57+
if owner != nil {
58+
bd.SetOwnerReferences([]metav1.OwnerReference{
59+
{
60+
APIVersion: operatorsv1alpha1.GroupVersion.String(),
61+
Kind: "Operator",
62+
Name: owner.Name,
63+
UID: owner.UID,
64+
Controller: pointer.Bool(true),
65+
BlockOwnerDeletion: pointer.Bool(true),
66+
},
67+
})
68+
}
69+
70+
return bd
4271
}
4372

4473
var _ = Describe("BundleDeploymentVariableSource", func() {
@@ -102,11 +131,13 @@ var _ = Describe("BundleDeploymentVariableSource", func() {
102131
})
103132

104133
It("should produce RequiredPackage variables", func() {
134+
fakeOperator := fakeOperator("test-operator", "test-prometheus", operatorsv1alpha1.UpgradeConstraintPolicyEnforce)
135+
operators := []operatorsv1alpha1.Operator{fakeOperator}
105136
bundleDeployments := []rukpakv1alpha1.BundleDeployment{
106-
bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"),
137+
bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", &fakeOperator),
107138
}
108139

109-
bdVariableSource := variablesources.NewBundleDeploymentVariableSource(bundleDeployments, testBundleList, &MockRequiredPackageSource{})
140+
bdVariableSource := variablesources.NewBundleDeploymentVariableSource(operators, bundleDeployments, testBundleList, &MockRequiredPackageSource{})
110141
variables, err := bdVariableSource.GetVariables(context.Background())
111142
Expect(err).ToNot(HaveOccurred())
112143

@@ -125,11 +156,13 @@ var _ = Describe("BundleDeploymentVariableSource", func() {
125156
})))
126157
})
127158
It("should return an error if the bundleDeployment image doesn't match any operator resource", func() {
159+
fakeOperator := fakeOperator("test-operator", "test-prometheus", operatorsv1alpha1.UpgradeConstraintPolicyEnforce)
160+
operators := []operatorsv1alpha1.Operator{fakeOperator}
128161
bundleDeployments := []rukpakv1alpha1.BundleDeployment{
129-
bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:nonexistent"),
162+
bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:nonexistent", &fakeOperator),
130163
}
131164

132-
bdVariableSource := variablesources.NewBundleDeploymentVariableSource(bundleDeployments, testBundleList, &MockRequiredPackageSource{})
165+
bdVariableSource := variablesources.NewBundleDeploymentVariableSource(operators, bundleDeployments, testBundleList, &MockRequiredPackageSource{})
133166
_, err := bdVariableSource.GetVariables(context.Background())
134167
Expect(err.Error()).To(Equal("bundleImage \"quay.io/operatorhubio/prometheus@sha256:nonexistent\" not found"))
135168
})

internal/resolution/variablesources/installed_package.go

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@ import (
55
"sort"
66

77
mmsemver "github.com/Masterminds/semver/v3"
8+
"k8s.io/apimachinery/pkg/types"
89
"k8s.io/apimachinery/pkg/util/sets"
910

1011
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
1112

13+
operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
1214
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
1315
catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
1416
catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
@@ -22,16 +24,21 @@ import (
2224
// has own variable.
2325
func MakeInstalledPackageVariables(
2426
allBundles []*catalogmetadata.Bundle,
27+
operators []operatorsv1alpha1.Operator,
2528
bundleDeployments []rukpakv1alpha1.BundleDeployment,
2629
) ([]*olmvariables.InstalledPackageVariable, error) {
27-
var successors successorsFunc = legacySemanticsSuccessors
28-
if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) {
29-
successors = semverSuccessors
30-
}
30+
ownerIDToBundleDeployment := mapOwnerIDToBundleDeployment(bundleDeployments)
3131

32-
result := make([]*olmvariables.InstalledPackageVariable, 0, len(bundleDeployments))
32+
result := make([]*olmvariables.InstalledPackageVariable, 0, len(operators))
3333
processed := sets.Set[string]{}
34-
for _, bundleDeployment := range bundleDeployments {
34+
for _, operator := range operators {
35+
bundleDeployment, ok := ownerIDToBundleDeployment[operator.UID]
36+
if !ok {
37+
// This can happen when an Operator is requested,
38+
// but not yet installed (e.g. no BundleDeployment created for it)
39+
continue
40+
}
41+
3542
if bundleDeployment.Spec.Template == nil {
3643
continue
3744
}
@@ -58,6 +65,7 @@ func MakeInstalledPackageVariables(
5865
})
5966
installedBundle := resultSet[0]
6067

68+
successors := successorsFuncForOperator(operator)
6169
upgradeEdges, err := successors(allBundles, installedBundle)
6270
if err != nil {
6371
return nil, err
@@ -76,6 +84,18 @@ func MakeInstalledPackageVariables(
7684
// Must not return installed bundle as a successor
7785
type successorsFunc func(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error)
7886

87+
func successorsFuncForOperator(operator operatorsv1alpha1.Operator) successorsFunc {
88+
if operator.Spec.UpgradeConstraintPolicy == operatorsv1alpha1.UpgradeConstraintPolicyIgnore {
89+
return ignoreConstraints
90+
}
91+
92+
if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) {
93+
return semverSuccessors
94+
}
95+
96+
return legacySemanticsSuccessors
97+
}
98+
7999
// legacySemanticsSuccessors returns successors based on legacy OLMv0 semantics
80100
// which rely on Replaces, Skips and skipRange.
81101
func legacySemanticsSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
@@ -119,3 +139,39 @@ func semverSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *cat
119139

120140
return upgradeEdges, nil
121141
}
142+
143+
// ignoreConstraints returns all available bundles for a package
144+
// as successors of the currently installed bundle. This allows downgrades.
145+
func ignoreConstraints(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
146+
currentVersion, err := installedBundle.Version()
147+
if err != nil {
148+
return nil, err
149+
}
150+
151+
excludeSelfConstraint, err := mmsemver.NewConstraint(fmt.Sprintf("!= %s", currentVersion.String()))
152+
if err != nil {
153+
return nil, err
154+
}
155+
156+
upgradeEdges := catalogfilter.Filter(allBundles, catalogfilter.And(
157+
catalogfilter.WithPackageName(installedBundle.Package),
158+
catalogfilter.InMastermindsSemverRange(excludeSelfConstraint),
159+
))
160+
sort.SliceStable(upgradeEdges, func(i, j int) bool {
161+
return catalogsort.ByVersion(upgradeEdges[i], upgradeEdges[j])
162+
})
163+
164+
return upgradeEdges, nil
165+
}
166+
167+
func mapOwnerIDToBundleDeployment(bundleDeployments []rukpakv1alpha1.BundleDeployment) map[types.UID]*rukpakv1alpha1.BundleDeployment {
168+
result := map[types.UID]*rukpakv1alpha1.BundleDeployment{}
169+
170+
for idx := range bundleDeployments {
171+
for _, ref := range bundleDeployments[idx].OwnerReferences {
172+
result[ref.UID] = &bundleDeployments[idx]
173+
}
174+
}
175+
176+
return result
177+
}

0 commit comments

Comments
 (0)