Helm operator CPU utilization during inactivity is high

[resinten]

Bug Report

What did you do?
I created a Helm operator to manage deployments of custom objects. To reduce the number of operator Docker images required, I put multiple CRDs in the Helm operator and added each to the watches.yaml.

What did you expect to see?
A controller pod that sleeps and uses little resources when changes are not being applied to the system.

What did you see instead? Under which circumstances?
It has been 48+ hours since the last object modification on the system (over the weekend). But when I run kubectl top (pods|nodes) I see that the Helm operator pod is using around 10% CPU utilization. While not extremely high, this is during a period of complete inactivity, which is making me think that it may be excessively polling the Kubernetes API for object changes. I would expect during inactivity that the resource usage would be minimal. RAM usage is quite low.

Environment

• operator-sdk version:

Git SHA: b1d5e62bcf750515d5b34fe9705844217373a5cf

• Kubernetes version information:

Client Version: version.Info{Major:"1", Minor:"13", GitVersion:"v1.13.2", GitCommit:"cff46ab41ff0bb44d8584413b598ad8360ec1def", GitTreeState:"clean", BuildDate:"2019-01-13T23:15:13Z", GoVersion:"go1.11.4", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"11+", GitVersion:"v1.11.5-eks-6bad6d", GitCommit:"6bad6d9c768dc0864dab48a11653aa53b5a47043", GitTreeState:"clean", BuildDate:"2018-12-06T23:13:14Z", GoVersion:"go1.10.3", Compiler:"gc", Platform:"linux/amd64"}

• Kubernetes cluster kind:

• Are you writing your operator in ansible, helm, or go?

Helm

Additional context
We have 4 different CRDs managed by this operator. 3 have only one object present, and the 4th only has 2. This makes me worry about how this will scale to creating more CRDs in the future.

[joelanford]

@BrianMWest what is the version of the helm-operator in your project's Dockerfile?

[resinten]

quay.io/operator-framework/helm-operator:v0.4.0

[joelanford]

@BrianMWest Can you try bumping to v0.5.0 and using the --reconcile-period flag? In v0.4.0, we didn't watch the resources created by the Helm release, so we had to have a tight reconcile period (60s) to ensure underlying resources didn't drift from the ones specified by the Helm release.

In v0.5.0, we started watching the chart resources. With that, you should be able to use the --reconcile-period flag to back that off quite a bit.

For now, that flag applies to all the watches in watches.yaml, but we plan to make that configurable per watch in the future. If you need different reconcile periods for each watch right now, you'll have to use multiple operators

[resinten]

@joelanford I changes the deployment object to include the following under the container spec:

command: ["/usr/local/bin/entrypoint"]
args: ["--reconcile-period=180s"]

based on what I saw from doing a docker inspect on the image and my understanding of what you were saying. I'm not seeing any improvement on the CPU usage, though. It seems to hovering around the same spot that it was before after about 5 minutes.

I didn't see any documentation on the arguments/flags available for the image. Is there somewhere where that documentation resides?

[joelanford]

Yes, it's in our SDK CLI reference here (admittedly not the most obvious place): https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#helm

Is there anything interesting in the operator logs? If none of the CRs are changing, I wouldn't expect any logging other than a "Reconciled release" line for each CR every reconcile-period seconds.

[resinten]

@joelanford it seems to be a very noisy logger, but I think I see why. One of the operators spins up an ephemeral DB container in the same pod. The Helm chart randomly generates a password for the two to communicate so that redeploys will automatically rotate passwords. Based on the log diffs and issue #1069, it seems that it is looping over that randomly generated field and constantly redeploying. I'm noticing another thing that may be similarly related when I'm trying to mess with some horizontal pod autoscaling. I'm not positive, but it seems like the scaling from the HPA is being reset whenever the Helm operator notices the deployment object is different than what the template says it should be. Thoughts?

[resinten]

Regarding the HPA, I've confirmed that when I turn off the Helm operator, the HPA functions properly and my deployment scaled up to 3 pods during a stress test. When I turn it back on, it seems to overwrite any changes made by the HPA.

Regarding the randomly generated passwords, is there a way that I can set the random seed to be based on the timestamp of the original object creation so that subsequent executions of the template will yield the same password until the custom resource object is modified?

[joelanford]

is there a way that I can set the random seed to be based on the timestamp of the original object creation so that subsequent executions of the template will yield the same password until the custom resource object is modified?

That's an interesting idea. I'll look into that.

In the meantime, the best way to handle that situation with the current Helm operator is to setup the Helm chart to allow you to specify an existing Secret that can be used to inject the credentials into the containers. You can also override that particular field as a value in the CR, but that wouldn't be as secure as using a Secret since anyone with read access to the CR or the underlying pod objects would be able to see the credentials.

I'll have to think a bit about the best way to handle the HPA interaction. The Helm operator works by constantly reconciling the set of resources passed to it in the chart, so it makes sense that it keeps changing back the Deployment's spec.replicas field when the HPA updates it.

I haven't tried this, but one possible fix may be to remove spec.replicas from your Deployment template so that the operator doesn't try to reconcile that field.

[resinten]

I will try removing the replicas spec for autoscaled objects and report back

[resinten]

@joelanford Removing the replicas spec worked! So, if anyone else encountered the HPA issue with the helm operator, the solution is that autoscaled deployments should not set the replicas in the Deployment. Our solution is to have the spec contain autoscaled: true|false and if the CR is autoscaled, then use the provided value for replicas as minReplicas instead.

At this point, I think we are well beyond the scope of the original issue, which was the CPU usage of the operator. In summary, the root cause is the use of a randomly generated password string for a state DB in one of the CR templates. The temporary fix is to remove the randomly generated portions from the template. A longer term fix might be to either have the operator set the random seed deterministically based on a milliseconds timestamp of when the CR was created.

[joelanford]

@BrianMWest I looked into setting the seed, and it doesn't seem possible without re-implementing (and reducing the security of) the template functions that rely on randomly generated data.

Helm uses template functions from sprig, which uses crypto/rand to generate randomness. crypto/rand reads from the environment's cryptographically secure random source, so it isn't possible to set a seed.

The only solution I can see is to re-implement the sprig functions that require randomness using non-cryptographically secure random sources, which seems like a bad idea from a security and maintenance perspective.

So I think the best solutions are to either override the random value by setting it in the CR spec or to use a secret reference in the CR spec that points to an existing secret managed outside of the CR's release.