CNF-24269 Add Core control-plane PerformanceProfile#806
Open
imiller0 wants to merge 1 commit into
Open
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: imiller0 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
📝 WalkthroughWalkthroughAdds a control-plane PerformanceProfile, supplies control-plane CPU allocation values, enhances the Helm PerformanceProfile template to detect/render control-plane profiles, updates the overlay and Tuned recommendation to target control-plane, and updates kube-compare ignore rules. ChangesControl-plane performance profile configuration
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
`@telco-core/configuration/reference-crs-kube-compare/required/performance/PerformanceProfile.yaml`:
- Around line 14-25: The template currently calls fromJson on the
"kubeletconfig.experimental" annotation without guarding for its absence which
can cause a failure; update the logic that builds $config (where you currently
do {{- $config := (index (.metadata.annotations | default dict)
"kubeletconfig.experimental") | fromJson }}) to default the annotation to an
empty JSON object (e.g., "{}") or skip fromJson when missing, then proceed to
check $config.allowedUnsafeSysctls and render
kubeletconfig.experimental.systemReserved as before; ensure references to
$isControlPlane, kubeletconfig.experimental, $config, fromJson,
allowedUnsafeSysctls, and systemReserved are preserved so the same checks and
fail message remain valid.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 5b66416c-0dac-4f7d-9dcb-b1d089fb8f2e
📒 Files selected for processing (6)
telco-core/configuration/core-overlay.yamltelco-core/configuration/reference-crs-kube-compare/compare_ignoretelco-core/configuration/reference-crs-kube-compare/required/performance/PerformanceProfile.yamltelco-core/configuration/reference-crs/required/performance/PerformanceProfile-control-plane.yamltelco-core/configuration/reference-crs/required/performance/TunedPerformancePatch.yamltelco-core/configuration/template-values/hw-types.yaml
imiller0
force-pushed
the
add-core-cp-perfprof
branch
from
e8a21e8 to
30d7b33
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
telco-core/configuration/reference-crs-kube-compare/required/performance/PerformanceProfile.yaml (1)
14-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPotential template failure if
kubeletconfig.experimentalannotation is missing.The
fromJsoncall on line 16 will fail if the annotation doesn't exist (empty string). This was flagged in a prior review.Additionally, if
systemReservedkey is missing from the parsed config, line 24 will produce"systemReserved": nullwhich creates an invalid kubelet configuration.Proposed fix
{{- if $isControlPlane }} # systemReserved: when used, it should be tailored for each environment. - {{- $config := (index (.metadata.annotations | default dict) "kubeletconfig.experimental") | fromJson }} + {{- $rawConfig := index (.metadata.annotations | default dict) "kubeletconfig.experimental" }} + {{- $config := $rawConfig | default "{}" | fromJson }} {{- if hasKey $config "allowedUnsafeSysctls" }} {{- if ne (len $config.allowedUnsafeSysctls) 0 }} {{- fail "Control plane PerformanceProfile must not have any allowedUnsafeSysctls" }} {{- end }} {{- end }} + {{- if not (hasKey $config "systemReserved") }} + {{- fail "Control plane PerformanceProfile must have systemReserved in kubeletconfig.experimental annotation" }} + {{- end }} kubeletconfig.experimental: |🤖 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 `@telco-core/configuration/reference-crs-kube-compare/required/performance/PerformanceProfile.yaml` around lines 14 - 25, The template can fail when the "kubeletconfig.experimental" annotation is missing and can emit invalid JSON if "systemReserved" is absent; change the parsing to default the annotation to an empty JSON object string before fromJson (e.g. assign the raw annotation to a variable with default "{}" and then call fromJson into $config), keep the existing conditional checks against $config.allowedUnsafeSysctls, and when rendering kubeletconfig.experimental ensure "systemReserved" is rendered as a non-null JSON object by using a default empty map (e.g. $config.systemReserved | default dict) before toJson so the output produces valid JSON instead of null.
🤖 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.
Duplicate comments:
In
`@telco-core/configuration/reference-crs-kube-compare/required/performance/PerformanceProfile.yaml`:
- Around line 14-25: The template can fail when the "kubeletconfig.experimental"
annotation is missing and can emit invalid JSON if "systemReserved" is absent;
change the parsing to default the annotation to an empty JSON object string
before fromJson (e.g. assign the raw annotation to a variable with default "{}"
and then call fromJson into $config), keep the existing conditional checks
against $config.allowedUnsafeSysctls, and when rendering
kubeletconfig.experimental ensure "systemReserved" is rendered as a non-null
JSON object by using a default empty map (e.g. $config.systemReserved | default
dict) before toJson so the output produces valid JSON instead of null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 853bb5bb-8f1e-489e-9379-fd9b42556850
📒 Files selected for processing (6)
telco-core/configuration/core-overlay.yamltelco-core/configuration/reference-crs-kube-compare/compare_ignoretelco-core/configuration/reference-crs-kube-compare/required/performance/PerformanceProfile.yamltelco-core/configuration/reference-crs/required/performance/PerformanceProfile-control-plane.yamltelco-core/configuration/reference-crs/required/performance/TunedPerformancePatch.yamltelco-core/configuration/template-values/hw-types.yaml
✅ Files skipped from review due to trivial changes (1)
- telco-core/configuration/reference-crs-kube-compare/compare_ignore
🚧 Files skipped from review as they are similar to previous changes (3)
- telco-core/configuration/template-values/hw-types.yaml
- telco-core/configuration/reference-crs/required/performance/PerformanceProfile-control-plane.yaml
- telco-core/configuration/reference-crs/required/performance/TunedPerformancePatch.yaml
Collaborator
Author
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
imiller0
force-pushed
the
add-core-cp-perfprof
branch
from
30d7b33 to
8bee47d
Compare
There was a problem hiding this comment.
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 `@telco-core/configuration/core-overlay.yaml`:
- Around line 135-141: The control-plane PerformanceProfile patch currently only
templates spec.cpu.* and leaves kubeletconfig.experimental.systemReserved.memory
and hugepages hard-coded in
reference-crs/required/performance/PerformanceProfile-control-plane.yaml; update
the overlay to also patch kubeletconfig.experimental.systemReserved.memory and
kubeletconfig.kubeletConfig.podPidsLimit/hugepages (or the hugepages entries
present in that file) by sourcing values from the same hw-types ConfigMap keys
(e.g., add templated patches that read control-plane memory and hugepage sizes
from hw-types, analogous to the existing
control-plane-reserved/control-plane-isolated usage), or alternatively move the
control-plane profile under the same templated path used by worker profiles so
all performance-sensitive fields are consistently parameterized.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 4be7fb8c-e4a6-491f-a6dc-cf4e998abbe2
📒 Files selected for processing (6)
telco-core/configuration/core-overlay.yamltelco-core/configuration/reference-crs-kube-compare/compare_ignoretelco-core/configuration/reference-crs-kube-compare/required/performance/PerformanceProfile.yamltelco-core/configuration/reference-crs/required/performance/PerformanceProfile-control-plane.yamltelco-core/configuration/reference-crs/required/performance/TunedPerformancePatch.yamltelco-core/configuration/template-values/hw-types.yaml
✅ Files skipped from review due to trivial changes (1)
- telco-core/configuration/template-values/hw-types.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- telco-core/configuration/reference-crs-kube-compare/compare_ignore
- telco-core/configuration/reference-crs/required/performance/PerformanceProfile-control-plane.yaml
- telco-core/configuration/reference-crs/required/performance/TunedPerformancePatch.yaml
Add PerformanceProfile and Tuned configuration for Core control-plane MCP. Number of reserved/isolated CPUs, hugepages, and systemReserved memory are all variable content. The kube-compare reference uses the same PerformanceProfile template for both control plane and worker profiles with differences keyed off the machineConfigPoolSelector. Signed-off-by: Ian Miller <imiller@redhat.com>
imiller0
force-pushed
the
add-core-cp-perfprof
branch
from
8bee47d to
453b45c
Compare
There was a problem hiding this comment.
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
`@telco-core/configuration/reference-crs-kube-compare/required/performance/PerformanceProfile.yaml`:
- Around line 55-57: The template is hardcoding defaultHugepagesSize as "1G";
replace the literal with the value from the profile spec so kube-compare renders
the actual requested size. In the block guarded by hasKey .spec "hugepages" (the
hugepages: section), read the size from the profile (e.g.
.spec.hugepages.defaultHugepagesSize or the appropriate
.spec.hugepages.<sizeField> used elsewhere in the PR) and output that value
instead of "1G", preserving YAML structure and indentation so different hugepage
sizes are correctly represented in the generated reference object.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 54323f3c-e239-46c0-9db8-5e4c86ab092a
📒 Files selected for processing (6)
telco-core/configuration/core-overlay.yamltelco-core/configuration/reference-crs-kube-compare/compare_ignoretelco-core/configuration/reference-crs-kube-compare/required/performance/PerformanceProfile.yamltelco-core/configuration/reference-crs/required/performance/PerformanceProfile-control-plane.yamltelco-core/configuration/reference-crs/required/performance/TunedPerformancePatch.yamltelco-core/configuration/template-values/hw-types.yaml
✅ Files skipped from review due to trivial changes (2)
- telco-core/configuration/reference-crs-kube-compare/compare_ignore
- telco-core/configuration/template-values/hw-types.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- telco-core/configuration/reference-crs/required/performance/TunedPerformancePatch.yaml
- telco-core/configuration/core-overlay.yaml
Comment on lines
+55
to
57
| {{- if hasKey .spec "hugepages" }} | ||
| hugepages: | ||
| defaultHugepagesSize: 1G |
There was a problem hiding this comment.
Stop hardcoding defaultHugepagesSize in the shared template.
Line 57 always renders 1G, so kube-compare will produce the wrong reference object for any profile that uses a different hugepage size. That breaks the cross-file contract here because this template is the shared comparison source and the PR is explicitly parameterizing hugepages.
♻️ Suggested fix
hugepages:
- defaultHugepagesSize: 1G
+ defaultHugepagesSize: {{ .spec.hugepages.defaultHugepagesSize }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {{- if hasKey .spec "hugepages" }} | |
| hugepages: | |
| defaultHugepagesSize: 1G | |
| {{- if hasKey .spec "hugepages" }} | |
| hugepages: | |
| defaultHugepagesSize: {{ .spec.hugepages.defaultHugepagesSize }} |
🤖 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
`@telco-core/configuration/reference-crs-kube-compare/required/performance/PerformanceProfile.yaml`
around lines 55 - 57, The template is hardcoding defaultHugepagesSize as "1G";
replace the literal with the value from the profile spec so kube-compare renders
the actual requested size. In the block guarded by hasKey .spec "hugepages" (the
hugepages: section), read the size from the profile (e.g.
.spec.hugepages.defaultHugepagesSize or the appropriate
.spec.hugepages.<sizeField> used elsewhere in the PR) and output that value
instead of "1G", preserving YAML structure and indentation so different hugepage
sizes are correctly represented in the generated reference object.
Collaborator
Author
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
bartwensley
reviewed
Jun 16, 2026
| isolated: {{ .spec.cpu.isolated }} | ||
| reserved: {{ .spec.cpu.reserved }} | ||
| globallyDisableIrqLoadBalancing: false | ||
| {{- if hasKey .spec "hugepages" }} |
There was a problem hiding this comment.
Comment from Claude:
Previously hugepages was always rendered, so kube-compare would flag any PerformanceProfile missing it. The hasKey guard is necessary for the control-plane profile (which intentionally omits hugepages), but it also makes hugepages optional for worker profiles. A tighter guard like {{- if or (not $isControlPlane) (hasKey .spec "hugepages") }} would keep hugepages mandatory for workers while optional for control-plane.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Add PerformanceProfile and Tuned configuration for Core control-plane MCP. Number of reserved/isolated CPUs, hugepages, and systemReserved memory are all variable content.
The kube-compare reference uses the same PerformanceProfile template for both control plane and worker profiles with differences keyed off the machineConfigPoolSelector.