Merge pull request #6070 from alvaroaleman/break

⚠️ Make ClusterToInfrastructureMapFunc check if the cluster is externally managed

Author: k8s-ci-robot

docs/book/src/developer/providers/v1.1-to-v1.2.md

@@ -10,7 +10,7 @@ maintainers of providers and consumers of our Go API.
 ## Dependencies
 
 **Note**: Only the most relevant dependencies are listed, `k8s.io/` and `ginkgo`/`gomega` dependencies
-in ClusterAPI are kept in sync with the versions used by `sigs.k8s.io/controller-runtime`. 
+in ClusterAPI are kept in sync with the versions used by `sigs.k8s.io/controller-runtime`.
 
 -
 
@@ -26,7 +26,8 @@ in ClusterAPI are kept in sync with the versions used by `sigs.k8s.io/controller
 
 ### API Changes
 
--
+- `util.ClusterToInfrastructureMapFuncWithExternallyManagedCheck` was removed and the externally managed check was added to `util.ClusterToInfrastructureMapFunc`, which required changing its signature.
+   Users of the former simply need to start using the latter and users of the latter need to add the new arguments to their call.
 
 ### Other
 

test/infrastructure/docker/internal/controllers/dockercluster_controller.go

@@ -200,7 +200,7 @@ func (r *DockerClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl
 	}
 	return c.Watch(
 		&source.Kind{Type: &clusterv1.Cluster{}},
-		handler.EnqueueRequestsFromMapFunc(util.ClusterToInfrastructureMapFuncWithExternallyManagedCheck(ctx, infrav1.GroupVersion.WithKind("DockerCluster"), mgr.GetClient(), &infrav1.DockerCluster{})),
+		handler.EnqueueRequestsFromMapFunc(util.ClusterToInfrastructureMapFunc(ctx, infrav1.GroupVersion.WithKind("DockerCluster"), mgr.GetClient(), &infrav1.DockerCluster{})),
 		predicates.ClusterUnpaused(ctrl.LoggerFrom(ctx)),
 	)
 }

util/util.go

@@ -195,65 +195,48 @@ func ObjectKey(object metav1.Object) client.ObjectKey {
 
 // ClusterToInfrastructureMapFunc returns a handler.ToRequestsFunc that watches for
 // Cluster events and returns reconciliation requests for an infrastructure provider object.
-func ClusterToInfrastructureMapFunc(gvk schema.GroupVersionKind) handler.MapFunc {
+func ClusterToInfrastructureMapFunc(ctx context.Context, gvk schema.GroupVersionKind, c client.Client, providerCluster client.Object) handler.MapFunc {
+	log := ctrl.LoggerFrom(ctx)
 	return func(o client.Object) []reconcile.Request {
-		c, ok := o.(*clusterv1.Cluster)
+		cluster, ok := o.(*clusterv1.Cluster)
 		if !ok {
 			return nil
 		}
 
 		// Return early if the InfrastructureRef is nil.
-		if c.Spec.InfrastructureRef == nil {
+		if cluster.Spec.InfrastructureRef == nil {
 			return nil
 		}
 		gk := gvk.GroupKind()
 		// Return early if the GroupKind doesn't match what we expect.
-		infraGK := c.Spec.InfrastructureRef.GroupVersionKind().GroupKind()
+		infraGK := cluster.Spec.InfrastructureRef.GroupVersionKind().GroupKind()
 		if gk != infraGK {
 			return nil
 		}
+		providerCluster := providerCluster.DeepCopyObject().(client.Object)
+		key := types.NamespacedName{Namespace: cluster.Namespace, Name: cluster.Spec.InfrastructureRef.Name}
+
+		if err := c.Get(ctx, key, providerCluster); err != nil {
+			log.V(4).Error(err, fmt.Sprintf("Failed to get %T", providerCluster))
+			return nil
+		}
+
+		if annotations.IsExternallyManaged(providerCluster) {
+			log.V(4).Info(fmt.Sprintf("%T is externally managed, skipping mapping", providerCluster))
+			return nil
+		}
 
 		return []reconcile.Request{
 			{
 				NamespacedName: client.ObjectKey{
-					Namespace: c.Namespace,
-					Name:      c.Spec.InfrastructureRef.Name,
+					Namespace: cluster.Namespace,
+					Name:      cluster.Spec.InfrastructureRef.Name,
 				},
 			},
 		}
 	}
 }
 
-// ClusterToInfrastructureMapFuncWithExternallyManagedCheck is like ClusterToInfrastructureMapFunc but will exclude externally managed infrastructures from the mapping.
-// We will update  ClusterToInfrastructureMapFunc to include this check in an upcoming release but defer that for now as adjusting the signature is a breaking change.
-func ClusterToInfrastructureMapFuncWithExternallyManagedCheck(ctx context.Context, gvk schema.GroupVersionKind, c client.Client, providerCluster client.Object) handler.MapFunc {
-	baseMapper := ClusterToInfrastructureMapFunc(gvk)
-	log := ctrl.LoggerFrom(ctx)
-	return func(o client.Object) []reconcile.Request {
-		var result []reconcile.Request
-		for _, request := range baseMapper(o) {
-			providerCluster := providerCluster.DeepCopyObject().(client.Object)
-			key := types.NamespacedName{Namespace: request.Namespace, Name: request.Name}
-
-			if err := c.Get(ctx, key, providerCluster); err != nil {
-				log.V(4).Error(err, fmt.Sprintf("Failed to get %T", providerCluster))
-				fmt.Printf("failed to get %s: %v\n", key, err)
-				continue
-			}
-
-			if annotations.IsExternallyManaged(providerCluster) {
-				log.V(4).Info(fmt.Sprintf("%T is externally managed, skipping mapping", providerCluster))
-				fmt.Printf("%T is externally managed\n", providerCluster)
-				continue
-			}
-
-			result = append(result, request)
-		}
-
-		return result
-	}
-}
-
 // GetOwnerMachine returns the Machine object owning the current resource.
 func GetOwnerMachine(ctx context.Context, c client.Client, obj metav1.ObjectMeta) (*clusterv1.Machine, error) {
 	for _, ref := range obj.OwnerReferences {

util/util_test.go

@@ -224,7 +224,7 @@ func TestClusterToInfrastructureMapFunc(t *testing.T) {
 			referenceObject.SetAPIVersion(tc.request.Spec.InfrastructureRef.APIVersion)
 			referenceObject.SetKind(tc.request.Spec.InfrastructureRef.Kind)
 
-			fn := ClusterToInfrastructureMapFuncWithExternallyManagedCheck(context.Background(), tc.input, clientBuilder.Build(), referenceObject)
+			fn := ClusterToInfrastructureMapFunc(context.Background(), tc.input, clientBuilder.Build(), referenceObject)
 			out := fn(tc.request)
 			g.Expect(out).To(Equal(tc.output))
 		})