Buildkit-optimized dockerfile + buildx #39340
Conversation
|
docker-bake.hcl file included b/c I was just messing around with it. |
|
I expect CI to fail on this because it's using |
Makefile
Outdated
There was a problem hiding this comment.
I guess this is the epitome of windows-last development
|
One thing to note, now |
Dockerfile
Outdated
There was a problem hiding this comment.
can we add stages with precompiled test suites?
FROM dev as test-dynbinary
RUN --mount=target=--mount=type=cache,target=/root/.cache/go
F440
-build \
build-integration-test-binary dynbinary
ENV DOCKER_INTEGRATION_TESTS_VERIFIED=1
ENV KEEPBUNDLE=1(ref: https://github.com/AkihiroSuda/kube-moby-integration/blob/master/Dockerfile)
There was a problem hiding this comment.
I have this, however it's a bit tricky because the tests expecting a certain work dir.
Some more polishing to do.
Dockerfile
Outdated
There was a problem hiding this comment.
Probably we should add back comment lines with updated contents
There was a problem hiding this comment.
I'm not sure most of them are useful since you can see that by looking at the build targets.
There was a problem hiding this comment.
Realizing I can't use the sha here since this dockerfile is used for multiple platforms.
d15dee1 to
c3ae2b0
Compare
|
@andrewhsu power and z still need updates to support buildkit. |
b4e816c to
4d4ac70
Compare
7ec5b43 to
f1aacc5
Compare
|
@thaJeztah Ok so I've made buildx opt-in here and updated the Jenkinsfile so power/z can build using buildx. |
f1aacc5 to
91f1945
Compare
177b1c2 to
c095fc5
Compare
df73e1d to
cfa0433
Compare
|
Ok, think I fixed it, we'll see... magic |
|
Yep, that was it, those arches work now. |
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Since the dockerfile now requires buildkit, let's just use buildx can bootstrap itself into even an old version of Docker which does not support buildkit. This also decouples the Dockerfile/build from the version of Docker which is installed. One major downside: If buildx needs to setup a container driver (ie, docker does not support buildkit), the `make shell` target (and others which call `DOCKER_RUN_DOCKER`) must export the image from buildkit and into docker. This added an extra 70s to a full build for me (agan only for targets which call `DOCKER_RUN_DOCKER`) and 40s on a rebuild (with no changes). Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Z and Power does not currently work with buildkit so use buildx instead. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
cfa0433 to
c04ea11
Compare
So, the problem is that the first line of each command is basically indented with spaces; tab-width is configurable, so I think we should use spaces for indentation in Dockerfiles if we want things to line up. I've been playing a bit with possible changes we could make (including the indentation), and pushed it to a branch; master...thaJeztah:buildkit_dockerfile_formatting |
There was a problem hiding this comment.
Other than the indentation (not a show-stopper, but if we want to reduce churn in this file, and if we want to change it, would be good to have it in this PR).
So, LGTM, but we can discuss that change (and perhaps there's changes in the branch I mentioned above that you think would be useful)
|
So indentation depends on the editor. So yeah, maybe spaces will fair better. |
|
But let's get this in first 😅😇 |
SGTM; I can open a WIP pr for my other changes for easier discussion on those; do you want @AkihiroSuda to have another look for the changes you made since his LGTM? If you think it's ok without, I can merge (or feel free to press the button yourself) |
|
still LGTM, merged 🙂 |
|
Thanks! |