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 Claude Paroz, 7 years ago

Needs tests: set
Triage Stage: UnreviewedAccepted

comment:2 by Slavek Kabrda, 7 years ago

I amended the PR with tests, so I believe it's now ready for review. PR

comment:3 by Claude Paroz, 7 years ago

Needs tests: unset
Patch needs improvement: set

Please uncheck Patch needs improvement when the Windows issue is solved.

comment:4 by Slavek Kabrda, 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 Carlton Gibson, 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 Carlton Gibson, 7 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Carlton Gibson, 7 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

comment:8 by Carlton Gibson, 7 years ago

Resolution: wontfix
Status: newclosed

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.

Note: See TracTickets for help on using tickets.
Back to Top