[improvement][breaking] : allow auto-allocation of NB backend ips (linode#391)

* allow auto-allocation of NB backend ips
* add priority for annotations and add unittests
* fix couple of e2e tests and use latest linodego
* address review comments and fix some failing tests
* use  var and also increase timeout for certain steps
* add flag to disable nb-vpc integration, add docs for newly added flags
* address review comments
* move repeated code to separate script, address review comments
* use subnet-id specified in flag if its set

Author: rahulait

cloud/annotations/annotations.go

@@ -47,4 +47,5 @@ const (
 
 	NodeBalancerBackendVPCName    = "service.beta.kubernetes.io/linode-loadbalancer-backend-vpc-name"
 	NodeBalancerBackendSubnetName = "service.beta.kubernetes.io/linode-loadbalancer-backend-subnet-name"
+	NodeBalancerBackendSubnetID   = "service.beta.kubernetes.io/linode-loadbalancer-backend-subnet-id"
 )

cloud/linode/cloud.go

@@ -39,22 +39,25 @@ var Options struct {
 	EnableRouteController    bool
 	EnableTokenHealthChecker bool
 	// Deprecated: use VPCNames instead
-	VPCName                       string
-	VPCNames                      string
-	SubnetNames                   string
-	LoadBalancerType              string
-	BGPNodeSelector               string
-	IpHolderSuffix                string
-	LinodeExternalNetwork         *net.IPNet
-	NodeBalancerTags              []string
-	DefaultNBType                 string
-	NodeBalancerBackendIPv4Subnet string
-	GlobalStopChannel             chan<- struct{}
-	EnableIPv6ForLoadBalancers    bool
-	AllocateNodeCIDRs             bool
-	ClusterCIDRIPv4               string
-	NodeCIDRMaskSizeIPv4          int
-	NodeCIDRMaskSizeIPv6          int
+	VPCName                           string
+	VPCNames                          string
+	SubnetNames                       string
+	LoadBalancerType                  string
+	BGPNodeSelector                   string
+	IpHolderSuffix                    string
+	LinodeExternalNetwork             *net.IPNet
+	NodeBalancerTags                  []string
+	DefaultNBType                     string
+	NodeBalancerBackendIPv4Subnet     string
+	NodeBalancerBackendIPv4SubnetID   int
+	NodeBalancerBackendIPv4SubnetName string
+	DisableNodeBalancerVPCBackends    bool
+	GlobalStopChannel                 chan<- struct{}
+	EnableIPv6ForLoadBalancers        bool
+	AllocateNodeCIDRs                 bool
+	ClusterCIDRIPv4                   string
+	NodeCIDRMaskSizeIPv4              int
+	NodeCIDRMaskSizeIPv6              int
 }
 
 type linodeCloud struct {
@@ -150,6 +153,22 @@ func newCloud() (cloudprovider.Interface, error) {
 		Options.SubnetNames = ""
 	}
 
+	if Options.NodeBalancerBackendIPv4SubnetID != 0 && Options.NodeBalancerBackendIPv4SubnetName != "" {
+		return nil, fmt.Errorf("cannot have both --nodebalancer-backend-ipv4-subnet-id and --nodebalancer-backend-ipv4-subnet-name set")
+	}
+
+	if Options.DisableNodeBalancerVPCBackends {
+		klog.Infof("NodeBalancer VPC backends are disabled, no VPC backends will be created for NodeBalancers")
+		Options.NodeBalancerBackendIPv4SubnetID = 0
+		Options.NodeBalancerBackendIPv4SubnetName = ""
+	} else if Options.NodeBalancerBackendIPv4SubnetName != "" {
+		Options.NodeBalancerBackendIPv4SubnetID, err = getNodeBalancerBackendIPv4SubnetID(linodeClient)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get backend IPv4 subnet ID for subnet name %s: %w", Options.NodeBalancerBackendIPv4SubnetName, err)
+		}
+		klog.Infof("Using NodeBalancer backend IPv4 subnet ID %d for subnet name %s", Options.NodeBalancerBackendIPv4SubnetID, Options.NodeBalancerBackendIPv4SubnetName)
+	}
+
 	instanceCache = newInstances(linodeClient)
 	routes, err := newRoutes(linodeClient, instanceCache)
 	if err != nil {

cloud/linode/cloud_test.go

@@ -80,13 +80,35 @@ func TestNewCloud(t *testing.T) {
 		assert.Equal(t, "tt", Options.VPCNames, "expected vpcnames to be set to vpcname")
 	})
 
+	t.Run("should fail if both nodeBalancerBackendIPv4SubnetID and nodeBalancerBackendIPv4SubnetName are set", func(t *testing.T) {
+		Options.VPCNames = "tt"
+		Options.NodeBalancerBackendIPv4SubnetID = 12345
+		Options.NodeBalancerBackendIPv4SubnetName = "test-subnet"
+		defer func() {
+			Options.VPCNames = ""
+			Options.NodeBalancerBackendIPv4SubnetID = 0
+			Options.NodeBalancerBackendIPv4SubnetName = ""
+		}()
+		_, err := newCloud()
+		assert.Error(t, err, "expected error when both nodeBalancerBackendIPv4SubnetID and nodeBalancerBackendIPv4SubnetName are set")
+	})
+
 	t.Run("should fail if incorrect loadbalancertype is set", func(t *testing.T) {
 		rtEnabled := Options.EnableRouteController
 		Options.EnableRouteController = false
 		Options.LoadBalancerType = "test"
+		Options.VPCNames = "vpc-test1,vpc-test2"
+		Options.NodeBalancerBackendIPv4SubnetName = "t1"
+		vpcIDs = map[string]int{"vpc-test1": 1, "vpc-test2": 2, "vpc-test3": 3}
+		subnetIDs = map[string]int{"t1": 1, "t2": 2, "t3": 3}
 		defer func() {
 			Options.LoadBalancerType = ""
 			Options.EnableRouteController = rtEnabled
+			Options.VPCNames = ""
+			Options.NodeBalancerBackendIPv4SubnetID = 0
+			Options.NodeBalancerBackendIPv4SubnetName = ""
+			vpcIDs = map[string]int{}
+			subnetIDs = map[string]int{}
 		}()
 		_, err := newCloud()
 		assert.Error(t, err, "expected error if incorrect loadbalancertype is set")

cloud/linode/loadbalancers.go

@@ -444,12 +444,16 @@ func (l *loadbalancers) updateNodeBalancer(
 		// Add all of the Nodes to the config
 		newNBNodes := make([]linodego.NodeBalancerConfigRebuildNodeOptions, 0, len(nodes))
 		subnetID := 0
+		if Options.NodeBalancerBackendIPv4SubnetID != 0 {
+			subnetID = Options.NodeBalancerBackendIPv4SubnetID
+		}
 		backendIPv4Range, ok := service.GetAnnotations()[annotations.NodeBalancerBackendIPv4Range]
 		if ok {
 			if err = validateNodeBalancerBackendIPv4Range(backendIPv4Range); err != nil {
 				return err
 			}
-
+		}
+		if Options.VPCNames != "" && !Options.DisableNodeBalancerVPCBackends {
 			var id int
 			id, err = l.getSubnetIDForSVC(ctx, service)
 			if err != nil {
@@ -717,6 +721,86 @@ func (l *loadbalancers) GetLinodeNBType(service *v1.Service) linodego.NodeBalanc
 	return linodego.NodeBalancerPlanType(Options.DefaultNBType)
 }
 
+// getVPCCreateOptions returns the VPC options for the NodeBalancer creation.
+// Order of precedence:
+// 1. NodeBalancerBackendIPv4Range annotation
+// 2. NodeBalancerBackendVPCName and NodeBalancerBackendSubnetName annotation
+// 3. NodeBalancerBackendIPv4SubnetID/NodeBalancerBackendIPv4SubnetName flag
+// 4. NodeBalancerBackendIPv4Subnet flag
+// 5. Default to using the subnet ID of the service's VPC
+func (l *loadbalancers) getVPCCreateOptions(ctx context.Context, service *v1.Service) ([]linodego.NodeBalancerVPCOptions, error) {
+	// Evaluate subnetID based on annotations or flags
+	subnetID, err := l.getSubnetIDForSVC(ctx, service)
+	if err != nil {
+		return nil, err
+	}
+
+	// Precedence 1: If the user has specified a NodeBalancerBackendIPv4Range, use that
+	backendIPv4Range, ok := service.GetAnnotations()[annotations.NodeBalancerBackendIPv4Range]
+	if ok {
+		if err := validateNodeBalancerBackendIPv4Range(backendIPv4Range); err != nil {
+			return nil, err
+		}
+		// If the user has specified a NodeBalancerBackendIPv4Range, use that
+		// for the NodeBalancer backend ipv4 range
+		if backendIPv4Range != "" {
+			vpcCreateOpts := []linodego.NodeBalancerVPCOptions{
+				{
+					SubnetID:  subnetID,
+					IPv4Range: backendIPv4Range,
+				},
+			}
+			return vpcCreateOpts, nil
+		}
+	}
+
+	// Precedence 2: If the user wants to overwrite the default VPC name or subnet name
+	// and have specified it in the annotations, use it to set subnetID
+	// and auto-allocate subnets from it for the NodeBalancer
+	_, vpcInAnnotation := service.GetAnnotations()[annotations.NodeBalancerBackendVPCName]
+	_, subnetInAnnotation := service.GetAnnotations()[annotations.NodeBalancerBackendSubnetName]
+	if vpcInAnnotation || subnetInAnnotation {
+		vpcCreateOpts := []linodego.NodeBalancerVPCOptions{
+			{
+				SubnetID: subnetID,
+			},
+		}
+		return vpcCreateOpts, nil
+	}
+
+	// Precedence 3: If the user has specified a NodeBalancerBackendIPv4SubnetID, use that
+	// and auto-allocate subnets from it for the NodeBalancer
+	if Options.NodeBalancerBackendIPv4SubnetID != 0 {
+		vpcCreateOpts := []linodego.NodeBalancerVPCOptions{
+			{
+				SubnetID: Options.NodeBalancerBackendIPv4SubnetID,
+			},
+		}
+		return vpcCreateOpts, nil
+	}
+
+	// Precedence 4: If the user has specified a NodeBalancerBackendIPv4Subnet, use that
+	// and auto-allocate subnets from it for the NodeBalancer
+	if Options.NodeBalancerBackendIPv4Subnet != "" {
+		vpcCreateOpts := []linodego.NodeBalancerVPCOptions{
+			{
+				SubnetID:            subnetID,
+				IPv4Range:           Options.NodeBalancerBackendIPv4Subnet,
+				IPv4RangeAutoAssign: true,
+			},
+		}
+		return vpcCreateOpts, nil
+	}
+
+	// Default to using the subnet ID of the service's VPC
+	vpcCreateOpts := []linodego.NodeBalancerVPCOptions{
+		{
+			SubnetID: subnetID,
+		},
+	}
+	return vpcCreateOpts, nil
+}
+
 func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName string, service *v1.Service, configs []*linodego.NodeBalancerConfigCreateOptions) (lb *linodego.NodeBalancer, err error) {
 	connThrottle := getConnectionThrottle(service)
 
@@ -732,21 +816,11 @@ func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName stri
 		Type:               nbType,
 	}
 
-	backendIPv4Range, ok := service.GetAnnotations()[annotations.NodeBalancerBackendIPv4Range]
-	if ok {
-		if err := validateNodeBalancerBackendIPv4Range(backendIPv4Range); err != nil {
-			return nil, err
-		}
-		subnetID, err := l.getSubnetIDForSVC(ctx, service)
+	if Options.VPCNames != "" && !Options.DisableNodeBalancerVPCBackends {
+		createOpts.VPCs, err = l.getVPCCreateOptions(ctx, service)
 		if err != nil {
 			return nil, err
 		}
-		createOpts.VPCs = []linodego.NodeBalancerVPCOptions{
-			{
-				SubnetID:  subnetID,
-				IPv4Range: backendIPv4Range,
-			},
-		}
 	}
 
 	fwid, ok := service.GetAnnotations()[annotations.AnnLinodeCloudFirewallID]
@@ -875,25 +949,49 @@ func (l *loadbalancers) addTLSCert(ctx context.Context, service *v1.Service, nbC
 	return nil
 }
 
-// getSubnetIDForSVC returns the subnet ID for the service's VPC and subnet.
-// By default, first VPCName and SubnetName are used to calculate subnet id for the service.
-// If the service has annotations specifying VPCName and SubnetName, they are used instead.
+// getSubnetIDForSVC returns the subnet ID for the service when running within VPC.
+// Following precedence rules are applied:
+// 1. If the service has an annotation for NodeBalancerBackendSubnetID, use that.
+// 2. If the service has annotations specifying VPCName or SubnetName, use them.
+// 3. If CCM is configured with --nodebalancer-backend-ipv4-subnet-id, it will be used as the subnet ID.
+// 4. Else, use first VPCName and SubnetName to calculate subnet id for the service.
 func (l *loadbalancers) getSubnetIDForSVC(ctx context.Context, service *v1.Service) (int, error) {
 	if Options.VPCNames == "" {
 		return 0, fmt.Errorf("CCM not configured with VPC, cannot create NodeBalancer with specified annotation")
 	}
+	// Check if the service has an annotation for NodeBalancerBackendSubnetID
+	if specifiedSubnetID, ok := service.GetAnnotations()[annotations.NodeBalancerBackendSubnetID]; ok {
+		subnetID, err := strconv.Atoi(specifiedSubnetID)
+		if err != nil {
+			return 0, err
+		}
+		return subnetID, nil
+	}
+
+	specifiedVPCName, vpcOk := service.GetAnnotations()[annotations.NodeBalancerBackendVPCName]
+	specifiedSubnetName, subnetOk := service.GetAnnotations()[annotations.NodeBalancerBackendSubnetName]
+
+	// If no VPCName or SubnetName is specified in annotations, but NodeBalancerBackendIPv4SubnetID is set,
+	// use the NodeBalancerBackendIPv4SubnetID as the subnet ID.
+	if !vpcOk && !subnetOk && Options.NodeBalancerBackendIPv4SubnetID != 0 {
+		return Options.NodeBalancerBackendIPv4SubnetID, nil
+	}
+
 	vpcName := strings.Split(Options.VPCNames, ",")[0]
-	if specifiedVPCName, ok := service.GetAnnotations()[annotations.NodeBalancerBackendVPCName]; ok {
+	if vpcOk {
 		vpcName = specifiedVPCName
 	}
 	vpcID, err := GetVPCID(ctx, l.client, vpcName)
 	if err != nil {
 		return 0, err
 	}
+
 	subnetName := strings.Split(Options.SubnetNames, ",")[0]
-	if specifiedSubnetName, ok := service.GetAnnotations()[annotations.NodeBalancerBackendSubnetName]; ok {
+	if subnetOk {
 		subnetName = specifiedSubnetName
 	}
+
+	// Use the VPC ID and Subnet Name to get the subnet ID
 	return GetSubnetID(ctx, l.client, vpcID, subnetName)
 }
 
@@ -907,11 +1005,17 @@ func (l *loadbalancers) buildLoadBalancerRequest(ctx context.Context, clusterNam
 	configs := make([]*linodego.NodeBalancerConfigCreateOptions, 0, len(ports))
 
 	subnetID := 0
+	if Options.NodeBalancerBackendIPv4SubnetID != 0 {
+		subnetID = Options.NodeBalancerBackendIPv4SubnetID
+	}
+	// Check for the NodeBalancerBackendIPv4Range annotation
 	backendIPv4Range, ok := service.GetAnnotations()[annotations.NodeBalancerBackendIPv4Range]
 	if ok {
 		if err := validateNodeBalancerBackendIPv4Range(backendIPv4Range); err != nil {
 			return nil, err
 		}
+	}
+	if Options.VPCNames != "" && !Options.DisableNodeBalancerVPCBackends {
 		id, err := l.getSubnetIDForSVC(ctx, service)
 		if err != nil {
 			return nil, err
@@ -1088,9 +1192,9 @@ func getPortConfigAnnotation(service *v1.Service, port int) (portConfigAnnotatio
 }
 
 // getNodePrivateIP provides the Linode Backend IP the NodeBalancer will communicate with.
-// If a service specifies NodeBalancerBackendIPv4Range annotation, it will
+// If CCM runs within VPC and DisableNodeBalancerVPCBackends is set to false, it will
 // use NodeInternalIP of node.
-// For services which don't have NodeBalancerBackendIPv4Range annotation,
+// For services outside of VPC, it will use linode specific private IP address
 // Backend IP can be overwritten to the one specified using AnnLinodeNodePrivateIP
 // annotation over the NodeInternalIP.
 func getNodePrivateIP(node *v1.Node, subnetID int) string {