Skip to content

Commit ed3db60

Browse files
perf(lineage): cache owner GETs and short-circuit re-admission
The admission webhook walks the owner chain (Pod → STS → HelmRelease → app CR) using uncached dynamic GETs against the apiserver. Each level costs a full RT, so under apiserver load a single Pod admission spent 300-500ms p99 in the lineage handler — making it the top admission-latency contributor in clusters with many tenants. Two changes: * New TTL cache in pkg/lineage/cache.go, threaded through WalkOwnershipGraph via a *walkState wrapper. Successful lookups and permanent NotFound errors are cached; transient apiserver errors (timeouts, throttling) are not, so the next admission retries instead of returning a stale failure. The cache is opt-in (nil is a valid no-op), so existing callers and the legacy variadic visited-map form keep working — and the legacy nil-map case is now defensively replaced with a usable empty map to avoid a panic on the first visited[id] = true write. Eviction is bounded: when the map outgrows evictionThreshold, Set scans only evictionSampleSize entries per call, bounding the write-lock hold time regardless of map size and amortising the work across many admissions. * Handle() returns early if the object already carries every lineage label (only for non-Pod kinds, since Pods also need applySchedulingClass on every admission). The objectSelector on the MutatingWebhookConfiguration already excludes managed objects, but a defensive check costs nothing and protects against the selector being relaxed or labels being applied by a concurrent controller mid-admission. Tests cover nil-safety, TTL expiry, negative caching, key discrimination, backwards compatibility of the variadic memory parameter including the nil legacy-map form, and that the eviction sweep stays bounded under pathological load. Signed-off-by: mattia-eleuteri <[email protected]>
1 parent e4f9a18 commit ed3db60

5 files changed

Lines changed: 493 additions & 43 deletions

File tree

‎internal/lineagecontrollerwebhook/types.go‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,30 @@ package lineagecontrollerwebhook
33
import (
44
"sync"
55
"sync/atomic"
6+
"time"
67

8+
"github.com/cozystack/cozystack/pkg/lineage"
79
"k8s.io/apimachinery/pkg/api/meta"
810
"k8s.io/apimachinery/pkg/runtime"
911
"k8s.io/client-go/dynamic"
1012
"sigs.k8s.io/controller-runtime/pkg/client"
1113
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
1214
)
1315

16+
// ownerCacheTTL controls how long a successful or negative dynamic-client GET
17+
// result is reused across admission requests. The labels we derive are based on
18+
// stable identity (kind/name/owner refs), so even a generous TTL is safe; the
19+
// dominant cost we are saving is the apiserver round-trip on every admission.
20+
const ownerCacheTTL = 60 * time.Second
21+
1422
// +kubebuilder:webhook:path=/mutate-lineage,mutating=true,failurePolicy=Fail,sideEffects=None,groups="",resources=pods,secrets,services,persistentvolumeclaims,verbs=create;update,versions=v1,name=mlineage.cozystack.io,admissionReviewVersions={v1}
1523
type LineageControllerWebhook struct {
1624
client.Client
17-
Scheme *runtime.Scheme
18-
decoder admission.Decoder
19-
dynClient dynamic.Interface
20-
mapper meta.RESTMapper
21-
config atomic.Value
22-
initOnce sync.Once
25+
Scheme *runtime.Scheme
26+
decoder admission.Decoder
27+
dynClient dynamic.Interface
28+
mapper meta.RESTMapper
29+
config atomic.Value
30+
initOnce sync.Once
31+
ownerCache *lineage.ObjectCache
2332
}

‎internal/lineagecontrollerwebhook/webhook.go‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,13 @@ func (h *LineageControllerWebhook) SetupWithManagerAsWebhook(mgr ctrl.Manager) e
7373
return err
7474
}
7575

76+
// Owner lookups dominate admission latency: each Pod admission walks the
77+
// owner chain (Pod → STS → HelmRelease → app CR) with sequential dynamic
78+
// GETs to the apiserver. The cache collapses repeat lookups across
79+
// concurrent admissions for siblings of the same parent chain. TTL is short
80+
// enough that DELETE/recreate of an owner is reflected within a minute.
81+
h.ownerCache = lineage.NewObjectCache(ownerCacheTTL)
82+
7683
h.initConfig()
7784
// Register HTTP path -> handler.
7885
mgr.GetWebhookServer().Register("/mutate-lineage", &admission.Webhook{Handler: h})
@@ -101,6 +108,17 @@ func (h *LineageControllerWebhook) Handle(ctx context.Context, req admission.Req
101108
return admission.Errored(400, fmt.Errorf("decode object: %w", err))
102109
}
103110

111+
// Fast-path: the objectSelector on the MutatingWebhookConfiguration already
112+
// skips admission for objects that carry the managed-by-cozystack label, so
113+
// in steady state this branch only triggers for races (a controller
114+
// re-applying labels mid-flight) and for safety if the selector is ever
115+
// loosened. Pods still need applySchedulingClass each admission though, so
116+
// we only short-circuit non-Pod kinds.
117+
if obj.GetKind() != "Pod" && hasAllLineageLabels(obj) {
118+
logger.V(1).Info("object already carries lineage labels, skipping owner walk")
119+
return admission.Allowed("lineage labels already present")
120+
}
121+
104122
owner, err := h.getOwner(ctx, obj)
105123
switch {
106124
case err != nil && errors.Is(err, AncestryAmbiguous):
@@ -133,11 +151,11 @@ func (h *LineageControllerWebhook) Handle(ctx context.Context, req admission.Req
133151
}
134152

135153
func (h *LineageControllerWebhook) getOwner(ctx context.Context, o *unstructured.Unstructured) (*unstructured.Unstructured, error) {
136-
owners := lineage.WalkOwnershipGraph(ctx, h.dynClient, h.mapper, h, o)
154+
owners := lineage.WalkOwnershipGraphWithCache(ctx, h.dynClient, h.mapper, h, h.ownerCache, o)
137155
if len(owners) == 0 {
138156
return nil, NoAncestors
139157
}
140-
obj, err := owners[0].GetUnstructured(ctx, h.dynClient, h.mapper)
158+
obj, err := owners[0].GetUnstructuredCached(ctx, h.dynClient, h.mapper, h.ownerCache)
141159
if err != nil {
142160
return nil, err
143161
}
@@ -201,6 +219,26 @@ func (h *LineageControllerWebhook) applyLabels(o *unstructured.Unstructured, lab
201219
o.SetLabels(existing)
202220
}
203221

222+
// hasAllLineageLabels reports whether `o` already carries every label the
223+
// webhook would set. We treat ManagedObjectKey alone as insufficient because
224+
// the "unmanaged" path writes it with value "false" without setting the rest;
225+
// only the full set proves a previous successful walk.
226+
func hasAllLineageLabels(o *unstructured.Unstructured) bool {
227+
labels := o.GetLabels()
228+
if labels == nil {
229+
return false
230+
}
231+
if labels[ManagedObjectKey] != "true" {
232+
return false
233+
}
234+
for _, k := range []string{ManagerGroupKey, ManagerKindKey, ManagerNameKey} {
235+
if _, ok := labels[k]; !ok {
236+
return false
237+
}
238+
}
239+
return true
240+
}
241+
204242
// applySchedulingClass injects schedulerName and scheduling-class annotation
205243
// into Pods whose namespace carries the scheduler.cozystack.io/scheduling-class label.
206244
// If the referenced SchedulingClass CR does not exist (e.g. the scheduler

‎pkg/lineage/cache.go‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
package lineage
2+
3+
import (
4+
"sync"
5+
"time"
6+
7+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
8+
)
9+
10+
// ObjectCache is a small TTL cache shared across WalkOwnershipGraph calls so the
11+
// admission webhook can avoid repeatedly issuing the same uncached dynamic GET to
12+
// the apiserver. A child Pod's owner (StatefulSet, HelmRelease, application CR)
13+
// is looked up on every admission of every sibling — the cache collapses those
14+
// per-process lookups to a single round-trip per TTL window.
15+
//
16+
// The cache stores both successful lookups and negative results (NotFound or
17+
// other transient errors) so a tight stream of admissions for the same parent
18+
// chain does not amplify pressure on the apiserver. Entries expire after `ttl`
19+
// from the time of insertion; staleness is acceptable because the labels we
20+
// compute are derived from immutable identity fields (kind, name, namespace,
21+
// owner refs) — not from object state that mutates frequently.
22+
//
23+
// A nil ObjectCache is a valid no-op cache: Get always misses, Set is a no-op.
24+
type ObjectCache struct {
25+
ttl time.Duration
26+
mu sync.RWMutex
27+
items map[cacheKey]cacheEntry
28+
}
29+
30+
type cacheKey struct {
31+
apiVersion string
32+
kind string
33+
namespace string
34+
name string
35+
}
36+
37+
type cacheEntry struct {
38+
obj *unstructured.Unstructured
39+
err error
40+
expiresAt time.Time
41+
}
42+
43+
// NewObjectCache returns a cache with the given TTL. A ttl of zero or less
44+
// disables caching by returning nil; callers can pass the result straight to
45+
// WalkOwnershipGraph without nil-checking.
46+
func NewObjectCache(ttl time.Duration) *ObjectCache {
47+
if ttl <= 0 {
48+
return nil
49+
}
50+
return &ObjectCache{
51+
ttl: ttl,
52+
items: make(map[cacheKey]cacheEntry),
53+
}
54+
}
55+
56+
// Get returns a cached lookup, if any. The second return value reports whether
57+
// the entry was present and unexpired. A nil receiver is safe.
58+
func (c *ObjectCache) Get(apiVersion, kind, namespace, name string) (*unstructured.Unstructured, error, bool) {
59+
if c == nil {
60+
return nil, nil, false
61+
}
62+
k := cacheKey{apiVersion, kind, namespace, name}
63+
c.mu.RLock()
64+
entry, ok := c.items[k]
65+
c.mu.RUnlock()
66+
if !ok || time.Now().After(entry.expiresAt) {
67+
return nil, nil, false
68+
}
69+
return entry.obj, entry.err, true
70+
}
71+
72+
// Set stores a lookup result. Both successful objects and errors are cached so
73+
// that repeated misses (NotFound, NoKindMatch) do not retry every admission.
74+
// A nil receiver is safe.
75+
func (c *ObjectCache) Set(apiVersion, kind, namespace, name string, obj *unstructured.Unstructured, err error) {
76+
if c == nil {
77+
return
78+
}
79+
k := cacheKey{apiVersion, kind, namespace, name}
80+
entry := cacheEntry{
81+
obj: obj,
82+
err: err,
83+
expiresAt: time.Now().Add(c.ttl),
84+
}
85+
c.mu.Lock()
86+
c.items[k] = entry
87+
// Bounded sampling eviction: when the map outgrows the threshold, scan a
88+
// fixed slice of entries (cheap O(1) amortised, ~evictionSampleSize per
89+
// Set) and drop the expired ones. This avoids the O(N) full-map scan
90+
// pattern that would otherwise hold the write lock long enough to
91+
// serialise admission webhook calls under bursty load.
92+
if len(c.items) > evictionThreshold {
93+
c.evictExpiredSampleLocked(time.Now(), evictionSampleSize)
94+
}
95+
c.mu.Unlock()
96+
}
97+
98+
// evictExpiredSampleLocked scans at most n entries and removes the expired
99+
// ones. The caller must hold c.mu. Map iteration in Go is randomised, so
100+
// repeated invocations probabilistically cover the whole map without ever
101+
// holding the lock for an unbounded amount of time.
102+
func (c *ObjectCache) evictExpiredSampleLocked(now time.Time, n int) {
103+
if n <= 0 {
104+
return
105+
}
106+
i := 0
107+
for key, e := range c.items {
108+
if now.After(e.expiresAt) {
109+
delete(c.items, key)
110+
}
111+
i++
112+
if i >= n {
113+
return
114+
}
115+
}
116+
}
117+
118+
const (
119+
// evictionThreshold is the size at which Set begins sampling for expired
120+
// entries. Set high enough that the sweep stays infrequent under steady
121+
// load, low enough that the map cannot grow unbounded under pathological
122+
// admission patterns.
123+
evictionThreshold = 4096
124+
// evictionSampleSize caps how many entries each Set inspects when
125+
// evicting. With sampleSize ≪ threshold the work is amortised across many
126+
// Sets, bounding the write-lock hold time regardless of map size.
127+
evictionSampleSize = 64
128+
)

0 commit comments

Comments
 (0)