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

ADD enhancement: allow adding from another image #4933

Closed
wants to merge 5 commits into from

Conversation

vbatts
Copy link
Contributor

@vbatts vbatts commented Mar 31, 2014

Rather than adding an additional buildfile instruction, this enhancement
will use a mock-URI prefix of "image://", followed by a image name, and
a ":" suffix. (i.e. image://[registry[:port]][:tag]: )

Such that if you have a container that has a built artifact, you can
copy it over to a new/different image.

FROM vbatts/minimal-runtime

ADD image://vbatts/fat-buildtime:/build/server /server

CMD "/server"

Docker-DCO-1.1-Signed-off-by: Vincent Batts vbatts@redhat.com (github: vbatts)

Rather than adding an additional buildfile instruction, this enhancement
will use a mock-URI prefix of "image://", followed by a image name, and
a ":<path>" suffix. (i.e. image://[registry[:port]]<name>[:tag]:<path> )

Such that if you have a container that has a built artifact, you can
copy it over to a new/different image.

```
FROM vbatts/minimal-runtime

ADD image://vbatts/fat-buildtime:/build/server /server

CMD "/server"
```

Docker-DCO-1.1-Signed-off-by: Vincent Batts <vbatts@redhat.com> (github: vbatts)
@vbatts
Copy link
Contributor Author

vbatts commented Mar 31, 2014

@shykes I expect this mock-URI will have overlap with the notion of provenance, but I would like to get this conversation started.

@jamtur01
Copy link
Contributor

This definitely needs docs.

@vbatts
Copy link
Contributor Author

vbatts commented Mar 31, 2014

@jamtur01 true fact. I was hoping to iron out the functionality, before having to iterate constantly over the docs while determining the layout of it.

@SvenDowideit
Copy link
Contributor

@vbatts - on the other hand, if you write the simplest docs you can, we all know what you intended, and can figure out the weird ideas.

my first weird q - what happens when the ADDed path is a tgz - is it 'consistently' expanded?

if i read the tests right, you're only testing the parsing? those will be interesting integration tests :)

(I want this so very much)

@vbatts
Copy link
Contributor Author

vbatts commented Apr 2, 2014

@SvenDowideit I just pushed some docs, but not sure whether to enumerate examples like this in the docs. Like

ADD image://test:/etc.tar.gz /mnt

will result in the file 'etc.tar.gz' being created as a file called 'mnt', or where

ADD image://test:/etc.tar.gz /mnt/

will result in the file 'etc.tar.gz' being copied to '/mnt/etc.tar.gz'

@SvenDowideit
Copy link
Contributor

@vbatts yeah, I'm not suggesting to enumerate all examples in the docs - the tgz one is 'special' this just means we're treating image:// as non-local. Perhaps the easy solution is to change the docs where it says If <src> is a local tar archive in a to If <src> is a tar archive in the uploaded context, in a.

@jamtur01 second opinion?

ooo: I suddenly realize - can you make the last example a little more explicit please? If I understand right, ADD image://my.registry.lan:5000/vbatts/fat-buildtime:stable:/build/runtime/bin/myapp / would grab that path from the local copy (possibly outdated), or if it does not we just error out?

ie, there isn't some magic in the registry that will just send it the one file it wants (which imo would be an awesome feature :) )

And lastly, I wonder how we can hook this together with being able to use multiple FROM statements - ie ADD image://2imagesbeforethis:/path /binaries :) (no, not a request for this PR)

@crosbymichael @shykes

Docker-DCO-1.1-Signed-off-by: Vincent Batts <vbatts@redhat.com> (github: vbatts)
A command is required for the context setup as the <src> image.

Docker-DCO-1.1-Signed-off-by: Vincent Batts <vbatts@redhat.com> (github: vbatts)
If the image named in the ``image://`` src is not present, then pull it.

Docker-DCO-1.1-Signed-off-by: Vincent Batts <vbatts@redhat.com> (github: vbatts)
@vbatts
Copy link
Contributor Author

vbatts commented Apr 3, 2014

@SvenDowideit regarding ADD image://my.registry.lan:5000/vbatts/fat-buildtime:stable:/build/runtime/bin/myapp /, if my.registry.lan:5000/vbatts/fat-buildtime:stable was not found locally, then it would attempt to pull it. If the image is outdated, then you'd get outdated content. Same as docker run, right?

As for magically copying a path, from an image, on a remote registry... not yet :-)

@vbatts
Copy link
Contributor Author

vbatts commented Apr 3, 2014

though, i'm still hoping to hear from @shykes and/or @crosbymichael on the layout of the image:// [mock] URI

@gabrtv
Copy link

gabrtv commented Apr 21, 2014

+1. Moving build artifacts to a minimal runtime is a really important feature outside the world of interpreted languages. The ADD instruction seems the right place for this functionality. I'm "meh" on the image URI since it seems a little verbose for Dockerfile syntax, but I don't really know a good alternative.

If this gets the OK from the maintainers (I hope it does soon!) I'm happy to help with testing.

}

base_str := strings.TrimPrefix(str, "image://")
i := strings.LastIndex(base_str, ":")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not IPv6 compatible. Why not simply use url.Parse from "net/url"

http://play.golang.org/p/kClKARcxHc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worked with url.Parse, but to find an image schema that could satisfy referencing a path, and all the permutations of the docker image reference (registry:port/namespace/image:tag), it would best be a bunch of query parameters. Which is even more ugly.

I had not considered the ipv6 address as a registry name. Though this logic should still hold up for an ipv6 address as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK can you add tests with a range of rfc 2732 ipv6 literals - eg [2060::1]

The case I think won't work is an ipv6 address with no port

Docker-DCO-1.1-Signed-off-by: Vincent Batts <vbatts@redhat.com> (github: vbatts)
@vbatts
Copy link
Contributor Author

vbatts commented Apr 21, 2014

@pnasrat i've added several tests, and the parsing looks good.

@pmorie
Copy link
Contributor

pmorie commented Apr 29, 2014

@vbatts It would be great if this could be used with docker run as well - for example, you could have an image that does its own incremental build without a Dockerfile.

@vbatts
Copy link
Contributor Author

vbatts commented May 1, 2014

@pmorie oh interesting, though a more general purpose --add <src>:<dest> would be good, then if it were to support this as well. (something for a separate PR)

@vbatts
Copy link
Contributor Author

vbatts commented May 1, 2014

@shykes and @crosbymichael what are the next steps on this PR?

@vbatts
Copy link
Contributor Author

vbatts commented May 2, 2014

ah @proppy has already proposed an --add flag in #3578

@vbatts
Copy link
Contributor Author

vbatts commented May 14, 2014

@shykes can you please weigh in on this? We are needing for this for an incrementally image build workflow. (Having and --add flag for docker run would sweeten the deal)

@aigarius
Copy link

+1 on wanting this, one possible simplification - just skip the registry specification. It can be fine to force people to pull the image from whatever registry they want explicitly before building (if the image is not available in their default registry). This will leave just image://namespace/image:tag/folder/file and avoid all the hostname lookup and IPv4/v6 logic.

@alexlarsson
Copy link
Contributor

I wrote this blog entry: http://blogs.gnome.org/alexl/2014/06/12/building-clean-docker-images/

In it i use a two-Dockerfile setup to build a clean image. The first container just builds rpms and at the end sets up a yum repo with the result. We then start this container exposing the results with lightttp. The cool thing about this is that we can avoid ADDing the rpms to the target image so that we can then install it, thus avoiding a useless layer in the final image that has the rpms.

@alexlarsson
Copy link
Contributor

The above would be a lot nicer if docker build could use linked containers though. Without it I had to hack it by using the host ip and a global port map.

@proppy
Copy link
Contributor

proppy commented Jun 16, 2014

@alexlarsson I'm curious what you think of #5715, it allow things like: docker build; docker run | docker build.

@shykes
Copy link
Contributor

shykes commented Jul 19, 2014

@vbatts it looks like the primary use case is to build images where the build and run environments
don't have the same base image.

I think instead of adding a way for images to cross-reference each other in ADD, the problem will
be solved by 2 specific improvements:

  1. Nested builds: Proposal: Nested builds #7115

  2. Squashing of build dependencies: Squash build dependencies #6906

Note the 2 new keywords, IN and PUBLISH. The syntax is still up in the air,

With the 2 new keywords proposed in #7115 (IN and PUBLISH), you can compose the contents of multiple images into a
complex build, without having to make a special case in the ADD syntax.

I'm going to close this in favor of #7115. If you feel it doesn't solve your problem, would you mind leaving a comment
there? I'm open to changes and improvements.

Thanks!

@shykes shykes closed this Jul 19, 2014
@vbatts vbatts mentioned this pull request Jul 22, 2014
@TomasTomecek
Copy link
Contributor

I think this is an excellent idea! @shykes any updates what's docker strategy in this area (different build and runtime env)? There are no new comments in #7115 and #6906 for months.

@thaJeztah
Copy link
Member

I think the multi-stage builds (implemented in #31257, #32063, and #32496) largely cover this;

FROM node AS build-env
ADD ./ /app
WORKDIR /app
RUN npm install
RUN npm run build

FROM nginx AS prod-env
COPY --from=build-env /app /var/www/html

Using that Dockerfile, running this command would only perform the steps up to the build-env stage, and tag it as development;

docker build -t development --target build-env .

Whereas omitting the --target would build all stages, and tag the last stage as production

docker build -t production .

There's two open proposals in this area that may be relevant; #32507 (COPY --mount), and #30449 (Proposal: mount from image) - which is for runtime

@thaJeztah thaJeztah added area/builder kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny labels Jul 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/builder kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet