feat(quota): Kubernetes quota-enforcement — M1 (tenancy + hard quotas + scoped kubeconfig) - #475
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #475 +/- ##
==========================================
- Coverage 72.94% 72.83% -0.11%
==========================================
Files 157 160 +3
Lines 53701 54299 +598
==========================================
+ Hits 39171 39547 +376
- Misses 14530 14752 +222 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR implements M1 of Kubernetes quota-enforcement in Spur by persisting account-level resource allocations (grp_tres), projecting accounts into Kubernetes tenancy/quota/RBAC objects via an operator reconciler, and adding a kubeconfig --user flow that mints namespace-scoped ServiceAccount tokens via the control-plane agent.
Changes:
- Extend proto + controller/agent RPCs to support per-user (scoped) kubeconfig minting, while keeping the admin kubeconfig path.
- Persist
grp_treson accounts end-to-end (CLI → gRPC → DB → list/read back) to drive per-account ResourceQuota projection. - Add quota projection + reconciler logic in
spur-k8s, and shared DNS-safe naming inspur-coreso reconciler and kubeconfig minting agree.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| proto/slurm.proto | Adds request fields for scoped kubeconfig minting and grp_tres on accounts. |
| crates/spurd/src/cluster.rs | Implements SA creation + token mint + kubeconfig templating on control-plane node. |
| crates/spurd/src/agent_server.rs | Extends get_admin_kubeconfig RPC to optionally mint scoped kubeconfigs. |
| crates/spurctld/src/server.rs | Adds ClusterKubeconfigRequest.user handling and fail-closed association-cache gating. |
| crates/spurctld/src/cluster_k8s.rs | Adds client helper to request scoped kubeconfig from control-plane agent. |
| crates/spurctld/src/accounting/grpc.rs | Wires grp_tres through CreateAccount/ListAccounts gRPC. |
| crates/spurctld/src/accounting/db.rs | Adds grp_tres column + upsert/list plumbing and a DB round-trip test. |
| crates/spur-k8s/src/quota.rs | Pure mapping from account allocation/users → Namespace/ResourceQuota/LimitRange/RBAC. |
| crates/spur-k8s/src/quota_controller.rs | Implements reconciler loop + SSA apply with force and fail-closed grp_tres parsing. |
| crates/spur-k8s/src/main.rs | Adds --enable-quota flag and spawns quota reconciler when enabled. |
| crates/spur-k8s/src/lib.rs | Exposes quota modules. |
| crates/spur-core/src/quota_names.rs | Shared DNS-safe naming for account namespaces and per-user ServiceAccounts. |
| crates/spur-core/src/lib.rs | Exports the new quota_names module. |
| crates/spur-cli/src/sacctmgr.rs | Adds grptres=... support for account add/modify. |
| crates/spur-cli/src/k8s.rs | Adds spur k8s kubeconfig --user <name> option to request scoped kubeconfig. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
crates/spurd/src/agent_server.rs:1752
- When
req.useris set butnamespace/service_accountare empty, the call correctly fails inuser_kubeconfig, but the RPC maps it toStatus::internal. This is a client-input error; returninginvalid_argumenthere makes the contract clearer and avoids treating caller bugs as server faults.
let result = if req.user.is_empty() {
self.k0s.admin_kubeconfig().await
} else {
self.k0s
.user_kubeconfig(&req.user, &req.namespace, &req.service_account)
shiv-tyagi
left a comment
There was a problem hiding this comment.
Verified core (non-K8s) spur is not broken: build is green and 394 tests pass on the PR head. Proto fields are append-only, the grp_tres migration is idempotent and runs at startup before any read, create_account validation accepts empty, and both kubeconfig paths preserve the exact old behavior for an empty user. The quota reconciler is opt-in behind --enable-quota.
Four comments below: one RBAC/security question on secrets, an RPC-naming clarity point, a missing test on the scoped-kubeconfig resolution, and a unit-consistency nit. None are blocking.
c8fc654 to
c3bd575
Compare
|
Rebased onto latest One commit from this branch ( Two small follow-up commits were needed to make the rebase build clean:
Build, clippy, and the |
…a/LimitRange/RBAC) Pure, unit-tested mapping from a SPUR account allocation (grp_tres) to the native k8s objects that enforce it. 6 tests green. Foundation for the quota policy reconciler; DB/proto persistence + controller + `kubeconfig --user` follow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Makes the quota layer live: an opt-in reconcile loop that projects every SPUR account into its native k8s objects and drift-corrects them. - quota_controller::run: connects a SlurmAccountingClient to spurctld, then every 30s lists accounts (with grp_tres) + members and server-side-applies (force, so an admin hand-edit is reverted) the Namespace + ResourceQuota + LimitRange + Role + RoleBinding from crate::quota. - build_account_quota: pure AccountInfo + members -> AccountQuota (unit-tested). - apply<K>: generic SSA helper (injects apiVersion/kind, which k8s-openapi types don't serialize but server-side apply requires). - Wired into main.rs behind --enable-quota (default off; opt-in policy plane), spawned with the same run_with_retry backoff as the other controllers. Verified: build + spur-k8s tests (146) + fmt + clippy -D warnings. The apply/RPC path is integration-level (needs a cluster); the mapping + build logic are unit-tested. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…onfig (M1 increment 4) A SPUR user gets a namespace-scoped credential into their account's namespace, without an admin ever handing out the cluster-admin kubeconfig. - spur-core::quota_names: the per-account namespace + per-user ServiceAccount naming, shared so the operator (which creates them) and spurctld (which mints the kubeconfig) agree. Moved out of spur-k8s::quota. - proto: ClusterKubeconfigRequest.user; GetAdminKubeconfigRequest gains user/namespace/service_account (reuses the existing RPC — no new trait method). - spurd: K0sAgent::user_kubeconfig ensures the SA exists + mints a bound token via `k0s kubectl create token`, then templates a scoped kubeconfig from the admin cluster CA/server (pure parse + template helpers, unit-tested). - spurctld: cluster_kubeconfig resolves the user's account (association cache) -> namespace + SA and forwards to the control-plane agent. - cli: `spur k8s kubeconfig --user <u>`. Verified (no cluster run, per request): build + tests + fmt + clippy -D warnings across all crates incl. spurd on Linux (shark-a). The SA/token minting is integration-level; the naming, CA/server parse, and kubeconfig template are unit-tested. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…om review - quota_hard: TRES mem is base-10 MB; emit `M` not `Mi` (Mi over-allocated ~4.86%). - quota_names: include the `spur-acct-`/`spur-user-` prefix in the 63-char DNS cap so long account/user names produce a valid namespace/ServiceAccount name. - limit_range: drop the default *limit* (keep the default request); a forced small limit would reject ordinary pods that omit limits — the ResourceQuota bounds usage. - build_account_quota: fail closed on a non-empty-but-unparseable grp_tres instead of silently uncapping the namespace; the reconciler skips such an account. - cluster_kubeconfig: fail closed when associations aren't loaded (the cache resolves fail-open, which could mint an unscoped token). - tests: mem `M`, prefixed-name length, sanitizer-collision, malformed-grp_tres. Co-Authored-By: Claude <noreply@anthropic.com>
… user lookup in reconciler
…ange memory units Extract the scoped-kubeconfig resolve-or-reject step into a pure resolve_user_namespace_sa helper and unit-test its fail-closed paths (cold association cache -> Unavailable, unassociated user -> NotFound) and the namespace/ServiceAccount derivation. Use base-10 128M for the LimitRange default memory request so it matches the base-10 units the ResourceQuota already emits.
The RPC now serves both the cluster-admin kubeconfig (empty user) and a scoped per-user token (user set), so the "admin" name is misleading and easy to misread as an admin-only credential path. Rename the RPC and its request/response messages while the surface is still internal and pre-1.0, avoiding a wire-breaking change after it ships.
Formatting drift introduced by auto-merging quota.rs during the rebase onto latest upstream/main; no functional change.
upstream/main added a `user` filter field to ListUsersRequest (ROCm#447) after this branch diverged; the reconciler's all-users lookup needs to pass an empty filter to keep its "empty account/user = all" behavior.
Leading "+ " on a wrapped doc-comment line was parsed as a markdown list item, tripping clippy::doc_lazy_continuation under -D warnings in CI.
611cae8 to
ee59990
Compare
What
Implements M1 of the Kubernetes quota-enforcement design (#444 RFC): the tenancy + hard-quota layer that projects SPUR accounts onto native Kubernetes objects, plus per-user scoped kubeconfigs — so a cluster looks like a normal Kubernetes deployment to its users while SPUR is the invisible policy plane.
Picks up the work from #471 (which was stacked on #432). Rebased cleanly onto
main— the diff here is exactly the 4 quota commits, no#432dependency, plus one review-fix commit (below).End-to-end flow
Increments (each independently reviewable)
account → Namespace/ResourceQuota/LimitRange/RBACmapping (spur-k8s/quota.rs), unit-testedgrp_tressacctmgrquota_controllerin the operator: lists accounts+members over gRPC, server-side-applies withforce(drift-correcting), opt-in--enable-quotakubeconfig --userspur-core, scoped kubeconfig minted by the control-plane agent, CLI flagDesign decision
Per the RFC, SPUR persists resource caps on QoS, not accounts. M1 wires account-level allocations (
grp_treson theAccount) so each account/namespace has a definite ResourceQuota — the cleanest tenancy model, and it gives the GPU cluster the GPU quota it lacks today.Review fixes applied
A round of review surfaced several correctness/safety issues, all fixed here:
memis base-10 MB, but the ResourceQuota emittedMi(mebibytes) → ~4.86% over-allocation. Now emitsM.spur-acct-/spur-user-prefix, so long names produced invalid (>63-char) namespace/SA names. The cap now includes the prefix.grp_tresused to silently leave the namespace uncapped; the reconciler now skips such an account.limits.cpu/limits.memoryhard keys — with no default limit those keys would reject any pod that omits limits at admission. The cap is enforced onrequests.*(matching the requests-only LimitRange).grp_tresvalidated on create:create_accountnow runsvalidate_tres(as add/modify user already did), so an invalid allocation is rejected withinvalid_argumentinstead of being stored and later skipped by the reconciler.ListUsers(N+1) with a singleListUsersgrouped client-side by account.Known limitations (M1 scope)
app.kubernetes.io/managed-by=spur-quota, so a prune pass is a natural follow-up).physics_labandphysics.labcollapse to the same namespace). Documented with a test; a hash-suffix disambiguation is a follow-up.sacctmgr modify accountis a full-resend upsert, so it clears fields not restated (a pre-existing behavior across all account fields, not specific togrp_tres). Out of scope here; worth a dedicated fix.Testing
cargo build/clippy -D warnings/fmtgreen across the workspace (excl. spur-ffi).spur-k8s(146) +spur-core(339) unit tests pass, including the new mem-unit, prefixed-name-length, sanitizer-collision, and malformed-grp_trestests. DB round-trip test is#[ignore](runs in CI's Postgres).Also validated end-to-end on an isolated 2-node deployment:
sacctmgr add account ... grptres=...persists through gRPC → Postgres and reads back; thegrp_tresmigration applies cleanly (ADD COLUMN IF NOT EXISTS); andk8s kubeconfig --userfails closed for unknown users / unresolved accounts and never falls back to the admin kubeconfig. (Cluster-side SA/token minting still needs a live k0s control plane — that path is integration-level.)