docker / cli

The Docker CLI
Apache License 2.0
4.83k stars 1.9k forks source link

`docker container logs --follow` will quit if the container is restarted #5305

Open karolz-ms opened 1 month ago

karolz-ms commented 1 month ago

Description

The command docker container logs --follow with quit if the container is restarted. Re-issuing the command will get the logs from before restart as well as after restart, but it should not be necessary.

Reproduce

  1. docker run -d -p:6556:80 nginx
  2. (in a separate console) docker container logs --follow <nginx container id>
  3. docker container restart <nginx container id>

Expected behavior

The container logs --follow command should just continue streaming logs. The container is just restarted; it has the same container ID, there is no need to stop execution of the logs command.

docker version

> docker version
Client:
 Version:           27.1.1
 API version:       1.46
 Go version:        go1.21.12
 Git commit:        6312585
 Built:             Tue Jul 23 19:57:57 2024
 OS/Arch:           windows/amd64
 Context:           desktop-linux

Server: Docker Desktop 4.33.0 (160616)
 Engine:
  Version:          27.1.1
  API version:      1.46 (minimum version 1.24)
  Go version:       go1.21.12
  Git commit:       cc13f95
  Built:            Tue Jul 23 19:57:19 2024
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.7.19
  GitCommit:        2bf793ef6dc9a18e00cb12efb64355c2c9d5eb41
 runc:
  Version:          1.7.19
  GitCommit:        v1.1.13-0-g58aa920
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

docker info

> docker info
Client:
 Version:    27.1.1
 Context:    desktop-linux
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.16.1-desktop.1
    Path:     C:\Program Files\Docker\cli-plugins\docker-buildx.exe
  compose: Docker Compose (Docker Inc.)
    Version:  v2.29.1-desktop.1
    Path:     C:\Program Files\Docker\cli-plugins\docker-compose.exe
  debug: Get a shell into any image or container (Docker Inc.)
    Version:  0.0.34
    Path:     C:\Program Files\Docker\cli-plugins\docker-debug.exe
  desktop: Docker Desktop commands (Alpha) (Docker Inc.)
    Version:  v0.0.14
    Path:     C:\Program Files\Docker\cli-plugins\docker-desktop.exe
  dev: Docker Dev Environments (Docker Inc.)
    Version:  v0.1.2
    Path:     C:\Program Files\Docker\cli-plugins\docker-dev.exe
  extension: Manages Docker extensions (Docker Inc.)
    Version:  v0.2.25
    Path:     C:\Program Files\Docker\cli-plugins\docker-extension.exe
  feedback: Provide feedback, right in your terminal! (Docker Inc.)
    Version:  v1.0.5
    Path:     C:\Program Files\Docker\cli-plugins\docker-feedback.exe
  init: Creates Docker-related starter files for your project (Docker Inc.)
    Version:  v1.3.0
    Path:     C:\Program Files\Docker\cli-plugins\docker-init.exe
  sbom: View the packaged-based Software Bill Of Materials (SBOM) for an image (Anchore Inc.)
    Version:  0.6.0
    Path:     C:\Program Files\Docker\cli-plugins\docker-sbom.exe
  scout: Docker Scout (Docker Inc.)
    Version:  v1.11.0
    Path:     C:\Program Files\Docker\cli-plugins\docker-scout.exe

Server:
 Containers: 1
  Running: 1
  Paused: 0
  Stopped: 0
 Images: 2
 Server Version: 27.1.1
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 2bf793ef6dc9a18e00cb12efb64355c2c9d5eb41
 runc version: v1.1.13-0-g58aa920
 init version: de40ad0
 Security Options:
  seccomp
   Profile: unconfined
 Kernel Version: 5.15.146.1-microsoft-standard-WSL2
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 16
 Total Memory: 31.2GiB
 Name: docker-desktop
 ID: 7bda9438-fc74-4548-a179-caa9ea932af8
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 No Proxy: hubproxy.docker.internal
 Labels:
  com.docker.desktop.address=npipe://\\.\pipe\docker_cli
 Experimental: false
 Insecure Registries:
  hubproxy.docker.internal:5555
  127.0.0.0/8
 Live Restore Enabled: false

WARNING: No blkio throttle.read_bps_device support
WARNING: No blkio throttle.write_bps_device support
WARNING: No blkio throttle.read_iops_device support
WARNING: No blkio throttle.write_iops_device support
WARNING: daemon is not using the default seccomp profile

Diagnostics ID

57094215-4B96-4228-8AC0-63705A3A1BAD/20240729165901

Additional Info

No response

karolz-ms commented 1 month ago

@savannahostrowski FYI

laurazard commented 1 month ago

Hiya, thanks for the report.

The command docker container logs --follow with quit if the container is restarted. Re-issuing the command will get the logs from before restart as well as after restart, but it should not be necessary.

This isn't quite so – when a container dies, the logger attached to it streaming messages over from the daemon is closed. This causes the CLI to exit, since running docker logs --follow just initiates a message stream from the daemon until the user cancels the command or the stream is closed.

If the CLI was aware that the reason the stream was ended was because the container was being restarted, it could hang (instead of exiting) and attempt to restart the stream. However, this isn't currently the case. docker restart calls the daemon API ContainerRestart (/containers/id/restart) which simply stops + start the container.

I guess the CLI could start streaming events from the daemon while streaming logs, and decide based on that. docker events while doing a docker restart in another terminal shows:

2024-07-30T09:27:45.637609939Z container stop 81554c2ed5536f632dbd3b0942574ac528594b5e7c941f3d82fcaeba338131f4 (image=alpine, name=quizzical_colden)
2024-07-30T09:27:45.639755151Z container die 81554c2ed5536f632dbd3b0942574ac528594b5e7c941f3d82fcaeba338131f4 (execDuration=41, exitCode=137, image=alpine, name=quizzical_colden)
2024-07-30T09:27:45.655764850Z network connect e96cf3e6b07b3bd6b8b0e93aca78ff46fac383b1023c72f7efbe3ae2ff7c828b (container=81554c2ed5536f632dbd3b0942574ac528594b5e7c941f3d82fcaeba338131f4, name=bridge, type=bridge)
2024-07-30T09:27:45.791383745Z container start 81554c2ed5536f632dbd3b0942574ac528594b5e7c941f3d82fcaeba338131f4 (image=alpine, name=quizzical_colden)
2024-07-30T09:27:45.791437579Z container restart 81554c2ed5536f632dbd3b0942574ac528594b5e7c941f3d82fcaeba338131f4 (image=alpine, name=quizzical_colden)

Which means that we could do something like hold for a few seconds after a container stops and wait for a restart/attempt to restart the log stream in that case, but there's a few questions in this case, and this requires some discussion.

karolz-ms commented 1 month ago

Laura thanks for quick reply.

I am thinking about this from the contract perspective: what is container logs --follow supposed to do? And I think what I proposed makes sense from that perspective, especially since the restarted container is otherwise the same container from CLI/API perspective, with the same container ID.

The other reason for suggested improvement is the complexity associated with the use of the CLI. The client (the user of the CLI) does not necessarily know why the container logs --follow terminated: was it because of container being restarted, or something else? If it is supposed to re-issue container logs --follow command, how soon should it do so? Should it re-try once, or more than once? I guess it could do additional monitoring of the daemon like you described, with docker events, but encapsulating that inside the CLI make the life of the client much easier.

laurazard commented 1 month ago

Hiya!

FWIW, we were discussing adding a -F option to docker logs that would follow the UX of tail:

  • -f would only work on existing containers and exit when the container ends
  • -F would never exit, it would always just wait for the container, even if it doesn't exist at the time

Re: the contract perspective for --follow, I think the point is moot because at this point changing the behavior here would likely "break the world" 😅.

We can however introduce a new flag for the desired (more intuitive) behavior. The fact that this also mimics the tail UX:

  -f      The -f option causes tail to not stop when end of file is reached, but rather to wait for additional data to be ap‐
             pended to the input.  The -f option is ignored if the standard input is a pipe, but not if it is a FIFO.

 -F      The -F option implies the -f option, but tail will also check to see if the file being followed has been renamed or
             rotated.  The file is closed and reopened when tail detects that the filename being read from has a new inode number.

             If the file being followed does not (yet) exist or if it is removed, tail will keep looking and will display the file
             from the beginning if and when it is created.

             The -F option is the same as the -f option if reading from standard input rather than a file.

is a plus for me since it's UX that users might already be aware of.

Do you have other suggestions? I also considered adding a message to the output of docker logs providing a reason why logs exited, but we'd have to discuss how to do that carefully/it might not be possible since a common usecase is capturing the output of docker logs for parsing/other things so the output there shouldn't be "polluted" with meta-information by default.

karolz-ms commented 1 month ago

Thanks Laura.

I agree that adding a "message" to the output of docker logs will not be a great idea since the command captures both stdout and stderr output from the container, and with additional message it will be impossible to differentiate between the diagnostic output of the command vs the output from the container.

Regarding the new flag, the way I use the container logs --follow command (programmatically) is by supplying the full container ID. The full ID does not change during container restart, and my understanding is, the ID will be unique at least during the Docker daemon lifetime. Using tail analogy, the full container ID is more like inode number and less like file name. The proposed semantics of the -F (capital F) flag does not really work for me that well. I DO want to be told that the container does not exist if the ID I supply is invalid; I do not want to wait indefinitely for a container that may never appear.

If you are unwilling to change the default behavior of the command , I understand your point, although FWIIW your competition works like I suggested originally (the container logs --follow command without any additional parameters survives the container restart). My second choice then would be a targeted flag that disables the "quit on restart" behavior without changing anything else. So if I use container logs --follow --continue-on-restart <full container ID>, the command does not quit when the container is restarted, but instead continues pumping the logs from the container. Let me know how this sounds to you.

laurazard commented 1 month ago

Thanks for the feedback! FWIW, (from my highly biased perspective, so take that for what it's worth), for however many people might prefer our "competition"'s behavior, I'm sure there are lots that would prefer to have docker logs exit by default when the container stops so they can just programmatically capture the output of a container and exit when the container is stopped (there is no more output) rather than have to figure out when to do that.

Regardless, I see your points about:

I DO want to be told that the container does not exist if the ID I supply is invalid; I do not want to wait indefinitely for a container that may never appear.

This seems sane to me (and was the approach I was initially considering for https://github.com/docker/cli/pull/5307).

@thaJeztah @vvoland thoughts on the UX here? We could do a docker logs -f --no-detach (or some better wording), or something of the sort.