Make controller as mandatory param in external object tracker

kubernetes-sigs · Oct 15, 2024 · d44453e · d44453e
1 parent 7b752c6
commit d44453e
Show file tree

Hide file tree

Showing 8 changed files with 215 additions and 7 deletions.
diff --git a/controllers/external/fake/controller.go b/controllers/external/fake/controller.go
@@ -0,0 +1,32 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package fake
+
+import (
+	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/source"
+)
+
+// Controller implements controller.Controller interface.
+type Controller struct {
+	controller.Controller
+}
+
+// Watch helps in testing and does nothing but returns nil.
+func (c Controller) Watch(_ source.Source) error {
+	return nil
+}
diff --git a/controllers/external/fake/doc.go b/controllers/external/fake/doc.go
@@ -0,0 +1,18 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package fake provides a fake controllers for testing.
+package fake
diff --git a/controllers/external/tracker.go b/controllers/external/tracker.go
@@ -44,13 +44,8 @@ type ObjectTracker struct {
 
 // Watch uses the controller to issue a Watch only if the object hasn't been seen before.
 func (o *ObjectTracker) Watch(log logr.Logger, obj client.Object, handler handler.EventHandler, p ...predicate.Predicate) error {
-	// Consider this a no-op if the controller isn't present.
-	if o.Controller == nil {
-		return nil
-	}
-
-	if o.Cache == nil || o.Scheme == nil {
-		return errors.New("both scheme and cache must be set for object tracker")
+	if o.Controller == nil || o.Cache == nil || o.Scheme == nil {
+		return errors.New("all of controller, cache and scheme must be set for object tracker")
 	}
 
 	gvk := obj.GetObjectKind().GroupVersionKind()

diff --git a/exp/internal/controllers/machinepool_controller_phases_test.go b/exp/internal/controllers/machinepool_controller_phases_test.go
@@ -26,16 +26,19 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/cache/informertest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/controllers/clustercache"
 	"sigs.k8s.io/cluster-api/controllers/external"
+	fakeController "sigs.k8s.io/cluster-api/controllers/external/fake"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api/internal/test/builder"
 	"sigs.k8s.io/cluster-api/internal/util/ssa"
@@ -128,6 +131,11 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 		r := &MachinePoolReconciler{
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -165,6 +173,11 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 		r := &MachinePoolReconciler{
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -199,6 +212,11 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 		r := &MachinePoolReconciler{
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -249,6 +267,11 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 		r := &MachinePoolReconciler{
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -311,6 +334,11 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 		r := &MachinePoolReconciler{
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -351,6 +379,11 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 		r := &MachinePoolReconciler{
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -398,6 +431,11 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 		r := &MachinePoolReconciler{
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -458,6 +496,11 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 		r := &MachinePoolReconciler{
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -524,6 +567,11 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 		r := &MachinePoolReconciler{
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -588,6 +636,11 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 		r := &MachinePoolReconciler{
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -674,6 +727,11 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
 		r := &MachinePoolReconciler{
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -990,6 +1048,11 @@ func TestReconcileMachinePoolBootstrap(t *testing.T) {
 			bootstrapConfig := &unstructured.Unstructured{Object: tc.bootstrapConfig}
 			r := &MachinePoolReconciler{
 				Client: fake.NewClientBuilder().WithObjects(tc.machinepool, bootstrapConfig, builder.TestBootstrapConfigCRD, builder.TestInfrastructureMachineTemplateCRD).Build(),
+				externalTracker: external.ObjectTracker{
+					Controller: fakeController.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     runtime.NewScheme(),
+				},
 			}
 
 			scope := &scope{
@@ -1285,6 +1348,11 @@ func TestReconcileMachinePoolInfrastructure(t *testing.T) {
 			r := &MachinePoolReconciler{
 				Client:       fakeClient,
 				ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
+				externalTracker: external.ObjectTracker{
+					Controller: fakeController.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     runtime.NewScheme(),
+				},
 			}
 
 			scope := &scope{
@@ -1368,6 +1436,11 @@ func TestReconcileMachinePoolMachines(t *testing.T) {
 			r := &MachinePoolReconciler{
 				Client:   env,
 				ssaCache: ssa.NewCache(),
+				externalTracker: external.ObjectTracker{
+					Controller: fakeController.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     runtime.NewScheme(),
+				},
 			}
 			scope := &scope{
 				machinePool: &machinePool,
@@ -1431,6 +1504,11 @@ func TestReconcileMachinePoolMachines(t *testing.T) {
 			r := &MachinePoolReconciler{
 				Client:   env,
 				ssaCache: ssa.NewCache(),
+				externalTracker: external.ObjectTracker{
+					Controller: fakeController.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     runtime.NewScheme(),
+				},
 			}
 
 			scope := &scope{
@@ -1789,6 +1867,11 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(env.GetClient(), client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
 			recorder:     record.NewFakeRecorder(32),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -1851,6 +1934,11 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(env.GetClient(), client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
 			recorder:     record.NewFakeRecorder(32),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -1896,6 +1984,11 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
 			Client:       fakeClient,
 			recorder:     record.NewFakeRecorder(32),
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -1937,6 +2030,11 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
 			Client:       fakeClient,
 			recorder:     record.NewFakeRecorder(32),
 			ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{
@@ -2000,6 +2098,11 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
 			Client:       fakeClient,
 			ClusterCache: clustercache.NewFakeClusterCache(env.GetClient(), client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
 			recorder:     record.NewFakeRecorder(32),
+			externalTracker: external.ObjectTracker{
+				Controller: fakeController.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     runtime.NewScheme(),
+			},
 		}
 
 		scope := &scope{

diff --git a/exp/internal/controllers/machinepool_controller_test.go b/exp/internal/controllers/machinepool_controller_test.go
@@ -32,13 +32,16 @@ import (
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/cache/informertest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/controllers/clustercache"
+	"sigs.k8s.io/cluster-api/controllers/external"
+	fakeController "sigs.k8s.io/cluster-api/controllers/external/fake"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api/internal/test/builder"
 	"sigs.k8s.io/cluster-api/util"
@@ -556,6 +559,11 @@ func TestReconcileMachinePoolRequest(t *testing.T) {
 				Client:       clientFake,
 				APIReader:    clientFake,
 				ClusterCache: clustercache.NewFakeClusterCache(trackerClientFake, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
+				externalTracker: external.ObjectTracker{
+					Controller: fakeController.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     runtime.NewScheme(),
+				},
 			}
 
 			result, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: util.ObjectKey(&tc.machinePool)})
@@ -1103,6 +1111,11 @@ func TestMachinePoolConditions(t *testing.T) {
 				Client:       clientFake,
 				APIReader:    clientFake,
 				ClusterCache: clustercache.NewFakeClusterCache(clientFake, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
+				externalTracker: external.ObjectTracker{
+					Controller: fakeController.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     runtime.NewScheme(),
+				},
 			}
 
 			_, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: util.ObjectKey(machinePool)})