diff --git a/design-proposals/tenant-quota-reservation/README.md b/design-proposals/tenant-quota-reservation/README.md index 50ead396..baf4b84d 100644 --- a/design-proposals/tenant-quota-reservation/README.md +++ b/design-proposals/tenant-quota-reservation/README.md @@ -2,8 +2,6 @@ - **Title:** `Tenant quotas as reservation limits` - **Author(s):** `@mattia-eleuteri` -- **Date:** `2026-08-03` -- **Status:** Accepted ## Overview @@ -30,12 +28,13 @@ This proposal is about quotas: what a declared application consumes from a tenan - **[Out-of-tree app catalogs](https://github.com/cozystack/community/pull/43)** proposes splitting the managed-application catalog out of the core repository. This turns the "declarative, not Go" choice in [§2](#2-consumption-declared-on-the-applicationdefinition) from a preference into a requirement: an evaluator implemented as a `switch` over kinds inside the aggregated apiserver cannot describe an application whose package lives in another repository. How a kind consumes has to travel with the package. - **[Fold `extra` into `apps`](https://github.com/cozystack/community/pull/39)** makes tenant modules regular applications and moves their distinguishing traits into declarative capabilities on `ApplicationDefinition`. It sets the precedent this proposal follows, per-kind behavior expressed as data on the definition rather than as a directory or a code branch. Tenant modules are platform, with one exception, monitoring's storage ([§2](#2-consumption-declared-on-the-applicationdefinition)). That one figure needs a gate when a module appears, which #39 provides by turning the module into an order; [§5](#5-generalizing-the-gate-to-every-kind) states the fallback if #39 stalls. - **`proposal/application-definition-versioning`** (branch on this repository, by `@kvaps`, not yet a PR) splits `ApplicationDefinition` into per-version `ApplicationSchema` objects and converts tenant-supplied values into a single **storage version** before persisting them into the HelmRelease. The two compose cleanly: a reservation is evaluated against the storage form, so a kind declares its consumption **once**, against the storage version, and served versions inherit it through the existing conversion. If that proposal lands first, the declaration moves to the storage-version `ApplicationSchema` with no change in semantics. -- **[Public IPs as a first-class resource](https://github.com/cozystack/community/pull/35)** would make a public address a `PublicIPClaim` rather than an implicit consequence of `external: true`. Counting `services.loadbalancers` from that boolean is the interim form: when addresses become claimable objects, the count should follow the claims instead. +- **[Public IPs as a first-class resource](https://github.com/cozystack/community/pull/35)** would make a public address an `IPAddressClaim` rather than an implicit consequence of `external: true`. Counting `services.loadbalancers` from that boolean is the interim form: when addresses become claimable objects, the count should follow the claims instead. - **[`kubernetes-nodes-split`](../kubernetes-nodes-split/README.md)** (Accepted) has now landed in full. Phase 1 made `KubernetesNodes` a registered application kind with its own `ApplicationDefinition` (`packages/system/kubernetes-nodes-rd/cozyrds/kubernetes-nodes.yaml`), carrying `minReplicas`, `maxReplicas`, `instanceType`, `resources` and `diskSize` at the top level of its values. Phase 2 ([cozystack/cozystack#3315](https://github.com/cozystack/cozystack/pull/3315)) merged on 2026-08-26 and removed `spec.nodeGroups` from the `Kubernetes` CR, together with the implicit `md0` default. A worker pool's reservation is therefore read from the values of one flat kind, with nothing transitional about it and no iteration over a parent's map. The implicit default pool it retired did not disappear from the platform, though: it moved into the ComputePlane module, which is discussed in [§2](#2-consumption-declared-on-the-applicationdefinition). - **[Database Horizontal Autoscaler](../database-horizontal-autoscaling/README.md)** (Accepted) does **not** pass through this gate, and an earlier revision of this proposal said it did. The accepted DHA design is entirely stock: the application chart renders a KEDA `ScaledObject` whose `scaleTargetRef` is the CNPG `Cluster`, KEDA's managed HPA drives that CR's `scale` subresource, and the chart writes `spec.instances` as a **constant seed** — `max(replicas, effectiveMin)` — that the autoscaler never writes back to (`packages/apps/postgres/templates/scaledobject.yaml`, `templates/db.yaml`, `templates/_autoscaling.tpl`). The `Application`'s `replicas` value is untouched by any scaling decision, so no `Update` ever reaches admission. This is the same asymmetry this proposal already identifies for the cluster-autoscaler on worker pools, and it takes the same answer: **when `autoscaling.enabled` is set, the reservation is taken at `autoscaling.maxReplicas`, not `replicas`.** [§2](#2-consumption-declared-on-the-applicationdefinition) lists this among the things a kind's declaration must be able to express. DHA also needs a matching amendment. Its §4 states that "quota is not re-implemented: the HPA scales the engine CR and pod creation passes through the tenant `ResourceQuota` admission, so an over-quota scale-up simply fails to create pods". Once the namespace `ResourceQuota` stops being the tenant contract ([§7](#7-retiring-the-namespace-resourcequota)) that sentence is no longer true, and the `Pending`-independent quota alert it asks the implementation to add ([cozystack/cozystack#3954](https://github.com/cozystack/cozystack/pull/3954)) loses the signal it was keyed on. Reserving the ceiling is what replaces it: the capacity counts against the pool as soon as autoscaling is enabled, so the scale-up cannot be refused at all. - **[Per-cluster etcd](https://github.com/cozystack/community/pull/25)** moves etcd out of the tenant modules and into `apps/kubernetes`, which renders its own `EtcdCluster` per cluster and exposes only `etcd.replicas`, with member size fixed by the chart. That puts etcd on the platform side of the line this proposal draws: like the chart's other satellites (CSI, cloud-controller, cluster-autoscaler), its members are platform overhead and `etcd.replicas` does not change a cluster's reservation. The shared `Etcd` tenant module it replaces is platform too, like every tenant module ([§2](#2-consumption-declared-on-the-applicationdefinition)). +- **[Backup storage quota](https://github.com/cozystack/community/pull/50)** builds a backup quota on `ResourceQuota`, with two extended dimensions (`backups.cozystack.io/size`, `backups.cozystack.io/count`) carried in `tenant-quota`'s `spec.hard` and written into its `status.used` by the backup controller. That is the opposite direction from this proposal, and it is accepted: a VM's size is declared before the VM exists, a backup's size is only known after the backup has run, so the rule is **reservation-based where size is declared, measured where it is discovered**. The reservation evaluator emits no backup key and the gate reads none; #50's admission on `BackupJob` enforces them. The interaction is phase 5, which deletes `tenant-quota` ([§7](#7-retiring-the-namespace-resourcequota)). The backup dimensions do not go with it: they move to a `ResourceQuota` of their own that carries only those two keys in `spec.hard`. kube-controller-manager masks a quota's usage to its `spec.hard` keys, so that object counts no pod, cannot reject one, and does not bring back the pod-counter failure mode §7 removes. If #50 lands after phase 5, it renders that object directly instead of extending `tenant-quota`. - Any new application kind, such as the one in [`compute-plane`](../compute-plane/README.md), needs to declare its consumption to participate in quotas. By design that is a data change in the package. - **Deferred to separate work:** the root cause of `ResourceQuota.status.used` going stale (a kube-controller-manager behavior, see [The problem](#the-problem)), drift alerting, and a remediation runbook for already-affected namespaces. This proposal removes that counter from the tenant-facing contract; it does not fix it. @@ -239,7 +238,11 @@ But one `cpu` key cannot hold every core. A shared vCPU under the allocation rat | `hugepages-` | `spec.memory.hugepages.pageSize` | `m1`, `cx1`, `n1`, `rt1` (2Mi and 1Gi variants of each) | | `overcommitted-memory` | `spec.memory.overcommitPercent` | `o1` | -These class keys are the one place this proposal names quota keys, because an instance type has to land on some key and the classes differ in kind; their names are the platform's to fix, and nothing else is added. Two notes on the table, because an earlier sketch of it got both wrong by classifying on the series name. Hugepages are **not** a `cx1` property: `m1` is a shared-CPU series that nonetheless requests hugepages, and `n1` is a fourth dedicated series alongside `d1`/`cx1`/`rt1`. And `o1` sets `overcommitPercent: 50`, which means its launcher requests half the guest memory — the only case on the platform where the pod request is *smaller* than the reservation. Reserving the guest figure for it is still right, since it is what the tenant declared, and it gets its own key, as hugepages have, because a cluster cannot grant the same physical page as both `o1` memory and `u1` memory. A budget that does not mention `overcommitted-memory` grants none of it. +These class keys are the one place this proposal names quota keys, because an instance type has to land on some key and the classes differ in kind; their names are the platform's to fix, and nothing else is added. Two notes on the table, because an earlier sketch of it got both wrong by classifying on the series name. Hugepages are **not** a `cx1` property: `m1` is a shared-CPU series that nonetheless requests hugepages, and `n1` is a fourth dedicated series alongside `d1`/`cx1`/`rt1`. And `o1` sets `overcommitPercent: 50`, which means its launcher requests half the guest memory — the only case on the platform where the pod request is *smaller* than the reservation. Reserving the guest figure for it is still right, since it is what the tenant declared, and it gets its own key, as hugepages have, because a cluster cannot grant the same physical page as both `o1` memory and `u1` memory. + +**A class key a budget does not name falls back to its base key.** `dedicated-cpu` falls back to `cpu`; `hugepages-` and `overcommitted-memory` fall back to `memory`. The rule applies to each budget separately, at every level of the hierarchy: a reservation is charged to its class key in a budget that declares that key, and to the base key in a budget that does not. A tenant whose budget is `cpu: 16, memory: 64Gi` can therefore order any instance type, and every one of them draws from those two figures. An operator who wants pinned cores or hugepages to be a separate grant declares the class key, and from then on that class draws only from it. A child `Tenant` that declares `dedicated-cpu: 4` under a parent that declares only `cpu` carves those four out of the parent's `cpu`. A budget that names neither a class key nor its base key leaves that class unbounded at its level, like any undeclared key today ([Failure and edge cases](#failure-and-edge-cases)). + +The other two readings both fail. If an unnamed class key granted nothing, every existing tenant with a `cpu`/`memory` budget and `m1`, `cx1`, `d1`, `n1`, `rt1` or `o1` VMs would be over budget once the gate is on. If it were unbounded, the classes would be a way around `cpu` and `memory`: a tenant with a `memory` budget could order `m1` and pay nothing for its memory. With the fallback, a budget written before class keys existed keeps its meaning, since every core and every byte the tenant orders counts against it. Splitting a class out is then an explicit operator decision. The rule still changes what some VMs consume at upgrade, which [Upgrade and rollback compatibility](#upgrade-and-rollback-compatibility) covers. **What is reserved is what is stable and enumerable on the instance type.** For a dedicated type with `isolateEmulatorThread: true` — which every shipped dedicated type sets — the reservation is `guest + 1` dedicated cores, plus any supplemental I/O thread count, reading the even-parity annotation if one is ever set: `cx1.2xlarge` is eight guest vCPUs on nine pinned cores. The reason is capacity. A pinned core nobody else can be given has to come out of someone's budget, and since the figure is a property of the type, the catalog shows it, so a tenant can predict what an order will consume before making it. What KubeVirt computes at *runtime*, namely the launcher memory overhead, stays platform margin — that is the entire point of this proposal on the memory side. @@ -349,7 +352,7 @@ This also removes the last reason to keep the inflation factor in any form, whic Phase 5 deletes the only bound on the three paths in [Context](#context), so each is closed first. Phases 1 to 4 do not depend on this, because `tenant-quota` still binds there. -**Raw VirtualMachines: the super-admin grant becomes read-only.** `cozy:tenant:super-admin:base` drops its `'*'` rule on `kubevirt.io/virtualmachines`. The super-admin role keeps only the `get` and `list` that `cozy:tenant:admin:base` grants and aggregation folds in. It loses `update` and `patch` along with `create`. On a chart-rendered VM, the size lives in `spec.template.spec.domain` and `spec.instancetype`, and a policy allowing "every field but those" would have to follow KubeVirt's spec from release to release. Every legitimate write already has a narrower route: +**Raw VirtualMachines: the super-admin grant becomes read-only.** `cozy:tenant:super-admin:base` drops its `'*'` rule on `kubevirt.io/virtualmachines`. The super-admin rule keeps `get`, `list` and `watch`. `watch` is there for the dashboard, which watches the VM for its live power state instead of polling it. The rule loses `update` and `patch` along with `create`. On a chart-rendered VM, the size lives in `spec.template.spec.domain` and `spec.instancetype`, and a policy allowing "every field but those" would have to follow KubeVirt's spec from release to release. Every legitimate write already has a narrower route: - power operations: the `virtualmachines/start|stop|restart` subresources, granted at `use` level; - console, VNC and port-forward: the `virtualmachineinstances` subresources, also at `use` level; @@ -390,6 +393,8 @@ The upgrade has one consequence that no migration can handle automatically. Oper In the other direction, reserving at the autoscaling ceiling can make a write that used to succeed fail. This applies to two populations, not one: clusters with wide `KubernetesNodes` bounds, and any database with `autoscaling.enabled` whose `autoscaling.maxReplicas` is well above its resting `replicas`. Both should be checked against pool headroom before the flag is enabled, and the `reserved`/`budget` status shipped in the same release is what makes that checkable. +The class-key fallback in [§3](#3-environment-instance-types-and-presets) changes what some VMs count against an existing budget, in the same direction. The namespace quota counts a hugepages-backed VM (`m1`, `cx1`, `n1`, `rt1`) under `hugepages-`, which no tenant budget names, so today only its launcher overhead reaches `memory`. An `o1` launcher requests half of its guest memory. With the gate on, both count their full guest memory against `memory` unless the budget declares the class key. A tenant can therefore be over budget the moment the flag is turned on. This is handled the way a re-evaluation is handled ([Security](#security)). Before the flag is enabled, a pre-apply report lists, per pool, the reservation under each class key and its base key and the tenants that would go over. Nothing is evicted: an affected pool reports `Overcommitted` and refuses new orders until the operator raises the budget or declares the class key. + Rollback is turning the flag off, for every step except the last. Phase 5 is irreversible in the ordinary sense that objects are deleted — `tenant-quota-allocated`, and now `tenant-quota` itself — and it is deliberately sequenced after the flag has been on across a release. The `LimitRange`/launcher-limit interaction described in [§7](#7-retiring-the-namespace-resourcequota) must be resolved on a dev cluster before that phase ships; it is the one step that can break running VMs rather than merely rejecting new writes. The same phase leaves any raw `VirtualMachine` a super-admin created before the grant was narrowed unbounded. It is reported before `tenant-quota` is deleted, for the operator to adopt or remove, and nothing deletes it automatically. ## Security @@ -420,6 +425,7 @@ Rollback is turning the flag off, for every step except the last. Phase 5 is irr - A human edit raising `maxReplicas` or `autoscaling.maxReplicas` past the pool budget → *that* is an `Update`, and it is rejected with the delta named. Raising the ceiling is where the capacity conversation happens. - Unparseable values on the object being written → rejected. On another object in the pool → warned, pool condition set, and that object keeps its last known reservation rather than dropping to zero. - Tenant with no declared quota → unbounded, draws from its nearest bounded ancestor's pool, unchanged from today. +- Budget that declares `cpu` and `memory` but no class key, with a `cx1` VM in the pool → the VM's pinned cores count against `cpu` and its hugepages against `memory` ([§3](#3-environment-instance-types-and-presets)). Once the budget declares `dedicated-cpu`, the same cores count against that key only. - Tenant super-admin creates or patches a `kubevirt.io` `VirtualMachine` → forbidden by RBAC once the grant is narrowed. A raw VM created before that keeps running, and the phase-5 upgrade reports it as unowned. - Copy restore of a `VMInstance` into a pool without room, or an in-place restore of a larger backed-up size → the `RestoreJob` fails before the release reaches Flux, naming the pool and the size. - `VMImportTask` whose source VMs do not fit the pool → fails before the Forklift plan is created, so no DataVolume is filled. @@ -452,16 +458,17 @@ Rollback is turning the flag off, for every step except the last. Phase 5 is irr | 2 | Consumption declared on `ApplicationDefinition`, for the IaaS kinds first (`vm-instance`, `vm-disk`, `kubernetes`, `kubernetes-nodes`), which carry the whole overhead problem. ComputePlane's `md0` default materialized into its values | n/a | | 3 | Usage oracle and budget source switchable; gate generalized to kinds that declare their consumption, with per-pool-root serialization and post-write re-read; `reserved`/`budget` persisted on the `Tenant`'s HelmRelease and projected into its status | off by default | | 4 | Remaining kinds, package by package, until every kind declares its consumption or an explicit exemption; redis, valkey and Harbor's redis sentinels moved to a small fixed preset; exemptions on the tenant module kinds except `Monitoring`, which reserves its storage, and its gate (via #39 or the fallback in [§5](#5-generalizing-the-gate-to-every-kind)); the rendered-requests overhead figures checked in per kind; the [§7 prerequisites](#prerequisite-no-sized-workload-around-the-gate): super-admin write verbs on `kubevirt.io/virtualmachines` removed (landed on `main` independently, with the tenant write-verb allowlist in e2e), restore and import checked against the pool | on by default | -| 5 | Unowned raw `VirtualMachines` in tenant namespaces reported; then `tenant-quota` deleted and the `LimitRange` re-homed unconditionally; `EnforcedHard` / `tenant-quota-allocated` removed; `--tenant-quota-buffer-percent` deleted; flag removed | removed | +| 5 | Unowned raw `VirtualMachines` in tenant namespaces reported; then `tenant-quota` deleted, its backup dimensions (if #50 has landed) moved to a backup-only `ResourceQuota`, and the `LimitRange` re-homed unconditionally; `EnforcedHard` / `tenant-quota-allocated` removed; `--tenant-quota-buffer-percent` deleted; flag removed | removed | -Phases 1 to 3 form a self-contained, testable increment and are what implementation would start with — and they are already the bulk of the user-visible win, since they are what puts a gate in front of an ordinary application order for the first time. Phase 5 is only defensible once phase 4 is complete. Removing the runtime net presupposes two things: every kind reserves, and no sized workload reaches a tenant namespace around the gate. Phase 4 delivers both. Phase 5 additionally depends on resolving the `LimitRange`/launcher-limit interaction in [§7](#7-retiring-the-namespace-resourcequota) on a dev cluster. +Phases 1 to 3 form a self-contained, testable increment and are what implementation would start with — and they are already the bulk of the user-visible win, since they are what puts a gate in front of an ordinary application order for the first time. They do not fix the reported symptom, though. A tenant whose quota equals the guest memory of its VMs still cannot start the last one until phase 5, because `tenant-quota` still counts launcher overhead in phases 3 and 4. Until then, the workaround is the same as today: inflate the tenant's quota. Phase 5 is only defensible once phase 4 is complete. Removing the runtime net presupposes two things: every kind reserves, and no sized workload reaches a tenant namespace around the gate. Phase 4 delivers both. Phase 5 additionally depends on resolving the `LimitRange`/launcher-limit interaction in [§7](#7-retiring-the-namespace-resourcequota) on a dev cluster. ## Open questions Every question this proposal opened has been settled in review and is recorded here: - **`maxReplicas`, not `minReplicas`, for anything an autoscaler can raise without writing the CR.** Neither actuator writes the application CR — the cluster-autoscaler scales the MachineDeployment, KEDA's HPA scales the CNPG `Cluster`'s scale subresource — so there is no `Update` for a gate to see, and reserving the minimum would push the failure back to pod admission, which is what this proposal removes. It counts idle headroom against the pool; that is the accepted trade. -- **Instance types resolve to scalars, keyed by class**, over the alternative of a per-type `count/`-style key, which loses fungibility inside a class. `o1` gets its own `overcommitted-memory` key, consistent with hugepages. See [§3](#3-environment-instance-types-and-presets). +- **Instance types resolve to scalars, keyed by class**, over the alternative of a per-type `count/`-style key, which loses fungibility inside a class. `o1` gets its own `overcommitted-memory` key, consistent with hugepages. A class key a budget does not name falls back to its base key, so existing `cpu`/`memory` budgets keep bounding every instance type. The VMs this newly counts are reported before the flag is enabled. See [§3](#3-environment-instance-types-and-presets). +- **Backup storage is measured, not reserved**, and enforced on its own backup-only `ResourceQuota` once `tenant-quota` is gone. The rule "reservation-based where size is declared, measured where it is discovered" is accepted ([Scope](#scope-and-related-proposals)). - **How a kind expresses its consumption is not fixed by this proposal.** The evaluator's contract is ([§2](#2-consumption-declared-on-the-applicationdefinition)); [Appendix A](#appendix-a-non-normative-sketch-of-a-declaration) sketches one shape. - **Presets** are resolved from the same source the chart renders from; making them platform-shipped environment objects is the implementation direction ([§3](#3-environment-instance-types-and-presets)). - **The guard quota is dropped, not loosened.** See [§7](#7-retiring-the-namespace-resourcequota) for the reasoning and for the two things that had to be re-homed.