Re-implement CPU Resource Limits for all Containers

[BJKupka]

Is your feature request related to a problem?/Why is this needed
I noticed that the CPU limits were removed from all of the various aws-ebs-csi-driver containers when issue #1591 was resolved.

My team decided to leave the CPU limits in the controller and node configuration that we maintain. We felt that it was unwise to give these containers free reign to go rogue and potentially overdraw CPU in the event that an unintended consequence arises with respect to a future code change.

Our current resource configuration per each container:

          resources:
            limits:
              cpu: 100m
              memory: 256Mi
            requests:
              cpu: 10m
              memory: 40Mi

Note: We've been running these configurations in our production environment for some time and haven't seen any issues. However, we weren't certain if the CPU limit should be increased from 100 millicores to XXX millicores (maybe 250 to be safe?).

Is there a particular reason why these containers should not have CPU limits applied? I was unsure as to exactly why the limits were removed in this PR when the linked issue stemmed from the liveness check getting a bit rowdy.

/feature

Describe the solution you'd like in detail
Appropriate CPU resource limits are applied to all containers.

Describe alternatives you've considered
Keeping what is currently in place.

Additional context
N/A

[torredil]

Hi @BJKupka thank you for this request.

Is there a particular reason why these containers should not have CPU limits applied?

The reason CPU limits were removed from the manifests is because defining CPU limits is generally considered an anti-pattern, not a best practice. See this article, which goes over it in more detail: https://home.robusta.dev/blog/stop-using-cpu-limits.

Describe the solution you'd like in detail
Appropriate CPU resource limits are applied to all containers.

Based on discussions with other maintainers / contributors - we are not inclined to define default CPU limits on behalf of users. However, while not recommended, you still have the flexibility of defining CPU limits via the exposed resources parameter, at your discretion.

aws-ebs-csi-driver/charts/aws-ebs-csi-driver/values.yaml

Line 257 in d684d4d

resources:

Do let us know if you run into any trouble in configuring the above, need any further help, or have additional requests for us.

/close

[k8s-ci-robot]

@torredil: Closing this issue.

In response to this:

Hi @BJKupka thank you for this request.

Is there a particular reason why these containers should not have CPU limits applied?

The reason CPU limits were removed from the manifests is because defining CPU limits is generally considered an anti-pattern, not a best practice. See this article, which goes over it in more detail: https://home.robusta.dev/blog/stop-using-cpu-limits.

Describe the solution you'd like in detail
Appropriate CPU resource limits are applied to all containers.

Based on discussions with other maintainers / contributors - we are not inclined to define default CPU limits on behalf of users. However, while not recommended, you still have the flexibility of defining CPU limits via the exposed resources parameter, at your discretion.

aws-ebs-csi-driver/charts/aws-ebs-csi-driver/values.yaml

Line 257 in d684d4d

resources:

Do let us know if you run into any trouble in configuring the above, need any further help, or have additional requests for us.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[BJKupka]

@torredil Thanks for sharing.

I will bring it back to the team at large.