CNF-24269 Add Core control-plane PerformanceProfile#806
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a control-plane PerformanceProfile, supplies control-plane CPU allocation values, updates the Helm template to detect and render control-plane profiles, adjusts overlay and Tuned configuration for the control-plane target, and updates kube-compare ignore rules. ChangesControl-plane performance profile configuration
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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
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
|
/hold |
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
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
|
/unhold |
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>
453b45c to
61b6b2f
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 16-26: The kubeletconfig.experimental template in
PerformanceProfile.yaml renders systemReserved as null when
$config.systemReserved is missing, which creates a false kube-compare diff.
Update the template around the $rawConfig/$config handling and the
kubeletconfig.experimental block to conditionally include the systemReserved
field only when it exists in the parsed annotation, using the existing $config
and hasKey-style checks to keep the rendered reference aligned with a live
control-plane profile.
🪄 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: b5bb37ce-d426-4b12-8287-643ff5d7191a
📒 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 as they are similar to previous changes (5)
- telco-core/configuration/reference-crs-kube-compare/compare_ignore
- telco-core/configuration/template-values/hw-types.yaml
- telco-core/configuration/reference-crs/required/performance/TunedPerformancePatch.yaml
- telco-core/configuration/reference-crs/required/performance/PerformanceProfile-control-plane.yaml
- telco-core/configuration/core-overlay.yaml
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bartwensley, 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 |
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.