Docker build --pull fails if no manifest is found on remote registry

scaytrase commented 6 years ago

Looks like the --pull option disallows fallback to locally installed image if pull attempt failed. If I remove --pull flag from building B image - everything works OK. I think it's not good idea to manipulate the flag value base on knownledge, whether FROM already pushed to remote repo or not

Description

Given A/Dockerfile
```
FROM alpine:latest
```
Given B/Dockerfile
```
FROM local_a
```
Given Makefile
```
test: make-a make-b
```

make-a: docker build --pull -t local_a:latest A/

make-b: make-a docker build --pull -t local_b:latest B/

4. Run `make`

**Describe the results you received:**

PS C:\Work\Projects\tmp\dockerfile-pull-repro> make docker build --pull -t local_a:latest A/ Sending build context to Docker daemon 2.048kB Step 1/1 : FROM alpine:latest latest: Pulling from library/alpine Digest: sha256:7df6db5aa61ae9480f52f0b3a06a140ab98d427f86d8d5de0bedab9b8df6b1c0 Status: Image is up to date for alpine:latest ---> 3fd9065eaf02 Successfully built 3fd9065eaf02 Successfully tagged local_a:latest docker build --pull -t local_b:latest B/ Sending build context to Docker daemon 2.048kB Step 1/1 : FROM local_a pull access denied for local_a, repository does not exist or may require 'docker login' make: *** [Makefile:7: make-b] Error 1


If I choose some existing image, but not with existing tag (which is build on step `build-a`) then I'' recieve the following variant of output

PS C:\Work\Projects\tmp\dockerfile-pull-repro> make docker build --pull -t php:8-fpm A/ Sending build context to Docker daemon 2.048kB Step 1/1 : FROM alpine:latest latest: Pulling from library/alpine Digest: sha256:7df6db5aa61ae9480f52f0b3a06a140ab98d427f86d8d5de0bedab9b8df6b1c0 Status: Image is up to date for alpine:latest ---> 3fd9065eaf02 Successfully built 3fd9065eaf02 Successfully tagged php:8-fpm docker build --pull -t local_b:latest B/ Sending build context to Docker daemon 2.048kB Step 1/1 : FROM php:8-fpm manifest for php:8-fpm not found make: *** [Makefile:7: make-b] Error 1


**Describe the results you expected:**
Image `local_b` was built using local version of image

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

**Output of `docker version`:**

Client: Version: 17.07.0-ce API version: 1.31 Go version: go1.8.3 Git commit: 8784753 Built: Tue Aug 29 17:41:05 2017 OS/Arch: windows/amd64

Server: Version: 17.12.0-ce API version: 1.35 (minimum version 1.12) Go version: go1.9.2 Git commit: c97c6d6 Built: Wed Dec 27 20:09:19 2017 OS/Arch: linux/amd64 Experimental: false


**Output of `docker info`:**

Containers: 9 Running: 5 Paused: 0 Stopped: 4 Images: 670 Server Version: 17.12.0-ce Storage Driver: aufs Root Dir: /var/lib/docker/aufs Backing Filesystem: extfs Dirs: 394 Dirperm1 Supported: true Logging Driver: json-file Cgroup Driver: cgroupfs Plugins: Volume: local Network: bridge host macvlan null overlay Log: awslogs fluentd gcplogs gelf journald json-file logentries splunk syslog Swarm: inactive Runtimes: runc Default Runtime: runc Init Binary: docker-init containerd version: 89623f28b87a6004d4b785663257362d1658a729 runc version: b2567b37d7b75eb4cf325b77297b140ea686ce8f init version: 949e6fa Security Options: apparmor seccomp Profile: default Kernel Version: 4.13.0-38-generic Operating System: Ubuntu 17.10 OSType: linux Architecture: x86_64 CPUs: 4 Total Memory: 1.946GiB Name: payment-method ID: RH7N:DMCY:GJO6:XLOG:2N55:BUEU:MDN2:7M5T:GJUC:OIY6:2SXR:GX6O Docker Root Dir: /var/lib/docker Debug Mode (client): false Debug Mode (server): false Registry: https://index.docker.io/v1/ Labels: Experimental: false Insecure Registries: 127.0.0.0/8 Live Restore Enabled: false

WARNING: No swap limit support

thaJeztah commented 6 years ago

Hm, I'm not sure I agree with proceeding in that case; by passing the --pull flag, you're explicitly asking. docker to pull the image; failing the build of the pull fails seems the right action here (instead of silently ignoring that the pull failed)

@tonistiigi @cpuguy83 WDYT?

tonistiigi commented 6 years ago

@thaJeztah Yes, possible security problem if we would fallback. There could be a special syntax for specifying the desired behavior here though. Not sure what that would look like.

scaytrase commented 6 years ago

Currently we have to overcome it with

(docker pull parent || docker build parent) && docker build child

to make sure that parent image is fresh or pulled from outer repo, since the docker --pull build parent just fails if pull fail.

Current behavior of docker --pull build renders like

(docker pull parent && docker build parent)

part instead, which gives us less flexibility

thaJeztah commented 6 years ago

I'm having trouble understanding the use-case; by passing --pull to docker build you indicate that the registry should be the source of truth (i.e., use the latest version of the image as available in the registry), but your description doesn't match that expectation.

Without `--pull`

Situation 1A

No mybaseimage present in the registry, and no mybaseimage present locally:

build FROM mybaseimage:latest
build fails (mybaseimage is not found)

Situation 2A

No mybaseimage present in the registry, and no mybaseimage present locally:

build mybaseimage
build FROM mybaseimage:latest
build succeeds (using the local mybaseimage that was just built)

Situation 3A

No mybaseimage present in the registry, and no mybaseimage present locally:

build mybaseimage
push mybaseimage
build FROM mybaseimage:latest
build succeeds (using the local mybaseimage)

Situation 4A

No mybaseimage present in the registry, and no mybaseimage present locally:

build mybaseimage
push mybaseimage
build new version of mybaseimage (local image is now more current than mybaseimage in the registry)
build FROM mybaseimage:latest
build succeeds (using the local mybaseimage)

Situation 5A

mybaseimage present in the registry, but no mybaseimage present locally:

build FROM mybaseimage:latest
pulls mybaseimage:latest from the registry (because no image was present locally)
build succeeds (using the mybaseimage that was just pulled)

With `--pull`

Situation 1B

No mybaseimage present in the registry, and no mybaseimage present locally:

build --pull FROM mybaseimage:latest
build fails (mybaseimage is not found in registry)

Situation 2B

No mybaseimage present in the registry, and no mybaseimage present locally:

build mybaseimage
build --pull FROM mybaseimage:latest
build fails (mybaseimage is not found in registry)

Situation 3B

No mybaseimage present in the registry, and no mybaseimage present locally:

build mybaseimage
push mybaseimage
build --pull FROM mybaseimage:latest
- pulls mybaseimage:latest, replacing the local mybaseimage:latest
build succeeds (using the mybaseimage that was just pulled)

Situation 4B

No mybaseimage present in the registry, and no mybaseimage present locally:

build mybaseimage
push mybaseimage
build new version of mybaseimage (local image is now more current than mybaseimage in the registry)
build --pull FROM mybaseimage:latest
- pulls mybaseimage:latest, replacing the local mybaseimage:latest
build succeeds (using the old mybaseimage that was just pulled)

Situation 5B

mybaseimage present in the registry, but no mybaseimage present locally:

build --pull FROM mybaseimage:latest
- pulls mybaseimage:latest from the registry
build succeeds (using the mybaseimage that was just pulled)

Clarification of the use-case

Perhaps you can describe your use-case a bit more; what's the reason for adding --pull?

Your problem description matches situation 2B, but the desired behavior from your problem description matches situation 2A (run docker build without --pull)
Do you want the "latest" available image to be used?
If so: the latest from the registry, or the latest local image, or perhaps the latest in "general" (either local or from the registry)?

I think it's not good idea to manipulate the flag value base on knownledge, whether FROM already pushed to remote repo or not

Using --pull without this knowledge can be problematic: Situation 4B describes why it can be problematic; when using --pull in that situation, you will always overwrite your local image, potentially loosing the latest version of the image that you just built.

Possible enhancements

Based on the above, I agree with @tonistiigi that we should not change the behavior of --pull in its current form.

Having said that, I can see possible improvements, for example:

--pull=always current behavior: always attempt to pull the image, and fail if the image could not be pulled
--pull=changed pull if the image's digest on the registry is different than the local image (does not have to be "newer")
--pull=newer pull if the image in the registry was built (or pushed?) more recently than the image that's available locally (to prevent overwriting a local image)
- this would involve fetching the image configuration from the registry, and comparing the .Created date/time

scaytrase commented 6 years ago

@thaJeztah yes, the use-case is to update baseimage from remote registry if updated version available. This is needed for the CI cince images are stored on per-agent basis, so without pulling attempt we get the folloiwing:

build nr. 1, agent 1: build image_v1 - no base found, build baseimage_v1, push both build nr. 2, agent 2: build image_v2 - no base found, build baseimage_v2, push both build nr. 3, agent 1: build image_v3 - baseimage_v1 found - use stale image, push image_v3 based on it

your enhancement ideas looks good for me, I see both --pull=newer and --pull=changed solving our problem

thaJeztah commented 6 years ago

so without pulling attempt we get the folloiwing:

With the "fallback" (as you originally proposed), that problem wouldn't be solved, as in that case, the flow would look like:

build nr. 3, agent 1: build image_v3 -> pull baseimage -> failed -> local baseimage_v1 found -> use stale image, push image_v3 based on it

So, yes, newer or changed could address that, but would still raise the question if build should succeed when failing to pull (i.e. image does not exist in the registry at all). Falling back to a local image when requesting an image from the registry can be a security concern (i.e., it's easy to "spoof" the local image cache, and (say) docker build -t ubuntu:16.04 FROM myubuntu:bitcoin-miner

scaytrase commented 6 years ago

I think the whole local cache itself exposes a security vulnerability, so using --pull=always by default should make thing as safe as before, allowing to explicitly use cache with --pull=newer. Btw --pull=changed should also be safe as soon as local image is always changed against remote if spoofed

moby / moby