[fix] [improvement] move apiserver loadbalancing logic from linodemachine controller to linodecluster controller

cloud/services/domains.go

@@ -3,7 +3,6 @@
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"net"
 	"net/netip"
@@ -42,7 +41,7 @@
 	}
 
 	if len(dnsEntries) == 0 {
-		return errors.New("dnsEntries are empty")
+		return nil
 	}
 
 	if cscope.LinodeCluster.Spec.Network.DNSProvider == "akamai" {
@@ -93,13 +92,13 @@
 		}
 		// Record was not found - if operation is not "create", nothing to do
 		if operation != "create" {
 			return nil
 		}
 		// Create record
 		return createAkamaiEntry(ctx, akaDNSClient, dnsEntry, fqdn, rootDomain)
 	}
 	if recordBody == nil {
 		return fmt.Errorf("akamai dns returned empty dns record")
 	}
 
 	// if operation is delete and we got the record, delete it
@@ -108,20 +107,20 @@
 	}
 	// if operation is create and we got the record, update it
 	// Check if the target already exists in the target list
 	for _, target := range recordBody.Target {
 		if recordBody.RecordType == "TXT" {
 			if strings.Contains(target, dnsEntry.Target) {
 				return nil
 			}
 		} else {
 			if slices.Equal(net.ParseIP(target), net.ParseIP(dnsEntry.Target)) {
 				return nil
 			}
 		}
 	}
 	// Target doesn't exist so lets append it to the existing list and update it
 	recordBody.Target = append(recordBody.Target, dnsEntry.Target)
 	return akaDNSClient.UpdateRecord(ctx, recordBody, rootDomain)
 }
 
 func createAkamaiEntry(ctx context.Context, client clients.AkamClient, dnsEntry DNSOptions, fqdn, rootDomain string) error {
@@ -139,14 +138,14 @@
 
 func deleteAkamaiEntry(ctx context.Context, client clients.AkamClient, recordBody *dns.RecordBody, dnsEntry DNSOptions, rootDomain string) error {
 	switch {
 	case len(recordBody.Target) > 1:
 		recordBody.Target = removeElement(
 			recordBody.Target,
 			// Linode DNS API formats the IPv6 IPs using :: for :0:0: while the address from the LinodeMachine status keeps it as is
 			// So we need to match that
 			strings.Replace(dnsEntry.Target, "::", ":0:0:", 8), //nolint:mnd // 8 for 8 octest
 		)
 		return client.UpdateRecord(ctx, recordBody, rootDomain)
 	default:
 		return client.DeleteRecord(ctx, recordBody, rootDomain)
 	}
@@ -177,11 +176,11 @@
 		for _, IPs := range eachMachine.Status.Addresses {
 			recordType := linodego.RecordTypeA
 			if IPs.Type != v1beta1.MachineExternalIP {
 				continue
 			}
 			addr, err := netip.ParseAddr(IPs.Address)
 			if err != nil {
 				return nil, fmt.Errorf("not a valid IP %w", err)
 			}
 			if !addr.Is4() {
 				recordType = linodego.RecordTypeAAAA
@@ -291,7 +290,7 @@
 
 	// If record exists, update it
 	if len(domainRecords) == 0 {
 		return false, fmt.Errorf("no txt record %s found with value %s for machine %s", hostname, cscope.LinodeCluster.Name, cscope.LinodeCluster.Name)
 	}
 
 	return true, nil
@@ -299,7 +298,7 @@
 
 func getSubDomain(cscope *scope.ClusterScope) (subDomain string) {
 	if cscope.LinodeCluster.Spec.Network.DNSSubDomainOverride != "" {
 		subDomain = cscope.LinodeCluster.Spec.Network.DNSSubDomainOverride
 	} else {
 		uniqueID := ""
 		if cscope.LinodeCluster.Spec.Network.DNSUniqueIdentifier != "" {

cloud/services/domains_test.go

@@ -732,7 +732,7 @@ func TestAddIPToDNS(t *testing.T) {
 					},
 				}, nil).AnyTimes()
 			},
-			expectedError: fmt.Errorf("dnsEntries are empty"),
+			expectedError: nil,
 			expectK8sClient: func(mockK8sClient *mock.MockK8sClient) {
 				mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes()
 			},
@@ -981,7 +981,7 @@ func TestDeleteIPFromDNS(t *testing.T) {
 			},
 		},
 		{
-			name: "Error - failed to get machine ip",
+			name: "Error - failed to get machine",
 			clusterScope: &scope.ClusterScope{
 				Cluster: &clusterv1.Cluster{
 					ObjectMeta: metav1.ObjectMeta{
@@ -1002,23 +1002,9 @@ func TestDeleteIPFromDNS(t *testing.T) {
 						},
 					},
 				},
-				LinodeMachines: infrav1alpha2.LinodeMachineList{
-					Items: []infrav1alpha2.LinodeMachine{
-						{
-							ObjectMeta: metav1.ObjectMeta{
-								Name: "test-machine",
-								UID:  "test-uid",
-							},
-							Spec: infrav1alpha2.LinodeMachineSpec{
-								ProviderID: ptr.To("linode://123"),
-								InstanceID: ptr.To(123),
-							},
-						},
-					},
-				},
 			},
 			expects:       func(mockClient *mock.MockLinodeClient) {},
-			expectedError: fmt.Errorf("dnsEntries are empty"),
+			expectedError: nil,
 			expectK8sClient: func(mockK8sClient *mock.MockK8sClient) {
 				mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes()
 			},

controller/linodecluster_controller.go

@@ -99,12 +99,12 @@
 	clusterScope, err := scope.NewClusterScope(
 		ctx,
 		r.LinodeClientConfig,
 		r.DnsClientConfig,
 		scope.ClusterScopeParams{
 			Client:            r.TracedClient(),
 			Cluster:           cluster,
 			LinodeCluster:     linodeCluster,
 			LinodeMachineList: infrav1alpha2.LinodeMachineList{},
 		},
 	)
 
@@ -141,7 +141,7 @@
 		clusterv1.MachineControlPlaneLabel: "",
 	}
 	if err := r.TracedClient().List(ctx, &clusterScope.LinodeMachines, client.InNamespace(clusterScope.LinodeCluster.Namespace), client.MatchingLabels(labels)); err != nil {
 		return res, err
 	}
 
 	// Handle deleted clusters
@@ -177,15 +177,15 @@
 	conditions.MarkTrue(clusterScope.LinodeCluster, clusterv1.ReadyCondition)
 
 	for _, eachMachine := range clusterScope.LinodeMachines.Items {
 		if len(eachMachine.Status.Addresses) == 0 {
-			return res, nil
+			return res, fmt.Errorf("no public ips set for the linodemachine %s", eachMachine.Name)
 		}
 	}
 
 	err := r.addMachineToLB(ctx, clusterScope)
 	if err != nil {
 		logger.Error(err, "Failed to add Linode machine to loadbalancer option")
 		return retryIfTransient(err)
 	}
 	conditions.MarkTrue(clusterScope.LinodeCluster, ConditionLoadBalancing)
 
@@ -301,7 +301,7 @@
 	}
 
 	if err := r.removeMachineFromLB(ctx, logger, clusterScope); err != nil {
 		return fmt.Errorf("remove machine from loadbalancer: %w", err)
 	}
 	conditions.MarkFalse(clusterScope.LinodeCluster, ConditionLoadBalancing, "cleared loadbalancer", clusterv1.ConditionSeverityInfo, "")
 
@@ -351,10 +351,10 @@
 				kutil.ClusterToInfrastructureMapFunc(context.TODO(), infrav1alpha2.GroupVersion.WithKind("LinodeCluster"), mgr.GetClient(), &infrav1alpha2.LinodeCluster{}),
 			),
 			builder.WithPredicates(predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger())),
 		).
 		Watches(
 			&infrav1alpha2.LinodeMachine{},
 			handler.EnqueueRequestsFromMapFunc(r.linodeMachineToLinodeCluster(mgr.GetLogger())),
 		).Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
 	if err != nil {
 		return fmt.Errorf("failed to build controller: %w", err)
@@ -367,44 +367,44 @@
 	return wrappedruntimeclient.NewRuntimeClientWithTracing(r.Client, wrappedruntimereconciler.DefaultDecorator())
 }
 
 func (r *LinodeClusterReconciler) linodeMachineToLinodeCluster(logger logr.Logger) handler.MapFunc {
 	logger = logger.WithName("LinodeClusterReconciler").WithName("linodeMachineToLinodeCluster")
 
 	return func(ctx context.Context, o client.Object) []ctrl.Request {
 		ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultMappingTimeout)
 		defer cancel()
 
 		linodeMachine, ok := o.(*infrav1alpha2.LinodeMachine)
 		if !ok {
 			logger.Info("Failed to cast object to LinodeMachine")
 			return nil
 		}
 
 		// We only need control plane machines to trigger reconciliation
 		if !strings.Contains(linodeMachine.Name, "control-plane") {
 			return nil
 		}
 
 		linodeCluster := infrav1alpha2.LinodeCluster{}
 		if err := r.TracedClient().Get(
 			ctx,
 			types.NamespacedName{
 				Name:      linodeMachine.ObjectMeta.Labels[clusterv1.ClusterNameLabel],
 				Namespace: linodeMachine.Namespace,
 			},
 			&linodeCluster); err != nil {
 			logger.Info("Failed to get LinodeCluster")
 			return nil
 		}
 
 		result := make([]ctrl.Request, 0, 1)
 		result = append(result, ctrl.Request{
 			NamespacedName: client.ObjectKey{
 				Namespace: linodeCluster.Namespace,
 				Name:      linodeCluster.Name,
 			},
 		})
 
 		return result
 	}
 }

controller/linodecluster_controller_test.go

@@ -348,7 +348,7 @@ var _ = Describe("cluster-lifecycle-dns", Ordered, Label("cluster", "cluster-lif
 					clusterKey := client.ObjectKeyFromObject(&linodeCluster)
 					Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed())
 					Expect(linodeCluster.Status.Ready).To(BeTrue())
-					Expect(linodeCluster.Status.Conditions).To(HaveLen(1))
+					Expect(linodeCluster.Status.Conditions).To(HaveLen(2))
 					Expect(linodeCluster.Status.Conditions[0].Type).To(Equal(clusterv1.ReadyCondition))
 
 					By("checking controlPlaneEndpoint/NB host and port")
@@ -479,7 +479,7 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid
 			},
 		},
 	}
-	linodeMachine := infrav1alpha2.LinodeMachine{
+	linodeMachineWithAddress := infrav1alpha2.LinodeMachine{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "test-machine",
 			Namespace: defaultNamespace,
@@ -501,8 +501,18 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid
 			},
 		},
 	}
-	linodeMachines := infrav1alpha2.LinodeMachineList{
-		Items: []infrav1alpha2.LinodeMachine{linodeMachine},
+	linodeMachineWithNoAddress := infrav1alpha2.LinodeMachine{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-machine",
+			Namespace: defaultNamespace,
+		},
+		Spec: infrav1alpha2.LinodeMachineSpec{
+			ProviderID: ptr.To("linode://123"),
+			InstanceID: ptr.To(123),
+		},
+		Status: infrav1alpha2.LinodeMachineStatus{
+			Addresses: []clusterv1.MachineAddress{},
+		},
 	}
 
 	ctlrSuite := NewControllerSuite(GinkgoT(), mock.MockLinodeClient{})
@@ -514,7 +524,6 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid
 		cScope.Client = k8sClient
 		Expect(k8sClient.Create(ctx, &cluster)).To(Succeed())
 		Expect(k8sClient.Create(ctx, &linodeCluster)).To(Succeed())
-		Expect(k8sClient.Create(ctx, &linodeMachine)).To(Succeed())
 	})
 
 	ctlrSuite.BeforeEach(func(ctx context.Context, mck Mock) {
@@ -523,7 +532,6 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid
 		Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed())
 		cScope.Cluster = &cluster
 		cScope.LinodeCluster = &linodeCluster
-		cScope.LinodeMachines = linodeMachines
 
 		// Create patch helper with latest state of resource.
 		// This is only needed when relying on envtest's k8sClient.
@@ -539,6 +547,11 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid
 					cScope.LinodeClient = mck.LinodeClient
 					cScope.LinodeDomainsClient = mck.LinodeClient
 					cScope.AkamaiDomainsClient = mck.AkamEdgeDNSClient
+					linodeMachines := infrav1alpha2.LinodeMachineList{
+						Items: []infrav1alpha2.LinodeMachine{linodeMachineWithAddress},
+					}
+					Expect(k8sClient.Create(ctx, &linodeMachineWithAddress)).To(Succeed())
+					cScope.LinodeMachines = linodeMachines
 				}),
 				Result("cluster created", func(ctx context.Context, mck Mock) {
 					reconciler.Client = k8sClient
@@ -549,12 +562,44 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid
 					clusterKey := client.ObjectKeyFromObject(&linodeCluster)
 					Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed())
 					Expect(linodeCluster.Status.Ready).To(BeTrue())
-					Expect(linodeCluster.Status.Conditions).To(HaveLen(1))
+					Expect(linodeCluster.Status.Conditions).To(HaveLen(2))
+					Expect(linodeCluster.Status.Conditions[0].Type).To(Equal(clusterv1.ReadyCondition))
+
+					By("checking controlPlaneEndpoint/NB host and port")
+					Expect(linodeCluster.Spec.ControlPlaneEndpoint.Host).To(Equal(controlPlaneEndpointHost))
+					Expect(linodeCluster.Spec.ControlPlaneEndpoint.Port).To(Equal(int32(controlPlaneEndpointPort)))
+					Expect(k8sClient.Delete(ctx, &linodeMachineWithAddress)).To(Succeed())
+					cScope.LinodeMachines = infrav1alpha2.LinodeMachineList{}
+				}),
+			),
+			Path(
+				Call("no linodemachines available", func(ctx context.Context, mck Mock) {
+					cScope.LinodeClient = mck.LinodeClient
+					cScope.LinodeDomainsClient = mck.LinodeClient
+					cScope.AkamaiDomainsClient = mck.AkamEdgeDNSClient
+					linodeMachines := infrav1alpha2.LinodeMachineList{
+						Items: []infrav1alpha2.LinodeMachine{linodeMachineWithNoAddress},
+					}
+					Expect(k8sClient.Create(ctx, &linodeMachineWithNoAddress)).To(Succeed())
+					cScope.LinodeMachines = linodeMachines
+				}),
+				Result("cluster not created", func(ctx context.Context, mck Mock) {
+					reconciler.Client = k8sClient
+					_, err := reconciler.reconcile(ctx, cScope, logr.Logger{})
+					Expect(err).NotTo(HaveOccurred())
+
+					By("checking ready conditions")
+					clusterKey := client.ObjectKeyFromObject(&linodeCluster)
+					Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed())
+					Expect(linodeCluster.Status.Ready).To(BeTrue())
+					Expect(linodeCluster.Status.Conditions).To(HaveLen(2))
 					Expect(linodeCluster.Status.Conditions[0].Type).To(Equal(clusterv1.ReadyCondition))
 
 					By("checking controlPlaneEndpoint/NB host and port")
 					Expect(linodeCluster.Spec.ControlPlaneEndpoint.Host).To(Equal(controlPlaneEndpointHost))
 					Expect(linodeCluster.Spec.ControlPlaneEndpoint.Port).To(Equal(int32(controlPlaneEndpointPort)))
+					Expect(k8sClient.Delete(ctx, &linodeMachineWithNoAddress)).To(Succeed())
+					cScope.LinodeMachines = infrav1alpha2.LinodeMachineList{}
 				}),
 			),
 		),

e2e/linodemachine-controller/minimal-linodemachine/chainsaw-test.yaml

@@ -69,6 +69,11 @@ spec:
                 results: 1
     - name: Delete Cluster resource
       try:
+        - delete:
+            ref:
+              apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
+              kind: LinodeCluster
+              name: ($cluster)
         - delete:
             ref:
               apiVersion: cluster.x-k8s.io/v1beta1

e2e/linodemachine-controller/vpc-integration/chainsaw-test.yaml

@@ -106,6 +106,11 @@ spec:
                               - active: true
     - name: Delete the Cluster & LinodeVPC resource
       try:
+        - delete:
+            ref:
+              apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
+              kind: LinodeCluster
+              name: ($cluster)
         - delete:
             ref:
               apiVersion: cluster.x-k8s.io/v1beta1