Validate docker run's --pull values

[tbroyer]

Description

The value of the --pull flag on the run command is not validated against expected values (missing, always, never), this is error prone and can cause unexpected behavior and ignore other flags. This is particularly true given docker build has a --pull flag too with a different syntax.

We've been hit by this using a command like docker run --pull --rm …, which is actually being parsed as --pull=--rm and therefore a) not behaving as expected (we'd expected it to behave like --pull=always, just like docker build --pull) and b) ignoring/eating the --rm flag

Steps to reproduce the issue:

1. docker run --pull --rm hello-world

This step alone should be enough to reproduce (but behavior depends on context), but you can add:

2. docker pull hello-world
3. docker run --pull --rm hello-world
4. docker rmi hello-world
5. docker rmi -f hello-workd
6. docker run --pull --rm hello-world

Describe the results you received:

docker: Error response from daemon: No such image: hello-world:latest.
See 'docker run --help'.

Had I have the image locally, it would have used it without pulling it, and would have not removed the container.
With the above additional steps:

2. pulls the image
3. prints the hello world (but does not remove the container afterwards)
4. fails:

   $ docker rmi hello-world
   Error response from daemon: conflict: unable to remove repository reference "hello-world" (must force) - container a2824695f764 is 
   using its referenced image feb5d9fea6a5
   

5. removes the image
6. fails with the error above (No such image: hello-world:latest)

Describe the results you expected:

I expected this to pull the hello-world image (actually behaving like --pull=always, as we later discovered through docker run --help), or to tell me --rm is not a valid value for --pull.

Additional information you deem important (e.g. issue happens only occasionally):

Output of docker version:

Client:
 Version:           20.10.17
 API version:       1.41
 Go version:        go1.18.3
 Git commit:        100c70180f
 Built:             Sat Jun 11 23:27:28 2022
 OS/Arch:           linux/amd64
 Context:           default
 Experimental:      true

Server:
 Engine:
  Version:          20.10.17
  API version:      1.41 (minimum version 1.12)
  Go version:       go1.18.3
  Git commit:       a89b84221c
  Built:            Sat Jun 11 23:27:14 2022
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          v1.6.6
  GitCommit:        10c12954828e7c7c9b6e0ea9b0c02b01407d3ae1.m
 runc:
  Version:          1.1.3
  GitCommit:        
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Output of docker info:

Client:
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc., v0.8.2-docker)
  compose: Docker Compose (Docker Inc., 2.6.0)

Server:
 Containers: 0
  Running: 0
  Paused: 0
  Stopped: 0
 Images: 88
 Server Version: 20.10.17
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: false
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: systemd
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 io.containerd.runtime.v1.linux runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 10c12954828e7c7c9b6e0ea9b0c02b01407d3ae1.m
 runc version: 
 init version: de40ad0
 Security Options:
  seccomp
   Profile: default
  cgroupns
 Kernel Version: 5.18.5-arch1-1
 Operating System: Arch Linux
 OSType: linux
 Architecture: x86_64
 CPUs: 8
 Total Memory: 15.51GiB
 Name: tbr
 ID: 72NT:3R4Q:DMOV:Y4AD:LV37:CV5T:EA6W:U7OE:A4HD:WPTP:ZJKU:5YVR
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false
 Default Address Pools:
   Base: 192.168.234.0/24, Size: 28

Additional environment details (AWS, VirtualBox, physical, etc.):

Information above is on my laptop. We actually noticed the unexpected behavior on our CI server, where the step worked once on our pre-merge build (because at the time the image we use was available on the machine, as it had been built recently by another job) but later failed during a weekly build (as the image had been pruned since then)

[thaJeztah]

Thanks for reporting; it looks indeed that because --rm is no longer a boolean (which allows the value to be optional), it will always swallow what comes after, so the "value is optional" bit is broken. Wondering how to resolve that 🤔

[thaJeztah]

oh! I'm actually wrong; the --pull options does not have a default (I was mixing it up with another option / proposal where we went from a boolean flag to a boolean or value), so it's indeed just the missing validation