From 2f7f1480c00b0e6b0bf6833ceff08359f6b149cd Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Mon, 26 Jun 2023 14:00:10 +0100 Subject: [PATCH] Split variables and variable sources Signed-off-by: Mikalai Radchuk --- internal/controllers/operator_controller.go | 4 +- internal/resolution/variables/bundle.go | 58 +++++++++++++++ internal/resolution/variables/bundle_test.go | 70 +++++++++++++++++++ .../resolution/variables/required_package.go | 34 +++++++++ .../variables/required_package_test.go | 55 +++++++++++++++ .../resolution/variables/variables_test.go | 13 ++++ .../bundles_and_dependencies.go | 36 +--------- .../bundles_and_dependencies_test.go | 49 +++---------- .../variablesources/crd_constraints.go | 23 ++---- .../variablesources/crd_constraints_test.go | 61 ++++++---------- .../variablesources/operator_test.go | 17 ++--- .../variablesources/required_package.go | 25 +------ .../variablesources/required_package_test.go | 45 +----------- 13 files changed, 281 insertions(+), 209 deletions(-) create mode 100644 internal/resolution/variables/bundle.go create mode 100644 internal/resolution/variables/bundle_test.go create mode 100644 internal/resolution/variables/required_package.go create mode 100644 internal/resolution/variables/required_package_test.go create mode 100644 internal/resolution/variables/variables_test.go diff --git a/internal/controllers/operator_controller.go b/internal/controllers/operator_controller.go index d567a6660..a842b8d20 100644 --- a/internal/controllers/operator_controller.go +++ b/internal/controllers/operator_controller.go @@ -42,7 +42,7 @@ import ( operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/controllers/validators" "github.com/operator-framework/operator-controller/internal/resolution/entities" - "github.com/operator-framework/operator-controller/internal/resolution/variablesources" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) // OperatorReconciler reconciles a Operator object @@ -247,7 +247,7 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*entities.BundleEntity, error) { for _, variable := range solution.SelectedVariables() { switch v := variable.(type) { - case *variablesources.BundleVariable: + case *olmvariables.BundleVariable: entityPkgName, err := v.BundleEntity().PackageName() if err != nil { return nil, err diff --git a/internal/resolution/variables/bundle.go b/internal/resolution/variables/bundle.go new file mode 100644 index 000000000..dce8bed50 --- /dev/null +++ b/internal/resolution/variables/bundle.go @@ -0,0 +1,58 @@ +package variables + +import ( + "github.com/operator-framework/deppy/pkg/deppy" + "github.com/operator-framework/deppy/pkg/deppy/constraint" + "github.com/operator-framework/deppy/pkg/deppy/input" + + olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" +) + +var _ deppy.Variable = &BundleVariable{} + +type BundleVariable struct { + *input.SimpleVariable + bundleEntity *olmentity.BundleEntity + dependencies []*olmentity.BundleEntity +} + +func (b *BundleVariable) BundleEntity() *olmentity.BundleEntity { + return b.bundleEntity +} + +func (b *BundleVariable) Dependencies() []*olmentity.BundleEntity { + return b.dependencies +} + +func NewBundleVariable(bundleEntity *olmentity.BundleEntity, dependencyBundleEntities []*olmentity.BundleEntity) *BundleVariable { + dependencyIDs := make([]deppy.Identifier, 0, len(dependencyBundleEntities)) + for _, bundle := range dependencyBundleEntities { + dependencyIDs = append(dependencyIDs, bundle.ID) + } + var constraints []deppy.Constraint + if len(dependencyIDs) > 0 { + constraints = append(constraints, constraint.Dependency(dependencyIDs...)) + } + return &BundleVariable{ + SimpleVariable: input.NewSimpleVariable(bundleEntity.ID, constraints...), + bundleEntity: bundleEntity, + dependencies: dependencyBundleEntities, + } +} + +var _ deppy.Variable = &BundleUniquenessVariable{} + +type BundleUniquenessVariable struct { + *input.SimpleVariable +} + +// NewBundleUniquenessVariable creates a new variable that instructs the resolver to choose at most a single bundle +// from the input 'atMostID'. Examples: +// 1. restrict the solution to at most a single bundle per package +// 2. restrict the solution to at most a single bundler per provided gvk +// this guarantees that no two operators provide the same gvk and no two version of the same operator are running at the same time +func NewBundleUniquenessVariable(id deppy.Identifier, atMostIDs ...deppy.Identifier) *BundleUniquenessVariable { + return &BundleUniquenessVariable{ + SimpleVariable: input.NewSimpleVariable(id, constraint.AtMost(1, atMostIDs...)), + } +} diff --git a/internal/resolution/variables/bundle_test.go b/internal/resolution/variables/bundle_test.go new file mode 100644 index 000000000..506625642 --- /dev/null +++ b/internal/resolution/variables/bundle_test.go @@ -0,0 +1,70 @@ +package variables_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "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/property" + + olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" +) + +var _ = Describe("BundleVariable", func() { + var ( + bv *olmvariables.BundleVariable + bundleEntity *olmentity.BundleEntity + dependencies []*olmentity.BundleEntity + ) + + BeforeEach(func() { + bundleEntity = olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{ + property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, + property.TypeChannel: `{"channelName":"stable","priority":0}`, + })) + dependencies = []*olmentity.BundleEntity{ + olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{ + property.TypePackage: `{"packageName": "test-package-2", "version": "2.0.0"}`, + property.TypeChannel: `{"channelName":"stable","priority":0}`, + })), + olmentity.NewBundleEntity(input.NewEntity("bundle-3", map[string]string{ + property.TypePackage: `{"packageName": "test-package-3", "version": "2.0.0"}`, + property.TypeChannel: `{"channelName":"stable","priority":0}`, + })), + } + bv = olmvariables.NewBundleVariable(bundleEntity, dependencies) + }) + + It("should return the correct bundle entity", func() { + Expect(bv.BundleEntity()).To(Equal(bundleEntity)) + }) + + It("should return the correct dependencies", func() { + Expect(bv.Dependencies()).To(Equal(dependencies)) + }) +}) + +var _ = Describe("BundleUniquenessVariable", func() { + var ( + id deppy.Identifier + atMostIDs []deppy.Identifier + globalConstraintVariable *olmvariables.BundleUniquenessVariable + ) + + BeforeEach(func() { + id = deppy.IdentifierFromString("test-id") + atMostIDs = []deppy.Identifier{ + deppy.IdentifierFromString("test-at-most-id-1"), + deppy.IdentifierFromString("test-at-most-id-2"), + } + globalConstraintVariable = olmvariables.NewBundleUniquenessVariable(id, atMostIDs...) + }) + + It("should initialize a new global constraint variable", func() { + Expect(globalConstraintVariable.Identifier()).To(Equal(id)) + Expect(globalConstraintVariable.Constraints()).To(Equal([]deppy.Constraint{constraint.AtMost(1, atMostIDs...)})) + }) +}) diff --git a/internal/resolution/variables/required_package.go b/internal/resolution/variables/required_package.go new file mode 100644 index 000000000..d2916f5f3 --- /dev/null +++ b/internal/resolution/variables/required_package.go @@ -0,0 +1,34 @@ +package variables + +import ( + "fmt" + + "github.com/operator-framework/deppy/pkg/deppy" + "github.com/operator-framework/deppy/pkg/deppy/constraint" + "github.com/operator-framework/deppy/pkg/deppy/input" + + olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" +) + +var _ deppy.Variable = &RequiredPackageVariable{} + +type RequiredPackageVariable struct { + *input.SimpleVariable + bundleEntities []*olmentity.BundleEntity +} + +func (r *RequiredPackageVariable) BundleEntities() []*olmentity.BundleEntity { + return r.bundleEntities +} + +func NewRequiredPackageVariable(packageName string, bundleEntities []*olmentity.BundleEntity) *RequiredPackageVariable { + id := deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)) + entityIDs := make([]deppy.Identifier, 0, len(bundleEntities)) + for _, bundle := range bundleEntities { + entityIDs = append(entityIDs, bundle.ID) + } + return &RequiredPackageVariable{ + SimpleVariable: input.NewSimpleVariable(id, constraint.Mandatory(), constraint.Dependency(entityIDs...)), + bundleEntities: bundleEntities, + } +} diff --git a/internal/resolution/variables/required_package_test.go b/internal/resolution/variables/required_package_test.go new file mode 100644 index 000000000..3065ba81a --- /dev/null +++ b/internal/resolution/variables/required_package_test.go @@ -0,0 +1,55 @@ +package variables_test + +import ( + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/operator-framework/deppy/pkg/deppy" + "github.com/operator-framework/deppy/pkg/deppy/input" + "github.com/operator-framework/operator-registry/alpha/property" + + olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" +) + +var _ = Describe("RequiredPackageVariable", func() { + var ( + rpv *olmvariables.RequiredPackageVariable + packageName string + bundleEntities []*olmentity.BundleEntity + ) + + BeforeEach(func() { + packageName = "test-package" + bundleEntities = []*olmentity.BundleEntity{ + olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{ + property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, + property.TypeChannel: `{"channelName":"stable","priority":0}`, + })), + olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{ + property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`, + property.TypeChannel: `{"channelName":"stable","priority":0}`, + })), + olmentity.NewBundleEntity(input.NewEntity("bundle-3", map[string]string{ + property.TypePackage: `{"packageName": "test-package", "version": "3.0.0"}`, + property.TypeChannel: `{"channelName":"stable","priority":0}`, + })), + } + rpv = olmvariables.NewRequiredPackageVariable(packageName, bundleEntities) + }) + + It("should return the correct package name", func() { + Expect(rpv.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)))) + }) + + It("should return the correct bundle entities", func() { + Expect(rpv.BundleEntities()).To(Equal(bundleEntities)) + }) + + It("should contain both mandatory and dependency constraints", func() { + // TODO: add this test once https://github.com/operator-framework/deppy/pull/85 gets merged + // then we'll be able to inspect constraint types + }) +}) diff --git a/internal/resolution/variables/variables_test.go b/internal/resolution/variables/variables_test.go new file mode 100644 index 000000000..e1b7a553d --- /dev/null +++ b/internal/resolution/variables/variables_test.go @@ -0,0 +1,13 @@ +package variables_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestVariableSources(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Variables Suite") +} diff --git a/internal/resolution/variablesources/bundles_and_dependencies.go b/internal/resolution/variablesources/bundles_and_dependencies.go index d4605472c..b37f9b2e4 100644 --- a/internal/resolution/variablesources/bundles_and_dependencies.go +++ b/internal/resolution/variablesources/bundles_and_dependencies.go @@ -7,44 +7,14 @@ import ( "github.com/blang/semver/v4" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/constraint" "github.com/operator-framework/deppy/pkg/deppy/input" olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" "github.com/operator-framework/operator-controller/internal/resolution/util/predicates" entitysort "github.com/operator-framework/operator-controller/internal/resolution/util/sort" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) -type BundleVariable struct { - *input.SimpleVariable - bundleEntity *olmentity.BundleEntity - dependencies []*olmentity.BundleEntity -} - -func (b *BundleVariable) BundleEntity() *olmentity.BundleEntity { - return b.bundleEntity -} - -func (b *BundleVariable) Dependencies() []*olmentity.BundleEntity { - return b.dependencies -} - -func NewBundleVariable(bundleEntity *olmentity.BundleEntity, dependencyBundleEntities []*olmentity.BundleEntity) *BundleVariable { - dependencyIDs := make([]deppy.Identifier, 0, len(dependencyBundleEntities)) - for _, bundle := range dependencyBundleEntities { - dependencyIDs = append(dependencyIDs, bundle.ID) - } - var constraints []deppy.Constraint - if len(dependencyIDs) > 0 { - constraints = append(constraints, constraint.Dependency(dependencyIDs...)) - } - return &BundleVariable{ - SimpleVariable: input.NewSimpleVariable(bundleEntity.ID, constraints...), - bundleEntity: bundleEntity, - dependencies: dependencyBundleEntities, - } -} - var _ input.VariableSource = &BundlesAndDepsVariableSource{} type BundlesAndDepsVariableSource struct { @@ -73,7 +43,7 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context, entityS var bundleEntityQueue []*olmentity.BundleEntity for _, variable := range variables { switch v := variable.(type) { - case *RequiredPackageVariable: + case *olmvariables.RequiredPackageVariable: bundleEntityQueue = append(bundleEntityQueue, v.BundleEntities()...) } } @@ -101,7 +71,7 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context, entityS bundleEntityQueue = append(bundleEntityQueue, dependencyEntityBundles...) // create variable - variables = append(variables, NewBundleVariable(head, dependencyEntityBundles)) + variables = append(variables, olmvariables.NewBundleVariable(head, dependencyEntityBundles)) } return variables, nil diff --git a/internal/resolution/variablesources/bundles_and_dependencies_test.go b/internal/resolution/variablesources/bundles_and_dependencies_test.go index e84d7c9dc..d4a110ba1 100644 --- a/internal/resolution/variablesources/bundles_and_dependencies_test.go +++ b/internal/resolution/variablesources/bundles_and_dependencies_test.go @@ -10,43 +10,10 @@ import ( "github.com/operator-framework/operator-registry/alpha/property" olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -var _ = Describe("BundleVariable", func() { - var ( - bv *variablesources.BundleVariable - bundleEntity *olmentity.BundleEntity - dependencies []*olmentity.BundleEntity - ) - - BeforeEach(func() { - bundleEntity = olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })) - dependencies = []*olmentity.BundleEntity{ - olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-3", map[string]string{ - property.TypePackage: `{"packageName": "test-package-3", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - } - bv = variablesources.NewBundleVariable(bundleEntity, dependencies) - }) - - It("should return the correct bundle entity", func() { - Expect(bv.BundleEntity()).To(Equal(bundleEntity)) - }) - - It("should return the correct dependencies", func() { - Expect(bv.Dependencies()).To(Equal(dependencies)) - }) -}) - var _ = Describe("BundlesAndDepsVariableSource", func() { var ( bdvs *variablesources.BundlesAndDepsVariableSource @@ -58,7 +25,7 @@ var _ = Describe("BundlesAndDepsVariableSource", func() { &MockRequiredPackageSource{ ResultSet: []deppy.Variable{ // must match data in mockEntitySource - variablesources.NewRequiredPackageVariable("test-package", []*olmentity.BundleEntity{ + olmvariables.NewRequiredPackageVariable("test-package", []*olmentity.BundleEntity{ olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{ property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`, property.TypeChannel: `{"channelName":"stable","priority":0}`, @@ -76,7 +43,7 @@ var _ = Describe("BundlesAndDepsVariableSource", func() { &MockRequiredPackageSource{ ResultSet: []deppy.Variable{ // must match data in mockEntitySource - variablesources.NewRequiredPackageVariable("test-package-2", []*olmentity.BundleEntity{ + olmvariables.NewRequiredPackageVariable("test-package-2", []*olmentity.BundleEntity{ // test-package-2 required package - no dependencies olmentity.NewBundleEntity(input.NewEntity("bundle-15", map[string]string{ property.TypePackage: `{"packageName": "test-package-2", "version": "1.5.0"}`, @@ -185,10 +152,10 @@ var _ = Describe("BundlesAndDepsVariableSource", func() { variables, err := bdvs.GetVariables(context.TODO(), mockEntitySource) Expect(err).NotTo(HaveOccurred()) - var bundleVariables []*variablesources.BundleVariable + var bundleVariables []*olmvariables.BundleVariable for _, variable := range variables { switch v := variable.(type) { - case *variablesources.BundleVariable: + case *olmvariables.BundleVariable: bundleVariables = append(bundleVariables, v) } } @@ -241,8 +208,8 @@ func (m *MockRequiredPackageSource) GetVariables(_ context.Context, _ input.Enti return m.ResultSet, nil } -func VariableWithID(id deppy.Identifier) func(vars []*variablesources.BundleVariable) *variablesources.BundleVariable { - return func(vars []*variablesources.BundleVariable) *variablesources.BundleVariable { +func VariableWithID(id deppy.Identifier) func(vars []*olmvariables.BundleVariable) *olmvariables.BundleVariable { + return func(vars []*olmvariables.BundleVariable) *olmvariables.BundleVariable { for i := 0; i < len(vars); i++ { if vars[i].Identifier() == id { return vars[i] @@ -252,7 +219,7 @@ func VariableWithID(id deppy.Identifier) func(vars []*variablesources.BundleVari } } -func CollectBundleVariableIDs(vars []*variablesources.BundleVariable) []string { +func CollectBundleVariableIDs(vars []*olmvariables.BundleVariable) []string { ids := make([]string, 0, len(vars)) for _, v := range vars { ids = append(ids, v.Identifier().String()) diff --git a/internal/resolution/variablesources/crd_constraints.go b/internal/resolution/variablesources/crd_constraints.go index 3520baa66..798fca211 100644 --- a/internal/resolution/variablesources/crd_constraints.go +++ b/internal/resolution/variablesources/crd_constraints.go @@ -5,27 +5,12 @@ import ( "fmt" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/constraint" "github.com/operator-framework/deppy/pkg/deppy/input" olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) -type BundleUniquenessVariable struct { - *input.SimpleVariable -} - -// NewBundleUniquenessVariable creates a new variable that instructs the resolver to choose at most a single bundle -// from the input 'atMostID'. Examples: -// 1. restrict the solution to at most a single bundle per package -// 2. restrict the solution to at most a single bundler per provided gvk -// this guarantees that no two operators provide the same gvk and no two version of the same operator are running at the same time -func NewBundleUniquenessVariable(id deppy.Identifier, atMostIDs ...deppy.Identifier) *BundleUniquenessVariable { - return &BundleUniquenessVariable{ - SimpleVariable: input.NewSimpleVariable(id, constraint.AtMost(1, atMostIDs...)), - } -} - var _ input.VariableSource = &CRDUniquenessConstraintsVariableSource{} // CRDUniquenessConstraintsVariableSource produces variables that constraint the solution to @@ -62,7 +47,7 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex gvkToBundleMap := map[string]map[deppy.Identifier]struct{}{} for _, variable := range variables { switch v := variable.(type) { - case *BundleVariable: + case *olmvariables.BundleVariable: bundleEntities := []*olmentity.BundleEntity{v.BundleEntity()} bundleEntities = append(bundleEntities, v.Dependencies()...) for _, bundleEntity := range bundleEntities { @@ -100,7 +85,7 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex bundleIDs = append(bundleIDs, bundleID) } varID := deppy.IdentifierFromString(fmt.Sprintf("%s package uniqueness", packageName)) - variables = append(variables, NewBundleUniquenessVariable(varID, bundleIDs...)) + variables = append(variables, olmvariables.NewBundleUniquenessVariable(varID, bundleIDs...)) } for gvk, bundleIDMap := range gvkToBundleMap { @@ -109,7 +94,7 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex bundleIDs = append(bundleIDs, bundleID) } varID := deppy.IdentifierFromString(fmt.Sprintf("%s gvk uniqueness", gvk)) - variables = append(variables, NewBundleUniquenessVariable(varID, bundleIDs...)) + variables = append(variables, olmvariables.NewBundleUniquenessVariable(varID, bundleIDs...)) } return variables, nil diff --git a/internal/resolution/variablesources/crd_constraints_test.go b/internal/resolution/variablesources/crd_constraints_test.go index 7292fc620..67ffef22c 100644 --- a/internal/resolution/variablesources/crd_constraints_test.go +++ b/internal/resolution/variablesources/crd_constraints_test.go @@ -6,37 +6,16 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "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/property" olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -var _ = Describe("BundleUniquenessVariable", func() { - var ( - id deppy.Identifier - atMostIDs []deppy.Identifier - globalConstraintVariable *variablesources.BundleUniquenessVariable - ) - - BeforeEach(func() { - id = deppy.IdentifierFromString("test-id") - atMostIDs = []deppy.Identifier{ - deppy.IdentifierFromString("test-at-most-id-1"), - deppy.IdentifierFromString("test-at-most-id-2"), - } - globalConstraintVariable = variablesources.NewBundleUniquenessVariable(id, atMostIDs...) - }) - - It("should initialize a new global constraint variable", func() { - Expect(globalConstraintVariable.Identifier()).To(Equal(id)) - Expect(globalConstraintVariable.Constraints()).To(Equal([]deppy.Constraint{constraint.AtMost(1, atMostIDs...)})) - }) -}) - var bundleSet = map[deppy.Identifier]*input.Entity{ // required package bundles "bundle-1": input.NewEntity("bundle-1", map[string]string{ @@ -153,16 +132,16 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { It("should get variables from the input variable source and create global constraint variables", func() { inputVariableSource.ResultSet = []deppy.Variable{ - variablesources.NewRequiredPackageVariable("test-package", []*olmentity.BundleEntity{ + olmvariables.NewRequiredPackageVariable("test-package", []*olmentity.BundleEntity{ olmentity.NewBundleEntity(bundleSet["bundle-2"]), olmentity.NewBundleEntity(bundleSet["bundle-1"]), }), - variablesources.NewRequiredPackageVariable("test-package-2", []*olmentity.BundleEntity{ + olmvariables.NewRequiredPackageVariable("test-package-2", []*olmentity.BundleEntity{ olmentity.NewBundleEntity(bundleSet["bundle-14"]), olmentity.NewBundleEntity(bundleSet["bundle-15"]), olmentity.NewBundleEntity(bundleSet["bundle-16"]), }), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-2"]), []*olmentity.BundleEntity{ olmentity.NewBundleEntity(bundleSet["bundle-3"]), @@ -172,7 +151,7 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { olmentity.NewBundleEntity(bundleSet["bundle-7"]), }, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-1"]), []*olmentity.BundleEntity{ olmentity.NewBundleEntity(bundleSet["bundle-6"]), @@ -180,23 +159,23 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { olmentity.NewBundleEntity(bundleSet["bundle-8"]), }, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-3"]), []*olmentity.BundleEntity{}, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-4"]), []*olmentity.BundleEntity{}, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-5"]), []*olmentity.BundleEntity{}, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-6"]), []*olmentity.BundleEntity{}, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-7"]), []*olmentity.BundleEntity{ olmentity.NewBundleEntity(bundleSet["bundle-8"]), @@ -204,27 +183,27 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { olmentity.NewBundleEntity(bundleSet["bundle-10"]), }, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-8"]), []*olmentity.BundleEntity{}, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-9"]), []*olmentity.BundleEntity{}, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-10"]), []*olmentity.BundleEntity{}, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-14"]), []*olmentity.BundleEntity{}, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-15"]), []*olmentity.BundleEntity{}, ), - variablesources.NewBundleVariable( + olmvariables.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-16"]), []*olmentity.BundleEntity{}, ), @@ -232,10 +211,10 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { variables, err := crdConstraintVariableSource.GetVariables(ctx, entitySource) Expect(err).ToNot(HaveOccurred()) Expect(variables).To(HaveLen(26)) - var crdConstraintVariables []*variablesources.BundleUniquenessVariable + var crdConstraintVariables []*olmvariables.BundleUniquenessVariable for _, variable := range variables { switch v := variable.(type) { - case *variablesources.BundleUniquenessVariable: + case *olmvariables.BundleUniquenessVariable: crdConstraintVariables = append(crdConstraintVariables, v) } } @@ -296,7 +275,7 @@ func (m *MockInputVariableSource) GetVariables(_ context.Context, _ input.Entity return m.ResultSet, nil } -func CollectGlobalConstraintVariableIDs(vars []*variablesources.BundleUniquenessVariable) []string { +func CollectGlobalConstraintVariableIDs(vars []*olmvariables.BundleUniquenessVariable) []string { ids := make([]string, 0, len(vars)) for _, v := range vars { ids = append(ids, v.Identifier().String()) diff --git a/internal/resolution/variablesources/operator_test.go b/internal/resolution/variablesources/operator_test.go index 971887afc..8541c37a8 100644 --- a/internal/resolution/variablesources/operator_test.go +++ b/internal/resolution/variablesources/operator_test.go @@ -19,6 +19,7 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) @@ -94,9 +95,9 @@ var _ = Describe("OLMVariableSource", func() { variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) Expect(err).ToNot(HaveOccurred()) - packageRequiredVariables := filterVariables[*variablesources.RequiredPackageVariable](variables) + packageRequiredVariables := filterVariables[*olmvariables.RequiredPackageVariable](variables) Expect(packageRequiredVariables).To(HaveLen(2)) - Expect(packageRequiredVariables).To(WithTransform(func(bvars []*variablesources.RequiredPackageVariable) map[deppy.Identifier]int { + Expect(packageRequiredVariables).To(WithTransform(func(bvars []*olmvariables.RequiredPackageVariable) map[deppy.Identifier]int { out := map[deppy.Identifier]int{} for _, variable := range bvars { out[variable.Identifier()] = len(variable.BundleEntities()) @@ -115,9 +116,9 @@ var _ = Describe("OLMVariableSource", func() { variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) Expect(err).ToNot(HaveOccurred()) - bundleVariables := filterVariables[*variablesources.BundleVariable](variables) + bundleVariables := filterVariables[*olmvariables.BundleVariable](variables) Expect(bundleVariables).To(HaveLen(3)) - Expect(bundleVariables).To(WithTransform(func(bvars []*variablesources.BundleVariable) []*input.Entity { + Expect(bundleVariables).To(WithTransform(func(bvars []*olmvariables.BundleVariable) []*input.Entity { var out []*input.Entity for _, variable := range bvars { out = append(out, variable.BundleEntity().Entity) @@ -137,9 +138,9 @@ var _ = Describe("OLMVariableSource", func() { variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) Expect(err).ToNot(HaveOccurred()) - bundleVariables := filterVariables[*variablesources.BundleVariable](variables) + bundleVariables := filterVariables[*olmvariables.BundleVariable](variables) Expect(bundleVariables).To(HaveLen(2)) - Expect(bundleVariables).To(WithTransform(func(bvars []*variablesources.BundleVariable) []*input.Entity { + Expect(bundleVariables).To(WithTransform(func(bvars []*olmvariables.BundleVariable) []*input.Entity { var out []*input.Entity for _, variable := range bvars { out = append(out, variable.BundleEntity().Entity) @@ -160,11 +161,11 @@ var _ = Describe("OLMVariableSource", func() { variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) Expect(err).ToNot(HaveOccurred()) - globalConstraintsVariables := filterVariables[*variablesources.BundleUniquenessVariable](variables) + globalConstraintsVariables := filterVariables[*olmvariables.BundleUniquenessVariable](variables) Expect(globalConstraintsVariables).To(HaveLen(6)) // check global variables have the right names - Expect(globalConstraintsVariables).To(WithTransform(func(gvars []*variablesources.BundleUniquenessVariable) []string { + Expect(globalConstraintsVariables).To(WithTransform(func(gvars []*olmvariables.BundleUniquenessVariable) []string { var out []string for _, variable := range gvars { out = append(out, string(variable.Identifier())) diff --git a/internal/resolution/variablesources/required_package.go b/internal/resolution/variablesources/required_package.go index 2f8c4087d..d93e6b6a3 100644 --- a/internal/resolution/variablesources/required_package.go +++ b/internal/resolution/variablesources/required_package.go @@ -6,35 +6,14 @@ import ( "github.com/blang/semver/v4" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/constraint" "github.com/operator-framework/deppy/pkg/deppy/input" olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" "github.com/operator-framework/operator-controller/internal/resolution/util/predicates" "github.com/operator-framework/operator-controller/internal/resolution/util/sort" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) -type RequiredPackageVariable struct { - *input.SimpleVariable - bundleEntities []*olmentity.BundleEntity -} - -func (r *RequiredPackageVariable) BundleEntities() []*olmentity.BundleEntity { - return r.bundleEntities -} - -func NewRequiredPackageVariable(packageName string, bundleEntities []*olmentity.BundleEntity) *RequiredPackageVariable { - id := deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)) - entityIDs := make([]deppy.Identifier, 0, len(bundleEntities)) - for _, bundle := range bundleEntities { - entityIDs = append(entityIDs, bundle.ID) - } - return &RequiredPackageVariable{ - SimpleVariable: input.NewSimpleVariable(id, constraint.Mandatory(), constraint.Dependency(entityIDs...)), - bundleEntities: bundleEntities, - } -} - var _ input.VariableSource = &RequiredPackageVariableSource{} type RequiredPackageVariableSourceOption func(*RequiredPackageVariableSource) error @@ -102,7 +81,7 @@ func (r *RequiredPackageVariableSource) GetVariables(ctx context.Context, entity bundleEntities = append(bundleEntities, olmentity.NewBundleEntity(&resultSet[i])) } return []deppy.Variable{ - NewRequiredPackageVariable(r.packageName, bundleEntities), + olmvariables.NewRequiredPackageVariable(r.packageName, bundleEntities), }, nil } diff --git a/internal/resolution/variablesources/required_package_test.go b/internal/resolution/variablesources/required_package_test.go index a3d7f78a8..0a8f37759 100644 --- a/internal/resolution/variablesources/required_package_test.go +++ b/internal/resolution/variablesources/required_package_test.go @@ -11,49 +11,10 @@ import ( "github.com/operator-framework/operator-registry/alpha/property" olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -var _ = Describe("RequiredPackageVariable", func() { - var ( - rpv *variablesources.RequiredPackageVariable - packageName string - bundleEntities []*olmentity.BundleEntity - ) - - BeforeEach(func() { - packageName = "test-package" - bundleEntities = []*olmentity.BundleEntity{ - olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-3", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - } - rpv = variablesources.NewRequiredPackageVariable(packageName, bundleEntities) - }) - - It("should return the correct package name", func() { - Expect(rpv.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)))) - }) - - It("should return the correct bundle entities", func() { - Expect(rpv.BundleEntities()).To(Equal(bundleEntities)) - }) - - It("should contain both mandatory and dependency constraints", func() { - // TODO: add this test once https://github.com/operator-framework/deppy/pull/85 gets merged - // then we'll be able to inspect constraint types - }) -}) - var _ = Describe("RequiredPackageVariableSource", func() { var ( rpvs *variablesources.RequiredPackageVariableSource @@ -96,7 +57,7 @@ var _ = Describe("RequiredPackageVariableSource", func() { variables, err := rpvs.GetVariables(context.TODO(), mockEntitySource) Expect(err).NotTo(HaveOccurred()) Expect(variables).To(HaveLen(1)) - reqPackageVar, ok := variables[0].(*variablesources.RequiredPackageVariable) + reqPackageVar, ok := variables[0].(*olmvariables.RequiredPackageVariable) Expect(ok).To(BeTrue()) Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)))) @@ -125,7 +86,7 @@ var _ = Describe("RequiredPackageVariableSource", func() { variables, err := rpvs.GetVariables(context.TODO(), mockEntitySource) Expect(err).NotTo(HaveOccurred()) Expect(variables).To(HaveLen(1)) - reqPackageVar, ok := variables[0].(*variablesources.RequiredPackageVariable) + reqPackageVar, ok := variables[0].(*olmvariables.RequiredPackageVariable) Expect(ok).To(BeTrue()) Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName))))