🌱 Make controller mandatory param for external object tracker

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
+}

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

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()

exp/internal/controllers/machinepool_controller_phases_test.go

@@ -30,12 +30,14 @@ import (
 	"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"
+	externalfake "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 +130,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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{
@@ -165,6 +172,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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{
@@ -199,6 +211,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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{
@@ -249,6 +266,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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{
@@ -311,6 +333,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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{
@@ -351,6 +378,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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{
@@ -398,6 +430,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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{
@@ -458,6 +495,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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{
@@ -524,6 +566,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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{
@@ -588,6 +635,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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{
@@ -674,6 +726,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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{
@@ -988,8 +1045,14 @@ func TestReconcileMachinePoolBootstrap(t *testing.T) {
 			}
 
 			bootstrapConfig := &unstructured.Unstructured{Object: tc.bootstrapConfig}
+			fakeClient := fake.NewClientBuilder().WithObjects(tc.machinepool, bootstrapConfig, builder.TestBootstrapConfigCRD, builder.TestInfrastructureMachineTemplateCRD).Build()
 			r := &MachinePoolReconciler{
-				Client: fake.NewClientBuilder().WithObjects(tc.machinepool, bootstrapConfig, builder.TestBootstrapConfigCRD, builder.TestInfrastructureMachineTemplateCRD).Build(),
+				Client: fakeClient,
+				externalTracker: external.ObjectTracker{
+					Controller: externalfake.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     fakeClient.Scheme(),
+				},
 			}
 
 			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: externalfake.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     fakeClient.Scheme(),
+				},
 			}
 
 			scope := &scope{
@@ -1368,6 +1436,11 @@ func TestReconcileMachinePoolMachines(t *testing.T) {
 			r := &MachinePoolReconciler{
 				Client:   env,
 				ssaCache: ssa.NewCache(),
+				externalTracker: external.ObjectTracker{
+					Controller: externalfake.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     env.Scheme(),
+				},
 			}
 			scope := &scope{
 				machinePool: &machinePool,
@@ -1431,6 +1504,11 @@ func TestReconcileMachinePoolMachines(t *testing.T) {
 			r := &MachinePoolReconciler{
 				Client:   env,
 				ssaCache: ssa.NewCache(),
+				externalTracker: external.ObjectTracker{
+					Controller: externalfake.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     env.Scheme(),
+				},
 			}
 
 			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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		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: externalfake.Controller{},
+				Cache:      &informertest.FakeInformers{},
+				Scheme:     fakeClient.Scheme(),
+			},
 		}
 
 		scope := &scope{

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"
+	externalfake "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"
@@ -597,6 +600,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: externalfake.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     clientFake.Scheme(),
+				},
 			}
 
 			result, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: util.ObjectKey(&tc.machinePool)})
@@ -1156,6 +1164,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: externalfake.Controller{},
+					Cache:      &informertest.FakeInformers{},
+					Scheme:     clientFake.Scheme(),
+				},
 			}
 
 			_, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: util.ObjectKey(machinePool)})