Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kube: unify clients into single RESTClientGetter #630

Merged
merged 2 commits into from
Mar 7, 2023
Merged

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented Mar 6, 2023

This drops the twofold implementation in favor of a single
MemoryRESTClientGetter which can work with an arbitrary rest.Config.

The new MemoryRESTClientGetter lazy-loads and caches the objects it
initializes, thereby creating at most one instance of each object for
the duration of the reconcile of a single HelmRelease object.

Based on some initial tests, this seems to reduce the overal memory
footprint of the controller.

🚀 Release candidate: ghcr.io/fluxcd/helm-controller:rc-b68592c4

Memory usage before

Before screenshot

Memory usage after

After screenshot

@hiddeco hiddeco force-pushed the kube-improvements branch 4 times, most recently from fc281bf to 247ba12 Compare March 6, 2023 10:19
This drops the twofold implementation in favor of a single
`MemoryRESTClientGetter` which can work with an arbitrary `rest.Config`.

The new `MemoryRESTClientGetter` lazy-loads and caches the objects it
initializes, thereby creating at most one instance of each object for
the duration of the reconcile of a single `HelmRelease` object.

Based on some initial tests, this seems to reduce the overal memory
footprint of the controller.

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
As otherwise with a persistent discovery client and/or REST mapper
configuration, newly installed CRDs will not be recognized and cause a
`resource mapping not found for name` error.

In addition, remove the `ServerGroups` and `Invalidate` calls. As this
is later done (again) by Helm when gathering server capabilities.

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
@hiddeco hiddeco added enhancement New feature or request area/helm Helm related issues and pull requests labels Mar 6, 2023
@hiddeco hiddeco marked this pull request as ready for review March 6, 2023 16:29
@hiddeco hiddeco requested review from stefanprodan and pjbgf and removed request for stefanprodan March 6, 2023 16:41
Copy link
Member

@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really good stuff, @hiddeco! 👏

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks @hiddeco 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Helm related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants