Repository navigation
design-proposal(quota): settle unnamed class keys and backup quota after phase 5 - #109
Open
mattia-eleuteri wants to merge 10 commits into
Open
mattia-eleuteri wants to merge 10 commits into
mattia-eleuteri wants to merge 10 commits into
Conversation
Tenant quotas are declared in instance-type units but enforced in pod units, and the gap between the two is KubeVirt's per-VM virtualization overhead: additive, not proportional. That is why --tenant-quota-buffer-percent cannot be set correctly, the required buffer ranging from +6% to +183% with VM granularity alone. Behind that symptom, no quota gate stands in front of an ordinary application order: validateTenantResourceQuotas runs only for the Tenant kind. The proposal makes reservation the accounting authority: the sum of the sizes declared in a tenant's apps.cozystack.io resources, evaluated at admission from a declarative spec.reservation block on ApplicationDefinition, with the gate generalized to every kind. The v1.6.0 hierarchical pool arithmetic is kept; both the usage it counts and the budget it counts against move from rendered ResourceQuota objects to declared values, which takes ResourceQuota.status.used off the admission path. The namespace ResourceQuota is retired rather than loosened, and --tenant-quota-buffer-percent is deleted. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
Per design-proposals/template.md the status transitions Draft -> Review when the PR is opened. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
Evaluates KubeVirt's Application Aware Quota: it prices launchers by declared guest size and removes the additive overhead, but its accounting unit is the pod, non-schedulable resources fall back to a managed native ResourceQuota, and its cluster quota is a flat selector whose non-schedulable half exists only on OpenShift. Rejected as a replacement, recorded as complementary to the operational limit. Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
…nism - add a terminology section (reservation, budget, consumes) and drop billing and pricing vocabulary; one forward reference to metering in §4 - justify the emulator-thread core by capacity, not by contract - drop the ephemeral-storage key; give o1 its own overcommitted-memory key - replace the typed ReservationItem language with the evaluator contract; move a CEL sketch to a non-normative appendix - presets as platform-shipped environment objects, as an implementation note - settle the open questions: sentinels as a chart defect, re-evaluation over budget, declared storage, tenant-module fallback, and Tenant status persisted as annotations on its HelmRelease Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
… storage Etcd, ingress and seaweedfs modules are exempt as platform overhead, and etcd stays exempt once per-cluster etcd (cozystack#25) moves it into the Kubernetes app. Monitoring reserves its declared storage only, which is the one module figure that still needs a gate when the flag is enabled. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
…re phase 5 The Context, §1 and §7 claimed every workload a tenant can create goes through apps.cozystack.io. Three paths do not, and tenant-quota is the only bound on each today: - cozy:tenant:super-admin:base grants '*' on kubevirt.io/virtualmachines, so a super-admin can create or resize a raw VM outside any reservation; - RestoreJob (admin) restores VMInstance/VMDisk HelmReleases directly through Velero and postRestoreRename, never through the gate; - VMImportTask (admin) has Forklift fill DataVolumes before the VMDisk that adopts them reaches the gate. §7 now makes closing them a phase-5 prerequisite: the super-admin VM grant becomes read-only (a ResourceQuota cannot select pods by owner, so a quota for non-application workloads would be tenant-quota again), restore and import check the pool through pkg/reservation before writing, unowned raw VMs are reported before tenant-quota is deleted, and an e2e allowlist of tenant write verbs keeps the next such path from going unnoticed. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
…in on its own Status moves to Accepted. The '*' on kubevirt.io/virtualmachines in cozy:tenant:super-admin:base is a bug from cozystack/cozystack#516, whose sibling '*' on helmreleases was already removed, so §7 now says the narrowing and its write-verb allowlist e2e land on main independently; phase 4 only depends on that change having landed. Signed-off-by: Mattia Eleuteri <mattia@hidora.io> Assisted-by: LLM
… phase 5 Follow-up to the review on bbf7df0. - §3: a class key a budget does not name falls back to its base key (dedicated-cpu to cpu, hugepages-<size> and overcommitted-memory to memory), per budget and at every level of the hierarchy. Reading it as zero would put every existing cpu/memory tenant with m1, cx1, d1, n1, rt1 or o1 VMs over budget; reading it as unbounded would let the classes bypass cpu and memory. Replaces the "grants none" sentence for overcommitted-memory. - Upgrade: hugepages-backed and o1 VMs now count their full guest memory against memory; a pre-apply report names the pools that go over, as for a re-evaluation. - Scope: cozystack#50's measured backup dimensions are accepted ("reservation-based where size is declared, measured where it is discovered") and move to a backup-only ResourceQuota when phase 5 deletes tenant-quota. - Rollout: the reported symptom stays until phase 5, since tenant-quota still counts launcher overhead in phases 3 and 4. - §7: the super-admin VM rule keeps get, list and watch, matching cozystack/cozystack#4797. - cozystack#35 now calls the claim IPAddressClaim. Signed-off-by: Mattia Eleuteri <mattia@hidora.io> Assisted-by: LLM
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Contributor
|
Hi! We removed Sorry for the extra work, I wanted to mention this now so it doesn't come up late in review. |
cozystack#111 derives proposal status and dates from PR history, so the header no longer carries them. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
…-followup cozystack#48 landed on main by rebase, so its file arrives as an add/add conflict. Main's blob is identical to the PR 48 head this branch was built on, so this branch's side is kept. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
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 this PR does
Follow-up to Andrei Kvapil (@kvaps)' review of #48 on
bbf7df0. It is stacked on #48: until #48 merges, the diff also shows #48's commits. Only the last commit,05415d6, belongs to this PR.Class keys a budget does not name (§3, Upgrade)
Neither reading proposed in the review works. Zero puts every existing tenant with a
cpu/memorybudget andm1,cx1,d1,n1,rt1oro1VMs over budget. Unbounded lets the classes bypasscpuandmemory. The doc now uses a third rule: a class key a budget does not name falls back to its base key.dedicated-cpufalls back tocpu, andhugepages-<size>andovercommitted-memoryfall back tomemory. The rule applies to each budget separately, at every level of the hierarchy. An existing budget keeps bounding every instance type, and making a class a separate grant is an explicit operator decision.At upgrade, hugepages-backed VMs and
o1VMs start counting their full guest memory againstmemory, whichtenant-quotadoes not do today. The Upgrade section adds a pre-apply report, as for a re-evaluation. Nothing is evicted: an affected pool reportsOvercommitted. The "grants none" sentence aboutovercommitted-memoryis replaced.Backup storage after phase 5 (Scope, Rollout, Open questions)
#50 is now listed in Scope, and the rule it asks reviewers to accept is accepted: "reservation-based where size is declared, measured where it is discovered". When phase 5 deletes
tenant-quota, the two backup dimensions move to aResourceQuotathat carries only those keys. kube-controller-manager masks usage tospec.hard, so that object counts no pod and cannot reject one.Smaller changes
tenant-quotacounts launcher overhead in phases 3 and 4.get,listandwatch, matching fix(cozystack-basics): make the tenant super-admin VirtualMachine grant read-only cozystack#4797. If fix(rbac): drop tenant super-admin write on raw kubevirt VMs cozystack#4801 lands instead, this line dropswatch.IPAddressClaim.cx1VM under a budget that declares onlycpu/memory.🤖 Generated with Claude Code