Skip to content

fix: change default configuration for KubeVirt networking #1856

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

Merged
merged 1 commit into from
Sep 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
32 changes: 6 additions & 26 deletions pkg/cloudprovider/provider/kubevirt/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"k8c.io/machine-controller/pkg/cloudprovider/instance"
kubevirttypes "k8c.io/machine-controller/pkg/cloudprovider/provider/kubevirt/types"
cloudprovidertypes "k8c.io/machine-controller/pkg/cloudprovider/types"
netutil "k8c.io/machine-controller/pkg/cloudprovider/util"
controllerutil "k8c.io/machine-controller/pkg/controller/util"
"k8c.io/machine-controller/pkg/providerconfig"
providerconfigtypes "k8c.io/machine-controller/pkg/providerconfig/types"
Expand Down Expand Up @@ -597,7 +596,7 @@ func (p *provider) Create(ctx context.Context, _ *zap.SugaredLogger, machine *cl
userDataSecretName := fmt.Sprintf("userdata-%s-%s", machine.Name, strconv.Itoa(int(time.Now().Unix())))

virtualMachine, err := p.newVirtualMachine(ctx, c, pc, machine, userDataSecretName, userdata,
machineDeploymentNameAndRevisionForMachineGetter(ctx, machine, data.Client), randomMacAddressGetter)
machineDeploymentNameAndRevisionForMachineGetter(ctx, machine, data.Client))
if err != nil {
return nil, fmt.Errorf("could not create a VirtualMachine manifest %w", err)
}
Expand All @@ -621,7 +620,7 @@ func (p *provider) Create(ctx context.Context, _ *zap.SugaredLogger, machine *cl
}

func (p *provider) newVirtualMachine(_ context.Context, c *Config, pc *providerconfigtypes.Config, machine *clusterv1alpha1.Machine,
userdataSecretName, userdata string, mdNameGetter machineDeploymentNameGetter, macAddressGetter macAddressGetter) (*kubevirtv1.VirtualMachine, error) {
userdataSecretName, userdata string, mdNameGetter machineDeploymentNameGetter) (*kubevirtv1.VirtualMachine, error) {
// We add the timestamp because the secret name must be different when we recreate the VMI
// because its pod got deleted
// The secret has an ownerRef on the VMI so garbace collection will take care of cleaning up.
Expand Down Expand Up @@ -663,6 +662,7 @@ func (p *provider) newVirtualMachine(_ context.Context, c *Config, pc *providerc
}

annotations["ovn.kubernetes.io/allow_live_migration"] = "true"
annotations["kubevirt.io/allow-pod-bridge-network-live-migration"] = "true"

for k, v := range machine.Annotations {
if strings.HasPrefix(k, "cdi.kubevirt.io") {
Expand All @@ -673,11 +673,7 @@ func (p *provider) newVirtualMachine(_ context.Context, c *Config, pc *providerc
annotations[k] = v
}

defaultBridgeNetwork, err := defaultBridgeNetwork(macAddressGetter)
if err != nil {
return nil, fmt.Errorf("could not compute a random MAC address")
}

defaultBridgeNetwork := defaultBridgeNetwork()
runStrategyOnce := kubevirtv1.RunStrategyOnce

virtualMachine := &kubevirtv1.VirtualMachine{
Expand Down Expand Up @@ -799,24 +795,8 @@ func getVMDisks(config *Config) []kubevirtv1.Disk {
return disks
}

type macAddressGetter func() (string, error)

func randomMacAddressGetter() (string, error) {
mac, err := netutil.GenerateRandMAC()
if err != nil {
return "", err
}
return mac.String(), nil
}

func defaultBridgeNetwork(macAddressGetter macAddressGetter) (*kubevirtv1.Interface, error) {
defaultBridgeNetwork := kubevirtv1.DefaultBridgeNetworkInterface()
mac, err := macAddressGetter()
if err != nil {
return nil, err
}
defaultBridgeNetwork.MacAddress = mac
return defaultBridgeNetwork, nil
func defaultBridgeNetwork() *kubevirtv1.Interface {
return kubevirtv1.DefaultBridgeNetworkInterface()
}

func getVMVolumes(config *Config, dataVolumeName string, userDataSecretName string) []kubevirtv1.Volume {
Expand Down
11 changes: 4 additions & 7 deletions pkg/cloudprovider/provider/kubevirt/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (k kubevirtProviderSpecConf) rawProviderSpec(t *testing.T) []byte {
"primaryDisk": {
{{- if .StorageTarget }}
"storageTarget": "{{ .StorageTarget }}",
{{- end }}
{{- end }}
{{- if .OsImageDV }}
"osImage": "{{ .OsImageDV }}",
{{- else }}
Expand Down Expand Up @@ -221,7 +221,8 @@ func TestNewVirtualMachine(t *testing.T) {
{
name: "custom-local-disk",
specConf: kubevirtProviderSpecConf{OsImageDV: "ns/dvname"},
}, {
},
{
name: "use-storage-as-storage-target",
specConf: kubevirtProviderSpecConf{StorageTarget: Storage},
},
Expand Down Expand Up @@ -263,7 +264,7 @@ func TestNewVirtualMachine(t *testing.T) {
c.Namespace = testNamespace

// Check the created VirtualMachine
vm, _ := p.newVirtualMachine(context.TODO(), c, pc, machine, "udsn", userdata, fakeMachineDeploymentNameAndRevisionForMachineGetter(), fixedMacAddressGetter)
vm, _ := p.newVirtualMachine(context.TODO(), c, pc, machine, "udsn", userdata, fakeMachineDeploymentNameAndRevisionForMachineGetter())
vm.TypeMeta.APIVersion, vm.TypeMeta.Kind = kubevirtv1.VirtualMachineGroupVersionKind.ToAPIVersionAndKind()

if !equality.Semantic.DeepEqual(vm, expectedVms[tt.name]) {
Expand All @@ -289,10 +290,6 @@ func toVirtualMachines(objects []runtime.Object) map[string]*kubevirtv1.VirtualM
return vms
}

func fixedMacAddressGetter() (string, error) {
return "b6:f5:b4:fe:45:1d", nil
}

// runtimeFromYaml returns a list of Kubernetes runtime objects from their yaml templates.
// It returns the objects for all files included in the ManifestFS folder, skipping (with error log) the yaml files
// that would not contain correct yaml files.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ spec:
metadata:
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
cluster.x-k8s.io/cluster-name: cluster-name
cluster.x-k8s.io/role: worker
Expand All @@ -53,8 +54,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
resources:
limits:
Expand Down
4 changes: 2 additions & 2 deletions pkg/cloudprovider/provider/kubevirt/testdata/affinity.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ spec:
metadata:
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
cluster.x-k8s.io/cluster-name: cluster-name
cluster.x-k8s.io/role: worker
Expand All @@ -56,8 +57,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
resources:
limits:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ spec:
creationTimestamp: null
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
cluster.x-k8s.io/cluster-name: cluster-name
cluster.x-k8s.io/role: worker
Expand All @@ -48,8 +49,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
resources:
limits:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ spec:
creationTimestamp: null
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
kubevirt.io/vm: http-image-source
cluster.x-k8s.io/cluster-name: cluster-name
Expand All @@ -47,8 +48,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
resources:
limits:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ spec:
metadata:
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
cluster.x-k8s.io/cluster-name: cluster-name
cluster.x-k8s.io/role: worker
Expand All @@ -53,8 +54,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
networks:
- name: default
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ spec:
creationTimestamp: null
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
cluster.x-k8s.io/cluster-name: cluster-name
cluster.x-k8s.io/role: worker
Expand All @@ -53,8 +54,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
networks:
- name: default
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ spec:
creationTimestamp: null
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
cluster.x-k8s.io/cluster-name: cluster-name
cluster.x-k8s.io/role: worker
Expand All @@ -47,8 +48,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
resources:
limits:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ spec:
creationTimestamp: null
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
kubevirt.io/vm: pvc-image-source
cluster.x-k8s.io/cluster-name: cluster-name
Expand All @@ -48,8 +49,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
resources:
limits:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ spec:
creationTimestamp: null
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
kubevirt.io/vm: registry-image-source-pod
cluster.x-k8s.io/cluster-name: cluster-name
Expand All @@ -48,8 +49,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
resources:
limits:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ spec:
creationTimestamp: null
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
kubevirt.io/vm: registry-image-source
cluster.x-k8s.io/cluster-name: cluster-name
Expand All @@ -48,8 +49,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
resources:
limits:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ spec:
creationTimestamp: null
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
cluster.x-k8s.io/cluster-name: cluster-name
cluster.x-k8s.io/role: worker
Expand All @@ -79,8 +80,7 @@ spec:
bus: virtio
name: secondary-disks-secondarydisk1
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
resources:
limits:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ spec:
creationTimestamp: null
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
cluster.x-k8s.io/cluster-name: cluster-name
cluster.x-k8s.io/role: worker
Expand All @@ -47,8 +48,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
resources:
limits:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ spec:
creationTimestamp: null
annotations:
"ovn.kubernetes.io/allow_live_migration": "true"
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
labels:
cluster.x-k8s.io/cluster-name: cluster-name
cluster.x-k8s.io/role: worker
Expand All @@ -48,8 +49,7 @@ spec:
bus: virtio
name: cloudinitdisk
interfaces:
- macAddress: b6:f5:b4:fe:45:1d
name: default
- name: default
bridge: {}
resources:
limits:
Expand Down