Merge pull request #6064 from k8s-infra-cherrypick-robot/cherry-pick-…

…6039-to-release-1.1

🐛 ClusterToInfrastructureMapFunc: Exclude externally managed infrastructures

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.ClusterToInfrastructureMapFunc(infrav1.GroupVersion.WithKind("DockerCluster"))),
+		handler.EnqueueRequestsFromMapFunc(util.ClusterToInfrastructureMapFuncWithExternallyManagedCheck(ctx, infrav1.GroupVersion.WithKind("DockerCluster"), mgr.GetClient(), &infrav1.DockerCluster{})),
 		predicates.ClusterUnpaused(ctrl.LoggerFrom(ctx)),
 	)
 }

util/util.go

@@ -37,6 +37,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	k8sversion "k8s.io/apimachinery/pkg/version"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -45,6 +46,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/cluster-api/util/annotations"
 )
 
 const (
@@ -222,6 +224,36 @@ func ClusterToInfrastructureMapFunc(gvk schema.GroupVersionKind) handler.MapFunc
 	}
 }
 
+// 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

@@ -17,6 +17,7 @@ limitations under the License.
 package util
 
 import (
+	"context"
 	"fmt"
 	"testing"
 
@@ -109,13 +110,12 @@ func TestMachineToInfrastructureMapFunc(t *testing.T) {
 }
 
 func TestClusterToInfrastructureMapFunc(t *testing.T) {
-	g := NewWithT(t)
-
 	var testcases = []struct {
-		name    string
-		input   schema.GroupVersionKind
-		request client.Object
-		output  []reconcile.Request
+		name           string
+		input          schema.GroupVersionKind
+		request        *clusterv1.Cluster
+		infrastructure client.Object
+		output         []reconcile.Request
 	}{
 		{
 			name: "should reconcile infra-1",
@@ -137,6 +137,14 @@ func TestClusterToInfrastructureMapFunc(t *testing.T) {
 					},
 				},
 			},
+			infrastructure: &unstructured.Unstructured{Object: map[string]interface{}{
+				"apiVersion": "foo.cluster.x-k8s.io/v1beta1",
+				"kind":       "TestCluster",
+				"metadata": map[string]interface{}{
+					"namespace": metav1.NamespaceDefault,
+					"name":      "infra-1",
+				},
+			}},
 			output: []reconcile.Request{
 				{
 					NamespacedName: client.ObjectKey{
@@ -168,11 +176,55 @@ func TestClusterToInfrastructureMapFunc(t *testing.T) {
 			},
 			output: nil,
 		},
+		{
+			name: "Externally managed provider cluster is excluded",
+			input: schema.GroupVersionKind{
+				Group:   "foo.cluster.x-k8s.io",
+				Version: "v1alpha4",
+				Kind:    "TestCluster",
+			},
+			request: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: metav1.NamespaceDefault,
+					Name:      "test-1",
+				},
+				Spec: clusterv1.ClusterSpec{
+					InfrastructureRef: &corev1.ObjectReference{
+						APIVersion: "foo.cluster.x-k8s.io/v1beta1",
+						Kind:       "TestCluster",
+						Name:       "infra-1",
+					},
+				},
+			},
+			infrastructure: &unstructured.Unstructured{Object: map[string]interface{}{
+				"apiVersion": "foo.cluster.x-k8s.io/v1beta1",
+				"kind":       "TestCluster",
+				"metadata": map[string]interface{}{
+					"namespace": metav1.NamespaceDefault,
+					"name":      "infra-1",
+					"annotations": map[string]interface{}{
+						clusterv1.ManagedByAnnotation: "",
+					},
+				},
+			}},
+		},
 	}
 
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			fn := ClusterToInfrastructureMapFunc(tc.input)
+			g := NewWithT(t)
+			clientBuilder := fake.NewClientBuilder()
+			if tc.infrastructure != nil {
+				clientBuilder.WithObjects(tc.infrastructure)
+			}
+
+			// Unstructured simplifies testing but should not be used in real usage, because it will
+			// likely result in a duplicate cache in an unstructured projection.
+			referenceObject := &unstructured.Unstructured{}
+			referenceObject.SetAPIVersion(tc.request.Spec.InfrastructureRef.APIVersion)
+			referenceObject.SetKind(tc.request.Spec.InfrastructureRef.Kind)
+
+			fn := ClusterToInfrastructureMapFuncWithExternallyManagedCheck(context.Background(), tc.input, clientBuilder.Build(), referenceObject)
 			out := fn(tc.request)
 			g.Expect(out).To(Equal(tc.output))
 		})