diff --git a/build/components/versions.yml b/build/components/versions.yml index a22d26fe03..fa33907680 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.34 + 3p-kubevirt: feat/core/network-hotplug-support 3p-containerized-data-importer: v1.60.3-v12n.19 distribution: 2.8.3 package: diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index cca72c44cb..477bcc96c5 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -3097,7 +3097,8 @@ If you specify the main network, it must be the first entry in the `.spec.networ Important considerations when working with additional network interfaces: - The order of listing networks in `.spec.networks` determines the order in which interfaces are connected inside the virtual machine. -- Adding or removing additional networks takes effect only after the VM is rebooted. +- Adding or removing an additional network (`Network` or `ClusterNetwork`) on a running VM is applied live without reboot. ACPI indexes of existing interfaces are preserved across add/remove cycles, so interface names in the guest OS stay stable. +- Adding or removing the main network (`type: Main`) still requires a VM reboot, because it is tied to the pod's primary network interface and cannot be reconfigured on a running pod. - To preserve the order of network interfaces inside the guest operating system, it is recommended to add new networks to the end of the `.spec.networks` list (do not change the order of existing ones). - Network security policies (NetworkPolicy) do not apply to additional network interfaces. - Network parameters (IP addresses, gateways, DNS, etc.) for additional networks are configured manually from within the guest OS (for example, using Cloud-Init). diff --git a/docs/USER_GUIDE.ru.md b/docs/USER_GUIDE.ru.md index 23c6220e2c..380dad9431 100644 --- a/docs/USER_GUIDE.ru.md +++ b/docs/USER_GUIDE.ru.md @@ -3128,7 +3128,8 @@ EOF Особенности и важные моменты работы с дополнительными сетевыми интерфейсами: - порядок перечисления сетей в `.spec.networks` определяет порядок подключения интерфейсов внутри виртуальной машины; -- добавление или удаление дополнительных сетей вступает в силу только после перезагрузки ВМ; +- добавление или удаление дополнительной сети (`Network` или `ClusterNetwork`) на работающей ВМ применяется без перезагрузки. ACPI-индексы существующих интерфейсов сохраняются при добавлении/удалении, поэтому имена интерфейсов в гостевой ОС остаются стабильными; +- добавление или удаление основной сети (`type: Main`) по-прежнему требует перезагрузки ВМ, так как она связана с основным сетевым интерфейсом пода и не может быть изменена на работающем поде; - чтобы сохранить порядок сетевых интерфейсов внутри гостевой операционной системы, рекомендуется добавлять новые сети в конец списка `.spec.networks` (не менять порядок уже существующих); - политики сетевой безопасности (NetworkPolicy) не применяются к дополнительным сетевым интерфейсам; - параметры сети (IP-адреса, шлюзы, DNS и т.д.) для дополнительных сетей настраиваются вручную изнутри гостевой ОС (например, с помощью Cloud-Init). diff --git a/images/virt-artifact/werf.inc.yaml b/images/virt-artifact/werf.inc.yaml index 6381ba5eae..34e0e1b1a3 100644 --- a/images/virt-artifact/werf.inc.yaml +++ b/images/virt-artifact/werf.inc.yaml @@ -9,6 +9,7 @@ image: {{ .ModuleNamePrefix }}{{ .ImageName }}-src-artifact final: false fromImage: builder/src +fromCacheVersion: "2026-05-20-1" secrets: - id: SOURCE_REPO value: {{ $.SOURCE_REPO }} @@ -44,6 +45,7 @@ packages: image: {{ .ModuleNamePrefix }}{{ .ImageName }} final: false fromImage: {{ eq $.SVACE_ENABLED "false" | ternary "builder/golang-alt-1.25" "builder/golang-alt-svace-1.25" }} +fromCacheVersion: "2026-05-20-1" mount: {{- include "mount points for golang builds" . }} secrets: diff --git a/images/virtualization-artifact/pkg/common/network/readiness.go b/images/virtualization-artifact/pkg/common/network/readiness.go new file mode 100644 index 0000000000..168d06b6f3 --- /dev/null +++ b/images/virtualization-artifact/pkg/common/network/readiness.go @@ -0,0 +1,84 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package network + +import ( + "context" + "fmt" + + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +var ( + NetworkGVK = schema.GroupVersionKind{Group: "network.deckhouse.io", Version: "v1alpha1", Kind: "Network"} + ClusterNetworkGVK = schema.GroupVersionKind{Group: "network.deckhouse.io", Version: "v1alpha1", Kind: "ClusterNetwork"} +) + +func SpecKey(ns v1alpha2.NetworksSpec) string { + return fmt.Sprintf("%s/%s", ns.Type, ns.Name) +} + +func IsNetworkSpecReady(ctx context.Context, c client.Client, namespace string, ns v1alpha2.NetworksSpec) (bool, error) { + if ns.Type == v1alpha2.NetworksTypeMain { + return true, nil + } + obj := &unstructured.Unstructured{} + key := types.NamespacedName{Name: ns.Name} + switch ns.Type { + case v1alpha2.NetworksTypeClusterNetwork: + obj.SetGroupVersionKind(ClusterNetworkGVK) + case v1alpha2.NetworksTypeNetwork: + obj.SetGroupVersionKind(NetworkGVK) + key.Namespace = namespace + default: + return false, nil + } + if err := c.Get(ctx, key, obj); err != nil { + if k8serrors.IsNotFound(err) { + return false, nil + } + return false, err + } + return isReadyTrue(obj), nil +} + +func isReadyTrue(obj *unstructured.Unstructured) bool { + conds, found, err := unstructured.NestedSlice(obj.Object, "status", "conditions") + if err != nil || !found { + return false + } + for _, c := range conds { + condMap, ok := c.(map[string]any) + if !ok { + continue + } + typ, _, _ := unstructured.NestedString(condMap, "type") + if typ != "Ready" { + continue + } + status, _, _ := unstructured.NestedString(condMap, "status") + return status == string(metav1.ConditionTrue) + } + return false +} 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/common/testutil/testutil.go b/images/virtualization-artifact/pkg/common/testutil/testutil.go index c4e139a0a7..d5964d1ed9 100644 --- a/images/virtualization-artifact/pkg/common/testutil/testutil.go +++ b/images/virtualization-artifact/pkg/common/testutil/testutil.go @@ -22,6 +22,7 @@ import ( "reflect" "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" apiruntime "k8s.io/apimachinery/pkg/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" virtv1 "kubevirt.io/api/core/v1" @@ -31,11 +32,17 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "github.com/deckhouse/deckhouse/pkg/log" + commonnetwork "github.com/deckhouse/virtualization-controller/pkg/common/network" "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha3" ) +func registerNetworkGVKs(scheme *apiruntime.Scheme) { + scheme.AddKnownTypeWithName(commonnetwork.NetworkGVK, &unstructured.Unstructured{}) + scheme.AddKnownTypeWithName(commonnetwork.ClusterNetworkGVK, &unstructured.Unstructured{}) +} + func NewFakeClientWithObjects(objs ...client.Object) (client.WithWatch, error) { scheme := apiruntime.NewScheme() for _, f := range []func(*apiruntime.Scheme) error{ @@ -50,6 +57,7 @@ func NewFakeClientWithObjects(objs ...client.Object) (client.WithWatch, error) { return nil, err } } + registerNetworkGVKs(scheme) var newObjs []client.Object for _, obj := range objs { if reflect.ValueOf(obj).IsNil() { @@ -79,6 +87,7 @@ func NewFakeClientWithInterceptorWithObjects(interceptor interceptor.Funcs, objs return nil, err } } + registerNetworkGVKs(scheme) var newObjs []client.Object for _, obj := range objs { if reflect.ValueOf(obj).IsNil() { diff --git a/images/virtualization-artifact/pkg/controller/indexer/indexer.go b/images/virtualization-artifact/pkg/controller/indexer/indexer.go index fe9abac103..ce24c64a40 100644 --- a/images/virtualization-artifact/pkg/controller/indexer/indexer.go +++ b/images/virtualization-artifact/pkg/controller/indexer/indexer.go @@ -33,12 +33,14 @@ const ( ) const ( - IndexFieldVMByClass = "spec.virtualMachineClassName" - IndexFieldVMByVD = "spec.blockDeviceRefs.VirtualDisk" - IndexFieldVMByVI = "spec.blockDeviceRefs.VirtualImage" - IndexFieldVMByCVI = "spec.blockDeviceRefs.ClusterVirtualImage" - IndexFieldVMByUSBDevice = "spec.usbDevices.name" - IndexFieldVMByNode = "status.node" + IndexFieldVMByClass = "spec.virtualMachineClassName" + IndexFieldVMByVD = "spec.blockDeviceRefs.VirtualDisk" + IndexFieldVMByVI = "spec.blockDeviceRefs.VirtualImage" + IndexFieldVMByCVI = "spec.blockDeviceRefs.ClusterVirtualImage" + IndexFieldVMByUSBDevice = "spec.usbDevices.name" + IndexFieldVMByNode = "status.node" + IndexFieldVMByNetwork = "spec.networks.Network.name" + IndexFieldVMByClusterNetwork = "spec.networks.ClusterNetwork.name" IndexFieldVDByVDSnapshot = "vd,spec.DataSource.ObjectRef.Name,.Kind=VirtualDiskSnapshot" IndexFieldVIByVDSnapshot = "vi,spec.DataSource.ObjectRef.Name,.Kind=VirtualDiskSnapshot" @@ -84,6 +86,8 @@ var IndexGetters = []IndexGetter{ IndexVMByVI, IndexVMByCVI, IndexVMByNode, + IndexVMByNetwork, + IndexVMByClusterNetwork, IndexVMByProvisioningSecret, IndexVMSnapshotByVM, IndexVMSnapshotByVDSnapshot, @@ -173,6 +177,32 @@ func IndexVMByNode() (obj client.Object, field string, extractValue client.Index } } +func IndexVMByNetwork() (obj client.Object, field string, extractValue client.IndexerFunc) { + return &v1alpha2.VirtualMachine{}, IndexFieldVMByNetwork, func(object client.Object) []string { + return getNetworkNamesByType(object, v1alpha2.NetworksTypeNetwork) + } +} + +func IndexVMByClusterNetwork() (obj client.Object, field string, extractValue client.IndexerFunc) { + return &v1alpha2.VirtualMachine{}, IndexFieldVMByClusterNetwork, func(object client.Object) []string { + return getNetworkNamesByType(object, v1alpha2.NetworksTypeClusterNetwork) + } +} + +func getNetworkNamesByType(obj client.Object, typ string) []string { + vm, ok := obj.(*v1alpha2.VirtualMachine) + if !ok || vm == nil { + return nil + } + var names []string + for _, n := range vm.Spec.Networks { + if n.Type == typ && n.Name != "" { + names = append(names, n.Name) + } + } + return names +} + func IndexVMByProvisioningSecret() (obj client.Object, field string, extractValue client.IndexerFunc) { return &v1alpha2.VirtualMachine{}, IndexFieldVMByProvisioningSecret, func(object client.Object) []string { vm, ok := object.(*v1alpha2.VirtualMachine) diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go index 68cb1793a6..9d5ca35960 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go @@ -770,6 +770,35 @@ 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) RemoveNetworkInterface(name string) { + ifaces := b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces + filtered := ifaces[:0] + for _, iface := range ifaces { + if iface.Name != name { + filtered = append(filtered, iface) + } + } + b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces = filtered + + nets := b.Resource.Spec.Template.Spec.Networks + filteredNets := nets[:0] + for _, n := range nets { + if n.Name != name { + filteredNets = append(filteredNets, n) + } + } + b.Resource.Spec.Template.Spec.Networks = filteredNets +} + func (b *KVVM) SetNetworkInterface(name, macAddress string, acpiIndex int) { net := virtv1.Network{ Name: name, @@ -796,15 +825,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 8e18bf2031..72eddf3d10 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" ) @@ -250,3 +252,115 @@ 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 TestSetNetworkRemovesDefaultEntirely(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: "veth_n12345678", MAC: "aa:bb:cc:dd:ee:ff", ID: 2}, + }) + + for _, iface := range b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces { + if iface.Name == "default" { + t.Fatalf("default interface must be removed from KVVM template when Main is dropped (KubeVirt rejects State: absent on default)") + } + } + for _, n := range b.Resource.Spec.Template.Spec.Networks { + if n.Name == "default" { + t.Fatalf("default network entry must be removed alongside the interface") + } + } +} + +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 ba54337ff7..b97a1c19b6 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -340,7 +340,22 @@ 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 { + continue + } + if iface.Name == network.NameDefaultInterface { + kvvm.RemoveNetworkInterface(iface.Name) + continue + } + 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/network.go b/images/virtualization-artifact/pkg/controller/vm/internal/network.go index 9c852940cb..5f90551939 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/network.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/network.go @@ -20,6 +20,7 @@ import ( "context" "encoding/json" "fmt" + "slices" "strings" corev1 "k8s.io/api/core/v1" @@ -73,29 +74,53 @@ func (h *NetworkInterfaceHandler) Handle(ctx context.Context, s state.VirtualMac }() if !hasOnlyDefaultNetwork(vm) { - if !h.featureGate.Enabled(featuregates.SDN) { - cb.Status(metav1.ConditionFalse).Reason(vmcondition.ReasonSDNModuleDisabled).Message("For additional network interfaces, please enable SDN module") - return reconcile.Result{}, nil - } - - pods, err := s.Pods(ctx) - if err != nil { + if err := h.evaluateAdditionalNetworks(ctx, s, vm, cb); err != nil { return reconcile.Result{}, err } + } + + return h.UpdateNetworkStatus(ctx, s, vm) +} + +// evaluateAdditionalNetworks sets cb based on whether additional networks are +// usable: requires SDN feature gate, then that referenced Networks/ClusterNetworks +// are Ready, and finally that SDN reports the per-pod interfaces healthy. +func (h *NetworkInterfaceHandler) evaluateAdditionalNetworks(ctx context.Context, s state.VirtualMachineState, vm *v1alpha2.VirtualMachine, cb *conditions.ConditionBuilder) error { + if !h.featureGate.Enabled(featuregates.SDN) { + cb.Status(metav1.ConditionFalse).Reason(vmcondition.ReasonSDNModuleDisabled).Message("For additional network interfaces, please enable SDN module") + return nil + } - errMsg, err := extractNetworkStatusFromPods(pods) + var pending []string + for _, ns := range vm.Spec.Networks { + ready, err := network.IsNetworkSpecReady(ctx, s.Client(), vm.Namespace, ns) if err != nil { - return reconcile.Result{}, err + return err } - - if errMsg != "" { - cb.Status(metav1.ConditionFalse).Reason(vmcondition.ReasonNetworkNotReady).Message(errMsg) - } else { - cb.Status(metav1.ConditionTrue).Reason(vmcondition.ReasonNetworkReady).Message("") + if !ready { + pending = append(pending, network.SpecKey(ns)) } } + if len(pending) > 0 { + cb.Status(metav1.ConditionFalse).Reason(vmcondition.ReasonNetworkNotReady). + Message(fmt.Sprintf("Waiting for the following networks to become Ready: %s", strings.Join(pending, ", "))) + return nil + } - return h.UpdateNetworkStatus(ctx, s, vm) + pods, err := s.Pods(ctx) + if err != nil { + return err + } + errMsg, err := extractNetworkStatusFromPods(pods) + if err != nil { + return err + } + if errMsg != "" { + cb.Status(metav1.ConditionFalse).Reason(vmcondition.ReasonNetworkNotReady).Message(errMsg) + return nil + } + cb.Status(metav1.ConditionTrue).Reason(vmcondition.ReasonNetworkReady).Message("") + return nil } func hasOnlyDefaultNetwork(vm *v1alpha2.VirtualMachine) bool { @@ -108,13 +133,6 @@ func (h *NetworkInterfaceHandler) Name() string { } func (h *NetworkInterfaceHandler) UpdateNetworkStatus(ctx context.Context, s state.VirtualMachineState, vm *v1alpha2.VirtualMachine) (reconcile.Result, error) { - // check that vmmacName is not removed when deleting a network interface from the spec, as it is still in use - if len(vm.Status.Networks) > len(vm.Spec.Networks) { - if vm.Status.Phase != v1alpha2.MachinePending && vm.Status.Phase != v1alpha2.MachineStopped { - return reconcile.Result{}, nil - } - } - if hasOnlyDefaultNetwork(vm) { vm.Status.Networks = []v1alpha2.NetworksStatus{ { @@ -131,6 +149,11 @@ func (h *NetworkInterfaceHandler) UpdateNetworkStatus(ctx context.Context, s sta return reconcile.Result{}, err } + kvvmi, err := s.KVVMI(ctx) + if err != nil { + return reconcile.Result{}, err + } + macAddressesByInterfaceName := make(map[string]string) if kvvm != nil && kvvm.Status.PrintableStatus != virtv1.VirtualMachineStatusUnschedulable { for _, i := range kvvm.Spec.Template.Spec.Domain.Devices.Interfaces { @@ -170,6 +193,26 @@ func (h *NetworkInterfaceHandler) UpdateNetworkStatus(ctx context.Context, s sta }) } + // Network hot-unplug: retain a status entry the user removed from spec until + // KubeVirt fully detaches and drops the iface from KVVMI. The next reconcile + // then drops the entry, vmmac becomes unattached, deletion handler releases the MAC. + for _, prev := range vm.Status.Networks { + if prev.Type == v1alpha2.NetworksTypeMain || prev.MAC == "" { + continue + } + if slices.ContainsFunc(networksStatus, func(ns v1alpha2.NetworksStatus) bool { + return ns.Type == prev.Type && ns.Name == prev.Name + }) { + continue + } + if kvvmi == nil || !slices.ContainsFunc(kvvmi.Spec.Domain.Devices.Interfaces, func(i virtv1.Interface) bool { + return strings.EqualFold(i.MacAddress, prev.MAC) + }) { + continue + } + networksStatus = append(networksStatus, prev) + } + vm.Status.Networks = networksStatus return reconcile.Result{}, nil } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/network_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/network_test.go index 0ce49c45f7..f74e4977e3 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/network_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/network_test.go @@ -23,11 +23,13 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + commonnetwork "github.com/deckhouse/virtualization-controller/pkg/common/network" "github.com/deckhouse/virtualization-controller/pkg/common/testutil" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/reconciler" @@ -37,6 +39,20 @@ import ( "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" ) +func newReadyNetwork(name, namespace string) *unstructured.Unstructured { + u := &unstructured.Unstructured{} + u.SetGroupVersionKind(commonnetwork.NetworkGVK) + u.SetName(name) + u.SetNamespace(namespace) + _ = unstructured.SetNestedSlice(u.Object, []interface{}{ + map[string]interface{}{ + "type": "Ready", + "status": "True", + }, + }, "status", "conditions") + return u +} + var _ = Describe("NetworkInterfaceHandler", func() { const ( name = "vm" @@ -225,7 +241,7 @@ var _ = Describe("NetworkInterfaceHandler", func() { ] } ]` - fakeClient, resource, vmState = setupEnvironment(vm, vmPod, mac1) + fakeClient, resource, vmState = setupEnvironment(vm, vmPod, mac1, newReadyNetwork("test-network", namespace)) reconcile() newVM := &v1alpha2.VirtualMachine{} @@ -277,7 +293,7 @@ var _ = Describe("NetworkInterfaceHandler", func() { ] } ]` - fakeClient, resource, vmState = setupEnvironment(vm, vmPod, mac1) + fakeClient, resource, vmState = setupEnvironment(vm, vmPod, mac1, newReadyNetwork("test-network", namespace)) reconcile() newVM := &v1alpha2.VirtualMachine{} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go b/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go index fddc328bae..7a2ed6d969 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go @@ -41,6 +41,7 @@ import ( ) type VirtualMachineState interface { + Client() client.Client VirtualMachine() *reconciler.Resource[*v1alpha2.VirtualMachine, v1alpha2.VirtualMachineStatus] KVVM(ctx context.Context) (*virtv1.VirtualMachine, error) KVVMI(ctx context.Context) (*virtv1.VirtualMachineInstance, error) @@ -102,6 +103,10 @@ func (s *state) fill() { } } +func (s *state) Client() client.Client { + return s.client +} + func (s *state) Shared(fn func(s *Shared)) { fn(&s.shared) } 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 688b0f556d..2f35ada6a5 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -339,6 +339,15 @@ func (h *SyncKvvmHandler) syncKVVM(ctx context.Context, s state.VirtualMachineSt return true, nil case allChanges.IsEmpty(): + outOfSync, err := h.networksOutOfSync(ctx, s, kvvm) + if err != nil { + return false, fmt.Errorf("check network sync: %w", err) + } + if outOfSync { + if err := h.applyNetworkReadinessSync(ctx, s); err != nil { + return false, fmt.Errorf("apply network readiness sync: %w", err) + } + } return true, nil default: // Delay changes propagation to KVVM until user restarts VM. @@ -495,7 +504,11 @@ func MakeKVVMFromVMSpec(ctx context.Context, s state.VirtualMachineState) (*virt return nil, err } - networkSpec := network.CreateNetworkSpec(current, vmmacs) + filteredVM, err := filterReadyNetworks(ctx, s.Client(), current) + if err != nil { + return nil, err + } + networkSpec := network.CreateNetworkSpec(filteredVM, vmmacs) kvvmi, err := s.KVVMI(ctx) if err != nil { @@ -712,6 +725,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) } @@ -771,6 +801,142 @@ func (h *SyncKvvmHandler) isVMUnschedulable( return false } +func (h *SyncKvvmHandler) networksOutOfSync(ctx context.Context, s state.VirtualMachineState, kvvm *virtv1.VirtualMachine) (bool, error) { + if kvvm == nil { + return false, nil + } + filteredVM, err := filterReadyNetworks(ctx, s.Client(), s.VirtualMachine().Current()) + if err != nil { + return false, err + } + vmmacs, err := s.VirtualMachineMACAddresses(ctx) + if err != nil { + return false, err + } + desired := network.CreateNetworkSpec(filteredVM, vmmacs) + actual := make(map[string]struct{}) + for _, iface := range kvvm.Spec.Template.Spec.Domain.Devices.Interfaces { + if iface.State == virtv1.InterfaceStateAbsent { + continue + } + actual[iface.Name] = struct{}{} + } + if len(desired) != len(actual) { + return true, nil + } + for _, spec := range desired { + if _, ok := actual[spec.InterfaceName]; !ok { + return true, nil + } + } + return false, nil +} + +func (h *SyncKvvmHandler) applyNetworkReadinessSync(ctx context.Context, s state.VirtualMachineState) error { + if err := h.patchPodNetworkAnnotation(ctx, s); err != nil { + return fmt.Errorf("patch pod network annotation: %w", err) + } + ready, err := h.isNetworkReadyOnPod(ctx, s) + if err != nil { + return fmt.Errorf("check pod network status: %w", err) + } + if !ready { + logger.FromContext(ctx).Info("Waiting for SDN to configure network interfaces on the pod before updating KVVM") + return nil + } + return h.updateKVVM(ctx, s) +} + +func filterReadyNetworks(ctx context.Context, c client.Client, vm *v1alpha2.VirtualMachine) (*v1alpha2.VirtualMachine, error) { + if c == nil || vm == nil || len(vm.Spec.Networks) == 0 { + return vm, nil + } + kept := make([]v1alpha2.NetworksSpec, 0, len(vm.Spec.Networks)) + for _, ns := range vm.Spec.Networks { + ready, err := network.IsNetworkSpecReady(ctx, c, vm.Namespace, ns) + if err != nil { + return nil, fmt.Errorf("check readiness for network %s: %w", network.SpecKey(ns), err) + } + if ready { + kept = append(kept, ns) + } + } + if len(kept) == len(vm.Spec.Networks) { + return vm, nil + } + out := vm.DeepCopy() + out.Spec.Networks = kept + return out, nil +} + +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 + } + + filteredVM, err := filterReadyNetworks(ctx, s.Client(), current) + if err != nil { + return err + } + + networkConfigStr, err := network.CreateNetworkSpec(filteredVM, 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 hasNonHotpluggableVolumes(kvvmi *virtv1.VirtualMachineInstance) bool { if kvvmi == nil { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/networks_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/networks_validator.go index 1ff3085efb..21872742c5 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/networks_validator.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/networks_validator.go @@ -21,8 +21,12 @@ import ( "fmt" "k8s.io/apimachinery/pkg/api/equality" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" "k8s.io/component-base/featuregate" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" commonnetwork "github.com/deckhouse/virtualization-controller/pkg/common/network" @@ -31,11 +35,13 @@ import ( ) type NetworksValidator struct { + client client.Client featureGate featuregate.FeatureGate } -func NewNetworksValidator(featureGate featuregate.FeatureGate) *NetworksValidator { +func NewNetworksValidator(c client.Client, featureGate featuregate.FeatureGate) *NetworksValidator { return &NetworksValidator{ + client: c, featureGate: featureGate, } } @@ -53,7 +59,7 @@ func (v *NetworksValidator) ValidateCreate(_ context.Context, vm *v1alpha2.Virtu return v.validateNetworksSpec(networksSpec) } -func (v *NetworksValidator) ValidateUpdate(_ context.Context, oldVM, newVM *v1alpha2.VirtualMachine) (admission.Warnings, error) { +func (v *NetworksValidator) ValidateUpdate(ctx context.Context, oldVM, newVM *v1alpha2.VirtualMachine) (admission.Warnings, error) { newNetworksSpec := newVM.Spec.Networks if len(newNetworksSpec) == 0 { return nil, nil @@ -68,12 +74,67 @@ func (v *NetworksValidator) ValidateUpdate(_ context.Context, oldVM, newVM *v1al } isChanged := !equality.Semantic.DeepEqual(newNetworksSpec, oldVM.Spec.Networks) - if isChanged { - return v.validateNetworksSpec(newNetworksSpec) + if !isChanged { + return nil, nil + } + + if warn, err := v.validateNetworksSpec(newNetworksSpec); err != nil { + return warn, err + } + + added := networksAdded(oldVM.Spec.Networks, newNetworksSpec) + if len(added) == 0 { + return nil, nil + } + return v.validateNetworksExist(ctx, newVM.Namespace, added) +} + +func (v *NetworksValidator) validateNetworksExist(ctx context.Context, namespace string, networks []v1alpha2.NetworksSpec) (admission.Warnings, error) { + if v.client == nil { + return nil, nil + } + for _, n := range networks { + switch n.Type { + case v1alpha2.NetworksTypeClusterNetwork: + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(commonnetwork.ClusterNetworkGVK) + err := v.client.Get(ctx, types.NamespacedName{Name: n.Name}, obj) + if k8serrors.IsNotFound(err) { + return nil, fmt.Errorf("ClusterNetwork %q referenced in spec.networks does not exist", n.Name) + } + if err != nil { + return nil, fmt.Errorf("failed to verify ClusterNetwork %q: %w", n.Name, err) + } + case v1alpha2.NetworksTypeNetwork: + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(commonnetwork.NetworkGVK) + err := v.client.Get(ctx, types.NamespacedName{Name: n.Name, Namespace: namespace}, obj) + if k8serrors.IsNotFound(err) { + return nil, fmt.Errorf("Network %q referenced in spec.networks does not exist in namespace %q", n.Name, namespace) + } + if err != nil { + return nil, fmt.Errorf("failed to verify Network %q: %w", n.Name, err) + } + } } return nil, nil } +func networksAdded(oldSpec, newSpec []v1alpha2.NetworksSpec) []v1alpha2.NetworksSpec { + oldKey := func(n v1alpha2.NetworksSpec) string { return n.Type + "/" + n.Name } + old := make(map[string]struct{}, len(oldSpec)) + for _, n := range oldSpec { + old[oldKey(n)] = struct{}{} + } + var added []v1alpha2.NetworksSpec + for _, n := range newSpec { + if _, ok := old[oldKey(n)]; !ok { + added = append(added, n) + } + } + return added +} + func isSingleMainNet(networks []v1alpha2.NetworksSpec) bool { return len(networks) == 1 && networks[0].Type == v1alpha2.NetworksTypeMain } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/networks_validator_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/networks_validator_test.go index 08a6ddeced..347ee3591f 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/networks_validator_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/networks_validator_test.go @@ -20,8 +20,13 @@ import ( "fmt" "testing" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + commonnetwork "github.com/deckhouse/virtualization-controller/pkg/common/network" "github.com/deckhouse/virtualization-controller/pkg/featuregates" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -83,7 +88,7 @@ func TestNetworksValidateCreate(t *testing.T) { if test.sdnEnabled { _ = setFromMap(map[string]bool{string(featuregates.SDN): true}) } - networkValidator := NewNetworksValidator(featureGate) + networkValidator := NewNetworksValidator(nil, featureGate) _, err := networkValidator.ValidateCreate(t.Context(), vm) if test.valid && err != nil { @@ -330,7 +335,7 @@ func TestNetworksValidateUpdate(t *testing.T) { string(featuregates.SDN): true, }) } - networkValidator := NewNetworksValidator(featureGate) + networkValidator := NewNetworksValidator(nil, featureGate) _, err := networkValidator.ValidateUpdate(t.Context(), oldVM, newVM) if test.valid && err != nil { @@ -348,3 +353,85 @@ func TestNetworksValidateUpdate(t *testing.T) { }) } } + +func newUnstructured(gvk schema.GroupVersionKind, name, namespace string) *unstructured.Unstructured { + u := &unstructured.Unstructured{} + u.SetGroupVersionKind(gvk) + u.SetName(name) + if namespace != "" { + u.SetNamespace(namespace) + } + return u +} + +func TestNetworksValidatesExistence(t *testing.T) { + scheme := runtime.NewScheme() + scheme.AddKnownTypeWithName(commonnetwork.ClusterNetworkGVK, &unstructured.Unstructured{}) + scheme.AddKnownTypeWithName(commonnetwork.NetworkGVK, &unstructured.Unstructured{}) + + existingCN := newUnstructured(commonnetwork.ClusterNetworkGVK, "exists-cn", "") + existingNet := newUnstructured(commonnetwork.NetworkGVK, "exists-net", "default") + + cli := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(existingCN, existingNet). + Build() + + featureGate, _, setFromMap, _ := featuregates.New() + _ = setFromMap(map[string]bool{string(featuregates.SDN): true}) + v := NewNetworksValidator(cli, featureGate) + + t.Run("create: missing networks are allowed (no existence check)", func(t *testing.T) { + vm := &v1alpha2.VirtualMachine{} + vm.Namespace = "default" + vm.Spec.Networks = []v1alpha2.NetworksSpec{mainNetwork, {Type: v1alpha2.NetworksTypeClusterNetwork, Name: "ghost"}} + if _, err := v.ValidateCreate(t.Context(), vm); err != nil { + t.Fatalf("ValidateCreate must not check network existence; got: %v", err) + } + }) + + t.Run("update: adding existing ClusterNetwork passes", func(t *testing.T) { + oldVM := &v1alpha2.VirtualMachine{} + oldVM.Namespace = "default" + oldVM.Spec.Networks = []v1alpha2.NetworksSpec{mainNetwork} + newVM := oldVM.DeepCopy() + newVM.Spec.Networks = append(newVM.Spec.Networks, v1alpha2.NetworksSpec{Type: v1alpha2.NetworksTypeClusterNetwork, Name: "exists-cn"}) + if _, err := v.ValidateUpdate(t.Context(), oldVM, newVM); err != nil { + t.Fatalf("expected no error; got: %v", err) + } + }) + + t.Run("update: adding existing Network passes", func(t *testing.T) { + oldVM := &v1alpha2.VirtualMachine{} + oldVM.Namespace = "default" + oldVM.Spec.Networks = []v1alpha2.NetworksSpec{mainNetwork} + newVM := oldVM.DeepCopy() + newVM.Spec.Networks = append(newVM.Spec.Networks, v1alpha2.NetworksSpec{Type: v1alpha2.NetworksTypeNetwork, Name: "exists-net"}) + if _, err := v.ValidateUpdate(t.Context(), oldVM, newVM); err != nil { + t.Fatalf("expected no error; got: %v", err) + } + }) + + t.Run("update: existing networks are not re-checked", func(t *testing.T) { + oldVM := &v1alpha2.VirtualMachine{} + oldVM.Namespace = "default" + oldVM.Spec.Networks = []v1alpha2.NetworksSpec{mainNetwork, {Type: v1alpha2.NetworksTypeClusterNetwork, Name: "ghost"}} + newVM := oldVM.DeepCopy() + // Add another non-Main network; existing "ghost" should not be re-checked. + newVM.Spec.Networks = append(newVM.Spec.Networks, v1alpha2.NetworksSpec{Type: v1alpha2.NetworksTypeClusterNetwork, Name: "exists-cn"}) + if _, err := v.ValidateUpdate(t.Context(), oldVM, newVM); err != nil { + t.Fatalf("expected no error for adding an existing network when prior spec had a missing one; got: %v", err) + } + }) + + t.Run("update: adding a missing network is rejected", func(t *testing.T) { + oldVM := &v1alpha2.VirtualMachine{} + oldVM.Namespace = "default" + oldVM.Spec.Networks = []v1alpha2.NetworksSpec{mainNetwork, {Type: v1alpha2.NetworksTypeClusterNetwork, Name: "exists-cn"}} + newVM := oldVM.DeepCopy() + newVM.Spec.Networks = append(newVM.Spec.Networks, v1alpha2.NetworksSpec{Type: v1alpha2.NetworksTypeClusterNetwork, Name: "ghost"}) + if _, err := v.ValidateUpdate(t.Context(), oldVM, newVM); err == nil { + t.Fatalf("expected error when adding a missing network") + } + }) +} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/watcher/network_watcher.go b/images/virtualization-artifact/pkg/controller/vm/internal/watcher/network_watcher.go new file mode 100644 index 0000000000..436c910987 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vm/internal/watcher/network_watcher.go @@ -0,0 +1,121 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package watcher + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" + + "github.com/deckhouse/deckhouse/pkg/log" + commonnetwork "github.com/deckhouse/virtualization-controller/pkg/common/network" + "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +// NewNetworkWatcher enqueues VMs that reference a Network or ClusterNetwork +// when that network's Ready condition changes. +func NewNetworkWatcher(c client.Client) *NetworkWatcher { + return &NetworkWatcher{client: c} +} + +type NetworkWatcher struct { + client client.Client +} + +func (w *NetworkWatcher) Watch(mgr manager.Manager, ctr controller.Controller) error { + if err := w.watchOne(mgr, ctr, commonnetwork.ClusterNetworkGVK, indexer.IndexFieldVMByClusterNetwork, true); err != nil { + return err + } + return w.watchOne(mgr, ctr, commonnetwork.NetworkGVK, indexer.IndexFieldVMByNetwork, false) +} + +func (w *NetworkWatcher) watchOne(mgr manager.Manager, ctr controller.Controller, gvk schema.GroupVersionKind, indexField string, clusterScoped bool) error { + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + + if err := ctr.Watch( + source.Kind( + mgr.GetCache(), + obj, + handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, o *unstructured.Unstructured) []reconcile.Request { + return w.enqueueVMsReferencingNetwork(ctx, o, indexField, clusterScoped) + }), + predicate.TypedFuncs[*unstructured.Unstructured]{ + CreateFunc: func(e event.TypedCreateEvent[*unstructured.Unstructured]) bool { return true }, + DeleteFunc: func(e event.TypedDeleteEvent[*unstructured.Unstructured]) bool { return true }, + UpdateFunc: func(e event.TypedUpdateEvent[*unstructured.Unstructured]) bool { + return readyConditionStatus(e.ObjectOld) != readyConditionStatus(e.ObjectNew) + }, + }, + ), + ); err != nil { + return fmt.Errorf("error setting watch on %s: %w", gvk.Kind, err) + } + return nil +} + +func (w *NetworkWatcher) enqueueVMsReferencingNetwork(ctx context.Context, obj *unstructured.Unstructured, indexField string, clusterScoped bool) []reconcile.Request { + var vms v1alpha2.VirtualMachineList + listOpts := []client.ListOption{client.MatchingFields{indexField: obj.GetName()}} + if !clusterScoped { + listOpts = append(listOpts, client.InNamespace(obj.GetNamespace())) + } + if err := w.client.List(ctx, &vms, listOpts...); err != nil { + log.Default().Error(fmt.Sprintf("network watcher: failed to list VMs: %s", err)) + return nil + } + + requests := make([]reconcile.Request, 0, len(vms.Items)) + for _, vm := range vms.Items { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{Name: vm.Name, Namespace: vm.Namespace}, + }) + } + return requests +} + +func readyConditionStatus(obj *unstructured.Unstructured) string { + conds, found, err := unstructured.NestedSlice(obj.Object, "status", "conditions") + if err != nil || !found { + return "" + } + for _, c := range conds { + m, ok := c.(map[string]any) + if !ok { + continue + } + t, _, _ := unstructured.NestedString(m, "type") + if t != "Ready" { + continue + } + s, _, _ := unstructured.NestedString(m, "status") + return s + } + return "" +} diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go b/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go index de25ef3382..95b5f018c8 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go @@ -76,6 +76,7 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr watcher.NewVMOPWatcher(), watcher.NewVMMACWatcher(), watcher.NewSecretWatcher(mgr.GetClient()), + watcher.NewNetworkWatcher(mgr.GetClient()), } { err := w.Watch(mgr, ctr) if err != nil { diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go b/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go index ac4ab38cd5..d3afa74586 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go @@ -53,7 +53,7 @@ func NewValidator(client client.Client, blockDeviceService *service.BlockDeviceS validators.NewAffinityValidator(), validators.NewTopologySpreadConstraintValidator(), validators.NewCPUCountValidator(), - validators.NewNetworksValidator(featureGate), + validators.NewNetworksValidator(client, featureGate), validators.NewFirstDiskValidator(client), validators.NewUSBDevicesValidator(client, featureGate), validators.NewVMBDAConflictValidator(client), 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..f278b473e1 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,17 @@ 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 { + return hasMainNetwork(current) == hasMainNetwork(desired) +} + +func hasMainNetwork(networks []v1alpha2.NetworksSpec) bool { + return len(networks) == 0 || network.GetMainNetworkSpec(networks) != nil +} + 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 462f753abe..0020fdcd2b 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go @@ -535,6 +535,155 @@ 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 +`, + nil, + 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 +`, + nil, + 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 +`, + nil, + 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 +`, + nil, + assertChanges( + actionRequired(ActionRestart), + requirePathOperation("networks", ChangeReplace), + ), + }, + { + "apply immediate when removing non-main network from VM without main", + ` +networks: +- type: ClusterNetwork + name: net1 + id: 2 +- type: ClusterNetwork + name: net2 + id: 3 +`, + ` +networks: +- type: ClusterNetwork + name: net1 + id: 2 +`, + nil, + assertChanges( + actionRequired(ActionApplyImmediate), + requirePathOperation("networks", ChangeReplace), + ), + }, + { + "apply immediate when adding non-main network to VM without main", + ` +networks: +- type: ClusterNetwork + name: net1 + id: 2 +`, + ` +networks: +- type: ClusterNetwork + name: net1 + id: 2 +- type: ClusterNetwork + name: net2 + id: 3 +`, + nil, + assertChanges( + actionRequired(ActionApplyImmediate), + requirePathOperation("networks", ChangeReplace), + ), + }, + { + "restart when only-non-main spec gains main network", + ` +networks: +- type: ClusterNetwork + name: net1 + id: 2 +`, + ` +networks: +- type: Main + id: 1 +- type: ClusterNetwork + name: net1 + id: 2 +`, + nil, + assertChanges( + actionRequired(ActionRestart), + requirePathOperation("networks", ChangeReplace), + ), + }, } for _, tt := range tests { diff --git a/templates/virtualization-controller/rbac-for-us.yaml b/templates/virtualization-controller/rbac-for-us.yaml index 8bc92dc654..fd9a6efacd 100644 --- a/templates/virtualization-controller/rbac-for-us.yaml +++ b/templates/virtualization-controller/rbac-for-us.yaml @@ -327,6 +327,15 @@ rules: - get - update - patch +- apiGroups: + - network.deckhouse.io + resources: + - networks + - clusternetworks + verbs: + - get + - list + - watch --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding diff --git a/test/e2e/vm/additional_network_interfaces.go b/test/e2e/vm/additional_network_interfaces.go index 734b549e66..3a3c199d52 100644 --- a/test/e2e/vm/additional_network_interfaces.go +++ b/test/e2e/vm/additional_network_interfaces.go @@ -20,12 +20,14 @@ import ( "context" "encoding/json" "fmt" + "strconv" "strings" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" crclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -255,6 +257,116 @@ var _ = Describe("VirtualMachineAdditionalNetworkInterfaces", Label(precheck.NoP }) }) }) + + Describe("verifies hotplug and hotunplug of additional network interfaces", func() { + const countNonLoopbackInterfacesCmd = "ip -o link show | grep -v 'lo:' | wc -l" + + var ( + vdRoot *v1alpha2.VirtualDisk + testVM *v1alpha2.VirtualMachine + ) + + getIfaceCount := func() int { + GinkgoHelper() + output, err := f.SSHCommand(testVM.Name, testVM.Namespace, countNonLoopbackInterfacesCmd) + Expect(err).NotTo(HaveOccurred()) + count, err := strconv.Atoi(strings.TrimSpace(output)) + Expect(err).NotTo(HaveOccurred()) + return count + } + + expectNoRestartRequired := func() { + GinkgoHelper() + Consistently(func(g Gomega) { + err := f.Clients.GenericClient().Get(context.Background(), crclient.ObjectKeyFromObject(testVM), testVM) + g.Expect(err).NotTo(HaveOccurred()) + cond, _ := conditions.GetCondition(vmcondition.TypeAwaitingRestartToApplyConfiguration, testVM.Status.Conditions) + g.Expect(cond.Status).NotTo(Equal(metav1.ConditionTrue), + "VM should not require restart for non-Main network change") + }).WithTimeout(10 * time.Second).WithPolling(2 * time.Second).Should(Succeed()) + } + + It("should attach and detach ClusterNetwork on a running VM without reboot", func() { + var initialIfaceCount int + + By("Create VM with only Main network", func() { + ns := f.Namespace().Name + vdRoot = object.NewVDFromCVI("vd-root", ns, object.PrecreatedCVIUbuntu) + + testVM = vm.New( + vm.WithName("vm-hotplug"), + vm.WithNamespace(ns), + vm.WithBootloader(v1alpha2.EFI), + vm.WithCPU(1, ptr.To("5%")), + vm.WithMemory(resource.MustParse("512Mi")), + vm.WithRestartApprovalMode(v1alpha2.Manual), + vm.WithVirtualMachineClass(object.DefaultVMClass), + vm.WithLiveMigrationPolicy(v1alpha2.PreferSafeMigrationPolicy), + vm.WithProvisioningUserData(object.UbuntuCloudInit), + vm.WithBlockDeviceRefs(v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.VirtualDiskKind, + Name: vdRoot.Name, + }), + vm.WithNetwork(v1alpha2.NetworksSpec{Type: v1alpha2.NetworksTypeMain}), + ) + + err := f.CreateWithDeferredDeletion(context.Background(), vdRoot, testVM) + Expect(err).NotTo(HaveOccurred()) + + util.UntilObjectPhase(string(v1alpha2.MachineRunning), framework.LongTimeout, testVM) + util.UntilVMAgentReady(crclient.ObjectKeyFromObject(testVM), framework.LongTimeout) + util.UntilSSHReady(f, testVM, framework.LongTimeout) + + initialIfaceCount = getIfaceCount() + Expect(initialIfaceCount).To(BeNumerically(">=", 1), + "VM should have at least one non-loopback interface") + }) + + By("Hotplug: add a ClusterNetwork to spec.networks", func() { + err := f.Clients.GenericClient().Get(context.Background(), crclient.ObjectKeyFromObject(testVM), testVM) + Expect(err).NotTo(HaveOccurred()) + testVM.Spec.Networks = append(testVM.Spec.Networks, v1alpha2.NetworksSpec{ + Type: v1alpha2.NetworksTypeClusterNetwork, + Name: util.ClusterNetworkName(additionalInterfaceVLANID), + }) + err = f.Clients.GenericClient().Update(context.Background(), testVM) + Expect(err).NotTo(HaveOccurred()) + }) + + By("Verify new interface appears in the guest OS", func() { + util.UntilConditionStatus(vmcondition.TypeNetworkReady.String(), "True", framework.LongTimeout, testVM) + Eventually(getIfaceCount). + WithTimeout(framework.LongTimeout). + WithPolling(3*time.Second). + Should(Equal(initialIfaceCount+1), "new interface should appear after hotplug") + }) + + By("Verify VM did not ask for restart after hotplug", expectNoRestartRequired) + + By("Hotunplug: remove the ClusterNetwork from spec.networks", func() { + err := f.Clients.GenericClient().Get(context.Background(), crclient.ObjectKeyFromObject(testVM), testVM) + Expect(err).NotTo(HaveOccurred()) + testVM.Spec.Networks = []v1alpha2.NetworksSpec{testVM.Spec.Networks[0]} + err = f.Clients.GenericClient().Update(context.Background(), testVM) + Expect(err).NotTo(HaveOccurred()) + }) + + By("Verify interface disappears from the guest OS", func() { + Eventually(getIfaceCount). + WithTimeout(framework.LongTimeout). + WithPolling(3*time.Second). + Should(Equal(initialIfaceCount), "interface should disappear after hotunplug") + }) + + By("Verify VM did not ask for restart after hotunplug", expectNoRestartRequired) + + By("Verify VM phase stayed Running throughout", func() { + err := f.Clients.GenericClient().Get(context.Background(), crclient.ObjectKeyFromObject(testVM), testVM) + Expect(err).NotTo(HaveOccurred()) + Expect(string(testVM.Status.Phase)).To(Equal(string(v1alpha2.MachineRunning))) + }) + }) + }) }) // buildVMWithNetworks creates a VM with optional Main + ClusterNetwork.