diff --git a/go.mod b/go.mod index 728c53524..3b0683615 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/Masterminds/semver/v3 v3.2.1 github.com/blang/semver/v4 v4.0.0 github.com/go-logr/logr v1.3.0 + github.com/google/go-cmp v0.6.0 github.com/onsi/ginkgo/v2 v2.13.0 github.com/onsi/gomega v1.29.0 github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42 @@ -72,7 +73,6 @@ require ( github.com/golang/protobuf v1.5.3 // indirect github.com/google/cel-go v0.12.6 // indirect github.com/google/gnostic v0.5.7-v3refs // indirect - github.com/google/go-cmp v0.6.0 // indirect github.com/google/gofuzz v1.2.0 // indirect github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect github.com/google/uuid v1.3.0 // indirect diff --git a/internal/resolution/variablesources/crd_constraints.go b/internal/resolution/variablesources/bundle_uniqueness.go similarity index 61% rename from internal/resolution/variablesources/crd_constraints.go rename to internal/resolution/variablesources/bundle_uniqueness.go index 39084b36c..1247c0c2a 100644 --- a/internal/resolution/variablesources/crd_constraints.go +++ b/internal/resolution/variablesources/bundle_uniqueness.go @@ -4,15 +4,53 @@ import ( "context" "fmt" + "k8s.io/apimachinery/pkg/util/sets" + "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" - "k8s.io/apimachinery/pkg/util/sets" "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) -var _ input.VariableSource = &CRDUniquenessConstraintsVariableSource{} +// MakeBundleUniquenessVariables produces variables that constrain +// the solution to at most 1 bundle per package. +// These variables guarantee that no two versions of +// the same package are running at the same time. +func MakeBundleUniquenessVariables(bundleVariables []*olmvariables.BundleVariable) []*olmvariables.BundleUniquenessVariable { + result := []*olmvariables.BundleUniquenessVariable{} + + bundleIDs := sets.Set[deppy.Identifier]{} + packageOrder := []string{} + bundleOrder := map[string][]deppy.Identifier{} + for _, bundleVariable := range bundleVariables { + bundles := make([]*catalogmetadata.Bundle, 0, 1+len(bundleVariable.Dependencies())) + bundles = append(bundles, bundleVariable.Bundle()) + bundles = append(bundles, bundleVariable.Dependencies()...) + for _, bundle := range bundles { + id := olmvariables.BundleVariableID(bundle) + // get bundleID package and update map + packageName := bundle.Package + + if _, ok := bundleOrder[packageName]; !ok { + packageOrder = append(packageOrder, packageName) + } + + if !bundleIDs.Has(id) { + bundleIDs.Insert(id) + bundleOrder[packageName] = append(bundleOrder[packageName], id) + } + } + } + + // create global constraint variables + for _, packageName := range packageOrder { + varID := deppy.IdentifierFromString(fmt.Sprintf("%s package uniqueness", packageName)) + result = append(result, olmvariables.NewBundleUniquenessVariable(varID, bundleOrder[packageName]...)) + } + + return result +} // CRDUniquenessConstraintsVariableSource produces variables that constraint the solution to // 1. at most 1 bundle per package @@ -41,39 +79,17 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex return nil, err } - // todo(perdasilva): better handle cases where a provided gvk is not found - // not all packages will necessarily export a CRD - - bundleIDs := sets.Set[deppy.Identifier]{} - packageOrder := []string{} - bundleOrder := map[string][]deppy.Identifier{} + bundleVariables := []*olmvariables.BundleVariable{} for _, variable := range variables { switch v := variable.(type) { case *olmvariables.BundleVariable: - bundles := []*catalogmetadata.Bundle{v.Bundle()} - bundles = append(bundles, v.Dependencies()...) - for _, bundle := range bundles { - id := olmvariables.BundleVariableID(bundle) - // get bundleID package and update map - packageName := bundle.Package - - if _, ok := bundleOrder[packageName]; !ok { - packageOrder = append(packageOrder, packageName) - } - - if !bundleIDs.Has(id) { - bundleIDs.Insert(id) - bundleOrder[packageName] = append(bundleOrder[packageName], id) - } - } + bundleVariables = append(bundleVariables, v) } } - // create global constraint variables - for _, packageName := range packageOrder { - varID := deppy.IdentifierFromString(fmt.Sprintf("%s package uniqueness", packageName)) - variables = append(variables, olmvariables.NewBundleUniquenessVariable(varID, bundleOrder[packageName]...)) + bundleUniqueness := MakeBundleUniquenessVariables(bundleVariables) + for _, v := range bundleUniqueness { + variables = append(variables, v) } - return variables, nil } diff --git a/internal/resolution/variablesources/crd_constraints_test.go b/internal/resolution/variablesources/bundle_uniqueness_test.go similarity index 78% rename from internal/resolution/variablesources/crd_constraints_test.go rename to internal/resolution/variablesources/bundle_uniqueness_test.go index b0e054512..6b63f50b1 100644 --- a/internal/resolution/variablesources/crd_constraints_test.go +++ b/internal/resolution/variablesources/bundle_uniqueness_test.go @@ -4,11 +4,16 @@ import ( "context" "encoding/json" "fmt" + "testing" + "github.com/google/go-cmp/cmp" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/stretchr/testify/require" "github.com/operator-framework/deppy/pkg/deppy" + "github.com/operator-framework/deppy/pkg/deppy/constraint" + "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" @@ -17,6 +22,98 @@ import ( "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) +func TestMakeBundleUniquenessVariables(t *testing.T) { + const fakeCatalogName = "fake-catalog" + channel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}} + bundleSet := map[string]*catalogmetadata.Bundle{ + "test-package.v1.0.0": { + Bundle: declcfg.Bundle{ + Name: "test-package.v1.0.0", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}`)}, + }, + }, + CatalogName: fakeCatalogName, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "test-package.v1.0.1": { + Bundle: declcfg.Bundle{ + Name: "test-package.v1.0.1", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.1"}`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}`)}, + }, + }, + CatalogName: fakeCatalogName, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + + "some-package.v1.0.0": { + Bundle: declcfg.Bundle{ + Name: "some-package.v1.0.0", + Package: "some-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-package", "version": "1.0.0"}`)}, + }, + }, + CatalogName: fakeCatalogName, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + } + + // Input into the testable function must include more than one bundle + // from the same package to ensure that the function + // enforces uniqueness correctly. + // We also need at least one bundle variable to have a dependency + // on another package. This will help to make sure that the function + // also creates uniqueness variables for dependencies. + bundleVariables := []*olmvariables.BundleVariable{ + olmvariables.NewBundleVariable( + bundleSet["test-package.v1.0.0"], + []*catalogmetadata.Bundle{ + bundleSet["some-package.v1.0.0"], + }, + ), + olmvariables.NewBundleVariable( + bundleSet["test-package.v1.0.1"], + []*catalogmetadata.Bundle{ + bundleSet["some-package.v1.0.0"], + }, + ), + } + + variables := variablesources.MakeBundleUniquenessVariables(bundleVariables) + + // Each package in the input must have own uniqueness variable. + // Each variable expected to have one constraint enforcing at most one + // of the involved bundles to be allowed in the solution + expectedVariables := []*olmvariables.BundleUniquenessVariable{ + { + SimpleVariable: input.NewSimpleVariable( + "test-package package uniqueness", + constraint.AtMost( + 1, + deppy.Identifier("fake-catalog-test-package-test-package.v1.0.0"), + deppy.Identifier("fake-catalog-test-package-test-package.v1.0.1"), + ), + ), + }, + { + SimpleVariable: input.NewSimpleVariable( + "some-package package uniqueness", + constraint.AtMost( + 1, + deppy.Identifier("fake-catalog-some-package-some-package.v1.0.0"), + ), + ), + }, + } + require.Empty(t, cmp.Diff(variables, expectedVariables, cmp.AllowUnexported(input.SimpleVariable{}, constraint.AtMostConstraint{}))) +} + var channel = catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}} var bundleSet = map[string]*catalogmetadata.Bundle{ // required package bundles