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

Can't run multiple commands in Procfile #114

Open
stenius opened this issue Oct 19, 2014 · 13 comments
Open

Can't run multiple commands in Procfile #114

stenius opened this issue Oct 19, 2014 · 13 comments

Comments

@stenius
Copy link

stenius commented Oct 19, 2014

I am having trouble with the way I write Procfiles and the way they are evaluated in bash.

In commit 16e3f3c I started having issues with running
setuidgid u32178 $(eval echo "${command}") vs simply running eval ${command} in the /start script.

My Procfile looks like this.

web: python bin/production.py collectstatic --noinput --verbosity 3 ; gunicorn wsgi

@mjonuschat
Copy link
Contributor

The change is due to the apps no longer running as the root user in the container. The „official“ Procfile syntax only allows one command per Procfile entry. As a workaround I would suggest a little helper script within your app, for example bin/start which gets called from your Procfile. The script can then start all needed processes without relying on shell expansion in the container startup.

@stenius
Copy link
Author

stenius commented Oct 20, 2014

I know in the Django Heroku community, a lot of people use the same bash syntax to chain together Procfile commands and I have done so myself in several repos.

Would a patch be accepted that split the bash command and eval'd the splits to maintain compatibility with Heroku?

@mjonuschat
Copy link
Contributor

I think it will be hard to reliably split/interpret the string as bash does, most of the Procfile based process starters seem to have this problem. But I would really like buildstep behaving as closely to Heroku as possible.

@progrium
Copy link
Owner

@stenius can you do some playing around with Heroku to see various cases where their splitting works and doesn't work?

@yabawock perhaps if we just detect ; or && we run with bash -c? I tried some approaches with -x and -n to try and get bash to tell us the split commands without running, but it doesn't look possible. You're right it's hard to reliably split commands outside of just running them. :\

@mjonuschat
Copy link
Contributor

@progrium I've been playing around with using all of the available daemontools. I have a working setup that allows starting multiple processes and scaling of the processes under the control of supervise from daemontools while being API compatible with the current buildstep image but it doesn't work with multiple commands in the Procfile.

I want to use that as the base for a smaller / interim solution that only writes a run file for the web process (which is a simple bash script) and then start supervise only for this process.

Currently I still have to solve the logging to STDOUT and non-root startup. Normally setuidgid is called from the runfile, but then multiple commands break again. My plan is to start supervise as the user so that no „magic“ happens in the run file.

@progrium
Copy link
Owner

I guess we're getting into a slippery slope. I generally don't want to run multiple processes in containers. With some exceptions, but that's just not how it was designed. Heroku doesn't do this either. I'm a bit more lax on that now, but I'm still not wanting to support internal scaling or even really supervision. Supervision inside a container is an anti-pattern for me. When I do run multiple processes, I do it as a codependent group, something like:
https://gist.github.com/progrium/0ac0248f70e2adce964f

What are your thoughts?

@mjonuschat
Copy link
Contributor

I unexpectedly had some time on my hands yesterday and tried to solve this ticket before reading your comment. I'm torn between both reasonings. I don't mind sidecar processes in the container like Sidekiq or Clockwork for background processing or scheduled tasks and I've been using some kind of plugin for that until I integrated it into my buildstep fork so that it uses more reliable tools than foreman, shoreman and the like which weren't meant for production use. I also unterstand the desire/need for simplicity in dokku/buildstep and having bigger solutions like Deis or Flynn might be the way to go there. The solution I prepared in https://github.com/yabawock/buildstep/commit/385684e3acdcf9e08de9463a738fb478406de429 works but might be a bit complex for the goals of the project (given the size of export-procfile).

Given #102 and this issue I think we are between a rock and a hard place since having shell expansion, signal propagation and user switching all in one will be hard to accomplish without some kind of process manager.

Maybe there is some middle ground here, something like getting rid of export-procfile in my commit, adding one service template to /etc/service that will be started up using /start. It could use the parameter given to /start to extract the Procfile item and append the commands to the run file for the service definition. In theory this solution should allow to do user switching (done before starting the process manager), signal propagation and bash command expansion (would be done by the run script for the service).

@progrium
Copy link
Owner

When you say service template are you talking about an Upstart service?

@mjonuschat
Copy link
Contributor

No, upstart is way too much hassle in a container, I'm talking about daemontools, which is much simpler than upstart and available in cedar-14 by default. A typical file would look something like this:

#!/bin/bash
exec 2>&1
exec setuidgid u17834 bin/thin -p $PORT start

Add in sourcing of /app/.profile.d and thats about all there is to it.

@mjonuschat
Copy link
Contributor

@progrium I've submitted #117 which implements the minimal solution I had in mind.

@progrium
Copy link
Owner

@yabawock what do you think about just using forego for this purpose? Simple binary we can just drop in...

@mjonuschat
Copy link
Contributor

Sounds good and since precompiled binaries are being provided it should be a breeze to implement in the current master and/or the herokuish branch.

@stuartpb
Copy link

I know in the Django Heroku community, a lot of people use the same bash syntax to chain together Procfile commands and I have done so myself in several repos.

I'm a bit confused about this, since, as @yabawock said:

The „official“ Procfile syntax only allows one command per Procfile entry.

Don't Procfiles have to specify a single command line (ie. something that could be dropped after exec) on Heroku?

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

No branches or pull requests

4 participants