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

Add --max-fd argument to uwsgi to stop it from getting OOMKilled in Kubernetes #9564

Closed
wants to merge 12 commits into from

Conversation

hoeg
Copy link
Contributor

@hoeg hoeg commented Feb 16, 2024

Description

This PR fixes the issue described in issue #9562 regarding uWSGI that under some circumstances will take up an unnecessary amount of resources on a kubernetes node leading to the pod getting OOMKilled.

We introduce the possibility to set the --max-fd argument when starting up uWSGI to mitigate this issue.

Test results

I have tested the fix on a kubernetes cluster where it prevented the pod from getting OOMKilled. For more information see #9562.

Documentation

It is not clear to me where the documentation should be updated.

Copy link

dryrunsecurity bot commented Feb 16, 2024

Contextual Security Analysis

As DryRun Security performs checks, we’ll summarize them here. You can always dive into the detailed results in the section below for checks.

Status DryRun Security Check
Sensitive Functions Analyzer
Configured Sensitive Files Analyzer
Sensitive Files Analyzer

Chat with your AI-powered Security Buddy by typing @dryrunsecurity followed by your question into a comment.
Example: @dryrunsecurity What are common security issues with web application cookies?

Install and configure more repositories at DryRun Security

@dsever
Copy link
Contributor

dsever commented Feb 16, 2024

Please create PR against dev or bugfix

@hoeg hoeg changed the base branch from master to bugfix February 16, 2024 12:07
Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

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

Approved

helm/defectdojo/Chart.yaml Outdated Show resolved Hide resolved
docker/entrypoint-uwsgi.sh Outdated Show resolved Hide resolved
Copy link

dryrunsecurity bot commented Mar 1, 2024

Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and findings.

DryRun Security Status Findings
Sensitive Functions Analyzer 0 findings
Configured Sensitive Files Analyzer 4 findings
Sensitive Files Analyzer 1 findings

Note

🔴 Risk threshold exceeded. Adding a reviewer if one is configured in .dryrunsecurity.yaml.

notification list: @mtesauro @grendel513

Tip

Get answers to your security questions. Add a comment in this PR starting with @DryRunSecurity. For example...

@dryrunsecurity What are common security issues with web application cookies?

Powered by DryRun Security

@hoeg hoeg requested a review from Maffooch March 1, 2024 08:37
@Maffooch
Copy link
Contributor

Maffooch commented Mar 1, 2024

@hoeg thanks for updating your commits. It think this will work! There is not a some extra changes unrelated to your file descriptor changes that should not be here. Once those are removed, I think this will be good to go :)

Copy link
Contributor

github-actions bot commented Mar 3, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Contributor

github-actions bot commented Mar 3, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

@mtesauro
Copy link
Contributor

Closing this PR as the parent issue was addressed by changing the k8s config rather then a code change on the DD side.

@mtesauro mtesauro closed this May 15, 2024
@tmablunar
Copy link
Contributor

tmablunar commented Jun 6, 2024

I am trying to pick this is this issue up from my colleague @hoeg.

The described solution in k8s does not resolve the issue for us. We do not have any resource limits and requesting 4096Mi of memory. The pod is still OOMKilled. However, using the suggested solution in this PR solves the problem.

@tmablunar tmablunar mentioned this pull request Jun 10, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants