Skip to content

tuned:irqs: set default_smp_affinity using irqaffinity= - #1572

Open
Tal-or wants to merge 2 commits into
openshift:mainfrom
Tal-or:ovsdpdk_use_irqaffinity
Open

tuned:irqs: set default_smp_affinity using irqaffinity=#1572
Tal-or wants to merge 2 commits into
openshift:mainfrom
Tal-or:ovsdpdk_use_irqaffinity

Conversation

@Tal-or

@Tal-or Tal-or commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

When ovsDpdk CPUs are configured, add irqaffinity= to the kernel command line to set default_smp_affinity at boot time instead of manipulating it post-boot via clear-irqbalance-banned-cpus.sh.

AIA Human-AI blend, New content, Human-initiated, Reviewed, Claude Opus 4.6 v1.0

Summary by CodeRabbit

  • New Features
    • Added kernel IRQ-affinity configuration derived from isolated, reserved, and shared CPUs, excluding CPUs assigned to OVS-DPDK.
    • IRQ-affinity is now applied through tuned bootloader kernel arguments when configured.
  • Bug Fixes
    • Prevented the IRQBalance banned-CPU clearing logic from rewriting the system’s default IRQ affinity mask.
    • Kept IRQ-balance banned-CPU settings consistent with the related CRI-O configuration.
  • Tests
    • Reduced test coverage for default IRQ affinity mask handling in the “Clear IRQBalance Banned CPU List” script and updated rendering expectations accordingly.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Walkthrough

The change derives IRQ-affinity CPUs from isolated, reserved, and shared CPUs minus OVS-DPDK CPUs, passes them to tuned templates for conditional kernel arguments, and removes default IRQ affinity mask modification from the irqbalance cleanup script.

Changes

IRQ affinity configuration

Layer / File(s) Summary
Derive and render IRQ affinity
pkg/performanceprofile/controller/performanceprofile/components/tuned/tuned.go, assets/performanceprofile/tuned/openshift-node-performance, assets/performanceprofile/tuned/openshift-node-performance-intel-x86
NewNodePerformance computes the IRQ-affinity CPU set and tuned profiles conditionally render the corresponding bootloader argument.
Preserve default affinity mask
assets/performanceprofile/scripts/clear-irqbalance-banned-cpus.sh, pkg/performanceprofile/controller/performanceprofile/components/assets_scripts_test.go
The cleanup script no longer writes /proc/irq/default_smp_affinity, and tests for that removed behavior are deleted.
Refresh rendered outputs
test/e2e/performanceprofile/testdata/render-expected-output/**
Expected MachineConfig and Tuned outputs reflect the shortened scripts, new IRQ-affinity placeholders, and embedded profile formatting updates.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant NewNodePerformance
  participant TunedTemplate
  participant Bootloader
  NewNodePerformance->>TunedTemplate: Pass derived IRQ-affinity CPU set
  TunedTemplate->>Bootloader: Render cmdline_irqaffinity kernel argument
Loading

Suggested reviewers: swatisehgal, jmencak

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Test Structure And Quality ⚠️ Warning Tests use bare Expect(...).ToNot(HaveOccurred()) throughout, and writeTempFile ignores its content arg, so scenarios aren't properly isolated. Add descriptive failure messages to assertions and fix writeTempFile to write the provided content; consider DeferCleanup/AfterEach for temp files.
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main change: configuring default_smp_affinity via irqaffinity on boot.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The edited Go test file keeps static Ginkgo titles; no dynamic names, timestamps, IDs, or interpolated values were added.
Microshift Test Compatibility ✅ Passed No new Ginkgo e2e tests were added in the diff; the changes are fixture YAMLs only and show no MicroShift-incompatible APIs/features.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; changes are templates/scripts and unit-test removals, so no new SNO-specific risk is introduced.
Topology-Aware Scheduling Compatibility ✅ Passed Changes only add boot-time irqaffinity tuning and remove a post-boot affinity script path; no pod scheduling, replicas, affinity, or topology logic was introduced.
Ote Binary Stdout Contract ✅ Passed No changed entrypoint code writes to stdout; the only prints are GinkgoWriter inside It blocks, which are allowed.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new e2e Ginkgo tests were added; the PR only changes render-expected-output fixtures, so there are no new IPv4 or connectivity assumptions to flag.
No-Weak-Crypto ✅ Passed Changed files only adjust performanceprofile scripts/tuned YAML; scans of diffs and decoded payloads found no weak-crypto or secret-comparison usage.
Container-Privileges ✅ Passed Changed files are test fixtures only, and none contain privileged/hostPID/hostNetwork/hostIPC/allowPrivilegeEscalation/SYS_ADMIN/root settings.
No-Sensitive-Data-In-Logs ✅ Passed No added logs exposing passwords/tokens/PII were found; the only tracing is set -x and echoing CPU masks/paths in the shell script.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@qodo-for-rh-openshift

Copy link
Copy Markdown

PR Summary by Qodo

Set default IRQ affinity via kernel irqaffinity= when ovsDpdk CPUs are configured

✨ Enhancement 🐞 Bug fix ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Add irqaffinity= kernel arg to set default_smp_affinity at boot for ovsDpdk setups.
• Stop post-boot mutation of /proc/irq/default_smp_affinity in clear-irqbalance-banned-cpus.sh.
• Compute irqaffinity CPU list as “all profile CPUs minus ovsDpdk CPUs” for tuned rendering.
Diagram

graph TD
  A["PerformanceProfile (CPU spec)"] --> B["tuned.go (compute IrqAffinityCpus)"] --> C["Tuned asset: openshift-node-performance"] --> D["Kernel cmdline: irqaffinity="] --> E["/proc/irq/default_smp_affinity"]
  C --> F["Script: clear-irqbalance-banned-cpus.sh"] --> G["/etc/sysconfig/irqbalance"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Early-boot systemd unit to write /proc/irq/default_smp_affinity
  • ➕ Avoids changing kernel cmdline (potentially simpler rollback).
  • ➕ Can implement richer logic (e.g., topology-aware masks) at runtime.
  • ➖ Still a post-boot mutation path; ordering vs CRI-O/irqbalance can be fragile.
  • ➖ More moving parts (unit ordering, retries) than a kernel boot arg.
2. Rely only on irqbalance banned CPUs (no default_smp_affinity changes)
  • ➕ Minimal change surface; keeps behavior limited to irqbalance.
  • ➖ Does not address consumers like CRI-O that derive behavior from default_smp_affinity.
  • ➖ Can lead to inconsistent IRQ masks when other components “restore” or recompute settings.
3. Use a sysctl/tuned sysfs write instead of kernel cmdline
  • ➕ Keeps control inside tuned rather than bootloader args.
  • ➕ Potentially more portable across bootloader implementations.
  • ➖ Still depends on runtime ordering; may be too late for early IRQ routing decisions.
  • ➖ More susceptible to being overwritten by other agents.

Recommendation: The PR’s approach (setting default_smp_affinity via irqaffinity= at boot) is the most robust for ovsDpdk cases because it establishes the desired default before user space components (e.g., CRI-O restore flows) can observe or overwrite it. The main review focus should be verifying the computed CPU list semantics (“all profile CPUs minus ovsDpdk CPUs”) matches the intended IRQ routing policy, and that the kernel argument format produced by cpuset.String() is accepted in the target environments.

Files changed (3) +14 / -25

Enhancement (1) +10 / -0
tuned.goCompute IrqAffinityCpus for ovsDpdk and pass into tuned template +10/-0

Compute IrqAffinityCpus for ovsDpdk and pass into tuned template

• Adds a new template argument ('IrqAffinityCpus') when 'Spec.CPU.OvsDpdk' is set. The value is derived as the union of isolated (+ovsDpdk) and reserved CPUs, minus the ovsDpdk CPU set, and is used by the tuned asset to render 'irqaffinity='.

pkg/performanceprofile/controller/performanceprofile/components/tuned/tuned.go

Refactor (1) +0 / -25
clear-irqbalance-banned-cpus.shRemove post-boot default_smp_affinity rewrite from irqbalance helper script +0/-25

Remove post-boot default_smp_affinity rewrite from irqbalance helper script

• Deletes the logic that masked OVS-DPDK CPUs out of '/proc/irq/default_smp_affinity'. The script now only manages 'IRQBALANCE_BANNED_CPUS' and keeps CRI-O’s original-banned-cpus file consistent.

assets/performanceprofile/scripts/clear-irqbalance-banned-cpus.sh

Other (1) +4 / -0
openshift-node-performanceAdd conditional irqaffinity= kernel arg rendering to tuned profile +4/-0

Add conditional irqaffinity= kernel arg rendering to tuned profile

• Introduces a new 'cmdline_irqaffinity' stanza that appends 'irqaffinity={{.IrqAffinityCpus}}' when the template variable is present. This shifts default IRQ affinity setup to boot time.

assets/performanceprofile/tuned/openshift-node-performance

@openshift-ci
openshift-ci Bot requested review from jmencak and swatisehgal July 29, 2026 12:16
@openshift-ci

openshift-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Tal-or
Once this PR has been reviewed and has the lgtm label, please assign jmencak 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:

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
pkg/performanceprofile/controller/performanceprofile/components/tuned/tuned.go (1)

78-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add regression coverage for the CPU-set contract.

Extend pkg/performanceprofile/controller/performanceprofile/components/tuned/tuned_test.go to cover reserved/isolated/OVS-DPDK overlap, canonical output, and invalid empty results.

🤖 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
`@pkg/performanceprofile/controller/performanceprofile/components/tuned/tuned.go`
around lines 78 - 86, Extend the tests for the tuned template CPU-set
construction around the reserved/isolated merge and OVS-DPDK subtraction,
covering overlapping inputs, canonical ordering of the resulting affinity
string, and cases where subtraction produces an invalid empty set. Use the
existing test helpers and assertions in tuned_test.go, anchoring coverage to the
templateIrqAffinityCpus behavior and preserving the current error contract.
🤖 Prompt for all review comments with 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.

Nitpick comments:
In
`@pkg/performanceprofile/controller/performanceprofile/components/tuned/tuned.go`:
- Around line 78-86: Extend the tests for the tuned template CPU-set
construction around the reserved/isolated merge and OVS-DPDK subtraction,
covering overlapping inputs, canonical ordering of the resulting affinity
string, and cases where subtraction produces an invalid empty set. Use the
existing test helpers and assertions in tuned_test.go, anchoring coverage to the
templateIrqAffinityCpus behavior and preserving the current error contract.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 5148f8fc-6698-4f5a-875b-80fa874cf6bc

📥 Commits

Reviewing files that changed from the base of the PR and between a9d25d5 and ef6d67e.

📒 Files selected for processing (3)
  • assets/performanceprofile/scripts/clear-irqbalance-banned-cpus.sh
  • assets/performanceprofile/tuned/openshift-node-performance
  • pkg/performanceprofile/controller/performanceprofile/components/tuned/tuned.go
💤 Files with no reviewable changes (1)
  • assets/performanceprofile/scripts/clear-irqbalance-banned-cpus.sh

@qodo-for-rh-openshift

qodo-for-rh-openshift Bot commented Jul 29, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Stale SMP affinity tests ✓ Resolved 🐞 Bug ≡ Correctness
Description
clear-irqbalance-banned-cpus.sh no longer updates default_smp_affinity, but assets_scripts_test.go
still asserts the script clears OVS-DPDK CPUs from a default_smp_affinity file, so the unit tests
will fail.
Code

assets/performanceprofile/scripts/clear-irqbalance-banned-cpus.sh[L36-59]

-# CRI-O reads /proc/irq/default_smp_affinity to derive the IRQ banned mask
-# when pods with irq-load-balancing.crio.io=disable are scheduled.
-# If we don't remove the OVS-DPDK CPUs from default_smp_affinity here,
-# CRI-O will overwrite IRQBALANCE_BANNED_CPUS while ignoring the OVS-DPDK CPUs.
-SMP_AFFINITY="${3:-/proc/irq/default_smp_affinity}"
-if [ "${BANNED_CPUS}" != "${NONE}" ] && [ -f "${SMP_AFFINITY}" ]; then
-	# default_smp_affinity is comma-separated 32-bit hex groups (e.g. "ff,ffffffff,ffffffff")
-	IFS=',' read -ra smp < "${SMP_AFFINITY}"
-	n=${#smp[@]}
-	# pad BANNED_CPUS with leading zeros to match the same number of hex chars
-	padded="${BANNED_CPUS}"
-	while [ ${#padded} -lt $(( n * 8 )) ]; do
-		padded="0${padded}"
-	done
-	# clear banned bits from each 32-bit group: result = smp & ~banned
-	result=""
-	for (( i=0; i<n; i++ )); do
-		ban="${padded:$(( i * 8 )):8}"
-		val=$(printf "%08x" $(( 0x${smp[$i]} & ~0x${ban} )))
-		result+="${result:+,}${val}"
-	done
-	echo "Setting default_smp_affinity to ${result} (removing OVS-DPDK CPUs ${BANNED_CPUS} from default_smp_affinity mask)"
-	echo "${result}" > "${SMP_AFFINITY}"
-fi
Relevance

●●● Strong

Team previously updated assets_scripts_test.go when changing clear-irqbalance-banned-cpus.sh
behavior (PR #1546).

PR-#1546

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script no longer contains any logic to read/write a default_smp_affinity path, while the unit
tests still pass a smpAffinity file path and assert its contents equal the cleared mask.

assets/performanceprofile/scripts/clear-irqbalance-banned-cpus.sh[1-34]
pkg/performanceprofile/controller/performanceprofile/components/assets_scripts_test.go[131-201]

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

## Issue description
`clear-irqbalance-banned-cpus.sh` no longer edits `/proc/irq/default_smp_affinity`, but the unit tests still expect it to clear OVS-DPDK CPUs from a supplied `default_smp_affinity` file path.

## Issue Context
The script now only updates `IRQBALANCE_BANNED_CPUS` and the CRI-O backup file, leaving `default_smp_affinity` untouched. The existing tests invoke the script with a `default_smp_affinity` temp file and assert its contents change.

## Fix Focus Areas
- pkg/performanceprofile/controller/performanceprofile/components/assets_scripts_test.go[131-201]
- assets/performanceprofile/scripts/clear-irqbalance-banned-cpus.sh[1-34]

## Expected fix
- Either remove/replace the `DescribeTable("should clear ovs-dpdk cpus from default_smp_affinity", ...)` assertions to match the new script behavior, **or** reintroduce the old `default_smp_affinity` mutation if it is still required.
- If the new intended behavior is “set via kernel cmdline irqaffinity”, add a new test that validates the rendered tuned profile includes `irqaffinity=` when OVS-DPDK CPUs are configured (instead of testing the script modifies `default_smp_affinity`).

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



Remediation recommended

2. Shared CPUs omitted ✓ Resolved 🐞 Bug ≡ Correctness
Description
NewNodePerformance computes IrqAffinityCpus from reserved/isolated CPUs (minus ovsDpdk), ignoring
spec.cpu.shared; when MixedCpus is enabled, shared CPUs are included in kubelet ReservedSystemCPUs
but will be excluded from irqaffinity.
Code

pkg/performanceprofile/controller/performanceprofile/components/tuned/tuned.go[R78-86]

+			allCpus := isolatedSet
+			if profile.Spec.CPU.Reserved != nil {
+				reservedSet, err := cpuset.Parse(string(*profile.Spec.CPU.Reserved))
+				if err != nil {
+					return nil, fmt.Errorf("cannot parse reserved cpuset: %v", err)
+				}
+				allCpus = allCpus.Union(reservedSet)
+			}
+			templateArgs[templateIrqAffinityCpus] = allCpus.Difference(ovsDpdkSet).String()
Relevance

●● Moderate

No prior reviews found about including spec.cpu.shared in irqaffinity/MixedCpus calculations;
acceptance unclear.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new irqaffinity set is built only from isolated/reserved, while kubelet config explicitly adds
shared CPUs into ReservedSystemCPUs under MixedCPUs, demonstrating an inconsistency introduced by
the new computation.

pkg/performanceprofile/controller/performanceprofile/components/tuned/tuned.go[66-87]
pkg/performanceprofile/controller/performanceprofile/components/kubeletconfig/kubeletconfig.go[160-183]
pkg/apis/performanceprofile/v2/performanceprofile_types.go[118-152]

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

## Issue description
When OVS-DPDK CPUs are configured, `IrqAffinityCpus` is computed as `(isolated ∪ reserved) \ ovsDpdk`. This ignores `spec.cpu.shared`, which is used under MixedCpus and is treated as part of the kubelet’s `ReservedSystemCPUs`.

## Issue Context
In MixedCpus mode, the operator unions `shared` CPUs into `ReservedSystemCPUs`, indicating they’re intended to behave like system/housekeeping CPUs. Excluding them from `irqaffinity=` prevents default IRQ affinity from including CPUs the kubelet is explicitly reserving for system use.

## Fix Focus Areas
- pkg/performanceprofile/controller/performanceprofile/components/tuned/tuned.go[66-91]
- pkg/performanceprofile/controller/performanceprofile/components/kubeletconfig/kubeletconfig.go[160-183]

## Expected fix
- When calculating `allCpus` for `IrqAffinityCpus`, also union in `spec.cpu.shared` **when MixedCpus is enabled** (e.g., via `profilecomponent.IsMixedCPUsEnabled(profile)`), then subtract `ovsDpdkSet`.
- Keep validation/error handling consistent with existing cpuset parsing patterns.

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



Informational

3. Intel cmdline key missing ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
openshift-node-performance introduces cmdline_irqaffinity, but
openshift-node-performance-intel-x86’s documented cmdline_* ordering placeholder block does not
include cmdline_irqaffinity, undermining Intel’s stable kernel-arg ordering mechanism.
Code

assets/performanceprofile/tuned/openshift-node-performance[R180-182]

+{{if .IrqAffinityCpus}}
+cmdline_irqaffinity=+irqaffinity={{.IrqAffinityCpus}}
+{{end}}
Relevance

●● Moderate

No historical evidence found; file path wasn’t present on default branch, so team precedent is
unclear.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The base tuned profile adds a new cmdline_* variable and notes Intel ordering depends on these
names, while the Intel x86 ordering list does not include the new cmdline key.

assets/performanceprofile/tuned/openshift-node-performance[141-186]
assets/performanceprofile/tuned/openshift-node-performance-intel-x86[4-23]

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

## Issue description
Intel x86 profile has an explicit list of `cmdline_*` placeholders to control/lock kernel-arg ordering, but the newly added `cmdline_irqaffinity` key is not declared there.

## Issue Context
The base profile explicitly states Intel relies on these names/order. Without adding the new key to the Intel placeholder list, `irqaffinity=` may be appended in a different position than intended on Intel nodes.

## Fix Focus Areas
- assets/performanceprofile/tuned/openshift-node-performance[141-186]
- assets/performanceprofile/tuned/openshift-node-performance-intel-x86[4-23]

## Expected fix
- Add `cmdline_irqaffinity=` to the Intel x86 placeholder block near the other cmdline_* entries so ordering remains deterministic when the base profile sets `cmdline_irqaffinity`.

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


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread assets/performanceprofile/scripts/clear-irqbalance-banned-cpus.sh
Comment thread assets/performanceprofile/tuned/openshift-node-performance
When OVS-DPDK CPUs are configured, add irqaffinity= to the kernel
command line to set default_smp_affinity at boot time instead of
manipulating it post-boot via clear-irqbalance-banned-cpus.sh.

- Compute irqaffinity as all CPUs (reserved + isolated + shared) minus
  OVS-DPDK CPUs, so shared CPUs are not accidentally excluded when
  MixedCpus is enabled.
- Add cmdline_irqaffinity placeholder to the Intel x86 profile's
  ordering block to maintain stable kernel-arg ordering during upgrades.
- Remove stale unit tests that verified the now-removed
  default_smp_affinity manipulation in clear-irqbalance-banned-cpus.sh.

AIA Human-AI blend, New content, Human-initiated, Reviewed, Claude Opus 4.6 v1.0

Signed-off-by: Talor Itzhak <titzhak@redhat.com>
@Tal-or
Tal-or force-pushed the ovsdpdk_use_irqaffinity branch from ef6d67e to a810671 Compare July 29, 2026 12:38
Call  `make render-sync` to generate updated test data.

Signed-off-by: Talor Itzhak <titzhak@redhat.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In
`@test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-master_machineconfig.yaml`:
- Line 38: Remove or explicitly gate the unconditional set -x in the
clear-irqbalance-banned-cpus.sh source asset, then regenerate the embedded
payloads in all five listed MachineConfig fixtures:
test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-master_machineconfig.yaml:38-38,
test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-worker_machineconfig.yaml:38-38,
test/e2e/performanceprofile/testdata/render-expected-output/default/arm/manual_machineconfig.yaml:37-37,
test/e2e/performanceprofile/testdata/render-expected-output/default/cpuFrequency/manual_machineconfig.yaml:37-37,
and
test/e2e/performanceprofile/testdata/render-expected-output/default/manual_machineconfig.yaml:38-38,
ensuring none enables boot-time shell tracing by default.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 740993a2-405e-4348-b0f9-7ca44ecc5b74

📥 Commits

Reviewing files that changed from the base of the PR and between a810671 and 293da86.

📒 Files selected for processing (18)
  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/extra-mcp/openshift-bootstrap-master_machineconfig.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/extra-mcp/openshift-bootstrap-master_tuned.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/extra-mcp/openshift-bootstrap-worker_machineconfig.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/extra-mcp/openshift-bootstrap-worker_tuned.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-master_machineconfig.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-master_tuned.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-worker_machineconfig.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-worker_tuned.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/default/arm/manual_machineconfig.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/default/arm/manual_tuned.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/default/cpuFrequency/manual_machineconfig.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/default/cpuFrequency/manual_tuned.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/default/manual_machineconfig.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/default/manual_tuned.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/default/pp-norps/manual_machineconfig.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/default/pp-norps/manual_tuned.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/no-ref/manual_machineconfig.yaml
  • test/e2e/performanceprofile/testdata/render-expected-output/no-ref/manual_tuned.yaml

user: {}
- contents:
source: data:text/plain;charset=utf-8;base64,IyEvdXNyL2Jpbi9lbnYgYmFzaApzZXQgLWV1byBwaXBlZmFpbApzZXQgLXgKCiMgY29uc3QKU0VEPSIvdXNyL2Jpbi9zZWQiCiMgdHVuYWJsZSAtIG92ZXJyaWRhYmxlIGZvciB0ZXN0aW5nIHB1cnBvc2VzCklSUUJBTEFOQ0VfQ09ORj0iJHsxOi0vZXRjL3N5c2NvbmZpZy9pcnFiYWxhbmNlfSIKQ1JJT19PUklHX0JBTk5FRF9DUFVTPSIkezI6LS9ldGMvc3lzY29uZmlnL29yaWdfaXJxX2Jhbm5lZF9jcHVzfSIKTk9ORT0wCgojIEJBTk5FRF9DUFVTOiB0aGUgZmluYWwgaGV4IG1hc2sgd3JpdHRlbiB0byBJUlFCQUxBTkNFX0JBTk5FRF9DUFVTLgojICAgMS4gSWYgT1ZTX0RQREtfQ1BVUyBpcyBzZXQgKHZpYSBzeXN0ZW1kIEVudmlyb25tZW50KSwgYmFuIHRob3NlIENQVXMuCiMgICAyLiBPdGhlcndpc2UsIGRlZmF1bHQgdG8gMCAobm8gQ1BVcyBiYW5uZWQsIGFsbCBwYXJ0aWNpcGF0ZSBpbiBiYWxhbmNpbmcpLgppZiBbIC1uICIke09WU19EUERLX0NQVVM6LX0iIF07IHRoZW4KCUJBTk5FRF9DUFVTPSIke09WU19EUERLX0NQVVN9IgplbHNlCglCQU5ORURfQ1BVUz0iJHtOT05FfSIKZmkKClsgISAtZiAiJHtJUlFCQUxBTkNFX0NPTkZ9IiBdICYmIGV4aXQgMAoKJHtTRUR9IC1pICcvXlxzKklSUUJBTEFOQ0VfQkFOTkVEX0NQVVNcYi9kJyAiJHtJUlFCQUxBTkNFX0NPTkZ9IiB8fCBleGl0IDAKIyBDUFUgbnVtYmVycyB3aGljaCBoYXZlIHRoZWlyIGNvcnJlc3BvbmRpbmcgYml0cyBzZXQgdG8gb25lIGluIHRoaXMgbWFzawojIHdpbGwgbm90IGhhdmUgYW55IGlycSdzIGFzc2lnbmVkIHRvIHRoZW0gb24gcmViYWxhbmNlLgojIHNvIHplcm8gbWVhbnMgYWxsIGNwdXMgYXJlIHBhcnRpY2lwYXRpbmcgaW4gbG9hZCBiYWxhbmNpbmcuCmVjaG8gIklSUUJBTEFOQ0VfQkFOTkVEX0NQVVM9JHtCQU5ORURfQ1BVU30iID4+ICIke0lSUUJBTEFOQ0VfQ09ORn0iCgojIHdlIG5vdyBvd24gdGhpcyBjb25maWd1cmF0aW9uLiBCdXQgQ1JJLU8gaGFzIGNvZGUgdG8gcmVzdG9yZSB0aGUgY29uZmlndXJhdGlvbiwKIyBhbmQgdW50aWwgaXQgZ2FpbnMgdGhlIG9wdGlvbiB0byBkaXNhYmxlIHRoaXMgcmVzdG9yZSBmbG93LCB3ZSBuZWVkIHRvIG1ha2UKIyB0aGUgY29uZmlndXJhdGlvbiBjb25zaXN0ZW50IHN1Y2ggYXMgdGhlIENSSS1PIHJlc3RvcmUgd2lsbCBkbyBub3RoaW5nLgppZiBbIC1uICIke0NSSU9fT1JJR19CQU5ORURfQ1BVU30iIF0gJiYgWyAtZiAiJHtDUklPX09SSUdfQkFOTkVEX0NQVVN9IiBdOyB0aGVuCgllY2hvICIke0JBTk5FRF9DUFVTfSIgPiAiJHtDUklPX09SSUdfQkFOTkVEX0NQVVN9IgpmaQoKIyBDUkktTyByZWFkcyAvcHJvYy9pcnEvZGVmYXVsdF9zbXBfYWZmaW5pdHkgdG8gZGVyaXZlIHRoZSBJUlEgYmFubmVkIG1hc2sKIyB3aGVuIHBvZHMgd2l0aCBpcnEtbG9hZC1iYWxhbmNpbmcuY3Jpby5pbz1kaXNhYmxlIGFyZSBzY2hlZHVsZWQuCiMgSWYgd2UgZG9uJ3QgcmVtb3ZlIHRoZSBPVlMtRFBESyBDUFVzIGZyb20gZGVmYXVsdF9zbXBfYWZmaW5pdHkgaGVyZSwKIyBDUkktTyB3aWxsIG92ZXJ3cml0ZSBJUlFCQUxBTkNFX0JBTk5FRF9DUFVTIHdoaWxlIGlnbm9yaW5nIHRoZSBPVlMtRFBESyBDUFVzLgpTTVBfQUZGSU5JVFk9IiR7MzotL3Byb2MvaXJxL2RlZmF1bHRfc21wX2FmZmluaXR5fSIKaWYgWyAiJHtCQU5ORURfQ1BVU30iICE9ICIke05PTkV9IiBdICYmIFsgLWYgIiR7U01QX0FGRklOSVRZfSIgXTsgdGhlbgoJIyBkZWZhdWx0X3NtcF9hZmZpbml0eSBpcyBjb21tYS1zZXBhcmF0ZWQgMzItYml0IGhleCBncm91cHMgKGUuZy4gImZmLGZmZmZmZmZmLGZmZmZmZmZmIikKCUlGUz0nLCcgcmVhZCAtcmEgc21wIDwgIiR7U01QX0FGRklOSVRZfSIKCW49JHsjc21wW0BdfQoJIyBwYWQgQkFOTkVEX0NQVVMgd2l0aCBsZWFkaW5nIHplcm9zIHRvIG1hdGNoIHRoZSBzYW1lIG51bWJlciBvZiBoZXggY2hhcnMKCXBhZGRlZD0iJHtCQU5ORURfQ1BVU30iCgl3aGlsZSBbICR7I3BhZGRlZH0gLWx0ICQoKCBuICogOCApKSBdOyBkbwoJCXBhZGRlZD0iMCR7cGFkZGVkfSIKCWRvbmUKCSMgY2xlYXIgYmFubmVkIGJpdHMgZnJvbSBlYWNoIDMyLWJpdCBncm91cDogcmVzdWx0ID0gc21wICYgfmJhbm5lZAoJcmVzdWx0PSIiCglmb3IgKCggaT0wOyBpPG47IGkrKyApKTsgZG8KCQliYW49IiR7cGFkZGVkOiQoKCBpICogOCApKTo4fSIKCQl2YWw9JChwcmludGYgIiUwOHgiICQoKCAweCR7c21wWyRpXX0gJiB+MHgke2Jhbn0gKSkpCgkJcmVzdWx0Kz0iJHtyZXN1bHQ6Kyx9JHt2YWx9IgoJZG9uZQoJZWNobyAiU2V0dGluZyBkZWZhdWx0X3NtcF9hZmZpbml0eSB0byAke3Jlc3VsdH0gKHJlbW92aW5nIE9WUy1EUERLIENQVXMgJHtCQU5ORURfQ1BVU30gZnJvbSBkZWZhdWx0X3NtcF9hZmZpbml0eSBtYXNrKSIKCWVjaG8gIiR7cmVzdWx0fSIgPiAiJHtTTVBfQUZGSU5JVFl9IgpmaQo=
source: data:text/plain;charset=utf-8;base64,IyEvdXNyL2Jpbi9lbnYgYmFzaApzZXQgLWV1byBwaXBlZmFpbApzZXQgLXgKCiMgY29uc3QKU0VEPSIvdXNyL2Jpbi9zZWQiCiMgdHVuYWJsZSAtIG92ZXJyaWRhYmxlIGZvciB0ZXN0aW5nIHB1cnBvc2VzCklSUUJBTEFOQ0VfQ09ORj0iJHsxOi0vZXRjL3N5c2NvbmZpZy9pcnFiYWxhbmNlfSIKQ1JJT19PUklHX0JBTk5FRF9DUFVTPSIkezI6LS9ldGMvc3lzY29uZmlnL29yaWdfaXJxX2Jhbm5lZF9jcHVzfSIKTk9ORT0wCgojIEJBTk5FRF9DUFVTOiB0aGUgZmluYWwgaGV4IG1hc2sgd3JpdHRlbiB0byBJUlFCQUxBTkNFX0JBTk5FRF9DUFVTLgojICAgMS4gSWYgT1ZTX0RQREtfQ1BVUyBpcyBzZXQgKHZpYSBzeXN0ZW1kIEVudmlyb25tZW50KSwgYmFuIHRob3NlIENQVXMuCiMgICAyLiBPdGhlcndpc2UsIGRlZmF1bHQgdG8gMCAobm8gQ1BVcyBiYW5uZWQsIGFsbCBwYXJ0aWNpcGF0ZSBpbiBiYWxhbmNpbmcpLgppZiBbIC1uICIke09WU19EUERLX0NQVVM6LX0iIF07IHRoZW4KCUJBTk5FRF9DUFVTPSIke09WU19EUERLX0NQVVN9IgplbHNlCglCQU5ORURfQ1BVUz0iJHtOT05FfSIKZmkKClsgISAtZiAiJHtJUlFCQUxBTkNFX0NPTkZ9IiBdICYmIGV4aXQgMAoKJHtTRUR9IC1pICcvXlxzKklSUUJBTEFOQ0VfQkFOTkVEX0NQVVNcYi9kJyAiJHtJUlFCQUxBTkNFX0NPTkZ9IiB8fCBleGl0IDAKIyBDUFUgbnVtYmVycyB3aGljaCBoYXZlIHRoZWlyIGNvcnJlc3BvbmRpbmcgYml0cyBzZXQgdG8gb25lIGluIHRoaXMgbWFzawojIHdpbGwgbm90IGhhdmUgYW55IGlycSdzIGFzc2lnbmVkIHRvIHRoZW0gb24gcmViYWxhbmNlLgojIHNvIHplcm8gbWVhbnMgYWxsIGNwdXMgYXJlIHBhcnRpY2lwYXRpbmcgaW4gbG9hZCBiYWxhbmNpbmcuCmVjaG8gIklSUUJBTEFOQ0VfQkFOTkVEX0NQVVM9JHtCQU5ORURfQ1BVU30iID4+ICIke0lSUUJBTEFOQ0VfQ09ORn0iCgojIHdlIG5vdyBvd24gdGhpcyBjb25maWd1cmF0aW9uLiBCdXQgQ1JJLU8gaGFzIGNvZGUgdG8gcmVzdG9yZSB0aGUgY29uZmlndXJhdGlvbiwKIyBhbmQgdW50aWwgaXQgZ2FpbnMgdGhlIG9wdGlvbiB0byBkaXNhYmxlIHRoaXMgcmVzdG9yZSBmbG93LCB3ZSBuZWVkIHRvIG1ha2UKIyB0aGUgY29uZmlndXJhdGlvbiBjb25zaXN0ZW50IHN1Y2ggYXMgdGhlIENSSS1PIHJlc3RvcmUgd2lsbCBkbyBub3RoaW5nLgppZiBbIC1uICIke0NSSU9fT1JJR19CQU5ORURfQ1BVU30iIF0gJiYgWyAtZiAiJHtDUklPX09SSUdfQkFOTkVEX0NQVVN9IiBdOyB0aGVuCgllY2hvICIke0JBTk5FRF9DUFVTfSIgPiAiJHtDUklPX09SSUdfQkFOTkVEX0NQVVN9IgpmaQo=

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove set -x from the embedded early-boot script.

The same updated clear-irqbalance-banned-cpus.sh payload enables shell tracing in every affected MachineConfig, causing boot-time command and configuration values to be written to the system journal. Remove or explicitly gate tracing in the source asset, then regenerate all rendered fixtures.

  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-master_machineconfig.yaml#L38-L38: regenerate the payload without unconditional set -x.
  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-worker_machineconfig.yaml#L38-L38: regenerate the payload without unconditional set -x.
  • test/e2e/performanceprofile/testdata/render-expected-output/default/arm/manual_machineconfig.yaml#L37-L37: regenerate the payload without unconditional set -x.
  • test/e2e/performanceprofile/testdata/render-expected-output/default/cpuFrequency/manual_machineconfig.yaml#L37-L37: regenerate the payload without unconditional set -x.
  • test/e2e/performanceprofile/testdata/render-expected-output/default/manual_machineconfig.yaml#L38-L38: regenerate the payload without unconditional set -x.
📍 Affects 5 files
  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-master_machineconfig.yaml#L38-L38 (this comment)
  • test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-worker_machineconfig.yaml#L38-L38
  • test/e2e/performanceprofile/testdata/render-expected-output/default/arm/manual_machineconfig.yaml#L37-L37
  • test/e2e/performanceprofile/testdata/render-expected-output/default/cpuFrequency/manual_machineconfig.yaml#L37-L37
  • test/e2e/performanceprofile/testdata/render-expected-output/default/manual_machineconfig.yaml#L38-L38
🤖 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/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-master_machineconfig.yaml`
at line 38, Remove or explicitly gate the unconditional set -x in the
clear-irqbalance-banned-cpus.sh source asset, then regenerate the embedded
payloads in all five listed MachineConfig fixtures:
test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-master_machineconfig.yaml:38-38,
test/e2e/performanceprofile/testdata/render-expected-output/bootstrap/no-mcp/openshift-bootstrap-worker_machineconfig.yaml:38-38,
test/e2e/performanceprofile/testdata/render-expected-output/default/arm/manual_machineconfig.yaml:37-37,
test/e2e/performanceprofile/testdata/render-expected-output/default/cpuFrequency/manual_machineconfig.yaml:37-37,
and
test/e2e/performanceprofile/testdata/render-expected-output/default/manual_machineconfig.yaml:38-38,
ensuring none enables boot-time shell tracing by default.

@openshift-ci

openshift-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

@Tal-or: The following tests 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-gcp-pao 293da86 link true /test e2e-gcp-pao
ci/prow/e2e-hypershift-pao 293da86 link true /test e2e-hypershift-pao
ci/prow/e2e-gcp-pao-updating-profile 293da86 link true /test e2e-gcp-pao-updating-profile

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant