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.

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 support for docker buildkit, speeding up docker builds 5-10x #19156

Merged
merged 14 commits into from
Nov 26, 2019
17 changes: 16 additions & 1 deletion Makefile.core.mk
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ DOCKER_PROXY_CFG?=Dockerfile.proxy
# copied to the docker temp container - even if you add only a tiny file, >1G of data will
# be copied, for each docker image.
DOCKER_BUILD_TOP:=${ISTIO_OUT_LINUX}/docker_build
DOCKERX_BUILD_TOP:=${ISTIO_OUT_LINUX}/dockerx_build

# dir where tar.gz files from docker.save are stored
ISTIO_DOCKER_TAR:=${ISTIO_OUT_LINUX}/docker
Expand Down Expand Up @@ -365,7 +366,21 @@ build-linux: depend
STATIC=0 GOOS=linux GOARCH=amd64 LDFLAGS='-extldflags -static -s -w' common/scripts/gobuild.sh $(ISTIO_OUT_LINUX)/ $(BINARIES)

# Create targets for ISTIO_OUT_LINUX/binary
$(foreach bin,$(BINARIES),$(ISTIO_OUT_LINUX)/$(shell basename $(bin))): build-linux
# There are two use cases here:
# * Building all docker images (generally in CI). In this case we want to build everything at once, so they share work
# * Building a single docker image (generally during dev). In this case we just want to build the single binary alone
Comment on lines +369 to +371
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not strictly necessary, but is another optimization made possible by this change. This way both CI and dev experience is optimized

BUILD_ALL ?= true
define build-linux =
.PHONY: $(ISTIO_OUT_LINUX)/$(shell basename $(1))
ifeq ($(BUILD_ALL),true)
$(ISTIO_OUT_LINUX)/$(shell basename $(1)): build-linux
else
$(ISTIO_OUT_LINUX)/$(shell basename $(1)):
STATIC=0 GOOS=linux GOARCH=amd64 LDFLAGS='-extldflags -static -s -w' common/scripts/gobuild.sh $(ISTIO_OUT_LINUX)/ $(1)
endif
endef

$(foreach bin,$(BINARIES),$(eval $(call build-linux,$(bin))))

# Create helper targets for each binary, like "pilot-discovery"
# As an optimization, these still build everything
Expand Down
15 changes: 8 additions & 7 deletions prow/lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ function download_untar_istio_release() {

function build_images() {
# Build just the images needed for tests
for image in pilot proxyv2 app test_policybackend mixer citadel galley sidecar_injector kubectl node-agent-k8s; do
DOCKER_BUILD_VARIANTS="${VARIANT:-default}" make docker.${image}
done
targets="docker.pilot docker.proxyv2 "
targets+="docker.app docker.test_policybackend docker.kubectl "
targets+="docker.mixer docker.citadel docker.galley docker.sidecar_injector docker.node-agent-k8s"
DOCKER_BUILD_VARIANTS="${VARIANT:-default}" DOCKER_TARGETS="${targets}" make dockerx
Copy link
Member Author

Choose a reason for hiding this comment

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

The key here is we are modifying the targets and calling a single make target, rather than many make targets (which is slow)

}

function kind_load_images() {
Expand All @@ -78,17 +79,17 @@ function kind_load_images() {
# Archived local images and load it into KinD's docker daemon
# Kubernetes in KinD can only access local images from its docker daemon.
docker images "${hub}/*:${TAG}" --format '{{.Repository}}:{{.Tag}}' | xargs -n1 kind -v9 --name "${NAME}" load docker-image && break
echo "Attempt ${i} to load images failed, retrying in 5s..."
sleep 5
echo "Attempt ${i} to load images failed, retrying in 1s..."
Copy link
Member Author

Choose a reason for hiding this comment

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

not strictly required. but saves some time. We probably need no sleep, really

sleep 1
done

# If a variant is specified, load those images as well.
# We should still load non-variant images as well for things like `app` which do not use variants
if [[ "${VARIANT:-}" != "" ]]; then
for i in {1..3}; do
docker images "${hub}/*:${TAG}-${VARIANT}" --format '{{.Repository}}:{{.Tag}}' | xargs -n1 kind -v9 --name "${NAME}" load docker-image && break
echo "Attempt ${i} to load images failed, retrying in 5s..."
sleep 5
echo "Attempt ${i} to load images failed, retrying in 1s..."
sleep 1
done
fi
}
Expand Down
78 changes: 78 additions & 0 deletions tools/buildx-gen.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
#!/bin/bash

# Copyright 2019 Istio Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

set -eu

out="${1}"
config="${out}/docker-bake.hcl"
shift

variants=$(for i in ${DOCKER_ALL_VARIANTS}; do echo "\"${i}\""; done | xargs -d'\n' | sed -e 's/ /, /g')
cat <<EOF > "${config}"
group "all" {
targets = [${variants}]
}
EOF

# Generate the top header. This defines a group to build all images for each variant
for variant in ${DOCKER_ALL_VARIANTS}; do
# Get all images. Transform from `docker.target` to `"target"` as a comma seperated list
images=$(for i in "$@"; do echo "\"${i#docker.}-${variant}\""; done | xargs -d'\n' | sed -e 's/ /, /g')
cat <<EOF >> "${config}"
group "${variant}" {
targets = [${images}]
}
EOF
done

# For each docker image, define a target to build it
for file in "$@"; do
for variant in ${DOCKER_ALL_VARIANTS}; do
image=${file#docker.}
tag="${TAG}"
output="${image}"
# The default variant has no suffix, others do
if [[ "${variant}" != "default" ]]; then
tag+="-${variant}"
output+="-${variant}"
fi
# Add output only if docker save is specified
outputfield=""
if [[ -n "${DOCKER_SAVE:-}" ]]; then
outputfield="output = [\"type=tar,dest=${ISTIO_DOCKER_TAR}/${output}.tar\"]"
fi
cat <<EOF >> "${config}"
target "$image-$variant" {
context = "${out}/${file}"
dockerfile = "Dockerfile.$image"
tags = ["${HUB}/${image}:${tag}"]
args = {
BASE_VERSION = "${BASE_VERSION}"
BASE_DISTRIBUTION = "${variant}"
}
${outputfield}
}
EOF
# For the default variant, create an alias so we can do things like `build pilot` instead of `build pilot-default`
if [[ "${variant}" == "default" ]]; then
cat <<EOF >> "${config}"
target "$image" {
inherits = ["$image-$variant"]
}
EOF
fi
done
done
47 changes: 41 additions & 6 deletions tools/istio-docker.mk
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@

# Docker target will build the go binaries and package the docker for local testing.
# It does not upload to a registry.
docker: build-linux docker.all
docker: docker.all
Copy link
Member Author

Choose a reason for hiding this comment

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

targets already do a build

Copy link
Contributor

@ericvn ericvn Nov 25, 2019

Choose a reason for hiding this comment

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

But on a Mac, the build will generate only the Darwin binaries?

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 am fairly sure the rules like docker.citadel: $(ISTIO_DOCKER)/istio_ca are doing the right thing and this is completely redundant. Otherwise rules like make docker.citadel would fail. Would love if you could verify though

Copy link
Contributor

Choose a reason for hiding this comment

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

But I see that the docker. targets list the $(ISTIO_OUT_LINUX)... targets as dependencies so they will get built as part of the docker target. So this is fine.


# Add new docker targets to the end of the DOCKER_TARGETS list.
DOCKER_TARGETS:=docker.pilot docker.istiod docker.proxytproxy docker.proxyv2 docker.app docker.app_sidecar docker.test_policybackend \
DOCKER_TARGETS ?= docker.pilot docker.istiod docker.proxytproxy docker.proxyv2 docker.app docker.app_sidecar docker.test_policybackend \
docker.mixer docker.mixer_codegen docker.citadel docker.galley docker.sidecar_injector docker.kubectl docker.node-agent-k8s

$(ISTIO_DOCKER) $(ISTIO_DOCKER_TAR):
Expand Down Expand Up @@ -192,8 +192,41 @@ docker.mixer_codegen: mixer/docker/Dockerfile.mixer_codegen
docker.mixer_codegen: $(ISTIO_DOCKER)/mixgen
$(DOCKER_RULE)

# galley docker images
.PHONY: dockerx dockerx.save

# Docker has an experimental new build engine, https://github.com/docker/buildx
# This brings substantial (10x) performance improvements when building Istio
# However, its only built into docker since v19.03. Because its so new that devs are likely to not have
# this version, and because its experimental, this is not the default build method. As this matures we should migrate over.
# For performance, in CI this method is used.
# This target works by reusing the existing docker methods. Each docker target declares it's dependencies.
# We then override the docker rule and "build" all of these, where building just copies the dependencies
# We then generate a "bake" file, which defines all of the docker files in the repo
# Finally, we call `docker buildx bake` to generate the images. DOCKER_SAVE can be set to output to a .tar
dockerx: DOCKER_RULE?=mkdir -p $(DOCKERX_BUILD_TOP)/$@ && cp -r $^ $(DOCKERX_BUILD_TOP)/$@
dockerx: docker | $(ISTIO_DOCKER_TAR)
dockerx:
HUB=$(HUB) \
TAG=$(TAG) \
DOCKER_ALL_VARIANTS="$(DOCKER_ALL_VARIANTS)" \
ISTIO_DOCKER_TAR=$(ISTIO_DOCKER_TAR) \
BASE_VERSION=$(BASE_VERSION) \
DOCKER_SAVE="$(DOCKER_SAVE)" \
./tools/buildx-gen.sh $(DOCKERX_BUILD_TOP) $(DOCKER_TARGETS)
DOCKER_CLI_EXPERIMENTAL=enabled docker buildx bake -f $(DOCKERX_BUILD_TOP)/docker-bake.hcl $(DOCKER_BUILD_VARIANTS)

# Support individual images like `dockerx.pilot`
dockerx.%:
@DOCKER_TARGETS=docker.$* BUILD_ALL=false $(MAKE) --no-print-directory -f Makefile.core.mk dockerx

# Reuse the dockerx target, but export save variable to trigger output to .tar
dockerx.save: DOCKER_SAVE=true
dockerx.save: $(ISTIO_DOCKER_TAR)
dockerx.save: dockerx
# We also want to gzip all of them
gzip -f $(ISTIO_DOCKER_TAR)/*

# galley docker images
docker.galley: BUILD_PRE=chmod 755 galley &&
docker.galley: BUILD_ARGS=--build-arg BASE_VERSION=${BASE_VERSION}
docker.galley: galley/docker/Dockerfile.galley
Expand Down Expand Up @@ -247,9 +280,10 @@ docker.base: docker/Dockerfile.base
# 5. This rule finally runs docker build passing $(BUILD_ARGS) to docker if they are specified as a dependency variable

# DOCKER_BUILD_VARIANTS ?=default distroless
DOCKER_BUILD_VARIANTS ?=default
DOCKER_BUILD_VARIANTS ?= default
DOCKER_ALL_VARIANTS ?= default distroless
DEFAULT_DISTRIBUTION=default
DOCKER_RULE=$(foreach VARIANT,$(DOCKER_BUILD_VARIANTS), time (mkdir -p $(DOCKER_BUILD_TOP)/$@ && cp -r $^ $(DOCKER_BUILD_TOP)/$@ && cd $(DOCKER_BUILD_TOP)/$@ && $(BUILD_PRE) docker build $(BUILD_ARGS) --build-arg BASE_DISTRIBUTION=$(VARIANT) -t $(HUB)/$(subst docker.,,$@):$(subst -$(DEFAULT_DISTRIBUTION),,$(TAG)-$(VARIANT)) -f Dockerfile$(suffix $@) . ); )
DOCKER_RULE ?= $(foreach VARIANT,$(DOCKER_BUILD_VARIANTS), time (mkdir -p $(DOCKER_BUILD_TOP)/$@ && cp -r $^ $(DOCKER_BUILD_TOP)/$@ && cd $(DOCKER_BUILD_TOP)/$@ && $(BUILD_PRE) docker build $(BUILD_ARGS) --build-arg BASE_DISTRIBUTION=$(VARIANT) -t $(HUB)/$(subst docker.,,$@):$(subst -$(DEFAULT_DISTRIBUTION),,$(TAG)-$(VARIANT)) -f Dockerfile$(suffix $@) . ); )

# This target will package all docker images used in test and release, without re-building
# go binaries. It is intended for CI/CD systems where the build is done in separate job.
Expand All @@ -276,7 +310,8 @@ tar.docker.app: docker.app | $(ISTIO_DOCKER_TAR)
$(foreach TGT,$(DOCKER_TARGETS),$(eval DOCKER_TAR_TARGETS+=tar.$(TGT)))

# this target saves a tar.gz of each docker image to ${ISTIO_OUT_LINUX}/docker/
docker.save: $(DOCKER_TAR_TARGETS)
#docker.save: $(DOCKER_TAR_TARGETS)
docker.save: dockerx.save

# for each docker.XXX target create a push.docker.XXX target that pushes
# the local docker image to another hub
Expand Down