fix: remove default mtu

ionos-cloud · Dec 23, 2023 · 3fd27ea · 3fd27ea
1 parent eceed43
commit 3fd27ea
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 22 deletions.
diff --git a/api/v1alpha1/proxmoxmachine_types.go b/api/v1alpha1/proxmoxmachine_types.go
@@ -222,7 +222,6 @@ type NetworkDevice struct {
 	// +optional
 	// +kubebuilder:validation:Minimum=1
 	// +kubebuilder:validation:Maximum=65520
-	// +kubebuilder:default=1500
 	MTU *uint16 `json:"mtu,omitempty"`
 }
 

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml
@@ -206,7 +206,6 @@ spec:
                           - vmxnet3
                           type: string
                         mtu:
-                          default: 1500
                           description: MTU is the network device Maximum Transmission
                             Unit. Only works with virtio Model.
                           maximum: 65520
@@ -253,7 +252,6 @@ spec:
                         - vmxnet3
                         type: string
                       mtu:
-                        default: 1500
                         description: MTU is the network device Maximum Transmission
                           Unit. Only works with virtio Model.
                         maximum: 65520

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml
@@ -224,7 +224,6 @@ spec:
                                   - vmxnet3
                                   type: string
                                 mtu:
-                                  default: 1500
                                   description: MTU is the network device Maximum Transmission
                                     Unit. Only works with virtio Model.
                                   maximum: 65520
@@ -273,7 +272,6 @@ spec:
                                 - vmxnet3
                                 type: string
                               mtu:
-                                default: 1500
                                 description: MTU is the network device Maximum Transmission
                                   Unit. Only works with virtio Model.
                                 maximum: 65520

diff --git a/internal/service/vmservice/utils.go b/internal/service/vmservice/utils.go
@@ -18,11 +18,13 @@ package vmservice
 
 import (
 	"fmt"
+	"regexp"
+	"strconv"
+
 	"github.com/google/uuid"
+
 	infrav1alpha1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1"
 	"github.com/ionos-cloud/cluster-api-provider-proxmox/pkg/scope"
-	"regexp"
-	"strconv"
 )
 
 const (
@@ -108,11 +110,18 @@ func shouldUpdateNetworkDevices(machineScope *scope.MachineScope) bool {
 
 		model := extractNetworkModel(net0)
 		bridge := extractNetworkBridge(net0)
-		mtu := extractNetworkMTU(net0)
 
-		if model != *desiredDefault.Model || bridge != desiredDefault.Bridge || mtu != *desiredDefault.MTU {
+		if model != *desiredDefault.Model || bridge != desiredDefault.Bridge {
 			return true
 		}
+
+		if desiredDefault.MTU != nil {
+			mtu := extractNetworkMTU(net0)
+
+			if mtu != *desiredDefault.MTU {
+				return true
+			}
+		}
 	}
 
 	devices := machineScope.ProxmoxMachine.Spec.Network.AdditionalDevices
@@ -125,21 +134,32 @@ func shouldUpdateNetworkDevices(machineScope *scope.MachineScope) bool {
 
 		model := extractNetworkModel(net)
 		bridge := extractNetworkBridge(net)
-		mtu := extractNetworkMTU(net)
 
 		// current is different from the desired spec.
-		if model != *v.Model || bridge != v.Bridge || mtu != *v.MTU {
+		if model != *v.Model || bridge != v.Bridge {
 			return true
 		}
+
+		if v.MTU != nil {
+			mtu := extractNetworkMTU(net)
+
+			if mtu != *v.MTU {
+				return true
+			}
+		}
 	}
 
 	return false
 }
 
 // formatNetworkDevice formats a network device config
 // example 'virtio,bridge=vmbr0'.
-func formatNetworkDevice(model, bridge string, mtu uint16) string {
-	return fmt.Sprintf("%s,bridge=%s,mtu=%d", model, bridge, mtu)
+func formatNetworkDevice(model, bridge string, mtu *uint16) string {
+	if mtu == nil {
+		return fmt.Sprintf("%s,bridge=%s", model, bridge)
+	}
+
+	return fmt.Sprintf("%s,bridge=%s,mtu=%d", model, bridge, *mtu)
 }
 
 // extractMACAddress returns the macaddress out of net device input e.g. virtio=A6:23:64:4D:84:CB,bridge=vmbr1.

diff --git a/internal/service/vmservice/utils_test.go b/internal/service/vmservice/utils_test.go
@@ -118,7 +118,7 @@ func TestExtractNetworkBridge(t *testing.T) {
 
 	for _, s := range badstrings {
 		bridge := extractNetworkBridge(s)
-		require.Empty(t, bridge)
+		require.Equal(t, "unknown", bridge)
 	}
 }
 
@@ -134,7 +134,6 @@ func TestExtractNetworkMTU(t *testing.T) {
 		{"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500", 1500},
 		{"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=9000,foo", 9000},
 		{"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=9000,foo=bar", 9000},
-		{"foo=bar,e1000=A6:23:64:4D:84:CB,bridge=vmbr1,foo=bar", 1500},
 	}
 
 	badstrings := []string{
@@ -151,9 +150,7 @@ func TestExtractNetworkMTU(t *testing.T) {
 
 	for _, s := range badstrings {
 		mtu := extractNetworkMTU(s)
-
-		// when mtu is not specified, it should be default 1500
-		require.Equal(t, uint16(1500), mtu)
+		require.Equal(t, uint16(0), mtu)
 	}
 }
 

diff --git a/internal/service/vmservice/vm.go b/internal/service/vmservice/vm.go
@@ -195,7 +195,7 @@ func reconcileVirtualMachineConfig(ctx context.Context, machineScope *scope.Mach
 			Value: formatNetworkDevice(
 				*machineScope.ProxmoxMachine.Spec.Network.Default.Model,
 				machineScope.ProxmoxMachine.Spec.Network.Default.Bridge,
-				*machineScope.ProxmoxMachine.Spec.Network.Default.MTU,
+				machineScope.ProxmoxMachine.Spec.Network.Default.MTU,
 			),
 		})
 
@@ -204,7 +204,7 @@ func reconcileVirtualMachineConfig(ctx context.Context, machineScope *scope.Mach
 		for _, v := range devices {
 			vmOptions = append(vmOptions, proxmox.VirtualMachineOption{
 				Name:  v.Name,
-				Value: formatNetworkDevice(*v.Model, v.Bridge, *v.MTU),
+				Value: formatNetworkDevice(*v.Model, v.Bridge, v.MTU),
 			})
 		}
 	}

diff --git a/internal/service/vmservice/vm_test.go b/internal/service/vmservice/vm_test.go
@@ -176,8 +176,8 @@ func TestReconcileVirtualMachineConfig_ApplyConfig(t *testing.T) {
 		proxmox.VirtualMachineOption{Name: optionSockets, Value: machineScope.ProxmoxMachine.Spec.NumSockets},
 		proxmox.VirtualMachineOption{Name: optionCores, Value: machineScope.ProxmoxMachine.Spec.NumCores},
 		proxmox.VirtualMachineOption{Name: optionMemory, Value: machineScope.ProxmoxMachine.Spec.MemoryMiB},
-		proxmox.VirtualMachineOption{Name: "net0", Value: formatNetworkDevice("virtio", "vmbr0", 1500)},
-		proxmox.VirtualMachineOption{Name: "net1", Value: formatNetworkDevice("virtio", "vmbr1", 1500)},
+		proxmox.VirtualMachineOption{Name: "net0", Value: formatNetworkDevice("virtio", "vmbr0", ptr.To(uint16(1500)))},
+		proxmox.VirtualMachineOption{Name: "net1", Value: formatNetworkDevice("virtio", "vmbr1", ptr.To(uint16(1500)))},
 	}
 
 	proxmoxClient.EXPECT().ConfigureVM(context.TODO(), vm, expectedOptions...).Return(task, nil).Once()