Opened 7 years ago
Closed 7 years ago
#28835 closed Bug (wontfix)
Development server doesn't shut down on SIGTERM
Reported by: | Slavek Kabrda | Owned by: | nobody |
---|---|---|---|
Component: | Core (Management commands) | Version: | 1.11 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Hi, I've already opened a PR to fix this [1], but have been notified in a comment that I'll need to open a Trac ticket, so here it is. The explanation of this issue is at [1], but I'll C&P it here as well for clarity:
I've run into an issue where shutting down django devserver in a container (e.g. in kubernetes/openshift environment) doesn't work well. The original report is at [2].
Explanation:
- kubernetes/openshift send SIGTERM to PID 1 in the container when shutting the container down.
- If
manage.py runserver
is an entrypoint, it gets executed as PID 1 in container. - Linux kernel won't propagate SIGTERM to any process that is PID 1 (in or outside of container) if it doesn't have a handler installed for this signal.
The implication of this is that the container with django devserver will not do "soft" shutdown on SIGTERM and Kubernetes/Openshift will wait for timeout and kill the container with SIGKILL. Therefore the container will keep hanging and occupying system resources during the whole timeout.
Even though I understand that people should only use devserver for development and not for deployment, I think it's quite common to first make the container working with devserver for development and then consider Gunicorn or similar solution when moving from early stage of the project. IOW, I think this would really be beneficial for people using Kubernetes like environments to develop/deploy Django apps.
Thanks for considering!
[1] https://github.com/django/django/pull/9338
[2] https://github.com/sclorg/s2i-python-container/issues/93
Change History (8)
comment:1 by , 7 years ago
Needs tests: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 7 years ago
comment:3 by , 7 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | set |
Please uncheck Patch needs improvement
when the Windows issue is solved.
comment:4 by , 7 years ago
Patch needs improvement: | unset |
---|
I marked the test to be skipped on Windows, as I believe this is actually not (easily) achievable on Windows. The Windows behavior is unaffected by my patch. All tests pass now, I'm unchecking "Patch needs improvement".
comment:5 by , 7 years ago
Patch itself looks good.
There is some discussion on the PR as to whether this is quite the right fix: in the ideal case this probably isn't Django's responsibility vs it's a small change that should help some avoid a ??? moment, especially when just experimenting.
Given this was initially Accepted (rather than DDN, or wontfix), I'm going to mark this Ready for checkin, wherein it'll get a final review.
comment:6 by , 7 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:7 by , 7 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
comment:8 by , 7 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
After discussion on the PR we're going to close this off in favour of #21978 (having runserver use gunicorn).
In the meantime a specialised init process (or docker's --init
flag) is probably the way to go.
I amended the PR with tests, so I believe it's now ready for review. PR