From 446e3d3859accf4baad567c82859ef7196491839 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Thu, 22 Jun 2023 13:35:46 +0100 Subject: [PATCH 1/2] Changes the structure of resolution packages Signed-off-by: Mikalai Radchuk --- cmd/manager/main.go | 4 +- internal/controllers/operator_controller.go | 12 ++-- .../controllers/operator_controller_test.go | 4 +- .../entity => entities}/bundle_entity.go | 2 +- .../entity => entities}/bundle_entity_test.go | 4 +- .../entitysources/catalogdsource.go | 14 ++--- internal/resolution/resolver_test.go | 10 ++-- .../util/predicates/predicates.go | 2 +- .../util/predicates/predicates_test.go | 4 +- .../{variable_sources => }/util/sort/sort.go | 12 ++-- .../util/sort/sort_test.go | 2 +- .../resolution/variable_sources/olm/olm.go | 51 ---------------- .../bundles_and_dependencies.go | 11 ++-- .../bundles_and_dependencies_test.go | 35 +++++------ .../crd_constraints.go | 7 +-- .../crd_constraints_test.go | 60 ++++++++----------- .../resolution/variablesources/operator.go | 48 +++++++++++++++ .../operator_test.go} | 39 +++++------- .../required_package.go | 38 ++++++------ .../required_package_test.go | 28 ++++----- .../variablesources/variablesources_test.go | 13 ++++ 21 files changed, 189 insertions(+), 211 deletions(-) rename internal/resolution/{variable_sources/entity => entities}/bundle_entity.go (99%) rename internal/resolution/{variable_sources/entity => entities}/bundle_entity_test.go (99%) rename internal/resolution/{variable_sources => }/util/predicates/predicates.go (97%) rename internal/resolution/{variable_sources => }/util/predicates/predicates_test.go (96%) rename internal/resolution/{variable_sources => }/util/sort/sort.go (88%) rename internal/resolution/{variable_sources => }/util/sort/sort_test.go (99%) delete mode 100644 internal/resolution/variable_sources/olm/olm.go rename internal/resolution/{variable_sources/bundlesanddependencies => variablesources}/bundles_and_dependencies.go (94%) rename internal/resolution/{variable_sources/bundlesanddependencies => variablesources}/bundles_and_dependencies_test.go (90%) rename internal/resolution/{variable_sources/crdconstraints => variablesources}/crd_constraints.go (95%) rename internal/resolution/{variable_sources/crdconstraints => variablesources}/crd_constraints_test.go (86%) create mode 100644 internal/resolution/variablesources/operator.go rename internal/resolution/{variable_sources/olm/olm_test.go => variablesources/operator_test.go} (83%) rename internal/resolution/{variable_sources/requiredpackage => variablesources}/required_package.go (73%) rename internal/resolution/{variable_sources/requiredpackage => variablesources}/required_package_test.go (87%) create mode 100644 internal/resolution/variablesources/variablesources_test.go diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 611f189f8..ba020b5f2 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -37,7 +37,7 @@ import ( operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/controllers" "github.com/operator-framework/operator-controller/internal/resolution/entitysources" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/olm" + "github.com/operator-framework/operator-controller/internal/resolution/variablesources" "github.com/operator-framework/operator-controller/pkg/features" ) @@ -105,7 +105,7 @@ func main() { Scheme: mgr.GetScheme(), Resolver: solver.NewDeppySolver( entitysources.NewCatalogdEntitySource(mgr.GetClient()), - olm.NewOLMVariableSource(mgr.GetClient()), + variablesources.NewOperatorVariableSource(mgr.GetClient()), ), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Operator") diff --git a/internal/controllers/operator_controller.go b/internal/controllers/operator_controller.go index 5d626d54c..d567a6660 100644 --- a/internal/controllers/operator_controller.go +++ b/internal/controllers/operator_controller.go @@ -41,8 +41,8 @@ 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/variable_sources/bundlesanddependencies" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity" + "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) // OperatorReconciler reconciles a Operator object @@ -244,10 +244,10 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph } } -func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*entity.BundleEntity, error) { +func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*entities.BundleEntity, error) { for _, variable := range solution.SelectedVariables() { switch v := variable.(type) { - case *bundlesanddependencies.BundleVariable: + case *variablesources.BundleVariable: entityPkgName, err := v.BundleEntity().PackageName() if err != nil { return nil, err @@ -358,13 +358,13 @@ func (r *OperatorReconciler) existingBundleDeploymentUnstructured(ctx context.Co // rukpak bundle provisioner class name that is capable of unpacking the bundle type func mapBundleMediaTypeToBundleProvisioner(mediaType string) (string, error) { switch mediaType { - case entity.MediaTypePlain: + case entities.MediaTypePlain: return "core-rukpak-io-plain", nil // To ensure compatibility with bundles created with OLMv0 where the // olm.bundle.mediatype property doesn't exist, we assume that if the // property is empty (i.e doesn't exist) that the bundle is one created // with OLMv0 and therefore should use the registry provisioner - case entity.MediaTypeRegistry, "": + case entities.MediaTypeRegistry, "": return "core-rukpak-io-registry", nil default: return "", fmt.Errorf("unknown bundle mediatype: %s", mediaType) diff --git a/internal/controllers/operator_controller_test.go b/internal/controllers/operator_controller_test.go index 8cf7b1d5f..5e8ef85fa 100644 --- a/internal/controllers/operator_controller_test.go +++ b/internal/controllers/operator_controller_test.go @@ -22,7 +22,7 @@ import ( operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/conditionsets" "github.com/operator-framework/operator-controller/internal/controllers" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/olm" + "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) var _ = Describe("Operator Controller Test", func() { @@ -35,7 +35,7 @@ var _ = Describe("Operator Controller Test", func() { reconciler = &controllers.OperatorReconciler{ Client: cl, Scheme: sch, - Resolver: solver.NewDeppySolver(testEntitySource, olm.NewOLMVariableSource(cl)), + Resolver: solver.NewDeppySolver(testEntitySource, variablesources.NewOperatorVariableSource(cl)), } }) When("the operator does not exist", func() { diff --git a/internal/resolution/variable_sources/entity/bundle_entity.go b/internal/resolution/entities/bundle_entity.go similarity index 99% rename from internal/resolution/variable_sources/entity/bundle_entity.go rename to internal/resolution/entities/bundle_entity.go index 7c3958ede..020b68d06 100644 --- a/internal/resolution/variable_sources/entity/bundle_entity.go +++ b/internal/resolution/entities/bundle_entity.go @@ -1,4 +1,4 @@ -package entity +package entities import ( "encoding/json" diff --git a/internal/resolution/variable_sources/entity/bundle_entity_test.go b/internal/resolution/entities/bundle_entity_test.go similarity index 99% rename from internal/resolution/variable_sources/entity/bundle_entity_test.go rename to internal/resolution/entities/bundle_entity_test.go index dd044269f..f17756035 100644 --- a/internal/resolution/variable_sources/entity/bundle_entity_test.go +++ b/internal/resolution/entities/bundle_entity_test.go @@ -1,4 +1,4 @@ -package entity_test +package entities_test import ( "fmt" @@ -10,7 +10,7 @@ import ( "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/variable_sources/entity" + olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" ) func TestBundleEntity(t *testing.T) { diff --git a/internal/resolution/entitysources/catalogdsource.go b/internal/resolution/entitysources/catalogdsource.go index a4370c337..a9a0037b1 100644 --- a/internal/resolution/entitysources/catalogdsource.go +++ b/internal/resolution/entitysources/catalogdsource.go @@ -11,7 +11,7 @@ import ( "github.com/operator-framework/operator-registry/alpha/property" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity" + "github.com/operator-framework/operator-controller/internal/resolution/entities" ) // CatalogdEntitySource is a source for(/collection of) deppy defined input.Entity, built from content @@ -72,7 +72,7 @@ func (es *CatalogdEntitySource) Iterate(ctx context.Context, fn input.IteratorFu } func getEntities(ctx context.Context, client client.Client) (input.EntityList, error) { - entities := input.EntityList{} + entityList := input.EntityList{} bundleMetadatas, packageMetdatas, err := fetchMetadata(ctx, client) if err != nil { return nil, err @@ -88,8 +88,8 @@ func getEntities(ctx context.Context, client client.Client) (input.EntityList, e // this is already a json marshalled object, so it doesn't need to be marshalled // like the other ones props[property.TypePackage] = string(prop.Value) - case entity.PropertyBundleMediaType: - props[entity.PropertyBundleMediaType] = string(prop.Value) + case entities.PropertyBundleMediaType: + props[entities.PropertyBundleMediaType] = string(prop.Value) } } @@ -97,7 +97,7 @@ func getEntities(ctx context.Context, client client.Client) (input.EntityList, e if err != nil { return nil, err } - props[entity.PropertyBundlePath] = string(imgValue) + props[entities.PropertyBundlePath] = string(imgValue) catalogScopedPkgName := fmt.Sprintf("%s-%s", bundle.Spec.Catalog.Name, bundle.Spec.Package) bundlePkg := packageMetdatas[catalogScopedPkgName] for _, ch := range bundlePkg.Spec.Channels { @@ -110,12 +110,12 @@ func getEntities(ctx context.Context, client client.Client) (input.EntityList, e ID: deppy.IdentifierFromString(fmt.Sprintf("%s%s%s", bundle.Name, bundle.Spec.Package, ch.Name)), Properties: props, } - entities = append(entities, entity) + entityList = append(entityList, entity) } } } } - return entities, nil + return entityList, nil } func fetchMetadata(ctx context.Context, client client.Client) (catalogd.BundleMetadataList, map[string]catalogd.Package, error) { diff --git a/internal/resolution/resolver_test.go b/internal/resolution/resolver_test.go index 1f60c9f58..4c26f7216 100644 --- a/internal/resolution/resolver_test.go +++ b/internal/resolution/resolver_test.go @@ -16,7 +16,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/olm" + "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) func TestOperatorResolver(t *testing.T) { @@ -75,7 +75,7 @@ var _ = Describe("OperatorResolver", func() { } client := FakeClient(resources...) entitySource := input.NewCacheQuerier(testEntityCache) - variableSource := olm.NewOLMVariableSource(client) + variableSource := variablesources.NewOperatorVariableSource(client) resolver := solver.NewDeppySolver(entitySource, variableSource) solution, err := resolver.Solve(context.Background()) Expect(err).ToNot(HaveOccurred()) @@ -95,7 +95,7 @@ var _ = Describe("OperatorResolver", func() { var resources []client.Object client := FakeClient(resources...) entitySource := input.NewCacheQuerier(testEntityCache) - variableSource := olm.NewOLMVariableSource(client) + variableSource := variablesources.NewOperatorVariableSource(client) resolver := solver.NewDeppySolver(entitySource, variableSource) solution, err := resolver.Solve(context.Background()) Expect(err).ToNot(HaveOccurred()) @@ -113,7 +113,7 @@ var _ = Describe("OperatorResolver", func() { } client := FakeClient(resource) entitySource := FailEntitySource{} - variableSource := olm.NewOLMVariableSource(client) + variableSource := variablesources.NewOperatorVariableSource(client) resolver := solver.NewDeppySolver(entitySource, variableSource) solution, err := resolver.Solve(context.Background()) Expect(solution).To(BeNil()) @@ -123,7 +123,7 @@ var _ = Describe("OperatorResolver", func() { It("should return an error if the client throws an error", func() { client := NewFailClientWithError(fmt.Errorf("something bad happened")) entitySource := input.NewCacheQuerier(testEntityCache) - variableSource := olm.NewOLMVariableSource(client) + variableSource := variablesources.NewOperatorVariableSource(client) resolver := solver.NewDeppySolver(entitySource, variableSource) solution, err := resolver.Solve(context.Background()) Expect(solution).To(BeNil()) diff --git a/internal/resolution/variable_sources/util/predicates/predicates.go b/internal/resolution/util/predicates/predicates.go similarity index 97% rename from internal/resolution/variable_sources/util/predicates/predicates.go rename to internal/resolution/util/predicates/predicates.go index d0bc7c4b8..fc124cc7a 100644 --- a/internal/resolution/variable_sources/util/predicates/predicates.go +++ b/internal/resolution/util/predicates/predicates.go @@ -4,7 +4,7 @@ import ( "github.com/blang/semver/v4" "github.com/operator-framework/deppy/pkg/deppy/input" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity" + olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" ) func WithPackageName(packageName string) input.Predicate { diff --git a/internal/resolution/variable_sources/util/predicates/predicates_test.go b/internal/resolution/util/predicates/predicates_test.go similarity index 96% rename from internal/resolution/variable_sources/util/predicates/predicates_test.go rename to internal/resolution/util/predicates/predicates_test.go index 58d5e38f3..fd6da3833 100644 --- a/internal/resolution/variable_sources/util/predicates/predicates_test.go +++ b/internal/resolution/util/predicates/predicates_test.go @@ -9,8 +9,8 @@ import ( "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/variable_sources/entity" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/util/predicates" + olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/resolution/util/predicates" ) func TestPredicates(t *testing.T) { diff --git a/internal/resolution/variable_sources/util/sort/sort.go b/internal/resolution/util/sort/sort.go similarity index 88% rename from internal/resolution/variable_sources/util/sort/sort.go rename to internal/resolution/util/sort/sort.go index 0e64b469a..5006b7e71 100644 --- a/internal/resolution/variable_sources/util/sort/sort.go +++ b/internal/resolution/util/sort/sort.go @@ -5,7 +5,7 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity" + "github.com/operator-framework/operator-controller/internal/resolution/entities" ) // ByChannelAndVersion is an entity sort function that orders the entities in @@ -13,8 +13,8 @@ import ( // if a property does not exist for one of the entities, the one missing the property is pushed down // if both entities are missing the same property they are ordered by id func ByChannelAndVersion(entity1 *input.Entity, entity2 *input.Entity) bool { - e1 := entity.NewBundleEntity(entity1) - e2 := entity.NewBundleEntity(entity2) + e1 := entities.NewBundleEntity(entity1) + e2 := entities.NewBundleEntity(entity2) // first sort package lexical order pkgOrder := packageOrder(e1, e2) @@ -44,7 +44,7 @@ func compareErrors(err1 error, err2 error) int { return 0 } -func packageOrder(e1, e2 *entity.BundleEntity) int { +func packageOrder(e1, e2 *entities.BundleEntity) int { name1, err1 := e1.PackageName() name2, err2 := e2.PackageName() errComp := compareErrors(err1, err2) @@ -54,7 +54,7 @@ func packageOrder(e1, e2 *entity.BundleEntity) int { return strings.Compare(name1, name2) } -func channelOrder(e1, e2 *entity.BundleEntity) int { +func channelOrder(e1, e2 *entities.BundleEntity) int { channelProperties1, err1 := e1.ChannelProperties() channelProperties2, err2 := e2.ChannelProperties() errComp := compareErrors(err1, err2) @@ -67,7 +67,7 @@ func channelOrder(e1, e2 *entity.BundleEntity) int { return strings.Compare(channelProperties1.ChannelName, channelProperties2.ChannelName) } -func versionOrder(e1, e2 *entity.BundleEntity) int { +func versionOrder(e1, e2 *entities.BundleEntity) int { ver1, err1 := e1.Version() ver2, err2 := e2.Version() errComp := compareErrors(err1, err2) diff --git a/internal/resolution/variable_sources/util/sort/sort_test.go b/internal/resolution/util/sort/sort_test.go similarity index 99% rename from internal/resolution/variable_sources/util/sort/sort_test.go rename to internal/resolution/util/sort/sort_test.go index 4ca179eeb..6f1677cfe 100644 --- a/internal/resolution/variable_sources/util/sort/sort_test.go +++ b/internal/resolution/util/sort/sort_test.go @@ -9,7 +9,7 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/operator-registry/alpha/property" - entitysort "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/util/sort" + entitysort "github.com/operator-framework/operator-controller/internal/resolution/util/sort" ) func TestSort(t *testing.T) { diff --git a/internal/resolution/variable_sources/olm/olm.go b/internal/resolution/variable_sources/olm/olm.go deleted file mode 100644 index 53a944f87..000000000 --- a/internal/resolution/variable_sources/olm/olm.go +++ /dev/null @@ -1,51 +0,0 @@ -package olm - -import ( - "context" - - "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" - "sigs.k8s.io/controller-runtime/pkg/client" - - operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundlesanddependencies" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/crdconstraints" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/requiredpackage" -) - -var _ input.VariableSource = &VariableSource{} - -type VariableSource struct { - client client.Client -} - -func NewOLMVariableSource(cl client.Client) *VariableSource { - return &VariableSource{ - client: cl, - } -} - -func (o *VariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { - operatorList := operatorsv1alpha1.OperatorList{} - if err := o.client.List(ctx, &operatorList); err != nil { - return nil, err - } - - // build required package variable sources - inputVariableSources := make([]input.VariableSource, 0, len(operatorList.Items)) - for _, operator := range operatorList.Items { - rps, err := requiredpackage.NewRequiredPackage( - operator.Spec.PackageName, - requiredpackage.InVersionRange(operator.Spec.Version), - requiredpackage.InChannel(operator.Spec.Channel), - ) - if err != nil { - return nil, err - } - inputVariableSources = append(inputVariableSources, rps) - } - - // build variable source pipeline - variableSource := crdconstraints.NewCRDUniquenessConstraintsVariableSource(bundlesanddependencies.NewBundlesAndDepsVariableSource(inputVariableSources...)) - return variableSource.GetVariables(ctx, entitySource) -} diff --git a/internal/resolution/variable_sources/bundlesanddependencies/bundles_and_dependencies.go b/internal/resolution/variablesources/bundles_and_dependencies.go similarity index 94% rename from internal/resolution/variable_sources/bundlesanddependencies/bundles_and_dependencies.go rename to internal/resolution/variablesources/bundles_and_dependencies.go index 719e5848b..d4605472c 100644 --- a/internal/resolution/variable_sources/bundlesanddependencies/bundles_and_dependencies.go +++ b/internal/resolution/variablesources/bundles_and_dependencies.go @@ -1,4 +1,4 @@ -package bundlesanddependencies +package variablesources import ( "context" @@ -10,10 +10,9 @@ import ( "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/variable_sources/entity" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/requiredpackage" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/util/predicates" - entitysort "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/util/sort" + 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" ) type BundleVariable struct { @@ -74,7 +73,7 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context, entityS var bundleEntityQueue []*olmentity.BundleEntity for _, variable := range variables { switch v := variable.(type) { - case *requiredpackage.Variable: + case *RequiredPackageVariable: bundleEntityQueue = append(bundleEntityQueue, v.BundleEntities()...) } } diff --git a/internal/resolution/variable_sources/bundlesanddependencies/bundles_and_dependencies_test.go b/internal/resolution/variablesources/bundles_and_dependencies_test.go similarity index 90% rename from internal/resolution/variable_sources/bundlesanddependencies/bundles_and_dependencies_test.go rename to internal/resolution/variablesources/bundles_and_dependencies_test.go index 1aa874a5e..75d6ef56e 100644 --- a/internal/resolution/variable_sources/bundlesanddependencies/bundles_and_dependencies_test.go +++ b/internal/resolution/variablesources/bundles_and_dependencies_test.go @@ -1,8 +1,7 @@ -package bundlesanddependencies_test +package variablesources_test import ( "context" - "testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -10,19 +9,13 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/operator-registry/alpha/property" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundlesanddependencies" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/requiredpackage" + olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -func TestBundlesAndDeps(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "BundlesAndDependenciesVariableSource Suite") -} - var _ = Describe("BundleVariable", func() { var ( - bv *bundlesanddependencies.BundleVariable + bv *variablesources.BundleVariable bundleEntity *olmentity.BundleEntity dependencies []*olmentity.BundleEntity ) @@ -42,7 +35,7 @@ var _ = Describe("BundleVariable", func() { property.TypeChannel: `{"channelName":"stable","priority":0}`, })), } - bv = bundlesanddependencies.NewBundleVariable(bundleEntity, dependencies) + bv = variablesources.NewBundleVariable(bundleEntity, dependencies) }) It("should return the correct bundle entity", func() { @@ -56,16 +49,16 @@ var _ = Describe("BundleVariable", func() { var _ = Describe("BundlesAndDepsVariableSource", func() { var ( - bdvs *bundlesanddependencies.BundlesAndDepsVariableSource + bdvs *variablesources.BundlesAndDepsVariableSource mockEntitySource input.EntitySource ) BeforeEach(func() { - bdvs = bundlesanddependencies.NewBundlesAndDepsVariableSource( + bdvs = variablesources.NewBundlesAndDepsVariableSource( &MockRequiredPackageSource{ ResultSet: []deppy.Variable{ // must match data in mockEntitySource - requiredpackage.NewRequiredPackageVariable("test-package", []*olmentity.BundleEntity{ + variablesources.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}`, @@ -83,7 +76,7 @@ var _ = Describe("BundlesAndDepsVariableSource", func() { &MockRequiredPackageSource{ ResultSet: []deppy.Variable{ // must match data in mockEntitySource - requiredpackage.NewRequiredPackageVariable("test-package-2", []*olmentity.BundleEntity{ + variablesources.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"}`, @@ -192,10 +185,10 @@ var _ = Describe("BundlesAndDepsVariableSource", func() { variables, err := bdvs.GetVariables(context.TODO(), mockEntitySource) Expect(err).NotTo(HaveOccurred()) - var bundleVariables []*bundlesanddependencies.BundleVariable + var bundleVariables []*variablesources.BundleVariable for _, variable := range variables { switch v := variable.(type) { - case *bundlesanddependencies.BundleVariable: + case *variablesources.BundleVariable: bundleVariables = append(bundleVariables, v) } } @@ -247,8 +240,8 @@ func (m *MockRequiredPackageSource) GetVariables(_ context.Context, _ input.Enti return m.ResultSet, nil } -func VariableWithID(id deppy.Identifier) func(vars []*bundlesanddependencies.BundleVariable) *bundlesanddependencies.BundleVariable { - return func(vars []*bundlesanddependencies.BundleVariable) *bundlesanddependencies.BundleVariable { +func VariableWithID(id deppy.Identifier) func(vars []*variablesources.BundleVariable) *variablesources.BundleVariable { + return func(vars []*variablesources.BundleVariable) *variablesources.BundleVariable { for i := 0; i < len(vars); i++ { if vars[i].Identifier() == id { return vars[i] @@ -258,7 +251,7 @@ func VariableWithID(id deppy.Identifier) func(vars []*bundlesanddependencies.Bun } } -func CollectBundleVariableIDs(vars []*bundlesanddependencies.BundleVariable) []string { +func CollectBundleVariableIDs(vars []*variablesources.BundleVariable) []string { ids := make([]string, 0, len(vars)) for _, v := range vars { ids = append(ids, v.Identifier().String()) diff --git a/internal/resolution/variable_sources/crdconstraints/crd_constraints.go b/internal/resolution/variablesources/crd_constraints.go similarity index 95% rename from internal/resolution/variable_sources/crdconstraints/crd_constraints.go rename to internal/resolution/variablesources/crd_constraints.go index 016a9b2f6..3520baa66 100644 --- a/internal/resolution/variable_sources/crdconstraints/crd_constraints.go +++ b/internal/resolution/variablesources/crd_constraints.go @@ -1,4 +1,4 @@ -package crdconstraints +package variablesources import ( "context" @@ -8,8 +8,7 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/constraint" "github.com/operator-framework/deppy/pkg/deppy/input" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundlesanddependencies" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity" + olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" ) type BundleUniquenessVariable struct { @@ -63,7 +62,7 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex gvkToBundleMap := map[string]map[deppy.Identifier]struct{}{} for _, variable := range variables { switch v := variable.(type) { - case *bundlesanddependencies.BundleVariable: + case *BundleVariable: bundleEntities := []*olmentity.BundleEntity{v.BundleEntity()} bundleEntities = append(bundleEntities, v.Dependencies()...) for _, bundleEntity := range bundleEntities { diff --git a/internal/resolution/variable_sources/crdconstraints/crd_constraints_test.go b/internal/resolution/variablesources/crd_constraints_test.go similarity index 86% rename from internal/resolution/variable_sources/crdconstraints/crd_constraints_test.go rename to internal/resolution/variablesources/crd_constraints_test.go index dc8c14fec..1a5a4bf39 100644 --- a/internal/resolution/variable_sources/crdconstraints/crd_constraints_test.go +++ b/internal/resolution/variablesources/crd_constraints_test.go @@ -1,9 +1,8 @@ -package crdconstraints_test +package variablesources_test import ( "context" "fmt" - "testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -12,22 +11,15 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/operator-registry/alpha/property" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundlesanddependencies" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/crdconstraints" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/requiredpackage" + olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -func TestGlobalConstraints(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "CRDUniquenessConstraintsVariableSource Suite") -} - var _ = Describe("BundleUniquenessVariable", func() { var ( id deppy.Identifier atMostIDs []deppy.Identifier - globalConstraintVariable *crdconstraints.BundleUniquenessVariable + globalConstraintVariable *variablesources.BundleUniquenessVariable ) BeforeEach(func() { @@ -36,7 +28,7 @@ var _ = Describe("BundleUniquenessVariable", func() { deppy.IdentifierFromString("test-at-most-id-1"), deppy.IdentifierFromString("test-at-most-id-2"), } - globalConstraintVariable = crdconstraints.NewBundleUniquenessVariable(id, atMostIDs...) + globalConstraintVariable = variablesources.NewBundleUniquenessVariable(id, atMostIDs...) }) It("should initialize a new global constraint variable", func() { @@ -145,14 +137,14 @@ var bundleSet = map[deppy.Identifier]*input.Entity{ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { var ( inputVariableSource *MockInputVariableSource - crdConstraintVariableSource *crdconstraints.CRDUniquenessConstraintsVariableSource + crdConstraintVariableSource *variablesources.CRDUniquenessConstraintsVariableSource ctx context.Context entitySource input.EntitySource ) BeforeEach(func() { inputVariableSource = &MockInputVariableSource{} - crdConstraintVariableSource = crdconstraints.NewCRDUniquenessConstraintsVariableSource(inputVariableSource) + crdConstraintVariableSource = variablesources.NewCRDUniquenessConstraintsVariableSource(inputVariableSource) ctx = context.Background() // the entity is not used in this variable source @@ -161,16 +153,16 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { It("should get variables from the input variable source and create global constraint variables", func() { inputVariableSource.ResultSet = []deppy.Variable{ - requiredpackage.NewRequiredPackageVariable("test-package", []*olmentity.BundleEntity{ + variablesources.NewRequiredPackageVariable("test-package", []*olmentity.BundleEntity{ olmentity.NewBundleEntity(bundleSet["bundle-2"]), olmentity.NewBundleEntity(bundleSet["bundle-1"]), }), - requiredpackage.NewRequiredPackageVariable("test-package-2", []*olmentity.BundleEntity{ + variablesources.NewRequiredPackageVariable("test-package-2", []*olmentity.BundleEntity{ olmentity.NewBundleEntity(bundleSet["bundle-14"]), olmentity.NewBundleEntity(bundleSet["bundle-15"]), olmentity.NewBundleEntity(bundleSet["bundle-16"]), }), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-2"]), []*olmentity.BundleEntity{ olmentity.NewBundleEntity(bundleSet["bundle-3"]), @@ -180,7 +172,7 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { olmentity.NewBundleEntity(bundleSet["bundle-7"]), }, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-1"]), []*olmentity.BundleEntity{ olmentity.NewBundleEntity(bundleSet["bundle-6"]), @@ -188,23 +180,23 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { olmentity.NewBundleEntity(bundleSet["bundle-8"]), }, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-3"]), []*olmentity.BundleEntity{}, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-4"]), []*olmentity.BundleEntity{}, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-5"]), []*olmentity.BundleEntity{}, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-6"]), []*olmentity.BundleEntity{}, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-7"]), []*olmentity.BundleEntity{ olmentity.NewBundleEntity(bundleSet["bundle-8"]), @@ -212,27 +204,27 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { olmentity.NewBundleEntity(bundleSet["bundle-10"]), }, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-8"]), []*olmentity.BundleEntity{}, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-9"]), []*olmentity.BundleEntity{}, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-10"]), []*olmentity.BundleEntity{}, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-14"]), []*olmentity.BundleEntity{}, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-15"]), []*olmentity.BundleEntity{}, ), - bundlesanddependencies.NewBundleVariable( + variablesources.NewBundleVariable( olmentity.NewBundleEntity(bundleSet["bundle-16"]), []*olmentity.BundleEntity{}, ), @@ -240,10 +232,10 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { variables, err := crdConstraintVariableSource.GetVariables(ctx, entitySource) Expect(err).ToNot(HaveOccurred()) Expect(variables).To(HaveLen(26)) - var crdConstraintVariables []*crdconstraints.BundleUniquenessVariable + var crdConstraintVariables []*variablesources.BundleUniquenessVariable for _, variable := range variables { switch v := variable.(type) { - case *crdconstraints.BundleUniquenessVariable: + case *variablesources.BundleUniquenessVariable: crdConstraintVariables = append(crdConstraintVariables, v) } } @@ -264,7 +256,7 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { It("should return an error if input variable source returns an error", func() { inputVariableSource = &MockInputVariableSource{Err: fmt.Errorf("error getting variables")} - crdConstraintVariableSource = crdconstraints.NewCRDUniquenessConstraintsVariableSource(inputVariableSource) + crdConstraintVariableSource = variablesources.NewCRDUniquenessConstraintsVariableSource(inputVariableSource) _, err := crdConstraintVariableSource.GetVariables(ctx, entitySource) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("error getting variables")) @@ -303,7 +295,7 @@ func (m *MockInputVariableSource) GetVariables(_ context.Context, _ input.Entity return m.ResultSet, nil } -func CollectGlobalConstraintVariableIDs(vars []*crdconstraints.BundleUniquenessVariable) []string { +func CollectGlobalConstraintVariableIDs(vars []*variablesources.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.go b/internal/resolution/variablesources/operator.go new file mode 100644 index 000000000..537f5702f --- /dev/null +++ b/internal/resolution/variablesources/operator.go @@ -0,0 +1,48 @@ +package variablesources + +import ( + "context" + + "github.com/operator-framework/deppy/pkg/deppy" + "github.com/operator-framework/deppy/pkg/deppy/input" + "sigs.k8s.io/controller-runtime/pkg/client" + + operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" +) + +var _ input.VariableSource = &OperatorVariableSource{} + +type OperatorVariableSource struct { + client client.Client +} + +func NewOperatorVariableSource(cl client.Client) *OperatorVariableSource { + return &OperatorVariableSource{ + client: cl, + } +} + +func (o *OperatorVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { + operatorList := operatorsv1alpha1.OperatorList{} + if err := o.client.List(ctx, &operatorList); err != nil { + return nil, err + } + + // build required package variable sources + inputVariableSources := make([]input.VariableSource, 0, len(operatorList.Items)) + for _, operator := range operatorList.Items { + rps, err := NewRequiredPackageVariableSource( + operator.Spec.PackageName, + InVersionRange(operator.Spec.Version), + InChannel(operator.Spec.Channel), + ) + if err != nil { + return nil, err + } + inputVariableSources = append(inputVariableSources, rps) + } + + // build variable source pipeline + variableSource := NewCRDUniquenessConstraintsVariableSource(NewBundlesAndDepsVariableSource(inputVariableSources...)) + return variableSource.GetVariables(ctx, entitySource) +} diff --git a/internal/resolution/variable_sources/olm/olm_test.go b/internal/resolution/variablesources/operator_test.go similarity index 83% rename from internal/resolution/variable_sources/olm/olm_test.go rename to internal/resolution/variablesources/operator_test.go index 6e65d24b1..971887afc 100644 --- a/internal/resolution/variable_sources/olm/olm_test.go +++ b/internal/resolution/variablesources/operator_test.go @@ -1,11 +1,10 @@ -package olm_test +package variablesources_test import ( "context" "fmt" "sort" "strings" - "testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -20,10 +19,7 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundlesanddependencies" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/crdconstraints" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/olm" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/requiredpackage" + "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) func FakeClient(objects ...client.Object) client.Client { @@ -32,11 +28,6 @@ func FakeClient(objects ...client.Object) client.Client { return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() } -func TestGlobalConstraints(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "OLMVariableSource Suite") -} - var testEntityCache = map[deppy.Identifier]input.Entity{ "operatorhub/prometheus/0.37.0": *input.NewEntity("operatorhub/prometheus/0.37.0", map[string]string{ "olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`, @@ -99,13 +90,13 @@ var _ = Describe("OLMVariableSource", func() { It("should produce RequiredPackage variables", func() { cl := FakeClient(operator("prometheus"), operator("packageA")) - olmVariableSource := olm.NewOLMVariableSource(cl) + olmVariableSource := variablesources.NewOperatorVariableSource(cl) variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) Expect(err).ToNot(HaveOccurred()) - packageRequiredVariables := filterVariables[*requiredpackage.Variable](variables) + packageRequiredVariables := filterVariables[*variablesources.RequiredPackageVariable](variables) Expect(packageRequiredVariables).To(HaveLen(2)) - Expect(packageRequiredVariables).To(WithTransform(func(bvars []*requiredpackage.Variable) map[deppy.Identifier]int { + Expect(packageRequiredVariables).To(WithTransform(func(bvars []*variablesources.RequiredPackageVariable) map[deppy.Identifier]int { out := map[deppy.Identifier]int{} for _, variable := range bvars { out[variable.Identifier()] = len(variable.BundleEntities()) @@ -120,13 +111,13 @@ var _ = Describe("OLMVariableSource", func() { It("should produce BundleVariables variables", func() { cl := FakeClient(operator("prometheus"), operator("packageA")) - olmVariableSource := olm.NewOLMVariableSource(cl) + olmVariableSource := variablesources.NewOperatorVariableSource(cl) variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) Expect(err).ToNot(HaveOccurred()) - bundleVariables := filterVariables[*bundlesanddependencies.BundleVariable](variables) + bundleVariables := filterVariables[*variablesources.BundleVariable](variables) Expect(bundleVariables).To(HaveLen(3)) - Expect(bundleVariables).To(WithTransform(func(bvars []*bundlesanddependencies.BundleVariable) []*input.Entity { + Expect(bundleVariables).To(WithTransform(func(bvars []*variablesources.BundleVariable) []*input.Entity { var out []*input.Entity for _, variable := range bvars { out = append(out, variable.BundleEntity().Entity) @@ -142,13 +133,13 @@ var _ = Describe("OLMVariableSource", func() { It("should produce version filtered BundleVariables variables", func() { cl := FakeClient(operator("prometheus", withVersionRange(">0.40.0")), operator("packageA")) - olmVariableSource := olm.NewOLMVariableSource(cl) + olmVariableSource := variablesources.NewOperatorVariableSource(cl) variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) Expect(err).ToNot(HaveOccurred()) - bundleVariables := filterVariables[*bundlesanddependencies.BundleVariable](variables) + bundleVariables := filterVariables[*variablesources.BundleVariable](variables) Expect(bundleVariables).To(HaveLen(2)) - Expect(bundleVariables).To(WithTransform(func(bvars []*bundlesanddependencies.BundleVariable) []*input.Entity { + Expect(bundleVariables).To(WithTransform(func(bvars []*variablesources.BundleVariable) []*input.Entity { var out []*input.Entity for _, variable := range bvars { out = append(out, variable.BundleEntity().Entity) @@ -165,15 +156,15 @@ var _ = Describe("OLMVariableSource", func() { It("should produce GlobalConstraints variables", func() { cl := FakeClient(operator("prometheus"), operator("packageA")) - olmVariableSource := olm.NewOLMVariableSource(cl) + olmVariableSource := variablesources.NewOperatorVariableSource(cl) variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) Expect(err).ToNot(HaveOccurred()) - globalConstraintsVariables := filterVariables[*crdconstraints.BundleUniquenessVariable](variables) + globalConstraintsVariables := filterVariables[*variablesources.BundleUniquenessVariable](variables) Expect(globalConstraintsVariables).To(HaveLen(6)) // check global variables have the right names - Expect(globalConstraintsVariables).To(WithTransform(func(gvars []*crdconstraints.BundleUniquenessVariable) []string { + Expect(globalConstraintsVariables).To(WithTransform(func(gvars []*variablesources.BundleUniquenessVariable) []string { var out []string for _, variable := range gvars { out = append(out, string(variable.Identifier())) @@ -195,7 +186,7 @@ var _ = Describe("OLMVariableSource", func() { It("should return an errors when they occur", func() { cl := FakeClient(operator("prometheus"), operator("packageA")) - olmVariableSource := olm.NewOLMVariableSource(cl) + olmVariableSource := variablesources.NewOperatorVariableSource(cl) _, err := olmVariableSource.GetVariables(context.Background(), FailEntitySource{}) Expect(err).To(HaveOccurred()) }) diff --git a/internal/resolution/variable_sources/requiredpackage/required_package.go b/internal/resolution/variablesources/required_package.go similarity index 73% rename from internal/resolution/variable_sources/requiredpackage/required_package.go rename to internal/resolution/variablesources/required_package.go index eb9c0d5d4..2f8c4087d 100644 --- a/internal/resolution/variable_sources/requiredpackage/required_package.go +++ b/internal/resolution/variablesources/required_package.go @@ -1,4 +1,4 @@ -package requiredpackage +package variablesources import ( "context" @@ -9,38 +9,38 @@ import ( "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/variable_sources/entity" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/util/predicates" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/util/sort" + 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" ) -type Variable struct { +type RequiredPackageVariable struct { *input.SimpleVariable bundleEntities []*olmentity.BundleEntity } -func (r *Variable) BundleEntities() []*olmentity.BundleEntity { +func (r *RequiredPackageVariable) BundleEntities() []*olmentity.BundleEntity { return r.bundleEntities } -func NewRequiredPackageVariable(packageName string, bundleEntities []*olmentity.BundleEntity) *Variable { +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 &Variable{ + return &RequiredPackageVariable{ SimpleVariable: input.NewSimpleVariable(id, constraint.Mandatory(), constraint.Dependency(entityIDs...)), bundleEntities: bundleEntities, } } -var _ input.VariableSource = &VariableSource{} +var _ input.VariableSource = &RequiredPackageVariableSource{} -type Option func(*VariableSource) error +type RequiredPackageVariableSourceOption func(*RequiredPackageVariableSource) error -func InVersionRange(versionRange string) Option { - return func(r *VariableSource) error { +func InVersionRange(versionRange string) RequiredPackageVariableSourceOption { + return func(r *RequiredPackageVariableSource) error { if versionRange != "" { vr, err := semver.ParseRange(versionRange) if err == nil { @@ -55,8 +55,8 @@ func InVersionRange(versionRange string) Option { } } -func InChannel(channelName string) Option { - return func(r *VariableSource) error { +func InChannel(channelName string) RequiredPackageVariableSourceOption { + return func(r *RequiredPackageVariableSource) error { if channelName != "" { r.channelName = channelName r.predicates = append(r.predicates, predicates.InChannel(channelName)) @@ -65,18 +65,18 @@ func InChannel(channelName string) Option { } } -type VariableSource struct { +type RequiredPackageVariableSource struct { packageName string versionRange string channelName string predicates []input.Predicate } -func NewRequiredPackage(packageName string, options ...Option) (*VariableSource, error) { +func NewRequiredPackageVariableSource(packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) { if packageName == "" { return nil, fmt.Errorf("package name must not be empty") } - r := &VariableSource{ + r := &RequiredPackageVariableSource{ packageName: packageName, predicates: []input.Predicate{predicates.WithPackageName(packageName)}, } @@ -88,7 +88,7 @@ func NewRequiredPackage(packageName string, options ...Option) (*VariableSource, return r, nil } -func (r *VariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { +func (r *RequiredPackageVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { resultSet, err := entitySource.Filter(ctx, input.And(r.predicates...)) if err != nil { return nil, err @@ -106,7 +106,7 @@ func (r *VariableSource) GetVariables(ctx context.Context, entitySource input.En }, nil } -func (r *VariableSource) notFoundError() error { +func (r *RequiredPackageVariableSource) notFoundError() error { // TODO: update this error message when/if we decide to support version ranges as opposed to fixing the version // context: we originally wanted to support version ranges and take the highest version that satisfies the range // during the upstream call on the 2023-04-11 we decided to pin the version instead. But, we'll keep version range diff --git a/internal/resolution/variable_sources/requiredpackage/required_package_test.go b/internal/resolution/variablesources/required_package_test.go similarity index 87% rename from internal/resolution/variable_sources/requiredpackage/required_package_test.go rename to internal/resolution/variablesources/required_package_test.go index 970debad8..a3d7f78a8 100644 --- a/internal/resolution/variable_sources/requiredpackage/required_package_test.go +++ b/internal/resolution/variablesources/required_package_test.go @@ -1,9 +1,8 @@ -package requiredpackage_test +package variablesources_test import ( "context" "fmt" - "testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -11,18 +10,13 @@ import ( "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/variable_sources/entity" - "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/requiredpackage" + olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -func TestRequiredPackage(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "RequiredPackageVariableSource Suite") -} - var _ = Describe("RequiredPackageVariable", func() { var ( - rpv *requiredpackage.Variable + rpv *variablesources.RequiredPackageVariable packageName string bundleEntities []*olmentity.BundleEntity ) @@ -43,7 +37,7 @@ var _ = Describe("RequiredPackageVariable", func() { property.TypeChannel: `{"channelName":"stable","priority":0}`, })), } - rpv = requiredpackage.NewRequiredPackageVariable(packageName, bundleEntities) + rpv = variablesources.NewRequiredPackageVariable(packageName, bundleEntities) }) It("should return the correct package name", func() { @@ -62,7 +56,7 @@ var _ = Describe("RequiredPackageVariable", func() { var _ = Describe("RequiredPackageVariableSource", func() { var ( - rpvs *requiredpackage.VariableSource + rpvs *variablesources.RequiredPackageVariableSource packageName string mockEntitySource input.EntitySource ) @@ -70,7 +64,7 @@ var _ = Describe("RequiredPackageVariableSource", func() { BeforeEach(func() { var err error packageName = "test-package" - rpvs, err = requiredpackage.NewRequiredPackage(packageName) + rpvs, err = variablesources.NewRequiredPackageVariableSource(packageName) Expect(err).NotTo(HaveOccurred()) mockEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{ "bundle-1": *input.NewEntity("bundle-1", map[string]string{ @@ -102,7 +96,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].(*requiredpackage.Variable) + reqPackageVar, ok := variables[0].(*variablesources.RequiredPackageVariable) Expect(ok).To(BeTrue()) Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)))) @@ -125,13 +119,13 @@ var _ = Describe("RequiredPackageVariableSource", func() { It("should filter by version range", func() { // recreate source with version range option var err error - rpvs, err = requiredpackage.NewRequiredPackage(packageName, requiredpackage.InVersionRange(">=1.0.0 !2.0.0 <3.0.0")) + rpvs, err = variablesources.NewRequiredPackageVariableSource(packageName, variablesources.InVersionRange(">=1.0.0 !2.0.0 <3.0.0")) Expect(err).NotTo(HaveOccurred()) variables, err := rpvs.GetVariables(context.TODO(), mockEntitySource) Expect(err).NotTo(HaveOccurred()) Expect(variables).To(HaveLen(1)) - reqPackageVar, ok := variables[0].(*requiredpackage.Variable) + reqPackageVar, ok := variables[0].(*variablesources.RequiredPackageVariable) Expect(ok).To(BeTrue()) Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)))) @@ -145,7 +139,7 @@ var _ = Describe("RequiredPackageVariableSource", func() { }) It("should fail with bad semver range", func() { - _, err := requiredpackage.NewRequiredPackage(packageName, requiredpackage.InVersionRange("not a valid semver")) + _, err := variablesources.NewRequiredPackageVariableSource(packageName, variablesources.InVersionRange("not a valid semver")) Expect(err).To(HaveOccurred()) }) diff --git a/internal/resolution/variablesources/variablesources_test.go b/internal/resolution/variablesources/variablesources_test.go new file mode 100644 index 000000000..7bb8d97b8 --- /dev/null +++ b/internal/resolution/variablesources/variablesources_test.go @@ -0,0 +1,13 @@ +package variablesources_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestVariableSources(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Variable Sources Suite") +} From 88b2693a2bdd82d1f29bfeee56af39cbcad84470 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Mon, 26 Jun 2023 14:00:10 +0100 Subject: [PATCH 2/2] 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 75d6ef56e..db161ad6d 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) } } @@ -240,8 +207,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] @@ -251,7 +218,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 1a5a4bf39..bb456eae0 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) } } @@ -295,7 +274,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))))