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

Make supervisord exit when key processes die, fixes #1137 #1142

Merged
merged 6 commits into from Oct 9, 2018

Conversation

rfay
Copy link
Member

@rfay rfay commented Sep 29, 2018

The Problem/Issue/Bug:

In the web container, supervisord can happily work along even though php-fpm or the webserver has died or is not runnable.

How this PR Solves The Problem:

Use technique from https://blog.zhaw.ch/icclab/process-management-in-docker-containers/ to add an event listener, so supervisord and thus the container will die in cases like this.

Manual Testing Instructions:

  • Start a project
  • ddev ssh
  • Do things like killall nginx or killall php-fpm or introduce an error into /etc/nginx/nginx.conf and killall -1 nginx, or kill -1 1 (to reload supervisord itself).

Those should result in nginx or php-fpm or apache croaking, and the container exiting.

Automated Testing Overview:

Related Issue Link(s):

OP #1137

Release/Deployment notes:

This does in fact change the behavior of the web container. Previously, if a process died (like apache or nginx) the container would continue on. It would probably become "unhealthy" but would stay up. Here instead if one of those croaks you get a container that stops.

One interesting thing about this implementation is there is just one event listener, which is causing supervisord to exit if any process dies. It would be easy to implement separate listeners that had separate behavior. For example, we could then ignore the failure of mailhog.

@rfay rfay added this to the v1.3.0 milestone Sep 29, 2018
@rfay rfay self-assigned this Sep 29, 2018
@rfay rfay force-pushed the 20180929_nginx_quit_immediately branch 2 times, most recently from 8514ad8 to b7c463e Compare October 6, 2018 23:47
Copy link
Contributor

@andrewfrench andrewfrench left a comment

Choose a reason for hiding this comment

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

With ddev version:

commit	v1.2.0-32-gb7c463e3
domain	ddev.local
cli   	v1.2.0-32-gb7c463e3
web   	drud/ddev-webserver:20180929_nginx_quit_immediately
db    	drud/ddev-dbserver:20180929_mariadb_stderr
dba   	drud/phpmyadmin:v1.2.0
router	drud/ddev-router:20180922_upgrade_debian_stretch

After ddev ssh:

nginx-fpm

  • killall nginx
  • Container exited to the host

apache-fpm

  • killall apache2
  • Container exited to the host

apache-cgi

  • killall apache2
  • Container exited to the host

@rfay rfay force-pushed the 20180929_nginx_quit_immediately branch from b7c463e to f5bf974 Compare October 9, 2018 13:21
@rfay rfay merged commit 58f8023 into ddev:master Oct 9, 2018
@rfay rfay deleted the 20180929_nginx_quit_immediately branch October 9, 2018 15:38
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.

None yet

2 participants