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

create builder to use for multiarch #3924

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+8 −2
Open

create builder to use for multiarch #3924

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
004680e
create builder to use for multiarch
KPostOffice Jan 3, 2025
6197bc4
use new Make directive for multiarch kueueviz
KPostOffice Jan 8, 2025
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
11 changes: 5 additions & 6 deletions Makefile
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

I think

kueue/Makefile

Line 362 in 780675f

kueue-viz-image: VIZ_PLATFORMS=linux/amd64

no need anymore, correct?

Copy link
Contributor

@mbobrovskyi mbobrovskyi Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I added it to fix an issue: #3883. However, since you're now using docker buildx create --use, that should resolve it if I understand correctly.

Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ GIT_TAG ?= $(shell git describe --tags --dirty --always)
# Image URL to use all building/pushing image targets
PLATFORMS ?= linux/amd64,linux/arm64,linux/s390x,linux/ppc64le
CLI_PLATFORMS ?= linux/amd64,linux/arm64,darwin/amd64,darwin/arm64
VIZ_PLATFORMS ?= linux/amd64,linux/arm64,linux/s390x,linux/ppc64le
DOCKER_BUILDX_CMD ?= docker buildx
IMAGE_BUILD_CMD ?= $(DOCKER_BUILDX_CMD) build
IMAGE_BUILD_EXTRA_OPTS ?=
Expand Down Expand Up @@ -331,14 +330,15 @@ importer-image: PLATFORMS=linux/amd64
importer-image: PUSH=--load
importer-image: importer-image-build


# Build the kueue-viz dashboard images (frontend and backend)
.PHONY: kueue-viz-image-build
kueue-viz-image-build:
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.

This is not obvious to me. Can you add a comment that this is required for emulated builds for docker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes for sure, I'm not even sure this is the best place for this. I put it in quickly because I was having trouble reproducing on my local env.

/hold

BUILDER=$(shell $(DOCKER_BUILDX_CMD) create --use)
$(IMAGE_BUILD_CMD) \
-t $(IMAGE_REGISTRY)/kueue-viz-backend:$(GIT_TAG) \
-t $(IMAGE_REGISTRY)/kueue-viz-backend:$(RELEASE_BRANCH)-latest \
--platform=$(VIZ_PLATFORMS) \
--platform=$(PLATFORMS) \
Copy link
Contributor

@mbobrovskyi mbobrovskyi Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why were these changes made?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The VIZ_PLATFORMS was added since it wasn't working with multiarch builds. I figured it would be safe to remove and prefer the more generic PLATFORMS environment variable

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see, they are the same. In that case, could you please remove VIZ_PLATFORMS everywhere?

--build-arg BASE_IMAGE=$(BASE_IMAGE) \
--build-arg BUILDER_IMAGE=$(BUILDER_IMAGE) \
--build-arg CGO_ENABLED=$(CGO_ENABLED) \
Expand All @@ -347,18 +347,17 @@ kueue-viz-image-build:
$(IMAGE_BUILD_CMD) \
-t $(IMAGE_REGISTRY)/kueue-viz-frontend:$(GIT_TAG) \
-t $(IMAGE_REGISTRY)/kueue-viz-frontend:$(RELEASE_BRANCH)-latest \
--platform=$(VIZ_PLATFORMS) \
--platform=$(PLATFORMS) \
$(PUSH) \
-f ./cmd/experimental/kueue-viz/frontend/Dockerfile ./cmd/experimental/kueue-viz/frontend; \
$(DOCKER_BUILDX_CMD) rm $$BUILDER

.PHONY: kueue-viz-image-push
kueue-viz-image-push: PUSH=--push
kueue-viz-image-push: kueue-viz-image-build

# Build a docker local us-central1-docker.pkg.dev/k8s-staging-images/kueue/kueue-viz image
.PHONY: kueue-viz-image
kueue-viz-image: VIZ_PLATFORMS=linux/amd64
kueue-viz-image: PUSH=--load
kueue-viz-image: kueue-viz-image-build


Expand Down