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

Add docker buildx make target #3213

Merged
merged 4 commits into from
Aug 18, 2021
Merged

Add docker buildx make target #3213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1715b9a
Add docker buildx make target
pavolloffay Aug 17, 2021
fd6e177
Remove space
pavolloffay Aug 17, 2021
6ab36f3
make it idempotent
pavolloffay Aug 17, 2021
9e2cfe8
fix hotrod
pavolloffay Aug 17, 2021
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
9 changes: 0 additions & 9 deletions .github/workflows/ci-all-in-one-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ on:
jobs:
all-in-one:
runs-on: ubuntu-latest
services:
registry:
image: registry:2
ports:
- 5000:5000
steps:
- uses: actions/checkout@v2.3.4
with:
Expand All @@ -39,10 +34,6 @@ jobs:

- uses: docker/setup-qemu-action@v1

- uses: docker/setup-buildx-action@v1
with:
driver-opts: network=host

- name: Build, test, and publish all-in-one image
run: bash scripts/build-all-in-one-image.sh
env:
Expand Down
9 changes: 0 additions & 9 deletions .github/workflows/ci-crossdock.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ on:
jobs:
crossdock:
runs-on: ubuntu-latest
services:
registry:
image: registry:2
ports:
- 5000:5000
strategy:
matrix:
steps:
Expand Down Expand Up @@ -41,10 +36,6 @@ jobs:

- uses: docker/setup-qemu-action@v1

- uses: docker/setup-buildx-action@v1
with:
driver-opts: network=host

- name: Build, test, and publish ${{ matrix.steps.name }} image
run: ${{ matrix.steps.cmd }}
env:
Expand Down
10 changes: 0 additions & 10 deletions .github/workflows/ci-docker-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ jobs:
docker-images:
runs-on: ubuntu-latest

services:
registry:
image: registry:2
ports:
- 5000:5000

steps:
- uses: actions/checkout@v2.3.4
with:
Expand All @@ -41,10 +35,6 @@ jobs:

- uses: docker/setup-qemu-action@v1

- uses: docker/setup-buildx-action@v1
with:
driver-opts: network=host

- name: Build and upload all docker images
run: bash scripts/build-upload-docker-images.sh
env:
Expand Down
9 changes: 0 additions & 9 deletions .github/workflows/ci-hotrod.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ on:
jobs:
hotrod:
runs-on: ubuntu-latest
services:
registry:
image: registry:2
ports:
- 5000:5000
steps:
- uses: actions/checkout@v2.3.4
with:
Expand All @@ -35,10 +30,6 @@ jobs:

- uses: docker/setup-qemu-action@v1

- uses: docker/setup-buildx-action@v1
with:
driver-opts: network=host

- name: Build, test, and publish hotrod image
run: bash scripts/hotrod-integration-test.sh
env:
Expand Down
11 changes: 1 addition & 10 deletions .github/workflows/ci-release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,7 @@ on:
jobs:
publish-release:
runs-on: ubuntu-latest
services:
registry:
image: registry:2
ports:
- 5000:5000


steps:
- uses: actions/checkout@v2.3.4
with:
Expand Down Expand Up @@ -57,10 +52,6 @@ jobs:

- uses: docker/setup-qemu-action@v1

- uses: docker/setup-buildx-action@v1
with:
driver-opts: network=host

- name: Build and upload all docker images
run: bash scripts/build-upload-docker-images.sh
env:
Expand Down
14 changes: 12 additions & 2 deletions docker/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,25 @@ PLATFORMS := linux/amd64,linux/s390x,linux/ppc64le,linux/arm64

create-baseimg-debugimg: create-baseimg create-debugimg

create-baseimg:
create-baseimg: prepare-docker-buildx
docker buildx build -t $(BASE_IMAGE) --push \
--build-arg root_image=$(ROOT_IMAGE) \
--build-arg cert_image=$(CERT_IMAGE) \
--platform=$(PLATFORMS) \
docker/base

create-debugimg:
create-debugimg: prepare-docker-buildx
docker buildx build -t $(DEBUG_IMAGE) --push \
--build-arg golang_image=$(GOLANG_IMAGE) \
--platform=$(PLATFORMS) \
docker/debug

.PHONY: prepare-docker-buildx
prepare-docker-buildx:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nice idea. It would be even nicer if it was a seamless experience for local dev. For example, if I run the following locally:

make docker-images-jaeger-backend

I still get the error, because the local registry isn't started as part of this make target:

error: failed to solve: rpc error: code = Unknown desc = failed to do request: Head "http://localhost:5000/v2/baseimg_alpine/blobs/sha256:48167268581ca7f86ce8f794a24d144b0509556cf4c584eec9e95afebe88bcc6": dial tcp [::1]:5000: connect: connection refused

Given the only time we need the registry is when we do a push, would it make sense to add this target as a dependency to both create-baseimg and create-debugimg? i.e.

create-baseimg: prepare-docker-buildx
...
create-debugimg: prepare-docker-buildx
...

That way, developers won't need to know to run make prepare-docker-buildx for their docker builds to work.

If the above is feasible, it also means we could do away with the - run: make prepare-docker-buildx within our github actions steps, given create-baseimg and/or create-debugimg should be run.

The drawback I see is that make prepare-docker-buildx isn't idempotent and I don't have any good suggestions right now to solve for it, except for making sure that make clean-docker-buildx is always run at the end.

The other alternative is we document this extra manual step of make prepare-docker-buildx clearly somewhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

btw. are you on macos? Were you able to run it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I'm on macos and successfully ran this make target together with other docker buildx.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't mind requiring devs to execute make prepare-docker-buildx explicitly. It's quite easy to understand what needs to be run from the scripts.

I have made it idempotent in the last commit and removed prepare step from the ci jobs.

For instance IIRC install-tools target is not invoked automatically before a target that uses tools.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's quite easy to understand what needs to be run from the scripts.

yes, I agree, but knowing to where to find the scripts (I presume you mean the scripts run by github actions) seems less intuitive than looking in the Makefile since that's where the make targets are defined. For example, they may just want to build custom docker images for testing purposes and run make docker-images-jaeger-backend, and not have any desire to replicate CI locally.

Copy link
Member Author

Choose a reason for hiding this comment

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

When I am trying to build an unknown project I usually first go to CI config to see what is required to run. If there is a clear "prepare" step I can follow it and understand what needs to be done.

If the "prepare" for OOTB then it's good as well until it does not interfere with tools and services running on the host. In this case it might be the docker registry for instance.

docker buildx inspect jaeger-build > /dev/null || docker buildx create --use --name=jaeger-build --buildkitd-flags="--allow-insecure-entitlement security.insecure --allow-insecure-entitlement network.host" --driver-opt="network=host"
docker inspect registry > /dev/null || docker run --rm -d -p 5000:5000 --name registry registry:2

.PHONY: clean-docker-buildx
clean-docker-buildx:
docker buildx rm jaeger-build
docker rm -f registry
1 change: 1 addition & 0 deletions scripts/hotrod-integration-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ make build-examples GOOS=linux GOARCH=arm64

REPO=jaegertracing/example-hotrod
platforms="linux/amd64,linux/s390x,linux/ppc64le,linux/arm64"
make prepare-docker-buildx
#build image locally for integration test
bash scripts/build-upload-a-docker-image.sh -l -c example-hotrod -d examples/hotrod -p "${platforms}"

Expand Down