ephemeral storage resources

[beorereleverion]

this PR resolve #361 and #310

[hashicorp-cla]

All committers have signed the CLA.

[beorereleverion]

Hello. Can anyone review this pr?

[apolubotko]

Hi,

We have been waiting for this change for a very long time, we will be very happy to see it in the master branch

[beorereleverion]

@tvoran @swenson Hello, can you review this pr? please

[tomhjp]

Thanks for implementing this! I gave it a test run and it all worked really nicely, with just one tiny bug where the flags specifying the defaults weren't being threaded through. I also added a little changelog note, just to save a trivial review round.

[Kyslik]

This seems to be breaking change - that's not clearly communicated :(. Can you also include a note in the docs that you have/should to specify resources.limits.ephemeral-storage for all containers in the pod otherwise you are risking to have your pods evicted; more in https://github.com/kubernetes/kubernetes/blob/v1.25.0/pkg/kubelet/eviction/eviction_manager.go#L453-L479

Although you can only specify requests and limits for individual containers, it is also useful to think about the overall resource requests and limits for a Pod. For a particular resource, a Pod resource request/limit is the sum of the resource requests/limits of that type for each container in the Pod.

From https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#setting-requests-and-limits-for-local-ephemeral-storage

---

Using ENV variable (AGENT_INJECT_EPHEMERAL_LIMIT) one can not disable ephemeral-storage limits either, because how the checks for default are implemented. The only option is to use annotation vault.hashicorp.com/agent-limits-ephemeral which is not ideal, because you have to update all your deployments...

[tomhjp]

Thanks @Kyslik, apologies for the inconvenience. I think #386 should fix this by defaulting the ephemeral storage resources to unset, so it will then only be an explicitly opt-in feature. Feel free to leave any comments you have in the review.

[Kyslik]

Thanks @Kyslik, apologies for the inconvenience. I think #386 should fix this by defaulting the ephemeral storage resources to unset, so it will then only be an explicitly opt-in feature. Feel free to leave any comments you have in the review.

No worries; we learned something new here :). Thank you.