Not receiving SIGTERM

[andrewklau]

Running the node4 image, code doesn't seem to receive the SIGTERM event sent from openshift/kuberenetes.

[bparees]

what about if you just s2i build an image locally and docker run it?

under normal circumstances, npm is being exec'd so it is what will receive the signal, i don't know if it properly sends those signals to the node command it launches?

https://github.com/sclorg/s2i-nodejs-container/blob/master/4/s2i/bin/run#L18

Did it work for nodejs 0.10? that image uses the same logic:
https://github.com/sclorg/s2i-nodejs-container/blob/master/0.10/s2i/bin/run#L18

[andrewklau]

Digging deeper, I entered the container and sent the SIGTERM signal to the npm process and that worked. It's weird because other containers like php haven't been seeing this issue.

Will try 0.10 when I get a chance.

[elyscape]

This seems like it could be related to the Linux kernel's special handling of signals on PID 1. It's possible that npm doesn't have handlers configured for SIGTERM. If that's the issue, putting something like dumb-init or tini in place should help.

[andrewklau]

Yes! That sounds about right, sending the SIGTERM to the started node module works (seem to always be pid 23(?).

pid 1 which is npm seems to just be ignored. I noticed this also happens to a PHP worker script, so perhaps this issue should move to openshift/kuberentes ?

[elyscape]

Fundamentally, it's an issue with Docker, but until a good solution is available, the best answer is to ensure that container images have something in place to handle signals properly. Using dumb-init or tini as the entrypoint is a good way to do that.

[andrewklau]

Is it something that could make it's way into the base image so all container images will be able to take advantage of this? dumb-init seems to be making it's way into fedora (?) https://bugzilla.redhat.com/show_bug.cgi?id=1367033

[elyscape]

Yup. You just add an ENTRYPOINT line in the base image.

[elyscape]

That being said, rather than rely on packages, it makes more sense to just install tini or dumb-init directly, IMO.

[andrewklau]

@bparees is this something which could be accepted into the base image if I attempted a PR? I'm not sure where this should be tracked.

[bparees]

@andrewklau i'm not yet clear on exactly what is being proposed, but changing the ENTRYPOINT sounds like a non-starter.

Since this issue seems specific to the nodejs image because of the way the main app process actually gets started, i'd prefer to fix the issue in the nodejs image specifically rather than trying to take on a generic solution.

[elyscape]

The thing is, while the issue as documented here doesn't affect the other s2i images, if you read my link about Linux's special handling of PID 1, it becomes apparent that the underlying cause may cause other issues. For example, if an app spawns other processes that themselves spawn other processes, it's possible to end up with unreaped zombies. Having something like tini or dumb-init in place will help with that.

[andrewklau]

It does seem to effect other images that don't use the default command, eg. we have a php s2i container that is executing a long running php command. It also never receives the SIGTERM signal and ends up being killed ungracefully.

[phracek]

Sorry, we do not have capacity to fix this issue. In case you really need it or if it is still valid please re-open this issue.