use singleton for get workload cluster

controlplane/kubeadm/internal/control_plane.go

@@ -51,10 +51,13 @@ type ControlPlane struct {
 	// See discussion on https://github.com/kubernetes-sigs/cluster-api/pull/3405
 	KubeadmConfigs map[string]*bootstrapv1.KubeadmConfig
 	InfraResources map[string]*unstructured.Unstructured
+
+	managementCluster ManagementCluster
+	workloadCluster   WorkloadCluster
 }
 
 // NewControlPlane returns an instantiated ControlPlane.
-func NewControlPlane(ctx context.Context, client client.Client, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, ownedMachines collections.Machines) (*ControlPlane, error) {
+func NewControlPlane(ctx context.Context, managementCluster ManagementCluster, client client.Client, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, ownedMachines collections.Machines) (*ControlPlane, error) {
 	infraObjects, err := getInfraResources(ctx, client, ownedMachines)
 	if err != nil {
 		return nil, err
@@ -80,6 +83,7 @@ func NewControlPlane(ctx context.Context, client client.Client, cluster *cluster
 		KubeadmConfigs:       kubeadmConfigs,
 		InfraResources:       infraObjects,
 		reconciliationTime:   metav1.Now(),
+		managementCluster:    managementCluster,
 	}, nil
 }
 
@@ -268,3 +272,28 @@ func (c *ControlPlane) SetPatchHelpers(patchHelpers map[string]*patch.Helper) {
 		c.machinesPatchHelpers[machineName] = patchHelper
 	}
 }
+
+// GetWorkloadCluster builds a cluster object.
+// The cluster comes with an etcd client generator to connect to any etcd pod living on a managed machine.
+func (c *ControlPlane) GetWorkloadCluster(ctx context.Context) (WorkloadCluster, error) {
+	if c.workloadCluster != nil {
+		return c.workloadCluster, nil
+	}
+
+	workloadCluster, err := c.managementCluster.GetWorkloadCluster(ctx, client.ObjectKeyFromObject(c.Cluster))
+	if err != nil {
+		return nil, err
+	}
+	c.workloadCluster = workloadCluster
+	return c.workloadCluster, nil
+}
+
+// InjectTestManagementCluster allows to inject a test ManagementCluster during tests.
+// NOTE: This approach allows to keep the managementCluster field private, which will
+// prevent people from using managementCluster.GetWorkloadCluster because it creates a new
+// instance of WorkloadCluster at every call. People instead should use ControlPlane.GetWorkloadCluster
+// that creates only a single instance of WorkloadCluster for each reconcile.
+func (c *ControlPlane) InjectTestManagementCluster(managementCluster ManagementCluster) {
+	c.managementCluster = managementCluster
+	c.workloadCluster = nil
+}

controlplane/kubeadm/internal/controllers/controller.go

@@ -258,7 +258,7 @@ func (r *KubeadmControlPlaneReconciler) initControlPlaneScope(ctx context.Contex
 
 	// Return early if the cluster is not yet in a state where control plane machines exists
 	if !cluster.Status.InfrastructureReady || !cluster.Spec.ControlPlaneEndpoint.IsValid() {
-		controlPlane, err := internal.NewControlPlane(ctx, r.Client, cluster, kcp, collections.Machines{})
+		controlPlane, err := internal.NewControlPlane(ctx, r.managementCluster, r.Client, cluster, kcp, collections.Machines{})
 		if err != nil {
 			log.Error(err, "failed to initialize control plane scope")
 			return nil, false, err
@@ -285,7 +285,7 @@ func (r *KubeadmControlPlaneReconciler) initControlPlaneScope(ctx context.Contex
 		return nil, false, errors.New("Not all control plane machines are owned by this KubeadmControlPlane, refusing to operate in mixed management mode")
 	}
 
-	controlPlane, err := internal.NewControlPlane(ctx, r.Client, cluster, kcp, ownedMachines)
+	controlPlane, err := internal.NewControlPlane(ctx, r.managementCluster, r.Client, cluster, kcp, ownedMachines)
 	if err != nil {
 		log.Error(err, "failed to initialize control plane scope")
 		return nil, false, err
@@ -426,7 +426,7 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, controlPl
 	}
 
 	// Get the workload cluster client.
-	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster))
+	workloadCluster, err := controlPlane.GetWorkloadCluster(ctx)
 	if err != nil {
 		log.V(2).Info("cannot get remote client to workload cluster, will requeue", "cause", err)
 		return ctrl.Result{Requeue: true}, nil
@@ -668,7 +668,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileControlPlaneConditions(ctx cont
 		return ctrl.Result{}, nil
 	}
 
-	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster))
+	workloadCluster, err := controlPlane.GetWorkloadCluster(ctx)
 	if err != nil {
 		return ctrl.Result{}, errors.Wrap(err, "cannot get remote client to workload cluster")
 	}
@@ -719,7 +719,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileEtcdMembers(ctx context.Context
 		return ctrl.Result{}, nil
 	}
 
-	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster))
+	workloadCluster, err := controlPlane.GetWorkloadCluster(ctx)
 	if err != nil {
 		// Failing at connecting to the workload cluster can mean workload cluster is unhealthy for a variety of reasons such as etcd quorum loss.
 		return ctrl.Result{}, errors.Wrap(err, "cannot get remote client to workload cluster")
@@ -758,7 +758,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileCertificateExpiries(ctx context
 	// Ignore machines which are being deleted.
 	machines := controlPlane.Machines.Filter(collections.Not(collections.HasDeletionTimestamp))
 
-	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster))
+	workloadCluster, err := controlPlane.GetWorkloadCluster(ctx)
 	if err != nil {
 		return ctrl.Result{}, errors.Wrap(err, "failed to reconcile certificate expiries: cannot get remote client to workload cluster")
 	}

controlplane/kubeadm/internal/controllers/controller_test.go

@@ -1099,18 +1099,20 @@ func TestReconcileCertificateExpiries(t *testing.T) {
 		machineWithoutNodeRefKubeadmConfig,
 	)
 
-	controlPlane, err := internal.NewControlPlane(ctx, fakeClient, cluster, kcp, ownedMachines)
-	g.Expect(err).ToNot(HaveOccurred())
+	managementCluster := &fakeManagementCluster{
+		Workload: fakeWorkloadCluster{
+			APIServerCertificateExpiry: &detectedExpiry,
+		},
+	}
 
 	r := &KubeadmControlPlaneReconciler{
-		Client: fakeClient,
-		managementCluster: &fakeManagementCluster{
-			Workload: fakeWorkloadCluster{
-				APIServerCertificateExpiry: &detectedExpiry,
-			},
-		},
+		Client:            fakeClient,
+		managementCluster: managementCluster,
 	}
 
+	controlPlane, err := internal.NewControlPlane(ctx, managementCluster, fakeClient, cluster, kcp, ownedMachines)
+	g.Expect(err).ToNot(HaveOccurred())
+
 	_, err = r.reconcileCertificateExpiries(ctx, controlPlane)
 	g.Expect(err).NotTo(HaveOccurred())
 

controlplane/kubeadm/internal/controllers/remediation.go

@@ -29,12 +29,10 @@ import (
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
 	"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal"
-	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	"sigs.k8s.io/cluster-api/util/patch"
@@ -177,7 +175,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileUnhealthyMachines(ctx context.C
 		// - if the machine hosts the etcd leader, forward etcd leadership to another machine.
 		// - delete the etcd member hosted on the machine being deleted.
 		// - remove the etcd member from the kubeadm config map (only for kubernetes version older than v1.22.0)
-		workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster))
+		workloadCluster, err := controlPlane.GetWorkloadCluster(ctx)
 		if err != nil {
 			log.Error(err, "Failed to create client to workload cluster")
 			return ctrl.Result{}, errors.Wrapf(err, "failed to create client to workload cluster")
@@ -355,10 +353,7 @@ func max(x, y time.Duration) time.Duration {
 func (r *KubeadmControlPlaneReconciler) canSafelyRemoveEtcdMember(ctx context.Context, controlPlane *internal.ControlPlane, machineToBeRemediated *clusterv1.Machine) (bool, error) {
 	log := ctrl.LoggerFrom(ctx)
 
-	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, client.ObjectKey{
-		Namespace: controlPlane.Cluster.Namespace,
-		Name:      controlPlane.Cluster.Name,
-	})
+	workloadCluster, err := controlPlane.GetWorkloadCluster(ctx)
 	if err != nil {
 		return false, errors.Wrapf(err, "failed to get client for workload cluster %s", controlPlane.Cluster.Name)
 	}

controlplane/kubeadm/internal/controllers/remediation_test.go

@@ -461,6 +461,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.reconcileUnhealthyMachines(ctx, controlPlane)
 
@@ -504,6 +505,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.reconcileUnhealthyMachines(ctx, controlPlane)
 
@@ -686,6 +688,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.reconcileUnhealthyMachines(ctx, controlPlane)
 
@@ -737,6 +740,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.reconcileUnhealthyMachines(ctx, controlPlane)
 
@@ -789,6 +793,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.reconcileUnhealthyMachines(ctx, controlPlane)
 
@@ -841,6 +846,8 @@ func TestReconcileUnhealthyMachines(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
+
 		_, err = r.reconcileUnhealthyMachines(ctx, controlPlane)
 		g.Expect(err).ToNot(HaveOccurred())
 
@@ -1097,6 +1104,7 @@ func TestReconcileUnhealthyMachinesSequences(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.reconcileUnhealthyMachines(ctx, controlPlane)
 
@@ -1132,6 +1140,7 @@ func TestReconcileUnhealthyMachinesSequences(t *testing.T) {
 				EtcdMembersResult: nodes(controlPlane.Machines),
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err = r.reconcileUnhealthyMachines(ctx, controlPlane)
 
@@ -1207,6 +1216,7 @@ func TestReconcileUnhealthyMachinesSequences(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.reconcileUnhealthyMachines(ctx, controlPlane)
 
@@ -1278,6 +1288,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, m1)
 		g.Expect(ret).To(BeFalse())
@@ -1309,6 +1320,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, m1)
 		g.Expect(ret).To(BeTrue())
@@ -1346,6 +1358,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, m1)
 		g.Expect(ret).To(BeTrue())
@@ -1376,6 +1389,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, m1)
 		g.Expect(ret).To(BeFalse())
@@ -1407,6 +1421,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, m1)
 		g.Expect(ret).To(BeTrue())
@@ -1445,6 +1460,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, m1)
 		g.Expect(ret).To(BeTrue())
@@ -1476,6 +1492,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, m1)
 		g.Expect(ret).To(BeFalse())
@@ -1509,6 +1526,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, m1)
 		g.Expect(ret).To(BeTrue())
@@ -1542,6 +1560,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, m1)
 		g.Expect(ret).To(BeFalse())
@@ -1577,6 +1596,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, m1)
 		g.Expect(ret).To(BeTrue())
@@ -1612,6 +1632,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 				},
 			},
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		ret, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, m1)
 		g.Expect(ret).To(BeFalse())

controlplane/kubeadm/internal/controllers/scale.go

@@ -31,7 +31,6 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
 	"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal"
-	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/collections"
 	"sigs.k8s.io/cluster-api/util/conditions"
 )
@@ -105,7 +104,7 @@ func (r *KubeadmControlPlaneReconciler) scaleDownControlPlane(
 		return result, err
 	}
 
-	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster))
+	workloadCluster, err := controlPlane.GetWorkloadCluster(ctx)
 	if err != nil {
 		logger.Error(err, "Failed to create client to workload cluster")
 		return ctrl.Result{}, errors.Wrapf(err, "failed to create client to workload cluster")

controlplane/kubeadm/internal/controllers/scale_test.go

@@ -271,6 +271,7 @@ func TestKubeadmControlPlaneReconciler_scaleDownControlPlane_NoError(t *testing.
 			Cluster:  cluster,
 			Machines: machines,
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		result, err := r.scaleDownControlPlane(context.Background(), controlPlane, controlPlane.Machines)
 		g.Expect(err).ToNot(HaveOccurred())
@@ -311,6 +312,7 @@ func TestKubeadmControlPlaneReconciler_scaleDownControlPlane_NoError(t *testing.
 			Cluster:  cluster,
 			Machines: machines,
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		result, err := r.scaleDownControlPlane(context.Background(), controlPlane, controlPlane.Machines)
 		g.Expect(err).ToNot(HaveOccurred())
@@ -347,6 +349,7 @@ func TestKubeadmControlPlaneReconciler_scaleDownControlPlane_NoError(t *testing.
 			Cluster:  cluster,
 			Machines: machines,
 		}
+		controlPlane.InjectTestManagementCluster(r.managementCluster)
 
 		result, err := r.scaleDownControlPlane(context.Background(), controlPlane, controlPlane.Machines)
 		g.Expect(err).ToNot(HaveOccurred())

controlplane/kubeadm/internal/controllers/status.go

@@ -24,7 +24,6 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
 	"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal"
-	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/collections"
 	"sigs.k8s.io/cluster-api/util/conditions"
 )
@@ -82,7 +81,7 @@ func (r *KubeadmControlPlaneReconciler) updateStatus(ctx context.Context, contro
 		conditions.MarkTrue(controlPlane.KCP, controlplanev1.MachinesCreatedCondition)
 	}
 
-	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster))
+	workloadCluster, err := controlPlane.GetWorkloadCluster(ctx)
 	if err != nil {
 		return errors.Wrap(err, "failed to create remote cluster client")
 	}