Repository navigation
helm-operator: keep the chart label filter on dependent resource caches - #7154
Open
TarasLykhenko wants to merge 1 commit into
Open
TarasLykhenko wants to merge 1 commit into
TarasLykhenko wants to merge 1 commit into
Conversation
When watching all namespaces, configureWatchNamespaces set LabelSelector: labels.Everything() on the all-namespaces cache config. In controller-runtime that selector takes precedence over the chart-label DefaultLabelSelector set by configureSelectors, so every dependent informer cached every object of its kind in the cluster. Leave the selector unset so dependent resources fall back to the chart-label default again. Watched custom resources keep their own ByObject selector. Signed-off-by: TarasLykhenko <18290033+TarasLykhenko@users.noreply.github.com>
This branch has not been deployed
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.
Description of the change:
When the Helm operator watches all namespaces (no
WATCH_NAMESPACE),configureWatchNamespacesno longer setsLabelSelector: labels.Everything()on the all-namespaces cache config.configureSelectorssetsDefaultLabelSelectortohelm.sdk.operatorframework.io/chart in (<chart names>), so the cache holds only the objects the operator created. That filter was added in #6377. In controller-runtime's cache defaulting, a label selector on aDefaultNamespacesentry takes precedence overDefaultLabelSelector(defaultConfigonly fills a nil selector). Thelabels.Everything()override, added in #6664 while moving to the new cache API, therefore disables the chart filter for every dependent resource: each dependent informer caches every object of its kind in the cluster.With the selector left unset:
DefaultLabelSelectoragain;configureSelectorsgives each of them its ownByObjectselector, which takes precedence over both;DefaultNamespacesdoes not apply to them.Objects created by the operator carry the label, because
NewLabelInjectingClientadds it on every create and update.Motivation for the change:
A cluster-wide Helm operator whose chart renders common kinds (Secrets, ConfigMaps, Deployments, Services, Roles) caches all of them across the cluster, including the data of every Secret and ConfigMap. Memory then grows with the size of the cluster, not with the number of custom resources. We run one that manages a few dozen custom resources; it uses 1-3 GiB on most clusters and is OOMKilled at a 5 GiB limit on the largest one.
Testing:
TestConfigureWatchNamespacesininternal/cmd/helm-operator/run/cmd_test.gochecks that the all-namespaces config leaves the label selector unset, and that a namespace list still yields one config per namespace. The first case fails without the change.Checklist