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

Up default Podman rlimits to avoid max open files #1437

Closed
wants to merge 2 commits into from

Conversation

mheon
Copy link
Member

@mheon mheon commented Sep 10, 2018

Every port we open consumes an open FD. This can easily consume all available FDs for the podman process. Set rlimits to resolve this.

Fixes #1357

@mheon
Copy link
Member Author

mheon commented Sep 10, 2018

@baude PTAL

@rhatdan
Copy link
Member

rhatdan commented Sep 10, 2018

LGTM @mheon Does this match up to the Docker daemon?

@mheon
Copy link
Member Author

mheon commented Sep 10, 2018

Max processes 1048576 1048576 processes

Max open files 1048576 1048576 files

From containerd in 1.13

I don't think we need the max processes thing because conmon daemonizes

if os.Geteuid() == 0 {
rlimits := new(syscall.Rlimit)
rlimits.Cur = 1048576
rlimits.Max = 1048576
Copy link
Member

Choose a reason for hiding this comment

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

I don't think so, but I'll ask anyway. Is this a value we should tuck away in a conf file?

Copy link
Member

Choose a reason for hiding this comment

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

You probably need to check if you are running in rootless mode and then not do this, since this will be blocked in a user namespace.

Copy link
Member

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

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

LGTM assuming happy tests

@baude
Copy link
Member

baude commented Sep 10, 2018

bot, retest this please

3 similar comments
@rhatdan
Copy link
Member

rhatdan commented Sep 11, 2018

bot, retest this please

@baude
Copy link
Member

baude commented Sep 11, 2018

bot, retest this please

@mheon
Copy link
Member Author

mheon commented Sep 11, 2018

bot, retest this please

@mheon
Copy link
Member Author

mheon commented Sep 12, 2018

Alright, just going to start trying Homu to see if we can get things cleared
@rh-atomic-bot r=TomSweeneyRedHat

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 229c646 has been approved by TomSweeneyRedHat

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 229c646 with merge a082570...

@rh-atomic-bot
Copy link
Collaborator

💔 Test failed - status-papr

@mheon
Copy link
Member Author

mheon commented Sep 12, 2018

@rh-atomic-bot retry

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 229c646 with merge 022921a...

@rh-atomic-bot
Copy link
Collaborator

💔 Test failed - status-papr

@mheon
Copy link
Member Author

mheon commented Sep 12, 2018

@rh-atomic-bot retry

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 229c646 with merge 389cb67...

@rh-atomic-bot
Copy link
Collaborator

💔 Test failed - status-papr

@mheon
Copy link
Member Author

mheon commented Sep 12, 2018

bot, retest this please

@mheon
Copy link
Member Author

mheon commented Sep 12, 2018

@rh-atomic-bot retry

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 229c646 with merge 88e0913...

@rh-atomic-bot
Copy link
Collaborator

💔 Test failed - status-papr

@mheon
Copy link
Member Author

mheon commented Sep 12, 2018

FAH28 is failing consistently. I'm not sure what the difference is here.

@rhatdan
Copy link
Member

rhatdan commented Sep 13, 2018

@mheon Rebase and push again, see if that clears this up.

Every port we open consumes an open FD. This can easily consume
all available FDs for the podman process. Set rlimits to resolve
this.

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
@baude
Copy link
Member

baude commented Sep 13, 2018

bot, retest this please

@mheon
Copy link
Member Author

mheon commented Sep 13, 2018

@rh-atomic-bot r=rhatdan

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 8fd984a has been approved by rhatdan

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 8fd984a with merge fbf7a60...

@rh-atomic-bot
Copy link
Collaborator

💔 Test failed - status-papr

@mheon
Copy link
Member Author

mheon commented Sep 13, 2018

It's consistently failing on FAH28, and always on rootless. I'm really not clear as to how this breaks rootless considering it has a Geteuid() gate on it

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
@rhatdan
Copy link
Member

rhatdan commented Sep 13, 2018

LGTM
@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 2148ae2 has been approved by rhatdan

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 2148ae2 with merge 8e65b29...

rh-atomic-bot pushed a commit that referenced this pull request Sep 13, 2018
Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

Closes: #1437
Approved by: rhatdan
@mheon
Copy link
Member Author

mheon commented Sep 13, 2018

@rh-atomic-bot r=rhatdan

@rh-atomic-bot
Copy link
Collaborator

💡 This pull request was already approved, no need to approve it again.

  • This pull request is currently being tested. If there's no response from the continuous integration service, you may use retry to trigger a build again.

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 2148ae2 has been approved by rhatdan

@rh-atomic-bot
Copy link
Collaborator

☀️ Test successful - status-papr
Approved by: rhatdan
Pushing 8e65b29 to master...

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 27, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman: publishing huge amount of port requires modification of ulimit
5 participants