Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
27 changes: 21 additions & 6 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,24 @@ on:
permissions: {}

env:
REGISTRY: ghcr.io/${{ github.repository }}
TAG: smoke-test
REGISTRY: 127.0.0.1:5000/qubesome
TAG: latest

jobs:
build-images:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
platform: [ubuntu-latest, ubuntu-24.04-arm]

runs-on: ${{ matrix.platform }}

services:
registry:
image: registry:3
ports:
- 5000:5000

steps:
- uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
with:
Expand All @@ -26,10 +38,13 @@ jobs:

- name: Setup Docker Buildx
uses: docker/setup-buildx-action@e468171a9de216ec08956ac3ada2f0791b6bd435 # v3.11.1
with:
name: qubesome
driver-opts: network=host
platforms: linux/amd64,linux/arm64

Comment on lines 39 to 45
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

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

The setup-buildx-action creates a default buildx builder, but the Makefile's buildx-machine target will create its own builder named 'qubesome' with specific configuration (network=host). This means the builder created by the action may be unused. Consider either removing the setup-buildx-action step since the Makefile manages its own builder, or configuring the action to create a builder with the same name and options to avoid creating multiple builders.

Suggested change
- name: Setup Docker Buildx
uses: docker/setup-buildx-action@e468171a9de216ec08956ac3ada2f0791b6bd435 # v3.11.1

Copilot uses AI. Check for mistakes.
- name: Build images
run: |
make build
- name: Test build and push of images
run: make push
Comment on lines +46 to +47
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

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

Both matrix jobs (amd64 and arm64 runners) build and push images for both platforms (linux/amd64,linux/arm64), which means the same multi-platform images are built and pushed twice. This is redundant and wastes CI resources. Consider either having each runner build only for its native platform, or having only one runner build the multi-platform images. If the intent is to test that the build process works on both runner architectures, this should be documented.

Copilot uses AI. Check for mistakes.

- name: Smoke test binaries
run: |
Expand Down
39 changes: 27 additions & 12 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,33 +1,48 @@
REGISTRY ?= workload-images
TAG ?= latest

BUILDER ?= docker
RUNNER ?= docker
BUILDER ?= docker buildx

WORKLOADS=$(shell find workloads -mindepth 2 -maxdepth 2 -type f -name 'Dockerfile' | sort -u | cut -f 2 -d'/')
TOOLS=$(shell find tools -mindepth 2 -maxdepth 2 -type f -name 'Dockerfile' | sort -u | cut -f 2 -d'/')

build:
MACHINE = qubesome

# ACTION can only be --load when TARGET_PLATFORM is the current platform:
# TARGET_PLATFORMS=linux/amd64 ACTION=--load make build-workload-xorg
ACTION ?= --load
TARGET_PLATFORMS ?= $(shell docker info --format '{{.ClientInfo.Os}}/{{.ClientInfo.Arch}}')
SUPPORTED_PLATFORMS = linux/amd64,linux/arm64

build: build-workload-base
$(MAKE) $(addprefix build-workload-, $(WORKLOADS))
$(MAKE) $(addprefix build-tool-, $(TOOLS))

build-workload-%:
buildx-machine:
$(BUILDER) use $(MACHINE) >/dev/null 2>&1 || \
$(BUILDER) create --name=$(MACHINE) --driver-opt network=host --platform=$(SUPPORTED_PLATFORMS)

Comment on lines +26 to +28
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

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

The buildx-machine target attempts to use a builder before checking if it exists, which will always fail on the first run. The logic should first check if the builder exists (using docker buildx inspect) before attempting to use it. The current implementation with 'use' followed by 'create' on failure works but produces unnecessary error output on first run.

Suggested change
$(BUILDER) use $(MACHINE) >/dev/null 2>&1 || \
$(BUILDER) create --name=$(MACHINE) --driver-opt network=host --platform=$(SUPPORTED_PLATFORMS)
$(BUILDER) inspect $(MACHINE) >/dev/null 2>&1 || \
$(BUILDER) create --name=$(MACHINE) --driver-opt network=host --platform=$(SUPPORTED_PLATFORMS) >/dev/null 2>&1
$(BUILDER) use $(MACHINE) >/dev/null 2>&1

Copilot uses AI. Check for mistakes.
build-workload-%: buildx-machine
cd workloads/$(subst :,/,$*); \
$(BUILDER) build --build-arg=REGISTRY=$(REGISTRY) --build-arg=TAG=$(TAG) \
--load -t $(REGISTRY)/$(subst :,/,$*):$(TAG) -f Dockerfile .
$(BUILDER) build --builder $(MACHINE) --platform="$(TARGET_PLATFORMS)" \
--build-arg=REGISTRY=$(REGISTRY) --build-arg=TAG=$(TAG) \
$(ACTION) -t $(REGISTRY)/$(subst :,/,$*):$(TAG) -f Dockerfile .

build-tool-%:
build-tool-%: buildx-machine
cd tools/$(subst :,/,$*); \
$(BUILDER) build --build-arg=REGISTRY=$(REGISTRY) --build-arg=TAG=$(TAG) \
-t $(REGISTRY)/$(subst :,/,$*):$(TAG) -f Dockerfile .
$(BUILDER) build --builder $(MACHINE) --platform="$(TARGET_PLATFORMS)" \
--build-arg=REGISTRY=$(REGISTRY) --build-arg=TAG=$(TAG) \
$(ACTION) -t $(REGISTRY)/$(subst :,/,$*):$(TAG) -f Dockerfile .

push:
$(MAKE) $(addprefix push-workload-, $(WORKLOADS))
$(MAKE) $(addprefix push-tool-, $(TOOLS))

push-workload-%: build-workload-%
cd workloads/$(subst :,/,$*); \
$(BUILDER) push $(REGISTRY)/$(subst :,/,$*):$(TAG)
push-workload-%:
ACTION=--push \
TARGET_PLATFORMS=$(SUPPORTED_PLATFORMS) \
$(MAKE) build-workload-$(subst :,/,$*)
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

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

The target substitution uses the wrong separator. This line should use the colon separator to match the pattern variable, not a forward slash. The correct syntax should be build-workload-$* (not build-workload-$(subst :,/,$*)) since the subst operation is already done by the pattern matching.

Suggested change
$(MAKE) build-workload-$(subst :,/,$*)
$(MAKE) build-workload-$*

Copilot uses AI. Check for mistakes.

ifneq ($(TAG),latest)
cosign sign --yes "$(REGISTRY)/$(subst :,/,$*):$(TAG)"
endif
Expand Down
Loading