Skip to content

Conversation

Lucaber
Copy link
Contributor

@Lucaber Lucaber commented Mar 10, 2023

Description of the change:

Helm stores its state in secrets inside the cluster.
Instead of listing these secrets before every reconciliation of every release, we use an informer to query a local secrets list.

Whats the current status of the helm-operator-plugins repo, should I also create a MR there?

Motivation for the change:

We are running 2 helm-operators with 50 CRs each in a namespace with over 1000 secrets.
Listing these secrets, even with a filter, takes more than 1 second.
Running multiple of these list queries in parallel generates a high load on the kubernetes apiserver and etcd.
Combined with the default reconcile period of 1m this results in a constant high load.

Before and After metrics (after installing this MR on the first and second helm-operator in the cluster):
image
image

This issue was primarily noticed due to a very high load and network traffic between the kubernetes etcd instances.

image
image

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:25 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:25 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:25 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:25 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:25 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:25 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:25 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:25 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:25 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:28 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:28 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:28 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:33 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:33 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:34 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:34 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:34 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 13:35 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 14:03 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 14:03 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 14:03 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 14:03 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 14:03 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 14:03 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 14:03 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 10, 2023 14:03 — with GitHub Actions Inactive
@Lucaber
Copy link
Contributor Author

Lucaber commented Mar 21, 2023

In helm-operator-plugins, we already have an informer for secrets because we watch secrets in the controller.

Could you point me to the file where the watch for Secrets in helm-operator-plugins is setup?

I wonder if we should watch secrets here as well. If we did that, we would need a SecretsInterface implementation that wraps mgr.GetClient() so that we reuse the underlying/existing informer and cache.

What do you think about that approach?

Yes, wrapping the SecretsInterface seams to be best option, as we don't need to copying a lot of internal code from helm itself to for example write a custom driver.

@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:42 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:42 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:42 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:42 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:42 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:42 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:42 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:42 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:42 — with GitHub Actions Inactive
Helm stores its state in secrets inside the cluster. Instead of listing these secrets before every reconciliation of every release, we use an informer to query a local secrets list. This significantly reduced the load on the kubernetes apiserver and etcd Signed-off-by: Luca Berneking <l.berneking@mittwald.de>
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:52 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:52 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:52 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:52 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:52 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:52 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:52 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:52 — with GitHub Actions Inactive
@Lucaber Lucaber temporarily deployed to deploy March 21, 2023 14:53 — with GitHub Actions Inactive
@Lucaber Lucaber requested review from everettraven and removed request for joelanford March 21, 2023 15:34
@Lucaber
Copy link
Contributor Author

Lucaber commented May 3, 2023

Any new updates? Somehow @joelanford was automatically removed as a reviewer when "re-requesting" the review.

Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

@Lucaber Thanks for the contribution! Apologies for this PR getting lost in the weeds of other PRs. I'll try to get this pushed through.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 21, 2023
@everettraven everettraven merged commit cb83a61 into operator-framework:master Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged.

3 participants