Allow setting CPU and memory limits for the identity and proxy-injector services through Helm values

[dtaskai]

What problem are you trying to solve?

The only way to set CPU and memory limits for the identity and the proxy-injector services is by hand (or post rendering the manifests through kustomize).

How should the problem be solved?

Add an option to configure CPU and memory limits for the aforementioned services through Helm values.

Any alternatives you've considered?

Manually adding limits and post rendering are the 2 alternatives that I currently know of.

How would users interact with this feature?

Users will be able to set the limits either through setting the values when installing Linkerd with Helm or by providing the values file to the Helm CLI.

Would you like to work on this feature?

yes

[olix0r]

The only way to set CPU and memory limits for the identity and the proxy-injector services is by hand

I don't think that's correct:

https://github.com/linkerd/linkerd2/blob/fe29318313005a2ebfec8d4638ec9b633b896b87/charts/linkerd-control-plane/templates/identity.yaml#L177-L179

https://github.com/linkerd/linkerd2/blob/fe29318313005a2ebfec8d4638ec9b633b896b87/charts/linkerd-control-plane/templates/proxy-injector.yaml#L95-L97

[dtaskai]

Thank you very much for the input @olix0r, sorry for not catching that earlier, I've looked through the possible values on the ArtifactHub page, but couldn't find those options, perhaps that could be updated :stuck_out_tongue: If that is a separate issue then feel free to close this one!

[adleong]

@dtaskai you're right, those values seem to be missing from the documentation. I think this is probably because of the way that the default values are commented out here: https://github.com/linkerd/linkerd2/blob/main/charts/linkerd-control-plane/values.yaml#L292

I think we could likely fix this by uncommenting the value but giving it a an empty default value like {}.

[dtaskai]

Submitted a fix which should align with the already present options :stuck_out_tongue: #8954

[olix0r]

Unfortunately we had to revert the change because it broke falling back to the default control plane resources. We should have caught this in CI, but the workflow was misconfigured (fixed in #9003).

As we fix this, I would love to remove the identityResources, identityProxyResources, etc configurations in favor of identity.resources and identity.proxy.resources to be consistent with our other configurations.

[dtaskai]

That seems more sustainable in the long run, we ran into the issue with 2.10.2 (because of an AKS issue on the clients cluster), so we'll probably be still configuring it the "old" way.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.