From 6c9452bd1989ef71d8315d60b8a63c5372c6abd1 Mon Sep 17 00:00:00 2001 From: Ram Lavi Date: Wed, 25 Dec 2024 10:49:33 +0200 Subject: [PATCH] pool_test: Increase pool range to reduce false positives Since now MAC allocation is done randomly, it is better to increase the MAC pool range to be large, to greatly reduce cases where the randomly allocated MAC is the one accidentally expected by the tests. Also change managedNamespaceMAC, unmanagedNamespaceMAC to be outside of this range to reduce rare cases where the MAC allocator will assign that MAC to the interfaces, creating a forbidden duplication. Signed-off-by: Ram Lavi --- pkg/pool-manager/pool_test.go | 43 +++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/pkg/pool-manager/pool_test.go b/pkg/pool-manager/pool_test.go index 07680ed8c..9c8d16f0f 100644 --- a/pkg/pool-manager/pool_test.go +++ b/pkg/pool-manager/pool_test.go @@ -54,8 +54,11 @@ var _ = Describe("Pool", func() { return map[string]string{NetworksAnnotation: `[{"name":"ovs-conf","namespace":"` + namespace + `","mac":"` + macAddress + `","cni-args":null}]`} } const ( - managedNamespaceMAC = "02:00:00:00:00:00" - unmanagedNamespaceMAC = "02:00:00:00:00:FF" + managedNamespaceMAC = "03:00:00:00:00:00" + unmanagedNamespaceMAC = "03:00:00:00:00:FF" + + minRangeMACPool = "02:00:00:00:00:00" + maxRangeMACPool = "02:FF:FF:FF:FF:FF" ) managedPodWithMacAllocated := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "podpod", Namespace: managedNamespaceName, Annotations: afterAllocationAnnotation(managedNamespaceName, managedNamespaceMAC)}} unmanagedPodWithMacAllocated := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "unmanagedPod", Namespace: notManagedNamespaceName, Annotations: afterAllocationAnnotation(notManagedNamespaceName, unmanagedNamespaceMAC)}} @@ -137,7 +140,7 @@ var _ = Describe("Pool", func() { Describe("Pool Manager General Functions ", func() { It("should create a pool manager", func() { - poolManager := createPoolManager("02:00:00:00:00:00", "02:FF:FF:FF:FF:FF") + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool) Expect(poolManager).ToNot(BeNil()) }) Context("check NewPoolManager", func() { @@ -179,7 +182,7 @@ var _ = Describe("Pool", func() { Context("When poolManager is initialized when there are pods on managed and unmanaged namespaces", func() { var poolManager *PoolManager BeforeEach(func() { - poolManager = createPoolManager("02:00:00:00:00:00", "02:FF:FF:FF:FF:FF", &managedPodWithMacAllocated, &unmanagedPodWithMacAllocated) + poolManager = createPoolManager(minRangeMACPool, maxRangeMACPool, &managedPodWithMacAllocated, &unmanagedPodWithMacAllocated) Expect(poolManager).ToNot(BeNil()) }) It("Should initialize the macPoolmap only with macs on the mananged pods", func() { @@ -280,7 +283,7 @@ var _ = Describe("Pool", func() { }) It("should reject allocation if there are interfaces with the same name", func() { - poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool) newVM := duplicateInterfacesVM.DeepCopy() newVM.Name = "duplicateInterfacesVM" @@ -290,7 +293,7 @@ var _ = Describe("Pool", func() { Expect(poolManager.macPoolMap).To(BeEmpty(), "Should not allocate macs if there are duplicate interfaces") }) It("should not allocate a new mac for bridge interface on pod network", func() { - poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool) newVM := sampleVM newVM.Name = "newVM" @@ -302,7 +305,7 @@ var _ = Describe("Pool", func() { Context("and there is a pre-existing pod with mac allocated to it", func() { var poolManager *PoolManager BeforeEach(func() { - poolManager = createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02", &managedPodWithMacAllocated) + poolManager = createPoolManager(minRangeMACPool, maxRangeMACPool, &managedPodWithMacAllocated) }) It("should allocate a new mac and release it for masquerade", func() { newVM := masqueradeVM @@ -341,7 +344,7 @@ var _ = Describe("Pool", func() { vm.Spec.Template.Spec.Domain.Devices.Disks = make([]kubevirt.Disk, 1) vm.Spec.Template.Spec.Domain.Devices.Disks[0].IO = ioName } - poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool) newVM := multipleInterfacesVM.DeepCopy() newVM.Name = "newVM" @@ -359,7 +362,7 @@ var _ = Describe("Pool", func() { Expect(updateVm.Spec.Template.Spec.Domain.Devices.Disks[0].IO).To(Equal(kubevirt.DriverIO("native-update")), "disk.io configuration must be preserved after mac allocation update") }) It("should reject update allocation if there are interfaces with the same name", func() { - poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool) newVM := multipleInterfacesVM.DeepCopy() newVM.Name = "multipleInterfacesVM" @@ -378,7 +381,7 @@ var _ = Describe("Pool", func() { Expect(checkMacPoolMapEntries(poolManager.macPoolMap, &transactionTimestamp, expectedMACsAfterRejection, []string{})).To(Succeed(), "Failed to check macs in macMap") }) It("should preserve mac addresses on update", func() { - poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool) newVM := multipleInterfacesVM.DeepCopy() newVM.Name = "newVM" transactionTimestamp := updateTransactionTimestamp(0) @@ -396,7 +399,7 @@ var _ = Describe("Pool", func() { Expect(checkMacPoolMapEntries(poolManager.macPoolMap, &newTransactionTimestamp, expectedUpdatedMACs, []string{})).To(Succeed(), "Failed to check macs in macMap") }) It("should preserve mac addresses and allocate a requested one on update", func() { - poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool) newVM := multipleInterfacesVM.DeepCopy() newVM.Name = "newVM" @@ -422,7 +425,7 @@ var _ = Describe("Pool", func() { Expect(checkMacPoolMapEntries(poolManager.macPoolMap, &newTransactionTimestamp, expectedUpdatedMACs, []string{})).To(Succeed(), "Failed to check macs in macMap") }) It("should allow to add a new interface on update", func() { - poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool) newVM := multipleInterfacesVM.DeepCopy() newVM.Name = "newVM" @@ -445,7 +448,7 @@ var _ = Describe("Pool", func() { Expect(checkMacPoolMapEntries(poolManager.macPoolMap, &NewTransactionTimestamp, expectedUpdatedMACs, expectedNotUpdatedMacs)).To(Succeed(), "Failed to check macs in macMap") }) It("should allow to remove an interface on update", func() { - poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool) newVM := multipleInterfacesVM.DeepCopy() newVM.Name = "newVM" newVM.Spec.Template.Spec.Domain.Devices.Interfaces = append(newVM.Spec.Template.Spec.Domain.Devices.Interfaces, anotherMultusBridgeInterface) @@ -469,7 +472,7 @@ var _ = Describe("Pool", func() { Expect(checkMacPoolMapEntries(poolManager.macPoolMap, &NewTransactionTimestamp, expectedUpdatedMACs, expectedNotUpdatedMACs)).To(Succeed(), "Failed to check macs in macMap") }) It("should allow to remove and add an interface on update", func() { - poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool) newVM := multipleInterfacesVM.DeepCopy() newVM.Name = "newVM" @@ -506,7 +509,7 @@ var _ = Describe("Pool", func() { vmFirstUpdateTimestamp := now.Add(time.Duration(1) * time.Second) vmSecondUpdateTimestamp := now.Add(time.Duration(2) * time.Second) BeforeEach(func() { - poolManager = createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager = createPoolManager(minRangeMACPool, maxRangeMACPool) }) var vm, vmFirstUpdate, vmSecondUpdate *kubevirt.VirtualMachine var vmLastPersistedTransactionTimestampAnnotation *time.Time @@ -625,7 +628,7 @@ var _ = Describe("Pool", func() { transactionTimestamp time.Time ) BeforeEach(func() { - poolManager = createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:01", &vmConfigMap) + poolManager = createPoolManager(minRangeMACPool, maxRangeMACPool, &vmConfigMap) newVM = masqueradeVM.DeepCopy() newVM.Name = "newVM" @@ -718,7 +721,7 @@ var _ = Describe("Pool", func() { Describe("Pool Manager Functions For pod", func() { It("should allocate a new mac and release it", func() { - poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02", &managedPodWithMacAllocated) + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool, &managedPodWithMacAllocated) newPod := managedPodWithMacAllocated newPod.Name = "newPod" newPod.Annotations = beforeAllocationAnnotation @@ -750,7 +753,7 @@ var _ = Describe("Pool", func() { Expect(exist).To(BeFalse()) }) It("should allocate requested mac when empty", func() { - poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager := createPoolManager(minRangeMACPool, maxRangeMACPool) newPod := managedPodWithMacAllocated newPod.Name = "newPod" @@ -765,7 +768,7 @@ var _ = Describe("Pool", func() { poolManager := &PoolManager{} BeforeEach(func() { - poolManager = createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02") + poolManager = createPoolManager(minRangeMACPool, maxRangeMACPool) Expect(poolManager).ToNot(Equal(nil), "should create pool-manager") }) @@ -924,7 +927,7 @@ var _ = Describe("Pool", func() { } var poolManager *PoolManager BeforeEach(func() { - poolManager = createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:01", optOutMutatingWebhookConfiguration, optInMutatingWebhookConfiguration, namespaceWithIncludingLabel, namespaceWithExcludingLabel, namespaceWithNoLabels, namespaceWithIrrelevantLabels) + poolManager = createPoolManager(minRangeMACPool, maxRangeMACPool, optOutMutatingWebhookConfiguration, optInMutatingWebhookConfiguration, namespaceWithIncludingLabel, namespaceWithExcludingLabel, namespaceWithNoLabels, namespaceWithIrrelevantLabels) }) DescribeTable("Should return the expected namespace acceptance outcome according to the opt-mode or return an error", func(n *isNamespaceSelectorCompatibleWithOptModeLabelParams) {