From 4e7655caca6d38ce4f913e67e6b9e6ce6ae381a6 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Tue, 31 Oct 2023 14:53:16 +0000 Subject: [PATCH] Move bundle uniqueness code into a func Signed-off-by: Mikalai Radchuk --- .../variablesources/bundle_uniqueness.go | 74 ++++++++------ .../variablesources/bundle_uniqueness_test.go | 97 +++++++++++++++++++ 2 files changed, 142 insertions(+), 29 deletions(-) diff --git a/internal/resolution/variablesources/bundle_uniqueness.go b/internal/resolution/variablesources/bundle_uniqueness.go index 39084b36c..1247c0c2a 100644 --- a/internal/resolution/variablesources/bundle_uniqueness.go +++ b/internal/resolution/variablesources/bundle_uniqueness.go @@ -4,15 +4,53 @@ import ( "context" "fmt" + "k8s.io/apimachinery/pkg/util/sets" + "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" - "k8s.io/apimachinery/pkg/util/sets" "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) -var _ input.VariableSource = &CRDUniquenessConstraintsVariableSource{} +// MakeBundleUniquenessVariables produces variables that constrain +// the solution to at most 1 bundle per package. +// These variables guarantee that no two versions of +// the same package are running at the same time. +func MakeBundleUniquenessVariables(bundleVariables []*olmvariables.BundleVariable) []*olmvariables.BundleUniquenessVariable { + result := []*olmvariables.BundleUniquenessVariable{} + + bundleIDs := sets.Set[deppy.Identifier]{} + packageOrder := []string{} + bundleOrder := map[string][]deppy.Identifier{} + for _, bundleVariable := range bundleVariables { + bundles := make([]*catalogmetadata.Bundle, 0, 1+len(bundleVariable.Dependencies())) + bundles = append(bundles, bundleVariable.Bundle()) + bundles = append(bundles, bundleVariable.Dependencies()...) + for _, bundle := range bundles { + id := olmvariables.BundleVariableID(bundle) + // get bundleID package and update map + packageName := bundle.Package + + if _, ok := bundleOrder[packageName]; !ok { + packageOrder = append(packageOrder, packageName) + } + + if !bundleIDs.Has(id) { + bundleIDs.Insert(id) + bundleOrder[packageName] = append(bundleOrder[packageName], id) + } + } + } + + // create global constraint variables + for _, packageName := range packageOrder { + varID := deppy.IdentifierFromString(fmt.Sprintf("%s package uniqueness", packageName)) + result = append(result, olmvariables.NewBundleUniquenessVariable(varID, bundleOrder[packageName]...)) + } + + return result +} // CRDUniquenessConstraintsVariableSource produces variables that constraint the solution to // 1. at most 1 bundle per package @@ -41,39 +79,17 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex return nil, err } - // todo(perdasilva): better handle cases where a provided gvk is not found - // not all packages will necessarily export a CRD - - bundleIDs := sets.Set[deppy.Identifier]{} - packageOrder := []string{} - bundleOrder := map[string][]deppy.Identifier{} + bundleVariables := []*olmvariables.BundleVariable{} for _, variable := range variables { switch v := variable.(type) { case *olmvariables.BundleVariable: - bundles := []*catalogmetadata.Bundle{v.Bundle()} - bundles = append(bundles, v.Dependencies()...) - for _, bundle := range bundles { - id := olmvariables.BundleVariableID(bundle) - // get bundleID package and update map - packageName := bundle.Package - - if _, ok := bundleOrder[packageName]; !ok { - packageOrder = append(packageOrder, packageName) - } - - if !bundleIDs.Has(id) { - bundleIDs.Insert(id) - bundleOrder[packageName] = append(bundleOrder[packageName], id) - } - } + bundleVariables = append(bundleVariables, v) } } - // create global constraint variables - for _, packageName := range packageOrder { - varID := deppy.IdentifierFromString(fmt.Sprintf("%s package uniqueness", packageName)) - variables = append(variables, olmvariables.NewBundleUniquenessVariable(varID, bundleOrder[packageName]...)) + bundleUniqueness := MakeBundleUniquenessVariables(bundleVariables) + for _, v := range bundleUniqueness { + variables = append(variables, v) } - return variables, nil } diff --git a/internal/resolution/variablesources/bundle_uniqueness_test.go b/internal/resolution/variablesources/bundle_uniqueness_test.go index b0e054512..6b63f50b1 100644 --- a/internal/resolution/variablesources/bundle_uniqueness_test.go +++ b/internal/resolution/variablesources/bundle_uniqueness_test.go @@ -4,11 +4,16 @@ import ( "context" "encoding/json" "fmt" + "testing" + "github.com/google/go-cmp/cmp" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/stretchr/testify/require" "github.com/operator-framework/deppy/pkg/deppy" + "github.com/operator-framework/deppy/pkg/deppy/constraint" + "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" @@ -17,6 +22,98 @@ import ( "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) +func TestMakeBundleUniquenessVariables(t *testing.T) { + const fakeCatalogName = "fake-catalog" + channel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}} + bundleSet := map[string]*catalogmetadata.Bundle{ + "test-package.v1.0.0": { + Bundle: declcfg.Bundle{ + Name: "test-package.v1.0.0", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}`)}, + }, + }, + CatalogName: fakeCatalogName, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "test-package.v1.0.1": { + Bundle: declcfg.Bundle{ + Name: "test-package.v1.0.1", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.1"}`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}`)}, + }, + }, + CatalogName: fakeCatalogName, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + + "some-package.v1.0.0": { + Bundle: declcfg.Bundle{ + Name: "some-package.v1.0.0", + Package: "some-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-package", "version": "1.0.0"}`)}, + }, + }, + CatalogName: fakeCatalogName, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + } + + // Input into the testable function must include more than one bundle + // from the same package to ensure that the function + // enforces uniqueness correctly. + // We also need at least one bundle variable to have a dependency + // on another package. This will help to make sure that the function + // also creates uniqueness variables for dependencies. + bundleVariables := []*olmvariables.BundleVariable{ + olmvariables.NewBundleVariable( + bundleSet["test-package.v1.0.0"], + []*catalogmetadata.Bundle{ + bundleSet["some-package.v1.0.0"], + }, + ), + olmvariables.NewBundleVariable( + bundleSet["test-package.v1.0.1"], + []*catalogmetadata.Bundle{ + bundleSet["some-package.v1.0.0"], + }, + ), + } + + variables := variablesources.MakeBundleUniquenessVariables(bundleVariables) + + // Each package in the input must have own uniqueness variable. + // Each variable expected to have one constraint enforcing at most one + // of the involved bundles to be allowed in the solution + expectedVariables := []*olmvariables.BundleUniquenessVariable{ + { + SimpleVariable: input.NewSimpleVariable( + "test-package package uniqueness", + constraint.AtMost( + 1, + deppy.Identifier("fake-catalog-test-package-test-package.v1.0.0"), + deppy.Identifier("fake-catalog-test-package-test-package.v1.0.1"), + ), + ), + }, + { + SimpleVariable: input.NewSimpleVariable( + "some-package package uniqueness", + constraint.AtMost( + 1, + deppy.Identifier("fake-catalog-some-package-some-package.v1.0.0"), + ), + ), + }, + } + require.Empty(t, cmp.Diff(variables, expectedVariables, cmp.AllowUnexported(input.SimpleVariable{}, constraint.AtMostConstraint{}))) +} + var channel = catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}} var bundleSet = map[string]*catalogmetadata.Bundle{ // required package bundles