🌱 supervisor: create the VirtualMachineService for the control plane without a name suffix (#3415)

* supervisor: create the VirtualMachineService for the control plane without a name suffix

This allows creating clusters on supervisor having a name > 41 characters.
Before this change the VirtualMachineService was exceeding the name length limit of 63
and thus never got created.
The code falls back to the legacy name if there is no VirtualMachineService found.

* add test coverage

* fix

Author: chrischdi

pkg/services/vmoperator/control_plane_endpoint.go

@@ -154,8 +154,14 @@ func (s *CPService) ReconcileControlPlaneEndpointService(ctx context.Context, cl
 	return cpEndpoint, nil
 }
 
-func controlPlaneVMServiceName(ctx *vmware.ClusterContext) string {
-	return fmt.Sprintf("%s-control-plane-service", ctx.Cluster.Name)
+func controlPlaneVMServiceName(clusterName string) string {
+	return clusterName
+}
+
+// legacyControlPlaneVMServiceName was used for creating the ControlPlane VirtualMachineService prior to
+// v1.13.0. It resulted in limiting the name of a Cluster to 41 characters (besides other places).
+func legacyControlPlaneVMServiceName(clusterName string) string {
+	return fmt.Sprintf("%s-control-plane-service", clusterName)
 }
 
 // ClusterRoleVMLabels returns labels applied to a VirtualMachine in the cluster. The Control Plane
@@ -179,7 +185,7 @@ func clusterRoleVMLabels(ctx *vmware.ClusterContext, controlPlane bool) map[stri
 func newVirtualMachineService(ctx *vmware.ClusterContext) *vmoprv1.VirtualMachineService {
 	return &vmoprv1.VirtualMachineService{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      controlPlaneVMServiceName(ctx),
+			Name:      controlPlaneVMServiceName(ctx.Cluster.Name),
 			Namespace: ctx.Cluster.Namespace,
 		},
 		TypeMeta: metav1.TypeMeta{
@@ -247,15 +253,26 @@ func (s *CPService) getVMControlPlaneService(ctx context.Context, clusterCtx *vm
 	vmService := &vmoprv1.VirtualMachineService{}
 	vmServiceKey := client.ObjectKey{
 		Namespace: clusterCtx.Cluster.Namespace,
-		Name:      controlPlaneVMServiceName(clusterCtx),
+		Name:      controlPlaneVMServiceName(clusterCtx.Cluster.Name),
 	}
 	if err := s.Client.Get(ctx, vmServiceKey, vmService); err != nil {
 		if !apierrors.IsNotFound(err) {
 			return nil, fmt.Errorf("failed to get VirtualMachineService %s: %v", vmServiceKey.Name, err)
 		}
 
-		log.Info("VirtualMachineService was not found", "VirtualMachineService", klog.KRef(vmServiceKey.Namespace, vmServiceKey.Name))
-		return nil, err
+		// In case of not finding the ControlPlane VirtualMachineService: fallback to try the legacy name.
+		fallbackVMServiceKey := client.ObjectKey{
+			Namespace: vmServiceKey.Namespace,
+			Name:      legacyControlPlaneVMServiceName(clusterCtx.Cluster.Name),
+		}
+		if fallbackErr := s.Client.Get(ctx, fallbackVMServiceKey, vmService); fallbackErr != nil {
+			if !apierrors.IsNotFound(fallbackErr) {
+				return nil, fmt.Errorf("failed to get VirtualMachineService %s: %v", fallbackVMServiceKey.Name, fallbackErr)
+			}
+
+			log.Info("VirtualMachineService was not found", "VirtualMachineService", klog.KRef(vmServiceKey.Namespace, vmServiceKey.Name))
+			return nil, err
+		}
 	}
 
 	return vmService, nil

pkg/services/vmoperator/control_plane_endpoint_test.go

@@ -27,7 +27,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
@@ -39,17 +38,12 @@ import (
 	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/util"
 )
 
-func getVirtualMachineService(ctx context.Context, clusterCtx *vmware.ClusterContext, c ctrlclient.Client, _ CPService) *vmoprv1.VirtualMachineService {
-	vms := newVirtualMachineService(clusterCtx)
-	nsname := types.NamespacedName{
-		Namespace: vms.Namespace,
-		Name:      vms.Name,
-	}
-	err := c.Get(ctx, nsname, vms)
+func getVirtualMachineService(ctx context.Context, clusterCtx *vmware.ClusterContext, _ ctrlclient.Client, cpService CPService) *vmoprv1.VirtualMachineService {
+	vms, err := cpService.getVMControlPlaneService(ctx, clusterCtx)
 	if apierrors.IsNotFound(err) {
 		return nil
 	}
-	Expect(err).ShouldNot(HaveOccurred())
+	Expect(err).ToNot(HaveOccurred())
 	return vms
 }
 
@@ -184,10 +178,33 @@ var _ = Describe("ControlPlaneEndpoint Tests", func() {
 
 		Specify("DummyLBNetworkProvider has a LoadBalancer", func() {
 			expectReconcileError = true // VirtualMachineService LB does not yet have VIP assigned
+			expectAPIEndpoint = false
 			expectVMS = true
 			expectedType = vmoprv1.VirtualMachineServiceTypeLoadBalancer
 			apiEndpoint, err = cpService.ReconcileControlPlaneEndpointService(ctx, clusterCtx, network.DummyLBNetworkProvider())
 			verifyOutput()
+
+			// Set a VIP and reconcile again.
+			expectReconcileError = false
+			expectAPIEndpoint = true
+			updateVMServiceWithVIP(ctx, clusterCtx, c, cpService, vip)
+			expectedPort = defaultAPIBindPort
+			expectedHost = vip
+			apiEndpoint, err = cpService.ReconcileControlPlaneEndpointService(ctx, clusterCtx, network.DummyLBNetworkProvider())
+			verifyOutput()
+			vmService := getVirtualMachineService(ctx, clusterCtx, c, cpService)
+			Expect(vmService.Name).To(Equal(controlPlaneVMServiceName(clusterCtx.Cluster.Name)))
+
+			// Delete the apiservice and recreate using the legacy name and reconcile again.
+			Expect(c.Delete(ctx, vmService.DeepCopy())).To(Succeed())
+			vmService.Name = legacyControlPlaneVMServiceName(clusterCtx.Cluster.Name)
+			vmService.ResourceVersion = ""
+			Expect(c.Create(ctx, vmService)).To(Succeed())
+			updateVMServiceWithVIP(ctx, clusterCtx, c, cpService, vip)
+			apiEndpoint, err = cpService.ReconcileControlPlaneEndpointService(ctx, clusterCtx, network.DummyLBNetworkProvider())
+			verifyOutput()
+			vmService = getVirtualMachineService(ctx, clusterCtx, c, cpService)
+			Expect(vmService.Name).To(Equal(legacyControlPlaneVMServiceName(clusterCtx.Cluster.Name)))
 		})
 
 		Specify("Reconcile VirtualMachineService for NetOp", func() {