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 rlimit configuration for programs #229

Closed
wants to merge 7 commits into from

Conversation

davidbirdsong
Copy link

Hi, I've used a hacked up supervisord for years with the options:

[program:prog1]
command=/bin/cat
minprocs=4096
minfds=4096
stacksize=10240

Which directs supervisor to call setrlimit to the values just after forking to launch the program. For RLIMITS with a soft and hard limit, I've just forced the 'min' value to both since most programs under supervisord are not equipped to call setrlimit on their own behalf.

I think this is generally useful, and I'd love to stop having to manually patch to get updates.

I ran all tests and had to run tests with root privileges to get the final tests to pas.

Let me know if I've abused any code or if any code style is not good.

@davidbirdsong
Copy link
Author

Perhaps I need to update another test somewhere? How are any of the setuid or setgrp areas of code tested without root privileges?

@davidbirdsong
Copy link
Author

Any idea on when this might get a review? I'm pretty sure it's generally useful, many have commented on wanting this feature on the list.

@mnaberez
Copy link
Member

Thanks for this pull request. The beta releases before 3.0 final will fix bugs but not add any new features. We will review this after 3.0 final is out. I would like this feature myself.

@willejs
Copy link

willejs commented Jun 14, 2013

this is a game stopper for me. see you later supervisord, hello runit.

@davidbirdsong
Copy link
Author

first check out circus: http://circus.readthedocs.org/en/0.8.1/

@willejs
Copy link

willejs commented Jun 14, 2013

Even though i hare runit's config format, i think he has a point.
http://jtimberman.housepub.org/blog/2012/12/29/process-supervision-solved-problem/

@nphase
Copy link

nphase commented Aug 11, 2013

This is also a game stopper for me. I Love how simple supervisor was, but I can't use it until it respects hard and soft file limits on the user to run as.

@abourget
Copy link

abourget commented Sep 4, 2013

get it in ! :)

@daenney
Copy link

daenney commented Feb 25, 2014

Anyone?

@zerthimon
Copy link

I need this functionality badly!

@sarusso
Copy link

sarusso commented Aug 5, 2014

Hi all,
given that discovering that supervisord does not respect the limits.conf file was going to be a game stopper for me as well, I found a workaround which does not have too many downsides.

== Relevant content of the supervisord conf: ==
user=root
command=/etc/supervisor/conf.d/runMyStuff.sh

== Relevant content of the runMyStuff.sh script : ==
exec sudo -E -u myUser myDaemon

And execute the following line to make sudo respect the limits.conf file:
echo "session required pam_limits.so" >> /etc/pam.d/sudo

Downsides:

  • 2 processes spawned, but correctly handled by supervisor
  • log files of stdout and stderr under the root user

Cheers,
Stefano.

@zerthimon
Copy link

I found a better workaround (I think):
command=bash -c "ulimit -n 80000; exec nc -l 8000"

Where ulimit -n 80000 is an example of a pre-run command and nc -l 8000 is the example daemon.

What happens is this:

  1. bash runs a pre-command which sets the limits of the process.
  2. exec replaces the bash code with code of the daemon, which now runs in the same process having the limits set by the pre-command.

Result: One process controlled by supervisor. (supervisor-->your daemon)

P.S: you can have any number of the pre-commands, separated by semicolons. The daemon has allways be started by exec.

P.P.S: user needs to be root if you want to increase limits.

@abourget
Copy link

I switched to circusd

@davidbirdsong
Copy link
Author

me too

@zerthimon
Copy link

Yeah, can't blame you. I'm aslo looking to switch.

@ayashjorden
Copy link

Hi all,
Any update when this PR is going to be merged?
Setting the nofiles limit is really important for high-throughput processes.

Thanks,
Yarden

@Fizzadar
Copy link

I'm assuming this is never going to be merged? Would be nice to know for sure. Moving to either circus or runit in the meantime...

@mpictor
Copy link

mpictor commented Apr 28, 2016

If I understand correctly, the implementation of supervisor is such that a process can be started as a non-root user, with more privileges than that user is allowed - bypassing pam, limits.conf, etc. If so, this is an insane design and I'll be advising my company to run the hell away.

Even if this particular problem is fixed, I'll have second thoughts about using supervisor: developers who do not understand security or don't take it seriously are likely to continue creating security problems.

@woosley
Copy link

woosley commented Aug 3, 2016

I need this feature badly also, maybe time to take a look at circus?

@antgel
Copy link

antgel commented Aug 4, 2016

We moved to circus, which has it's own "idiosyncrasies". Now looking at systemd. :)

@carun
Copy link

carun commented Mar 22, 2017

After 4 years, will this PR be merged?

@davidbirdsong
Copy link
Author

davidbirdsong commented Nov 12, 2019

After 4 years, will this PR be merged?

I opened this PR 6 1/2 years ago, but at this point let's all stop bagging on this project and complaining about this PR. Many suitable alternative projects with active owners have emerged since.

systemd has taken over the world, but here's a plug for the s6 supervision suite. I found it a joy to work with.

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

Successfully merging this pull request may close these issues.