Make `FROM scratch` a special cased 'no-base' spec by jlhawn · Pull Request #8827 · moby/moby

jlhawn · 2014-10-28T22:02:54Z

Make FROM scratch a special cased 'no-base' spec

There has been a lot of discussion (issues 4242 and 5262) about making
FROM scratch either a special case or making FROM optional, implying
starting from an empty file system.

This patch makes the build command FROM scratch special cased from now on
and if used does not pull/set the the initial layer of the build to the ancient
image ID (511136ea..) but instead marks the build as having no base image. The
next command in the dockerfile will create an image with a parent image ID of "".
This means every image ever can now use one fewer layer!

This also makes the image name scratch a reserved name by the TagStore. You
will not be able to tag an image with this name from now on. If any users
currently have an image tagged as scratch, they will still be able to use that
image, but will not be able to tag a new image with that name.

Goodbye '511136ea3c5a64f264b78b5433614aec563103b4d4702f3ba7d4d2698e22c158',
it was nice knowing you.

Fixes #4242

jlhawn · 2014-10-28T22:16:20Z

This modifies more files than necessary only because I renamed Container.Image to Container.ImageID.

thaJeztah · 2014-10-28T22:49:40Z

I think this may affect a lot of beginning docker users if they forget to add a FROM? Not against it per se, but will make it harder to detect if someone left out the FROM deliberately or by accident.

Will require doc changes as well

erikh · 2014-10-28T22:50:39Z

@jlhawn is this necessary? We might re-introduce this requirement with another proposal I'm working on for Dockerfile v2.

erikh · 2014-10-28T22:51:49Z

to clarify: I like scratch being "special" but I don't think the implicit FROM is going to work out in the long run.

jlhawn · 2014-10-28T23:13:24Z

@erikh @thaJeztah There was discussion about this in #4242 @shykes, @vieux and a few others seemed to be in agreement that making FROM optional was the more elegant solution over special casing the name. Feel free to have more discussion on the issue over in that thread.

I'll update this with a change to the tests and documentation.

erikh · 2014-10-28T23:39:31Z

Like I said, we're going to be fixating the FROM requirement in Dockerfile v2. It is not going to be healthy for our users to have this changed twice.

erikh · 2014-10-29T17:39:25Z

daemon/daemon.go

what happens here if the imgID is an empty string?

Nothing happens at all :)

The container create logic just passes the image id string to the storage graph driver which knows how to handle it when making the container init layer.

It doesn't affect users at all (You can't do docker run "" for example) but if you do docker ps you get something like this (I ran docker build -t busybox --rm=false --no-cache .):

CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES ce55b20a0180 8a3051d7cf1a "/bin/sh -c '#(nop) 28 hours ago suspicious_pike 8e1877d9055b bbab8f8ff236 "/bin/sh -c '#(nop) 28 hours ago furious_bartik 907893df670c "/bin/sh -c '#(nop) 28 hours ago silly_ptolemy

and docker inspect 907893df670c shows the container's image as "" also.

Hmm. Can you adjust ps to say “scratch” or “” or something similar? That might be nice to clarify what’s going on here.

On Oct 29, 2014, at 10:51 AM, Josh Hawn [email protected] wrote:

In daemon/daemon.go:

@@ -575,7 +575,7 @@ func (daemon *Daemon) newContainer(name string, config *runconfig.Config, img *i
Args: args, //FIXME: de-duplicate from config
Config: config,
hostConfig: &runconfig.HostConfig{},

Image: img.ID, // Always use the resolved image id

ImageID: imgID,
Nothing happens at all :)

The container container create logic just passes the image id string to the storage graph driver which knows how to handle it when making the container init layer.

It doesn't affect users at all (You can't do docker run "" for example) but if you do docker ps you get something like this (I ran docker build -t busybox --rm=false --no-cache .):

CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
ce55b20a0180 8a3051d7cf1a "/bin/sh -c '#(nop) 28 hours ago suspicious_pike
8e1877d9055b bbab8f8ff236 "/bin/sh -c '#(nop) 28 hours ago furious_bartik
907893df670c "/bin/sh -c '#(nop) 28 hours ago silly_ptolemy
and docker inspect 907893df670c shows the container's image as "" also.

—
Reply to this email directly or view it on GitHub https://github.com/docker/docker/pull/8827/files#r19557572.

Here's the context of the docker build -t busybox ... from above. I used Jerome's busybox repo: https://github.com/jpetazzo/docker-busybox

but modified the Dockerfile to remove FROM scratch:

MAINTAINER Jérôme Petazzoni <[email protected]> ADD rootfs.tar / CMD ["/bin/sh"]

Hmm. Can you adjust ps to say “scratch” or “” or something similar? That might be nice to clarify what’s going on here.

I thought leaving it empty makes it pretty clear. It also follows what the other fields do when there's nothing to display, for example, thePORTS columns don't say <no ports>, it just leaves it empty.

Also, that field typically only shows an image ID or tagged repository name. I don't want to introduce a placeholder name like scratch since it would cause an issue if the user actually had a legitimate image named scratch. I'm agreeable to a placeholder like <no image> if other people are okay with it as long as it's on the client side and isn't something the daemon sets in the api response. I think the API responses for GET /containers/json and GET /containers/(id)/json should still return something with {"image": ""} in this case.

+1 on <no image> or simply <none>; it's just a bit clearer that the image column is deliberately empty (maybe the same should be applied for other columns?

If people want to grep for this, <no image> is probably more helpful than <none>

@thaJeztah @erikh I've added <no image> to what the client will now output on docker ps with the latest update.

erikh · 2014-10-29T17:40:36Z

Once my comments are addressed, LGTM

tiborvass · 2014-10-29T18:43:45Z

I hope this is not a huge blocker for you @jlhawn but I'd prefer to wait a little on this one, as we're rethinking Dockerfiles and versioning of them and this might actually be a nuisance in a scenario where we want to take advantage of the fact that all current Dockerfiles have a FROM statement in the beginning. Will get back to this PR as shortly as I can :)

jlhawn · 2014-10-29T22:00:34Z

With the latest change, we're now back to requiring FROM <ARG> at the top of the Dockerfile. scratch is a reserved image name going forward (you cannot tag an image as scratch) but you can continue to use any currently existing image which happens to be named scratch. FROM scratch is a special build directive which sets the build to have no base image, causing the subsequent build command to produce an image with no parent.

erikh · 2014-10-29T22:08:38Z

ok. I’ll review this tonight, thanks!

On Oct 29, 2014, at 3:00 PM, Josh Hawn [email protected] wrote:

With the latest change, we're now back to requiring FROM at the top of the Dockerfile. scratch is a reserved image name going forward (you cannot tag an image as scratch) but you can continue to use any currently existing image which happens to be named scratch. FROM scratch is a special build directive which sets the build to have no base image, causing the proceeding build command to produce an image with no parent.

—
Reply to this email directly or view it on GitHub #8827 (comment).

thaJeztah · 2014-10-29T22:22:22Z

Integration tests are failing on drone?

Update; nvm, see you updated the PR

jlhawn · 2014-10-29T22:24:57Z

one of the integration tests try to tag something as scratch which my latest change doesn't allow. I fixed that, then found out there are two other tests which assume an image exists named scratch, but I changed it to docker_scratch. I just pushed to trigger tests on drone since it's faster than my dev VM at building and running the suite.

thaJeztah · 2014-10-29T22:28:39Z

Alright then, didn't know if you would notice it before Erik would start reviewing :)

jlhawn · 2014-10-29T22:34:06Z

meh, I'll just work these out locally. don't mind the red X for now, it's just minor issues.

erikh · 2014-10-30T05:38:31Z

I think it needs a quick docs pass to ensure "scratch" is explained well, but LGTM from me.

jlhawn · 2014-10-30T16:48:48Z

Will do, but before I start, I'd like a definitive 'yes, we're going with this' from the core maintainers.

tiborvass · 2014-10-30T16:50:15Z

@jlhawn we'll have to do a design review on this with @shykes but you have my +1.

tianon · 2014-10-31T06:58:32Z

As I mentioned on the issue, I'm very strongly -1 on this because of the
errors it could obscure, and find an explicit "please make this image start
from nothing" to be much more transparent. If you add this together with
skipping unknown instructions, then we could have someone misspell FROM
and inadvertently end up with an empty broken image, as a concrete example
of one place this could cause issues that get unnecessarily harder to then
diagnose.

jessfraz · 2014-10-31T21:46:37Z

I honestly don't know about this one, I am uneasy as it might be confusing for
new users who forget the FROM directive. Plus the comments made by tianon above.

thaJeztah · 2014-10-31T21:55:16Z

@jfrazelle @tianon I think this PR was changed after starting; initially it made the FROM statement optional, but after that it was changed to make FROM scratch a "special case" and start with an empty image in stead of pulling the "scratch" image from the registry.

Yup, it's confusing 😄

jessfraz · 2014-10-31T21:59:51Z

oooo ok wow, now caught up

jessfraz · 2014-10-31T22:01:02Z

@jlhawn can you confirm the above is true, and its not removing the FROM line

jlhawn · 2014-12-16T21:18:16Z

@tiborvass it's back to being merge-able. can you and @erikh, and anyone else you need, please review

erikh · 2014-12-16T21:25:28Z

Will do tonight

On Dec 16, 2014, at 1:18 PM, Josh Hawn [email protected] wrote:

@tiborvass https://github.com/tiborvass it's back to being merge-able. can you and @erikh https://github.com/erikh, and anyone else you need, please review

—
Reply to this email directly or view it on GitHub #8827 (comment).

erikh · 2014-12-17T09:41:51Z

LGTM

jlhawn · 2014-12-17T20:00:53Z

thanks @erikh!

what do you think @tiborvass, @tianon ?

tianon · 2014-12-17T22:57:52Z

integration-cli/docker_cli_events_test.go

If you're just looking for something tiny, the hello-world image is even smaller than busybox. 😉

♥ hello-world would make test execution faster.

Oh I missed this one. I meant to switch it to docker_scratch too. We can't use scratch anymore because the tagStore will refuse to install it with that name. Do you recommend that I switch this to hello-world in the other location (https://github.com/docker/docker/pull/8827/files#r22012185) too?

jlhawn · 2014-12-18T19:19:58Z

I've rebased again. I also modified some test cases to pull the small hello-world image instead of busybox or docker_scratch

tianon · 2014-12-18T20:31:53Z

The new public top-level image docker_scratch still makes me sick to my stomach, but otherwise the feature LGTM.

jlhawn · 2014-12-18T20:39:47Z

@tianon It should be safe to remove it now since the tests shouldn't be pulling it any more.

There has been a lot of discussion (issues 4242 and 5262) about making `FROM scratch` either a special case or making `FROM` optional, implying starting from an empty file system. This patch makes the build command `FROM scratch` special cased from now on and if used does not pull/set the the initial layer of the build to the ancient image ID (511136ea..) but instead marks the build as having no base image. The next command in the dockerfile will create an image with a parent image ID of "". This means every image ever can now use one fewer layer! This also makes the image name `scratch` a reserved name by the TagStore. You will not be able to tag an image with this name from now on. If any users currently have an image tagged as `scratch`, they will still be able to use that image, but will not be able to tag a new image with that name. Goodbye '511136ea3c5a64f264b78b5433614aec563103b4d4702f3ba7d4d2698e22c158', it was nice knowing you. Fixes moby#4242 Docker-DCO-1.1-Signed-off-by: Josh Hawn <[email protected]> (github: jlhawn)

jlhawn · 2014-12-18T22:06:28Z

@tianon I've updated the tests to work with an image it creates called emptyfs and removed the docker_scratch image I created from the official registry.

@unclejack once the current test/build passes this should be okay for you to review again.

tianon · 2014-12-18T22:15:41Z

🎉 👍 ❤️

unclejack · 2014-12-18T22:24:52Z

LGTM

jlhawn · 2014-12-18T22:28:57Z

tiborvass · 2014-12-18T23:21:10Z

LGTM

Make `FROM scratch` a special cased 'no-base' spec

tiborvass · 2014-12-18T23:21:53Z

@jlhawn Thank you for your patience and for flying DockAir :)

erikh self-assigned this Oct 28, 2014

SvenDowideit added the /project/doc label Oct 29, 2014

erikh reviewed Oct 29, 2014
View reviewed changes

jlhawn force-pushed the build_implied_from_scratch branch from 30910e6 to 549ddae Compare October 29, 2014 21:55

jlhawn changed the title ~~Make FROM optional for base image Dockerfiles~~ Make FROM scratch a special cased 'no-base' spec Oct 29, 2014

jlhawn force-pushed the build_implied_from_scratch branch 2 times, most recently from 85d76cf to eb9ddfd Compare October 29, 2014 22:22

jlhawn force-pushed the build_implied_from_scratch branch from eb9ddfd to 965339a Compare October 29, 2014 22:46

tianon reviewed Dec 17, 2014
View reviewed changes

crosbymichael added this to the 1.5.0 milestone Dec 18, 2014

jlhawn force-pushed the build_implied_from_scratch branch from 411eaa6 to e2dd604 Compare December 18, 2014 19:18

jlhawn force-pushed the build_implied_from_scratch branch from e2dd604 to 8936789 Compare December 18, 2014 22:03

tiborvass added a commit that referenced this pull request Dec 18, 2014

Merge pull request #8827 from jlhawn/build_implied_from_scratch

610842f

Make `FROM scratch` a special cased 'no-base' spec

tiborvass merged commit 610842f into moby:master Dec 18, 2014

SvenDowideit mentioned this pull request Jan 27, 2015

Post 1.4.1 docs update 2 #9739

Merged

wallrj mentioned this pull request Apr 27, 2015

[FLOC-1746] Null environment check ClusterHQ/flocker#1340

Merged

tianon mentioned this pull request Jun 12, 2015

Is it normal that image id b7cf8f0d9e82 isn't based on image id 511136ea3c5a (scratch) like the previous ones ? tianon/docker-brew-ubuntu-core#30

Closed

jlhawn deleted the build_implied_from_scratch branch July 31, 2015 18:04

tiborvass removed their assignment Nov 3, 2015

codefromthecrypt mentioned this pull request Oct 27, 2020

Publishes to GitHub Container Registry instead of Docker Hub openzipkin/brave-example#66

Merged

floreks mentioned this pull request Oct 24, 2022

Support for different baseOS images kubernetes-retired/dashboard#7305

Closed

thaJeztah mentioned this pull request Aug 22, 2023

c8d: Fix building Dockerfiles that have FROM scratch #46284

Merged

dennis-iv mentioned this pull request Nov 20, 2024

[Bug]: Can't Build Image from Dockerfile with scratch Base Image testcontainers/testcontainers-dotnet#1303

Closed

Conversation

jlhawn commented Oct 28, 2014

Uh oh!

jlhawn commented Oct 28, 2014

Uh oh!

thaJeztah commented Oct 28, 2014

Uh oh!

erikh commented Oct 28, 2014

Uh oh!

erikh commented Oct 28, 2014

Uh oh!

jlhawn commented Oct 28, 2014

Uh oh!

erikh commented Oct 28, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erikh commented Oct 29, 2014

Uh oh!

tiborvass commented Oct 29, 2014

Uh oh!

jlhawn commented Oct 29, 2014

Uh oh!

erikh commented Oct 29, 2014

Uh oh!

thaJeztah commented Oct 29, 2014

Uh oh!

jlhawn commented Oct 29, 2014

Uh oh!

thaJeztah commented Oct 29, 2014

Uh oh!

jlhawn commented Oct 29, 2014

Uh oh!

erikh commented Oct 30, 2014

Uh oh!

jlhawn commented Oct 30, 2014

Uh oh!

tiborvass commented Oct 30, 2014

Uh oh!

tianon commented Oct 31, 2014

Uh oh!

jessfraz commented Oct 31, 2014

Uh oh!

thaJeztah commented Oct 31, 2014

Uh oh!

jessfraz commented Oct 31, 2014

Uh oh!

jessfraz commented Oct 31, 2014

Uh oh!

jlhawn commented Dec 16, 2014

Uh oh!

erikh commented Dec 16, 2014

Uh oh!

erikh commented Dec 17, 2014

Uh oh!

jlhawn commented Dec 17, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jlhawn commented Dec 18, 2014

Uh oh!

tianon commented Dec 18, 2014

Uh oh!

jlhawn commented Dec 18, 2014