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

Helm chart: added configuration of uwsgi using environment variables #2045

Merged
merged 2 commits into from
May 29, 2023
Merged

Helm chart: added configuration of uwsgi using environment variables #2045

merged 2 commits into from
May 29, 2023

Conversation

alexintech
Copy link
Contributor

@alexintech alexintech commented May 29, 2023

What this PR does

Adds uwsgi configuration to helm chart.
Sets environment variables with name capitalized and prefixed with UWSGI_, and dashes are substituted with underscores like described here

Sets UWSGI_LISTEN=1024 by default, but can be overwritten or completely removed by uwsgi: null

Or is it better to not specify default value (it's not backward compatible)?

Also, small indentation fixes for postgresql configuration.

Which issue(s) this PR fixes

closes #562

Also, this PR has been closed because of this PR

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • CHANGELOG.md updated (or pr:no changelog PR label added if not required)

@alexintech alexintech requested a review from a team May 29, 2023 12:33
Copy link
Contributor

@joeyorlando joeyorlando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! thanks for your continued contribution to improving the Helm charts @alexintech

@joeyorlando joeyorlando enabled auto-merge May 29, 2023 17:32
@joeyorlando joeyorlando added this pull request to the merge queue May 29, 2023
Merged via the queue into grafana:dev with commit 8049b50 May 29, 2023
@alexintech alexintech deleted the helm-uwsgi-settings branch May 30, 2023 05:21
@jrossBah
Copy link

@alexintech Big question for ya. I am currently working with getting grafana oncall implemented and we had to turn this setting down to 128 to get it to work in our kubernetes cluster. I am wondering why you default set it to 1024. As We understand it the container the chart spins up defaults to 128. We found this by changing it on the ec2 instance and still seeing the error until we lowered this new uwsgi setting.

Do you know is there a way in these values that changes the container default? Thank you!

@alexintech
Copy link
Contributor Author

@alexintech I am wondering why you default set it to 1024.

Do you know is there a way in these values that changes the container default? Thank you!

@jrossBah That's not my decision. 1024 has been hardcoded in helm chart. I only made this value configurable. To maintain the same behavior default value remained the same.

@jrossBah
Copy link

jrossBah commented Dec 4, 2023

@alexintech I am wondering why you default set it to 1024.
Do you know is there a way in these values that changes the container default? Thank you!

@jrossBah That's not my decision. 1024 has been hardcoded in helm chart. I only made this value configurable. To maintain the same behavior default value remained the same.

That's so interesting. What we found is that this limit seems to conflict with the limitations of the kubernetes and also docker containers that set at 128. So unless you bring this down to 128 or under the engine pods will fail.

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.

Non-functional deployment with helm chart
3 participants