feat(worker): decouple sliced compute and memory budgets#36
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR relaxes GPUStack worker admission rules for sliced accelerators so that per-card compute (SM) and memory (VRAM) percentages are treated as independent budgets, removing the prior constraint that compute must be ≥ memory.
Changes:
- Dropped the
cores% < memory%rejection from both Instance admission validation and downstream Pod admission validation. - Updated webhook unit tests to assert
cores% < memory%is now allowed. - Updated API/docs commentary and regenerated derived artifacts (OpenAPI/CRD/proto/applyconfiguration) to reflect the relaxed contract.
Reviewed changes
Copilot reviewed 6 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/worker/webhooks/worker/pod.go | Removes Pod webhook validation that rejected cores% < memory% for sliced requests; updates comments accordingly. |
| pkg/worker/webhooks/worker/pod_test.go | Updates Pod webhook validation test case to expect cores% < memory% to be allowed. |
| pkg/worker/webhooks/worker/instance.go | Removes Instance webhook validation that rejected cores% < memory%; updates comment to state independence. |
| pkg/worker/webhooks/worker/instance_test.go | Updates Instance webhook sliced-percentage contract test to allow cores% < memory%. |
| pkg/kubeclients/applyconfiguration/worker/v1alpha1/instanceresources.go | Updates applyconfiguration field doc for sliced cores percentage to reflect independence. |
| docs/architecture.md | Updates Pod admission webhook rejection list to remove the cores% < memory% rule. |
| api/worker/zz_generated.openapi.go | Regenerates OpenAPI description for sliced cores percentage to reflect independence. |
| api/worker/v1alpha1/zz_generated.crds.go | Regenerates CRD schema descriptions to reflect independence for sliced cores percentage. |
| api/worker/v1alpha1/instance.go | Updates API type field comment for sliced cores percentage to reflect independence. |
| api/worker/v1alpha1/generated.proto | Regenerates proto comments to reflect independence for sliced cores percentage. |
Files not reviewed (4)
- api/worker/v1alpha1/generated.proto: Generated file
- api/worker/v1alpha1/zz_generated.crds.go: Generated file
- api/worker/zz_generated.openapi.go: Generated file
- pkg/kubeclients/applyconfiguration/worker/v1alpha1/instanceresources.go: Generated file
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 11 changed files in this pull request and generated 1 comment.
Files not reviewed (5)
- api/worker/v1alpha1/generated.proto: Generated file
- api/worker/v1alpha1/zz_generated.crds.go: Generated file
- api/worker/zz_generated.openapi.go: Generated file
- pkg/kubeclients/applyconfiguration/worker/v1alpha1/instanceresources.go: Generated file
- pkg/kubeclients/applyconfiguration/worker/v1alpha1/instancetypeunitresources.go: Generated file
thxCode
force-pushed
the
feat/sliced-cores-memory-independent
branch
from
6368713 to
ca48c25
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 11 changed files in this pull request and generated 1 comment.
Files not reviewed (5)
- api/worker/v1alpha1/generated.proto: Generated file
- api/worker/v1alpha1/zz_generated.crds.go: Generated file
- api/worker/zz_generated.openapi.go: Generated file
- pkg/kubeclients/applyconfiguration/worker/v1alpha1/instanceresources.go: Generated file
- pkg/kubeclients/applyconfiguration/worker/v1alpha1/instancetypeunitresources.go: Generated file
thxCode
force-pushed
the
feat/sliced-cores-memory-independent
branch
from
ca48c25 to
c07c4fe
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 11 changed files in this pull request and generated 1 comment.
Files not reviewed (5)
- api/worker/v1alpha1/generated.proto: Generated file
- api/worker/v1alpha1/zz_generated.crds.go: Generated file
- api/worker/zz_generated.openapi.go: Generated file
- pkg/kubeclients/applyconfiguration/worker/v1alpha1/instanceresources.go: Generated file
- pkg/kubeclients/applyconfiguration/worker/v1alpha1/instancetypeunitresources.go: Generated file
Remove the validation that required acceleratorSlicedCoresPercentage to be greater than or equal to acceleratorSlicedMemoryPercentage. The per-card compute (SM) budget and the VRAM budget of a slice are independent, so a sliced request may set any combination of the two percentages in [0,100]. Drop the cores>=memory check from both admission points that enforced it: the Instance webhook (validateResourceRequests) and the downstream Pod webhook (validateSlicedContainer). Update the AcceleratorSlicedCoresPercentage field doc and regenerate the derived API code, and align docs/architecture.md. Signed-off-by: thxCode <thxcode0824@gmail.com>
Regenerate InstanceTypeUnitResources applyconfiguration so its unit CPU doc comment reads "cores", matching the source type comment that c0d4a01 changed from "milli-cores" to "cores" but did not regenerate. Pure generated-code sync; no behavior change. Signed-off-by: thxCode <thxcode0824@gmail.com>
thxCode
force-pushed
the
feat/sliced-cores-memory-independent
branch
from
c07c4fe to
f36c691
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 11 changed files in this pull request and generated 2 comments.
Files not reviewed (5)
- api/worker/v1alpha1/generated.proto: Generated file
- api/worker/v1alpha1/zz_generated.crds.go: Generated file
- api/worker/zz_generated.openapi.go: Generated file
- pkg/kubeclients/applyconfiguration/worker/v1alpha1/instanceresources.go: Generated file
- pkg/kubeclients/applyconfiguration/worker/v1alpha1/instancetypeunitresources.go: Generated file
| **Three-view status.** `InstanceType.status` carries three per-card bin-packing projections computed from the `Devices` ledger (not a credits fold-down): `Accelerator` = free whole cards; `AcceleratorShared` = shareable ownership slots; `AcceleratorSliced` = sliceable VRAM-percent units. Because the reconciler watches the `Devices` CR and writes the result into a real CRD's `.status`, a `kubectl get instancetype -w` observes capacity move as pods allocate and free — a read-only projection over the ClusterQueue could not (it borrows the CQ `resourceVersion`, unchanged on a `Devices`-only allocation). The v1 (`worker.gpustack.ai/v1`) InstanceType is a thin proxy + conversion over the real `v1alpha1` CRD; the unit spec lives **only** on the InstanceType — a derived type is stamped with the fixed default at creation, and an admin edit changes only the InstanceType (never a Node or the ClusterQueue notes). The InstanceType validating webhook **requires** the complete input on create — `acceleratorGroup` (only when `acceleratable`) / `os` / `arch` plus the unit triple (`unitResources.cpu`/`.ram` + `localStorage`; an empty or partial spec is rejected), read independently of any editable setting, and a **CPU-only (`acceleratable=false`) type's unit CPU must be exactly 1 core** (an accelerated type keeps any unitless positive integer) — and **freezes the spec on update**: every field is **immutable except `displayName` (rename) and `inactive` (take in/out of service)**, so to re-size or re-point a pool you delete and re-create the type. Create-time shape checks are not re-run on update (only immutability is), so a legacy type stored before a tightened rule can still be renamed or deactivated. The defaulting webhook defaults an empty `generalGroup` to the `generic` sentinel, stamps the pool's schedule labels (`nodefeature.PoolScheduleLabels`, grouped by `instance-type-aware-cpu-manufacturer`) and the `schedule.gpustack.ai/queue-entrance` label (`nodefeature.FormatLocalQueueName(name)`), enriches the descriptors from a matching ResourceFlavor, and — when awareness is on — folds that flavor's `cpuDetail` note into `spec.cpu` (generic) or `spec.accelerator.cpu` (accelerated). The Instance validating webhook enforces the unit spec on **Create and Update**: a submission's RAM must not exceed `unitRAM × count` and its local storage must not exceed the InstanceType's `LocalStorage`. The Pod webhook reads the **InstanceType** for a slice's per-card VRAM divisor — reverse-looked-up by the `schedule.gpustack.ai/queue-entrance` label to `spec.memory` (Stage 4, Pod admission webhook) — never the user-writable LocalQueue. | ||
|
|
||
| **Pod admission webhook.** A `pods` CREATE webhook (objectSelector on `kueue.x-k8s.io/queue-name`, `failurePolicy: Fail`) mutates a sliced request — defaulting `.sliced.cores-percentage` to 100 and folding `.sliced.memory-percentage` / `.sliced.memory-mib` into `.sliced.units` (only when units are absent; memory-percentage wins) — and validates it (rejecting no-memory / both-memory-keys / non-positive `.sliced.*` / `cores-% < memory-%` / mixed modes). The per-card VRAM divisor for the fold is read from the operator-owned **InstanceType**'s `spec.memory` (reverse-looked-up by the `schedule.gpustack.ai/queue-entrance` label), never from the user-writable LocalQueue. Its `MutatingWebhookConfiguration` name sorts before `kueue-mutating-webhook-configuration` so the fold runs before Kueue hashes container resources. | ||
| **Pod admission webhook.** A `pods` CREATE webhook (objectSelector on `kueue.x-k8s.io/queue-name`, `failurePolicy: Fail`) mutates a sliced request — defaulting `.sliced.cores-percentage` to 100 and folding `.sliced.memory-percentage` / `.sliced.memory-mib` into `.sliced.units` (only when units are absent; memory-percentage wins) — and validates it (rejecting no-memory / both-memory-keys / non-positive `.sliced.*` / mixed modes). The per-card VRAM divisor for the fold is read from the operator-owned **InstanceType**'s `spec.memory` (reverse-looked-up by the `schedule.gpustack.ai/queue-entrance` label), never from the user-writable LocalQueue. Its `MutatingWebhookConfiguration` name sorts before `kueue-mutating-webhook-configuration` so the fold runs before Kueue hashes container resources. |
Comment on lines
+10
to
11
| // CPU is the unit CPU resource(cores) of the InstanceType. | ||
| CPU *string `json:"cpu,omitempty"` |
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
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.
What type of PR is this?
/kind enhancement
/area worker
What this PR does / why we need it:
Removes the admission-time constraint that a sliced accelerator request's compute budget (
acceleratorSlicedCoresPercentage) must be greater than or equal to its memory budget (acceleratorSlicedMemoryPercentage).A slice's per-card compute (SM) and VRAM budgets are physically independent — the device-plugin already caps SM and VRAM separately (
CUDA_DEVICE_SM_LIMITvsCUDA_DEVICE_MEMORY_LIMIT_*; Ascendaicore-quotavsmemory-quota) — so this ordering rule was an unnecessary restriction that blocked valid configurations such as a memory-heavy, compute-light slice.The check was enforced at two admission points; both are dropped so the relaxation takes effect end-to-end:
InstanceWebhook.validateResourceRequests— theInstanceCR create/start path.PodWebhook.validateSlicedContainer— the downstream Pod the Instance controller materializes (the real runtime enforcement point). Dropping only the Instance webhook would leave the Pod webhook still rejecting the request, so the change would not have worked.Unchanged: the
[0,100]range checks, the "copy one percentage to the other when only one is set" defaulting, and the "accelerator count must be exactly 1 for a sliced request" rule.Which issue(s) this PR fixes:
None.
Special notes for your reviewer:
AcceleratorSlicedCoresPercentagefield doc updated and derived code regenerated viamake generate(openapi / CRD / proto / applyconfiguration).docs/architecture.mdPod-webhook rejection list updated to dropcores-% < memory-%.specs/2026-06-29-instancetype-unified-pool-refactor.mdstill records the original constraint; left as-is as a historical design record (a later relaxation belongs in a new record, not a retroactive edit).chore(api): sync applyconfiguration unit CPU comment to cores) regenerates one stale generated file whose source comment was changed inc0d4a01but not regenerated then. Pure generated-code sync, unrelated to the constraint change — reviewable on its own.make lint(0 issues) andmake test(full suite green). The webhook unit tests that assertedcores < memoryis rejected were flipped to assert it is now allowed.Does this PR introduce a user-facing change?