diff --git a/api/core/v1alpha2/virtual_machine.go b/api/core/v1alpha2/virtual_machine.go index b7895501f5..6ae2338e5a 100644 --- a/api/core/v1alpha2/virtual_machine.go +++ b/api/core/v1alpha2/virtual_machine.go @@ -262,6 +262,7 @@ const ( ) type NetworksSpec struct { + Id int `json:"id,omitempty"` Type string `json:"type"` Name string `json:"name,omitempty"` VirtualMachineMACAddressName string `json:"virtualMachineMACAddressName,omitempty"` @@ -423,6 +424,7 @@ type Versions struct { } type NetworksStatus struct { + Id int `json:"id,omitempty"` Type string `json:"type"` Name string `json:"name,omitempty"` MAC string `json:"macAddress,omitempty"` diff --git a/crds/doc-ru-virtualmachines.yaml b/crds/doc-ru-virtualmachines.yaml index 1979a58ba6..b34d2eade0 100644 --- a/crds/doc-ru-virtualmachines.yaml +++ b/crds/doc-ru-virtualmachines.yaml @@ -550,6 +550,9 @@ spec: Список конфигураций сетевых интерфейсов. items: properties: + id: + description: | + ID сетевого интерфейса. type: description: | Тип сетевого интерфейса. @@ -743,6 +746,9 @@ spec: Список сетевых интерфейсов, подключенных к ВМ. items: properties: + id: + description: | + ID сетевого интерфейса. type: description: | Тип сетевого интерфейса. diff --git a/crds/virtualmachines.yaml b/crds/virtualmachines.yaml index c8234cf73f..82a4f44102 100644 --- a/crds/virtualmachines.yaml +++ b/crds/virtualmachines.yaml @@ -981,6 +981,10 @@ spec: required: - type properties: + id: + type: integer + description: | + Network interface ID. type: type: string description: | @@ -1333,6 +1337,10 @@ spec: required: - type properties: + id: + type: integer + description: | + Network interface ID. type: type: string description: | diff --git a/images/virtualization-artifact/pkg/common/network/network.go b/images/virtualization-artifact/pkg/common/network/network.go index ba92f1e812..47075655ed 100644 --- a/images/virtualization-artifact/pkg/common/network/network.go +++ b/images/virtualization-artifact/pkg/common/network/network.go @@ -50,6 +50,7 @@ func HasMainNetworkSpec(networks []v1alpha2.NetworksSpec) bool { } type InterfaceSpec struct { + ID int `json:"id"` Type string `json:"type"` Name string `json:"name"` InterfaceName string `json:"ifName"` @@ -96,6 +97,7 @@ func CreateNetworkSpec(vm *v1alpha2.VirtualMachine, vmmacs []*v1alpha2.VirtualMa for _, n := range vm.Spec.Networks { if n.Type == v1alpha2.NetworksTypeMain { res = append(res, InterfaceSpec{ + ID: n.Id, Type: n.Type, Name: n.Name, InterfaceName: NameDefaultInterface, @@ -117,6 +119,7 @@ func CreateNetworkSpec(vm *v1alpha2.VirtualMachine, vmmacs []*v1alpha2.VirtualMa } if mac != "" { res = append(res, InterfaceSpec{ + ID: n.Id, Type: n.Type, Name: n.Name, InterfaceName: generateInterfaceName(mac, n.Type), diff --git a/images/virtualization-artifact/pkg/common/network/network_test.go b/images/virtualization-artifact/pkg/common/network/network_test.go index 7643aa0ed3..268673e911 100644 --- a/images/virtualization-artifact/pkg/common/network/network_test.go +++ b/images/virtualization-artifact/pkg/common/network/network_test.go @@ -78,6 +78,7 @@ var _ = Describe("Network Config Generation", func() { Expect(configs[0].Name).To(Equal("")) Expect(configs[0].InterfaceName).To(HavePrefix("default")) Expect(configs[0].MAC).To(HavePrefix("")) + Expect(configs[0].ID).To(Equal(0)) }) It("should generate correct interface name for Network type", func() { @@ -97,10 +98,12 @@ var _ = Describe("Network Config Generation", func() { Expect(configs[0].Name).To(Equal("")) Expect(configs[0].InterfaceName).To(HavePrefix("default")) Expect(configs[0].MAC).To(HavePrefix("")) + Expect(configs[0].ID).To(Equal(0)) Expect(configs[1].Type).To(Equal(v1alpha2.NetworksTypeNetwork)) Expect(configs[1].Name).To(Equal("mynet")) Expect(configs[1].InterfaceName).To(HavePrefix("veth_n")) + Expect(configs[1].ID).To(Equal(0)) }) It("should generate correct interface name for ClusterNetwork type", func() { @@ -281,4 +284,159 @@ var _ = Describe("Network Config Generation", func() { Expect(configs[3].Name).To(Equal("name1")) Expect(configs[3].MAC).To(Equal("00:1A:2B:3C:4D:6A")) }) + + It("should preserve id from spec for Main network", func() { + vm.Spec.Networks = []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 1, + }, + } + + configs := CreateNetworkSpec(vm, vmmacs) + + Expect(configs).To(HaveLen(1)) + Expect(configs[0].ID).To(Equal(1)) + }) + + It("should preserve id from spec for Main network", func() { + vm.Spec.Networks = []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 1, + }, + } + + configs := CreateNetworkSpec(vm, vmmacs) + + Expect(configs).To(HaveLen(1)) + Expect(configs[0].ID).To(Equal(1)) + }) + + It("should preserve id from spec for Network type with MAC", func() { + vm.Status.Networks = []v1alpha2.NetworksStatus{ + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "mynet", + MAC: "00:1A:2B:3C:4D:5E", + }, + } + vmmac1 := newMACAddress("mac1", "00:1A:2B:3C:4D:5E", v1alpha2.VirtualMachineMACAddressPhaseAttached, "vm1") + vmmacs = []*v1alpha2.VirtualMachineMACAddress{vmmac1} + + vm.Spec.Networks = []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 1, + }, + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "mynet", + Id: 5, + }, + } + + configs := CreateNetworkSpec(vm, vmmacs) + + Expect(configs).To(HaveLen(2)) + Expect(configs[0].ID).To(Equal(1)) + Expect(configs[1].ID).To(Equal(5)) + }) + + It("should preserve id from spec for ClusterNetwork type with MAC", func() { + vm.Status.Networks = []v1alpha2.NetworksStatus{ + { + Type: v1alpha2.NetworksTypeClusterNetwork, + Name: "clusternet", + MAC: "00:1A:2B:3C:4D:5E", + }, + } + vmmac1 := newMACAddress("mac1", "00:1A:2B:3C:4D:5E", v1alpha2.VirtualMachineMACAddressPhaseAttached, "vm1") + vmmacs = []*v1alpha2.VirtualMachineMACAddress{vmmac1} + + vm.Spec.Networks = []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 1, + }, + { + Type: v1alpha2.NetworksTypeClusterNetwork, + Name: "clusternet", + Id: 20, + }, + } + + configs := CreateNetworkSpec(vm, vmmacs) + + Expect(configs).To(HaveLen(2)) + Expect(configs[0].ID).To(Equal(1)) + Expect(configs[1].ID).To(Equal(20)) + }) + + It("should preserve different ids for multiple networks with MACs", func() { + vm.Status.Networks = []v1alpha2.NetworksStatus{ + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "net1", + MAC: "00:1A:2B:3C:4D:5E", + }, + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "net2", + MAC: "00:1A:2B:3C:4D:5F", + }, + { + Type: v1alpha2.NetworksTypeClusterNetwork, + Name: "cluster1", + MAC: "00:1A:2B:3C:4D:6A", + }, + } + vmmac1 := newMACAddress("mac1", "00:1A:2B:3C:4D:5E", v1alpha2.VirtualMachineMACAddressPhaseAttached, "vm1") + vmmac2 := newMACAddress("mac2", "00:1A:2B:3C:4D:5F", v1alpha2.VirtualMachineMACAddressPhaseAttached, "vm1") + vmmac3 := newMACAddress("mac3", "00:1A:2B:3C:4D:6A", v1alpha2.VirtualMachineMACAddressPhaseAttached, "vm1") + vmmacs = []*v1alpha2.VirtualMachineMACAddress{vmmac1, vmmac2, vmmac3} + + vm.Spec.Networks = []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 1, + }, + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "net1", + Id: 2, + }, + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "net2", + Id: 3, + }, + { + Type: v1alpha2.NetworksTypeClusterNetwork, + Name: "cluster1", + Id: 4, + }, + } + + configs := CreateNetworkSpec(vm, vmmacs) + + Expect(configs).To(HaveLen(4)) + Expect(configs[0].ID).To(Equal(1)) + Expect(configs[1].ID).To(Equal(2)) + Expect(configs[2].ID).To(Equal(3)) + Expect(configs[3].ID).To(Equal(4)) + }) + + It("should set id to zero when not specified", func() { + vm.Spec.Networks = []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + }, + } + + configs := CreateNetworkSpec(vm, vmmacs) + + Expect(configs).To(HaveLen(1)) + Expect(configs[0].ID).To(Equal(0)) + }) }) diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go index 606fae973b..7084be8e20 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go @@ -554,7 +554,7 @@ func (b *KVVM) ClearNetworkInterfaces() { b.Resource.Spec.Template.Spec.Domain.Devices.Interfaces = nil } -func (b *KVVM) SetNetworkInterface(name, macAddress string) { +func (b *KVVM) SetNetworkInterface(name, macAddress string, acpiIndex int) { net := virtv1.Network{ Name: name, NetworkSource: virtv1.NetworkSource{ @@ -571,8 +571,9 @@ func (b *KVVM) SetNetworkInterface(name, macAddress string) { devPreset := DeviceOptionsPresets.Find(b.opts.EnableParavirtualization) iface := virtv1.Interface{ - Name: name, - Model: devPreset.InterfaceModel, + Name: name, + Model: devPreset.InterfaceModel, + ACPIIndex: acpiIndex, } iface.InterfaceBindingMethod.Bridge = &virtv1.InterfaceBridge{} if macAddress != "" { diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go index 284c21eaa1..6d963e0fea 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -93,7 +93,6 @@ func ApplyVirtualMachineSpec( vdByName map[string]*v1alpha2.VirtualDisk, viByName map[string]*v1alpha2.VirtualImage, cviByName map[string]*v1alpha2.ClusterVirtualImage, - vmbdas map[v1alpha2.VMBDAObjectRef][]*v1alpha2.VirtualMachineBlockDeviceAttachment, class *v1alpha2.VirtualMachineClass, ipAddress string, networkSpec network.InterfaceSpecList, @@ -356,7 +355,7 @@ func ApplyMigrationVolumes(kvvm *KVVM, vm *v1alpha2.VirtualMachine, vdsByName ma func setNetwork(kvvm *KVVM, networkSpec network.InterfaceSpecList) { kvvm.ClearNetworkInterfaces() for _, n := range networkSpec { - kvvm.SetNetworkInterface(n.InterfaceName, n.MAC) + 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 51da70ea0c..6e7eb0fe39 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/network.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/network.go @@ -95,9 +95,41 @@ func (h *NetworkInterfaceHandler) Handle(ctx context.Context, s state.VirtualMac } } + h.lazyInitialization(vm) return h.UpdateNetworkStatus(ctx, s, vm) } +func (h *NetworkInterfaceHandler) lazyInitialization(vm *v1alpha2.VirtualMachine) { + // First pass: assign id=1 to Main network if it has id=0 + for i := range vm.Spec.Networks { + if vm.Spec.Networks[i].Type == v1alpha2.NetworksTypeMain && vm.Spec.Networks[i].Id == 0 { + vm.Spec.Networks[i].Id = 1 + } + } + + // Second pass: assign sequential ids starting from 2 to other networks with id=0 + nextID := 2 + for i := range vm.Spec.Networks { + if vm.Spec.Networks[i].Type != v1alpha2.NetworksTypeMain && vm.Spec.Networks[i].Id == 0 { + vm.Spec.Networks[i].Id = nextID + nextID++ + // Ensure we never use id=1 (reserved for Main) + if nextID == 1 { + nextID = 2 + } + } else if vm.Spec.Networks[i].Id > 0 { + // Track the highest ID used to avoid conflicts + if vm.Spec.Networks[i].Id >= nextID { + nextID = vm.Spec.Networks[i].Id + 1 + // Ensure we never use id=1 (reserved for Main) + if nextID == 1 { + nextID = 2 + } + } + } + } +} + func hasOnlyDefaultNetwork(vm *v1alpha2.VirtualMachine) bool { nets := vm.Spec.Networks return len(nets) == 0 || (len(nets) == 1 && nets[0].Type == v1alpha2.NetworksTypeMain) @@ -118,6 +150,7 @@ func (h *NetworkInterfaceHandler) UpdateNetworkStatus(ctx context.Context, s sta if hasOnlyDefaultNetwork(vm) { vm.Status.Networks = []v1alpha2.NetworksStatus{ { + Id: 1, Type: v1alpha2.NetworksTypeMain, Name: network.NameDefaultInterface, }, @@ -153,6 +186,7 @@ func (h *NetworkInterfaceHandler) UpdateNetworkStatus(ctx context.Context, s sta for _, interfaceSpec := range network.CreateNetworkSpec(vm, vmmacs) { if interfaceSpec.Type == v1alpha2.NetworksTypeMain { networksStatus = append(networksStatus, v1alpha2.NetworksStatus{ + Id: interfaceSpec.ID, Type: v1alpha2.NetworksTypeMain, Name: network.NameDefaultInterface, }) @@ -160,6 +194,7 @@ func (h *NetworkInterfaceHandler) UpdateNetworkStatus(ctx context.Context, s sta } networksStatus = append(networksStatus, v1alpha2.NetworksStatus{ + Id: interfaceSpec.ID, Type: interfaceSpec.Type, Name: interfaceSpec.Name, MAC: macAddressesByInterfaceName[interfaceSpec.InterfaceName], 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..d8e0696ce6 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/network_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/network_test.go @@ -292,4 +292,298 @@ var _ = Describe("NetworkInterfaceHandler", func() { }) }) }) + + Describe("Lazy initialization of network IDs", func() { + It("should assign id=1 to Main network when id=0", func() { + networkSpec := []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 0, + }, + } + vm.Spec.Networks = networkSpec + fakeClient, resource, vmState = setupEnvironment(vm, vmPod) + + gate, _, setFromMap, err := featuregates.New() + Expect(err).NotTo(HaveOccurred()) + Expect(setFromMap(map[string]bool{string(featuregates.SDN): true})).To(Succeed()) + + h := NewNetworkInterfaceHandler(gate) + _, err = h.Handle(ctx, vmState) + Expect(err).NotTo(HaveOccurred()) + + changedVM := vmState.VirtualMachine().Changed() + Expect(changedVM.Spec.Networks).To(HaveLen(1)) + Expect(changedVM.Spec.Networks[0].Type).To(Equal(v1alpha2.NetworksTypeMain)) + Expect(changedVM.Spec.Networks[0].Id).To(Equal(1)) + }) + + It("should not change Main network id when it is already set to 1", func() { + networkSpec := []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 1, + }, + } + vm.Spec.Networks = networkSpec + fakeClient, resource, vmState = setupEnvironment(vm, vmPod) + + gate, _, setFromMap, err := featuregates.New() + Expect(err).NotTo(HaveOccurred()) + Expect(setFromMap(map[string]bool{string(featuregates.SDN): true})).To(Succeed()) + + h := NewNetworkInterfaceHandler(gate) + _, err = h.Handle(ctx, vmState) + Expect(err).NotTo(HaveOccurred()) + + changedVM := vmState.VirtualMachine().Changed() + Expect(changedVM.Spec.Networks).To(HaveLen(1)) + Expect(changedVM.Spec.Networks[0].Type).To(Equal(v1alpha2.NetworksTypeMain)) + Expect(changedVM.Spec.Networks[0].Id).To(Equal(1)) + }) + + It("should assign sequential ids starting from 2 to networks with id=0", func() { + mac1 := newMACAddress("test-mac-address1", "aa:bb:cc:dd:ee:ff", v1alpha2.VirtualMachineMACAddressPhaseAttached, name) + mac2 := newMACAddress("test-mac-address2", "aa:bb:cc:dd:ee:00", v1alpha2.VirtualMachineMACAddressPhaseAttached, name) + networkSpec := []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 0, + }, + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "test-network-1", + Id: 0, + }, + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "test-network-2", + Id: 0, + }, + } + vm.Spec.Networks = networkSpec + fakeClient, resource, vmState = setupEnvironment(vm, vmPod, mac1, mac2) + + gate, _, setFromMap, err := featuregates.New() + Expect(err).NotTo(HaveOccurred()) + Expect(setFromMap(map[string]bool{string(featuregates.SDN): true})).To(Succeed()) + + h := NewNetworkInterfaceHandler(gate) + _, err = h.Handle(ctx, vmState) + Expect(err).NotTo(HaveOccurred()) + + changedVM := vmState.VirtualMachine().Changed() + Expect(changedVM.Spec.Networks).To(HaveLen(3)) + Expect(changedVM.Spec.Networks[0].Type).To(Equal(v1alpha2.NetworksTypeMain)) + Expect(changedVM.Spec.Networks[0].Id).To(Equal(1)) + Expect(changedVM.Spec.Networks[1].Type).To(Equal(v1alpha2.NetworksTypeNetwork)) + Expect(changedVM.Spec.Networks[1].Name).To(Equal("test-network-1")) + Expect(changedVM.Spec.Networks[1].Id).To(Equal(2)) + Expect(changedVM.Spec.Networks[2].Type).To(Equal(v1alpha2.NetworksTypeNetwork)) + Expect(changedVM.Spec.Networks[2].Name).To(Equal("test-network-2")) + Expect(changedVM.Spec.Networks[2].Id).To(Equal(3)) + }) + + It("should not change network id when it is already set", func() { + mac1 := newMACAddress("test-mac-address1", "aa:bb:cc:dd:ee:ff", v1alpha2.VirtualMachineMACAddressPhaseAttached, name) + networkSpec := []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 1, + }, + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "test-network", + Id: 5, + }, + } + vm.Spec.Networks = networkSpec + fakeClient, resource, vmState = setupEnvironment(vm, vmPod, mac1) + + gate, _, setFromMap, err := featuregates.New() + Expect(err).NotTo(HaveOccurred()) + Expect(setFromMap(map[string]bool{string(featuregates.SDN): true})).To(Succeed()) + + h := NewNetworkInterfaceHandler(gate) + _, err = h.Handle(ctx, vmState) + Expect(err).NotTo(HaveOccurred()) + + changedVM := vmState.VirtualMachine().Changed() + Expect(changedVM.Spec.Networks).To(HaveLen(2)) + Expect(changedVM.Spec.Networks[0].Type).To(Equal(v1alpha2.NetworksTypeMain)) + Expect(changedVM.Spec.Networks[0].Id).To(Equal(1)) + Expect(changedVM.Spec.Networks[1].Type).To(Equal(v1alpha2.NetworksTypeNetwork)) + Expect(changedVM.Spec.Networks[1].Id).To(Equal(5)) + }) + + It("should assign sequential ids considering already set ids", func() { + mac1 := newMACAddress("test-mac-address1", "aa:bb:cc:dd:ee:ff", v1alpha2.VirtualMachineMACAddressPhaseAttached, name) + mac2 := newMACAddress("test-mac-address2", "aa:bb:cc:dd:ee:00", v1alpha2.VirtualMachineMACAddressPhaseAttached, name) + networkSpec := []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 0, + }, + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "test-network-1", + Id: 5, // Already set + }, + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "test-network-2", + Id: 0, // Should get next available id after 5 + }, + } + vm.Spec.Networks = networkSpec + fakeClient, resource, vmState = setupEnvironment(vm, vmPod, mac1, mac2) + + gate, _, setFromMap, err := featuregates.New() + Expect(err).NotTo(HaveOccurred()) + Expect(setFromMap(map[string]bool{string(featuregates.SDN): true})).To(Succeed()) + + h := NewNetworkInterfaceHandler(gate) + _, err = h.Handle(ctx, vmState) + Expect(err).NotTo(HaveOccurred()) + + changedVM := vmState.VirtualMachine().Changed() + Expect(changedVM.Spec.Networks).To(HaveLen(3)) + Expect(changedVM.Spec.Networks[0].Type).To(Equal(v1alpha2.NetworksTypeMain)) + Expect(changedVM.Spec.Networks[0].Id).To(Equal(1)) + Expect(changedVM.Spec.Networks[1].Type).To(Equal(v1alpha2.NetworksTypeNetwork)) + Expect(changedVM.Spec.Networks[1].Name).To(Equal("test-network-1")) + Expect(changedVM.Spec.Networks[1].Id).To(Equal(5)) + Expect(changedVM.Spec.Networks[2].Type).To(Equal(v1alpha2.NetworksTypeNetwork)) + Expect(changedVM.Spec.Networks[2].Name).To(Equal("test-network-2")) + Expect(changedVM.Spec.Networks[2].Id).To(Equal(6)) + }) + + It("should handle ClusterNetwork type correctly", func() { + mac1 := newMACAddress("test-mac-address1", "aa:bb:cc:dd:ee:ff", v1alpha2.VirtualMachineMACAddressPhaseAttached, name) + networkSpec := []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 0, + }, + { + Type: v1alpha2.NetworksTypeClusterNetwork, + Name: "test-cluster-network", + Id: 0, + }, + } + vm.Spec.Networks = networkSpec + fakeClient, resource, vmState = setupEnvironment(vm, vmPod, mac1) + + gate, _, setFromMap, err := featuregates.New() + Expect(err).NotTo(HaveOccurred()) + Expect(setFromMap(map[string]bool{string(featuregates.SDN): true})).To(Succeed()) + + h := NewNetworkInterfaceHandler(gate) + _, err = h.Handle(ctx, vmState) + Expect(err).NotTo(HaveOccurred()) + + changedVM := vmState.VirtualMachine().Changed() + Expect(changedVM.Spec.Networks).To(HaveLen(2)) + Expect(changedVM.Spec.Networks[0].Type).To(Equal(v1alpha2.NetworksTypeMain)) + Expect(changedVM.Spec.Networks[0].Id).To(Equal(1)) + Expect(changedVM.Spec.Networks[1].Type).To(Equal(v1alpha2.NetworksTypeClusterNetwork)) + Expect(changedVM.Spec.Networks[1].Name).To(Equal("test-cluster-network")) + Expect(changedVM.Spec.Networks[1].Id).To(Equal(2)) + }) + + It("should skip id=1 when assigning to non-Main networks", func() { + mac1 := newMACAddress("test-mac-address1", "aa:bb:cc:dd:ee:ff", v1alpha2.VirtualMachineMACAddressPhaseAttached, name) + networkSpec := []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeMain, + Id: 1, // Already set to 1 + }, + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "test-network", + Id: 0, // Should get 2, not 1 + }, + } + vm.Spec.Networks = networkSpec + fakeClient, resource, vmState = setupEnvironment(vm, vmPod, mac1) + + gate, _, setFromMap, err := featuregates.New() + Expect(err).NotTo(HaveOccurred()) + Expect(setFromMap(map[string]bool{string(featuregates.SDN): true})).To(Succeed()) + + h := NewNetworkInterfaceHandler(gate) + _, err = h.Handle(ctx, vmState) + Expect(err).NotTo(HaveOccurred()) + + changedVM := vmState.VirtualMachine().Changed() + Expect(changedVM.Spec.Networks).To(HaveLen(2)) + Expect(changedVM.Spec.Networks[0].Type).To(Equal(v1alpha2.NetworksTypeMain)) + Expect(changedVM.Spec.Networks[0].Id).To(Equal(1)) + Expect(changedVM.Spec.Networks[1].Type).To(Equal(v1alpha2.NetworksTypeNetwork)) + Expect(changedVM.Spec.Networks[1].Id).To(Equal(2)) + }) + + It("should assign sequential ids starting from 2 when there is no Main network", func() { + mac1 := newMACAddress("test-mac-address1", "aa:bb:cc:dd:ee:ff", v1alpha2.VirtualMachineMACAddressPhaseAttached, name) + mac2 := newMACAddress("test-mac-address2", "aa:bb:cc:dd:ee:00", v1alpha2.VirtualMachineMACAddressPhaseAttached, name) + networkSpec := []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "test-network-1", + Id: 0, + }, + { + Type: v1alpha2.NetworksTypeNetwork, + Name: "test-network-2", + Id: 0, + }, + } + vm.Spec.Networks = networkSpec + fakeClient, resource, vmState = setupEnvironment(vm, vmPod, mac1, mac2) + + gate, _, setFromMap, err := featuregates.New() + Expect(err).NotTo(HaveOccurred()) + Expect(setFromMap(map[string]bool{string(featuregates.SDN): true})).To(Succeed()) + + h := NewNetworkInterfaceHandler(gate) + _, err = h.Handle(ctx, vmState) + Expect(err).NotTo(HaveOccurred()) + + changedVM := vmState.VirtualMachine().Changed() + Expect(changedVM.Spec.Networks).To(HaveLen(2)) + Expect(changedVM.Spec.Networks[0].Type).To(Equal(v1alpha2.NetworksTypeNetwork)) + Expect(changedVM.Spec.Networks[0].Name).To(Equal("test-network-1")) + Expect(changedVM.Spec.Networks[0].Id).To(Equal(2)) // Starts from 2, skipping 1 + Expect(changedVM.Spec.Networks[1].Type).To(Equal(v1alpha2.NetworksTypeNetwork)) + Expect(changedVM.Spec.Networks[1].Name).To(Equal("test-network-2")) + Expect(changedVM.Spec.Networks[1].Id).To(Equal(3)) + }) + + It("should handle only ClusterNetwork without Main network", func() { + mac1 := newMACAddress("test-mac-address1", "aa:bb:cc:dd:ee:ff", v1alpha2.VirtualMachineMACAddressPhaseAttached, name) + networkSpec := []v1alpha2.NetworksSpec{ + { + Type: v1alpha2.NetworksTypeClusterNetwork, + Name: "test-cluster-network", + Id: 0, + }, + } + vm.Spec.Networks = networkSpec + fakeClient, resource, vmState = setupEnvironment(vm, vmPod, mac1) + + gate, _, setFromMap, err := featuregates.New() + Expect(err).NotTo(HaveOccurred()) + Expect(setFromMap(map[string]bool{string(featuregates.SDN): true})).To(Succeed()) + + h := NewNetworkInterfaceHandler(gate) + _, err = h.Handle(ctx, vmState) + Expect(err).NotTo(HaveOccurred()) + + changedVM := vmState.VirtualMachine().Changed() + Expect(changedVM.Spec.Networks).To(HaveLen(1)) + Expect(changedVM.Spec.Networks[0].Type).To(Equal(v1alpha2.NetworksTypeClusterNetwork)) + Expect(changedVM.Spec.Networks[0].Name).To(Equal("test-cluster-network")) + Expect(changedVM.Spec.Networks[0].Id).To(Equal(2)) // Starts from 2, skipping 1 + }) + }) }) 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 370ee737a6..530551a938 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -434,13 +434,8 @@ func MakeKVVMFromVMSpec(ctx context.Context, s state.VirtualMachineState) (*virt networkSpec := network.CreateNetworkSpec(current, vmmacs) - vmbdas, err := s.VirtualMachineBlockDeviceAttachments(ctx) - if err != nil { - return nil, fmt.Errorf("get vmbdas: %w", err) - } - // Create kubevirt VirtualMachine resource from d8 VirtualMachine spec. - err = kvbuilder.ApplyVirtualMachineSpec(kvvmBuilder, current, bdState.VDByName, bdState.VIByName, bdState.CVIByName, vmbdas, class, ipAddress, networkSpec) + err = kvbuilder.ApplyVirtualMachineSpec(kvvmBuilder, current, bdState.VDByName, bdState.VIByName, bdState.CVIByName, class, ipAddress, networkSpec) if err != nil { return nil, err } 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 b41451f1b1..1f36a0dfac 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 @@ -28,6 +28,10 @@ import ( "github.com/deckhouse/virtualization/api/core/v1alpha2" ) +const ( + maxNetworkID = 16*1024 - 1 // 16383 +) + type NetworksValidator struct { featureGate featuregate.FeatureGate } @@ -61,6 +65,10 @@ func (v *NetworksValidator) ValidateUpdate(_ context.Context, oldVM, newVM *v1al return nil, fmt.Errorf("network configuration requires SDN to be enabled") } + if err := v.validateNetworkIDsUnchanged(oldVM.Spec.Networks, newNetworksSpec); err != nil { + return nil, err + } + isChanged := !equality.Semantic.DeepEqual(newNetworksSpec, oldVM.Spec.Networks) if isChanged { return v.validateNetworksSpec(newNetworksSpec) @@ -85,6 +93,10 @@ func (v *NetworksValidator) validateNetworksSpec(networksSpec []v1alpha2.Network if err := v.validateNetworkUniqueness(typ, name, namesSet); err != nil { return nil, err } + + if err := v.validateNetworkID(network); err != nil { + return nil, err + } } return nil, nil @@ -116,3 +128,58 @@ func (v *NetworksValidator) validateNetworkUniqueness(networkType, networkName s namesSet[key] = struct{}{} return nil } + +func (v *NetworksValidator) validateNetworkIDsUnchanged(oldNetworksSpec, newNetworksSpec []v1alpha2.NetworksSpec) error { + oldNetworksMap := v.buildNetworksMap(oldNetworksSpec) + newNetworksMap := v.buildNetworksMap(newNetworksSpec) + + for key, oldNetwork := range oldNetworksMap { + newNetwork, exists := newNetworksMap[key] + if !exists { + continue + } + + if oldNetwork.Id == newNetwork.Id { + continue + } + + // Allow changing id from 0 to a valid value (lazy initialization) + if oldNetwork.Id == 0 && newNetwork.Id > 0 && newNetwork.Id <= maxNetworkID { + continue + } + + networkIdentifier := v.getNetworkIdentifier(oldNetwork) + return fmt.Errorf("network id cannot be changed for network %s", networkIdentifier) + } + + return nil +} + +func (v *NetworksValidator) buildNetworksMap(networksSpec []v1alpha2.NetworksSpec) map[string]v1alpha2.NetworksSpec { + networksMap := make(map[string]v1alpha2.NetworksSpec) + for _, network := range networksSpec { + key := v.getNetworkIdentifier(network) + networksMap[key] = network + } + return networksMap +} + +func (v *NetworksValidator) validateNetworkID(network v1alpha2.NetworksSpec) error { + if network.Id == 0 { + return nil + } + + if network.Id < 1 || network.Id > maxNetworkID { + networkIdentifier := v.getNetworkIdentifier(network) + return fmt.Errorf("network id must be between 1 and %d for network %s, got %d", maxNetworkID, networkIdentifier, network.Id) + } + + return nil +} + +func (v *NetworksValidator) getNetworkIdentifier(network v1alpha2.NetworksSpec) string { + if network.Type == v1alpha2.NetworksTypeMain { + return network.Type + } + return fmt.Sprintf("%s/%s", network.Type, network.Name) +} 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 9a66e5c0fa..dbf15f4096 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 @@ -47,6 +47,19 @@ func TestNetworksValidateCreate(t *testing.T) { {[]v1alpha2.NetworksSpec{mainNetwork, networkTest, networkTest}, true, false}, {[]v1alpha2.NetworksSpec{mainNetwork, {Type: v1alpha2.NetworksTypeNetwork}}, true, false}, {[]v1alpha2.NetworksSpec{mainNetwork}, false, false}, + {[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeMain, Id: 1}}, true, true}, + {[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 2}}, true, true}, + {[]v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Id: 1}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "test1", Id: 1}, + {Type: v1alpha2.NetworksTypeClusterNetwork, Name: "test2", Id: 2}, + }, true, true}, + {[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 16383}}, true, true}, + {[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 0}}, true, true}, + {[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 16384}}, true, false}, + {[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: -1}}, true, false}, + {[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeMain, Id: 16383}}, true, true}, + {[]v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeMain, Id: 16384}}, true, false}, } for i, test := range tests { @@ -62,10 +75,10 @@ func TestNetworksValidateCreate(t *testing.T) { _, err := networkValidator.ValidateCreate(t.Context(), vm) if test.valid && err != nil { - t.Errorf("Validation failed for spec %s: expected valid, but got an error: %v", test.networks, err) + t.Errorf("Validation failed for spec %v: expected valid, but got an error: %v", test.networks, err) } if !test.valid && err == nil { - t.Errorf("Validation succeeded for spec %s: expected error, but got none", test.networks) + t.Errorf("Validation succeeded for spec %v: expected error, but got none", test.networks) } }) } @@ -160,6 +173,104 @@ func TestNetworksValidateUpdate(t *testing.T) { sdnEnabled: true, valid: true, }, + { + oldNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Id: 1}, + }, + newNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Id: 2}, + }, + sdnEnabled: true, + valid: false, + }, + { + oldNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 1}, + }, + newNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 2}, + }, + sdnEnabled: true, + valid: false, + }, + { + oldNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeClusterNetwork, Name: "cluster", Id: 5}, + }, + newNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeClusterNetwork, Name: "cluster", Id: 10}, + }, + sdnEnabled: true, + valid: false, + }, + { + oldNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Id: 1}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 2}, + }, + newNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Id: 1}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 2}, + }, + sdnEnabled: true, + valid: true, + }, + { + oldNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Id: 0}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "test1", Id: 1}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "test2", Id: 2}, + }, + newNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Id: 0}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "test1", Id: 1}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "test2", Id: 3}, + }, + sdnEnabled: true, + valid: false, + }, + { + oldNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Id: 0}, + }, + newNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Id: 0}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "new", Id: 5}, + }, + sdnEnabled: true, + valid: true, + }, + { + oldNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Id: 0}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 1}, + }, + newNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Id: 0}, + }, + sdnEnabled: true, + valid: true, + }, + { + oldNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 0}, + }, + newNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 1}, + }, + sdnEnabled: true, + valid: true, + }, + { + oldNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 1}, + }, + newNetworksSpec: []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeNetwork, Name: "test", Id: 0}, + }, + sdnEnabled: true, + valid: false, + }, } for i, test := range tests { diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go b/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go index ba45da80bd..f11f9d2e67 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go @@ -21,6 +21,7 @@ import ( "fmt" "reflect" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -106,7 +107,27 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco return h.Handle(ctx, s) }) rec.SetResourceUpdater(func(ctx context.Context) error { - return vm.Update(ctx) + var specToUpdate *v1alpha2.VirtualMachineSpec + if !reflect.DeepEqual(vm.Current().Spec, vm.Changed().Spec) { + specToUpdate = vm.Changed().Spec.DeepCopy() + } + + vm.Changed().Status.ObservedGeneration = vm.Changed().GetGeneration() + + err := vm.Update(ctx) + if err != nil && !k8serrors.IsNotFound(err) { + return fmt.Errorf("update status: %w", err) + } + + if specToUpdate != nil { + vm.Changed().Spec = *specToUpdate + err = r.client.Update(ctx, vm.Changed()) + if err != nil && !k8serrors.IsNotFound(err) { + return fmt.Errorf("update spec: %w", err) + } + } + + return nil }) return rec.Reconcile(ctx) diff --git a/test/e2e/internal/util/sdn.go b/test/e2e/internal/util/sdn.go index 74dbd39f24..9404a81ec7 100644 --- a/test/e2e/internal/util/sdn.go +++ b/test/e2e/internal/util/sdn.go @@ -45,6 +45,22 @@ spec: type: VLAN vlan: id: 1003 +EOF` + ClusterNetwork2Name = "cn-1004-for-e2e-test" + ClusterNetwork2CreateCommand = `kubectl apply -f - <