From 38370125bb731371ad3dc53d003a2f9833f849e6 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Tue, 1 Feb 2022 14:46:28 -0500 Subject: [PATCH] ClusterToInfrastructureMapFunc: Exclude externally managed infrastructures The map func needs to check if the infrastructure is managed externally and if so avoid creating a reconcile.Request. There is already code for this in capa, but it is needed for all providers: https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/cf6fea721385d490b570b4cf119defea705b3320/controllers/awscluster_controller.go#L331-L377 --- .../controllers/dockercluster_controller.go | 2 +- util/util.go | 32 +++++++++ util/util_test.go | 66 +++++++++++++++++-- 3 files changed, 92 insertions(+), 8 deletions(-) diff --git a/test/infrastructure/docker/internal/controllers/dockercluster_controller.go b/test/infrastructure/docker/internal/controllers/dockercluster_controller.go index c6d8fac957c3..025707fd97c5 100644 --- a/test/infrastructure/docker/internal/controllers/dockercluster_controller.go +++ b/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)), ) } diff --git a/util/util.go b/util/util.go index 988e2548d49d..2628f936c5b1 100644 --- a/util/util.go +++ b/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 { diff --git a/util/util_test.go b/util/util_test.go index f2c9d2b8780c..f7cb2e640926 100644 --- a/util/util_test.go +++ b/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)) })