Skip to content

NO-JIRA: e2e: fix: restart tuned pod after irqbalance test to prevent cpuset pollution #1575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tal-or wants to merge 1 commit into
base: main
Choose a base branch
from
Open

NO-JIRA: e2e: fix: restart tuned pod after irqbalance test to prevent cpuset pollution #1575

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions test/e2e/performanceprofile/functests/1_performance/irqbalance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ var _ = Describe("[performance] IRQBalance", Ordered, func() {
err = testclient.DataPlaneClient.Create(context.TODO(), testpod)
Expect(err).ToNot(HaveOccurred())
defer func() {
GinkgoHelper()
if testpod != nil {
testlog.Infof("deleting pod %q", testpod.Name)
Expect(pods.DeleteAndSync(context.TODO(), testclient.DataPlaneClient, testpod)).To(Succeed())
Expand All @@ -236,6 +237,16 @@ var _ = Describe("[performance] IRQBalance", Ordered, func() {
Expect(err).ToNot(HaveOccurred(), "failed to extract the default IRQ affinity from node %q", targetNode.Name)

testlog.Infof("IRQ Default affinity on %q when test ends: {%s}", targetNode.Name, irqAffBegin)

// Restart the tuned pod to restore clean CPU affinity.
// The tuned pod was restarted earlier while the guaranteed pod
// held exclusive CPUs, so its process affinity mask is permanently
// narrowed. A fresh start picks up the current (full) default cpuset.
By(fmt.Sprintf("restarting tuned pod on %s to restore clean CPU affinity", targetNode.Name))
tunedPod := nodes.TunedForNode(targetNode, RunningOnSingleNode)
Expect(pods.DeleteAndSync(context.TODO(), testclient.DataPlaneClient, tunedPod)).To(Succeed(), "failed to delete tuned pod on node %q", targetNode.Name)
nodes.TunedForNode(targetNode, RunningOnSingleNode)
Comment on lines +245 to +248

@qodo-for-rh-openshift qodo-for-rh-openshift Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Cleanup assertions obscure failures 🐞 Bug ☼ Reliability

The new tuned restart runs inside a Go defer and uses Expect(...) plus nodes.TunedForNode(...)
(which asserts via Eventually(...).Should(...)), so a cleanup problem can add secondary failures
that obscure the original test failure context.
This makes diagnosing the primary failure harder when the spec is already failing and the cleanup
path hits transient tuned/API issues.
Agent Prompt 
## Issue description
The tuned restart logic was added inside a Go `defer` and contains multiple assertions (`Expect(...)` and the assertion inside `nodes.TunedForNode`). If the spec already failed, a cleanup failure can add additional failures and make the original failure harder to interpret.

## Issue Context
`nodes.TunedForNode` performs an `Eventually(...).Should(...)` assertion internally and can wait for a long time before failing. Executing this in a Go `defer` means failures are not clearly separated as cleanup failures.

## Fix Focus Areas
- test/e2e/performanceprofile/functests/1_performance/irqbalance.go[225-249]

## Suggested fix
Convert the Go `defer func() { ... }()` cleanup to Ginkgo `DeferCleanup(...)` so failures are attributed to cleanup rather than confusing the primary assertion failure. Use the `cleanupCtx` parameter (or a derived context with timeout) for API calls, and consider recording/logging errors in cleanup separately from the main test expectations if you want to preserve the original failure signal.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

testlog.Infof("tuned pod restarted on node %q with clean CPU affinity", targetNode.Name)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +248 to +249

@coderabbitai coderabbitai Bot Aug 4, 2026

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

Require a real readiness check before logging the restart.

At Line 247, nodes.TunedForNode can return a pod while Status.ContainerStatuses is empty. Its polling loop then succeeds without checking readiness. A newly recreated pod can reach this state before TuneD is ready. Line 248 can log false success, and the next test can start too early.

Update TunedForNode to require populated container statuses and ready containers, or use an explicit PodReady wait here.

Suggested helper guard 
        if len(tunedList.Items) == 0 {
            return false
        }
+       if len(tunedList.Items[0].Status.ContainerStatuses) == 0 {
+           return false
+       }
        for _, s := range tunedList.Items[0].Status.ContainerStatuses {
🤖 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/1_performance/irqbalance.go` around
lines 247 - 248, The call to nodes.TunedForNode can return a pod before its
containers are actually ready, causing the testlog.Infof at line 248 to log
false success when Status.ContainerStatuses is still empty. Add an explicit
readiness check (such as a PodReady wait) after the TunedForNode call completes
and before the testlog.Infof call to verify that the pod's container statuses
are populated and the containers are ready, ensuring the restart success is only
logged when TuneD is truly running.

Comment on lines +246 to +249

@qodo-for-rh-openshift qodo-for-rh-openshift Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Cleanup can block too long 🐞 Bug ➹ Performance

The added tuned restart cleanup does two nodes.TunedForNode(...) waits (up to 480s each) plus a
pod deletion wait (up to 120s), so a single spec’s cleanup can be delayed by many minutes when tuned
is slow/unhealthy.
This can significantly slow feedback in failure scenarios and make unrelated failures take much
longer to complete.
Agent Prompt 
## Issue description
The new cleanup performs:
- `nodes.TunedForNode(...)` (waits up to `testTimeout=480s`)
- `pods.DeleteAndSync(...)` (waits up to `DefaultDeletionTimeout=120s`)
- another `nodes.TunedForNode(...)` (another up to 480s)

In failure scenarios, this can delay cleanup completion by a large amount.

## Issue Context
`nodes.TunedForNode` is implemented with `Eventually(..., cluster.ComputeTestTimeout(testTimeout*time.Second, sno), ...)` and `testTimeout` is 480 seconds.

## Fix Focus Areas
- test/e2e/performanceprofile/functests/1_performance/irqbalance.go[240-248]
- test/e2e/performanceprofile/functests/utils/nodes/nodes.go[35-38]
- test/e2e/performanceprofile/functests/utils/nodes/nodes.go[379-405]

## Suggested fix
In the cleanup, avoid the first long readiness wait before deletion:
- List the tuned pods for the node once (no `Eventually`), delete what you find (handling NotFound / empty list gracefully), then do a single `nodes.TunedForNode(...)` wait to ensure tuned is back.
- Alternatively, add a new helper like `nodes.TunedForNodeWithTimeout(node, sno, timeout)` and use a shorter timeout specifically for cleanup.

Also consider running the cleanup under a bounded context (`cleanupCtx` from `DeferCleanup`, optionally wrapped with `context.WithTimeout`) so the cleanup can’t stall indefinitely if the API server is degraded.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}()

testpod, err = pods.WaitForCondition(context.TODO(), client.ObjectKeyFromObject(testpod), corev1.PodReady, corev1.ConditionTrue, 10*time.Minute)
Expand Down