Skip to content

Conversation

@c-w
Copy link
Contributor

@c-w c-w commented Jul 10, 2018

Currently the nginx process is run in the background of the docker container and the gunicorn process is run in the foreground. This means that if the nginx process crashes, it won't get restarted and the nginx
logs don't show up in the docker logs since only the foreground gunicorn logs are captured. More generally, it is also not recommended to run multiple processes in a single docker container unless a process manager is used (more information).

This change introduces the supervisor process manager to run as the single foreground process in docker. The supervisor process in turn takes care of running both nginx and gunicorn.

Note that supervisor has its own environment variable isolation and doesn't have an easy way of passing all the environment variables to the child process. As a work-around to avoid having to declare all environment variables multiple times, we first dump the environment into files and then reload them in the child process from the files via the envdir utility.

Also:

  • Always dereference envs via os.getenv to make it easier to find all the environment variables used in the project via grep.
  • Modify the gunicorn custom logger to fix line length and unused import warnings and fix runtime crash when not in GCP or Azure.
  • Fix startup crash in docker-compose due to missing bash executable.

@c-w c-w requested a review from colinschoen July 10, 2018 17:09
c-w added 6 commits July 10, 2018 13:09
The ok-server image doesn't contain bash.
Currently the nginx process is run in the background of the docker
container and the gunicorn process is run in the foreground. This means
that if the nginx process crashes, it won't get restarted and the nginx
logs don't show up in the docker logs since only the foreground gunicorn
logs are captured. More generally, it is also not recommended to run
multiple processes in a single docker container unless a process manager
is used (more information can be found in [1]).

This change introduces the supervisor process manager to run as the
single foreground process in docker. The supervisor process in turn
takes care of running both nginx and gunicorn.

Note that supervisor has its own environment variable isolation and
doesn't have an easy way of passing all the environment variables to the
child process. As a work-around to avoid having to declare all
environment variables multiple times, we first dump the environment into
files and then reload them in the child process from the files via the
envdir utility [2].

[1] https://docs.docker.com/config/containers/multi-service_container/
[2] https://github.com/jezdez/envdir
Copy link
Member

@colinschoen colinschoen left a comment

Choose a reason for hiding this comment

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

LGTM

Excellent

@colinschoen colinschoen merged commit f00ebc0 into okpy:master Jul 13, 2018
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.

2 participants