From 54695fc83b3328a5401710b9b9d5debbbd05809c Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 3 Nov 2026 03:47:58 +0300 Subject: [PATCH] fix(api): read pool usage under the quota key the controller counts Hierarchical quota admission subtracts the parent pool's current usage from what a child tenant may still claim, and reads that usage from ResourceQuota.status.used. It looked every non-raw key up under limits., but the ResourceQuota pod evaluator reports usage under limits.* only for cpu, memory and ephemeral-storage. Extended resources and hugepages are counted under requests.* alone, so their usage always read as zero and a child could be granted budget the parent pool had already consumed. Read usage from requests. for every resource the evaluator counts only on the request side. flatten applies no allocation ratio there, so that key holds the declared amount just as limits. does for cpu, memory and ephemeral-storage. Fixes: e3f09bab1bcc ("fix(apiserver): subtract parent pool usage in hierarchical tenant quota admission") Assisted-by: LLM Signed-off-by: Aleksei Sviridkin --- pkg/registry/apps/application/quota.go | 44 ++++++++++++++------- pkg/registry/apps/application/quota_test.go | 29 ++++++++++++-- 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/pkg/registry/apps/application/quota.go b/pkg/registry/apps/application/quota.go index eb6f20622..5376139c0 100644 --- a/pkg/registry/apps/application/quota.go +++ b/pkg/registry/apps/application/quota.go @@ -233,7 +233,7 @@ func (r *REST) validateTenantResourceQuotas(ctx context.Context, app *appsv1alph } parentLimit := parentQuota[res] allocated := siblings[res] // zero value when no sibling bounds this resource - used := poolUsed[renderedLimitKey(res)] + used := poolUsed[usageKey(res)] remaining := parentLimit.DeepCopy() remaining.Sub(allocated) remaining.Sub(used) @@ -285,25 +285,39 @@ var rawQuotaKeys = map[string]struct{}{ "resourcequotas": {}, } -// renderedLimitKey maps a shorthand resourceQuotas key (the vocabulary admission -// works in) to the rendered ResourceQuota key that carries its limit-side -// budget, mirroring cozy-lib.resources.flatten. The mapping is allocation-ratio -// independent: ratios only scale the requests.* side that flatten derives from -// the same shorthand value, so the limit-side key always holds the operator's -// declared amount. This lets admission read a tenant's current usage from its -// rendered ResourceQuota.status.used without replicating the chart's ratio math. +// limitCountedKeys are the shorthand keys whose usage the ResourceQuota pod +// evaluator reports under limits.*. For every other resource it counts only +// requests.*, so limits. or limits.hugepages-* never carries +// usage even though the chart renders it. +var limitCountedKeys = map[string]struct{}{ + "cpu": {}, + "memory": {}, + "ephemeral-storage": {}, +} + +// usageKey maps a shorthand resourceQuotas key (the vocabulary admission works +// in) to the rendered ResourceQuota key whose status.used carries its usage. +// cozy-lib.resources.flatten renders a shorthand key both as limits. and as +// requests.; usage is read from whichever of the two the quota controller +// counts. For cpu, memory and ephemeral-storage that is the limit side, which +// holds the operator's declared amount whatever allocation ratio scales the +// request side. Every other resource is counted only on the request side, where +// flatten applies no ratio, so requests. holds the declared amount too. +// This lets admission read a tenant's current usage from its rendered +// ResourceQuota.status.used without replicating the chart's ratio math. // -// cpu, memory, ephemeral-storage, custom resources -> "limits." +// cpu, memory, ephemeral-storage -> "limits." // storage -> "requests.storage" (no hard limits.storage) // pods, services, services.loadbalancers, ... -> "" (verbatim) -func renderedLimitKey(raw string) string { - if raw == "storage" { - return "requests.storage" - } +// extended resources, hugepages-, ... -> "requests." +func usageKey(raw string) string { if _, ok := rawQuotaKeys[raw]; ok { return raw } - return "limits." + raw + if _, ok := limitCountedKeys[raw]; ok { + return "limits." + raw + } + return "requests." + raw } // parentNamespaceOf returns the namespace owned by the parent of the tenant that @@ -341,7 +355,7 @@ func (r *REST) poolRootOf(ns string, bounded map[string]bool) string { // created/updated). Usage is read from each member's ResourceQuota.status.used, // taking a per-resource max within a namespace to avoid double-counting the // chart-rendered and controller-allocated quotas, and is keyed in the rendered -// quota key space (see renderedLimitKey). Boundedness is decided by declared +// quota key space (see usageKey). Boundedness is decided by declared // resourceQuotas so the result does not depend on whether Flux has rendered a // freshly-created tenant's quota yet. func (r *REST) parentPoolUsage(ctx context.Context, parentNS, excludeNS string) (map[string]resource.Quantity, error) { diff --git a/pkg/registry/apps/application/quota_test.go b/pkg/registry/apps/application/quota_test.go index f6e5d3054..3ea2b7a2b 100644 --- a/pkg/registry/apps/application/quota_test.go +++ b/pkg/registry/apps/application/quota_test.go @@ -136,19 +136,20 @@ func usageQuota(t *testing.T, namespace, name string, used map[string]string) *c } } -func TestRenderedLimitKey(t *testing.T) { +func TestUsageKey(t *testing.T) { cases := map[string]string{ "cpu": "limits.cpu", "memory": "limits.memory", "ephemeral-storage": "limits.ephemeral-storage", - "devices.com/nvidia": "limits.devices.com/nvidia", + "devices.com/nvidia": "requests.devices.com/nvidia", + "hugepages-2Mi": "requests.hugepages-2Mi", "storage": "requests.storage", "pods": "pods", "services.loadbalancers": "services.loadbalancers", } for raw, want := range cases { - if got := renderedLimitKey(raw); got != want { - t.Errorf("renderedLimitKey(%q) = %q, want %q", raw, got, want) + if got := usageKey(raw); got != want { + t.Errorf("usageKey(%q) = %q, want %q", raw, got, want) } } } @@ -339,6 +340,26 @@ func TestValidateTenantResourceQuotas_PoolUsage(t *testing.T) { } }) + t.Run("parent usage of an extended resource shrinks remaining", func(t *testing.T) { + parent := tenantHelmRelease(t, "foo", "tenant-root", map[string]string{"example.com/device": "2"}) + used := usageQuota(t, "tenant-foo", "tenant-quota", map[string]string{"requests.example.com/device": "2"}) + r := newTenantREST(t, parent, used) + + if errs := r.validateTenantResourceQuotas(context.Background(), childApplication(t, "bar", "tenant-foo", map[string]string{"example.com/device": "1"})); len(errs) == 0 { + t.Fatalf("child example.com/device=1 should be denied when the parent pool already uses 2 of 2") + } + }) + + t.Run("parent usage of an extended resource leaves the rest grantable", func(t *testing.T) { + parent := tenantHelmRelease(t, "foo", "tenant-root", map[string]string{"example.com/device": "2"}) + used := usageQuota(t, "tenant-foo", "tenant-quota", map[string]string{"requests.example.com/device": "1"}) + r := newTenantREST(t, parent, used) + + if errs := r.validateTenantResourceQuotas(context.Background(), childApplication(t, "bar", "tenant-foo", map[string]string{"example.com/device": "1"})); len(errs) > 0 { + t.Fatalf("child example.com/device=1 should be allowed when the parent pool uses 1 of 2, got: %v", errs) + } + }) + // No rendered usage yet (freshly-created parent, Flux has not rendered the // quota): usage degrades to zero and behaviour matches the pre-usage gate. t.Run("absent usage degrades to budget-only check", func(t *testing.T) {