Skip to content

Commit

Permalink
pool_test: Increase pool range to reduce false positives
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@RamLavi
RamLavi committed Dec 30, 2024
1 parent ec9315c commit 6c9452b
Showing 1 changed file with 23 additions and 20 deletions.
43 changes: 23 additions & 20 deletions pkg/pool-manager/pool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)}}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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"

Expand All @@ -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"

Expand All @@ -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
Expand Down Expand Up @@ -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"

Expand All @@ -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"

Expand All @@ -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)
Expand All @@ -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"

Expand All @@ -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"

Expand All @@ -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)
Expand All @@ -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"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"

Expand All @@ -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")
})

Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 6c9452b

Please sign in to comment.