Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement setting mtu for virtio network device #42

Merged
merged 6 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions api/v1alpha1/proxmoxmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
mcbenjemaa marked this conversation as resolved.
Show resolved Hide resolved
// +kubebuilder:validation:Maximum=65520
MTU *uint16 `json:"mtu,omitempty"`
mcbenjemaa marked this conversation as resolved.
Show resolved Hide resolved
}

// AdditionalNetworkDevice the definition of a Proxmox network device.
Expand Down
24 changes: 24 additions & 0 deletions api/v1alpha1/proxmoxmachine_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")))
})
})
})
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
77 changes: 66 additions & 11 deletions internal/service/vmservice/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 {
mcbenjemaa marked this conversation as resolved.
Show resolved Hide resolved
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 {
mcbenjemaa marked this conversation as resolved.
Show resolved Hide resolved
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 {
mcbenjemaa marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand All @@ -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
Expand All @@ -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.
Expand Down
Loading
Loading