From a97e05bf51425027266c025cb24402319a830e52 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Thu, 2 Apr 2026 17:14:54 +0300 Subject: [PATCH] +++ Signed-off-by: Dmitry Lopatin --- .../pkg/common/network/ids.go | 9 +- .../pkg/common/network/ids_test.go | 108 ++++++++++++++++++ .../pkg/controller/vm/internal/mac.go | 27 +---- .../pkg/controller/vm/internal/mac_test.go | 16 +++ 4 files changed, 134 insertions(+), 26 deletions(-) diff --git a/images/virtualization-artifact/pkg/common/network/ids.go b/images/virtualization-artifact/pkg/common/network/ids.go index 4cad0def20..f65b7ee57c 100644 --- a/images/virtualization-artifact/pkg/common/network/ids.go +++ b/images/virtualization-artifact/pkg/common/network/ids.go @@ -39,12 +39,15 @@ func EnsureNetworkInterfaceIDs(networks []v1alpha2.NetworksSpec) bool { changed := false used := make(map[int]struct{}, len(networks)) + for i := range networks { + if networks[i].ID != nil && *networks[i].ID > 0 { + used[*networks[i].ID] = struct{}{} + } + } + nextID := StartGenericID for i := range networks { if networks[i].ID != nil { - if *networks[i].ID > 0 { - used[*networks[i].ID] = struct{}{} - } continue } diff --git a/images/virtualization-artifact/pkg/common/network/ids_test.go b/images/virtualization-artifact/pkg/common/network/ids_test.go index 4314413b5f..84732d24df 100644 --- a/images/virtualization-artifact/pkg/common/network/ids_test.go +++ b/images/virtualization-artifact/pkg/common/network/ids_test.go @@ -61,6 +61,114 @@ var _ = Describe("Network IDs", func() { Expect(*networks[2].ID).To(Equal(2)) }) + It("should skip already reserved IDs even if they appear later in list", func() { + idMain := 1 + idMax := 16383 + idNet := 2 + networks := []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, ID: &idMain}, + {Type: v1alpha2.NetworksTypeClusterNetwork, Name: "cnet-521"}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "net-501", ID: &idMax}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "net-502", ID: &idNet}, + } + + changed := EnsureNetworkInterfaceIDs(networks) + + Expect(changed).To(BeTrue()) + Expect(*networks[0].ID).To(Equal(1)) + Expect(*networks[1].ID).To(Equal(3)) + Expect(*networks[2].ID).To(Equal(16383)) + Expect(*networks[3].ID).To(Equal(2)) + }) + + It("should assign minimal free IDs for multiple missing networks", func() { + idMain := 1 + id4 := 4 + id7 := 7 + networks := []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, ID: &idMain}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "n-missing-1"}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "n-4", ID: &id4}, + {Type: v1alpha2.NetworksTypeClusterNetwork, Name: "cn-missing-1"}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "n-7", ID: &id7}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "n-missing-2"}, + } + + changed := EnsureNetworkInterfaceIDs(networks) + + Expect(changed).To(BeTrue()) + Expect(*networks[1].ID).To(Equal(2)) + Expect(*networks[3].ID).To(Equal(3)) + Expect(*networks[5].ID).To(Equal(5)) + }) + + It("should assign generic IDs starting from 2 when there is no main", func() { + id4 := 4 + networks := []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeNetwork, Name: "n1"}, + {Type: v1alpha2.NetworksTypeClusterNetwork, Name: "cn1"}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "n4", ID: &id4}, + } + + changed := EnsureNetworkInterfaceIDs(networks) + + Expect(changed).To(BeTrue()) + Expect(*networks[0].ID).To(Equal(2)) + Expect(*networks[1].ID).To(Equal(3)) + Expect(*networks[2].ID).To(Equal(4)) + }) + + It("should stop assignment when generic ID space is exhausted", func() { + idMain := 1 + networks := []v1alpha2.NetworksSpec{{Type: v1alpha2.NetworksTypeMain, ID: &idMain}} + for id := StartGenericID; id <= MaxID; id++ { + v := id + networks = append(networks, v1alpha2.NetworksSpec{Type: v1alpha2.NetworksTypeNetwork, ID: &v}) + } + networks = append(networks, v1alpha2.NetworksSpec{Type: v1alpha2.NetworksTypeClusterNetwork, Name: "overflow"}) + + changed := EnsureNetworkInterfaceIDs(networks) + + Expect(changed).To(BeFalse()) + Expect(networks[len(networks)-1].ID).To(BeNil()) + }) + + It("should keep duplicate id=1 when main has no ID but id=1 is already used", func() { + id1 := 1 + networks := []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeNetwork, Name: "n-with-1", ID: &id1}, + {Type: v1alpha2.NetworksTypeMain, Name: "main"}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "n-missing"}, + } + + changed := EnsureNetworkInterfaceIDs(networks) + + Expect(changed).To(BeTrue()) + Expect(*networks[0].ID).To(Equal(1)) + Expect(*networks[1].ID).To(Equal(1)) + Expect(*networks[2].ID).To(Equal(2)) + }) + + It("should preserve explicit invalid IDs and assign only missing ones", func() { + idZero := 0 + idNegative := -5 + idTooHigh := 20000 + networks := []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain, Name: "main", ID: &idZero}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "n-neg", ID: &idNegative}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "n-high", ID: &idTooHigh}, + {Type: v1alpha2.NetworksTypeClusterNetwork, Name: "cn-missing"}, + } + + changed := EnsureNetworkInterfaceIDs(networks) + + Expect(changed).To(BeTrue()) + Expect(*networks[0].ID).To(Equal(0)) + Expect(*networks[1].ID).To(Equal(-5)) + Expect(*networks[2].ID).To(Equal(20000)) + Expect(*networks[3].ID).To(Equal(2)) + }) + It("should return false when nothing to assign", func() { idMain := 1 idNet := 2 diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/mac.go b/images/virtualization-artifact/pkg/controller/vm/internal/mac.go index 9758984102..31c216b68d 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/mac.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/mac.go @@ -115,7 +115,10 @@ func (h *MACHandler) Handle(ctx context.Context, s state.VirtualMachineState) (r } } if createdCount < expectedMACAddresses { - macsToCreate := countNetworksWithMACRequest(vm.Spec.Networks, vmmacs) - createdCount + macsToCreate := expectedMACAddresses - len(vmmacs) - createdCount + if macsToCreate < 0 { + macsToCreate = 0 + } for i := 0; i < macsToCreate; i++ { err = h.macManager.CreateMACAddress(ctx, vm, h.client, "") if err != nil { @@ -177,25 +180,3 @@ func (h *MACHandler) Handle(ctx context.Context, s state.VirtualMachineState) (r func (h *MACHandler) Name() string { return nameMACHandler } - -func countNetworksWithMACRequest(networkSpec []v1alpha2.NetworksSpec, vmmacs []*v1alpha2.VirtualMachineMACAddress) int { - existingMACNames := make(map[string]bool) - for _, vmmac := range vmmacs { - existingMACNames[vmmac.Name] = true - } - - count := 0 - for _, ns := range networkSpec { - if ns.Type == v1alpha2.NetworksTypeMain { - continue - } - - if ns.VirtualMachineMACAddressName == "" { - count++ - } else if !existingMACNames[ns.VirtualMachineMACAddressName] { - count++ - } - } - - return count -} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/mac_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/mac_test.go index a97c76d3b1..9dbc34fa86 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/mac_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/mac_test.go @@ -112,6 +112,22 @@ var _ = Describe("MACHandler", func() { } Describe("Condition presence and absence scenarios", func() { + It("should create only one additional VMMAC when one additional network is added", func() { + vm.Spec.Networks = []v1alpha2.NetworksSpec{ + {Type: v1alpha2.NetworksTypeMain}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "test-network1"}, + {Type: v1alpha2.NetworksTypeNetwork, Name: "test-network2"}, + } + + macAddress1 := newMACAddress("test-mac-address1", "aa:bb:cc:dd:ee:ff", "", "") + fakeClient, resource, vmState = setupEnvironment(vm, macAddress1) + reconcile() + + vmmacList := &v1alpha2.VirtualMachineMACAddressList{} + err := fakeClient.List(ctx, vmmacList, client.InNamespace(namespace)) + Expect(err).NotTo(HaveOccurred()) + Expect(vmmacList.Items).To(HaveLen(2)) + }) Describe("NetworkSpec is nil", func() { It("Condition 'MACAddressReady' should have status 'True'", func() { fakeClient, resource, vmState = setupEnvironment(vm)