feat(aqc): add EnableBypassCPUSetAdjustment flag for CPU plugin - #204
feat(aqc): add EnableBypassCPUSetAdjustment flag for CPU plugin#204luomingmeng wants to merge 2 commits into
Conversation
Add a new boolean field EnableBypassCPUSetAdjustment to CPUPluginConfig to allow bypassing cpuset adjustment in QRM for shared/reclaimed/system pools, delegating the work to reconcile plugin instead. Also generate deepcopy functions for the new field and update CRD schema.
📝 WalkthroughWalkthroughAdds the optional ChangesEnableBypassCPUSetAdjustment Field Addition
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Temporarily uses a replace directive pointing to the fork branch dev/support-bypass-cpuset-adjustment (kubewharf/katalyst-api#204). Once the API PR is merged, this commit will be squashed and replaced with a proper version bump against kubewharf/katalyst-api. (cherry picked from commit 25587f9)
![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 `@config/crd/bases/config.katalyst.kubewharf.io_adminqosconfigurations.yaml`:
- Around line 820-825: Update the Go field comment for
enableBypassCPUSetAdjustment to state that PackAllocationResponse skips cpuset
population for shared_cores, reclaimed_cores, and system_cores, while dedicated
pools remain unaffected; then regenerate the CRD so its description matches.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e064476-6427-4890-b087-1ad4549eb0a2
📒 Files selected for processing (3)
config/crd/bases/config.katalyst.kubewharf.io_adminqosconfigurations.yamlpkg/apis/config/v1alpha1/adminqos.gopkg/apis/config/v1alpha1/zz_generated.deepcopy.go
✅ Files skipped from review due to trivial changes (1)
- pkg/apis/config/v1alpha1/zz_generated.deepcopy.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/apis/config/v1alpha1/adminqos.go
| enableBypassCPUSetAdjustment: | ||
| description: |- | ||
| EnableBypassCPUSetAdjustment controls whether GetResourcesAllocation clears | ||
| CPU AllocationResult for all QoS classes. Allocation responses returned by | ||
| Allocate/AllocateForPod keep their cpuset unchanged. | ||
| type: boolean |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 'EnableBypassCPUSetAdjustment|PackAllocationResponse|GetResourcesAllocation' \
pkg/apis/config/v1alpha1 configRepository: kubewharf/katalyst-api
Length of output: 3713
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Locate relevant symbols and their surrounding structure.
fd -a '.*\.go$' pkg | sed 's#^\./##' | while read -r f; do
if rg -n 'EnableBypassCPUSetAdjustment|PackAllocationResponse|GetResourcesAllocation' "$f" >/dev/null 2>&1; then
echo "### $f"
ast-grep outline "$f" --view expanded || true
fi
doneRepository: kubewharf/katalyst-api
Length of output: 37299
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Find all references and print compact context.
rg -n -C 6 'EnableBypassCPUSetAdjustment|PackAllocationResponse|GetResourcesAllocation' .Repository: kubewharf/katalyst-api
Length of output: 5866
🌐 Web query:
"EnableBypassCPUSetAdjustment PackAllocationResponse shared_cores reclaimed_cores system_cores"
💡 Result:
EnableBypassCPUSetAdjustment is a configuration feature within the Katalyst QoS Resource Manager (QRM) CPU plugin, designed to optimize how CPU sets (cpusets) are managed for specific Quality of Service (QoS) classes [1]. When this feature is enabled, the QRM CPU plugin skips the backfilling of the cpuset string in the ResourceAllocationResponse (the PackAllocationResponse) returned to the kubelet for pods belonging to the shared_cores, reclaimed_cores, and system_cores pools [1]. By bypassing this backfill process, the system allows the reconcile plugin to take ownership of cpuset delivery, rather than having it set during the initial allocation response [1]. Key aspects of this feature include: - Scope: It specifically impacts shared_cores, reclaimed_cores, and system_cores pools [1]. Dedicated_cores pools remain unaffected by this bypass [1]. - Implementation: It acts as a response post-processing step within the dynamic policy of the QRM CPU plugin [1]. It uses internal helpers (such as shouldBypassCPUSetAdjustment and applyCPUSetBypass) to clear the cpuset string in the AllocationResult while preserving other critical information like TopologyAssignments, AllocatedQuantity, annotations, and hints [1]. - Purpose: This mechanism provides more flexibility in how CPU isolation is managed at runtime, enabling the reconcile loops to handle cpuset updates more effectively without conflicts from the initial allocation fast-path [1]. Katalyst is a Kubernetes-based resource management system that implements these distinct QoS levels (dedicated, shared, reclaimed, and system) to improve workload colocation and resource efficiency [2][3]. The EnableBypassCPUSetAdjustment switch is disabled by default [1].
Citations:
- 1: feat(qrm/cpu): add EnableBypassCPUSetAdjustment to skip cpuset backfill for shared/reclaimed/system pools katalyst-core#1200
- 2: https://kubewharf.io/blog/2023/12/06/katalyst-a-qos-based-resource-management-system-for-workload-colocation-on-kubernetes/
- 3: https://gokatalyst.io/docs/overview/core-concepts/
Align the enableBypassCPUSetAdjustment description with the actual scope. It should say this skips cpuset population during PackAllocationResponse for shared_cores, reclaimed_cores, and system_cores; dedicated pools are unaffected. Update the Go field comment and regenerate the CRD.
🤖 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 `@config/crd/bases/config.katalyst.kubewharf.io_adminqosconfigurations.yaml`
around lines 820 - 825, Update the Go field comment for
enableBypassCPUSetAdjustment to state that PackAllocationResponse skips cpuset
population for shared_cores, reclaimed_cores, and system_cores, while dedicated
pools remain unaffected; then regenerate the CRD so its description matches.
1 participant
What type of PR is this?
/kind feature
What this PR does / why we need it
Add a new boolean field
EnableBypassCPUSetAdjustmenttoCPUPluginConfiginAdminQoSConfigurationto allow bypassing cpuset backfill in QRM CPU plugin'sPackAllocationResponseforshared_cores,reclaimed_coresandsystem_corespools.When enabled, QRM will not populate cpuset for those pools during allocation, and cpuset adjustment is delegated to the reconcile plugin instead. Dedicated pools are unaffected by this flag.
This enables a cleaner separation between allocation-time decisions (kept in QRM) and runtime cpuset reconciliation (moved to the reconcile plugin), which is required by the follow-up work in katalyst-core.
Changes
EnableBypassCPUSetAdjustment *boolfield toCPUPluginConfig(pkg/apis/config/v1alpha1/adminqos.go).zz_generated.deepcopy.gofor the new pointer field.config/crd/bases/config.katalyst.kubewharf.io_adminqosconfigurations.yamlaccordingly.Which issue(s) this PR fixes
None.
Special notes for your reviewer
+optionaland defaults to unset (equivalent tofalse), so existing behavior is preserved.Summary by CodeRabbit
GetResourcesAllocationand pod allocation treated consistently when enabled.GetResourcesAllocationresults andAllocate/AllocateForPodresponses when the flag is turned on.