Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
Context("Group Both Numa Nodes with restricted topology", Ordered, Label(string(label.Tier2)), func() {
var numaCoreSiblings map[int]map[int][]int
var reserved, isolated cpuset.CPUSet
// Number of hugepages of size 2M created on both numa nodes
const hpCount = 20
// Number of 2Mi hugepages allocated on each NUMA node (pinned via Node).
// Without Node, cmdline hugepages=N is system-wide and the kernel may place
// all pages on one NUMA node, which breaks TopologyAffinityError expectations.
const hpCountPerNuma = 10
testutils.CustomBeforeAll(func() {
var policy = "restricted"
workerRTNodes = getUpdatedNodes()
Expand Down Expand Up @@ -96,12 +98,19 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
hpSize1G := performancev2.HugePageSize("1G")
hpSize2M := performancev2.HugePageSize("2M")

// Pin equally so each NUMA has 20Mi of 2Mi pages (10 * 2Mi).
requiredHugepages := &performancev2.HugePages{
DefaultHugePagesSize: &hpSize1G,
Pages: []performancev2.HugePage{
{
Count: int32(hpCount),
Count: int32(hpCountPerNuma),
Size: hpSize2M,
Node: ptr.To(int32(0)),
},
{
Count: int32(hpCountPerNuma),
Size: hpSize2M,
Node: ptr.To(int32(1)),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
},
},
}
Expand Down Expand Up @@ -136,8 +145,11 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
By("creating test pod")
err = testclient.DataPlaneClient.Create(context.TODO(), testPod)
Expect(err).ToNot(HaveOccurred(), "Failed to create test pod")
DeferCleanup(func() {
Expect(mm1.removePod(context.TODO(), testPod)).ToNot(HaveOccurred(), "Failed to remove test pod")
})
testPod, err = pods.WaitForCondition(context.TODO(), client.ObjectKeyFromObject(testPod), corev1.PodConditionType(corev1.PodFailed), corev1.ConditionFalse, 2*time.Minute)
// Even though number of hugepage requests can be satisfied by 2 numa nodes together
// Each NUMA has only 20Mi of 2Mi hugepages, so 24Mi needs both NUMAs.
// Number of cpus are only 2 which only requires 1 numa node , So minimum number of numa nodes needed to satisfy is only 1.
// According to Restricted TM policy: only allow allocations from the minimum number of NUMA nodes.
// Look at each resource request, see what the minimum number of NUMA nodes are required to
Expand All @@ -146,7 +158,6 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
err := checkPodEvent(testPod, "TopologyAffinityError")
Expect(err).ToNot(HaveOccurred())
Expect(testPod.Status.QOSClass).To(Equal(corev1.PodQOSGuaranteed), "Test pod does not have QoS class of Guaranteed")
Expect(mm1.removePod(context.TODO(), testPod)).ToNot(HaveOccurred(), "Failed to remove test pod")
})

It("[test_id:60694] Accept guaranteed pod requesting resources that can be satisfied by 2 numa nodes together", func() {
Expand All @@ -155,19 +166,20 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
targetNode := &workerRTNodes[0]
mm2.memory = "200Mi"
mm2.cpu = fmt.Sprintf("%d", isolated.Size()-2)
// no. of hugepages is 20 * 2 (numazones). 40Mi
// we are asking for 30Mi, so it needs 2 numazones combined to
// satisfy the requirement
// 10 pages * 2Mi per NUMA * 2 NUMAs = 40Mi total; each NUMA has 20Mi.
// Asking for 30Mi needs both NUMAs combined.
mm2.noOfhpgs = "30Mi"
testPod := mm2.createPodTemplate(profile, true, targetNode)
DeferCleanup(func() {
Expect(mm2.removePod(context.TODO(), testPod)).ToNot(HaveOccurred(), "Failed to remove test pod")
})
// Initialize test pod, check if the pod uses both numa node 0 and 1
err := initializePod(context.TODO(), testPod)
Expect(err).ToNot(HaveOccurred(), "unable to initialize Pod")
numaZone, err := GetMemoryNodes(context.TODO(), testPod, targetNode)
// Expect both numa nodes to be used by pod
Expect(numaZone).To(Equal("0-1"))
Expect(err).ToNot(HaveOccurred(), "Pod's numa affinity is %s instead of %s", numaZone, "0-1")
Expect(mm2.removePod(context.TODO(), testPod)).ToNot(HaveOccurred(), "Failed to remove test pod")
})

It("[test_id:60695] Allow burstable pod with hugepages", func() {
Expand All @@ -178,14 +190,16 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
mm2.cpu = fmt.Sprintf("%d", isolated.Size()-2)
mm2.noOfhpgs = "8Mi"
testPod := mm2.createPodTemplate(profile, false, targetNode)
DeferCleanup(func() {
Expect(mm2.removePod(context.TODO(), testPod)).ToNot(HaveOccurred(), "Failed to remove test pod")
})
// Initialize test pod, check if the pod uses both numa node 0 and 1
err := initializePod(context.TODO(), testPod) // "0-1", targetNode)
Expect(err).ToNot(HaveOccurred(), "Unable to initialize pod")
numaZone, err := GetMemoryNodes(context.TODO(), testPod, targetNode)
Expect(err).ToNot(HaveOccurred(), "Unable to fetch numa zone")
// Expect both numa nodes to be used by pod
Expect(numaZone).To(Equal("0-1"), "Pod's numa affinity is %s instead of %s", numaZone, "0-1")
Expect(mm2.removePod(context.TODO(), testPod)).ToNot(HaveOccurred(), "Failed to remove test pod")
})

AfterAll(func() {
Expand Down Expand Up @@ -327,15 +341,16 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
// since numa zone 0 has only 10Mi hugepages
mm1.noOfhpgs = "14Mi"
testPod := mm1.createPodTemplate(profile, true, targetNode)
DeferCleanup(func() {
Expect(mm1.removePod(context.TODO(), testPod)).ToNot(HaveOccurred(), "Failed to remove test pod")
})
// Initialize test pod, check if the pod uses only numa node 1
err := initializePod(context.TODO(), testPod)
Expect(err).ToNot(HaveOccurred(), "Unable to initialize pod")
numaZone, err := GetMemoryNodes(context.TODO(), testPod, targetNode)
// Expect numa node 1 to be used by pod
Expect(numaZone).To(Equal("1"))
Expect(err).ToNot(HaveOccurred(), "Pod's numa affinity is %s instead of %s", numaZone, "1")
// Delete pod
Expect(mm1.removePod(context.TODO(), testPod)).ToNot(HaveOccurred(), "Failed to remove test pod")
})

It("[test_id:60697] Verify Pod is rejected when the numa zone doesn't have enough resources", func() {
Expand All @@ -356,6 +371,9 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
// since numa zone 0 has only 10Mi hugepages
mm1.noOfhpgs = "14Mi"
testPod1 := mm1.createPodTemplate(profile, true, targetNode)
DeferCleanup(func() {
Expect(mm1.removePod(context.TODO(), testPod1)).ToNot(HaveOccurred(), "Failed to remove testpod1")
})
// Initialize test pod, check if the pod numa affinity is 1
err := initializePod(context.TODO(), testPod1)
Expect(err).ToNot(HaveOccurred(), "Unable to initialize pod")
Expand All @@ -372,13 +390,13 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
By("creating test pod")
err = testclient.DataPlaneClient.Create(context.TODO(), testPod2)
Expect(err).ToNot(HaveOccurred(), "failed to create testpod2")
DeferCleanup(func() {
Expect(mm2.removePod(context.TODO(), testPod2)).ToNot(HaveOccurred(), "Failed to remove testpod2")
})
testPod2, err = pods.WaitForCondition(context.TODO(), client.ObjectKeyFromObject(testPod2), corev1.PodConditionType(corev1.PodFailed), corev1.ConditionTrue, 2*time.Minute)
Expect(err).To(HaveOccurred(), "testpod2 did not go in to failed condition")
err = checkPodEvent(testPod2, "FailedScheduling")
Expect(err).ToNot(HaveOccurred(), "failed to find expected event: failedScheduling")
// Delete pods
Expect(mm1.removePod(context.TODO(), testPod1)).ToNot(HaveOccurred(), "Failed to remove testpod1")
Expect(mm2.removePod(context.TODO(), testPod2)).ToNot(HaveOccurred(), "Failed to remove testpod2")
})

AfterAll(func() {
Expand All @@ -405,8 +423,8 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
var numaCoreSiblings map[int]map[int][]int
var reserved, isolated cpuset.CPUSet
var err error
// Number of hugepages of size 2M
const hpCount = 20
// Number of 2Mi hugepages allocated on each NUMA node (pinned via Node).
const hpCountPerNuma = 10
testutils.CustomBeforeAll(func() {
var policy = "single-numa-node"
workerRTNodes = getUpdatedNodes()
Expand Down Expand Up @@ -442,15 +460,21 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
reservedSet := performancev2.CPUSet(reserved.String())
isolatedSet := performancev2.CPUSet(isolated.String())

// Enable Hugepages
// Enable Hugepages — pin equally so each NUMA has 20Mi of 2Mi pages.
hpSize2M := performancev2.HugePageSize("2M")
hpSize1G := performancev2.HugePageSize("1G")
profile.Spec.HugePages = &performancev2.HugePages{
DefaultHugePagesSize: &hpSize1G,
Pages: []performancev2.HugePage{
{
Count: int32(hpCount),
Count: int32(hpCountPerNuma),
Size: hpSize2M,
Node: ptr.To(int32(0)),
},
{
Count: int32(hpCountPerNuma),
Size: hpSize2M,
Node: ptr.To(int32(1)),
},
},
}
Expand Down Expand Up @@ -482,11 +506,13 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
By("creating test pod")
err = testclient.DataPlaneClient.Create(context.TODO(), testPod)
Expect(err).ToNot(HaveOccurred(), "failed to create testpod")
DeferCleanup(func() {
Expect(mm1.removePod(context.TODO(), testPod)).ToNot(HaveOccurred(), "Failed to remove test pod")
})
testPod, err = pods.WaitForCondition(context.TODO(), client.ObjectKeyFromObject(testPod), corev1.PodConditionType(corev1.PodFailed), corev1.ConditionFalse, 2*time.Minute)
err := checkPodEvent(testPod, "TopologyAffinityError")
Expect(err).ToNot(HaveOccurred(), "pod did not fail with TopologyAffinityError")
Expect(testPod.Status.QOSClass).To(Equal(corev1.PodQOSGuaranteed), "Test pod does not have QoS class of Guaranteed")
Expect(mm1.removePod(context.TODO(), testPod)).ToNot(HaveOccurred(), "Failed to remove test pod")
})
AfterAll(func() {
By("Reverting the Profile")
Expand Down Expand Up @@ -614,30 +640,34 @@ var _ = Describe("[rfe_id: 43186][memorymanager] Memorymanager feature", Label(s
// we are requesting 8Mi hugepages which again can be satisfied by numa zone 0
mm1.noOfhpgs = "8Mi"
testPod1 := mm1.createPodTemplate(profile, true, targetNode)
DeferCleanup(func() {
Expect(mm1.removePod(context.TODO(), testPod1)).ToNot(HaveOccurred(), "Failed to remove test pod")
})
Comment on lines +643 to +645

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Handle unexpected cleanup errors.

Line 644 discards every deletion error. If the test fails before Line 654, a cleanup failure can leave the pod and its NUMA resources allocated for later tests.

Ignore only a NotFound result after the explicit deletion. Fail the cleanup for every other error.

As per path instructions, **/*.go: Never ignore error returns.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/e2e/performanceprofile/functests/2_performance_update/memorymanager.go`
around lines 643 - 645, Update the DeferCleanup callback around mm1.removePod to
inspect and handle its error instead of discarding it: tolerate only a NotFound
result after the explicit deletion, and fail cleanup for every other error using
the test framework’s established failure mechanism.

Source: Path instructions

// Initialize test pod, check if the pod uses Numa node 0
err := initializePod(context.TODO(), testPod1)
Expect(err).ToNot(HaveOccurred(), "Unable to initialize pod")
numaZone, err := GetMemoryNodes(context.TODO(), testPod1, targetNode)
// Expect numa node 0 to be used by pod
Expect(numaZone).To(Equal("0"))
Expect(err).ToNot(HaveOccurred(), "Pod's numa affinity is %s instead of %s", numaZone, "0")
// Delete pod
// Free NUMA 0 resources before scheduling the second pod
Expect(mm1.removePod(context.TODO(), testPod1)).ToNot(HaveOccurred(), "Failed to remove testpod1")
// Schedule pod on numa zone 1
mm2.noOfhpgs = "4Gi"
mm2.memory = "200Mi"
mm2.hpgSize = profile.Spec.HugePages.Pages[1].Size
mm2.cpu = fmt.Sprintf("%d", available_node1_cpus.Size()-2)
testPod2 := mm2.createPodTemplate(profile, true, targetNode)
DeferCleanup(func() {
Expect(mm2.removePod(context.TODO(), testPod2)).ToNot(HaveOccurred(), "Failed to remove test pod")
})
// Initialize test pod, check if the pod uses Numa node 1
err = initializePod(context.TODO(), testPod2)
Expect(err).ToNot(HaveOccurred(), "Unable to initialize pod")
numaZone, err = GetMemoryNodes(context.TODO(), testPod2, targetNode)
// Expect numa node 1 to be used by pod
Expect(numaZone).To(Equal("1"))
Expect(err).ToNot(HaveOccurred(), "Pod's numa affinity is %s instead of %s", numaZone, "1")
// Delete pod
Expect(mm2.removePod(context.TODO(), testPod2)).ToNot(HaveOccurred(), "Failed to remove test pod")
})

AfterAll(func() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,18 +224,32 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance

Context("Verify that all performance profile parameters can be updated", Ordered, Label(string(label.Tier2)), func() {
var removedKernelArgs string
var expectedReserved, expectedIsolated cpuset.CPUSet

hpSize2M := performancev2.HugePageSize("2M")
hpSize1G := performancev2.HugePageSize("1G")
isolated := performancev2.CPUSet("1-2")
reserved := performancev2.CPUSet("0,3")
policy := "best-effort"

// Modify profile and verify that MCO successfully updated the node
// Modify profile and verify that MCO successfully updated the node.
// Keep the existing reserved/isolated CPUs from the performance profile;
// overriding them with a tiny isolated set on large-CPU nodes inflates
// tuned.non_isolcpus / systemd.cpu_affinity and can drop later cmdline
// args such as hugepages (see test_id:34081 failures).
testutils.CustomBeforeAll(func() {
By(fmt.Sprintf("Modifying profile to nodes=%#v MCPs=%#v", profile.Spec.NodeSelector, profile.Spec.MachineConfigPoolSelector))
initialProfile = profile.DeepCopy()

Expect(profile.Spec.CPU).ToNot(BeNil(), "performance profile must define CPU settings")
Expect(profile.Spec.CPU.Reserved).ToNot(BeNil(), "performance profile must define reserved CPUs")
Expect(profile.Spec.CPU.Isolated).ToNot(BeNil(), "performance profile must define isolated CPUs")

var err error
expectedReserved, err = cpuset.Parse(string(*profile.Spec.CPU.Reserved))
Expect(err).ToNot(HaveOccurred(), "failed to parse reserved CPUs from performance profile")
expectedIsolated, err = cpuset.Parse(string(*profile.Spec.CPU.Isolated))
Expect(err).ToNot(HaveOccurred(), "failed to parse isolated CPUs from performance profile")
By(fmt.Sprintf("Preserving existing CPU configuration reserved=%s isolated=%s", expectedReserved.String(), expectedIsolated.String()))

profile.Spec.HugePages = &performancev2.HugePages{
DefaultHugePagesSize: &hpSize2M,
Pages: []performancev2.HugePage{
Expand All @@ -249,11 +263,6 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance
},
},
}
profile.Spec.CPU = &performancev2.CPU{
BalanceIsolated: ptr.To(false),
Reserved: &reserved,
Isolated: &isolated,
}
profile.Spec.NUMA = &performancev2.NUMA{
TopologyPolicy: &policy,
}
Expand Down Expand Up @@ -300,11 +309,59 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance
Entry("[test_id:28070] verify that hugepages updated (NUMA node unspecified)", context.TODO(), chkCmdLineFn, []string{"hugepagesz=2M"}, true, false),
Entry("verify that the right number of hugepages 1G is available on the system", context.TODO(), chkHugepages1GFn, []string{"3"}, true, false),
Entry("verify that the right number of hugepages 2M is available on the system", context.TODO(), chkHugepages2MFn, []string{"256"}, true, false),
Entry("[test_id:28025] verify that cpu affinity mask was updated", context.TODO(), chkCmdLineFn, []string{"tuned.non_isolcpus=.*9"}, true, true),
Entry("[test_id:28071] verify that cpu balancer disabled", context.TODO(), chkCmdLineFn, []string{"isolcpus=domain,managed_irq,1-2"}, true, false),
Entry("[test_id:28071] verify that cpu balancer disabled", context.TODO(), chkCmdLineFn, []string{"systemd.cpu_affinity=0,3"}, true, false),
)

It("[test_id:28025] verify that cpu affinity mask matches the performance profile", func() {
for _, node := range workerRTNodes {
onlineCPUs, err := nodes.GetOnlineCPUsSet(context.TODO(), &node)
Expect(err).ToNot(HaveOccurred())
expectedNonIsolated := onlineCPUs.Difference(expectedIsolated)
expectedMask, err := components.CPUListToMaskList(expectedNonIsolated.String())
Expect(err).ToNot(HaveOccurred())

cmdline, err := chkCmdLineFn(context.TODO(), &node)
Expect(err).ToNot(HaveOccurred())
val := nodes.FindCmdlineParam(cmdline, "tuned.non_isolcpus")
Expect(val).ToNot(BeEmpty(), "tuned.non_isolcpus parameter not found in %q", cmdline)
Expect(val).To(Equal(expectedMask),
"tuned.non_isolcpus=%s does not match expected non-isolated mask %s", val, expectedMask)
}
})

It("[test_id:28071] verify that isolcpus matches the performance profile", func() {
Expect(profile.Spec.CPU).ToNot(BeNil())
expectedPrefix := "managed_irq,"
if profile.Spec.CPU.BalanceIsolated != nil && !*profile.Spec.CPU.BalanceIsolated {
expectedPrefix = "domain,managed_irq,"
}
expectedIsol := expectedPrefix + expectedIsolated.String()
for _, node := range workerRTNodes {
cmdline, err := chkCmdLineFn(context.TODO(), &node)
Expect(err).ToNot(HaveOccurred())
val := nodes.FindCmdlineParam(cmdline, "isolcpus")
Expect(val).ToNot(BeEmpty(), "isolcpus parameter not found in %q", cmdline)
Expect(val).To(Equal(expectedIsol),
"isolcpus mismatch: got %s, expected %s", val, expectedIsol)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
})

It("[test_id:28071] verify that systemd.cpu_affinity matches non-isolated CPUs", func() {
for _, node := range workerRTNodes {
onlineCPUs, err := nodes.GetOnlineCPUsSet(context.TODO(), &node)
Expect(err).ToNot(HaveOccurred())
expectedNonIsolated := onlineCPUs.Difference(expectedIsolated)

cmdline, err := chkCmdLineFn(context.TODO(), &node)
Expect(err).ToNot(HaveOccurred())
val := nodes.FindCmdlineParam(cmdline, "systemd.cpu_affinity")
Expect(val).ToNot(BeEmpty(), "systemd.cpu_affinity parameter not found in %q", cmdline)
affinitySet, err := cpuset.Parse(val)
Expect(err).ToNot(HaveOccurred())
Expect(affinitySet.Equals(expectedNonIsolated)).To(BeTrue(),
"systemd.cpu_affinity=%s does not match non-isolated CPUs %s", affinitySet.String(), expectedNonIsolated.String())
}
})

DescribeTable("Verify that kubelet parameters were updated", func(ctx context.Context, cmdFn checkFunction, getterFn func(kubeletCfg *kubeletconfigv1beta1.KubeletConfiguration) string, wantedValue string) {
for _, node := range workerRTNodes {
result, err := cmdFn(ctx, &node)
Expand All @@ -317,10 +374,24 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance
Expect(getterFn(kc)).To(Equal(wantedValue))
}
},
Entry("[test_id:28935] verify that reservedSystemCPUs was updated", context.TODO(), chkKubeletConfigFn, func(k *kubeletconfigv1beta1.KubeletConfiguration) string { return k.ReservedSystemCPUs }, "0,3"),
Entry("[test_id:28760] verify that topologyManager was updated", context.TODO(), chkKubeletConfigFn, func(k *kubeletconfigv1beta1.KubeletConfiguration) string { return k.TopologyManagerPolicy }, "best-effort"),
)

It("[test_id:28935] verify that reservedSystemCPUs matches the performance profile", func() {
for _, node := range workerRTNodes {
result, err := chkKubeletConfigFn(context.TODO(), &node)
Expect(err).ToNot(HaveOccurred())
obj, err := manifestsutil.DeserializeObjectFromData([]byte(result), kubeletconfigv1beta1.AddToScheme)
Expect(err).ToNot(HaveOccurred())
kc, ok := obj.(*kubeletconfigv1beta1.KubeletConfiguration)
Expect(ok).To(BeTrue(), "wrong type %T", obj)
got, err := cpuset.Parse(kc.ReservedSystemCPUs)
Expect(err).ToNot(HaveOccurred())
Expect(got.Equals(expectedReserved)).To(BeTrue(),
"ReservedSystemCPUs=%s does not match expected reserved CPUs %s", got.String(), expectedReserved.String())
}
})

It("[test_id:27738] should succeed to disable the RT kernel", func() {
for _, node := range workerRTNodes {
err := nodes.HasPreemptRTKernel(context.TODO(), &node)
Expand Down
Loading