mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-10 06:24:09 +09:00
fix(api): read pool usage under the quota key the controller counts (#5184)
## What this PR does Tenant quota admission does not see how much of an extended resource or hugepages the parent pool already uses, so a child tenant can be granted a share the parent has already spent. Admission reads that usage from `limits.<key>`, but Kubernetes reports it only under `requests.<key>`, so the value is always zero. Now each key is read where the quota controller counts it. cpu, memory and ephemeral-storage still read `limits.*`. The new tests fail on main. ### Screenshots ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(api): tenant quota admission now counts the parent pool's usage of extended resources and hugepages, so a child tenant can no longer get a share the parent has already used. After upgrading, updates to existing tenants may be rejected if their quota for these resources exceeds the parent's remaining budget. This includes updates that leave the quota unchanged. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Quota checks now account for existing usage of extended resources, preventing child requests from exceeding the parent’s remaining quota. * Requests are allowed when current usage leaves enough quota available, and denied when usage has consumed the parent’s full quota. * Resource usage is matched against the appropriate quota category, including requests for extended resources and hugepages. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
2 files changed
+54
-19
No files matched your search
@@ -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.<extended resource> 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.<key> and as
|
||||
// requests.<key>; 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.<key> 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.<key>"
|
||||
// cpu, memory, ephemeral-storage -> "limits.<key>"
|
||||
// storage -> "requests.storage" (no hard limits.storage)
|
||||
// pods, services, services.loadbalancers, ... -> "<key>" (verbatim)
|
||||
func renderedLimitKey(raw string) string {
|
||||
if raw == "storage" {
|
||||
return "requests.storage"
|
||||
}
|
||||
// extended resources, hugepages-<size>, ... -> "requests.<key>"
|
||||
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) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in new issue
Block a user