Proposal: TRIGGER Dockerfile instruction #8240

Closed
proppy opened this Issue Sep 25, 2014 · 14 comments

Comments

Projects
None yet
9 participants
@proppy
Contributor

proppy commented Sep 25, 2014

Proposal: TRIGGER Dockerfile instruction

This is a proposal to generalize the ONBUILD Dockerfile instruction into a flexible TRIGGER mecanism.

Rational

ONBUILD is a very straightforward mecanism that is (ab)used the official and other popular base images:
https://registry.hub.docker.com/u/_/golang
https://registry.hub.docker.com/u/google/golang-runtime/
https://registry.hub.docker.com/u/_/node
https://registry.hub.docker.com/u/google/nodejs-runtime

It allows user to easily Dockerize their application, w/ very few instruction in their Dockerfile:

FROM node:onbuild

The ONBUILD instructions get automatically inserted from the parent image just after the FROM.

So if you have a base with:

ONBUILD ADD package.json /app/
ONBUILD RUN npm install
ONBUILD ADD . /app/

and a child image with

FROM base

It will result into

FROM base
ADD package.json /app/
RUN npm install
ADD . /app/

However if you have a child image with:

FROM base
RUN apt-get install -yq fortunes

It will result into:

FROM base
ADD package.json /app/
RUN npm install
ADD . /app/
RUN apt-get install -yq fortunes

Causing the layer w/ apt-get install to be invalidated from the cache everytime you change a file in your context.

Proposal

The TRIGGER instruction allows you to trigger Dockerfile instructions from a parent image.

If a parent image define:

ONBUILD ADD package.json /app/
ONBUILD RUN npm install
ONBUILD ADD . /app/

A child image can decide where to trigger the ONBUILD instructions with:

FROM BASE
RUN apt-get install fortunes
TRIGGER ONBUILD
RUN fortune -m perl

Resulting into:

RUN apt-get install fortunes
ADD package.json /app/
RUN npm install
ADD . /app/
RUN fortune -m perl

The default would still be to TRIGGER ONBUILD just after FROM, if no other TRIGGER ONBUILD instructions is found in the Dockerfile.

Running TRIGGER ONBUILD twice should(?) be forbidden.

Appendix

(#maybe #later #oneday)

The mecanism could be then be extended to allow triggering any Dockerfile instructions annotated in the parent image.

See the over complicated example below:

Base:

@SECURE RUN apt-get update && apt-get install -yq bash
@NPM ADD package.json /app
@NPM RUN npm install
@BOWER ADD bower.json /app
@BOWER RUN bower install
@GULP RUN gulp test
@GULP RUN gulp build

Child:

FROM base IMPORT SECURE, NPM, BOWER, GULP
TRIGGER SECURE
RUN apt-get install fortunes
TRIGGER NPM
RUN curl some/file
TRIGGER BOWER
ADD . /app
TRIGGER GULP  

Result:

FROM base
RUN apt-get update && apt-get install -yq bash
RUN apt-get install fortunes
ADD package.json /app
RUN npm install
RUN curl some/file
ADD bower.json /app
RUN bower install
ADD . /app
RUN gulp test
RUN gulp build

Classical ONBUILD would then be a special case of the more general TRIGGER functionality and downstream image could opt out classical ONBUILD with an empty IMPORT.

IMPORT'ed instructions would br inserted at the corresponding TRIGGER location or after FROM if not triggered explicitly.

Changelog

@duglin

This comment has been minimized.

Show comment
Hide comment
@duglin

duglin Sep 25, 2014

Contributor

I assume the parser would need to scan forward to see if a TRIGGER is present in the child's Dockerfile before it blindly ran the parent's ONBUILD stuff, right?

Contributor

duglin commented Sep 25, 2014

I assume the parser would need to scan forward to see if a TRIGGER is present in the child's Dockerfile before it blindly ran the parent's ONBUILD stuff, right?

@tiborvass

This comment has been minimized.

Show comment
Hide comment
@tiborvass

tiborvass Sep 25, 2014

Collaborator

@duglin that would be the job of the evaluator to check if there is a TRIGGER. The parser will simply parse the dockerfile as-is into S-expressions.

Collaborator

tiborvass commented Sep 25, 2014

@duglin that would be the job of the evaluator to check if there is a TRIGGER. The parser will simply parse the dockerfile as-is into S-expressions.

@erikh

This comment has been minimized.

Show comment
Hide comment
@erikh

erikh Sep 25, 2014

Contributor

Hmm. the @ syntax is likely going to be very hard to support. I'll have a think about this and get back to you, ping me if I forget. :)

Contributor

erikh commented Sep 25, 2014

Hmm. the @ syntax is likely going to be very hard to support. I'll have a think about this and get back to you, ping me if I forget. :)

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Sep 25, 2014

Contributor

What would happen if someone did:

FROM debian
ONBUILD ADD . /foo

TRIGGER ONBUILD

Also, this should probably be restricted as well: ONBUILD TRIGGER <foo>

Contributor

cpuguy83 commented Sep 25, 2014

What would happen if someone did:

FROM debian
ONBUILD ADD . /foo

TRIGGER ONBUILD

Also, this should probably be restricted as well: ONBUILD TRIGGER <foo>

@proppy

This comment has been minimized.

Show comment
Hide comment
@proppy

proppy Sep 25, 2014

Contributor

@erikh I'm not fixed on the syntax, I'm looking for consensus on the behavior :)

@cpuguy83 in the same Dockerfile? It would trigger the ONBUILD from the parent if any.

Contributor

proppy commented Sep 25, 2014

@erikh I'm not fixed on the syntax, I'm looking for consensus on the behavior :)

@cpuguy83 in the same Dockerfile? It would trigger the ONBUILD from the parent if any.

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Sep 25, 2014

Contributor

@proppy Yep, same file.

Contributor

cpuguy83 commented Sep 25, 2014

@proppy Yep, same file.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Sep 25, 2014

Member

I like the general concept and possibility of future expansion, it's great to have more control over the triggers. Also agree that the syntax and behavior need some refining. I suggest to create "roadmap" that defines the initial implementation, and described how to improve on that in a later stage.

One thing that comes in mind is to disable or "skip" an ONBUILD instruction defined in a parent image. Possibly this could be covered by the UNSET proposal in #8177

Member

thaJeztah commented Sep 25, 2014

I like the general concept and possibility of future expansion, it's great to have more control over the triggers. Also agree that the syntax and behavior need some refining. I suggest to create "roadmap" that defines the initial implementation, and described how to improve on that in a later stage.

One thing that comes in mind is to disable or "skip" an ONBUILD instruction defined in a parent image. Possibly this could be covered by the UNSET proposal in #8177

@proppy proppy referenced this issue in docker-library/node Oct 1, 2014

Merged

node: add slim variant #19

@proppy

This comment has been minimized.

Show comment
Hide comment
@proppy

proppy Dec 20, 2014

Contributor

Fixed a typo, in the later example (redundant ONBUIL were not indented).

Contributor

proppy commented Dec 20, 2014

Fixed a typo, in the later example (redundant ONBUIL were not indented).

@discordianfish

This comment has been minimized.

Show comment
Hide comment
@discordianfish

discordianfish Dec 20, 2014

Contributor

I really like this proposal but I would be cool to make it more explicit that those 'macros' are things defined in the parent Dockerfile. This could be done by extending the FROM to specify which "macros" you want to use (based on the child example in the proposal):

FROM base IMPORT SECURE, NPM, BOWER, GULP
TRIGGER @SECURE
RUN apt-get install fortunes
TRIGGER @NPM
RUN curl some/file
TRIGGER @BOWER
ADD . /app
TRIGGER @GULP

That way it's explicit that those macros come from the parent image.

Beside that, with this in place you can use a 'ON-BUILD' base image even if you don't want any of those macros to run (no need for golang + golang:onbuild!):

FROM base IMPORT
...
Contributor

discordianfish commented Dec 20, 2014

I really like this proposal but I would be cool to make it more explicit that those 'macros' are things defined in the parent Dockerfile. This could be done by extending the FROM to specify which "macros" you want to use (based on the child example in the proposal):

FROM base IMPORT SECURE, NPM, BOWER, GULP
TRIGGER @SECURE
RUN apt-get install fortunes
TRIGGER @NPM
RUN curl some/file
TRIGGER @BOWER
ADD . /app
TRIGGER @GULP

That way it's explicit that those macros come from the parent image.

Beside that, with this in place you can use a 'ON-BUILD' base image even if you don't want any of those macros to run (no need for golang + golang:onbuild!):

FROM base IMPORT
...
@proppy

This comment has been minimized.

Show comment
Hide comment
@proppy

proppy Dec 20, 2014

Contributor

Beside that, with this in place you can use a 'ON-BUILD' base image even if you don't want any of those macros to run (no need for golang + golang:onbuild!):

Very true!

But in the golang image case I think I would prefer to have the golang base define @ONBUILD with the new syntax and require an explicit IMPORT / TRIGGER.

Contributor

proppy commented Dec 20, 2014

Beside that, with this in place you can use a 'ON-BUILD' base image even if you don't want any of those macros to run (no need for golang + golang:onbuild!):

Very true!

But in the golang image case I think I would prefer to have the golang base define @ONBUILD with the new syntax and require an explicit IMPORT / TRIGGER.

@discordianfish

This comment has been minimized.

Show comment
Hide comment
@discordianfish

discordianfish Dec 20, 2014

Contributor

Good point! So in that case we could add the build macros to the regular golang base image and if you don't use them, you use them as a regular base image. If you use them, you can use them as the on-build image.

Contributor

discordianfish commented Dec 20, 2014

Good point! So in that case we could add the build macros to the regular golang base image and if you don't use them, you use them as a regular base image. If you use them, you can use them as the on-build image.

@proppy

This comment has been minimized.

Show comment
Hide comment
@proppy

proppy Dec 20, 2014

Contributor

Updated the proposal to include @discordianfish suggestions. PTAL

Contributor

proppy commented Dec 20, 2014

Updated the proposal to include @discordianfish suggestions. PTAL

@proppy

This comment has been minimized.

Show comment
Hide comment

@proppy proppy closed this Jul 1, 2015

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jul 1, 2015

Member

thanks @proppy!

Member

thaJeztah commented Jul 1, 2015

thanks @proppy!

@tianon tianon referenced this issue in docker-library/python Jan 9, 2016

Closed

Provide a slim version of onbuild #77

@mattrobenolt mattrobenolt referenced this issue in getsentry/docker-sentry Jan 28, 2016

Closed

Can't change pip's parameters with -onbuild #38

@pejter pejter referenced this issue in AdventureLookup/adventurelookup-backend May 20, 2016

Closed

Python image should be a non -on-build version #2

@yosifkit yosifkit referenced this issue in docker-library/official-images Aug 19, 2016

Closed

[Proposal] Deprecate `onbuild` tags #2076

@tianon tianon referenced this issue in docker-library/docs Sep 5, 2017

Merged

Fix variant detection for tags like "1.8-onbuild" #994

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