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) {