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

docker build: initial work on the include command #2108

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@flavio
Contributor

flavio commented Oct 7, 2013

This is some initial work to add the 'include' command to dockerfile build as suggested by issue #735.

When the client starts a build process it creates a tarball of the whole directory containing the Dockerfile and it sends it to the server. Hence including a file which is located outside of the directory containing the Dockerfile (eg: include ../common/server_stuff) would result in a "file not found" error on the server site. Trying to circumvent this issue with symbolic links won't help, since on the server side they will still be broken.

Including remote files works fine. Nested includes work fine too as long as they do not reference external files.

I think we could instruct the client to look for external include statements inside of the Dockerfile and take care of them by 1) temporarily copying these files into the working directory 2) update the path to the file to include. I'm not completely satisfied by this approach, but I do not have other ideas...

A couple of final notes:

  • I'm still new to go, keep that in mind while reviewing my patch :)
  • I updated the unit tests but I have not been able to run them. I got some failures even without my changes. I'm running the tests using `sudo go test'.
@dsissitka

This comment has been minimized.

Show comment
Hide comment
@dsissitka
Contributor

dsissitka commented Oct 7, 2013

docker build: initial work on the include command
Added the 'include' command to dockerfile build as suggested by issue #735.
Right now the include command works only with files in the same directory of
the main Dockerfile or with remote ones.
@shykes

This comment has been minimized.

Show comment
Hide comment
@shykes

shykes Nov 11, 2013

Collaborator

Hey guys, I apologize for the slow review.

I am not comfortable merging an "INCLUDE" instruction as described here. It feels like C-style macros: it doesn't operate at the same level as the rest of the language, so introduces an extra layer of complexity and can cause side effects. For example if I include a snippet with a "FROM", it may reset the build process, or cause it to fail.

I agree there is a need to compose builds beyond what the current FROM can do. I think the long-term solution is to upgrade FROM so that it can point to a source, not just a build image. That source could be a local directory, or a remote repo (similar to ADD). It would then be built recursively, and used as a base image.

Thanks guys for taking the time, and sorry for the disappointment. Always happy to discuss design on #docker-dev or the mailing list.

Collaborator

shykes commented Nov 11, 2013

Hey guys, I apologize for the slow review.

I am not comfortable merging an "INCLUDE" instruction as described here. It feels like C-style macros: it doesn't operate at the same level as the rest of the language, so introduces an extra layer of complexity and can cause side effects. For example if I include a snippet with a "FROM", it may reset the build process, or cause it to fail.

I agree there is a need to compose builds beyond what the current FROM can do. I think the long-term solution is to upgrade FROM so that it can point to a source, not just a build image. That source could be a local directory, or a remote repo (similar to ADD). It would then be built recursively, and used as a base image.

Thanks guys for taking the time, and sorry for the disappointment. Always happy to discuss design on #docker-dev or the mailing list.

@drewcrawford

This comment has been minimized.

Show comment
Hide comment
@drewcrawford

drewcrawford Jan 28, 2014

So I understand @shykes' position (in this thread and more clearly enumerated on #3562), but I think some things are missing from the discussion. #3562 is not an effective replacement for INCLUDE: there are a lot of workflows that #3562 either cannot do at all, or cannot do as well as an include verb. So I mean maybe #3562 is a good idea, but it's not an effective substitute for an include verb and so "we should do #3562 instead" is not really responsive to the problems that motivate(d) INCLUDE.

a la carte dockerfiles

One case is multiple inheritance. Right now, in the real world, my dockerfiles have common "units". For example, it's common to install postgres, it's common to install python, it's common to install supervisor... Right now my Dockerfile is organized like

# postgres stuff
RUN ...
RUN ...
RUN ...

# python stuff
RUN ...
RUN ...
RUN ...

# etc

whereas with import it could be

import python.dockerfile
import postgres.dockerfile

This allows me to better share that "how do I install postgres" work between different images. It's possible to achieve this today with adding and running shell scripts, but that's clumsy, and doesn't do things like volumes or ports (which are important, for example, in the postgres case). @shykes solution, as I understand it, doesn't have the flexibility that would allow me to define high-level "install postgres" actions and insert them a la carte into a docker container. Import does.

It's worth pointing out that "how do I install [complicated package] in a separate file" problem is the problem people are filing that gets duped to this issue.

Any serious proposal in lieu of include should address this underlying motivation for include: composing Docker images from smaller units. #3562 doesn't do that, like, at all.

identifier hell

Over on #3763, I am trying to run two docker commands back-to-back, and I am annoyed that I have to save some GUID that's the output of one command and pipe it to the other. Like, the power of having two steps is great, but, in the common case, I don't need that power, and so there is this bookkeping step of piping this GUID from one step to the next that overcomplicates a core workflow.

The solution in #3562, in addition to being nonresponsive to the core problem, has a similar bookkeeping problem, in that: the common case is that I want to build one image, that for developer convenience is split up into multiple files. But in order to do that I have to introduce this abstract notion of a source and start to worry about what the source should be named and where it should be located on the filesystem and this clutters up listing images and such and now I am at least five minutes into the README learning a constellation of nouns and meanwhile all I wanted to do was refactor a few lines from my Dockerfile into another file.

drewcrawford commented Jan 28, 2014

So I understand @shykes' position (in this thread and more clearly enumerated on #3562), but I think some things are missing from the discussion. #3562 is not an effective replacement for INCLUDE: there are a lot of workflows that #3562 either cannot do at all, or cannot do as well as an include verb. So I mean maybe #3562 is a good idea, but it's not an effective substitute for an include verb and so "we should do #3562 instead" is not really responsive to the problems that motivate(d) INCLUDE.

a la carte dockerfiles

One case is multiple inheritance. Right now, in the real world, my dockerfiles have common "units". For example, it's common to install postgres, it's common to install python, it's common to install supervisor... Right now my Dockerfile is organized like

# postgres stuff
RUN ...
RUN ...
RUN ...

# python stuff
RUN ...
RUN ...
RUN ...

# etc

whereas with import it could be

import python.dockerfile
import postgres.dockerfile

This allows me to better share that "how do I install postgres" work between different images. It's possible to achieve this today with adding and running shell scripts, but that's clumsy, and doesn't do things like volumes or ports (which are important, for example, in the postgres case). @shykes solution, as I understand it, doesn't have the flexibility that would allow me to define high-level "install postgres" actions and insert them a la carte into a docker container. Import does.

It's worth pointing out that "how do I install [complicated package] in a separate file" problem is the problem people are filing that gets duped to this issue.

Any serious proposal in lieu of include should address this underlying motivation for include: composing Docker images from smaller units. #3562 doesn't do that, like, at all.

identifier hell

Over on #3763, I am trying to run two docker commands back-to-back, and I am annoyed that I have to save some GUID that's the output of one command and pipe it to the other. Like, the power of having two steps is great, but, in the common case, I don't need that power, and so there is this bookkeping step of piping this GUID from one step to the next that overcomplicates a core workflow.

The solution in #3562, in addition to being nonresponsive to the core problem, has a similar bookkeeping problem, in that: the common case is that I want to build one image, that for developer convenience is split up into multiple files. But in order to do that I have to introduce this abstract notion of a source and start to worry about what the source should be named and where it should be located on the filesystem and this clutters up listing images and such and now I am at least five minutes into the README learning a constellation of nouns and meanwhile all I wanted to do was refactor a few lines from my Dockerfile into another file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment