Reduce number of variable sources

Signed-off-by: Mikalai Radchuk <[email protected]>

Author: Mikalai Radchuk

cmd/manager/main.go

@@ -40,6 +40,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/catalogmetadata/cache"
 	catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client"
 	"github.com/operator-framework/operator-controller/internal/controllers"
+	"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
 	"github.com/operator-framework/operator-controller/pkg/features"
 )
 
@@ -113,7 +114,7 @@ func main() {
 		Client: cl,
 		Scheme: mgr.GetScheme(),
 		Resolver: solver.NewDeppySolver(
-			controllers.NewVariableSource(cl, catalogClient),
+			variablesources.NewOLMVariableSource(cl, catalogClient),
 		),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "Operator")

cmd/resolutioncli/main.go

@@ -22,8 +22,7 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/operator-framework/deppy/pkg/deppy/solver"
-	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -32,10 +31,11 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
+	"github.com/operator-framework/deppy/pkg/deppy/solver"
+	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
 
 	operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
 	"github.com/operator-framework/operator-controller/internal/catalogmetadata"
-	"github.com/operator-framework/operator-controller/internal/controllers"
 	olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
 	"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
 )
@@ -71,12 +71,12 @@ func main() {
 	ctx := context.Background()
 
 	var packageName string
-	var packageVersion string
+	var packageVersionRange string
 	var packageChannel string
 	var indexRef string
 	var inputDir string
 	flag.StringVar(&packageName, flagNamePackageName, "", "Name of the package to resolve")
-	flag.StringVar(&packageVersion, flagNamePackageVersion, "", "Version of the package")
+	flag.StringVar(&packageVersionRange, flagNamePackageVersion, "", "Version or version range of the package")
 	flag.StringVar(&packageChannel, flagNamePackageChannel, "", "Channel of the package")
 	// TODO: Consider adding support of multiple refs
 	flag.StringVar(&indexRef, flagNameIndexRef, "", "Index reference (FBC image or dir)")
@@ -89,7 +89,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	err := run(ctx, packageName, packageVersion, packageChannel, indexRef, inputDir)
+	err := run(ctx, packageName, packageChannel, packageVersionRange, indexRef, inputDir)
 	if err != nil {
 		fmt.Fprintln(os.Stderr, err)
 		os.Exit(1)
@@ -108,7 +108,19 @@ func validateFlags(packageName, indexRef string) error {
 	return nil
 }
 
-func run(ctx context.Context, packageName, packageVersion, packageChannel, indexRef, inputDir string) error {
+func run(ctx context.Context, packageName, packageChannel, packageVersionRange, indexRef, inputDir string) error {
+	// Using the fake Kubernetes client and creating objects
+	// in it from manifests & CLI flags is fine for PoC.
+	// But when/if we decide to proceed with CLI/offline resolution
+	// we will need to come up with a better way to create inputs
+	// for resolver when working with CLI.
+	//
+	// We will need to think about multiple types of inputs:
+	//   - How to read required package (what we want to install/update)
+	//   - How to read bundles from multiple catalogs
+	//   - How to take into account cluster information. Some package
+	//     will have constraints like "need Kubernetes version to be >= X"
+	//     or "need >= 3 worker nodes").
 	clientBuilder := fake.NewClientBuilder().WithScheme(scheme)
 
 	if inputDir != "" {
@@ -120,14 +132,22 @@ func run(ctx context.Context, packageName, packageVersion, packageChannel, index
 		clientBuilder.WithRuntimeObjects(objects...)
 	}
 
+	clientBuilder.WithRuntimeObjects(&operatorsv1alpha1.Operator{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "resolutioncli-input-operator",
+		},
+		Spec: operatorsv1alpha1.OperatorSpec{
+			PackageName: packageName,
+			Channel:     packageChannel,
+			Version:     packageVersionRange,
+		},
+	})
+
 	cl := clientBuilder.Build()
 	catalogClient := newIndexRefClient(indexRef)
 
 	resolver := solver.NewDeppySolver(
-		append(
-			variablesources.NestedVariableSource{newPackageVariableSource(catalogClient, packageName, packageVersion, packageChannel)},
-			controllers.NewVariableSource(cl, catalogClient)...,
-		),
+		variablesources.NewOLMVariableSource(cl, catalogClient),
 	)
 
 	bundleImage, err := resolve(ctx, resolver, packageName)

cmd/resolutioncli/variable_source.go

@@ -28,6 +28,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/catalogmetadata"
 	"github.com/operator-framework/operator-controller/internal/conditionsets"
 	"github.com/operator-framework/operator-controller/internal/controllers"
+	"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
 	"github.com/operator-framework/operator-controller/pkg/features"
 	testutil "github.com/operator-framework/operator-controller/test/util"
 )
@@ -44,7 +45,7 @@ var _ = Describe("Operator Controller Test", func() {
 		reconciler = &controllers.OperatorReconciler{
 			Client:   cl,
 			Scheme:   sch,
-			Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)),
+			Resolver: solver.NewDeppySolver(variablesources.NewOLMVariableSource(cl, &fakeCatalogClient)),
 		}
 	})
 	When("the operator does not exist", func() {
@@ -1059,7 +1060,7 @@ func TestOperatorUpgrade(t *testing.T) {
 	reconciler := &controllers.OperatorReconciler{
 		Client:   cl,
 		Scheme:   sch,
-		Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)),
+		Resolver: solver.NewDeppySolver(variablesources.NewOLMVariableSource(cl, &fakeCatalogClient)),
 	}
 
 	t.Run("semver upgrade constraints", func(t *testing.T) {

internal/controllers/operator_controller_test.go

@@ -13,7 +13,7 @@ import (
 	olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
 )
 
-func TestRequiredPackageVariable(t *testing.T) {
+func TestRequiredPackageVariables(t *testing.T) {
 	packageName := "test-package"
 	bundles := []*catalogmetadata.Bundle{
 		{Bundle: declcfg.Bundle{Name: "bundle-1", Properties: []property.Property{

internal/controllers/variable_source.go

@@ -1,63 +1,34 @@
 package variablesources
 
 import (
-	"context"
 	"fmt"
 	"sort"
 
-	"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/deppy/pkg/deppy"
+
 	"github.com/operator-framework/operator-controller/internal/catalogmetadata"
 	catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
 	catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
 	olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
 )
 
-var _ input.VariableSource = &BundlesAndDepsVariableSource{}
-
-type BundlesAndDepsVariableSource struct {
-	catalogClient   BundleProvider
-	variableSources []input.VariableSource
-}
-
-func NewBundlesAndDepsVariableSource(catalogClient BundleProvider, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource {
-	return &BundlesAndDepsVariableSource{
-		catalogClient:   catalogClient,
-		variableSources: inputVariableSources,
-	}
-}
-
-func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) {
-	var variables []deppy.Variable
-
-	// extract required package variables
-	for _, variableSource := range b.variableSources {
-		inputVariables, err := variableSource.GetVariables(ctx)
-		if err != nil {
-			return nil, err
-		}
-		variables = append(variables, inputVariables...)
-	}
-
-	// create bundle queue for dependency resolution
+func MakeBundleVariables(
+	allBundles []*catalogmetadata.Bundle,
+	requiredPackages []*olmvariables.RequiredPackageVariable,
+	installedPackages []*olmvariables.InstalledPackageVariable,
+) ([]*olmvariables.BundleVariable, error) {
 	var bundleQueue []*catalogmetadata.Bundle
-	for _, variable := range variables {
-		switch v := variable.(type) {
-		case *olmvariables.RequiredPackageVariable:
-			bundleQueue = append(bundleQueue, v.Bundles()...)
-		case *olmvariables.InstalledPackageVariable:
-			bundleQueue = append(bundleQueue, v.Bundles()...)
-		}
+	for _, variable := range requiredPackages {
+		bundleQueue = append(bundleQueue, variable.Bundles()...)
 	}
-
-	allBundles, err := b.catalogClient.Bundles(ctx)
-	if err != nil {
-		return nil, err
+	for _, variable := range installedPackages {
+		bundleQueue = append(bundleQueue, variable.Bundles()...)
 	}
 
 	// build bundle and dependency variables
+	var result []*olmvariables.BundleVariable
 	visited := sets.Set[deppy.Identifier]{}
 	for len(bundleQueue) > 0 {
 		// pop head of queue
@@ -73,7 +44,7 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp
 		visited.Insert(id)
 
 		// get bundle dependencies
-		dependencies, err := b.filterBundleDependencies(allBundles, head)
+		dependencies, err := filterBundleDependencies(allBundles, head)
 		if err != nil {
 			return nil, fmt.Errorf("could not determine dependencies for bundle with id '%s': %w", id, err)
 		}
@@ -82,21 +53,23 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp
 		bundleQueue = append(bundleQueue, dependencies...)
 
 		// create variable
-		variables = append(variables, olmvariables.NewBundleVariable(head, dependencies))
+		result = append(result, olmvariables.NewBundleVariable(head, dependencies))
 	}
 
-	return variables, nil
+	return result, nil
 }
 
-func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*catalogmetadata.Bundle, bundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
+func filterBundleDependencies(allBundles []*catalogmetadata.Bundle, bundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
 	var dependencies []*catalogmetadata.Bundle
 	added := sets.Set[deppy.Identifier]{}
 
 	// gather required package dependencies
-	// todo(perdasilva): disambiguate between not found and actual errors
 	requiredPackages, _ := bundle.RequiredPackages()
 	for _, requiredPackage := range requiredPackages {
-		packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(catalogfilter.WithPackageName(requiredPackage.PackageName), catalogfilter.InBlangSemverRange(requiredPackage.SemverRange)))
+		packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(
+			catalogfilter.WithPackageName(requiredPackage.PackageName),
+			catalogfilter.InBlangSemverRange(requiredPackage.SemverRange),
+		))
 		if len(packageDependencyBundles) == 0 {
 			return nil, fmt.Errorf("could not find package dependencies for bundle '%s'", bundle.Name)
 		}