Makes OLMVariableSource responsible for fetching

`OLMVariableSource` now contains the client and makes requests
to get available operators.

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

cmd/manager/main.go

@@ -35,6 +35,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/controllers"
 	"github.com/operator-framework/operator-controller/internal/resolution"
 	"github.com/operator-framework/operator-controller/internal/resolution/entitysources"
+	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/olm"
 )
 
 var (
@@ -94,9 +95,12 @@ func main() {
 	}
 
 	if err = (&controllers.OperatorReconciler{
-		Client:   mgr.GetClient(),
-		Scheme:   mgr.GetScheme(),
-		Resolver: resolution.NewOperatorResolver(mgr.GetClient(), entitysources.NewCatalogdEntitySource(mgr.GetClient())),
+		Client: mgr.GetClient(),
+		Scheme: mgr.GetScheme(),
+		Resolver: resolution.NewOperatorResolver(
+			entitysources.NewCatalogdEntitySource(mgr.GetClient()),
+			olm.NewOLMVariableSource(mgr.GetClient()),
+		),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "Operator")
 		os.Exit(1)

internal/controllers/operator_controller.go

@@ -34,9 +34,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
-	"github.com/operator-framework/operator-controller/internal/controllers/validators"
-
 	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"
 	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundles_and_dependencies"
 	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity"

internal/controllers/operator_controller_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/conditionsets"
 	"github.com/operator-framework/operator-controller/internal/controllers"
 	"github.com/operator-framework/operator-controller/internal/resolution"
+	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/olm"
 )
 
 var _ = Describe("Operator Controller Test", func() {
@@ -31,10 +32,13 @@ var _ = Describe("Operator Controller Test", func() {
 	)
 	BeforeEach(func() {
 		ctx = context.Background()
+		variableSource := olm.NewOLMVariableSource(cl)
+		solver := resolution.NewOperatorResolver(testEntitySource, variableSource)
+
 		reconciler = &controllers.OperatorReconciler{
 			Client:   cl,
 			Scheme:   sch,
-			Resolver: resolution.NewOperatorResolver(cl, testEntitySource),
+			Resolver: solver,
 		}
 	})
 	When("the operator does not exist", func() {

internal/resolution/resolver.go

@@ -5,42 +5,23 @@ import (
 
 	"github.com/operator-framework/deppy/pkg/deppy/input"
 	"github.com/operator-framework/deppy/pkg/deppy/solver"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-
-	"github.com/operator-framework/operator-controller/api/v1alpha1"
-	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/olm"
 )
 
 type OperatorResolver struct {
-	entitySource input.EntitySource
-	client       client.Client
+	solver *solver.DeppySolver
 }
 
-func NewOperatorResolver(client client.Client, entitySource input.EntitySource) *OperatorResolver {
-	return &OperatorResolver{
-		entitySource: entitySource,
-		client:       client,
+func NewOperatorResolver(entitySource input.EntitySource, variableSource input.VariableSource) *OperatorResolver {
+	deppySolver, err := solver.NewDeppySolver(entitySource, variableSource)
+	if err != nil {
+		// TODO: Clean up
+		// NewDeppySolver never returns an error: https://github.com/operator-framework/deppy/pull/98
+		panic(err)
 	}
+
+	return &OperatorResolver{solver: deppySolver}
 }
 
 func (o *OperatorResolver) Resolve(ctx context.Context) (*solver.Solution, error) {
-	operatorList := v1alpha1.OperatorList{}
-	if err := o.client.List(ctx, &operatorList); err != nil {
-		return nil, err
-	}
-	if len(operatorList.Items) == 0 {
-		return &solver.Solution{}, nil
-	}
-
-	olmVariableSource := olm.NewOLMVariableSource(operatorList.Items...)
-	deppySolver, err := solver.NewDeppySolver(o.entitySource, olmVariableSource)
-	if err != nil {
-		return nil, err
-	}
-
-	solution, err := deppySolver.Solve(ctx)
-	if err != nil {
-		return nil, err
-	}
-	return solution, nil
+	return o.solver.Solve(ctx)
 }

internal/resolution/resolver_test.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/operator-framework/operator-controller/api/v1alpha1"
 	"github.com/operator-framework/operator-controller/internal/resolution"
+	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/olm"
 )
 
 func TestOperatorResolver(t *testing.T) {
@@ -74,7 +75,8 @@ var _ = Describe("OperatorResolver", func() {
 		}
 		client := FakeClient(resources...)
 		entitySource := input.NewCacheQuerier(testEntityCache)
-		resolver := resolution.NewOperatorResolver(client, entitySource)
+		variableSource := olm.NewOLMVariableSource(client)
+		resolver := resolution.NewOperatorResolver(entitySource, variableSource)
 		solution, err := resolver.Resolve(context.Background())
 		Expect(err).ToNot(HaveOccurred())
 		// 2 * required package variables + 2 * bundle variables
@@ -93,7 +95,8 @@ var _ = Describe("OperatorResolver", func() {
 		var resources []client.Object
 		client := FakeClient(resources...)
 		entitySource := input.NewCacheQuerier(testEntityCache)
-		resolver := resolution.NewOperatorResolver(client, entitySource)
+		variableSource := olm.NewOLMVariableSource(client)
+		resolver := resolution.NewOperatorResolver(entitySource, variableSource)
 		solution, err := resolver.Resolve(context.Background())
 		Expect(err).ToNot(HaveOccurred())
 		Expect(solution.SelectedVariables()).To(HaveLen(0))
@@ -110,7 +113,8 @@ var _ = Describe("OperatorResolver", func() {
 		}
 		client := FakeClient(resource)
 		entitySource := FailEntitySource{}
-		resolver := resolution.NewOperatorResolver(client, entitySource)
+		variableSource := olm.NewOLMVariableSource(client)
+		resolver := resolution.NewOperatorResolver(entitySource, variableSource)
 		solution, err := resolver.Resolve(context.Background())
 		Expect(solution).To(BeNil())
 		Expect(err).To(HaveOccurred())
@@ -119,10 +123,11 @@ 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)
-		resolver := resolution.NewOperatorResolver(client, entitySource)
+		variableSource := olm.NewOLMVariableSource(client)
+		resolver := resolution.NewOperatorResolver(entitySource, variableSource)
 		solution, err := resolver.Resolve(context.Background())
 		Expect(solution).To(BeNil())
-		Expect(err).To(HaveOccurred())
+		Expect(err).To(Equal(fmt.Errorf("something bad happened")))
 	})
 })
 

internal/resolution/variable_sources/olm/olm.go

@@ -5,6 +5,7 @@ import (
 
 	"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/bundles_and_dependencies"
@@ -15,20 +16,25 @@ import (
 var _ input.VariableSource = &OLMVariableSource{}
 
 type OLMVariableSource struct {
-	operators []operatorsv1alpha1.Operator
+	client client.Client
 }
 
-func NewOLMVariableSource(operators ...operatorsv1alpha1.Operator) *OLMVariableSource {
+func NewOLMVariableSource(cl client.Client) *OLMVariableSource {
 	return &OLMVariableSource{
-		operators: operators,
+		client: cl,
 	}
 }
 
 func (o *OLMVariableSource) 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
+	}
+
 	var inputVariableSources []input.VariableSource
 
 	// build required package variable sources
-	for _, operator := range o.operators {
+	for _, operator := range operatorList.Items {
 		rps, err := o.requiredPackageFromOperator(&operator)
 		if err != nil {
 			return nil, err

internal/resolution/variable_sources/olm/olm_test.go

@@ -11,14 +11,25 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/operator-framework/deppy/pkg/deppy"
 	"github.com/operator-framework/deppy/pkg/deppy/input"
+	"github.com/operator-framework/operator-controller/api/v1alpha1"
 	operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
 	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundles_and_dependencies"
 	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/crd_constraints"
 	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/olm"
 	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/required_package"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
+func FakeClient(objects ...client.Object) client.Client {
+	scheme := runtime.NewScheme()
+	utilruntime.Must(v1alpha1.AddToScheme(scheme))
+	return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build()
+}
+
 func TestGlobalConstraints(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "OLMVariableSource Suite")
@@ -59,7 +70,7 @@ func withVersionRange(versionRange string) opOption {
 	}
 }
 
-func operator(name string, opts ...opOption) operatorsv1alpha1.Operator {
+func operator(name string, opts ...opOption) *operatorsv1alpha1.Operator {
 	op := operatorsv1alpha1.Operator{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: name,
@@ -73,7 +84,7 @@ func operator(name string, opts ...opOption) operatorsv1alpha1.Operator {
 			Fail(err.Error())
 		}
 	}
-	return op
+	return &op
 }
 
 var _ = Describe("OLMVariableSource", func() {
@@ -84,20 +95,30 @@ var _ = Describe("OLMVariableSource", func() {
 	})
 
 	It("should produce RequiredPackage variables", func() {
-		olmVariableSource := olm.NewOLMVariableSource(operator("prometheus"), operator("packageA"))
+		cl := FakeClient(operator("prometheus"), operator("packageA"))
+
+		olmVariableSource := olm.NewOLMVariableSource(cl)
 		variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource)
 		Expect(err).ToNot(HaveOccurred())
 
 		packageRequiredVariables := filterVariables[*required_package.RequiredPackageVariable](variables)
 		Expect(packageRequiredVariables).To(HaveLen(2))
-		Expect(packageRequiredVariables[0].Identifier()).To(Equal(deppy.IdentifierFromString("required package prometheus")))
-		Expect(packageRequiredVariables[0].BundleEntities()).To(HaveLen(2))
-		Expect(packageRequiredVariables[1].Identifier()).To(Equal(deppy.IdentifierFromString("required package packageA")))
-		Expect(packageRequiredVariables[1].BundleEntities()).To(HaveLen(1))
+		Expect(packageRequiredVariables).To(WithTransform(func(bvars []*required_package.RequiredPackageVariable) map[deppy.Identifier]int {
+			out := map[deppy.Identifier]int{}
+			for _, variable := range bvars {
+				out[variable.Identifier()] = len(variable.BundleEntities())
+			}
+			return out
+		}, Equal(map[deppy.Identifier]int{
+			deppy.IdentifierFromString("required package prometheus"): 2,
+			deppy.IdentifierFromString("required package packageA"):   1,
+		})))
 	})
 
 	It("should produce BundleVariables variables", func() {
-		olmVariableSource := olm.NewOLMVariableSource(operator("prometheus"), operator("packageA"))
+		cl := FakeClient(operator("prometheus"), operator("packageA"))
+
+		olmVariableSource := olm.NewOLMVariableSource(cl)
 		variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource)
 		Expect(err).ToNot(HaveOccurred())
 
@@ -109,15 +130,17 @@ var _ = Describe("OLMVariableSource", func() {
 				out = append(out, variable.BundleEntity().Entity)
 			}
 			return out
-		}, Equal([]*input.Entity{
+		}, ConsistOf([]*input.Entity{
 			entityFromCache("operatorhub/prometheus/0.47.0"),
 			entityFromCache("operatorhub/prometheus/0.37.0"),
 			entityFromCache("operatorhub/packageA/2.0.0"),
 		})))
 	})
 
 	It("should produce version filtered BundleVariables variables", func() {
-		olmVariableSource := olm.NewOLMVariableSource(operator("prometheus", withVersionRange(">0.40.0")), operator("packageA"))
+		cl := FakeClient(operator("prometheus", withVersionRange(">0.40.0")), operator("packageA"))
+
+		olmVariableSource := olm.NewOLMVariableSource(cl)
 		variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource)
 		Expect(err).ToNot(HaveOccurred())
 
@@ -129,7 +152,7 @@ var _ = Describe("OLMVariableSource", func() {
 				out = append(out, variable.BundleEntity().Entity)
 			}
 			return out
-		}, Equal([]*input.Entity{
+		}, ConsistOf([]*input.Entity{
 			entityFromCache("operatorhub/prometheus/0.47.0"),
 			// filtered out
 			// entityFromCache("operatorhub/prometheus/0.37.0"),
@@ -138,7 +161,9 @@ var _ = Describe("OLMVariableSource", func() {
 	})
 
 	It("should produce GlobalConstraints variables", func() {
-		olmVariableSource := olm.NewOLMVariableSource(operator("prometheus"), operator("packageA"))
+		cl := FakeClient(operator("prometheus"), operator("packageA"))
+
+		olmVariableSource := olm.NewOLMVariableSource(cl)
 		variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource)
 		Expect(err).ToNot(HaveOccurred())
 
@@ -166,7 +191,9 @@ var _ = Describe("OLMVariableSource", func() {
 	})
 
 	It("should return an errors when they occur", func() {
-		olmVariableSource := olm.NewOLMVariableSource(operator("prometheus"), operator("packageA"))
+		cl := FakeClient(operator("prometheus"), operator("packageA"))
+
+		olmVariableSource := olm.NewOLMVariableSource(cl)
 		_, err := olmVariableSource.GetVariables(context.Background(), FailEntitySource{})
 		Expect(err).To(HaveOccurred())
 	})