Skip to content

Commit 6fbff19

Browse files
committed
The BZ 2094046 test cases for oslat and cyclictest were negative tests
expecting to fail on HT-enabled systems, but they passed unexpectedly on HT-disabled systems because the tools executed successfully. Changes: - Add hyperthreading detection in its test execution path - Skip BZ 2094046 tests when HT is disabled to prevent false passes - Isolated CPU selection with fallback to online CPU Signed-Off-by: Sargun Narula <snarula@redhat.com>
1 parent b5d66f8 commit 6fbff19

File tree

2 files changed

+67
-2
lines changed

2 files changed

+67
-2
lines changed

test/e2e/performanceprofile/functests/5_latency_testing/latency_testing.go

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package __latency_testing
22

33
import (
44
"bytes"
5+
"context"
56
"fmt"
67
"math"
78
"os"
@@ -12,7 +13,12 @@ import (
1213
. "github.com/onsi/ginkgo/v2"
1314
. "github.com/onsi/gomega"
1415
"github.com/onsi/gomega/format"
16+
"k8s.io/utils/cpuset"
17+
18+
testutils "github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils"
1519
testlog "github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils/log"
20+
"github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils/nodes"
21+
"github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils/profiles"
1622
)
1723

1824
// TODO get commonly used variables from one shared file that defines constants
@@ -107,6 +113,16 @@ var _ = DescribeTable("Test latency measurement tools tests", func(testGroup []l
107113
// We only override ginkgo default value for positive tests
108114
output, err = exec.Command(testExecutablePath, "-ginkgo.v", "-ginkgo.focus", test.toolToTest, "-ginkgo.timeout", test.ginkgoTimeout).Output()
109115
} else {
116+
// Special handling for BZ 2094046 test case - check if it should be skipped based on Hyper Threading status
117+
if test.testCpus == "2" {
118+
htEnabled, htErr := checkHyperthreading()
119+
if htErr != nil {
120+
testlog.Warningf("Failed to check hyperthreading status, running test anyway: %v", htErr)
121+
} else if !htEnabled {
122+
testlog.Infof("Skipping BZ 2094046 %s test case - hyperthreading is disabled, test would pass unexpectedly", test.toolToTest)
123+
continue
124+
}
125+
}
110126
// default ginkgo timeout value for negative tests
111127
output, err = exec.Command(testExecutablePath, "-ginkgo.v", "-ginkgo.focus", test.toolToTest).Output()
112128
}
@@ -219,6 +235,48 @@ func clearEnv() {
219235
os.Unsetenv(latencyTestCpus)
220236
}
221237

238+
// checkHyperthreading determines if the BZ 2094046 test should run based on HT status.
239+
// Returns true if the test should run (HT enabled), false if it should be skipped (HT disabled).
240+
func checkHyperthreading() (bool, error) {
241+
profile, err := profiles.GetByNodeLabels(testutils.NodeSelectorLabels)
242+
if err != nil {
243+
return false, fmt.Errorf("get performance profile: %w", err)
244+
}
245+
246+
workerNodes, err := nodes.GetByLabels(testutils.NodeSelectorLabels)
247+
if err != nil {
248+
return false, fmt.Errorf("get worker nodes: %w", err)
249+
}
250+
if len(workerNodes) == 0 {
251+
return false, fmt.Errorf("no worker nodes found")
252+
}
253+
workerNode := &workerNodes[0]
254+
255+
ctx := context.Background()
256+
var cpuID int = -1 // Use -1 to indicate no CPU selected yet
257+
258+
// Prefer isolated CPUs if defined
259+
if profile.Spec.CPU != nil && profile.Spec.CPU.Isolated != nil {
260+
if set, err := cpuset.Parse(string(*profile.Spec.CPU.Isolated)); err == nil && set.Size() > 0 {
261+
cpuID = set.List()[0]
262+
}
263+
}
264+
265+
// Fallback: first online CPU
266+
if cpuID == -1 {
267+
set, err := nodes.GetOnlineCPUsSet(ctx, workerNode)
268+
if err != nil || set.Size() == 0 {
269+
return false, fmt.Errorf("get online CPUs: %w", err)
270+
}
271+
cpuID = set.List()[0]
272+
}
273+
274+
isHTEnabled := nodes.IsHyperthreadingEnabled(ctx, cpuID, workerNode)
275+
testlog.Infof("Hyperthreading status on node %s (CPU %d): %t", workerNode.Name, cpuID, isHTEnabled)
276+
277+
return isHTEnabled, nil
278+
}
279+
222280
func getValidValuesTests(toolToTest string) []latencyTest {
223281
var testSet []latencyTest
224282

@@ -284,14 +342,14 @@ func getNegativeTests(toolToTest string) []latencyTest {
284342
testSet = append(testSet, latencyTest{testRuntime: "2", oslatMaxLatency: fmt.Sprint(math.MaxInt32 + 1), outputMsgs: []string{invalidNumberOslatMaxLatency, fail}, toolToTest: toolToTest})
285343
testSet = append(testSet, latencyTest{testRuntime: "2", oslatMaxLatency: "-3", outputMsgs: []string{invalidNumberOslatMaxLatency, fail}, toolToTest: toolToTest})
286344
// Covers the scenario of BZ 2094046
287-
testSet = append(testSet, latencyTest{testRuntime: "2", testCpus: "2", outputMsgs: []string{unexpectedError, fail}, toolToTest: toolToTest})
345+
testSet = append(testSet, latencyTest{testRuntime: "2", testCpus: "2", oslatMaxLatency: untunedLatencyThreshold, outputMsgs: []string{unexpectedError, fail}, toolToTest: toolToTest})
288346
}
289347
if toolToTest == cyclictest {
290348
testSet = append(testSet, latencyTest{testRuntime: "2", cyclictestMaxLatency: "&", outputMsgs: []string{incorrectCyclictestMaxLatency, fail}, toolToTest: toolToTest})
291349
testSet = append(testSet, latencyTest{testRuntime: "2", cyclictestMaxLatency: fmt.Sprint(math.MaxInt32 + 1), outputMsgs: []string{invalidNumberCyclictestMaxLatency, fail}, toolToTest: toolToTest})
292350
testSet = append(testSet, latencyTest{testRuntime: "2", cyclictestMaxLatency: "-3", outputMsgs: []string{invalidNumberCyclictestMaxLatency, fail}, toolToTest: toolToTest})
293351
// Covers the scenario of BZ 2094046
294-
testSet = append(testSet, latencyTest{testRuntime: "2", testCpus: "2", outputMsgs: []string{unexpectedError, fail}, toolToTest: toolToTest})
352+
testSet = append(testSet, latencyTest{testRuntime: "2", testCpus: "2", cyclictestMaxLatency: untunedLatencyThreshold, outputMsgs: []string{unexpectedError, fail}, toolToTest: toolToTest})
295353
}
296354
if toolToTest == hwlatdetect {
297355
testSet = append(testSet, latencyTest{testRuntime: "2", hwlatdetectMaxLatency: "&", outputMsgs: []string{incorrectHwlatdetectMaxLatency, fail}, toolToTest: toolToTest})

test/e2e/performanceprofile/functests/utils/nodes/nodes.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,13 @@ func GetSMTLevel(ctx context.Context, cpuID int, node *corev1.Node) int {
274274
return cpus.Size()
275275
}
276276

277+
// IsHyperthreadingEnabled checks if hyperthreading (SMT) is enabled on the node
278+
// Returns true if SMT level > 1, false otherwise
279+
func IsHyperthreadingEnabled(ctx context.Context, cpuID int, node *corev1.Node) bool {
280+
smtLevel := GetSMTLevel(ctx, cpuID, node)
281+
return smtLevel > 1
282+
}
283+
277284
// GetNumaNodes returns the number of numa nodes and the associated cpus as list on the node
278285
func GetNumaNodes(ctx context.Context, node *corev1.Node) (map[int][]int, error) {
279286
lscpuCmd := []string{"lscpu", "-e=node,core,cpu", "-J"}

0 commit comments

Comments
 (0)