Skip to content

Commit

Permalink
Respect .spec.upgradeConstraintPolicy
Browse files Browse the repository at this point in the history
This commit makes OLM respect `.spec.upgradeConstraintPolicy`
set on an `Operator` CR when chosing upgrade edges.

Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
  • Loading branch information
m1kola committed Nov 6, 2023
1 parent cd784a0 commit f1d4d8e
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 22 deletions.
2 changes: 1 addition & 1 deletion internal/controllers/variable_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (v *VariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, er
return variablesources.NewOperatorVariableSource(operatorList.Items, allBundles, inputVariableSource), nil
},
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
return variablesources.NewBundleDeploymentVariableSource(bundleDeploymentList.Items, allBundles, inputVariableSource), nil
return variablesources.NewBundleDeploymentVariableSource(operatorList.Items, bundleDeploymentList.Items, allBundles, inputVariableSource), nil
},
func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
return variablesources.NewBundlesAndDepsVariableSource(allBundles, inputVariableSource), nil
Expand Down
7 changes: 5 additions & 2 deletions internal/resolution/variablesources/bundle_deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,22 @@ import (
"github.com/operator-framework/deppy/pkg/deppy/input"
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"

operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
)

var _ input.VariableSource = &BundleDeploymentVariableSource{}

type BundleDeploymentVariableSource struct {
operators []operatorsv1alpha1.Operator
bundleDeployments []rukpakv1alpha1.BundleDeployment
allBundles []*catalogmetadata.Bundle
inputVariableSource input.VariableSource
}

func NewBundleDeploymentVariableSource(bundleDeployments []rukpakv1alpha1.BundleDeployment, allBundles []*catalogmetadata.Bundle, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource {
func NewBundleDeploymentVariableSource(operators []operatorsv1alpha1.Operator, bundleDeployments []rukpakv1alpha1.BundleDeployment, allBundles []*catalogmetadata.Bundle, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource {
return &BundleDeploymentVariableSource{
operators: operators,
bundleDeployments: bundleDeployments,
allBundles: allBundles,
inputVariableSource: inputVariableSource,
Expand All @@ -37,7 +40,7 @@ func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context) ([]de
return nil, err
}

installedPackages, err := MakeInstalledPackageVariables(o.allBundles, o.bundleDeployments)
installedPackages, err := MakeInstalledPackageVariables(o.allBundles, o.operators, o.bundleDeployments)
if err != nil {
return nil, err
}
Expand Down
45 changes: 39 additions & 6 deletions internal/resolution/variablesources/bundle_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,32 @@ import (
"github.com/operator-framework/operator-registry/alpha/declcfg"
"github.com/operator-framework/operator-registry/alpha/property"

operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"

"github.com/operator-framework/deppy/pkg/deppy"
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
)

func bundleDeployment(name, image string) rukpakv1alpha1.BundleDeployment {
return rukpakv1alpha1.BundleDeployment{
func fakeOperator(name, packageName string, upgradeConstraintPolicy operatorsv1alpha1.UpgradeConstraintPolicy) operatorsv1alpha1.Operator {
return operatorsv1alpha1.Operator{
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Spec: operatorsv1alpha1.OperatorSpec{
PackageName: packageName,
UpgradeConstraintPolicy: upgradeConstraintPolicy,
},
}
}

func bundleDeployment(name, image string, owner *operatorsv1alpha1.Operator) rukpakv1alpha1.BundleDeployment {
bd := rukpakv1alpha1.BundleDeployment{
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Expand All @@ -39,6 +53,21 @@ func bundleDeployment(name, image string) rukpakv1alpha1.BundleDeployment {
},
},
}

if owner != nil {
bd.SetOwnerReferences([]metav1.OwnerReference{
{
APIVersion: operatorsv1alpha1.GroupVersion.String(),
Kind: "Operator",
Name: owner.Name,
UID: owner.UID,
Controller: pointer.Bool(true),
BlockOwnerDeletion: pointer.Bool(true),
},
})
}

return bd
}

var _ = Describe("BundleDeploymentVariableSource", func() {
Expand Down Expand Up @@ -102,11 +131,13 @@ var _ = Describe("BundleDeploymentVariableSource", func() {
})

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

bdVariableSource := variablesources.NewBundleDeploymentVariableSource(bundleDeployments, testBundleList, &MockRequiredPackageSource{})
bdVariableSource := variablesources.NewBundleDeploymentVariableSource(operators, bundleDeployments, testBundleList, &MockRequiredPackageSource{})
variables, err := bdVariableSource.GetVariables(context.Background())
Expect(err).ToNot(HaveOccurred())

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

bdVariableSource := variablesources.NewBundleDeploymentVariableSource(bundleDeployments, testBundleList, &MockRequiredPackageSource{})
bdVariableSource := variablesources.NewBundleDeploymentVariableSource(operators, bundleDeployments, testBundleList, &MockRequiredPackageSource{})
_, err := bdVariableSource.GetVariables(context.Background())
Expect(err.Error()).To(Equal("bundleImage \"quay.io/operatorhubio/prometheus@sha256:nonexistent\" not found"))
})
Expand Down
68 changes: 62 additions & 6 deletions internal/resolution/variablesources/installed_package.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import (
"sort"

mmsemver "github.com/Masterminds/semver/v3"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"

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

operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
"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"
Expand All @@ -22,16 +24,21 @@ import (
// has own variable.
func MakeInstalledPackageVariables(
allBundles []*catalogmetadata.Bundle,
operators []operatorsv1alpha1.Operator,
bundleDeployments []rukpakv1alpha1.BundleDeployment,
) ([]*olmvariables.InstalledPackageVariable, error) {
var successors successorsFunc = legacySemanticsSuccessors
if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) {
successors = semverSuccessors
}
ownerIDToBundleDeployment := mapOwnerIDToBundleDeployment(bundleDeployments)

result := make([]*olmvariables.InstalledPackageVariable, 0, len(bundleDeployments))
result := make([]*olmvariables.InstalledPackageVariable, 0, len(operators))
processed := sets.Set[string]{}
for _, bundleDeployment := range bundleDeployments {
for _, operator := range operators {
bundleDeployment, ok := ownerIDToBundleDeployment[operator.UID]
if !ok {
// This can happen when an Operator is requested,
// but not yet installed (e.g. no BundleDeployment created for it)
continue
}

if bundleDeployment.Spec.Template == nil {
continue
}
Expand All @@ -58,6 +65,7 @@ func MakeInstalledPackageVariables(
})
installedBundle := resultSet[0]

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

func successorsFuncForOperator(operator operatorsv1alpha1.Operator) successorsFunc {
if operator.Spec.UpgradeConstraintPolicy == operatorsv1alpha1.UpgradeConstraintPolicyIgnore {
return ignoreConstraints
}

if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) {
return semverSuccessors
}

return legacySemanticsSuccessors
}

// legacySemanticsSuccessors returns successors based on legacy OLMv0 semantics
// which rely on Replaces, Skips and skipRange.
func legacySemanticsSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
Expand Down Expand Up @@ -119,3 +139,39 @@ func semverSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *cat

return upgradeEdges, nil
}

// ignoreConstraints returns all available bundles for a package
// as successors of the currently installed bundle. This allows downgrades.
func ignoreConstraints(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
currentVersion, err := installedBundle.Version()
if err != nil {
return nil, err
}

excludeSelfConstraint, err := mmsemver.NewConstraint(fmt.Sprintf("!= %s", currentVersion.String()))
if err != nil {
return nil, err
}

upgradeEdges := catalogfilter.Filter(allBundles, catalogfilter.And(
catalogfilter.WithPackageName(installedBundle.Package),
catalogfilter.InMastermindsSemverRange(excludeSelfConstraint),
))
sort.SliceStable(upgradeEdges, func(i, j int) bool {
return catalogsort.ByVersion(upgradeEdges[i], upgradeEdges[j])
})

return upgradeEdges, nil
}

func mapOwnerIDToBundleDeployment(bundleDeployments []rukpakv1alpha1.BundleDeployment) map[types.UID]*rukpakv1alpha1.BundleDeployment {
result := map[types.UID]*rukpakv1alpha1.BundleDeployment{}

for idx := range bundleDeployments {
for _, ref := range bundleDeployments[idx].OwnerReferences {
result[ref.UID] = &bundleDeployments[idx]
}
}

return result
}
Loading

0 comments on commit f1d4d8e

Please sign in to comment.