Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource constraints are not enforced on all the containers #637

Closed
tirumerla opened this issue Dec 5, 2020 · 3 comments
Closed

Resource constraints are not enforced on all the containers #637

tirumerla opened this issue Dec 5, 2020 · 3 comments

Comments

@tirumerla
Copy link
Contributor

Is your feature request related to a problem?/Why is this needed

  • When you enforce resource constraints on specific namespace. Deployment of aws-ebs-csi-driver using helm chart throws the error since resources is only specified for ebs-plugin container but not for csi-* containers in controller.yaml and none of the containers in node.yaml.

/feature

Describe the solution you'd like in detail

  • Add resources block for all the containers in controller.yaml and in node.yaml reading the resources from values file.
{{- with .Values.resources }}
resources: {{ toYaml . | nindent 12 }}
{{- end }}

Describe alternatives you've considered

  • Downloaded & Extracted helm chart -> changed yaml files controller.yaml and node.yaml to have resource blocks for all the containers -> redeployed with the changes.

Additional context

  • k8s version: EKS v1.17.11
  • ebs-csi helm chart: 0.7.1

Happy to raise PR :)

Thanks

@wongma7
Copy link
Contributor

wongma7 commented Dec 7, 2020

PR welcome!

I would be OK with just templating that same .Values.resources to all containers . Some of the sidecars with their own informers might use a lot of memory depending of # of objects in cluster so it might make sense to have per container values. But probably that'll make the chart too complicatged and we can cross that bridge when we get to it...

@tirumerla
Copy link
Contributor Author

PR welcome!

I would be OK with just templating that same .Values.resources to all containers . Some of the sidecars with their own informers might use a lot of memory depending of # of objects in cluster so it might make sense to have per container values. But probably that'll make the chart too complicatged and we can cross that bridge when we get to it...

Thanks @wongma7 this makes sense. Raised PR :)

@tirumerla
Copy link
Contributor Author

Fixed with #640

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants