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

Enable multi-platform support for bridge marker image builds #77

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
51 changes: 45 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
REGISTRY ?= quay.io/kubevirt
IMAGE_TAG ?= latest
IMAGE_GIT_TAG ?= $(shell git describe --abbrev=8 --tags)
PLATFORM_LIST ?= linux/amd64,linux/s390x,linux/arm64
ARCH := $(shell uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/')
PLATFORMS ?= linux/${ARCH}
PLATFORMS := $(if $(filter all,$(PLATFORMS)),$(PLATFORM_LIST),$(PLATFORMS))
# Set the platforms for building a multi-platform supported image.
# Example:
# PLATFORMS ?= linux/amd64,linux/arm64,linux/s390x
# Alternatively, you can export the PLATFORMS variable like this:
# export PLATFORMS=linux/arm64,linux/s390x,linux/amd64
# or export PLATFORMS=all to automatically include all supported platforms.
DOCKER_BUILDER ?= marker-docker-builder
MARKER_IMAGE_TAGGED := ${REGISTRY}/bridge-marker:${IMAGE_TAG}
MARKER_IMAGE_GIT_TAGGED := ${REGISTRY}/bridge-marker:${IMAGE_GIT_TAG}

BIN_DIR = $(CURDIR)/build/_output/bin/
export GOPROXY=direct
Expand Down Expand Up @@ -40,15 +53,22 @@ functest: $(GINKGO)

marker: $(GO)
hack/version.sh > $(BIN_DIR)/.version
CGO_ENABLED=0 GOOS=linux GOARCH=amd64 $(GO) build -o $(BIN_DIR)/marker github.com/kubevirt/bridge-marker/cmd/marker

docker-build: marker
$(OCI_BIN) build -t ${REGISTRY}/bridge-marker:${IMAGE_TAG} ./build
ifeq ($(OCI_BIN),podman)
$(MAKE) build-multiarch-marker-podman
else ifeq ($(OCI_BIN),docker)
$(MAKE) build-multiarch-marker-docker
else
$(error Unsupported OCI_BIN value: $(OCI_BIN))
endif

docker-push:
$(OCI_BIN) push ${TLS_SETTING} ${REGISTRY}/bridge-marker:${IMAGE_TAG}
$(OCI_BIN) tag ${REGISTRY}/bridge-marker:${IMAGE_TAG} ${REGISTRY}/bridge-marker:${IMAGE_GIT_TAG}
$(OCI_BIN) push ${TLS_SETTING} ${REGISTRY}/bridge-marker:${IMAGE_GIT_TAG}
ifeq ($(OCI_BIN),podman)
podman manifest push ${TLS_SETTING} ${MARKER_IMAGE_TAGGED} ${MARKER_IMAGE_TAGGED}
podman tag ${MARKER_IMAGE_TAGGED} ${MARKER_IMAGE_GIT_TAGGED}
podman manifest push ${TLS_SETTING} ${MARKER_IMAGE_GIT_TAGGED} ${MARKER_IMAGE_GIT_TAGGED}
endif

manifests:
./hack/build-manifests.sh
Expand All @@ -69,4 +89,23 @@ vendor: $(GO)
tools: $(GO)
./hack/install-tools.sh

.PHONY: build format docker-build docker-push manifests cluster-up cluster-down cluster-sync vendor marker tools
build-multiarch-marker-docker:
ARCH=$(ARCH) PLATFORMS=$(PLATFORMS) MARKER_IMAGE_TAGGED=$(MARKER_IMAGE_TAGGED) MARKER_IMAGE_GIT_TAGGED=$(MARKER_IMAGE_GIT_TAGGED) DOCKER_BUILDER=$(DOCKER_BUILDER) ./hack/build-marker-docker.sh

build-multiarch-marker-podman:
ARCH=$(ARCH) PLATFORMS=$(PLATFORMS) MARKER_IMAGE_TAGGED=$(MARKER_IMAGE_TAGGED) MARKER_IMAGE_GIT_TAGGED=$(MARKER_IMAGE_GIT_TAGGED) ./hack/build-marker-podman.sh

.PHONY: \
build \
build-multiarch-marker-docker \
build-multiarch-marker-podman \
format \
docker-build \
docker-push \
manifests \
cluster-up \
cluster-down \
cluster-sync \
vendor \
marker \
tools
32 changes: 29 additions & 3 deletions build/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,30 @@
FROM registry.access.redhat.com/ubi8/ubi-minimal
COPY _output/bin/marker /marker
COPY _output/bin/.version /.version
ARG BUILD_ARCH=amd64
FROM --platform=linux/${BUILD_ARCH} quay.io/centos/centos:stream9 AS builder
ARG TARGETOS
ARG TARGETARCH
ENV TARGETOS=${TARGETOS:-linux}
ENV TARGETARCH=${TARGETARCH:-amd64}

ARG BUILDOS
ARG BUILDARCH
ENV BUILDOS=${BUILDOS:-linux}
ENV BUILDARCH=${BUILDARCH:-amd64}

WORKDIR /go/src/bridge-marker
RUN dnf install -y wget && dnf clean all
COPY go.mod .
COPY go.sum .
RUN GO_VERSION=$(sed -En 's/^go +(.*)$/\1/p' go.mod) && \
wget https://dl.google.com/go/go${GO_VERSION}.${BUILDOS}-${BUILDARCH}.tar.gz && \
tar -C /usr/local -xzf go${GO_VERSION}.${BUILDOS}-${BUILDARCH}.tar.gz && \
rm go${GO_VERSION}.${BUILDOS}-${BUILDARCH}.tar.gz

ENV PATH=$PATH:/usr/local/go/bin
RUN go mod download
COPY . .
RUN CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build -o build/_output/bin/marker github.com/kubevirt/bridge-marker/cmd/marker

Copy link
Collaborator

@oshoval oshoval Dec 3, 2024

Choose a reason for hiding this comment

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

imho it is better to be explicit with what we build please
./cmd/marker

EDIT - as we spoke, since it was like that, lets keep it

FROM --platform=linux/${TARGETARCH} registry.access.redhat.com/ubi8/ubi-minimal
COPY --from=builder /go/src/bridge-marker/build/_output/bin/marker /marker
COPY --from=builder /go/src/bridge-marker/build/_output/bin/.version /.version
ENTRYPOINT [ "/marker"]
18 changes: 18 additions & 0 deletions hack/build-marker-docker.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/usr/bin/env bash

if [ -z "$ARCH" ] || [ -z "$PLATFORMS" ] || [ -z "$MARKER_IMAGE_TAGGED" ] || [ -z "$MARKER_IMAGE_GIT_TAGGED" ]; then
echo "Error: ARCH, PLATFORMS, MARKER_IMAGE_TAGGED, and MARKER_IMAGE_GIT_TAGGED must be set."
exit 1
fi

IFS=',' read -r -a PLATFORM_LIST <<< "$PLATFORMS"

BUILD_ARGS="--build-arg BUILD_ARCH=$ARCH -f build/Dockerfile -t $MARKER_IMAGE_TAGGED -t $MARKER_IMAGE_GIT_TAGGED . --push"

if [ ${#PLATFORM_LIST[@]} -eq 1 ]; then
docker build --platform "$PLATFORMS" $BUILD_ARGS
else
./hack/init-buildx.sh "$DOCKER_BUILDER"
docker buildx build --platform "$PLATFORMS" $BUILD_ARGS
docker buildx rm "$DOCKER_BUILDER" 2>/dev/null || echo "Builder ${DOCKER_BUILDER} not found or already removed, skipping."
fi
23 changes: 23 additions & 0 deletions hack/build-marker-podman.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#!/usr/bin/env bash

if [ -z "$ARCH" ] || [ -z "$PLATFORMS" ] || [ -z "$MARKER_IMAGE_TAGGED" ]; then
echo "Error: ARCH, PLATFORMS, and MARKER_IMAGE_TAGGED must be set."
exit 1
fi

IFS=',' read -r -a PLATFORM_LIST <<< "$PLATFORMS"

podman manifest rm "${MARKER_IMAGE_TAGGED}" 2>/dev/null || true
podman manifest rm "${MARKER_IMAGE_GIT_TAGGED}" 2>/dev/null || true
podman rmi "${MARKER_IMAGE_TAGGED}" 2>/dev/null || true
podman rmi "${MARKER_IMAGE_GIT_TAGGED}" 2>/dev/null || true

podman manifest create "${MARKER_IMAGE_TAGGED}"

for platform in "${PLATFORM_LIST[@]}"; do
podman build \
--build-arg BUILD_ARCH="$ARCH" \
--platform "$platform" \
--manifest "${MARKER_IMAGE_TAGGED}" \
-f build/Dockerfile .
done
56 changes: 56 additions & 0 deletions hack/init-buildx.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#!/usr/bin/env bash

check_buildx() {
export DOCKER_CLI_EXPERIMENTAL=enabled

if ! docker buildx > /dev/null 2>&1; then
mkdir -p ~/.docker/cli-plugins
BUILDX_VERSION=$(curl -s https://api.github.com/repos/docker/buildx/releases/latest | jq -r .tag_name)
ARCH=$(uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/')
curl -L https://github.com/docker/buildx/releases/download/"${BUILDX_VERSION}"/buildx-"${BUILDX_VERSION}".linux-"${ARCH}" --output ~/.docker/cli-plugins/docker-buildx
chmod a+x ~/.docker/cli-plugins/docker-buildx
fi
}

create_or_use_buildx_builder() {
local builder_name=$1
if [ -z "$builder_name" ]; then
echo "Error: Builder name is required."
exit 1
fi

check_buildx

current_builder="$(docker buildx inspect "${builder_name}" 2>/dev/null)" || echo "Builder '${builder_name}' not found"

if ! grep -q "^Driver: docker$" <<<"${current_builder}" && \
grep -q "linux/amd64" <<<"${current_builder}" && \
grep -q "linux/arm64" <<<"${current_builder}" && \
grep -q "linux/s390x" <<<"${current_builder}"; then
echo "The current builder already has multi-architecture support (amd64, arm64, s390x)."
echo "Skipping setup as the builder is already configured correctly."
exit 0
fi

# Check if the builder already exists by parsing the output of `docker buildx ls`
# We check if the builder_name appears in the list of active builders
existing_builder=$(docker buildx ls | grep -w "$builder_name" | awk '{print $1}')

if [ -n "$existing_builder" ]; then
echo "Builder '$builder_name' already exists."
echo "Using existing builder '$builder_name'."
docker buildx use "$builder_name"
else
echo "Creating a new Docker Buildx builder: $builder_name"
docker buildx create --driver-opt network=host --use --name "$builder_name"
echo "The new builder '$builder_name' has been created and set as active."
fi
}

if [ $# -eq 1 ]; then
create_or_use_buildx_builder "$1"
else
echo "Usage: $0 <builder_name>"
echo "Example: $0 mybuilder"
exit 1
fi
3 changes: 2 additions & 1 deletion hack/install-go.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

destination=$1
version=$(grep "^go " go.mod |awk '{print $2}')
tarball=go$version.linux-amd64.tar.gz
arch=$(uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/')
Copy link
Collaborator

@oshoval oshoval Dec 3, 2024

Choose a reason for hiding this comment

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

kubevirt/kubesecondarydns@d03ce8e

here the logic is bit different (for s390x), are both fine ? or one should be fixed
thanks

Copy link
Contributor Author

@ashokpariya0 ashokpariya0 Dec 3, 2024

Choose a reason for hiding this comment

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

kubevirt/kubesecondarydns@d03ce8e

Both are same for s390x case.
Both approaches are fine. However, one thing to note is that linux/ppc64le will fail with [this commit](kubevirt/kubesecondarydns@d03ce8e), as it explicitly supports only amd64, arm64, and s390x.

Copy link
Collaborator

@oshoval oshoval Dec 3, 2024

Choose a reason for hiding this comment

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

thank you
i think we dont support ppc64le atm, please raise it there as well if you want (for visibility)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you i think we dont support ppc64le atm, please raise it there as well if you want (for visibility)

okay, I can see for ipam controller (https://github.com/kubevirt/ipam-extensions/blob/925bee6139751fcb241a435bae8b79ce1dfce595/Makefile#L121C50-L121C63)

Copy link
Collaborator

@oshoval oshoval Dec 3, 2024

Choose a reason for hiding this comment

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

yes but i dont think we need to support ppc64le generally speaking on other sig network repos
@phoracek please keep me honest here, i think we discussed it once

EDIT - anyhow, imho, even if we need it, it should be on a follow-up all over,
not to be mixed with the current effort which lays the foundations and support for new archs already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oshoval Just to inform you, I noticed that linux/ppc64le is supported for components like the bridge-operator, OVS, Multus CNI, etc.

tarball=go$version.linux-$arch.tar.gz
url=https://dl.google.com/go/

mkdir -p $destination
Expand Down
Loading