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

F: add tini as Docker entrypoint to properly handle termination signals #65

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

meck93
Copy link
Contributor

@meck93 meck93 commented Jan 2, 2025

Current Situation

If I run the image using docker run -it --rm --name sqlite3 -d keinos/sqlite3 the container starts as expected.

docker ps shows the running container:

8373aade31e9   keinos/sqlite3   "/bin/sh -c /usr/bin…"   7 seconds ago   Up 6 seconds (healthy)             sqlite3

If I try to stop the container using docker stop sqlite3, the stop process takes very long (waits for 10 seconds) and once the timeout is reached Docker kills the container (force removes the container).

This can be reproduced using:

  • docker kill -s SIGTERM sqlite3 which does nothing -> the container keeps running
  • only docker kill -s SIGKILL sqlite3 stops and removes the container

Issue

The sqlite3 container does not register signal handlers or the alpine base image does not support it by default.

Solution

A solution for this entrypoint (signal handlers) issue that is natively built into docker is tini:

Option 1: Run the container using --init

If I run the image with the additional --init option (e.g., docker run --init -it --rm --name sqlite3 -d keinos/sqlite3) the container starts as expected and immediately stops when stopped with docker stop sqlite3. Additionally, this can be verified if instead of docker stop sqlite3 the command docker kill -s SIGTERM sqlite3 is used. The container now respects the SIGTERM signal and stops immediately.

Option 2: Add tini as Docker entrypoint

Option 1 only works if I use docker run directly. The --init option is not available in Kubernetes. Additionally, new users should not have to know that it is necessary to pass the --init option.

The current PR adds tini as a Dockerfile ENTRYPOINT which ensures that termination signals are always handled correctly.

@meck93
Copy link
Contributor Author

meck93 commented Jan 2, 2025

@KEINOS would be great if you could have a look at it. Thanks for your work!

@KEINOS
Copy link
Owner

KEINOS commented Jan 4, 2025

I did reproduce the delay when docker stop <<container name>>.

$ docker run -it --rm --name sqlite3 -d keinos/sqlite3
a718ebef779a0edf5350251c4aa33fc9c5360e9274f1b0e3c8120479fa1d62a1

$ docker ps
CONTAINER ID   IMAGE     COMMAND              CREATED         STATUS                            PORTS     NAMES
a718ebef779a   test      "/usr/bin/sqlite3"   4 seconds ago   Up 4 seconds (health: starting)             sqlite3

$ time docker stop sqlite3
sqlite3

real	0m10.206s
user	0m0.035s
sys	0m0.024s

I like the tini implementation idea. But after some investigation, I found that the underlying problem is that the interactive mode of SQLite3 does not accept signals such as SIGTERM and SIGINT.

Try, docker run --rm -it --name sqlite3 keinos/sqlite3 without detaching it. You will be prompted sqlite>, but sending ^C(ctrl+c, SIGINT) doesn't work unless you type.quit or send a SIGKILL to the process.

As you already mentioned, docker stop sends SIGTERM by default. Therefore, docker stop has no effect and waits for a timeout. This is the reason for the delay.

Therefore, "SIGKILL" is still sent even when "tini" is used.

Now, we have two choices:

  1. Use tini to send SIGKILL right away.
    • tini may handle a zombie-processes, in case the user over-rides the CMD.
    • In my opinion, in such cases, users should create a Dockerfile that uses keinos/sqlite3 as a base-image.
  2. Add STOPSIGNAL SIGKILL in the original Dockerfile.
    • The image size and the number of layers are retained.
    • If the user over-rides the CMD, docker stop sends SIGKILL and may cause trouble closing the mounted script.
FROM keinos/sqlite3:latest

STOPSIGNAL SIGKILL
CMD ["/usr/bin/sqlite3"]
$ ls
Dockerfile

$ docker build -t sqlite3 .
**snip**

$ docker run -it --rm --name sqlite3 -d sqlite3
27163fb07680d52d89a2c0fa8e12f5d2cd741ed8e5104532404c21ed37cc5079

$ docker ps
CONTAINER ID   IMAGE     COMMAND              CREATED         STATUS                            PORTS     NAMES
27163fb07680   sqlite3   "/usr/bin/sqlite3"   3 seconds ago   Up 2 seconds (health: starting)             sqlite3

$ time docker stop sqlite3
sqlite3

real	0m0.180s
user	0m0.027s
sys	0m0.021s

They both have pros/cons depending on the use-case.

What do you think?

@meck93
Copy link
Contributor Author

meck93 commented Jan 4, 2025

I like the tini implementation idea. But after some investigation, I found that the underlying problem is that the interactive mode of SQLite3 does not accept signals such as SIGTERM and SIGINT.

Try, docker run --rm -it --name sqlite3 keinos/sqlite3 without detaching it. You will be prompted sqlite>, but sending ^C(ctrl+c, SIGINT) doesn't work unless you type.quit or send a SIGKILL to the process.

Good point. I didn't know. Now, it makes a lot of sense why SIGTERM or SIGINT didn't work.

As you already mentioned, docker stop sends SIGTERM by default. Therefore, docker stop has no effect and waits for a timeout. This is the reason for the delay.

Therefore, "SIGKILL" is still sent even when "tini" is used.

Did tini translate the SIGTERM to SIGKILL immediately?
Because docker kill -s SIGTERM sqlite3 worked immediately after adding tini.

Now, we have two choices:

  1. Use tini to send SIGKILL right away.

    • tini may handle a zombie-processes, in case the user over-rides the CMD.
    • In my opinion, in such cases, users should create a Dockerfile that uses keinos/sqlite3 as a base-image.
  2. Add STOPSIGNAL SIGKILL in the original Dockerfile.

    • The image size and the number of layers are retained.
    • If the user over-rides the CMD, docker stop sends SIGKILL and may cause trouble closing the mounted script.
FROM keinos/sqlite3:latest

STOPSIGNAL SIGKILL
CMD ["/usr/bin/sqlite3"]

I guess adding STOPSIGNAL SIGKILL to the base image is a nice solution as it doesn't add an additional dependency (if we don't really need it). To prevent causing trouble to users' mounting different scripts, we could maybe add a little note in the README.

What is your preferred solution? I can change this PR to use STOPSIGNAL SIGKILL and make a little note in the README if you're agree with it.

@KEINOS
Copy link
Owner

KEINOS commented Jan 7, 2025

tl;dr : I decided to implement tini due to the behavior of SQLite3 being run in PID 1.

LooksGorillaToMe


ts;dr

I wrote a simple application that ignores (captures) incoming signals and output them. And observed the behavior.

Test Code (in Go)
FROM golang:alpine
WORKDIR /app
COPY . /app
RUN \
    go mod init test/process_kill && \
    go build -o myapp .
CMD [ "/app/myapp" ]
FROM golang:alpine
WORKDIR /app
COPY . /app
RUN apk add --no-cache tini
RUN \
    go mod init test/process_kill && \
    go build -o myapp .
ENTRYPOINT ["/sbin/tini", "-g", "--"]
CMD [ "/app/myapp" ]
package main

import (
	"fmt"
	"os"
	"os/signal"
	"time"
)

const ttl = 60 * time.Second

func main() {
	fmt.Println("Let the program begin!")
	defer fmt.Println("And so, it comes to an end!")

	sigChan := make(chan os.Signal, 1)
	signal.Notify(sigChan) // Capture all signals

	timeout := time.After(ttl)

	count := 0
	for {
		select {
		case sig := <-sigChan:
			fmt.Println("Received signal: ", sig.String())
		case <-timeout:
			fmt.Println("Timeout reached, exiting program.")
			return // break out of the loop
		default:
			count++
			if count%5 == 0 {
				fmt.Println("I'm still alive!")
				count = 0
			}

			time.Sleep(1 * time.Second)
		}
	}
}

I expected the application to behave in the same way as SQLite3 does. But it didn't.

Did tini translate the SIGTERM to SIGKILL immediately?
Because docker kill -s SIGTERM sqlite3 worked immediately after adding tini.

I was wrong about this. Indeed, tini sends SIGTERM, not SIGKILL. And docker kill -s SIGTERM sqlite3 sends SIGTERM as well.

Try, docker run --rm -it --name sqlite3 keinos/sqlite3 without detaching it. You will be prompted sqlite>, but sending ^C(ctrl+c, SIGINT) doesn't work unless you type .quit or send a SIGKILL to the process.

SQLite3 does this behavior only when it is running as PID 1. Otherwise SQLite3 seems to handle SIGTERM even during interaction mode.

I think this is somewhat similar behavior as sleep command running as PID 1.

$ # Wait for 30 seconds to terminate
$ docker run --rm alpine sh -c 'sleep 30'
^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C

Usually, both sqlite3 and sleep command accepts SIGTERM, but they are not designed to be run as PID 1.

$ # Running locally, sending ^C three times stops the process
$ sqlite3
SQLite version 3.37.0 2021-12-09 01:34:53
Enter ".help" for usage hints.
Connected to a transient in-memory database.
Use ".open FILENAME" to reopen on a persistent database.
sqlite> ^C^C

In most of the use cases, sending SIGKILL to sqlite3 process does no harm.

But I don't like it.

Since SQLite3 is file-base, killing (not terminating nor interrupting) the process makes me uncomfortable.

Thus, I decided to implement tini. Thanks @meck93 to pointing this out.

@KEINOS KEINOS merged commit 1fd80f0 into KEINOS:master Jan 7, 2025
3 of 4 checks passed
@meck93 meck93 deleted the properly-handle-termination-signals branch January 7, 2025 12:25
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