Preserve current capi cluster CP endpoint on updates

pkg/controller/clusters/controlplane.go

@@ -79,33 +79,35 @@ const skipCAPIAutoPauseKCPForExternalEtcdAnnotation = "cluster.x-k8s.io/skip-pau
 
 // ReconcileControlPlane orchestrates the ControlPlane reconciliation logic.
 func ReconcileControlPlane(ctx context.Context, log logr.Logger, c client.Client, cp *ControlPlane) (controller.Result, error) {
-	if cp.EtcdCluster == nil {
-		// For stacked etcd, we don't need orchestration, apply directly
-		return controller.Result{}, applyAllControlPlaneObjects(ctx, c, cp)
+	cluster, kcp, etcdadmCluster, err := readCurrentControlPlane(ctx, c, cp)
+	if err != nil {
+		return controller.Result{}, err
 	}
 
-	// always add skip pause annotation since we want to have full control over the kcp-etcd orchestration
-	clientutil.AddAnnotation(cp.KubeadmControlPlane, skipCAPIAutoPauseKCPForExternalEtcdAnnotation, "true")
+	if cp.EtcdCluster != nil {
+		// always add skip pause annotation since we want to have full control over the kcp-etcd orchestration
+		clientutil.AddAnnotation(cp.KubeadmControlPlane, skipCAPIAutoPauseKCPForExternalEtcdAnnotation, "true")
+	}
 
-	cluster := &clusterv1.Cluster{}
-	err := c.Get(ctx, client.ObjectKeyFromObject(cp.Cluster), cluster)
-	if apierrors.IsNotFound(err) {
-		log.Info("Creating cluster with external etcd")
-		// If the CAPI cluster doesn't exist, this is a new cluster, create all objects
+	if cluster == nil {
+		log.Info("Creating cluster")
+		// If the CAPI cluster doesn't exist, this is a new cluster, create all objects, no need for extra orchestration.
 		return controller.Result{}, applyAllControlPlaneObjects(ctx, c, cp)
 	}
-	if err != nil {
-		return controller.Result{}, errors.Wrap(err, "reading CAPI cluster")
-	}
 
-	etcdadmCluster, err := getEtcdadmCluster(ctx, c, cluster)
-	if err != nil {
-		return controller.Result{}, errors.Wrap(err, "reading CAPI cluster")
+	currentCPEndpoint := cluster.Spec.ControlPlaneEndpoint
+	desiredCPEndpoint := cp.Cluster.Spec.ControlPlaneEndpoint
+	if desiredCPEndpoint.IsZero() && !currentCPEndpoint.IsZero() {
+		// If the control plane endpoint is not set in the desired cluster, we want to keep the current one.
+		// In practice, this condition will always be hit because:
+		// * We don't set the endpoint in the cluster object in our code, we let CAPI do that
+		// * The endpoint never changes once the cluster has been created
+		cp.Cluster.Spec.ControlPlaneEndpoint = currentCPEndpoint
 	}
 
-	kcp := &controlplanev1.KubeadmControlPlane{}
-	if err = c.Get(ctx, objKeyForRef(cluster.Spec.ControlPlaneRef), kcp); err != nil {
-		return controller.Result{}, errors.Wrap(err, "reading kubeadm control plane")
+	if cp.EtcdCluster == nil {
+		// For stacked etcd, we don't need orchestration, apply directly
+		return controller.Result{}, applyAllControlPlaneObjects(ctx, c, cp)
 	}
 
 	// If there are changes for etcd, we only apply those changes for now and we wait.
@@ -123,6 +125,34 @@ func ReconcileControlPlane(ctx context.Context, log logr.Logger, c client.Client
 	return reconcileControlPlaneNodeChanges(ctx, log, c, cp, kcp)
 }
 
+func readCurrentControlPlane(ctx context.Context, c client.Client, cp *ControlPlane) (*clusterv1.Cluster, *controlplanev1.KubeadmControlPlane, *etcdv1.EtcdadmCluster, error) {
+	cluster := &clusterv1.Cluster{}
+	err := c.Get(ctx, client.ObjectKeyFromObject(cp.Cluster), cluster)
+	if apierrors.IsNotFound(err) {
+		// If the CAPI cluster doesn't exist, this is a new cluster, no need to read the rest of the objects.
+		return nil, nil, nil, nil
+	}
+	if err != nil {
+		return nil, nil, nil, errors.Wrap(err, "reading CAPI cluster")
+	}
+
+	kcp := &controlplanev1.KubeadmControlPlane{}
+	if err = c.Get(ctx, objKeyForRef(cluster.Spec.ControlPlaneRef), kcp); err != nil {
+		return nil, nil, nil, errors.Wrap(err, "reading kubeadm control plane")
+	}
+
+	if cp.EtcdCluster == nil {
+		return cluster, kcp, nil, nil
+	}
+
+	etcdadmCluster, err := getEtcdadmCluster(ctx, c, cluster)
+	if err != nil {
+		return nil, nil, nil, errors.Wrap(err, "reading etcdadm cluster")
+	}
+
+	return cluster, kcp, etcdadmCluster, nil
+}
+
 func applyAllControlPlaneObjects(ctx context.Context, c client.Client, cp *ControlPlane) error {
 	if err := serverside.ReconcileObjects(ctx, c, cp.AllObjects()); err != nil {
 		return errors.Wrap(err, "applying control plane objects")

pkg/controller/clusters/controlplane_test.go

@@ -41,6 +41,37 @@ func TestReconcileControlPlaneStackedEtcd(t *testing.T) {
 	api.ShouldEventuallyExist(ctx, cp.ProviderCluster)
 }
 
+func TestReconcileControlPlaneUpdateAfterClusterCreation(t *testing.T) {
+	g := NewWithT(t)
+	c := env.Client()
+	api := envtest.NewAPIExpecter(t, c)
+	ctx := context.Background()
+	ns := env.CreateNamespaceForTest(ctx, t)
+	log := test.NewNullLogger()
+	cp := controlPlaneStackedEtcd(ns)
+
+	originalCPEndpoint := clusterv1.APIEndpoint{
+		Host: "my-server.example.com",
+		Port: 6443,
+	}
+	cp.Cluster.Spec.ControlPlaneEndpoint = originalCPEndpoint
+
+	envtest.CreateObjs(ctx, t, c, cp.AllObjects()...)
+
+	// We never set the endpoint ourselves, so we mimic that here
+	// we want to test the original one set by capi is preserved
+	cp.Cluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{}
+
+	g.Expect(clusters.ReconcileControlPlane(ctx, log, c, cp)).To(Equal(controller.Result{}))
+	api.ShouldEventuallyExist(ctx, cp.Cluster)
+	api.ShouldEventuallyMatch(ctx, cp.Cluster, func(g Gomega) {
+		g.Expect(cp.Cluster.Spec.ControlPlaneEndpoint).To(Equal(originalCPEndpoint))
+	})
+	api.ShouldEventuallyExist(ctx, cp.KubeadmControlPlane)
+	api.ShouldEventuallyExist(ctx, cp.ControlPlaneMachineTemplate)
+	api.ShouldEventuallyExist(ctx, cp.ProviderCluster)
+}
+
 func TestReconcileControlPlaneExternalEtcdNewCluster(t *testing.T) {
 	g := NewWithT(t)
 	c := env.Client()