Repository navigation
feat(tenants): add dedicatedCPU quota sugar for pinned-CPU workloads - #2435
Andrei Kvapil (kvaps) wants to merge 1 commit into
Conversation
Introduce an optional `dedicatedCPU` key in the cozy-lib resource sanitizer
(and therefore in tenant `resourceQuotas`). Its value is added 1:1 to both
`limits.cpu` and `requests.cpu`, bypassing the `cpuAllocationRatio` divisor.
This fixes the quota mismatch between regular VMs (where KubeVirt divides pod
`requests.cpu` by `cpuAllocationRatio`) and instance types with
`dedicatedCPUPlacement` (where pod `requests == limits`): previously, a single
dedicated-CPU VM could exhaust the tenant's `requests.cpu` quota while the
`limits.cpu` quota stayed largely unused, effectively shrinking the usable
tenant capacity for CX-series workloads to `cpu / cpuAllocationRatio`.
With the new key, operators can declare dedicated capacity explicitly:
resourceQuotas:
cpu: 10 # oversubscribable (burstable) CPU budget
dedicatedCPU: 2 # plus 2 cores of dedicated physical CPU
This yields `limits.cpu=12, requests.cpu=3` at the default ratio of 10, so
both sides of the ResourceQuota stay in lockstep regardless of which
instance-type series a workload uses.
Co-Authored-By: Claude <[email protected]>
Signed-off-by: Andrei Kvapil <[email protected]>
📝 WalkthroughWalkthroughThe changes introduce support for a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Andrei Kvapil (kvaps)
added
the
kind/backport
dosubot
Bot
added
the
size/M
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances tenant resource management by providing a mechanism to accurately account for CPU-pinned workloads. By allowing operators to specify dedicated CPU capacity separately from oversubscribable CPU, the system ensures that resource quotas remain balanced for Guaranteed-QoS pods, preventing premature exhaustion of tenant capacity. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
dosubot
Bot
added
the
kind/feature
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for dedicatedCPU in resource quotas, allowing for workloads that require pinned physical cores. The cozy-lib library's sanitize function was updated to add the dedicatedCPU value to both CPU limits and requests at a 1:1 ratio, ensuring these resources are not oversubscribed. Documentation and examples were added to the tenant README, and new test cases were included to verify the calculation logic. All review comments were filtered out as they primarily validated the implementation or provided praise without suggesting actionable improvements; therefore, I have no feedback to provide.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/apps/tenant/README.md (1)
95-139:⚠️ Potential issue | 🟡 MinorMove resourceQuotas documentation to
values.yamland regenerate README.The README is generated from
values.yamlby thecozyvalues-gentool (see Makefile:cozyvalues-gen ... -r README.md). Manual edits to README.md will be overwritten whenmake generateruns.Add the detailed
cpu,dedicatedCPU,memory,ephemeral-storage,storage, and quota documentation as comments in theresourceQuotasparameter section ofvalues.yaml, then runmake generateto regenerate the README.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/apps/tenant/README.md` around lines 95 - 139, The detailed resourceQuotas documentation in README.md needs to be moved into values.yaml and the README regenerated: copy the explanations for resourceQuotas (including cpu, dedicatedCPU, memory, ephemeral-storage, storage, and the object count quotas like pods, services.loadbalancers, etc.) as commented docs under the resourceQuotas parameter in values.yaml, ensure the comment formatting matches other parameter comments so cozyvalues-gen will pick it up, then run the documented generation step (cozyvalues-gen via the Makefile target — run make generate or the exact cozyvalues-gen ... -r README.md command) to regenerate README.md so it is not overwritten by future runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/apps/tenant/README.md`:
- Around line 95-139: The detailed resourceQuotas documentation in README.md
needs to be moved into values.yaml and the README regenerated: copy the
explanations for resourceQuotas (including cpu, dedicatedCPU, memory,
ephemeral-storage, storage, and the object count quotas like pods,
services.loadbalancers, etc.) as commented docs under the resourceQuotas
parameter in values.yaml, ensure the comment formatting matches other parameter
comments so cozyvalues-gen will pick it up, then run the documented generation
step (cozyvalues-gen via the Makefile target — run make generate or the exact
cozyvalues-gen ... -r README.md command) to regenerate README.md so it is not
overwritten by future runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bf50111c-3596-4722-ba8e-ec5940238736
📒 Files selected for processing (5)
packages/apps/tenant/README.mdpackages/library/cozy-lib/templates/_resources.tplpackages/tests/cozy-lib-tests/tests/quota_test.yamlpackages/tests/cozy-lib-tests/tests/quota_values_combined.yamlpackages/tests/cozy-lib-tests/tests/quota_values_dedicated.yaml
|
Thanks for the careful writeup, but I'd like to close this PR without merging. Reasoning below, roughly in order of weight. The premise that this is a bug is debatable. The motivating complaint — "my The PR doesn't actually make limits/requests "stay in lockstep." It just shifts where the mismatch breaks. With
It leaks into every app that uses Sugar is lossy. The rendered API hygiene. Where I'd rather put the effort. The real user pain — "I don't understand why my requests quota is full while limits isn't" — is a presentation problem. I'd like to address it at the API/UI surface that shows a tenant their quota: render remaining capacity in two complementary views, e.g. "≈10 vCPU of burstable workload remaining, or ≈1 physical core of dedicated workload remaining." That resolves the confusion without committing the quota model to a new first-class concept. Happy to open an issue to scope that out. |
github-actions
Bot
added
the
do-not-merge/work-in-progress
3 participants
What this PR does
Adds an optional
dedicatedCPUkey to tenantresourceQuotas(via the cozy-lib resource sanitizer). Its value is added 1:1 to bothlimits.cpuandrequests.cpu, bypassing thecpuAllocationRatiodivisor.Problem
The library chart divides tenant
requests.cpubycpuAllocationRatio(default: 10), which matches how KubeVirt sizesvirt-launcherpods for regular VMs. Instance types withdedicatedCPUPlacement(thecx*series, and other Guaranteed-QoS workloads) do not follow that ratio: their pods haverequests.cpu == limits.cpu. As a result, a single dedicated-CPU VM exhausts the tenant'srequests.cpuquota N times faster than itslimits.cpuquota, shrinking the usable tenant capacity for CX-series workloads tocpu / cpuAllocationRatio.Solution
Operators can now declare dedicated capacity separately:
At the default ratio of 10 this produces
limits.cpu=12, requests.cpu=3— both sides of theResourceQuotastay in lockstep regardless of which instance-type series a workload uses.Existing tenant configurations (
cpuonly) are unaffected: the sanitizer output is byte-identical whendedicatedCPUis absent.Release note
Summary by CodeRabbit
New Features
dedicatedCPUresource quota allocation, which applies directly to both CPU limits and requests without ratio calculations. Can be combined with standardcpuallocation.Documentation
dedicatedCPUusage and combined configurations.Tests
dedicatedCPUquota functionality, including standalone and combined allocation scenarios.