Reduce number of variable sources

Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>

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

@@ -35,9 +35,7 @@ import (
 
 	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"
 )
 
 const pocMessage = `This command is a proof of concept for off-cluster resolution and is not intended for production use!
@@ -71,12 +69,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 +87,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 +106,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 != "" {
@@ -124,10 +134,7 @@ func run(ctx context.Context, packageName, packageVersion, packageChannel, index
 	catalogClient := newIndexRefClient(indexRef)
 
 	resolver := solver.NewDeppySolver(
-		append(
-			variablesources.NestedVariableSource{newPackageVariableSource(catalogClient, packageName, packageVersion, packageChannel)},
-			controllers.NewVariableSource(cl, catalogClient)...,
-		),
+		NewOfflineOLMVariableSource(cl, catalogClient, packageName, packageChannel, packageVersionRange),
 	)
 
 	bundleImage, err := resolve(ctx, resolver, packageName)

cmd/resolutioncli/variable_source.go

@@ -17,28 +17,87 @@ limitations under the License.
 package main
 
 import (
-	"github.com/operator-framework/deppy/pkg/deppy/input"
+	"context"
 
+	"github.com/operator-framework/deppy/pkg/deppy"
+	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	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"
 )
 
-func newPackageVariableSource(catalogClient *indexRefClient, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
-	return func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
-		pkgSource, err := variablesources.NewRequiredPackageVariableSource(
-			catalogClient,
-			packageName,
-			variablesources.InVersionRange(packageVersion),
-			variablesources.InChannel(packageChannel),
-		)
-		if err != nil {
-			return nil, err
-		}
-
-		sliceSource := variablesources.SliceVariableSource{pkgSource}
-		if inputVariableSource != nil {
-			sliceSource = append(sliceSource, inputVariableSource)
-		}
-
-		return sliceSource, nil
+type OfflineOLMVariableSource struct {
+	client        client.Client
+	catalogClient *indexRefClient
+
+	packageName         string
+	packageChannel      string
+	packageVersionRange string
+}
+
+func NewOfflineOLMVariableSource(cl client.Client, catalogClient *indexRefClient, packageName, packageChannel, packageVersionRange string) *OfflineOLMVariableSource {
+	return &OfflineOLMVariableSource{
+		client:        cl,
+		catalogClient: catalogClient,
+
+		packageName:         packageName,
+		packageChannel:      packageChannel,
+		packageVersionRange: packageVersionRange,
+	}
+}
+
+func (o *OfflineOLMVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) {
+	operatorList := operatorsv1alpha1.OperatorList{}
+	if err := o.client.List(ctx, &operatorList); err != nil {
+		return nil, err
+	}
+
+	bundleDeployments := rukpakv1alpha1.BundleDeploymentList{}
+	if err := o.client.List(ctx, &bundleDeployments); err != nil {
+		return nil, err
+	}
+
+	allBundles, err := o.catalogClient.Bundles(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	requiredPackages := []*olmvariables.RequiredPackageVariable{}
+	requiredPackage, err := variablesources.RequiredPackageVariable(allBundles, o.packageName, o.packageChannel, o.packageVersionRange)
+	if err != nil {
+		return nil, err
+	}
+	requiredPackages = append(requiredPackages, requiredPackage)
+
+	installedPackages, err := variablesources.InstalledPackageVariables(allBundles, bundleDeployments.Items)
+	if err != nil {
+		return nil, err
+	}
+
+	bundles, err := variablesources.BundleVariables(allBundles, requiredPackages, installedPackages)
+	if err != nil {
+		return nil, err
+	}
+
+	bundleUniqueness, err := variablesources.BundleUniquenessVariables(bundles)
+	if err != nil {
+		return nil, err
+	}
+
+	result := []deppy.Variable{}
+	for _, v := range requiredPackages {
+		result = append(result, v)
+	}
+	for _, v := range installedPackages {
+		result = append(result, v)
+	}
+	for _, v := range bundles {
+		result = append(result, v)
+	}
+	for _, v := range bundleUniqueness {
+		result = append(result, v)
 	}
+	return result, nil
 }

internal/controllers/operator_controller_test.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/resolution/variablesources/bundles_and_dependencies.go → internal/resolution/variablesources/bundle.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 BundleVariables(
+	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)
 		}