WIP: drop dynamic client from reconcile object

Refactor acm code, add tests, update vendor code

Author: darkdoc

internal/controller/acm.go

@@ -23,15 +23,19 @@ import (
 	"log"
 
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func haveACMHub(r *PatternReconciler) bool {
-	gvrMCH := schema.GroupVersionResource{Group: "operator.open-cluster-management.io", Version: "v1", Resource: "multiclusterhubs"}
+var mchGVK = schema.GroupVersionKind{
+	Group:   "operator.open-cluster-management.io",
+	Kind:    "multiclusterhubs",
+	Version: "v1",
+}
 
+func haveACMHub(r *PatternReconciler) bool {
 	labelSelector, err := labels.Parse(fmt.Sprintf("%v = %v", "ocm-configmap-type", "image-manifest"))
 
 	if err != nil {
@@ -54,7 +58,11 @@ func haveACMHub(r *PatternReconciler) bool {
 	}
 	ns := cms.Items[0].Namespace
 
-	umch, err := r.dynamicClient.Resource(gvrMCH).Namespace(ns).List(context.TODO(), metav1.ListOptions{})
+	umch := &unstructured.UnstructuredList{}
+	umch.SetGroupVersionKind(mchGVK)
+
+	err = r.List(context.Background(), umch, &client.ListOptions{Namespace: ns})
+
 	if err != nil {
 		log.Printf("Error obtaining hub: %s\n", err)
 		return false

internal/controller/acm_test.go

@@ -4,75 +4,74 @@ import (
 	"context"
 	"fmt"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	dynamicfake "k8s.io/client-go/dynamic/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"
 
 	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
 var _ = Describe("HaveACMHub", func() {
+
 	var (
 		patternReconciler *PatternReconciler
-		dynamicClient     *dynamicfake.FakeDynamicClient
-		gvrMCH            schema.GroupVersionResource
 		fakeClient        client.Client
+		configMap         *v1.ConfigMap
+		hub               *unstructured.Unstructured
 	)
-
 	BeforeEach(func() {
-		gvrMCH = schema.GroupVersionResource{Group: "operator.open-cluster-management.io", Version: "v1", Resource: "multiclusterhubs"}
+		configMap = &v1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-configmap",
+				Namespace: "default",
+				Labels: map[string]string{
+					"ocm-configmap-type": "image-manifest",
+				},
+			},
+		}
+		hub = &unstructured.Unstructured{
+			Object: map[string]any{
+				"apiVersion": "operator.open-cluster-management.io/v1",
+				"kind":       "MultiClusterHub",
+				"metadata": map[string]any{
+					"name":      "test-hub",
+					"namespace": "default",
+				},
+			},
+		}
+		hub.SetGroupVersionKind(mchGVK)
+	})
 
-		dynamicClient = dynamicfake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), map[schema.GroupVersionResource]string{
-			gvrMCH: "MultiClusterHubList",
-		})
+	Context("when the ACM Hub exists in same ns as configmap", func() {
+		It("should return true", func() {
+			fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
+				WithRuntimeObjects(configMap, hub).Build()
+			patternReconciler = &PatternReconciler{
+				Client: fakeClient,
+			}
 
+			result := haveACMHub(patternReconciler)
+			Expect(result).To(BeTrue())
+		})
 	})
 
-	Context("when the ACM Hub exists", func() {
-		BeforeEach(func() {
-			configMap := &v1.ConfigMap{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-configmap",
-					Namespace: "default",
-					Labels: map[string]string{
-						"ocm-configmap-type": "image-manifest",
-					},
-				},
-			}
+	Context("when the ACM Hub exists different ns as configmap", func() {
+		It("should return false", func() {
+			hub.SetNamespace("different")
 
 			fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
-				WithRuntimeObjects(configMap).Build()
+				WithRuntimeObjects(configMap, hub).Build()
 			patternReconciler = &PatternReconciler{
-				Client:        fakeClient,
-				dynamicClient: dynamicClient,
+				Client: fakeClient,
 			}
 
-			hub := &unstructured.Unstructured{
-				Object: map[string]any{
-					"apiVersion": "operator.open-cluster-management.io/v1",
-					"kind":       "MultiClusterHub",
-					"metadata": map[string]any{
-						"name":      "test-hub",
-						"namespace": "default",
-					},
-				},
-			}
-			_, err := dynamicClient.Resource(gvrMCH).Namespace("default").Create(context.Background(), hub, metav1.CreateOptions{})
-			Expect(err).ToNot(HaveOccurred())
-		})
-
-		It("should return true", func() {
 			result := haveACMHub(patternReconciler)
-			Expect(result).To(BeTrue())
+			Expect(result).To(BeFalse())
 		})
 	})
 
@@ -81,8 +80,7 @@ var _ = Describe("HaveACMHub", func() {
 			fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
 				WithRuntimeObjects().Build()
 			patternReconciler = &PatternReconciler{
-				Client:        fakeClient,
-				dynamicClient: dynamicClient,
+				Client: fakeClient,
 			}
 
 			result := haveACMHub(patternReconciler)
@@ -91,47 +89,32 @@ var _ = Describe("HaveACMHub", func() {
 	})
 
 	Context("when there is an error listing ConfigMaps", func() {
-		BeforeEach(func() {
+		It("should return false and log the error", func() {
 			fakeClient = fake.NewClientBuilder().WithInterceptorFuncs(
 				interceptor.Funcs{List: func(ctx context.Context, client client.WithWatch, obj client.ObjectList, opts ...client.ListOption) error {
 					return fmt.Errorf("list error")
 				}}).WithScheme(testEnv.Scheme).Build()
+
 			patternReconciler = &PatternReconciler{
-				Client:        fakeClient,
-				dynamicClient: dynamicClient,
+				Client: fakeClient,
 			}
-		})
 
-		It("should return false and log the error", func() {
 			result := haveACMHub(patternReconciler)
 			Expect(result).To(BeFalse())
 		})
 	})
 
 	Context("when there is an error listing the MultiClusterHubs", func() {
-		BeforeEach(func() {
-			configMap := &v1.ConfigMap{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-configmap",
-					Namespace: "default",
-					Labels: map[string]string{
-						"ocm-configmap-type": "image-manifest",
-					},
-				},
-			}
-			fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
-				WithRuntimeObjects(configMap).Build()
+		It("should return false and log the error", func() {
+			fakeClient = fake.NewClientBuilder().WithInterceptorFuncs(
+				interceptor.Funcs{List: func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error {
+					return fmt.Errorf("list error")
+				}}).WithScheme(testEnv.Scheme).WithRuntimeObjects(configMap).Build()
 
-			dynamicClient.PrependReactor("list", "multiclusterhubs", func(testing.Action) (handled bool, ret runtime.Object, err error) {
-				return true, nil, fmt.Errorf("multiclusterhub error")
-			})
 			patternReconciler = &PatternReconciler{
-				Client:        fakeClient,
-				dynamicClient: dynamicClient,
+				Client: fakeClient,
 			}
-		})
 
-		It("should return false and log the error", func() {
 			result := haveACMHub(patternReconciler)
 			Expect(result).To(BeFalse())
 		})

internal/controller/pattern_controller.go

@@ -32,7 +32,6 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -60,7 +59,6 @@ type PatternReconciler struct {
 	logger logr.Logger
 
 	config          *rest.Config
-	dynamicClient   dynamic.Interface
 	routeClient     routeclient.Interface
 	operatorClient  operatorclient.OperatorV1Interface
 	driftWatcher    driftWatcher
@@ -596,10 +594,6 @@ func (r *PatternReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	var err error
 	r.config = mgr.GetConfig()
 
-	if r.dynamicClient, err = dynamic.NewForConfig(r.config); err != nil {
-		return err
-	}
-
 	if r.operatorClient, err = operatorclient.NewForConfig(r.config); err != nil {
 		return err
 	}