diff --git a/build/components/versions.yml b/build/components/versions.yml index 35efe181a1..67d0a6c9c6 100644 --- a/build/components/versions.yml +++ b/build/components/versions.yml @@ -3,7 +3,7 @@ firmware: libvirt: v10.9.0 edk2: stable202411 core: - 3p-kubevirt: v1.6.2-v12n.21 + 3p-kubevirt: feat/core/network-hotplug-support 3p-containerized-data-importer: v1.60.3-v12n.18 distribution: 2.8.3 package: diff --git a/images/virtualization-artifact/pkg/common/network/spec.go b/images/virtualization-artifact/pkg/common/network/spec.go index 4d1976cbc9..176fb7340f 100644 --- a/images/virtualization-artifact/pkg/common/network/spec.go +++ b/images/virtualization-artifact/pkg/common/network/spec.go @@ -30,6 +30,14 @@ func CreateNetworkSpec(vm *v1alpha2.VirtualMachine, vmmacs []*v1alpha2.VirtualMa macPool := NewMacAddressPool(vm, vmmacs) var specs InterfaceSpecList + if len(vm.Spec.Networks) == 0 { + specs = append(specs, createMainInterfaceSpec(v1alpha2.NetworksSpec{ + Type: v1alpha2.NetworksTypeMain, + ID: ptr.To(ReservedMainID), + })) + return specs + } + for _, net := range vm.Spec.Networks { if net.Type == v1alpha2.NetworksTypeMain { specs = append(specs, createMainInterfaceSpec(net)) diff --git a/images/virtualization-artifact/pkg/common/network/types.go b/images/virtualization-artifact/pkg/common/network/types.go index 58d70914e5..2540b58db2 100644 --- a/images/virtualization-artifact/pkg/common/network/types.go +++ b/images/virtualization-artifact/pkg/common/network/types.go @@ -37,13 +37,16 @@ func HasMainNetworkStatus(networks []v1alpha2.NetworksStatus) bool { } func HasMainNetworkSpec(networks []v1alpha2.NetworksSpec) bool { - for _, network := range networks { - if network.Type == v1alpha2.NetworksTypeMain { - return true + return GetMainNetworkSpec(networks) != nil +} + +func GetMainNetworkSpec(networks []v1alpha2.NetworksSpec) *v1alpha2.NetworksSpec { + for i := range networks { + if networks[i].Type == v1alpha2.NetworksTypeMain { + return &networks[i] } } - - return false + return nil } type InterfaceSpec struct { diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go index 5ef04f9ed9..9e58030663 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go @@ -564,6 +564,15 @@ func (b *KVVM) ClearNetworkInterfaces() { b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces = nil } +func (b *KVVM) SetNetworkInterfaceAbsent(name string) { + for i, iface := range b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces { + if iface.Name == name { + b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces[i].State = virtv1.InterfaceStateAbsent + return + } + } +} + func (b *KVVM) SetNetworkInterface(name, macAddress string, acpiIndex int) { net := virtv1.Network{ Name: name, @@ -590,15 +599,16 @@ func (b *KVVM) SetNetworkInterface(name, macAddress string, acpiIndex int) { iface.MacAddress = macAddress } - ifaceExists := false - for _, i := range b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces { - if i.Name == name { - ifaceExists = true + updated := false + for i, existing := range b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces { + if existing.Name == name { + b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces[i] = iface + updated = true break } } - if !ifaceExists { + if !updated { b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces = append(b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces, iface) } } diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go index 8a14050251..c67eff2afc 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go @@ -22,7 +22,9 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + virtv1 "kubevirt.io/api/core/v1" + "github.com/deckhouse/virtualization-controller/pkg/common/network" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -145,3 +147,94 @@ func TestSetOsType(t *testing.T) { } }) } + +func newTestKVVM() *KVVM { + return NewEmptyKVVM(types.NamespacedName{Name: "test", Namespace: "default"}, KVVMOptions{ + EnableParavirtualization: true, + }) +} + +func TestSetNetworkInterfaceAbsent(t *testing.T) { + b := newTestKVVM() + b.SetNetworkInterface("default", "", 1) + b.SetNetworkInterface("veth_n12345678", "aa:bb:cc:dd:ee:ff", 2) + + b.SetNetworkInterfaceAbsent("veth_n12345678") + + for _, iface := range b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces { + if iface.Name == "veth_n12345678" { + if iface.State != virtv1.InterfaceStateAbsent { + t.Errorf("expected State %q, got %q", virtv1.InterfaceStateAbsent, iface.State) + } + return + } + } + t.Error("interface veth_n12345678 not found") +} + +func TestSetNetworkInterfaceReplacesExisting(t *testing.T) { + b := newTestKVVM() + b.SetNetworkInterface("veth_n12345678", "aa:bb:cc:dd:ee:ff", 2) + b.SetNetworkInterfaceAbsent("veth_n12345678") + + b.SetNetworkInterface("veth_n12345678", "aa:bb:cc:dd:ee:ff", 2) + + for _, iface := range b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces { + if iface.Name == "veth_n12345678" { + if iface.State != "" { + t.Errorf("expected empty State after re-add, got %q", iface.State) + } + return + } + } + t.Error("interface veth_n12345678 not found") +} + +func TestSetNetworkMarksRemovedAsAbsent(t *testing.T) { + b := newTestKVVM() + b.SetNetworkInterface("default", "", 1) + b.SetNetworkInterface("veth_n12345678", "aa:bb:cc:dd:ee:ff", 2) + + setNetwork(b, network.InterfaceSpecList{ + {InterfaceName: "default", MAC: "", ID: 1}, + }) + + found := false + for _, iface := range b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces { + if iface.Name == "veth_n12345678" { + found = true + if iface.State != virtv1.InterfaceStateAbsent { + t.Errorf("removed interface should have State %q, got %q", virtv1.InterfaceStateAbsent, iface.State) + } + } + if iface.Name == "default" && iface.State != "" { + t.Errorf("kept interface should have empty State, got %q", iface.State) + } + } + if !found { + t.Error("removed interface should be retained with absent state, not deleted") + } +} + +func TestSetNetworkAddsNewInterface(t *testing.T) { + b := newTestKVVM() + b.SetNetworkInterface("default", "", 1) + + setNetwork(b, network.InterfaceSpecList{ + {InterfaceName: "default", MAC: "", ID: 1}, + {InterfaceName: "veth_n12345678", MAC: "aa:bb:cc:dd:ee:ff", ID: 2}, + }) + + found := false + for _, iface := range b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces { + if iface.Name == "veth_n12345678" { + found = true + if iface.ACPIIndex != 2 { + t.Errorf("expected ACPIIndex 2, got %d", iface.ACPIIndex) + } + } + } + if !found { + t.Error("new interface should be added") + } +} diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go index 594a6d7bdb..732f502d68 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -355,7 +355,17 @@ func ApplyMigrationVolumes(kvvm *KVVM, vm *v1alpha2.VirtualMachine, vdsByName ma } func setNetwork(kvvm *KVVM, networkSpec network.InterfaceSpecList) { - kvvm.ClearNetworkInterfaces() + desiredByName := make(map[string]struct{}, len(networkSpec)) + for _, n := range networkSpec { + desiredByName[n.InterfaceName] = struct{}{} + } + + for _, iface := range kvvm.Resource.Spec.Template.Spec.Domain.Devices.Interfaces { + if _, wanted := desiredByName[iface.Name]; !wanted { + kvvm.SetNetworkInterfaceAbsent(iface.Name) + } + } + for _, n := range networkSpec { kvvm.SetNetworkInterface(n.InterfaceName, n.MAC, n.ID) } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go index 495d9aaa5d..0389fc3e5e 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -654,6 +654,23 @@ func (h *SyncKvvmHandler) applyVMChangesToKVVM(ctx context.Context, s state.Virt h.recorder.Event(current, corev1.EventTypeNormal, v1alpha2.ReasonVMChangesApplied, message) log.Debug(message, "vm.name", current.GetName(), "changes", changes) + if hasNetworkChange(changes) { + if err := h.patchPodNetworkAnnotation(ctx, s); err != nil { + return fmt.Errorf("unable to patch pod network annotation: %w", err) + } + + ready, err := h.isNetworkReadyOnPod(ctx, s) + if err != nil { + return fmt.Errorf("unable to check pod network status: %w", err) + } + if !ready { + msg := "Waiting for SDN to configure network interfaces on the pod" + log.Info(msg) + h.recorder.Event(current, corev1.EventTypeNormal, v1alpha2.ReasonVMChangesApplied, msg) + return nil + } + } + if err := h.updateKVVM(ctx, s); err != nil { return fmt.Errorf("unable to update KVVM using new VM spec: %w", err) } @@ -713,6 +730,69 @@ func (h *SyncKvvmHandler) isVMUnschedulable( return false } +func hasNetworkChange(changes vmchange.SpecChanges) bool { + for _, c := range changes.GetAll() { + if c.Path == "networks" { + return true + } + } + return false +} + +func (h *SyncKvvmHandler) isNetworkReadyOnPod(ctx context.Context, s state.VirtualMachineState) (bool, error) { + pods, err := s.Pods(ctx) + if err != nil { + return false, err + } + if pods == nil || len(pods.Items) == 0 { + return false, nil + } + errMsg, err := extractNetworkStatusFromPods(pods) + if err != nil { + return false, err + } + return errMsg == "", nil +} + +func (h *SyncKvvmHandler) patchPodNetworkAnnotation(ctx context.Context, s state.VirtualMachineState) error { + log := logger.FromContext(ctx) + + pod, err := s.Pod(ctx) + if err != nil { + return err + } + if pod == nil { + return nil + } + + current := s.VirtualMachine().Current() + vmmacs, err := s.VirtualMachineMACAddresses(ctx) + if err != nil { + return err + } + + networkConfigStr, err := network.CreateNetworkSpec(current, vmmacs).ToString() + if err != nil { + return fmt.Errorf("failed to serialize network spec: %w", err) + } + + if pod.Annotations[annotations.AnnNetworksSpec] == networkConfigStr { + return nil + } + + patch := client.MergeFrom(pod.DeepCopy()) + if pod.Annotations == nil { + pod.Annotations = make(map[string]string) + } + pod.Annotations[annotations.AnnNetworksSpec] = networkConfigStr + if err := h.client.Patch(ctx, pod, patch); err != nil { + return fmt.Errorf("failed to patch pod %s network annotation: %w", pod.Name, err) + } + log.Info("Patched pod network annotation", "pod", pod.Name, "networks", networkConfigStr) + + return nil +} + // isPlacementPolicyChanged returns true if any of the Affinity, NodePlacement, or Toleration rules have changed. func (h *SyncKvvmHandler) isPlacementPolicyChanged(allChanges vmchange.SpecChanges) bool { for _, c := range allChanges.GetAll() { diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparator_pod_placement.go b/images/virtualization-artifact/pkg/controller/vmchange/comparator_pod_placement.go index 7a4b90c24a..b8f8de9b7c 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparator_pod_placement.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparator_pod_placement.go @@ -19,6 +19,7 @@ package vmchange import ( "reflect" + "github.com/deckhouse/virtualization-controller/pkg/common/network" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -89,10 +90,10 @@ func compareNetworks(current, desired *v1alpha2.VirtualMachineSpec) []FieldChang desiredValue := NewValue(desired.Networks, desired.Networks == nil, false) action := ActionRestart - // During upgrade from 1.6.0 to 1.7.0, network interface IDs are auto-populated for all existing VMs in the cluster. - // This allows avoiding a virtual machine restart during the version upgrade. if isOnlyNetworkIDAutofillChange(current.Networks, desired.Networks) { action = ActionNone + } else if isOnlyNonMainNetworksChanged(current.Networks, desired.Networks) { + action = ActionApplyImmediate } return compareValues( @@ -104,6 +105,23 @@ func compareNetworks(current, desired *v1alpha2.VirtualMachineSpec) []FieldChang ) } +// isOnlyNonMainNetworksChanged returns true when the Main network is unchanged +// between current and desired (so only non-Main networks differ). +// Empty networks list is equivalent to having an implicit default Main. +func isOnlyNonMainNetworksChanged(current, desired []v1alpha2.NetworksSpec) bool { + currentMain := network.GetMainNetworkSpec(current) + desiredMain := network.GetMainNetworkSpec(desired) + currentHasMain := currentMain != nil || len(current) == 0 + desiredHasMain := desiredMain != nil || len(desired) == 0 + if !currentHasMain || !desiredHasMain { + return false + } + if currentMain == nil || desiredMain == nil { + return true + } + return reflect.DeepEqual(*currentMain, *desiredMain) +} + func isOnlyNetworkIDAutofillChange(current, desired []v1alpha2.NetworksSpec) bool { if len(current) != len(desired) { return false diff --git a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go index 2ab26e66a8..779c384924 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go @@ -369,6 +369,106 @@ networks: requirePathOperation("networks", ChangeReplace), ), }, + { + "apply immediate when adding non-main network to nil networks", + ``, + ` +networks: +- type: Main + id: 1 +- type: ClusterNetwork + name: additional + id: 2 +`, + assertChanges( + actionRequired(ActionApplyImmediate), + requirePathOperation("networks", ChangeAdd), + ), + }, + { + "apply immediate when adding non-main network to existing main", + ` +networks: +- type: Main + id: 1 +`, + ` +networks: +- type: Main + id: 1 +- type: ClusterNetwork + name: additional + id: 2 +`, + assertChanges( + actionRequired(ActionApplyImmediate), + requirePathOperation("networks", ChangeReplace), + ), + }, + { + "apply immediate when removing non-main network", + ` +networks: +- type: Main + id: 1 +- type: Network + name: net1 + id: 2 +`, + ` +networks: +- type: Main + id: 1 +`, + assertChanges( + actionRequired(ActionApplyImmediate), + requirePathOperation("networks", ChangeReplace), + ), + }, + { + "restart when main network is removed", + ` +networks: +- type: Main + id: 1 +- type: Network + name: net1 + id: 2 +`, + ` +networks: +- type: Network + name: net1 + id: 2 +`, + assertChanges( + actionRequired(ActionRestart), + requirePathOperation("networks", ChangeReplace), + ), + }, + { + "restart when main network id changes", + ` +networks: +- type: Main + id: 1 +- type: Network + name: net1 + id: 2 +`, + ` +networks: +- type: Main + id: 5 +- type: Network + name: net1 + id: 2 +`, + assertChanges( + actionRequired(ActionRestart), + requirePathOperation("networks", ChangeReplace), + ), + }, } for _, tt := range tests {