fix(cozy-lib): round CPU request to whole milliCPU by IvanHunters · Pull Request #2920 · cozystack/cozystack

IvanHunters · 2026-06-15T09:31:59Z

What this PR does

cozy-lib.resources.sanitize divided the CPU limit by cpuAllocationRatio without rounding to an integer milliCPU, producing values like 0.035714286 (kube-apiserver representation: 35714286n) when cpuAllocationRatio did not evenly divide the input.

kube-apiserver tolerates sub-milliCPU on pod requests, but the VPA admission webhook rejects MinAllowed/MaxAllowed that are not whole milliCPU. As a result, charts that feed the sanitized output into VPA bounds (the etcd chart is currently the only one in-tree) failed to install for any cpuAllocationRatio that is not a divisor of 250 (e.g. 3, 6, 7, 8, 9):

Helm install failed for release <namespace>/etcd with chart etcd@0.0.0+...:
  admission webhook "vpa.k8s.io" denied the request:
  MinAllowed: CPU [35714286n] must be a whole number of milli CPUs

This PR rounds the milliCPU value to the nearest whole milliCPU in _resources.tpl before formatting, and emits the value with the m suffix (200m instead of 0.2) which is semantically identical for kube-apiserver.

Verification

helm template etcd packages/extra/etcd --set _cluster.cpu-allocation-ratio=7 --show-only templates/vpa.yaml:
- minAllowed.cpu: 36m (was rejected as 35714286n)
- maxAllowed.cpu: 714m (was rejected as 714285714n)
helm template etcd packages/extra/etcd --set _cluster.cpu-allocation-ratio=3 --show-only templates/vpa.yaml:
- minAllowed.cpu: 83m, maxAllowed.cpu: 1667m
helm unittest passes in all packages that exercise sanitize (cozy-lib-tests, backupstrategy-controller, cilium-networkpolicy, ouroboros, harbor, linstor-scheduler, platform).
New regression test under packages/tests/cozy-lib-tests/tests/resources_test.yaml covers default ratio (10), non-divisor ratio (7), upper-bound rounding, and whole-core inputs.

The format change from 0.025 to 25m flows through to every Pod template that uses sanitize. Both forms are accepted by kube-apiserver; existing HelmReleases will see a one-time diff on the requests.cpu field.

Release note

fix(cozy-lib): CPU requests produced by `cozy-lib.resources.sanitize` are now rounded to whole milliCPU. This unblocks `cpuAllocationRatio` values that are not divisors of 250 (e.g. 3, 6, 7, 8, 9), which previously caused the etcd HelmRelease to fail with VPA admission webhook errors like `MinAllowed: CPU [...n] must be a whole number of milli CPUs`. Pod `requests.cpu` is now emitted in milliCPU form (e.g. `200m` instead of `0.2`) — semantically identical for kube-apiserver.

Closes #2917

Summary by CodeRabbit

Bug Fixes
- CPU resource requests are now rounded to whole milliCPU values for improved Kubernetes and VPA compatibility.
- CPU limits retain their original formatting while requests use consistent milliCPU notation.
- CPU quota expectations now correctly use milliCPU formatting.
Tests
- Added coverage for rounding behavior across allocation ratios, upper-bound inputs, and whole-core CPU values.

The cozy-lib.resources.sanitize helper divided the CPU limit by cpuAllocationRatio without rounding to an integer milliCPU, producing values like 0.0357142... when cpuAllocationRatio did not evenly divide the input. kube-apiserver tolerates sub-milliCPU on pod requests, but the VPA admission webhook rejects MinAllowed and MaxAllowed when they are not whole milliCPU, so any chart that feeds the sanitized output into VPA bounds failed to install. The etcd chart was the only place feeding sanitize output into VPA bounds in the tree, and its HelmRelease failed for any cpuAllocationRatio that is not a divisor of 250 (e.g. 7, 3, 6, 8, 9). Round the milliCPU value to the nearest whole milliCPU before formatting. Pod resources.requests.cpu is now emitted in milliCPU form (e.g. 200m instead of 0.2) which is semantically identical. Closes #2917 Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>

coderabbitai · 2026-06-15T09:32:07Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4f8a6e1f-8a99-40b7-bbe9-c2304ef1b7dd

📥 Commits

Reviewing files that changed from the base of the PR and between 42ca3e6 and e494f16.

📒 Files selected for processing (4)

packages/library/cozy-lib/templates/_resources.tpl
packages/tests/cozy-lib-tests/templates/tests/resources.yaml
packages/tests/cozy-lib-tests/tests/quota_test.yaml
packages/tests/cozy-lib-tests/tests/resources_test.yaml

📝 Walkthrough

Walkthrough

The resource sanitizer now rounds allocation-adjusted CPU requests to whole milliCPU quantities. New Helm tests cover rounding across ratios and input formats, while the quota expectation uses milliCPU notation.

Changes

CPU request formatting

Layer / File(s)	Summary
Sanitizer rounding and formatting `packages/library/cozy-lib/templates/_resources.tpl`	CPU requests are rounded to integer milliCPU values and emitted as `"%dm"` quantities.
Resource formatting validation `packages/tests/cozy-lib-tests/templates/tests/resources.yaml`, `packages/tests/cozy-lib-tests/tests/resources_test.yaml`, `packages/tests/cozy-lib-tests/tests/quota_test.yaml`	Tests cover allocation ratios, CPU input formats, preserved limits, and the updated `"2000m"` quota value.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

cozystack/cozystack#1155: Changes the CPU request calculation logic used by the same sanitizer helper.
cozystack/cozystack#1372: Consumes sanitized CPU requests in flattened quota values.

Suggested labels: area/testing

Suggested reviewers: lllamnyp, kvaps, androndo

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: rounding cozy-lib CPU requests to whole milliCPU.
Linked Issues check	✅ Passed	The change addresses `#2917` by rounding CPU values to whole milliCPU and adding tests for default, non-divisor, and whole-core cases.
Out of Scope Changes check	✅ Passed	The added template and test updates appear directly related to validating the CPU rounding fix and do not introduce unrelated scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/cozy-lib-resources-cpu-integer-millicpu

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Checkov (3.3.8)

packages/tests/cozy-lib-tests/templates/tests/resources.yaml

Traceback (most recent call last):
File "/usr/local/bin/checkov", line 2, in
from checkov.main import Checkov
ModuleNotFoundError: No module named 'checkov'

packages/tests/cozy-lib-tests/tests/quota_test.yaml

Traceback (most recent call last):
File "/usr/local/bin/checkov", line 2, in
from checkov.main import Checkov
ModuleNotFoundError: No module named 'checkov'

packages/tests/cozy-lib-tests/tests/resources_test.yaml

Traceback (most recent call last):
File "/usr/local/bin/checkov", line 2, in
from checkov.main import Checkov
ModuleNotFoundError: No module named 'checkov'

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.

❤️ Share

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

Aleksei Sviridkin (lexfrei)

LGTM — approving.

Business context: cozy-lib.resources.sanitize is a shared Helm template that derives requests from limits using the cluster CPU-allocation ratio, and is consumed by every managed app. When the ratio did not evenly divide the input, the CPU request came out as a sub-milliCPU float; the etcd chart — the only place that feeds sanitize output into VPA minAllowed/maxAllowed — then failed admission because VPA requires whole milliCPU. This change rounds the CPU request to whole milliCPU and emits the m form.

I verified the fix: with ratio 7, 250m→36m and 5000m→714m (whole milliCPU, accepted by VPA), and helm unittest passes 5/5. No consumer performs arithmetic on the sanitized CPU request that would break on the new m suffix — the kubernetes chart's reservation math reads the raw limit and its helper already strips m, and cozy-lib.resources.toFloat handles m elsewhere.

No blockers.

Non-blocking:

CPU request can exceed the limit for sub-milliCPU inputs at a low ratio. Round-half-up is applied only to the request while the limit is emitted unchanged, so the request can be pushed above the limit, which kube-apiserver rejects. Evidence: packages/library/cozy-lib/templates/_resources.tpl:107-108; rendering resources.cpu=0.0006 with cpu-allocation-ratio=1 yields requests.cpu: 1m against limits.cpu: "0.0006", and resources.cpu=1.5m yields requests.cpu: 2m against limits.cpu: 1.5m — both request > limit. No in-tree chart uses fractional-milliCPU CPU values, but values.schema.json permits them and this is a shared library, so consider clamping the request to the integer-milliCPU limit (or rounding the limit the same way) so request <= limit always holds, with a regression case to match.
Sub-0.5m requests collapse to 0m. The same rounding applied to tiny values drops the CPU request floor entirely. Evidence: rendering resources.cpu=0.4m with cpu-allocation-ratio=1 yields requests.cpu: 0m. This is valid for kube-apiserver but removes the guaranteed CPU share, and is only reachable with sub-milliCPU input.
One-time requests.cpu diff across every app using sanitize. The format change from e.g. 0.2 to 200m rewrites requests.cpu on every existing HelmRelease, so pods may roll on the next reconcile even though the value is semantically identical. This is already called out in the PR description; noting it for upgrade awareness. Evidence: packages/library/cozy-lib/templates/_resources.tpl:108, where printf "%dm" replaces the prior float toString.

gemini-code-assist · 2026-07-22T17:58:34Z

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

myasnikovdaniil · 2026-08-15T16:28:33Z

Closing this in favour of #3522, and the reason is arithmetic rather than preference, so it is worth stating.

Both PRs fix a real bug and both ship tests that genuinely fail against main. The tiebreak is which way to round, and the answer is that kube-apiserver already rounds this value up. I checked it server-side: a pod requesting 0.0833333333 is stored as 84m. Round-half-up gives 83m, and across 70 inputs where the two modes disagree it matched the cluster in none of them. So half-up would quietly lower every inexact CPU request by 1m fleet-wide, and can render 0m.

Your approach does have one advantage over the one being taken, and it should not go unsaid: it cannot emit a value VPA rejects, by construction. #3522 as written still can, in 53 cases out of a 905k-combination sweep, and it needs a one-line change to close that. It is being asked for there.

The diagnosis in #2917 was right and this sat too long. Thank you for it.

github-actions Bot added size/L This PR changes 100-499 lines, ignoring generated files area/platform Issues or PRs related to platform infrastructure (bundle, flux, talos, installer) kind/bug Categorizes issue or PR as related to a bug labels Jun 15, 2026

Aleksei Sviridkin (lexfrei) approved these changes Jun 24, 2026

View reviewed changes

IvanHunters marked this pull request as ready for review July 22, 2026 17:58

IvanHunters requested review from Andrey Kolkov (androndo), Andrei Kvapil (kvaps), Timofei Larkin (lllamnyp), myasnikovdaniil and Kirill Ilin (sircthulhu) as code owners July 22, 2026 17:58

dosubot Bot mentioned this pull request Aug 3, 2026

VPA minAllowed is not rounded to whole milli-CPUs, so a non-default cpuAllocationRatio breaks the etcd app (and blocks CDI in 1.6.0) #3517

Closed

myasnikovdaniil closed this Aug 15, 2026

This was referenced Aug 15, 2026

fix(cozy-lib): round the sanitized CPU request up to a whole milli-CPU #3522

Open

bug(etcd): VPA creation fails with non-integer milliCPU in minAllowed/maxAllowed when cpuAllocationRatio is not a divisor of 250 #2917

Open

Navigation Menu

Uh oh!

fix(cozy-lib): round CPU request to whole milliCPU - #2920

fix(cozy-lib): round CPU request to whole milliCPU#2920
IvanHunters wants to merge 1 commit into
mainfrom
fix/cozy-lib-resources-cpu-integer-millicpu

IvanHunters commented Jun 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 15, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

Aleksei Sviridkin (lexfrei) left a comment

Uh oh!

gemini-code-assist Bot commented Jul 22, 2026

Uh oh!

myasnikovdaniil commented Aug 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

IvanHunters commented Jun 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Verification

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

Aleksei Sviridkin (lexfrei) left a comment

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot commented Jul 22, 2026

Uh oh!

myasnikovdaniil commented Aug 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

IvanHunters commented Jun 15, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 15, 2026 •

edited

Loading