Skip to content

Commit 5b9e713

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 5b9e713

File tree

5 files changed

+186
-22
lines changed

5 files changed

+186
-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+
}

‎internal/resolution/variablesources/installed_package_test.go‎

Lines changed: 79 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ import (
1212
"github.com/stretchr/testify/require"
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1414
featuregatetesting "k8s.io/component-base/featuregate/testing"
15+
"k8s.io/utils/pointer"
1516

17+
operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
1618
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
1719
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
1820
"github.com/operator-framework/operator-controller/pkg/features"
@@ -201,8 +203,20 @@ func TestMakeInstalledPackageVariables(t *testing.T) {
201203
},
202204
}
203205

204-
fakeBundleDeployment := func(name, bundleImage string) rukpakv1alpha1.BundleDeployment {
205-
return rukpakv1alpha1.BundleDeployment{
206+
fakeOperator := func(name, packageName string, upgradeConstraintPolicy operatorsv1alpha1.UpgradeConstraintPolicy) operatorsv1alpha1.Operator {
207+
return operatorsv1alpha1.Operator{
208+
ObjectMeta: metav1.ObjectMeta{
209+
Name: name,
210+
},
211+
Spec: operatorsv1alpha1.OperatorSpec{
212+
PackageName: packageName,
213+
UpgradeConstraintPolicy: upgradeConstraintPolicy,
214+
},
215+
}
216+
}
217+
218+
fakeBundleDeployment := func(name, bundleImage string, owner *operatorsv1alpha1.Operator) rukpakv1alpha1.BundleDeployment {
219+
bd := rukpakv1alpha1.BundleDeployment{
206220
ObjectMeta: metav1.ObjectMeta{
207221
Name: name,
208222
},
@@ -218,17 +232,34 @@ func TestMakeInstalledPackageVariables(t *testing.T) {
218232
},
219233
},
220234
}
235+
236+
if owner != nil {
237+
bd.SetOwnerReferences([]metav1.OwnerReference{
238+
{
239+
APIVersion: operatorsv1alpha1.GroupVersion.String(),
240+
Kind: "Operator",
241+
Name: owner.Name,
242+
UID: owner.UID,
243+
Controller: pointer.Bool(true),
244+
BlockOwnerDeletion: pointer.Bool(true),
245+
},
246+
})
247+
}
248+
249+
return bd
221250
}
222251

223252
t.Run("with ForceSemverUpgradeConstraints feature gate enabled", func(t *testing.T) {
224253
defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)()
225254

226255
t.Run("with non-zero major version", func(t *testing.T) {
227256
const bundleImage = "registry.io/repo/test-package@v2.0.0"
257+
fakeOperator := fakeOperator("test-operator", "test-package", operatorsv1alpha1.UpgradeConstraintPolicyEnforce)
228258
installedPackages, err := variablesources.MakeInstalledPackageVariables(
229259
allBundles,
260+
[]operatorsv1alpha1.Operator{fakeOperator},
230261
[]rukpakv1alpha1.BundleDeployment{
231-
fakeBundleDeployment("test-bd", bundleImage),
262+
fakeBundleDeployment("test-package-bd", bundleImage, &fakeOperator),
232263
},
233264
)
234265
require.NoError(t, err)
@@ -248,10 +279,12 @@ func TestMakeInstalledPackageVariables(t *testing.T) {
248279
t.Run("with zero major version", func(t *testing.T) {
249280
t.Run("with zero minor version", func(t *testing.T) {
250281
const bundleImage = "registry.io/repo/test-package@v0.0.1"
282+
fakeOperator := fakeOperator("test-operator", "test-package", operatorsv1alpha1.UpgradeConstraintPolicyEnforce)
251283
installedPackages, err := variablesources.MakeInstalledPackageVariables(
252284
allBundles,
285+
[]operatorsv1alpha1.Operator{fakeOperator},
253286
[]rukpakv1alpha1.BundleDeployment{
254-
fakeBundleDeployment("test-bd", bundleImage),
287+
fakeBundleDeployment("test-package-bd", bundleImage, &fakeOperator),
255288
},
256289
)
257290
require.NoError(t, err)
@@ -268,10 +301,12 @@ func TestMakeInstalledPackageVariables(t *testing.T) {
268301

269302
t.Run("with non-zero minor version", func(t *testing.T) {
270303
const bundleImage = "registry.io/repo/test-package@v0.1.0"
304+
fakeOperator := fakeOperator("test-operator", "test-package", operatorsv1alpha1.UpgradeConstraintPolicyEnforce)
271305
installedPackages, err := variablesources.MakeInstalledPackageVariables(
272306
allBundles,
307+
[]operatorsv1alpha1.Operator{fakeOperator},
273308
[]rukpakv1alpha1.BundleDeployment{
274-
fakeBundleDeployment("test-bd", bundleImage),
309+
fakeBundleDeployment("test-package-bd", bundleImage, &fakeOperator),
275310
},
276311
)
277312
require.NoError(t, err)
@@ -293,10 +328,12 @@ func TestMakeInstalledPackageVariables(t *testing.T) {
293328
defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)()
294329

295330
const bundleImage = "registry.io/repo/test-package@v2.0.0"
331+
fakeOperator := fakeOperator("test-operator", "test-package", operatorsv1alpha1.UpgradeConstraintPolicyEnforce)
296332
installedPackages, err := variablesources.MakeInstalledPackageVariables(
297333
allBundles,
334+
[]operatorsv1alpha1.Operator{fakeOperator},
298335
[]rukpakv1alpha1.BundleDeployment{
299-
fakeBundleDeployment("test-bd", bundleImage),
336+
fakeBundleDeployment("test-package-bd", bundleImage, &fakeOperator),
300337
},
301338
)
302339
require.NoError(t, err)
@@ -312,12 +349,47 @@ func TestMakeInstalledPackageVariables(t *testing.T) {
312349
assert.Equal(t, "test-package.v2.0.0", packageVariable.Bundles()[1].Name)
313350
})
314351

352+
t.Run("UpgradeConstraintPolicy is set to Ignore", func(t *testing.T) {
353+
const bundleImage = "registry.io/repo/test-package@v2.0.0"
354+
fakeOperator := fakeOperator("test-operator", "test-package", operatorsv1alpha1.UpgradeConstraintPolicyIgnore)
355+
installedPackages, err := variablesources.MakeInstalledPackageVariables(
356+
allBundles,
357+
[]operatorsv1alpha1.Operator{fakeOperator},
358+
[]rukpakv1alpha1.BundleDeployment{
359+
fakeBundleDeployment("test-package-bd", bundleImage, &fakeOperator),
360+
},
361+
)
362+
require.NoError(t, err)
363+
364+
require.Len(t, installedPackages, 1)
365+
packageVariable := installedPackages[0]
366+
assert.Equal(t, deppy.IdentifierFromString("installed package test-package"), packageVariable.Identifier())
367+
368+
// ensure bundles are in version order (high to low)
369+
bundles := packageVariable.Bundles()
370+
require.Len(t, bundles, 12)
371+
assert.Equal(t, "test-package.v5.0.0", bundles[0].Name)
372+
assert.Equal(t, "test-package.v4.0.0", bundles[1].Name)
373+
assert.Equal(t, "test-package.v3.0.0", bundles[2].Name)
374+
assert.Equal(t, "test-package.v2.2.0", bundles[3].Name)
375+
assert.Equal(t, "test-package.v2.1.0", bundles[4].Name)
376+
assert.Equal(t, "test-package.v1.0.0", bundles[5].Name)
377+
assert.Equal(t, "test-package.v0.2.0", bundles[6].Name)
378+
assert.Equal(t, "test-package.v0.1.1", bundles[7].Name)
379+
assert.Equal(t, "test-package.v0.1.0", bundles[8].Name)
380+
assert.Equal(t, "test-package.v0.0.2", bundles[9].Name)
381+
assert.Equal(t, "test-package.v0.0.1", bundles[10].Name)
382+
assert.Equal(t, "test-package.v2.0.0", bundles[11].Name)
383+
})
384+
315385
t.Run("installed bundle not found", func(t *testing.T) {
316386
const bundleImage = "registry.io/repo/test-package@v9.0.0"
387+
fakeOperator := fakeOperator("test-operator", "test-package", operatorsv1alpha1.UpgradeConstraintPolicyEnforce)
317388
installedPackages, err := variablesources.MakeInstalledPackageVariables(
318389
allBundles,
390+
[]operatorsv1alpha1.Operator{fakeOperator},
319391
[]rukpakv1alpha1.BundleDeployment{
320-
fakeBundleDeployment("test-bd", bundleImage),
392+
fakeBundleDeployment("test-package-bd", bundleImage, &fakeOperator),
321393
},
322394
)
323395
assert.Nil(t, installedPackages)

0 commit comments

Comments
 (0)