Scope kube informer caches to operator-generated objects; make informer resync configurable - #2058
Draft
gregakinman wants to merge 2 commits into
Draft
Scope kube informer caches to operator-generated objects; make informer resync configurable#2058gregakinman wants to merge 2 commits into
gregakinman wants to merge 2 commits into
Conversation
gregakinman
marked this pull request as draft
August 5, 2026 19:32
…gurable The kube informer factory watched every Pod, Service, EndpointSlice, ConfigMap and StatefulSet in the cluster whenever watchNamespaces is anything other than a single literal namespace name, since GetInformerNamespace() falls back to NamespaceAll. Filtering happened client-side in isTrackedObject, i.e. after every object in the cluster had already been transferred, decoded and queued in the DeltaFIFO, so informer memory scaled with total cluster size rather than with the number of managed installations. Push that filter server-side via WithTweakListOptions. The selector is built from the same labeler that IsCHOPGeneratedObject() uses, so the server-side and client-side predicates cannot diverge. Scoping the cache also bounds the cost of resync, which replays cached objects through the handlers as sync deltas. Expose the resync periods as flags and env vars so operators can trade drift-detection latency against that cost. Defaults are unchanged at 60s, and 0 still disables resync. CHOp custom resource informers are intentionally left unfiltered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…period
Two remaining cluster-wide informers, same root cause as the kube informer
factory: GetCacheNamespaces() and GetInformerNamespace() both fall back to
NamespaceAll whenever a watch namespace is a regexp.
Owns(&apps.StatefulSet{}) starts a StatefulSet informer unconditionally, for
every operator, whether or not any CHK exists - so every operator cached every
StatefulSet in the cluster, full pod template spec each. Scope that cache to
CHK-generated objects. CHK objects carry the clickhouse-keeper API group's app
label, a different key from the CHI one, so this needs its own selector; a label
selector cannot express a disjunction over two keys. Safe because the only
cached-client StatefulSet reads are List calls that already carry this very
selector, built from the same labeler in the CHK discoverer, while point Gets go
through the non-cached APIReader.
The CHK watcher had its own hardcoded 60s resync with no way to change it. It
watches a custom resource, so hand it the CHOp custom resource resync period
instead, which makes it honor the flag and env var added alongside.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gregakinman
force-pushed
the
informer-scope-fix
branch
from
August 5, 2026 19:35
2f1a148 to
8b1e200
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The operator's informer memory footprint scales with total cluster size instead of with the number of ClickHouse installations it manages. On a large cluster with many ephemeral namespaces this OOM-kills the operator.
GetInformerNamespace()returnsmeta.NamespaceAllwheneverwatchNamespacescontains anything other than exactly one literal DNS label, so any regexp watch config forces cluster-wide informers on Pods, Services, EndpointSlices, ConfigMaps and StatefulSets. Filtering then happens client-side inisTrackedObject, i.e. after every object in the cluster has already been transferred, decoded and queued in the DeltaFIFO.Compounding it,
defaultInformerFactoryResyncPeriodwas hardcoded with no flag or env var, so the entire cluster-wide cache replayed through the DeltaFIFO as sync deltas every minute with no way to tune it.Changes
1. Push the label filter server-side.
kubeinformers.WithTweakListOptionssetsopts.LabelSelectoron the kube informer factory. The selector is built from the same labeler thatIsCHOPGeneratedObject()resolves through, so the server-side selector and the client-side predicate cannot drift apart. CHOp custom resource informers (CHI/CHIT/CHOpConfig) are intentionally left unfiltered — they are cluster-wide by necessity and small.2. Make the resync periods configurable.
--kube-informer-resync-period/OPERATOR_KUBE_INFORMER_RESYNC_PERIODand--chop-informer-resync-period/OPERATOR_CHOP_INFORMER_RESYNC_PERIOD. Precedence is flag > env > default.Defaults are deliberately left at 60s, so this change is purely additive — no existing deployment changes behavior. Scoping the cache in (1) is what actually bounds resync cost, and resync is cheaper in this operator than it first appears: the ConfigMap and StatefulSet
UpdateFuncs only log,EndpointSliceis gated onisUpdatedEndpointSliceand resync feedsold == newso it never enqueues, and CHI resync enqueues but the worker exits early onisGenerationTheSame. Only the Pod handler enqueues unconditionally, and after (1) that is bounded by the number of operator-managed pods. Raising the period trades drift-detection latency for DeltaFIFO churn, so it is exposed as a knob rather than repriced for everyone.3. Scope the keeper manager's StatefulSet cache.
Owns(&apps.StatefulSet{})starts a StatefulSet informer unconditionally, for every operator, whether or not any CHK exists — andGetCacheNamespaces()falls back toNamespaceAllunder the same regexp condition, so every operator cached every StatefulSet in the cluster, full pod template spec each.4. Remove the CHK watcher's private resync constant. It watches a custom resource, so it now receives the CHOp custom resource resync period and honors the flag and env var from (2). Its effective default is unchanged.
Equivalence
The label selector is intended to be exactly equivalent to the pre-existing client-side filtering. Verified:
kubeInformerFactoryalready gate onisTrackedObject, so no handler regresses.Lister()calls and noGetStore()/GetIndexer()reads on the kube informers anywhere in the tree — the caches are pure event sources, so nothing can read an object the selector excludes.Two things worth reviewer attention:
clickhouse.altinity.com/appvsclickhouse-keeper.altinity.com/app, and a k8s label selector cannot express a disjunction over two keys. This is fine becauseisTrackedObjectuses the CHI labeler exclusively, so CHK-generated objects were already dropped client-side; and the keeper cache gets its own separate selector. Tests assert the two selectors are mutually non-matching so this can't silently regress.Endpointsinformer is never constructed — that handler is commented out — so legacy Endpoints are moot either way.For the keeper cache, only StatefulSets are scoped. Point
Gets on StatefulSets go through the non-cachedAPIReader, and the only cached-client StatefulSet reads areLists that already carry this very selector, built from the same labeler in the CHK discoverer. The other types the CHK adapter reads through the cached client (ConfigMap, Service, Secret, PVC, PDB, Pod) are cached lazily, only once a CHK is actually reconciled, and are deliberately left alone here — I had no cluster with CHKs deployed to exercise those read paths against.Testing
go build ./...clean.go vetproduces an identical finding set before and after the change (compared in the same tree, same build cache) — no new findings.go testcurrently requires-vet=offon master due to pre-existing vet failures, andTestEnforceVerifiedLegacyTLSfails on macOS independently of this change.IsCHOPGeneratedObject()across a table of label sets, and assert the CHI and CHK selectors never match each other's objects.Measured on one production-shaped cluster with a regexp watch config, cached objects across the five watched types drop from 22,194 to 761.
🤖 Generated with Claude Code