Support ONBUILD instruction in Dockerfiles by orzeh · Pull Request #393 · docker-java/docker-java · GitHub
Skip to content

Support ONBUILD instruction in Dockerfiles#393

Merged
marcuslinke merged 2 commits into
docker-java:masterfrom
orzeh:master
Dec 8, 2015
Merged

Support ONBUILD instruction in Dockerfiles#393
marcuslinke merged 2 commits into
docker-java:masterfrom
orzeh:master

Conversation

@orzeh

@orzeh orzeh commented Dec 7, 2015

Copy link
Copy Markdown
Contributor

I have rewritten the logic that adds files to the build context: now it adds all files in Dockerfile parent directory, except those excluded by .dockerignore.

I have also added some tests that demonstrates the issue, which use ONBUILD in Dockerfile.

I also fixed small bug in .dockerignore test: exclude pattern was wrong according to the Dockerfile doc.

@marcuslinke

Copy link
Copy Markdown
Contributor

@marcuslinke

Copy link
Copy Markdown
Contributor

@orzeh After thinking about again probably you're right. So uploading all files and ignore only those that matches patterns in .dockerignore. Seems our original implementation that adds only files mentioned in the Dockerfile was some kind of premature optimization then.

@orzeh

orzeh commented Dec 8, 2015

Copy link
Copy Markdown
Contributor Author

@marcuslinke I agree with you.

If one has to deal with huge files, he can use .dockerignore or - to take full control over what is send to docker daemon - provide own InputStream via BuildImageCmd.withTarInputStream().

@KostyaSha

Copy link
Copy Markdown
Member

Wouldn't better place Dockerfile in separate directory?

@orzeh

orzeh commented Dec 8, 2015

Copy link
Copy Markdown
Contributor Author

@KostyaSha which Dockerfile do you mean?

@KostyaSha

Copy link
Copy Markdown
Member

Any :) from my experience i always placing Dockerfile in clean directory to not have accidental uploads to build context.

@KostyaSha

Copy link
Copy Markdown
Member

In general PR LGTM 👍 even if i not fully understand ONBUILD feature and all possible cases :)

@orzeh

orzeh commented Dec 8, 2015

Copy link
Copy Markdown
Contributor Author

Ok, now I get it.

I use ONBUILD when I have several Spring Boot microservices and I add them to the image as a exploded JAR to leverage Docker caching (see this blog post). Then I create base image with number of ONBUILD ADD ... instructions and just put FROM my-base-image in microservices Dockerfiles.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@orzeh Could you explain this change please?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

According to .dockerignore doc:

the root of the context is considered to be both the working and the root directory

then pattern b excludes all files and directories that are named b in root directory of build context.
However, in this test the file named b is under directory a, so it should not be ignored. After my changes this tests fails.

Pattern */b works as expected (and described in the doc see */temp* example)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK, now I got it. Thanks for explanation.

@marcuslinke marcuslinke merged commit 9653b70 into docker-java:master Dec 8, 2015
@marcuslinke marcuslinke changed the title Fixes #219 Support ONBUILD instruction in Dockerfiles Dec 8, 2015
@marcuslinke

Copy link
Copy Markdown
Contributor

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants