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

feat: update resource assignment #852

Closed
wants to merge 3 commits into from

Conversation

cdobbyn
Copy link

@cdobbyn cdobbyn commented Aug 16, 2023

Which problem is this PR solving?

There are two problems this solves:

  1. There shouldn't be default resource assignments, the helm standard is to not assign but to recommend defaults as comments in the values file. The way it's configured today makes it so that by default this component uses WAY too many resources by default.
  2. Individual containers should have their resources configured separately. Again the way this works today means this component uses too much resources.

Fixes: #853, #499

Short description of the changes

  • default resource assignment to not have resource assignment
  • make it so individual containers have separate resource assignment

Type of change

  • New feature / enhancement (non-breaking change which adds functionality)

New Tests?

n/a

Checklist:

  • Add changelog entry following the contributing guide
  • Tests have been added
  • Documentation has been updated

- default resource assignment to not have resource assignment
- make it so individual containers have separate resource assignment
@CLAassistant
Copy link

CLAassistant commented Aug 16, 2023

CLA assistant check
All committers have signed the CLA.

@cdobbyn cdobbyn marked this pull request as ready for review August 16, 2023 17:25
@cdobbyn cdobbyn requested a review from a team August 16, 2023 17:25
@juanjjaramillo
Copy link
Contributor

Closing this PR after having an internal discussion on the issues that can be generated if we do not set resource limits to our integration

@cdobbyn
Copy link
Author

cdobbyn commented Oct 24, 2023

@juanjjaramillo I will open a separate PR for granular resource config (as I this is still necessary). Can I at least set the default requests lower than this? They are quite high for most clusters:
image

@cdobbyn
Copy link
Author

cdobbyn commented Oct 24, 2023

@juanjjaramillo #918

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

Successfully merging this pull request may close these issues.

[chart] Resource assignment should follow helm standards
3 participants