WIP: drop olm client from reconcile object

Author: darkdoc

go.mod

@@ -15,7 +15,6 @@ require (
 	github.com/openshift/api v3.9.1-0.20190916204813-cdbe64fb0c91+incompatible
 	github.com/openshift/client-go v0.0.0-20240115204758-e6bf7d631d5e // release-4.16
 	github.com/operator-framework/api v0.33.0
-	github.com/operator-framework/operator-lifecycle-manager v0.33.0
 	github.com/segmentio/analytics-go/v3 v3.3.0
 	go.uber.org/mock v0.6.0
 	golang.org/x/crypto v0.41.0

internal/controller/pattern_controller.go

@@ -43,11 +43,8 @@ import (
 	argoapi "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
 	v1 "github.com/openshift/api/config/v1"
 	routeclient "github.com/openshift/client-go/route/clientset/versioned"
-	olmclient "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned"
 	"k8s.io/client-go/kubernetes"
 
-	//	olmapi "github.com/operator-framework/api/pkg/operators/v1alpha1"
-
 	api "github.com/hybrid-cloud-patterns/patterns-operator/api/v1alpha1"
 	operatorclient "github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1"
 )
@@ -63,7 +60,6 @@ type PatternReconciler struct {
 	logger logr.Logger
 
 	config          *rest.Config
-	olmClient       olmclient.Interface
 	fullClient      kubernetes.Interface
 	dynamicClient   dynamic.Interface
 	routeClient     routeclient.Interface
@@ -190,14 +186,14 @@ func (r *PatternReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	targetSub, _ := newSubscriptionFromConfigMap(r.fullClient)
 	_ = controllerutil.SetOwnerReference(qualifiedInstance, targetSub, r.Scheme)
 
-	sub, _ := getSubscription(r.olmClient, targetSub.Name)
+	sub, _ := getSubscription(r.Client, targetSub.Name)
 	if sub == nil {
-		err = createSubscription(r.olmClient, targetSub)
+		err = createSubscription(r.Client, targetSub)
 		return r.actionPerformed(qualifiedInstance, "create gitops subscription", err)
 	} else if ownedBySame(targetSub, sub) {
 		// Check version/channel etc
 		// Dangerous if multiple patterns do not agree, or automatic upgrades are in place...
-		changed, errSub := updateSubscription(r.olmClient, targetSub, sub)
+		changed, errSub := updateSubscription(r.Client, targetSub, sub)
 		if changed {
 			return r.actionPerformed(qualifiedInstance, "update gitops subscription", errSub)
 		}
@@ -601,10 +597,6 @@ func (r *PatternReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	var err error
 	r.config = mgr.GetConfig()
 
-	if r.olmClient, err = olmclient.NewForConfig(r.config); err != nil {
-		return err
-	}
-
 	if r.fullClient, err = kubernetes.NewForConfig(r.config); err != nil {
 		return err
 	}

internal/controller/pattern_controller_test.go

@@ -29,7 +29,6 @@ import (
 	v1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	operatorclient "github.com/openshift/client-go/operator/clientset/versioned/fake"
-	olmclient "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/fake"
 	gomock "go.uber.org/mock/gomock"
 
 	kubeclient "k8s.io/client-go/kubernetes/fake"
@@ -230,7 +229,6 @@ func newFakeReconciler(initObjects ...runtime.Object) *PatternReconciler {
 	return &PatternReconciler{
 		Scheme:          scheme.Scheme,
 		Client:          fakeClient,
-		olmClient:       olmclient.NewSimpleClientset(),
 		driftWatcher:    watcher,
 		fullClient:      kubeclient.NewSimpleClientset(),
 		operatorClient:  operatorclient.NewSimpleClientset(osControlManager).OperatorV1(),

internal/controller/subscription.go

@@ -23,10 +23,12 @@ import (
 	"reflect"
 
 	operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
-	olmclient "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
 )
 
@@ -80,20 +82,20 @@ func newSubscriptionFromConfigMap(r kubernetes.Interface) (*operatorv1alpha1.Sub
 	return newSubscription, nil
 }
 
-func getSubscription(client olmclient.Interface, name string) (*operatorv1alpha1.Subscription, error) {
-	sub, err := client.OperatorsV1alpha1().Subscriptions(SubscriptionNamespace).Get(context.Background(), name, metav1.GetOptions{})
+func getSubscription(cl client.Client, name string) (*operatorv1alpha1.Subscription, error) {
+	sub := &operatorv1alpha1.Subscription{}
+	err := cl.Get(context.Background(), types.NamespacedName{Name: name, Namespace: SubscriptionNamespace}, sub)
 	if err != nil {
 		return nil, err
 	}
 	return sub, nil
 }
 
-func createSubscription(client olmclient.Interface, sub *operatorv1alpha1.Subscription) error {
-	_, err := client.OperatorsV1alpha1().Subscriptions(SubscriptionNamespace).Create(context.Background(), sub, metav1.CreateOptions{})
-	return err
+func createSubscription(cl client.Client, sub *operatorv1alpha1.Subscription) error {
+	return cl.Create(context.Background(), sub)
 }
 
-func updateSubscription(client olmclient.Interface, target, current *operatorv1alpha1.Subscription) (bool, error) {
+func updateSubscription(cl client.Client, target, current *operatorv1alpha1.Subscription) (bool, error) {
 	changed := false
 	if current == nil || current.Spec == nil {
 		return false, fmt.Errorf("current subscription was nil")
@@ -133,7 +135,7 @@ func updateSubscription(client olmclient.Interface, target, current *operatorv1a
 
 	if changed {
 		target.Spec.DeepCopyInto(current.Spec)
-		_, err := client.OperatorsV1alpha1().Subscriptions(SubscriptionNamespace).Update(context.Background(), current, metav1.UpdateOptions{})
+		err := cl.Update(context.Background(), current)
 		return changed, err
 	}
 

internal/controller/subscription_test.go

@@ -7,12 +7,13 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
-	olmclient "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/fake"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	kubeclient "k8s.io/client-go/kubernetes/fake"
-	"k8s.io/client-go/testing"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 )
 
 var defaultTestSubscription = operatorv1alpha1.Subscription{
@@ -53,26 +54,23 @@ var defaultTestSubConfigMap = corev1.ConfigMap{
 }
 
 var _ = Describe("Subscription Functions", func() {
-	Context("getSubscription", func() {
-		var testSubscription *operatorv1alpha1.Subscription
-		var fakeOlmClientSet *olmclient.Clientset
-
-		BeforeEach(func() {
-			testSubscription = defaultTestSubscription.DeepCopy()
-			fakeOlmClientSet = olmclient.NewSimpleClientset()
-		})
+	var fakeClient client.Client
 
+	Context("getSubscription", func() {
 		It("should error out with a non existing a Subscription", func() {
-			err := createSubscription(fakeOlmClientSet, testSubscription)
-			Expect(err).ToNot(HaveOccurred())
-			_, err = getSubscription(fakeOlmClientSet, "foo")
+			fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
+				WithRuntimeObjects().Build()
+			_, err := getSubscription(fakeClient, "foo")
 			Expect(err).To(HaveOccurred())
 		})
 
 		It("should return a proper Subscription", func() {
-			err := createSubscription(fakeOlmClientSet, testSubscription)
+			fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).Build()
+
+			err := createSubscription(fakeClient, &defaultTestSubscription)
 			Expect(err).ToNot(HaveOccurred())
-			sub, err := getSubscription(fakeOlmClientSet, "foosubscription")
+
+			sub, err := getSubscription(fakeClient, "foosubscription")
 			Expect(err).ToNot(HaveOccurred())
 			Expect(sub.Spec.Channel).To(Equal("foochannel"))
 			Expect(sub.Spec.CatalogSource).To(Equal("foosource"))
@@ -123,14 +121,14 @@ var _ = Describe("Subscription Functions", func() {
 
 var _ = Describe("UpdateSubscription", func() {
 	var (
-		client         *olmclient.Clientset
+		fakeClient     client.Client
 		target         *operatorv1alpha1.Subscription
 		current        *operatorv1alpha1.Subscription
 		subscriptionNs string
 	)
 
 	BeforeEach(func() {
-		client = olmclient.NewSimpleClientset()
+
 		subscriptionNs = "openshift-operators"
 
 		current = &operatorv1alpha1.Subscription{
@@ -148,11 +146,15 @@ var _ = Describe("UpdateSubscription", func() {
 			},
 		}
 		target = current.DeepCopy()
+
+		fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
+			WithRuntimeObjects(current).Build()
+
 	})
 
 	Context("when current subscription is nil", func() {
 		It("should return an error", func() {
-			changed, err := updateSubscription(client, target, nil)
+			changed, err := updateSubscription(fakeClient, target, nil)
 			Expect(changed).To(BeFalse())
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("current subscription was nil"))
@@ -161,7 +163,7 @@ var _ = Describe("UpdateSubscription", func() {
 
 	Context("when target subscription is nil", func() {
 		It("should return an error", func() {
-			changed, err := updateSubscription(client, nil, current)
+			changed, err := updateSubscription(fakeClient, nil, current)
 			Expect(changed).To(BeFalse())
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("target subscription was nil"))
@@ -170,79 +172,85 @@ var _ = Describe("UpdateSubscription", func() {
 
 	Context("when the subscription specs are the same", func() {
 		It("should return false and no error", func() {
-			changed, err := updateSubscription(client, target, current)
+			changed, err := updateSubscription(fakeClient, target, current)
 			Expect(changed).To(BeFalse())
 			Expect(err).ToNot(HaveOccurred())
 		})
 	})
 
 	Context("when the subscription specs are different", func() {
-		BeforeEach(func() {
-			_, _ = client.OperatorsV1alpha1().Subscriptions(subscriptionNs).Create(context.Background(), current, metav1.CreateOptions{})
-		})
 
 		It("channel difference should return true and update the current subscription", func() {
 			target.Spec.Channel = "beta"
-			changed, err := updateSubscription(client, target, current)
+			changed, err := updateSubscription(fakeClient, target, current)
 			Expect(changed).To(BeTrue())
 			Expect(err).ToNot(HaveOccurred())
 
-			updated, err := client.OperatorsV1alpha1().Subscriptions(subscriptionNs).Get(context.Background(), current.Name, metav1.GetOptions{})
+			updated := operatorv1alpha1.Subscription{}
+			err = fakeClient.Get(context.Background(), types.NamespacedName{Name: current.Name, Namespace: current.Namespace}, &updated)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(updated.Spec.Channel).To(Equal("beta"))
 		})
 
 		It("catalgsource difference should return true and update the current subscription", func() {
 			target.Spec.CatalogSource = "somesource"
-			changed, err := updateSubscription(client, target, current)
+			changed, err := updateSubscription(fakeClient, target, current)
 			Expect(changed).To(BeTrue())
 			Expect(err).ToNot(HaveOccurred())
 
-			updated, err := client.OperatorsV1alpha1().Subscriptions(subscriptionNs).Get(context.Background(), current.Name, metav1.GetOptions{})
+			updated := operatorv1alpha1.Subscription{}
+			err = fakeClient.Get(context.Background(), types.NamespacedName{Name: current.Name, Namespace: current.Namespace}, &updated)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(updated.Spec.CatalogSource).To(Equal("somesource"))
 		})
 
 		It("catalogsourcenamespace difference should return true and update the current subscription", func() {
 			target.Spec.CatalogSourceNamespace = "another"
-			changed, err := updateSubscription(client, target, current)
+			changed, err := updateSubscription(fakeClient, target, current)
 			Expect(changed).To(BeTrue())
 			Expect(err).ToNot(HaveOccurred())
 
-			updated, err := client.OperatorsV1alpha1().Subscriptions(subscriptionNs).Get(context.Background(), current.Name, metav1.GetOptions{})
+			updated := operatorv1alpha1.Subscription{}
+			err = fakeClient.Get(context.Background(), types.NamespacedName{Name: current.Name, Namespace: current.Namespace}, &updated)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(updated.Spec.CatalogSourceNamespace).To(Equal("another"))
 		})
 
 		It("package difference should return true and update the current subscription", func() {
 			target.Spec.Package = "notdefault"
-			changed, err := updateSubscription(client, target, current)
+			changed, err := updateSubscription(fakeClient, target, current)
 			Expect(changed).To(BeTrue())
 			Expect(err).ToNot(HaveOccurred())
 
-			updated, err := client.OperatorsV1alpha1().Subscriptions(subscriptionNs).Get(context.Background(), current.Name, metav1.GetOptions{})
+			updated := operatorv1alpha1.Subscription{}
+			err = fakeClient.Get(context.Background(), types.NamespacedName{Name: current.Name, Namespace: current.Namespace}, &updated)
+
 			Expect(err).ToNot(HaveOccurred())
 			Expect(updated.Spec.Package).To(Equal("notdefault"))
 		})
 
 		It("InstallPlanApproval difference should return true and update the current subscription", func() {
 			target.Spec.InstallPlanApproval = operatorv1alpha1.ApprovalManual
-			changed, err := updateSubscription(client, target, current)
+			changed, err := updateSubscription(fakeClient, target, current)
 			Expect(changed).To(BeTrue())
 			Expect(err).ToNot(HaveOccurred())
 
-			updated, err := client.OperatorsV1alpha1().Subscriptions(subscriptionNs).Get(context.Background(), current.Name, metav1.GetOptions{})
+			updated := operatorv1alpha1.Subscription{}
+			err = fakeClient.Get(context.Background(), types.NamespacedName{Name: current.Name, Namespace: current.Namespace}, &updated)
+
 			Expect(err).ToNot(HaveOccurred())
 			Expect(updated.Spec.InstallPlanApproval).To(Equal(operatorv1alpha1.ApprovalManual))
 		})
 
 		It("StartingCSV difference should return true and update the current subscription", func() {
 			target.Spec.StartingCSV = "v1.1.0"
-			changed, err := updateSubscription(client, target, current)
+			changed, err := updateSubscription(fakeClient, target, current)
 			Expect(changed).To(BeTrue())
 			Expect(err).ToNot(HaveOccurred())
 
-			updated, err := client.OperatorsV1alpha1().Subscriptions(subscriptionNs).Get(context.Background(), current.Name, metav1.GetOptions{})
+			updated := operatorv1alpha1.Subscription{}
+			err = fakeClient.Get(context.Background(), types.NamespacedName{Name: current.Name, Namespace: current.Namespace}, &updated)
+
 			Expect(err).ToNot(HaveOccurred())
 			Expect(updated.Spec.StartingCSV).To(Equal("v1.1.0"))
 		})
@@ -257,26 +265,28 @@ var _ = Describe("UpdateSubscription", func() {
 				},
 			}
 			target.Spec.Config = tmp
-			changed, err := updateSubscription(client, target, current)
+			changed, err := updateSubscription(fakeClient, target, current)
 			Expect(changed).To(BeTrue())
 			Expect(err).ToNot(HaveOccurred())
 
-			updated, err := client.OperatorsV1alpha1().Subscriptions(subscriptionNs).Get(context.Background(), current.Name, metav1.GetOptions{})
+			updated := operatorv1alpha1.Subscription{}
+			err = fakeClient.Get(context.Background(), types.NamespacedName{Name: current.Name, Namespace: current.Namespace}, &updated)
+
 			Expect(err).ToNot(HaveOccurred())
 			Expect(updated.Spec.Config.Env[0].Name).To(Equal("foo"))
 		})
 	})
 
 	Context("when there is an error updating the subscription", func() {
-		BeforeEach(func() {
-			client.PrependReactor("update", "subscriptions", func(testing.Action) (handled bool, ret runtime.Object, err error) {
-				return true, nil, fmt.Errorf("update error")
-			})
-			target.Spec.Channel = "beta"
-		})
 
 		It("should return true and an error", func() {
-			changed, err := updateSubscription(client, target, current)
+			fakeClient = fake.NewClientBuilder().WithInterceptorFuncs(
+				interceptor.Funcs{Update: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.UpdateOption) error {
+					return fmt.Errorf("update error")
+				}}).WithScheme(testEnv.Scheme).Build()
+			target.Spec.Channel = "beta"
+
+			changed, err := updateSubscription(fakeClient, target, current)
 			Expect(changed).To(BeTrue())
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("update error"))

internal/controller/suite_test.go

@@ -23,10 +23,12 @@ import (
 
 	argoapi "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
 	gitopsv1alpha1 "github.com/hybrid-cloud-patterns/patterns-operator/api/v1alpha1"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	apiv1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
+	operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
 
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -81,6 +83,8 @@ var _ = BeforeSuite(func() {
 	Expect(err).NotTo(HaveOccurred())
 	err = argoapi.AddToScheme(scheme.Scheme)
 	Expect(err).NotTo(HaveOccurred())
+	err = operatorv1alpha1.AddToScheme(scheme.Scheme)
+	Expect(err).NotTo(HaveOccurred())
 	//+kubebuilder:scaffold:scheme
 
 	k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})