Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Setup docker buildx for all-in-one #2325

Closed
wants to merge 2 commits into from
Closed

Setup docker buildx for all-in-one #2325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
260bb4a
build: setup buildx for all-in-ones
morlay Oct 21, 2020
2376d75
Merge branch 'master' into master
jpkrohling Nov 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ script:
- export BRANCH=$(if [ "$TRAVIS_PULL_REQUEST" == "false" ]; then echo $TRAVIS_BRANCH; else echo $TRAVIS_PULL_REQUEST_BRANCH; fi)
- if [ "$TESTS" == true ]; then make test-ci ; else echo 'skipping tests'; fi
- if [ "$PROTO_GEN_TEST" == true ]; then make proto && git diff --name-status --exit-code ; else echo 'skipping protoc validation'; fi
- if [ "$ALL_IN_ONE" == true ]; then make create-baseimg-debugimg && bash ./scripts/travis/build-all-in-one-image.sh ; else echo 'skipping all_in_one'; fi
- if [ "$ALL_IN_ONE" == true ]; then bash ./scripts/travis/setup-docker-buildx.sh && bash ./scripts/travis/build-all-in-one-image.sh ; else echo 'skipping all_in_one'; fi
- if [ "$CROSSDOCK" == true ]; then travis_retry bash ./scripts/travis/build-crossdock.sh ; else echo 'skipping crossdock'; fi
- if [ "$CROSSDOCK_OTEL" == true ]; then travis_retry make build-crossdock crossdock-otel ; else echo 'skipping OpenTelemetry crossdock'; fi
- if [ "$DOCKER" == true ]; then bash ./scripts/travis/build-docker-images.sh ; else echo 'skipping build-docker-images'; fi
Expand Down
38 changes: 38 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,41 @@ Before merging a PR make sure:

Merge the PR by using "Squash and merge" option on Github. Avoid creating merge commits.
After the merge make sure referenced issues were closed.

### Developing guidelines

#### Building OCI Images for multiple arch (linux/arm64, linux/amd64)
Copy link
Contributor

Choose a reason for hiding this comment

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

This section needs some better formatting and grammar check (@objectiser?), but I'm fine in having this done by someone else or in a future PR.


[OCI Images](https://github.com/opencontainers/image-spec/blob/master/spec.md) could be built and published by [buildx](https://github.com/docker/buildx),
it could be executed for local publish `all-in-one` images via:

```shell
TRAVIS_SECURE_ENV_VARS=true NAMESPACE=$(whoami) BRANCH=master ./scripts/travis/build-all-in-one-image.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this TRAVIS_SECURE_ENV_VARS=true needed? Isn't there a make target that could be used instead of the Travis script?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to change too much in this pr.

We could refactor this in future

```

more arch support only need to change `--platform=linux/amd64,linux/arm64,linux/s390x`

if we want to execute this in local env, need to setup buildx:

1. install docker cli plugin

```shell
$ export DOCKER_BUILDKIT=1
$ docker build --platform=local -o . git://github.com/docker/buildx
$ mkdir -p ~/.docker/cli-plugins
$ mv buildx ~/.docker/cli-plugins/docker-buildx
```
(via https://github.com/docker/buildx#with-buildx-or-docker-1903, if docker issue, could check https://docs.docker.com/engine/install/linux-postinstall/#troubleshooting)

2. install qemu for multi arch

```shell
$ docker run --privileged --rm tonistiigi/binfmt --install all
```
(via https://github.com/docker/buildx#building-multi-platform-images)

3. create a builder

```shell
$ docker buildx create --use --name builder
```
59 changes: 29 additions & 30 deletions cmd/all-in-one/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,23 @@
ARG base_image
ARG debug_image
ARG TARGET=release

FROM $base_image AS release
ARG TARGETARCH=amd64
FROM golang:1.15-alpine as build
ENV GOPATH /go

RUN apk add --update --no-cache ca-certificates make git && \
go get github.com/go-delve/delve/cmd/dlv && \
cd /go/src/github.com/go-delve/delve && \
make install

FROM golang:1.15-alpine AS base-image-debug
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you're using golang:1.15-alpine as base image for debug, but it will still have delve missing. I feel like we reverted back changes to this Dockerfile from my PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

This PR here should not revert that PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. i added it.

We could setup multi arch build for base images next


COPY --from=build /go/bin/dlv /go/bin/dlv
COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt

FROM alpine:3.12 AS base-image-release

COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt

FROM base-image-${TARGET} as common

# Agent zipkin.thrift compact
EXPOSE 5775/udp
Expand All @@ -25,43 +40,27 @@ EXPOSE 14250
# Web HTTP
EXPOSE 16686

COPY all-in-one-linux-$TARGETARCH /go/bin/all-in-one-linux
COPY sampling_strategies.json /etc/jaeger/

VOLUME ["/tmp"]
ENTRYPOINT ["/go/bin/all-in-one-linux"]
CMD ["--sampling.strategies-file=/etc/jaeger/sampling_strategies.json"]

FROM $debug_image AS debug
ARG TARGETARCH=amd64
FROM common AS release

# Agent zipkin.thrift compact
EXPOSE 5775/udp

# Agent jaeger.thrift compact
EXPOSE 6831/udp

# Agent jaeger.thrift binary
EXPOSE 6832/udp
ARG TARGETARCH
COPY all-in-one-linux-${TARGETARCH} /go/bin/all-in-one-linux

# Agent config HTTP
EXPOSE 5778
ENTRYPOINT ["/go/bin/all-in-one-linux"]
CMD ["--sampling.strategies-file=/etc/jaeger/sampling_strategies.json"]

# Collector HTTP
EXPOSE 14268
FROM common AS debug

# Collector gRPC
EXPOSE 14250

# Web HTTP
EXPOSE 16686
ARG TARGETARCH
COPY all-in-one-debug-linux-${TARGETARCH} /go/bin/all-in-one-linux

# Delve
EXPOSE 12345

COPY all-in-one-debug-linux-$TARGETARCH /go/bin/all-in-one-linux
COPY sampling_strategies.json /etc/jaeger/

VOLUME ["/tmp"]
ENTRYPOINT ["/go/bin/dlv", "exec", "/go/bin/all-in-one-linux", "--headless", "--listen=:12345", "--api-version=2", "--accept-multiclient", "--log", "--"]
CMD ["--sampling.strategies-file=/etc/jaeger/sampling_strategies.json"]

FROM ${TARGET}
Copy link
Contributor

@Ashmita152 Ashmita152 Oct 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I have zero knowledge of docker buildx, curious what is the meaning of having FROM at the end of Dockerfile ?

Copy link
Contributor Author

@morlay morlay Oct 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It is not new of buildx.

FROM could be a stage name.

if TARGET = release, that means FROM release
if TARGET = debug, that means FROM debug

i learn this from istio Dockerfiles, which is used to switch default and distroless

4 changes: 2 additions & 2 deletions cmd/opentelemetry/cmd/all-in-one/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ FROM alpine:latest as certs
RUN apk add --update --no-cache ca-certificates

FROM scratch
ARG TARGETARCH=amd64

COPY --from=certs /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt

COPY opentelemetry-all-in-one-linux-$TARGETARCH /go/bin/opentelemetry-all-in-one-linux
ARG TARGETARCH
COPY opentelemetry-all-in-one-linux-${TARGETARCH} /go/bin/opentelemetry-all-in-one-linux
ENTRYPOINT ["/go/bin/opentelemetry-all-in-one-linux"]
120 changes: 85 additions & 35 deletions scripts/travis/build-all-in-one-image.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ BRANCH=${BRANCH:?'missing BRANCH env var'}
# be overrided by passing architecture value to the script:
# `GOARCH=<target arch> ./scripts/travis/build-all-in-one-image.sh`.
GOARCH=${GOARCH:-$(go env GOARCH)}
# local run with `TRAVIS_SECURE_ENV_VARS=true NAMESPACE=$(whoami) BRANCH=master ./scripts/travis/build-all-in-one-image.sh`
NAMESPACE=${NAMESPACE:-jaegertracing}
ARCHS="amd64 arm64 s390x"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does travis support building images for s390x architecture ? Doesn't that means it will require base image of this architecture for alpine too ?

Copy link
Contributor Author

@morlay morlay Oct 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yes. We could build base images by buildx in next step. now i have to put base images stages to all-in-one Dockerfile.

image golang and image alpine both support s390x too.
so the base images of jaeger could support s390x easier.

the magic is tonistiigi/binfmt who installs qemu


source ~/.nvm/nvm.sh
nvm use 10
Expand All @@ -26,42 +29,89 @@ run_integration_test() {
docker kill $CID
}

upload_to_docker() {
# Only push the docker container to Docker Hub for master branch
if [[ ("$BRANCH" == "master" || $BRANCH =~ ^v[0-9]+\.[0-9]+\.[0-9]+$) && "$TRAVIS_SECURE_ENV_VARS" == "true" ]]; then
echo 'upload to Docker Hub'
docker_buildx() {
CMD_ROOT=${1}
FLAGS=${@:2}

if [[ "$FLAGS" == *"-push"* ]]; then
if [[ ("$BRANCH" == "master" || $BRANCH =~ ^v[0-9]+\.[0-9]+\.[0-9]+$) && "$TRAVIS_SECURE_ENV_VARS" == "true" ]]; then
echo 'upload to Docker Hub'
else
echo 'skip docker upload for PR'
exit 0
fi
fi

docker buildx build -f ${CMD_ROOT}/Dockerfile ${FLAGS} ${CMD_ROOT}
}

image_tags_for() {
REPO=${1}

major=""
minor=""
patch=""

if [[ "$BRANCH" == "master" ]]; then
TAG="latest"
elif [[ $BRANCH =~ ^v([0-9]+)\.([0-9]+)\.([0-9]+)$ ]]; then
major="${BASH_REMATCH[1]}"
minor="${BASH_REMATCH[2]}"
patch="${BASH_REMATCH[3]}"
TAG=${major}.${minor}.${patch}
else
echo 'skip docker upload for PR'
exit 0
TAG="${BRANCH///}"
fi

IMAGE_TAGS="--tag ${REPO}:${TAG}"
Copy link
Contributor

Choose a reason for hiding this comment

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

@aebirim, for your change, you might just need to add a new tag here, as well as a docker login to all configured repositories.

Copy link

Choose a reason for hiding this comment

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

@aebirim, for your change, you might just need to add a new tag here, as well as a docker login to all configured repositories.

np. @albertteoh @jpkrohling I'll wait for this script to be merged before adding my quay change

Copy link
Contributor

Choose a reason for hiding this comment

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

This PR was closed in favor of #2611


# add major, major.minor and major.minor.patch tags
if [[ -n $major ]]; then
IMAGE_TAGS="${IMAGE_TAGS} -t ${REPO}:${major}"
if [[ -n $minor ]]; then
IMAGE_TAGS="${IMAGE_TAGS} -t ${REPO}:${major}.${minor}"
if [[ -n $patch ]]; then
IMAGE_TAGS="${IMAGE_TAGS} -t ${REPO}:${major}.${minor}.${patch}"
fi
fi
fi
export REPO=$1
bash ./scripts/travis/upload-to-docker.sh

echo "${IMAGE_TAGS}"
}

target_platforms() {
PLATFORMS=""
for arch in ${ARCHS}; do
if [ -n "${PLATFORMS}" ]; then
PLATFORMS="${PLATFORMS},linux/${arch}"
else
PLATFORMS="linux/${arch}"
fi
done
echo ${PLATFORMS}
}

make build-all-in-one GOOS=linux GOARCH=$GOARCH
repo=jaegertracing/all-in-one
docker build -f cmd/all-in-one/Dockerfile \
--target release \
--tag $repo:latest cmd/all-in-one \
--build-arg base_image=localhost/baseimg:1.0.0-alpine-3.12 \
--build-arg debug_image=localhost/debugimg:1.0.0-golang-1.15-alpine \
--build-arg TARGETARCH=$GOARCH
run_integration_test $repo
upload_to_docker $repo

make build-all-in-one-debug GOOS=linux GOARCH=$GOARCH
repo=jaegertracing/all-in-one-debug
docker build -f cmd/all-in-one/Dockerfile \
--target debug \
--tag $repo:latest cmd/all-in-one \
--build-arg base_image=localhost/baseimg:1.0.0-alpine-3.12 \
--build-arg debug_image=localhost/debugimg:1.0.0-golang-1.15-alpine \
--build-arg TARGETARCH=$GOARCH
Copy link
Contributor

Choose a reason for hiding this comment

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

@morlay I noticed all occurrences of TARGETARCH is removed from this script but I don't see the place it is passed as argument to Dockerfile from this script. TARGETARCH is being used in all-in-one's Dockerfile to decide which binary to copy inside container. Is this some arg which buildx populates internally by default ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TARGETARCH and other platform args will be passed by buildx

https://docs.docker.com/engine/reference/builder/#automatic-platform-args-in-the-global-scope

run_integration_test $repo
upload_to_docker $repo

make build-otel-all-in-one GOOS=linux GOARCH=$GOARCH
repo=jaegertracing/opentelemetry-all-in-one
docker build -f cmd/opentelemetry/cmd/all-in-one/Dockerfile -t $repo:latest cmd/opentelemetry/cmd/all-in-one --build-arg TARGETARCH=$GOARCH
run_integration_test $repo
upload_to_docker $repo

for arch in ${ARCHS}; do
make build-all-in-one GOOS=linux GOARCH=${arch}
jpkrohling marked this conversation as resolved.
Show resolved Hide resolved
done
repo=${NAMESPACE}/all-in-one
docker_buildx cmd/all-in-one --load --build-arg=TARGET=release --platform=linux/${GOARCH} -t $repo:latest
run_integration_test ${repo}
docker_buildx cmd/all-in-one --push --build-arg=TARGET=release --platform=$(target_platforms) $(image_tags_for ${repo})

for arch in ${ARCHS}; do
jpkrohling marked this conversation as resolved.
Show resolved Hide resolved
make build-all-in-one-debug GOOS=linux GOARCH=${arch}
done
repo=${NAMESPACE}/all-in-one-debug
docker_buildx cmd/all-in-one --load --build-arg=TARGET=debug --platform=linux/${GOARCH} -t $repo:latest
run_integration_test ${repo}
docker_buildx cmd/all-in-one --push --build-arg=TARGET=debug --platform=$(target_platforms) $(image_tags_for ${repo})

for arch in ${ARCHS}; do
make build-otel-all-in-one GOOS=linux GOARCH=${arch}
done
repo=${NAMESPACE}/opentelemetry-all-in-one
docker_buildx cmd/opentelemetry/cmd/all-in-one --load --platform=linux/${GOARCH} -t $repo:latest
run_integration_test ${repo}
docker_buildx cmd/opentelemetry/cmd/all-in-one --push --platform=$(target_platforms) $(image_tags_for ${repo})
56 changes: 56 additions & 0 deletions scripts/travis/setup-docker-buildx.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#!/bin/bash

set -eux

# update docker
# for setup buildx https://travis-ci.community/t/installing-docker-19-03/8077/2
sudo apt update

sudo systemctl stop docker
sudo apt install -y docker.io

sudo systemctl unmask docker.service
sudo systemctl unmask docker.socket
sudo systemctl start docker
sudo systemctl status docker.socket

docker version

DOCKER_BUILDX_VERSION=v0.4.2

LOCAL_OS=$(uname -s | tr '[A-Z]' '[a-z]')

case $(uname -m) in
x86_64)
LOCAL_ARCH=amd64
;;
aarch64)
LOCAL_ARCH=arm64
;;
*)
echo "unsupported architecture"
exit 1
;;
esac

if [[ ! -f ~/.docker/cli-plugins/docker-buildx ]]; then
DOCKER_BUILDX_DOWNLOAD_URL=https://github.com/docker/buildx/releases/download/${DOCKER_BUILDX_VERSION}/buildx-${DOCKER_BUILDX_VERSION}.${LOCAL_OS}-${LOCAL_ARCH}
mkdir -p ~/.docker/cli-plugins
echo "downloading from ${DOCKER_BUILDX_DOWNLOAD_URL}"
curl -sL --output ~/.docker/cli-plugins/docker-buildx "${DOCKER_BUILDX_DOWNLOAD_URL}"
chmod a+x ~/.docker/cli-plugins/docker-buildx
fi

# enable buildx
export DOCKER_CLI_EXPERIMENTAL=enabled

# checkout buildx available
docker buildx version

# enabled qemu if needed
if [[ ! $(docker buildx inspect default | grep Platforms) == *arm64* ]]; then
docker run --privileged --rm tonistiigi/binfmt --install all
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed you're using tonistiigi/binfmt image. I guess this is because it has images for all architecture which we need. Can we use append the tag too tonistiigi/binfmt:<tag> Right now it is pulling latest tag and the owner updating the latest image may break Jaeger travis CI.

Copy link
Contributor Author

@morlay morlay Oct 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

tonistiigi/binfmt is not provide the images we need for architecture.

it just install qemu as simulator for target arch, which let us could run arm64 binary on amd64 host. (but it is a very slow way, and we don't use this for compiling)

the images alpine:3.12, you could see the arch list in docker hub.

image

This means docker hub store each image for each arch.

we could pull the target arch image to local using --platform

docker pull --platform=linux/arm64 alpine:3.12
docker pull --platform=linux/amd64 alpine:3.12
docker pull --platform=linux/s390x alpine:3.12

With buildx, the FROM alpine:3.12 in Dockerfile, will pull all images of target archs.
And in RUN apt add git, apt (arm64 binary) need to execute on matched arch (arm64) host.
This why we need qemu. with qemu we could run apt (arm64 binary) on amd64 host.

And we don't need the tag. it is a official tool in buildx doc. when this tool is stable, i think it will merge back to docker's namespace.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean you are using tonistiigi/binfmt instead of official image linuxkit/binfmt Is there any particular reason for that ?

Copy link
Contributor Author

@morlay morlay Oct 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

no special reason, just it is on README of buildx Which is widely used by github actions https://github.com/docker/setup-qemu-action.

https://github.com/docker/buildx#building-multi-platform-images
image

Copy link
Contributor Author

@morlay morlay Oct 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

May be the version of qemu.

tonistiigi/binfmt with qemu@5.0
linuxkit/binfmt with qemu@4.2

And linuxkit/binfmt not provide OCI images for multi arch. it could not be used on a non-amd64 host.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh I see, thank you @morlay that answers my question.

fi

# setup builder
docker buildx create --use