fix(apiserver): subtract parent pool usage in hierarchical tenant quota admission

The declaration-time gate only subtracted sibling carve-outs from a parent's
budget, not the parent pool's current usage. A tenant already consuming most of
its budget could therefore carve a full-budget child, letting the sub-tree
collectively exceed the root's allocation.

Sum the current usage of the parent pool (its own namespace plus its unbounded
descendants, read from each member's rendered ResourceQuota.status.used) and
subtract it from the remaining budget, mapping each shorthand quota key to its
rendered limit key (renderedLimitKey). Boundedness is decided by declared
resourceQuotas so the result is independent of whether Flux has rendered a
freshly-created tenant's quota yet. This is best-effort at declaration time (a
snapshot; concurrent writes can briefly overshoot and running pods are never
evicted) — the tenant-quota controller remains the runtime backstop.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
This commit is contained in:
Алексей Артамонов authored and Andrei Kvapil committed 2026-06-30 16:12:17 +02:00
1 parent b0556cc753
commit e3f09bab1b
2 files changed
+262 -2

No files matched your search

+178 -2
View File
@@ -24,6 +24,7 @@ import (
"strings"
helmv2 "github.com/fluxcd/helm-controller/api/v2"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/labels"
@@ -211,6 +212,20 @@ func (r *REST) validateTenantResourceQuotas(ctx context.Context, app *appsv1alph
return append(allErrs, field.InternalError(fldPath, err))
}
// Current usage of the parent's pool (its own namespace plus its unbounded
// descendants). Counting it keeps the invariant — a sub-tree may never, via
// quota, be promised more than its root owns — holding at declaration time:
// without it a parent already using most of its budget could still carve a
// full-budget child and the two would collectively exceed the root. This is
// best-effort (a snapshot; concurrent writes can briefly overshoot and
// already-running pods are never evicted) — the tenant-quota controller is
// the runtime backstop.
childNamespace := r.computeTenantNamespace(app.Namespace, app.Name)
poolUsed, err := r.parentPoolUsage(ctx, app.Namespace, childNamespace)
if err != nil {
return append(allErrs, field.InternalError(fldPath, err))
}
for _, res := range sortedQuotaKeys(parentQuota) {
want, requested := childQuota[res]
if !requested {
@@ -218,12 +233,14 @@ 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)]
remaining := parentLimit.DeepCopy()
remaining.Sub(allocated)
remaining.Sub(used)
if want.Cmp(remaining) > 0 {
allErrs = append(allErrs, field.Forbidden(fldPath.Key(res),
fmt.Sprintf("requested %s exceeds the parent tenant's remaining %q quota of %s (parent allows %s, %s already allocated to sibling tenants); a child tenant may not be granted more quota than its parent has left",
want.String(), res, remaining.String(), parentLimit.String(), allocated.String())))
fmt.Sprintf("requested %s exceeds the parent tenant's remaining %q quota of %s (parent allows %s, %s already allocated to sibling tenants, %s currently in use by the parent pool); a child tenant may not be granted more quota than its parent has left",
want.String(), res, remaining.String(), parentLimit.String(), allocated.String(), used.String())))
}
}
return allErrs
@@ -251,3 +268,162 @@ func sortedQuotaKeys(q map[string]resource.Quantity) []string {
sort.Strings(keys)
return keys
}
// rawQuotaKeys are the shorthand resourceQuotas keys that the tenant chart emits
// verbatim as ResourceQuota hard keys (no limits./requests. prefix). Kept in
// sync with cozy-lib.resources.flattenResource ($rawQuotaKeys).
var rawQuotaKeys = map[string]struct{}{
"pods": {},
"services": {},
"services.loadbalancers": {},
"services.nodeports": {},
"services.clusterip": {},
"configmaps": {},
"secrets": {},
"persistentvolumeclaims": {},
"replicationcontrollers": {},
"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.
//
// cpu, memory, ephemeral-storage, custom resources -> "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"
}
if _, ok := rawQuotaKeys[raw]; ok {
return raw
}
return "limits." + raw
}
// parentNamespaceOf returns the namespace owned by the parent of the tenant that
// owns ns, or "" for the root tenant and non-tenant namespaces. It mirrors
// tenantquota.parentNamespace / the inverse of computeTenantNamespace: the
// hierarchy is encoded in the namespace name, so the parent is recovered by
// stripping the trailing "-<name>" segment.
func (r *REST) parentNamespaceOf(ns string) string {
if ns == rootTenantNamespace || !strings.HasPrefix(ns, r.releaseConfig.Prefix) {
return ""
}
segments := strings.Split(ns, "-")
if len(segments) == 2 {
return rootTenantNamespace
}
return strings.Join(segments[:len(segments)-1], "-")
}
// poolRootOf returns the nearest ancestor namespace (inclusive of ns) that is
// bounded — declares its own quota — or "" when none is. That ancestor is the
// pool root governing ns.
func (r *REST) poolRootOf(ns string, bounded map[string]bool) string {
for cur := ns; cur != ""; cur = r.parentNamespaceOf(cur) {
if bounded[cur] {
return cur
}
}
return ""
}
// parentPoolUsage sums the current usage of the pool rooted at parentNS: the
// parent's own namespace plus every unbounded descendant whose nearest bounded
// ancestor is parentNS. Bounded sub-tenants are excluded — their reservations
// are already charged as sibling carve-outs — as is excludeNS (the tenant being
// 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
// 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) {
releases := &helmv2.HelmReleaseList{}
selector := labels.SelectorFromSet(labels.Set{
ApplicationKindLabel: r.kindName,
ApplicationGroupLabel: r.gvk.Group,
})
if err := r.c.List(ctx, releases, &client.ListOptions{LabelSelector: selector}); err != nil {
return nil, err
}
bounded := map[string]bool{}
owned := make([]string, 0, len(releases.Items))
for i := range releases.Items {
hr := &releases.Items[i]
name := strings.TrimPrefix(hr.Name, r.releaseConfig.Prefix)
ns := r.computeTenantNamespace(hr.Namespace, name)
owned = append(owned, ns)
quotas, err := declaredQuotasFromHelmRelease(hr)
if err != nil {
klog.Warningf("skipping tenant %s/%s with unparseable quotas while summing pool usage: %v", hr.Namespace, hr.Name, err)
continue
}
if len(quotas) > 0 {
bounded[ns] = true
}
}
members := map[string]bool{parentNS: true}
for _, ns := range owned {
if ns == excludeNS {
continue
}
if r.poolRootOf(ns, bounded) == parentNS {
members[ns] = true
}
}
delete(members, excludeNS)
quotas := &corev1.ResourceQuotaList{}
if err := r.c.List(ctx, quotas); err != nil {
return nil, err
}
perNamespace := map[string]map[string]resource.Quantity{}
for i := range quotas.Items {
rq := &quotas.Items[i]
if !members[rq.Namespace] {
continue
}
perNamespace[rq.Namespace] = maxQuotas(perNamespace[rq.Namespace], resourceListToQuotas(rq.Status.Used))
}
total := map[string]resource.Quantity{}
for _, used := range perNamespace {
addQuotas(total, used)
}
return total, nil
}
// resourceListToQuotas converts a corev1.ResourceList to the string-keyed quota
// map the admission arithmetic uses.
func resourceListToQuotas(rl corev1.ResourceList) map[string]resource.Quantity {
out := make(map[string]resource.Quantity, len(rl))
for k, v := range rl {
out[string(k)] = v
}
return out
}
// maxQuotas returns the per-resource maximum of dst and src as a new map.
// Multiple ResourceQuotas in one namespace each report the same usage, so usage
// is merged with a max (not a sum) to avoid double counting.
func maxQuotas(dst, src map[string]resource.Quantity) map[string]resource.Quantity {
out := map[string]resource.Quantity{}
for k, v := range dst {
out[k] = v.DeepCopy()
}
for k, v := range src {
if cur, ok := out[k]; !ok || v.Cmp(cur) > 0 {
out[k] = v.DeepCopy()
}
}
return out
}
@@ -22,7 +22,9 @@ import (
"testing"
helmv2 "github.com/fluxcd/helm-controller/api/v2"
corev1 "k8s.io/api/core/v1"
apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -79,6 +81,9 @@ func newTenantREST(t *testing.T, objects ...client.Object) *REST {
if err := helmv2.AddToScheme(scheme); err != nil {
t.Fatalf("register helmv2 scheme: %v", err)
}
if err := corev1.AddToScheme(scheme); err != nil {
t.Fatalf("register corev1 scheme: %v", err)
}
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build()
return &REST{
c: fakeClient,
@@ -116,6 +121,37 @@ func childApplication(t *testing.T, name, namespace string, quotas map[string]st
}
}
// usageQuota builds a ResourceQuota reporting current usage (status.used) in a
// namespace, keyed in the rendered quota key space (e.g. "limits.cpu").
func usageQuota(t *testing.T, namespace, name string, used map[string]string) *corev1.ResourceQuota {
t.Helper()
rl := corev1.ResourceList{}
for k, v := range used {
rl[corev1.ResourceName(k)] = resource.MustParse(v)
}
return &corev1.ResourceQuota{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
Status: corev1.ResourceQuotaStatus{Used: rl},
}
}
func TestRenderedLimitKey(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",
"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)
}
}
}
func TestValidateTenantResourceQuotas(t *testing.T) {
tests := []struct {
name string
@@ -265,6 +301,54 @@ func TestValidateTenantResourceQuotas_UpdateExcludesSelf(t *testing.T) {
}
}
// TestValidateTenantResourceQuotas_PoolUsage verifies that the parent pool's
// current usage is carved out of the budget available to a new child, so the
// sub-tree invariant holds at declaration time.
func TestValidateTenantResourceQuotas_PoolUsage(t *testing.T) {
// foo budget cpu=10, already using 8 in its own namespace. A child asking
// for 10 must be denied (only 2 left); a child asking for 2 is allowed.
t.Run("parent usage shrinks remaining", func(t *testing.T) {
parent := tenantHelmRelease(t, "foo", "tenant-root", map[string]string{"cpu": "10"})
used := usageQuota(t, "tenant-foo", "tenant-quota", map[string]string{"limits.cpu": "8"})
r := newTenantREST(t, parent, used)
denied := r.validateTenantResourceQuotas(context.Background(), childApplication(t, "bar", "tenant-foo", map[string]string{"cpu": "10"}))
if len(denied) == 0 {
t.Fatalf("child cpu=10 should be denied when parent pool already uses 8 of 10")
}
allowed := r.validateTenantResourceQuotas(context.Background(), childApplication(t, "bar", "tenant-foo", map[string]string{"cpu": "2"}))
if len(allowed) > 0 {
t.Fatalf("child cpu=2 should be allowed (2 left), got: %v", allowed)
}
})
// Usage in a bounded child's own namespace (its own pool) must not count
// against the parent pool's remaining budget.
t.Run("bounded child usage excluded from parent pool", func(t *testing.T) {
parent := tenantHelmRelease(t, "foo", "tenant-root", map[string]string{"cpu": "10"})
child := tenantHelmRelease(t, "bar", "tenant-foo", map[string]string{"cpu": "4"})
// bar uses its full 4 in its own namespace.
childUsed := usageQuota(t, "tenant-foo-bar", "tenant-quota", map[string]string{"limits.cpu": "4"})
r := newTenantREST(t, parent, child, childUsed)
// Raising bar to 9 must still be allowed: bar's own usage is in its own
// pool, and as the tenant being updated it is excluded anyway.
if errs := r.validateTenantResourceQuotas(context.Background(), childApplication(t, "bar", "tenant-foo", map[string]string{"cpu": "9"})); len(errs) > 0 {
t.Fatalf("raising own quota within budget must be allowed, 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) {
parent := tenantHelmRelease(t, "foo", "tenant-root", map[string]string{"cpu": "10"})
r := newTenantREST(t, parent)
if errs := r.validateTenantResourceQuotas(context.Background(), childApplication(t, "bar", "tenant-foo", map[string]string{"cpu": "10"})); len(errs) > 0 {
t.Fatalf("with no recorded usage a full-budget child must be allowed, got: %v", errs)
}
})
}
func TestParentTenantHelmReleaseRef(t *testing.T) {
tests := []struct {
owned string