feat: implement setting mtu for virtio network device (#42)

* feat: implement setting mtu for virtio network device

See https://git.proxmox.com/?p=qemu-server.git;a=commit;h=61a14cde8d568e552d3deaab2da76b479b8aca7b

* fix: split extractNetworkModelAndBridge into multiple functions

* fix: set maximum mtu to 65520

* test: should not allow mtu less than 1 or more than 65520

* feat: add validating webhook for proxmoxmachine

* fix: remove default mtu

PROJECT

@@ -30,6 +30,10 @@ resources:
   kind: ProxmoxMachine
   path: github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1
   version: v1alpha1
+  webhooks:
+    defaulting: true
+    validation: true
+    webhookVersion: v1
 - api:
     crdVersion: v1
     namespaced: true

api/v1alpha1/proxmoxmachine_types.go

@@ -216,6 +216,13 @@ type NetworkDevice struct {
 	// +kubebuilder:validation:Enum=e1000;virtio;rtl8139;vmxnet3
 	// +kubebuilder:default=virtio
 	Model *string `json:"model,omitempty"`
+
+	// MTU is the network device Maximum Transmission Unit.
+	// Only works with virtio Model.
+	// +optional
+	// +kubebuilder:validation:Minimum=1
+	// +kubebuilder:validation:Maximum=65520
+	MTU *uint16 `json:"mtu,omitempty"`
 }
 
 // AdditionalNetworkDevice the definition of a Proxmox network device.

api/v1alpha1/proxmoxmachine_types_test.go

@@ -200,5 +200,29 @@ var _ = Describe("ProxmoxMachine Test", func() {
 			}
 			Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("at least one pool reference must be set, either ipv4PoolRef or ipv6PoolRef")))
 		})
+
+		It("Should not allow machine with network device mtu less than 1", func() {
+			dm := defaultMachine()
+			dm.Spec.Network = &NetworkSpec{
+				Default: &NetworkDevice{
+					Bridge: "vmbr0",
+					MTU:    ptr.To(uint16(0)),
+				},
+			}
+
+			Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be greater than or equal to 1")))
+		})
+
+		It("Should not allow machine with network device mtu greater than 65520", func() {
+			dm := defaultMachine()
+			dm.Spec.Network = &NetworkSpec{
+				Default: &NetworkDevice{
+					Bridge: "vmbr0",
+					MTU:    ptr.To(uint16(65521)),
+				},
+			}
+
+			Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be less than or equal to 65520")))
+		})
 	})
 })

config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml

@@ -205,6 +205,12 @@ spec:
                           - rtl8139
                           - vmxnet3
                           type: string
+                        mtu:
+                          description: MTU is the network device Maximum Transmission
+                            Unit. Only works with virtio Model.
+                          maximum: 65520
+                          minimum: 1
+                          type: integer
                         name:
                           description: Name is the network device name. must be unique
                             within the virtual machine and different from the primary
@@ -245,6 +251,12 @@ spec:
                         - rtl8139
                         - vmxnet3
                         type: string
+                      mtu:
+                        description: MTU is the network device Maximum Transmission
+                          Unit. Only works with virtio Model.
+                        maximum: 65520
+                        minimum: 1
+                        type: integer
                     required:
                     - bridge
                     type: object

config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml

@@ -223,6 +223,12 @@ spec:
                                   - rtl8139
                                   - vmxnet3
                                   type: string
+                                mtu:
+                                  description: MTU is the network device Maximum Transmission
+                                    Unit. Only works with virtio Model.
+                                  maximum: 65520
+                                  minimum: 1
+                                  type: integer
                                 name:
                                   description: Name is the network device name. must
                                     be unique within the virtual machine and different
@@ -265,6 +271,12 @@ spec:
                                 - rtl8139
                                 - vmxnet3
                                 type: string
+                              mtu:
+                                description: MTU is the network device Maximum Transmission
+                                  Unit. Only works with virtio Model.
+                                maximum: 65520
+                                minimum: 1
+                                type: integer
                             required:
                             - bridge
                             type: object

config/webhook/manifests.yaml

@@ -26,3 +26,24 @@ webhooks:
     resources:
     - proxmoxclusters
   sideEffects: None
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-infrastructure-cluster-x-k8s-io-v1alpha1-proxmoxmachine
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: validation.proxmoxmachine.infrastructure.cluster.x-k8s.io
+  rules:
+  - apiGroups:
+    - infrastructure.cluster.x-k8s.io
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - proxmoxmachines
+  sideEffects: None

internal/service/vmservice/utils.go

@@ -19,8 +19,10 @@ 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"
 )
@@ -55,14 +57,39 @@ func IPAddressWithPrefix(ip string, prefix int) string {
 	return fmt.Sprintf("%s/%d", ip, prefix)
 }
 
-// extractNetworkModelAndBridge returns the model & bridge out of net device input e.g. virtio=A6:23:64:4D:84:CB,bridge=vmbr1.
-func extractNetworkModelAndBridge(input string) (string, string) {
-	re := regexp.MustCompile(`([^=,]+)=([^,]+),bridge=([^,]+)`)
+// extractNetworkModel returns the model out of net device input e.g. virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500.
+func extractNetworkModel(input string) string {
+	re := regexp.MustCompile(`([^,=]+)(?:=[^,]*)?,bridge=([^,]+)`)
 	matches := re.FindStringSubmatch(input)
-	if len(matches) == 4 {
-		return matches[1], matches[3]
+	if len(matches) >= 2 {
+		return matches[1]
+	}
+	return ""
+}
+
+// extractNetworkBridge returns the bridge out of net device input e.g. virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500.
+func extractNetworkBridge(input string) string {
+	re := regexp.MustCompile(`bridge=(\w+)`)
+	match := re.FindStringSubmatch(input)
+	if len(match) > 1 {
+		return match[1]
+	}
+	return "unknown"
+}
+
+// extractNetworkMTU returns the mtu out of net device input e.g. virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500.
+func extractNetworkMTU(input string) uint16 {
+	re := regexp.MustCompile(`mtu=(\d+)`)
+	match := re.FindStringSubmatch(input)
+	if len(match) > 1 {
+		mtu, err := strconv.ParseUint(match[1], 10, 16)
+		if err != nil {
+			return 0
+		}
+		return uint16(mtu)
 	}
-	return "", ""
+
+	return 0
 }
 
 func shouldUpdateNetworkDevices(machineScope *scope.MachineScope) bool {
@@ -78,10 +105,23 @@ func shouldUpdateNetworkDevices(machineScope *scope.MachineScope) bool {
 		if net0 == "" {
 			return true
 		}
-		model, bridge := extractNetworkModelAndBridge(net0)
-		if model != *machineScope.ProxmoxMachine.Spec.Network.Default.Model || bridge != machineScope.ProxmoxMachine.Spec.Network.Default.Bridge {
+
+		desiredDefault := *machineScope.ProxmoxMachine.Spec.Network.Default
+
+		model := extractNetworkModel(net0)
+		bridge := extractNetworkBridge(net0)
+
+		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
@@ -91,20 +131,35 @@ func shouldUpdateNetworkDevices(machineScope *scope.MachineScope) bool {
 		if len(net) == 0 {
 			return true
 		}
-		model, bridge := extractNetworkModelAndBridge(net)
+
+		model := extractNetworkModel(net)
+		bridge := extractNetworkBridge(net)
+
 		// current is different from the desired spec.
 		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) string {
-	return fmt.Sprintf("%s,bridge=%s", model, bridge)
+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.