operator-framework / helm-operator-plugins

Experimental refactoring of the operator-framework's helm operator
Apache License 2.0
49 stars 49 forks source link

Make it possible to use a different `*rest.Config` based on the content of the reconciled object #316

Closed joelanford closed 5 months ago

joelanford commented 6 months ago

I have a new use case, where I'd like to use helm-operator-plugins to manage the lifecycle of arbitrary helm charts. In order to avoid the controller requiring cluster-admin permissions (which would be necessary to truly handle arbitrary helm charts), I want to require a service account to be provided alongside the helm chart so that the controller can get a short-lived token for the service account and manage the chart lifecycle with that service account.

Another potential use case here is managing chart installations in a separate cluster from the one running the controller and deciding which cluster based on the spec of the object being reconciled.

In order to make this possible, it seems like our prior refactoring of the actionConfigGetter to fix a memory leak needs to change such that the *rest.Config can be derived at reconciliation time and then all the clients can be created using that *rest.Config.

That mostly amounts to a revert of that PR, but it begs the question: can we revert that PR and still avoid the memory leak that PR fixed?

joelanford commented 6 months ago

cc @misberner

joelanford commented 6 months ago

For what it's worth, I updated my controller to use the code in #317 and forced it to continuously reconcile and didn't see the memory growing. So hopefully something subtle changed here or in upstream code somewhere in the meantime.