Use shared informers #8796
Use shared informers #8796
Conversation
The updater setup multiple informers for the same resources, so my assumption is that this will be more optimal and consume less RAM
k8s-ci-robot
added
release-note
k8s-ci-robot
added
area/vertical-pod-autoscaler
and removed
do-not-merge/needs-area
labels
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adrianmoisey The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
| vpaClient := vpa_clientset.NewForConfigOrDie(config) | ||
| vpaLister := vpa_api_util.NewVpasLister(vpaClient, make(chan struct{}), commonFlags.VpaObjectNamespace) | ||
| kubeClient := kube_client.NewForConfigOrDie(config) | ||
| factory := informers.NewSharedInformerFactory(kubeClient, defaultResyncPeriod) |
There was a problem hiding this comment.
I'm wondering if this factory should be passed the WithNamespace(commonFlags.VpaObjectNamespace) option
There was a problem hiding this comment.
Thinking about this more, I think it does make sense to pass the namespace in to the informer, however, I'm nervous for a change like this without e2e tests.
So I think I'll first make e2e tests to test the namespace command line args before I proceed with this.
| targetSelectorFetcher := target.NewVpaTargetSelectorFetcher(config, kubeClient, factory) | ||
| controllerFetcher := controllerfetcher.NewControllerFetcher(config, kubeClient, factory, scaleCacheEntryFreshnessTime, scaleCacheEntryLifetime, scaleCacheEntryJitterFactor) | ||
|
|
||
| kubeFactory := informers.NewSharedInformerFactory(kubeClient, defaultResyncPeriod) |
There was a problem hiding this comment.
I'm wondering if this factory should be passed the WithNamespace(commonFlags.VpaObjectNamespace) option
k8s-ci-robot
added
the
do-not-merge/work-in-progress
3 participants
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
The updater setup multiple informers for the same resources, so my assumption is that this change will be more optimal and consume less RAM, since it's using a shared informer.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: