add some fixes and validation for linode creation with new network interfaces

Author: AshleyDumaine

internal/controller/linodemachine_controller.go

@@ -624,6 +624,10 @@ func (r *LinodeMachineReconciler) reconcilePreflightConfigure(ctx context.Contex
 			Network: true,
 		},
 	}
+	// LinodeInterfaces does not support network helpers, so we disable it
+	if machineScope.LinodeMachine.Spec.LinodeInterfaces != nil {
+		configData.Helpers.Network = false
+	}
 
 	if machineScope.LinodeMachine.Spec.Configuration != nil && machineScope.LinodeMachine.Spec.Configuration.Kernel != "" {
 		configData.Kernel = machineScope.LinodeMachine.Spec.Configuration.Kernel

internal/controller/linodemachine_controller_helpers.go

@@ -76,15 +76,22 @@ func retryIfTransient(err error, logger logr.Logger) (ctrl.Result, error) {
 		}
 		return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerRetryDelay}, nil
 	}
-	logger.Error(err, "unknown Linode API error")
-	return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerRetryDelay}, nil
+	logger.Error(err, "non-retryable Linode API error, skipping requeue")
+	return ctrl.Result{}, nil
 }
 
 func fillCreateConfig(createConfig *linodego.InstanceCreateOptions, machineScope *scope.MachineScope) {
 	if machineScope.LinodeMachine.Spec.PrivateIP != nil {
 		createConfig.PrivateIP = *machineScope.LinodeMachine.Spec.PrivateIP
 	} else {
-		createConfig.PrivateIP = true
+		if machineScope.LinodeMachine.Spec.LinodeInterfaces == nil {
+			// Supported only for legacy network interfaces.
+			createConfig.PrivateIP = true
+		} else {
+			// Network Helper is not supported for the new network interfaces.
+			createConfig.NetworkHelper = ptr.To(false)
+			createConfig.InterfaceGeneration = linodego.GenerationLinode
+		}
 	}
 
 	if createConfig.Tags == nil {
@@ -638,12 +645,21 @@ func getVPCLinodeInterfaceConfig(ctx context.Context, machineScope *scope.Machin
 	}
 
 	// Check if a VPC interface already exists
-	for i, netInterface := range linodeInterfaces {
+	for iface, netInterface := range linodeInterfaces {
 		if netInterface.VPC != nil {
-			linodeInterfaces[i].VPC.SubnetID = subnetID
+			linodeInterfaces[iface].VPC.SubnetID = subnetID
 			// If IPv6 range config is not empty, add it to the interface configuration
 			if !isVPCInterfaceIPv6ConfigEmpty(ipv6Config) {
-				linodeInterfaces[i].VPC.IPv6 = ipv6Config
+				linodeInterfaces[iface].VPC.IPv6 = ipv6Config
+			}
+			if netInterface.VPC.IPv4 == nil {
+				linodeInterfaces[iface].VPC.IPv4 = &linodego.VPCInterfaceIPv4CreateOptions{
+					Addresses: []linodego.VPCInterfaceIPv4AddressCreateOptions{{
+						Primary:        ptr.To(true),
+						NAT1To1Address: ptr.To("auto"),
+						Address:        "auto",
+					}},
+				}
 			}
 			return nil, nil //nolint:nilnil // it is important we don't return an interface if a VPC interface already exists
 		}
@@ -656,7 +672,8 @@ func getVPCLinodeInterfaceConfig(ctx context.Context, machineScope *scope.Machin
 			IPv4: &linodego.VPCInterfaceIPv4CreateOptions{
 				Addresses: []linodego.VPCInterfaceIPv4AddressCreateOptions{{
 					Primary:        ptr.To(true),
-					NAT1To1Address: ptr.To("any"),
+					NAT1To1Address: ptr.To("auto"),
+					Address:        "auto",
 				}},
 			},
 		},
@@ -667,6 +684,8 @@ func getVPCLinodeInterfaceConfig(ctx context.Context, machineScope *scope.Machin
 		vpcIntfCreateOpts.VPC.IPv6 = ipv6Config
 	}
 
+	logger.Info("Creating LinodeInterfaceCreateOptions", "VPC", *vpcIntfCreateOpts)
+
 	return vpcIntfCreateOpts, nil
 }
 
@@ -738,7 +757,7 @@ func getVPCLinodeInterfaceConfigFromDirectID(ctx context.Context, machineScope *
 			IPv4: &linodego.VPCInterfaceIPv4CreateOptions{
 				Addresses: []linodego.VPCInterfaceIPv4AddressCreateOptions{{
 					Primary:        ptr.To(true),
-					NAT1To1Address: ptr.To("any"),
+					NAT1To1Address: ptr.To("auto"),
 				}},
 			},
 		},
@@ -905,7 +924,6 @@ func getVPCInterfaceIPv6Config(machineScope *scope.MachineScope, numIPv6RangesIn
 
 // Unfortunately, this is necessary since DeepCopy can't be generated for linodego.LinodeInterfaceCreateOptions
 // so here we manually create the options for Linode interfaces.
-// /
 //
 //nolint:gocognit,cyclop,gocritic,nestif,nolintlint // Also, unfortunately, this cannot be made any reasonably simpler with how complicated the linodego struct is
 func constructLinodeInterfaceCreateOpts(createOpts []infrav1alpha2.LinodeInterfaceCreateOptions) []linodego.LinodeInterfaceCreateOptions {
@@ -946,7 +964,8 @@ func constructLinodeInterfaceCreateOpts(createOpts []infrav1alpha2.LinodeInterfa
 				ipv4Addrs = []linodego.VPCInterfaceIPv4AddressCreateOptions{
 					{
 						Primary:        ptr.To(true),
-						NAT1To1Address: ptr.To("any"),
+						NAT1To1Address: ptr.To("auto"),
+						Address:        "auto", // Default to auto-assigned address
 					},
 				}
 			}
@@ -1021,37 +1040,39 @@ func constructLinodeInterfaceCreateOpts(createOpts []infrav1alpha2.LinodeInterfa
 	return linodeInterfaces
 }
 
+// for converting LinodeMachineSpec to linodego.InstanceCreateOptions. Any defaulting should be done in fillCreateConfig instead
 func linodeMachineSpecToInstanceCreateConfig(machineSpec infrav1alpha2.LinodeMachineSpec, machineTags []string) *linodego.InstanceCreateOptions {
-	interfaces := make([]linodego.InstanceConfigInterfaceCreateOptions, len(machineSpec.Interfaces))
-	for idx, iface := range machineSpec.Interfaces {
-		interfaces[idx] = linodego.InstanceConfigInterfaceCreateOptions{
-			IPAMAddress: iface.IPAMAddress,
-			Label:       iface.Label,
-			Purpose:     iface.Purpose,
-			Primary:     iface.Primary,
-			SubnetID:    iface.SubnetID,
-			IPRanges:    iface.IPRanges,
-		}
-	}
-	privateIP := false
-	if machineSpec.PrivateIP != nil {
-		privateIP = *machineSpec.PrivateIP
-	}
 	instCreateOpts := &linodego.InstanceCreateOptions{
 		Region:          machineSpec.Region,
 		Type:            machineSpec.Type,
 		AuthorizedKeys:  machineSpec.AuthorizedKeys,
 		AuthorizedUsers: machineSpec.AuthorizedUsers,
 		RootPass:        machineSpec.RootPass,
 		Image:           machineSpec.Image,
-		Interfaces:      interfaces,
-		PrivateIP:       privateIP,
 		Tags:            machineTags,
 		FirewallID:      machineSpec.FirewallID,
 		DiskEncryption:  linodego.InstanceDiskEncryption(machineSpec.DiskEncryption),
 	}
+
+	if machineSpec.PrivateIP != nil {
+		instCreateOpts.PrivateIP = *machineSpec.PrivateIP
+	}
+
 	if len(machineSpec.LinodeInterfaces) > 0 {
 		instCreateOpts.LinodeInterfaces = constructLinodeInterfaceCreateOpts(machineSpec.LinodeInterfaces)
+	} else {
+		interfaces := make([]linodego.InstanceConfigInterfaceCreateOptions, len(machineSpec.Interfaces))
+		for idx, iface := range machineSpec.Interfaces {
+			interfaces[idx] = linodego.InstanceConfigInterfaceCreateOptions{
+				IPAMAddress: iface.IPAMAddress,
+				Label:       iface.Label,
+				Purpose:     iface.Purpose,
+				Primary:     iface.Primary,
+				SubnetID:    iface.SubnetID,
+				IPRanges:    iface.IPRanges,
+			}
+		}
+		instCreateOpts.Interfaces = interfaces
 	}
 
 	return instCreateOpts
@@ -1478,6 +1499,14 @@ func configureFirewall(ctx context.Context, machineScope *scope.MachineScope, cr
 	}
 
 	createConfig.FirewallID = fwID
+
+	// If using LinodeInterfaces that needs to know about the firewall ID
+	if machineScope.LinodeMachine.Spec.LinodeInterfaces != nil {
+		for i := range createConfig.LinodeInterfaces {
+			createConfig.LinodeInterfaces[i].FirewallID = ptr.To(fwID)
+		}
+	}
+
 	return nil
 }
 

internal/controller/linodemachine_controller_helpers_test.go

@@ -436,7 +436,7 @@ func validateInterfaceExpectations(
 		require.Equal(t, expectSubnetID, linodeIface.VPC.SubnetID)
 		require.NotNil(t, linodeIface.VPC.IPv4)
 		require.NotNil(t, linodeIface.VPC.IPv4.Addresses[0].NAT1To1Address)
-		require.Equal(t, "any", *linodeIface.VPC.IPv4.Addresses[0].NAT1To1Address)
+		require.Equal(t, "auto", *linodeIface.VPC.IPv4.Addresses[0].NAT1To1Address)
 	} else {
 		require.Nil(t, linodeIface)
 	}

internal/controller/linodemachine_controller_test.go

@@ -2567,8 +2567,9 @@ var _ = Describe("machine in VPC with new network interfaces", Label("machine",
 					SubnetID: 1,
 					IPv4: &linodego.VPCInterfaceIPv4CreateOptions{
 						Addresses: []linodego.VPCInterfaceIPv4AddressCreateOptions{{
-							NAT1To1Address: ptr.To("any"),
+							NAT1To1Address: ptr.To("auto"),
 							Primary:        ptr.To(true),
+							Address:        "auto",
 						}},
 					},
 					IPv6: nil,
@@ -2640,8 +2641,9 @@ var _ = Describe("machine in VPC with new network interfaces", Label("machine",
 					SubnetID: 1,
 					IPv4: &linodego.VPCInterfaceIPv4CreateOptions{
 						Addresses: []linodego.VPCInterfaceIPv4AddressCreateOptions{{
-							NAT1To1Address: ptr.To("any"),
+							NAT1To1Address: ptr.To("auto"),
 							Primary:        ptr.To(true),
+							Address:        "auto",
 						}},
 					},
 					IPv6: &linodego.VPCInterfaceIPv6CreateOptions{
@@ -2727,8 +2729,9 @@ var _ = Describe("machine in VPC with new network interfaces", Label("machine",
 					SubnetID: 27,
 					IPv4: &linodego.VPCInterfaceIPv4CreateOptions{
 						Addresses: []linodego.VPCInterfaceIPv4AddressCreateOptions{{
-							NAT1To1Address: ptr.To("any"),
+							NAT1To1Address: ptr.To("auto"),
 							Primary:        ptr.To(true),
+							Address:        "auto",
 						}},
 					},
 					IPv6: &linodego.VPCInterfaceIPv6CreateOptions{SLAAC: nil, Ranges: nil, IsPublic: false},

internal/webhook/v1alpha2/linodemachine_webhook.go

@@ -125,6 +125,7 @@ func (r *linodeMachineValidator) ValidateDelete(ctx context.Context, obj runtime
 	return nil, nil
 }
 
+//nolint:cyclop // as simple as it gets
 func (r *linodeMachineValidator) validateLinodeMachineSpec(ctx context.Context, linodeclient clients.LinodeClient, spec infrav1alpha2.LinodeMachineSpec, skipAPIValidation bool) field.ErrorList {
 	var errs field.ErrorList
 
@@ -156,6 +157,30 @@ func (r *linodeMachineValidator) validateLinodeMachineSpec(ctx context.Context,
 		})
 	}
 
+	if spec.LinodeInterfaces != nil && spec.Interfaces != nil {
+		errs = append(errs, &field.Error{
+			Field:  "spec.linodeInterfaces/spec.interfaces",
+			Type:   field.ErrorTypeInvalid,
+			Detail: "Cannot specify both LinodeInterfaces and Interfaces",
+		})
+	}
+
+	if spec.LinodeInterfaces != nil && spec.PrivateIP != nil && *spec.PrivateIP != false {
+		errs = append(errs, &field.Error{
+			Field:  "spec.linodeInterfaces/spec.privateIP",
+			Type:   field.ErrorTypeInvalid,
+			Detail: "Linode Interfaces do not support private IPs",
+		})
+	}
+
+	if spec.LinodeInterfaces != nil && spec.NetworkHelper != nil && *spec.NetworkHelper != false {
+		errs = append(errs, &field.Error{
+			Field:  "spec.linodeInterfaces/spec.networkHelper",
+			Type:   field.ErrorTypeInvalid,
+			Detail: "Linode Interfaces do not support network helper (enabled by default), it must be explicitly set to false",
+		})
+	}
+
 	if spec.FirewallID != 0 && spec.FirewallRef != nil {
 		errs = append(errs, &field.Error{
 			Field:  "spec.firewallID/spec.firewallRef",

templates/infra/linodeMachineTemplate.yaml

@@ -14,6 +14,12 @@ spec:
         kind: LinodeFirewall
         name: ${CLUSTER_NAME}
       # diskEncryption: disabled
+      #
+      # linodeInterfaces can't be used for the control plane
+      # if using the default NodeBalancer for the associated
+      # LinodeCluster's .Spec.Network.LoadBalancerType
+      # because we need a private IP to add the machine to the
+      # NodeBalancer
       interfaces:
         - purpose: public
       authorizedKeys:
@@ -35,8 +41,11 @@ spec:
         kind: LinodeFirewall
         name: ${CLUSTER_NAME}
       # diskEncryption: disabled
-      interfaces:
-        - purpose: public
+      linodeInterfaces:
+        - public:
+            ipv4:
+              addresses:
+                - address: "auto"
       authorizedKeys:
       # uncomment to include your ssh key in linode provisioning
       # - ${LINODE_SSH_PUBKEY}