tinkerbell / tink

Workflow Engine for provisioning Bare Metal
https://tinkerbell.org
Apache License 2.0
917 stars 134 forks source link

Multi arch container support #312

Closed gianarb closed 3 years ago

gianarb commented 3 years ago

Hello

Currently, we build our binaries outside a container using go build directly. I would like to build them inside a container for a couple of reasons:

  1. We serve a docker-compose file to work locally and it uses the build directive to refresh a container starting from Dockerfile. As it is today it refresh the container using the old binary. There are workaround but if we build the binaries as part of the Dockerfile the refresh will pick up code changes as well
  2. If we build binaries inside a container we can use buildx to support multiple architectures. We can do it as well outside but it sounds less trivial

Multi Arch means tinkerbell on ARM!

thebsdbox commented 3 years ago

I'm 💯 x 💯 in support of this, a simple Dockerfile like below would work:

 # syntax=docker/dockerfile:experimental

FROM golang:1.15-alpine as dev
RUN apk add --no-cache git ca-certificates make
RUN adduser -D appuser
COPY . /src/
WORKDIR /src

ENV GO111MODULE=on
RUN --mount=type=cache,sharing=locked,id=gomod,target=/go/pkg/mod/cache \
    --mount=type=cache,sharing=locked,id=goroot,target=/root/.cache/go-build \
    CGO_ENABLED=0 GOOS=linux make build

FROM scratch
COPY --from=dev /src/tink-server /
ENTRYPOINT ["/tink-server"]
gauravgahlot commented 3 years ago

A definite YES for it. 💯

rugwirobaker commented 3 years ago

@gianarb can I take this on?

gianarb commented 3 years ago

Sure @rugwirobaker !! Thank you for your help!

rugwirobaker commented 3 years ago

@thebsdbox I haven't used this method of mounting the module cache as a volume before.

RUN --mount=type=cache,sharing=locked,id=gomod,target=/go/pkg/mod/cache \ --mount=type=cache,sharing=locked,id=goroot,target=/root/.cache/go-build \ CGO_ENABLED=0 GOOS=linux make build

Is there an advantage over downloading them in their own stage like:

# syntax=docker/dockerfile:experimental

FROM golang:1.15-alpine as dev
WORKDIR $GOPATH/src/github.com/tinkerbell/tink
COPY go.mod go.sum  ./
RUN GO111MODULE=on GOPROXY="https://proxy.golang.org" go mod download
COPY . .
CGO_ENABLED=0 GOOS=linux make build

FROM scratch
.....
mmlb commented 3 years ago

I'm not a fan of building inside docker actually. I've found it to be much slower to iterate on things. Admittedly using plain/vanilla/default docker build. Does buildx make it much faster/better? Do we also want to add docs to the tune of "you should really use buildx to build this"?

I'm still not sure this would be better than just doing multi-arch builds in the Makefile.

gianarb commented 3 years ago

@mmlb Go supports cross-compilation but Docker Images needs to build in the target architecture. Buildx serves a very convenient way to pilot Qemu and build Docker images for multiple architectures.

I am not commenting about "slow" because it is irrelevant for the size of this project, at least for me and I can't compare yet.

For local development it really depends on how you work, so even for that my pain point won't apply to you I presume. As an example, I work a lot locally (not inside a VM or a container) and when I run make all from mac the binaries won't work out of the box in vagrant because they are built for Mac. I have to remember to set GOOS. If we move the compilation in the container docker will do the right things for me, maybe slowly but now I have to do it twice and it takes more time. (but as I said, everybody has its own pain when it comes to local development)

Staying away from to local development and speaking about multi-arch support for Docker you can have a look at this to have an idea about how complicated it is to build for multi-arch with buildx https://github.com/docker/build-push-action#multi-platform-image

rugwirobaker commented 3 years ago

@mmlb in my experience the only inconvenience I have had is to re-download the modules all over again after docker image prune. But after doing some research I think mounting them into a volume solves since you get to keep them until at least when you prune unused volumes.

ENV GO111MODULE=on RUN --mount=type=cache,sharing=locked,id=gomod,target=/go/pkg/mod/cache \ --mount=type=cache,sharing=locked,id=goroot,target=/root/.cache/go-build \ CGO_ENABLED=0 GOOS=linux make build

mmlb commented 3 years ago

A pattern we use in some internal repos is to build for GOOS=linux and GOOS= and using the GOOS=linux build COPY'd into the image. I've just taken that bit and added aarch64 too:

~/go/src/github.com/tinkerbell/tink> git diff Makefile | e:cat 
diff --git a/Makefile b/Makefile
index bc172fb..32f5a53 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,19 @@
-server := cmd/tink-server
-cli := cmd/tink-cli
-worker := cmd/tink-worker
-binaries := ${server} ${cli} ${worker}
+cli := cmd/tink-cli/tink-cli
+clis := ${cli}
+clis += $(addprefix ${cli}-x86_64-,darwin freebsd linux windows.exe)
+clis += $(addprefix ${cli}-aarch64-,darwin freebsd linux windows.exe)
+
+server := cmd/tink-server/tink-server
+servers := ${server}
+servers += $(addprefix ${server}-x86_64-,darwin freebsd linux windows.exe)
+servers += $(addprefix ${server}-aarch64-,darwin freebsd linux windows.exe)
+
+worker := cmd/tink-worker/tink-worker
+workers := ${worker}
+workers += $(addprefix ${worker}-x86_64-,darwin freebsd linux windows.exe)
+workers += $(addprefix ${worker}-aarch64-,darwin freebsd linux windows.exe)
+
+all: ${servers} ${clis} ${workers}

 git_version?=$(shell git log -1 --format="%h")
 version?=$(git_version)
@@ -11,17 +23,19 @@ version:=$(release_tag)-$(version)
 endif
 LDFLAGS?=-ldflags "-X main.version=$(version)"

-all: ${binaries}
-
-.PHONY: server ${binaries} cli worker test
 server: ${server}
 cli: ${cli}
 worker : ${worker}

-${server} ${cli} ${worker}:
-   CGO_ENABLED=0 GOOS=$$GOOS go build ${LDFLAGS} -o $@ ./$@
+.PHONY: ${servers} ${clis} ${workers}
+%-darwin: GOOS=darwin
+%-freebsd: GOOS=freebsd
+%-linux: GOOS=linux
+%-windows.exe: GOOS=windows
+${servers} ${clis} ${workers}:
+   CGO_ENABLED=0 GOOS=${GOOS} go build ${LDFLAGS} -o $@ ./$(@D)

-run: ${binaries}
+run: ${all}
    docker-compose up -d --build db
    docker-compose up --build tinkerbell boots
 test:

I'm also building freebsd and windows binaries here. I'm just a little hesitant to lump both the go binary builds and the docker image builds into the same bucket/process when we can easily do multiarch for the go binaries (and has practical purposes for local dev like you @gianarb, me too). Having the multiarch go builds outside of go also makes it so we can add to github releases (especially tink-cli imo) and since we already build the go binaries outside why do we want to build them again inside the container?

gianarb commented 3 years ago

@mmlb if you push a Go binary cross-compiled for ARM in a Macbook Pro and you build the docker image in your laptop the image won't work on ARM to the best of my knowledge. You have to use buildx/qemu or something like that to get the docker image for the right target. Go has great support for cross-compilation and I am fine with it that's not what I am trying to solve.

if trying to avoid myself having to think about the architecture when developing. Moving to Docker brings us a bit closer to get docker images running on multiple architectures.

I will suggest you to avoid overthinking about binary releases because it is something we do not do yet

mmlb commented 3 years ago

Wait I think I see the issue here. We can build the go binaries cross compiled from Mac to Linux Arm and it would work fine in docker, if we use buildx or another specific image (we do this in osie actually, where we build for both x86_64 and aarch64 on a x86_64 machine).

But the way we do it in OSIE is by messing with the FROM and we'd also have to mess with the COPYs (COPY tink-server-linux-x86_64 vs COPY tink-server-linux-aarch64 for example), so the really the Dockerfile is really more like a Dockerfile template.

And we don't really want to do that here right? We want one Dockerfile and leave the cpu arch logic to buildx instead of wanting to mess with the Dockerfile?

gianarb commented 3 years ago

And we don't really want to do that here right? We want one Dockerfile and leave the cpu arch logic to buildx instead of wanting to mess with the Dockerfile?

Yep! (if you have a look at buildx I think you will move osie to buildx as well, it is super powerful)

I would also like to remove the needs for us when developing locally with vagrant (😕 ) to build binaries and Docker containers as 2 different steps, missing the benefit of docker-compose build

mmlb commented 3 years ago

And we don't really want to do that here right? We want one Dockerfile and leave the cpu arch logic to buildx instead of wanting to mess with the Dockerfile?

Yep! (if you have a look at buildx I think you will move osie to buildx as well, it is super powerful)

Yep I was watching buildx for a long time to do that, but drone.packet.net was tied to a very old version of docker :/, no longer true :D.

I would also like to remove the needs for us when developing locally with vagrant (confused ) to build binaries and Docker containers as 2 different steps, missing the benefit of docker-compose build

make run works for that :p (and I actually will most likely PR my Makefile diff too anyway)

But I get it now whey building in the container makes sense, thanks for the info.

gianarb commented 3 years ago

If we realize that it is not the right way I think we can rollback or do something else 👍

@rugwirobaker can't wait to try your PR!

mmlb commented 3 years ago

@rugwirobaker can't wait to try your PR!

ditto!

rugwirobaker commented 3 years ago

@mmlb @gianarb Is there a minimal requirement for go version? Would like to add an ARG GOLANG_VERSION so that we can specify the version at build time.

ARG GOLANG_VERSION=1.14
FROM golang:${GOLANG_VERSION}-alpine as build
thebsdbox commented 3 years ago

There are binary size improvements in the latest release of Go.. might be worth considering 🤷

gianarb commented 3 years ago

I think we can go with the last one. One opinion, let's keep the flexibility and input from the outside as low as possible for now so, even if the ARG looks convenient I think I like the idea to have a fixed one, at the end changing the image is one line change away as an extra flag :) So I don't think it gives us huge flexibility and I see it as an extra config to update.

gianarb commented 3 years ago

This is done: hegel, tink-server, tink-cli, tink-worker, boots are not built with buildx on multiple platforms