E2E: Fix ovs dynamic pinning, kubelet and mustgather tests #1565
+82
−81
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,6 @@ package __performance_kubelet_node_test | |
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "strconv" | ||
| "strings" | ||
|
|
@@ -24,6 +23,7 @@ import ( | |
| testutils "github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils" | ||
| testclient "github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils/client" | ||
| "github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils/hypershift" | ||
| "github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils/infrastructure" | ||
| "github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils/label" | ||
| "github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils/nodes" | ||
| "github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils/poolname" | ||
|
|
@@ -41,13 +41,11 @@ var _ = Describe("[ref_id: 45487][performance]additional kubelet arguments", Ord | |
| ) | ||
|
|
||
| testutils.CustomBeforeAll(func() { | ||
| var err error | ||
| workerRTNodes, err = nodes.GetByLabels(testutils.NodeSelectorLabels) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| workerRTNodes, err = nodes.MatchingOptionalSelector(workerRTNodes) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
Comment on lines
+44
to
49
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Validate and consistently use the selected test nodes.
📍 Affects 1 file
🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. can you check the latest commit in the PR , this should be fixed There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
file="test/e2e/performanceprofile/functests/7_performance_kubelet_node/kubelet.go"
echo "Checked-out revision:"
git log -1 --oneline
echo
echo "File structure:"
ast-grep outline "$file" --items all --type function --view compact || true
echo
echo "Affected code sections:"
nl -ba "$file" | sed -n '35,215p'
echo
echo "Relevant node-selection assertions and ARM lookup:"
rg -n -C 3 'MatchingOptionalSelector|workerRTNodes|updatedNodes|IsARM|cnfNodes' "$file"Length of output: 8151 🏁 Script executed: #!/bin/bash
set -euo pipefail
file="test/e2e/performanceprofile/functests/7_performance_kubelet_node/kubelet.go"
echo "Setup (lines 38-60):"
sed -n '38,60p' "$file"
echo
echo "Updated-node check (lines 148-175):"
sed -n '148,175p' "$file"
echo
echo "ARM check (lines 188-215):"
sed -n '188,215p' "$file"
echo
echo "Exact relevant occurrences:"
rg -n -C 2 'MatchingOptionalSelector|Expect\(len\((workerRTNodes|updatedNodes)\)\)|infrastructure\.IsARM|cnfNodes' "$file"Length of output: 5327
So the original finding remains valid and should stay unresolved. 🧬 Code Graph Analysis Results
|
||
|
|
||
| profile, err = profiles.GetByNodeLabels(testutils.NodeSelectorLabels) | ||
|
|
@@ -58,6 +56,12 @@ var _ = Describe("[ref_id: 45487][performance]additional kubelet arguments", Ord | |
|
|
||
| }) | ||
| Context("Additional kubelet arguments", Label(string(label.Tier2)), func() { | ||
| BeforeEach(func() { | ||
| var err error | ||
| profile, err = profiles.GetByNodeLabels(testutils.NodeSelectorLabels) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| }) | ||
|
|
||
| It("[test_id:45488]Test performance profile annotation for changing multiple kubelet settings", func() { | ||
| sysctls := "{\"allowedUnsafeSysctls\":[\"net.core.somaxconn\",\"kernel.msg*\"],\"systemReserved\":{\"memory\":\"300Mi\"},\"kubeReserved\":{\"memory\":\"768Mi\"},\"imageMinimumGCAge\":\"3m\"}" | ||
| profile.Annotations = updateKubeletConfigOverrideAnnotations(profile.Annotations, sysctls) | ||
|
|
@@ -77,19 +81,20 @@ var _ = Describe("[ref_id: 45487][performance]additional kubelet arguments", Ord | |
| sysctlsValue := kubeletConfig.AllowedUnsafeSysctls | ||
| Expect(sysctlsValue).Should(ContainElements("net.core.somaxconn", "kernel.msg*")) | ||
| Expect(kubeletConfig.KubeReserved["memory"]).To(Equal("768Mi")) | ||
| Expect(kubeletConfig.ImageMinimumGCAge.Seconds()).To(BeNumerically("==", 180)) | ||
| } | ||
|
|
||
| autoSizingCmd := []string{"cat", "/rootfs/etc/openshift/kubelet.conf.d/20-auto-sizing.conf"} | ||
| for _, node := range workerRTNodes { | ||
| out, err := nodes.ExecCommand(context.TODO(), &node, autoSizingCmd) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| stdout := testutils.ToString(out) | ||
| Expect(stdout).To(ContainSubstring("300Mi")) | ||
| } | ||
|
Comment on lines
+87
to
93
There was a problem hiding this comment. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Use bounded contexts for new node-inspector calls. These new external calls use As per path instructions, Go code must use 📍 Affects 1 file
🤖 Prompt for AI AgentsSource: Path instructions
Contributor
Author
There was a problem hiding this comment. can you check the latest commit , this should be fixed There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='test/e2e/performanceprofile/functests/7_performance_kubelet_node/kubelet.go'
echo '== Current HEAD =='
git log -1 --format='%H%n%s'
echo
echo '== Recent commits affecting the target file =='
git log -5 --format='%h %s' -- "$target"
echo
echo '== Target file outline =='
ast-grep outline "$target" --items all --type function
echo
echo '== ExecCommand call sites with surrounding context =='
rg -n -C 8 'nodes\.ExecCommand\s*\(' "$target"
echo
echo '== Context timeout/cancel use with surrounding context =='
rg -n -C 4 'context\.(TODO|WithTimeout|WithCancel)\s*\(|\bcancel\s*\(' "$target"Length of output: 13688
🧬 Code Graph Analysis ResultsRelevant snippets
|
||
| }) | ||
| Context("When setting cpu manager related parameters", func() { | ||
| It("[test_id:45493]Should not override performance-addon-operator values", func() { | ||
| paoValues := "{\"cpuManagerPolicy\":\"none\",\"cpuManagerReconcilePeriod\":\"10s\"}" | ||
| profile.Annotations = updateKubeletConfigOverrideAnnotations(profile.Annotations, paoValues) | ||
|
|
||
| By("updating Performance profile") | ||
|
|
@@ -105,11 +110,11 @@ var _ = Describe("[ref_id: 45487][performance]additional kubelet arguments", Ord | |
| kubeletConfig, err := nodes.GetKubeletConfig(context.TODO(), &node) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(kubeletConfig.CPUManagerPolicy).Should(Equal("static")) | ||
| Expect(kubeletConfig.CPUManagerReconcilePeriod.Seconds()).To(BeNumerically("==", 5)) | ||
| } | ||
| }) | ||
| }) | ||
| It("[test_id:45490]Test memory reservation changes", Label("test1"), func() { | ||
| // In this test case we check if after applying reserving memory for | ||
| // systemReserved and KubeReserved, the allocatable is reduced and Allocatable | ||
| // Verify that Allocatable = Node capacity - (kubereserved + systemReserved + EvictionMemory) | ||
|
|
@@ -144,10 +149,18 @@ var _ = Describe("[ref_id: 45487][performance]additional kubelet arguments", Ord | |
| Expect(kubeletConfigString).To(ContainSubstring(`"kubeReserved":{"memory":"768Mi"}`)) | ||
| Expect(kubeletConfigString).To(ContainSubstring(`"systemReserved":{"memory":"300Mi"}`)) | ||
|
|
||
| // Re-fetch nodes to get current allocatable and capacity after | ||
| // the tuning update, since workerRTNodes was populated before the | ||
| // annotation was applied and its Status values are stale. | ||
| updatedNodes, err := nodes.GetByLabels(testutils.NodeSelectorLabels) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| updatedNodes, err = nodes.MatchingOptionalSelector(updatedNodes) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| for _, node := range updatedNodes { | ||
| kubeletConfig, err := nodes.GetKubeletConfig(context.TODO(), &node) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| totalCapacity := node.Status.Capacity.Memory().MilliValue() | ||
| evictionMemory := kubeletConfig.EvictionHard["memory.available"] | ||
| kubeReserved := kubeletConfig.KubeReserved["memory"] | ||
| evictionMemoryInt, err := strconv.ParseInt(strings.TrimSuffix(evictionMemory, "Mi"), 10, 64) | ||
|
|
@@ -158,12 +171,35 @@ var _ = Describe("[ref_id: 45487][performance]additional kubelet arguments", Ord | |
| kubeReservedMemoryResource := resource.NewQuantity(kubeReservedMemoryInt*1024*1024, resource.BinarySI) | ||
| evictionMemoryResource := resource.NewQuantity(evictionMemoryInt*1024*1024, resource.BinarySI) | ||
| totalKubeMemory := systemReservedResource.MilliValue() + kubeReservedMemoryResource.MilliValue() + evictionMemoryResource.MilliValue() | ||
|
|
||
| // Pre-allocated hugepages are subtracted from allocatable memory by the | ||
| // kubelet but are still included in node capacity. The standard formula | ||
| // Allocatable = Capacity - kubeReserved - systemReserved - evictionHard | ||
| // does not account for this, so we must subtract hugepages to match the | ||
| // actual allocatable reported by the node. | ||
| var totalHugepages int64 | ||
| for resourceName, quantity := range node.Status.Capacity { | ||
| if strings.HasPrefix(string(resourceName), corev1.ResourceHugePagesPrefix) { | ||
| totalHugepages += quantity.MilliValue() | ||
| } | ||
| } | ||
|
|
||
| calculatedAllocatable := totalCapacity - totalKubeMemory - totalHugepages | ||
| currentAllocatable := node.Status.Allocatable.Memory().MilliValue() | ||
| Expect(calculatedAllocatable).To(Equal(currentAllocatable)) | ||
| } | ||
| }) | ||
|
|
||
| It("[test_id:45495] Test setting PAO managed parameters", func() { | ||
| cnfNodes, err := nodes.GetByLabels(testutils.NodeSelectorLabels) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(len(cnfNodes)).To(BeNumerically(">", 0), "expected at least one node to match the selector") | ||
| isArm, err := infrastructure.IsARM(ctx, &cnfNodes[0]) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| if isArm { | ||
| Skip("Changing topologyManagerPolicy is not supported on ARM architecture") | ||
| } | ||
|
|
||
| var paoParameters string | ||
| if *profile.Spec.NUMA.TopologyPolicy == "single-numa-node" { | ||
| paoParameters = "{\"topologyManagerPolicy\":\"restricted\"}" | ||
|
|
@@ -196,30 +232,35 @@ var _ = Describe("[ref_id: 45487][performance]additional kubelet arguments", Ord | |
| By("Reverting the Profile") | ||
| profiles.UpdateWithRetry(initialProfile) | ||
|
|
||
| By(fmt.Sprintf("Applying changes in performance profile and waiting until %s will start updating", poolName)) | ||
| profilesupdate.WaitForTuningUpdating(ctx, initialProfile) | ||
|
|
||
| By(fmt.Sprintf("Waiting when %s finishes updates", poolName)) | ||
| profilesupdate.WaitForTuningUpdated(ctx, initialProfile) | ||
|
|
||
| for _, node := range workerRTNodes { | ||
| kubeletConfig, err := nodes.GetKubeletConfig(context.TODO(), &node) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(kubeletConfig.AllowedUnsafeSysctls).To(BeEmpty()) | ||
| Expect(kubeletConfig.KubeReserved["memory"]).ToNot(Equal("768Mi")) | ||
| Expect(kubeletConfig.ImageMinimumGCAge.Seconds()).ToNot(Equal(180)) | ||
| } | ||
|
|
||
| autoSizingCmd := []string{"cat", "/rootfs/etc/openshift/kubelet.conf.d/20-auto-sizing.conf"} | ||
| for _, node := range workerRTNodes { | ||
| out, err := nodes.ExecCommand(context.TODO(), &node, autoSizingCmd) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| stdout := testutils.ToString(out) | ||
| Expect(stdout).ToNot(ContainSubstring("300Mi")) | ||
| } | ||
|
|
||
| }) | ||
| AfterAll(func() { | ||
| By("Reverting the Profile") | ||
| profile, err := profiles.GetByNodeLabels(testutils.NodeSelectorLabels) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| // revert only if the profile changes. | ||
| if !equality.Semantic.DeepEqual(profile.Spec, initialProfile.Spec) || !equality.Semantic.DeepEqual(profile.Annotations, initialProfile.Annotations) { | ||
| profiles.UpdateWithRetry(initialProfile) | ||
|
|
||
| By(fmt.Sprintf("Applying changes in performance profile and waiting until %s will start updating", poolName)) | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.