[release-4.21] OCPBUGS-85016: e2e: Add irqbalance StartLimitBurst >= 100 config test

[openshift-cherrypick-robot]

This is an automated cherry-pick of #1493

/assign oblau

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f8fc7c17-babd-464b-97b1-3dbc9457ab57

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[qodo-code-review]

Review Summary by Qodo

Add irqbalance StartLimitBurst >= 100 e2e configuration test

🧪 Tests ✨ Enhancement

Walkthroughs

Description

• Add e2e test for irqbalance StartLimitBurst >= 100 configuration
• Implement systemd.ShowPropertyValue utility function with --value flag
• Automate OCPBUGS-45112 verification for pod restart issue
• Update test suite description for clarity

Diagram

flowchart LR
  A["irqbalance.go test file"] -->|adds new test| B["StartLimitBurst >= 100 test"]
  B -->|uses| C["systemd.ShowPropertyValue"]
  D["systemd.go utility"] -->|implements| C
  C -->|executes| E["systemctl show with --value flag"]

Loading

File Changes

1. test/e2e/performanceprofile/functests/1_performance/irqbalance.go 🧪 Tests +16/-1

Add StartLimitBurst >= 100 validation test

• Added new e2e test to verify irqbalance StartLimitBurst >= 100 configuration
• Imported strconv package for integer conversion and systemd utility package
• Updated test suite description from "Checking IRQBalance settings" to "IRQBalance"
• Test validates systemd StartLimitBurst property and converts string value to integer for
 comparison

test/e2e/performanceprofile/functests/1_performance/irqbalance.go

---

2. test/e2e/performanceprofile/functests/utils/systemd/systemd.go ✨ Enhancement +6/-0

Add ShowPropertyValue utility function

• Added new ShowPropertyValue function to retrieve systemd property values
• Function uses systemctl show command with --value flag for cleaner output
• Complements existing ShowProperty function with simplified value extraction

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

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0)

1. Shell command injection risk 🐞 Bug ⛨ Security

Description

systemd.ShowPropertyValue builds a /bin/bash -c string with fmt.Sprintf and interpolates
property and unitfile, so any spaces/shell metacharacters in those values can alter the executed
command. This runs inside the privileged node-inspector pod, increasing the blast radius if the
helper is ever called with non-constant inputs.

Code

test/e2e/performanceprofile/functests/utils/systemd/systemd.go[R23-26]

+func ShowPropertyValue(ctx context.Context, unitfile string, property string, node *corev1.Node) (string, error) {
+	cmd := []string{"/bin/bash", "-c", fmt.Sprintf("chroot /rootfs systemctl show -p %s %s --value --no-pager", property, unitfile)}
+	out, err := nodes.ExecCommand(ctx, node, cmd)
+	return string(out), err

Evidence

The helper constructs a single shell string (via bash -c) that includes unescaped property and
unitfile, then executes it through the node inspector, which execs commands into a privileged
daemon pod on the target node.

test/e2e/performanceprofile/functests/utils/systemd/systemd.go[17-27]
test/e2e/performanceprofile/functests/utils/node_inspector/inspector.go[152-169]
test/e2e/performanceprofile/functests/utils/node_inspector/inspector.go[196-225]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ShowPropertyValue` uses `[]string{"/bin/bash","-c", fmt.Sprintf("...")}` which is vulnerable to shell interpretation and breaks on values containing spaces.

### Issue Context
This command is executed in the node-inspector privileged pod via `pods.WaitForPodOutput`, so it is best to avoid the shell entirely.

### Fix Focus Areas
- test/e2e/performanceprofile/functests/utils/systemd/systemd.go[23-26]

### Implementation notes
Prefer an argv-style command, e.g.:
- `cmd := []string{"chroot","/rootfs","systemctl","show","-p",property,unitfile,"--value","--no-pager"}`
This removes shell parsing and eliminates the injection surface.

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

---

2. Inconsistent bug ID reference 🐞 Bug ⚙ Maintainability

Description

The newly added StartLimitBurst test references OCPBUGS-45112 in comments and the failure message,
while the PR context is OCPBUGS-84938, which can mislead failure triage and future maintenance.
The code should consistently reference the intended tracking bug (or include both if they are
related).

Code

test/e2e/performanceprofile/functests/1_performance/irqbalance.go[R365-376]

+	// Automates OCPBUGS-45112 - Config test
+	// Default StartLimitBurst=5 was too low - each pod created/deleted would trigger a restart of irqbalance
+	// Systemd coalesces concurrent requests, making the reproduction by deployment impractical.
+	It("[test_id:88711] should have irqbalance StartLimitBurst >= 100", Label(string(label.Tier0)), func() {
+		startLimitBurst, err := systemd.ShowPropertyValue(context.TODO(), "irqbalance.service", "StartLimitBurst", targetNode)
+		Expect(err).ToNot(HaveOccurred())
+		startLimitBurst = strings.TrimSpace(startLimitBurst)
+		testlog.Infof("irqbalance.service StartLimitBurst=%s on node %s", startLimitBurst, targetNode.Name)
+		startLimitBurstInt, err := strconv.Atoi(startLimitBurst)
+		Expect(err).ToNot(HaveOccurred(), "unexpected StartLimitBurst value %q", startLimitBurst)
+		Expect(startLimitBurstInt).To(BeNumerically(">=", 100), "irqbalance StartLimitBurst must be >=100 (OCPBUGS-45112)")
+	})

Evidence

The test text added in this PR hard-codes OCPBUGS-45112 in both a comment and an assertion
message; there is no corresponding OCPBUGS-84938 reference in the codebase, creating ambiguity
about which bug the test is intended to cover.

test/e2e/performanceprofile/functests/1_performance/irqbalance.go[365-376]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The StartLimitBurst test references `OCPBUGS-45112` in the comment and assertion message, which may not match the intended tracking bug for this PR.

### Issue Context
This is a maintenance/triage problem: when the test fails, engineers will search the wrong bug.

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

### Suggested change
Update the comment and assertion message to the correct bug ID (or include both IDs if that’s the intent), keeping the test_id intact.

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

---

[openshift-ci-robot]

@openshift-cherrypick-robot: Jira Issue OCPBUGS-84938 has been cloned as Jira Issue OCPBUGS-85016. Will retitle bug to link to clone.

WARNING: Unexpected sprint field type []interface {} on source issue. Please update sprint manually on clone.

/retitle [release-4.21] OCPBUGS-85016: e2e: Add irqbalance StartLimitBurst >= 100 config test

Details

In response to this:

This is an automated cherry-pick of #1493

/assign oblau

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: openshift-cherrypick-robot
Once this PR has been reviewed and has the lgtm label, please assign marsik for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci-robot]

@openshift-cherrypick-robot: This pull request references Jira Issue OCPBUGS-85016, which is invalid:

• expected dependent Jira Issue OCPBUGS-84938 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but it is MODIFIED instead
• expected dependent Jira Issue OCPBUGS-84938 to target a version in 4.22.0, but it targets "5.0" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

This is an automated cherry-pick of #1493

/assign oblau

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[oblau]

/jira refresh

[openshift-ci-robot]

@oblau: This pull request references Jira Issue OCPBUGS-85016, which is invalid:

• expected dependent Jira Issue OCPBUGS-84938 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but it is MODIFIED instead
• expected dependent Jira Issue OCPBUGS-84938 to target a version in 4.22.0, but it targets "5.0" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

@openshift-cherrypick-robot: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-upgrade	de92948	link	true	/test e2e-upgrade

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.