knative / build-templates

A library of build templates.
Apache License 2.0
184 stars 68 forks source link

buildah: convert Dockerfile to a multi-stage build #74

Closed vdemeester closed 5 years ago

vdemeester commented 5 years ago

It makes the final image smaller, and allow better caching when building it. It also updates the README.md and sets a default builder image for buildah (vdemeester/buildah-builder that is automatically build on the docker hub).

Signed-off-by: Vincent Demeester vdemeest@redhat.com

vbatts commented 5 years ago

on the whole it LGTM. One thing I still don't like about multi-stage (there are a few things, but this one currently) is that the intermediary build steps are not named, pushed and associated to the final image. So that provenance is lost. If the tool doesn't include the git commit in a version variable or something, then the version used to build is lost. For debugging and maintaining audit info, it makes gaps. To that end buildah does include git commit, but runc does not.

vdemeester commented 5 years ago

One thing I still don't like about multi-stage (there are a few things, but this one currently) is that the intermediary build steps are not named, pushed and associated to the final image. So that provenance is lost. If the tool doesn't include the git commit in a version variable or something, then the version used to build is lost. For debugging and maintaining audit info, it makes gaps. To that end buildah does include git commit, but runc does not.

@vbatts make sense… at some point we'll use packaged version so it won't matter anymore (and multi-stage might not even required anymore). In the meantime we could put some git hash for those component somewhere in the produced image, it would fix it.

vbatts commented 5 years ago

@vdemeester is it possible to export a variable from one layer/stage that could be set as a LABEL in the final image/stage?

vdemeester commented 5 years ago

@vbatts sadly nope :sweat: I was thinking more something like setting the git commit hash as args and use them instead of master (pining a specific version).

vbatts commented 5 years ago

How best to handle this? git log -1 --oneline > /.version.runc and so on?

vdemeester commented 5 years ago

@vbatts or that :angel: it's even better :+1: I'll update with that :wink:

vbatts commented 5 years ago

would it get copied forward as well?

vdemeester commented 5 years ago

@vbatts yeah I can make sure it's copied in the final image :angel:

knative-prow-robot commented 5 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arilivigni, ImJasonH, vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/knative/build-templates/blob/master/OWNERS)~~ [ImJasonH] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
vbatts commented 5 years ago

single nit LGTM

imjasonh commented 5 years ago

/lgtm

imjasonh commented 5 years ago

/hold

Fix vbatts' nit first

vdemeester commented 5 years ago

Done :tada: :man_dancing:

imjasonh commented 5 years ago

/hold cancel /lgtm

🕺