[fix] make VLAN IPs unique across controller pod restarts

controller/linodemachine_controller_helpers.go

@@ -132,7 +132,10 @@
 
 	// if vlan is enabled, attach additional interface as eth0 to linode
 	if machineScope.LinodeCluster.Spec.Network.UseVlan {
-		iface := getVlanInterfaceConfig(machineScope, logger)
+		iface, err := getVlanInterfaceConfig(ctx, machineScope, logger)
+		if err != nil {
+			return nil, err
+		}
 		if iface != nil {
 			// add VLAN interface as first interface
 			createConfig.Interfaces = slices.Insert(createConfig.Interfaces, 0, *iface)
@@ -358,18 +361,21 @@
 	return *linodeFirewall.Spec.FirewallID, nil
 }
 
-func getVlanInterfaceConfig(machineScope *scope.MachineScope, logger logr.Logger) *linodego.InstanceConfigInterfaceCreateOptions {
+func getVlanInterfaceConfig(ctx context.Context, machineScope *scope.MachineScope, logger logr.Logger) (*linodego.InstanceConfigInterfaceCreateOptions, error) {
 	logger = logger.WithValues("vlanName", machineScope.Cluster.Name)
 
 	// Try to obtain a IP for the machine using its name
-	ip := util.GetNextVlanIP(machineScope.Cluster.Name, machineScope.Cluster.Namespace)
-	logger.Info("obtained IP for machine", "name", machineScope.LinodeMachine.Name, "ip", ip)
+	ip, err := util.GetNextVlanIP(ctx, machineScope.Cluster.Name, machineScope.Cluster.Namespace, machineScope.Client)
+	if err != nil {
+		return nil, fmt.Errorf("getting vlanIP: %w", err)
+	}
 
+	logger.Info("obtained IP for machine", "name", machineScope.LinodeMachine.Name, "ip", ip)
 	return &linodego.InstanceConfigInterfaceCreateOptions{
 		Purpose:     linodego.InterfacePurposeVLAN,
 		Label:       machineScope.Cluster.Name,
 		IPAMAddress: fmt.Sprintf(vlanIPFormat, ip),
-	}
+	}, nil
 }
 
 func getVPCInterfaceConfig(ctx context.Context, machineScope *scope.MachineScope, interfaces []linodego.InstanceConfigInterfaceCreateOptions, logger logr.Logger) (*linodego.InstanceConfigInterfaceCreateOptions, error) {

util/vlanips.go

@@ -17,10 +17,19 @@
 package util
 
 import (
+	"context"
 	"fmt"
+	"net"
 	"net/netip"
 	"slices"
 	"sync"
+
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/selection"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/linode/cluster-api-provider-linode/api/v1alpha2"
 )
 
 var (
@@ -35,16 +44,52 @@
 	ips []string
 }
 
-func getClusterIPs(key string) *ClusterIPs {
+func getExistingIPsForCluster(ctx context.Context, clusterName, namespace string, kubeclient client.Client) ([]string, error) {
+	clusterReq, err := labels.NewRequirement("cluster.x-k8s.io/cluster-name", selection.Equals, []string{clusterName})
+	if err != nil {
+		return nil, fmt.Errorf("building label selector: %w", err)
+	}
+
+	selector := labels.NewSelector()
+	selector = selector.Add(*clusterReq)
+	var linodeMachineList v1alpha2.LinodeMachineList
+	err = kubeclient.List(ctx, &linodeMachineList, &client.ListOptions{Namespace: namespace, LabelSelector: selector})
+	if err != nil {
+		return nil, fmt.Errorf("listing all linodeMachines %w", err)
+	}
+
+	_, ipnet, err := net.ParseCIDR(vlanIPRange)
+	if err != nil {
+		return nil, fmt.Errorf("parsing vlanIPRange: %w", err)
+	}
+
+	existingIPs := []string{}
+	for _, lm := range linodeMachineList.Items {
+		for _, addr := range lm.Status.Addresses {
+			if addr.Type == clusterv1.MachineInternalIP && ipnet.Contains(net.ParseIP(addr.Address)) {
+				existingIPs = append(existingIPs, addr.Address)
+			}
+		}
+	}
+	return existingIPs, nil
+}
+
+func getClusterIPs(ctx context.Context, clusterName, namespace string, kubeclient client.Client) (*ClusterIPs, error) {
+	key := fmt.Sprintf("%s.%s", namespace, clusterName)
 	vlanIPsMu.Lock()
 	defer vlanIPsMu.Unlock()
-	ips, exists := vlanIPsMap[key]
+	clusterIps, exists := vlanIPsMap[key]
 	if !exists {
-		ips = &ClusterIPs{
-			ips: []string{},
+		ips, err := getExistingIPsForCluster(ctx, clusterName, namespace, kubeclient)
+		if err != nil {
+			return nil, fmt.Errorf("getting existingIPs for a cluster: %w", err)
+		}
+		clusterIps = &ClusterIPs{
+			ips: ips,
 		}
+		vlanIPsMap[key] = clusterIps
 	}
-	return ips
+	return clusterIps, nil
 }
 
 func (c *ClusterIPs) getNextIP() string {
@@ -66,10 +111,12 @@
 }
 
 // GetNextVlanIP returns the next available IP for a cluster
-func GetNextVlanIP(clusterName, namespace string) string {
-	key := fmt.Sprintf("%s.%s", namespace, clusterName)
-	clusterIPs := getClusterIPs(key)
-	return clusterIPs.getNextIP()
+func GetNextVlanIP(ctx context.Context, clusterName, namespace string, kubeclient client.Client) (string, error) {
+	clusterIPs, err := getClusterIPs(ctx, clusterName, namespace, kubeclient)
+	if err != nil {
+		return "", err
+	}
+	return clusterIPs.getNextIP(), nil
 }
 
 func DeleteClusterIPs(clusterName, namespace string) {

util/vlanips_test.go

@@ -17,8 +17,13 @@ limitations under the License.
 package util
 
 import (
+	"context"
 	"reflect"
 	"testing"
+
+	"go.uber.org/mock/gomock"
+
+	"github.com/linode/cluster-api-provider-linode/mock"
 )
 
 func TestGetNextVlanIP(t *testing.T) {
@@ -28,27 +33,42 @@ func TestGetNextVlanIP(t *testing.T) {
 		clusterName      string
 		clusterNamespace string
 		want             string
+		expects          func(mock *mock.MockK8sClient)
 	}{
 		{
 			name:             "provide key which exists in map",
 			clusterName:      "test",
 			clusterNamespace: "testna",
 			want:             "10.0.0.3",
+			expects: func(mock *mock.MockK8sClient) {
+			},
 		},
 		{
 			name:             "provide key which doesn't exist",
 			clusterName:      "test",
 			clusterNamespace: "testnonexistent",
 			want:             "10.0.0.1",
+			expects: func(mock *mock.MockK8sClient) {
+				mock.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).MinTimes(1)
+			},
 		},
 	}
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockK8sClient := mock.NewMockK8sClient(ctrl)
+
 	for _, tt := range tests {
 		vlanIPsMap["testna.test"] = &ClusterIPs{
 			ips: []string{"10.0.0.1", "10.0.0.2"},
 		}
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			if got := GetNextVlanIP(tt.clusterName, tt.clusterNamespace); !reflect.DeepEqual(got, tt.want) {
+			tt.expects(mockK8sClient)
+			got, err := GetNextVlanIP(context.Background(), tt.clusterName, tt.clusterNamespace, mockK8sClient)
+			if err != nil {
+				t.Error("error")
+			}
+			if !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("GetNextVlanIP() = %v, want %v", got, tt.want)
 			}
 		})