mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-23 03:27:50 +09:00
fix(build): honour PUSH and LOAD from the environment and in image-talos (#4654)
## What this PR does A local build that asks to load images into docker no longer pushes them. `hack/common-envs.mk` set `PUSH` and `LOAD` with plain `:=`, and a makefile assignment beats the environment, so `LOAD=1 PUSH=0 make image` quietly pushed a multi-arch build. Both now take the environment value, and an empty one falls back to the default. I also made `PUSH` default to 0 when `LOAD=1`, so `LOAD=1` alone only loads. Before this, `make image LOAD=1` pushed and loaded at once. An explicit `PUSH` from the environment or the command line still wins, and `OCI_EXPORT_DIR` still forces both off. Every buildx target gets the values through `BUILDX_ARGS`, so the fix lives in that one file. The Talos installer was the one image that ignored `PUSH`. It is published with skopeo, not buildx, so it never saw `--push=$(PUSH)`. Its registry copies now run only when `PUSH=1`. A build that pushes nothing no longer writes `PUSHED_TAGS_LOG`. That log tells the multi-arch stitch which tags to move, so it must hold only tags that were really pushed. The cache helper lost an argument nobody used. No caller passes the second argument of `cache-args`, and an explicit tag there would bypass `CACHE_TAG`, so the amd64 and arm64 builds of that image would share one cache ref. CI sets neither `PUSH` nor `LOAD`, so CI builds do the same thing as before. Each fix has a `make --dry-run` test in `hack/common-envs_test.bats`. Closes #4499 Closes #4504 Closes #4551 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The ccp skills pass `PUSH=1` explicitly, which still works. The website docs do not mention `PUSH`, `LOAD` or `cache-args`. ### Release note ```release-note fix(build): `LOAD` and `PUSH` set in the environment are now honoured, `LOAD=1` no longer pushes unless `PUSH=1` is also set, and the Talos installer is no longer published when `PUSH=0`. ```
This commit is contained in:
4 files changed
+56
-22
No files matched your search
+11
-9
@@ -18,7 +18,7 @@ CACHE_REGISTRY ?= $(REGISTRY)
|
||||
# that does not collide between concurrent builds:
|
||||
# * pull-requests.yaml -> pr-<N>-<sha>
|
||||
# * tags.yaml -> <ref_name> (e.g. v1.5.0)
|
||||
# * local -> dev (with PUSH=0 by default)
|
||||
# * local -> dev (pushes unless PUSH=0; LOAD=1 defaults PUSH to 0)
|
||||
IMAGE_TAG ?= dev
|
||||
|
||||
# Opt-in extra tags. Workflows set these explicitly; defaults are off so a
|
||||
@@ -34,8 +34,11 @@ PUBLISH_FLOATING ?= 0
|
||||
# never race on the cache manifest (the 409 collisions PR #2711 fixed for tags).
|
||||
WRITE_CACHE ?= 0
|
||||
|
||||
PUSH := 1
|
||||
LOAD := 0
|
||||
# Environment and command line both override these, and an empty value from the
|
||||
# environment counts as unset. LOAD=1 loads into the local docker store, so PUSH
|
||||
# follows it to 0 unless PUSH is set too.
|
||||
LOAD := $(or $(strip $(LOAD)),0)
|
||||
PUSH := $(or $(strip $(PUSH)),$(if $(filter 1,$(LOAD)),0,1))
|
||||
|
||||
# OCI_EXPORT_DIR: when set, build every image to a per-image OCI archive
|
||||
# (<dir>/<name>.oci.tar) instead of pushing or loading it. Fork PRs use this:
|
||||
@@ -116,28 +119,27 @@ oci-output = $(if $(strip $(OCI_EXPORT_DIR)), --output type=oci$(comma)dest=$(OC
|
||||
# line. hack/stitch-multiarch.sh moves exactly those tags onto the multi-arch
|
||||
# index; no ref in the tree names a versioned tag, so this is how it learns
|
||||
# one was pushed. The append happens when the recipe is expanded, `make -n`
|
||||
# included, and is skipped under OCI_EXPORT_DIR, which pushes nothing.
|
||||
# included, and is skipped when the build pushes nothing (PUSH other than 1, or
|
||||
# OCI_EXPORT_DIR).
|
||||
image-tag-refs = $(REGISTRY)/$(1):$(IMAGE_TAG)$(if $(strip $(OCI_EXPORT_DIR)),,$(if $(filter 1,$(PUBLISH_VERSIONED)),$(if $(filter-out $(IMAGE_TAG),$(strip $(2))), $(REGISTRY)/$(1):$(strip $(2))))$(if $(filter 1,$(PUBLISH_FLOATING)), $(REGISTRY)/$(1):latest))
|
||||
|
||||
define image-tags
|
||||
$(foreach r,$(call image-tag-refs,$(1),$(2)),--tag $(r))$(call oci-output,$(1))$(if $(and $(strip $(PUSHED_TAGS_LOG)),$(if $(strip $(OCI_EXPORT_DIR)),,1)),$(shell printf '%s\n' $(call image-tag-refs,$(1),$(2)) >>'$(PUSHED_TAGS_LOG)'))
|
||||
$(foreach r,$(call image-tag-refs,$(1),$(2)),--tag $(r))$(call oci-output,$(1))$(if $(and $(strip $(PUSHED_TAGS_LOG)),$(filter 1,$(PUSH)),$(if $(strip $(OCI_EXPORT_DIR)),,1)),$(shell printf '%s\n' $(call image-tag-refs,$(1),$(2)) >>'$(PUSHED_TAGS_LOG)'))
|
||||
endef
|
||||
|
||||
# cache-args <image-name> [<cache-tag>]
|
||||
# cache-args <image-name>
|
||||
# Expands to buildx cache flags for one image:
|
||||
# --cache-from is always emitted (a missing cache 404s harmlessly -> cold build)
|
||||
# --cache-to is emitted only when WRITE_CACHE=1 (main/release), writing mode=max
|
||||
# so ALL stages are cached -- including the multistage `builder` layers that
|
||||
# `--cache-to type=inline` could never export. oci-mediatypes + image-manifest
|
||||
# keep the cache manifest portable across registries (OCIR/ghcr/ECR).
|
||||
# <cache-tag> defaults to $(CACHE_TAG); pass an explicit tag for images that build
|
||||
# a distinct artifact per loop iteration (e.g. ubuntu-container-disk per k8s ver).
|
||||
# CACHE_TAG lets a build for another platform keep a cache of its own
|
||||
# (buildcache-arm64) instead of overwriting the amd64 one with every write.
|
||||
# $(comma) (defined above) escapes the literal commas in the --cache-to value so
|
||||
# make does not mis-parse them as $(if ...) argument separators.
|
||||
CACHE_TAG ?= buildcache
|
||||
cache-args = --cache-from type=registry,ref=$(CACHE_REGISTRY)/$(1):$(if $(2),$(2),$(CACHE_TAG))$(if $(filter 1,$(WRITE_CACHE)), --cache-to type=registry$(comma)ref=$(CACHE_REGISTRY)/$(1):$(if $(2),$(2),$(CACHE_TAG))$(comma)mode=max$(comma)oci-mediatypes=true$(comma)image-manifest=true)
|
||||
cache-args = --cache-from type=registry,ref=$(CACHE_REGISTRY)/$(1):$(CACHE_TAG)$(if $(filter 1,$(WRITE_CACHE)), --cache-to type=registry$(comma)ref=$(CACHE_REGISTRY)/$(1):$(CACHE_TAG)$(comma)mode=max$(comma)oci-mediatypes=true$(comma)image-manifest=true)
|
||||
|
||||
ifeq ($(COZYSTACK_VERSION),)
|
||||
$(shell git remote add upstream https://github.com/cozystack/cozystack.git || true)
|
||||
|
||||
@@ -121,7 +121,35 @@
|
||||
if echo "$out" | grep -q -- '--platform'; then echo "FAIL: LOAD=1 passes a platform, so buildx builds an index it cannot load"; false; fi
|
||||
}
|
||||
|
||||
@test "CACHE_TAG moves the default cache ref and an explicit cache tag still wins" {
|
||||
@test "LOAD and PUSH from the environment reach buildx, and LOAD=1 alone does not push" {
|
||||
out=$(LOAD=1 PUSH=0 make -n -C packages/system/cozystack-controller image IMAGE_TAG=pr-1-abc COZYSTACK_VERSION=0 BUILDER=b)
|
||||
echo "$out" | grep -q -- '--push=0 --load=1' || { echo "FAIL: environment LOAD=1 PUSH=0 ignored"; false; }
|
||||
if echo "$out" | grep -q -- '--platform'; then echo "FAIL: environment LOAD=1 still builds a multi-arch index"; false; fi
|
||||
out=$(LOAD=1 make -n -C packages/system/cozystack-controller image IMAGE_TAG=pr-1-abc COZYSTACK_VERSION=0 BUILDER=b)
|
||||
echo "$out" | grep -q -- '--push=0 --load=1' || { echo "FAIL: LOAD=1 alone pushes"; false; }
|
||||
# An explicit PUSH still wins over the LOAD-derived default.
|
||||
out=$(LOAD=1 PUSH=1 make -n -C packages/system/cozystack-controller image IMAGE_TAG=pr-1-abc COZYSTACK_VERSION=0 BUILDER=b)
|
||||
echo "$out" | grep -q -- '--push=1 --load=1'
|
||||
# The fork export must keep beating an environment PUSH=1.
|
||||
out=$(PUSH=1 make -n -C packages/system/cozystack-controller image IMAGE_TAG=pr-1-abc COZYSTACK_VERSION=0 BUILDER=b OCI_EXPORT_DIR=/tmp/ocitest)
|
||||
echo "$out" | grep -q -- '--push=0 --load=0'
|
||||
}
|
||||
|
||||
@test "an empty PUSH or LOAD in the environment falls back to the default" {
|
||||
out=$(PUSH= LOAD= make -n -C packages/system/cozystack-controller image IMAGE_TAG=pr-1-abc COZYSTACK_VERSION=0 BUILDER=b)
|
||||
echo "$out" | grep -q -- '--push=1 --load=0' || { echo "FAIL: empty PUSH/LOAD reach buildx as empty flags"; false; }
|
||||
out=$(PUSH= LOAD=1 make -n -C packages/system/cozystack-controller image IMAGE_TAG=pr-1-abc COZYSTACK_VERSION=0 BUILDER=b)
|
||||
echo "$out" | grep -q -- '--push=0 --load=1'
|
||||
}
|
||||
|
||||
@test "talos publishes nothing to the registry with PUSH=0" {
|
||||
out=$(make -n -B -C packages/core/talos image-talos PUSH=0 PUBLISH_VERSIONED=1 PUBLISH_FLOATING=1 IMAGE_TAG=pr-1-abc COZYSTACK_VERSION=0)
|
||||
echo "$out" | grep -q 'docker://'
|
||||
pushes=$(echo "$out" | grep 'docker://' | grep -vF '[ "0" = "1" ]' || true)
|
||||
if [ -n "$pushes" ]; then echo "FAIL: talos pushes with PUSH=0: $pushes"; false; fi
|
||||
}
|
||||
|
||||
@test "CACHE_TAG moves the cache ref of every image" {
|
||||
# The arm64 leg writes its own mode=max cache. Sharing the amd64 ref would
|
||||
# make every write from one leg evict the other's layers.
|
||||
out=$(make -n -C packages/system/cozystack-controller image IMAGE_TAG=pr-1-abc COZYSTACK_VERSION=0 BUILDER=b WRITE_CACHE=1)
|
||||
@@ -130,14 +158,13 @@
|
||||
out=$(make -n -C packages/system/cozystack-controller image IMAGE_TAG=pr-1-abc COZYSTACK_VERSION=0 BUILDER=b WRITE_CACHE=1 CACHE_TAG=buildcache-arm64)
|
||||
echo "$out" | grep -q -- '--cache-from type=registry,ref=[^ ]*/cozystack-controller:buildcache-arm64 '
|
||||
echo "$out" | grep -q -- '--cache-to type=registry,ref=[^ ]*/cozystack-controller:buildcache-arm64,'
|
||||
# The second argument of cache-args names a per-iteration cache; CACHE_TAG
|
||||
# must not override it.
|
||||
# No per-call tag may bypass CACHE_TAG, or both legs write one cache ref.
|
||||
tmp=$(mktemp -d)
|
||||
printf 'include %s/hack/common-envs.mk\nprobe:\n\t@echo $(call cache-args,img,percall)\n' "$(pwd)" > "$tmp/Makefile"
|
||||
out=$(make -s -C "$tmp" probe COZYSTACK_VERSION=0 WRITE_CACHE=1 CACHE_TAG=buildcache-arm64)
|
||||
rm -rf "$tmp"
|
||||
echo "$out" | grep -q -- '--cache-from type=registry,ref=[^ ]*/img:percall '
|
||||
echo "$out" | grep -q -- '--cache-to type=registry,ref=[^ ]*/img:percall,'
|
||||
echo "$out" | grep -q -- '--cache-from type=registry,ref=[^ ]*/img:buildcache-arm64 '
|
||||
echo "$out" | grep -q -- '--cache-to type=registry,ref=[^ ]*/img:buildcache-arm64,'
|
||||
}
|
||||
|
||||
@test "the kamaji provider image is pushed under the build's IMAGE_TAG like every other image" {
|
||||
@@ -159,7 +186,7 @@
|
||||
if echo "$tags" | grep -q -- '-$'; then echo "FAIL: a tag ends in '-': $tags"; false; fi
|
||||
}
|
||||
|
||||
@test "image-tags records every tag it pushes in PUSHED_TAGS_LOG, and nothing when unset or exporting" {
|
||||
@test "image-tags records every tag it pushes in PUSHED_TAGS_LOG, and nothing when unset, exporting or not pushing" {
|
||||
# The stitch moves exactly these tags onto the multi-arch index. A component
|
||||
# version pushed under PUBLISH_VERSIONED=1 is named by no ref in the tree, so
|
||||
# a tag missing here would stay on the amd64-only image.
|
||||
@@ -174,6 +201,10 @@
|
||||
make -n -C packages/system/cozystack-controller image IMAGE_TAG=pr-1-abc BUILDER=b \
|
||||
OCI_EXPORT_DIR=/tmp/ocitest PUSHED_TAGS_LOG="$tmp/export-log" >/dev/null
|
||||
[ ! -e "$tmp/export-log" ]
|
||||
# A build that does not push must not hand its tags to the stitch.
|
||||
LOAD=1 make -n -C packages/system/cozystack-controller image IMAGE_TAG=pr-1-abc BUILDER=b \
|
||||
PUSHED_TAGS_LOG="$tmp/load-log" >/dev/null
|
||||
if [ -s "$tmp/load-log" ]; then echo "FAIL: LOAD=1 logs tags it never pushed: $(cat "$tmp/load-log")"; false; fi
|
||||
rm -rf "$tmp"
|
||||
}
|
||||
|
||||
|
||||
@@ -21,10 +21,11 @@
|
||||
# if it points at A, so none is left naming the amd64 half alone: <tag>,
|
||||
# the tag <t> the ref carries, and every <repo>:<tag> line in the file
|
||||
# PUSHED_TAGS_LOG names. image-tags in hack/common-envs.mk writes that
|
||||
# log when the variable is set, and it is how the component version
|
||||
# pushed beside <image-tag> under PUBLISH_VERSIONED=1 is found, since
|
||||
# most refs name only <image-tag>. These are the only tags read; a tag of
|
||||
# another build that sits on the same bytes is not this build's to move.
|
||||
# log when the variable is set and PUSH=1, and it is how the component
|
||||
# version pushed beside <image-tag> under PUBLISH_VERSIONED=1 is found,
|
||||
# since most refs name only <image-tag>. These are the only tags read;
|
||||
# a tag of another build that sits on the same bytes is not this build's
|
||||
# to move.
|
||||
# PUSHED_TAGS_LOG set to a file that does not exist fails the run.
|
||||
# 3. A is replaced with the index digest in the files hack/lib/image-refs.sh
|
||||
# enumerates, and nowhere else.
|
||||
|
||||
@@ -30,9 +30,9 @@ image-talos: talos-installer
|
||||
for a in $(TALOS_ARCHES); do skopeo copy docker-archive:$(ASSETS)/installer-$$a.tar oci:$(ASSETS)/installer-oci:$$a; done; \
|
||||
hack/installer-index.sh $(ASSETS)/installer-oci $(TALOS_ARCHES); \
|
||||
SRC=oci:$(ASSETS)/installer-oci:index; \
|
||||
$(if $(strip $(OCI_EXPORT_DIR)),mkdir -p "$(OCI_EXPORT_DIR)"; skopeo copy --all "$$SRC" "oci-archive:$(OCI_EXPORT_DIR)/talos.oci.tar:$(IMAGE_TAG)",skopeo copy --all "$$SRC" docker://$(REGISTRY)/talos:$(IMAGE_TAG)); \
|
||||
if [ -z "$(OCI_EXPORT_DIR)" ] && [ "$(PUBLISH_VERSIONED)" = "1" ]; then skopeo copy --all "$$SRC" docker://$(REGISTRY)/talos:$(TALOS_VERSION); fi; \
|
||||
if [ -z "$(OCI_EXPORT_DIR)" ] && [ "$(PUBLISH_FLOATING)" = "1" ]; then skopeo copy --all "$$SRC" docker://$(REGISTRY)/talos:latest; fi
|
||||
$(if $(strip $(OCI_EXPORT_DIR)),mkdir -p "$(OCI_EXPORT_DIR)"; skopeo copy --all "$$SRC" "oci-archive:$(OCI_EXPORT_DIR)/talos.oci.tar:$(IMAGE_TAG)",if [ "$(PUSH)" = "1" ]; then skopeo copy --all "$$SRC" docker://$(REGISTRY)/talos:$(IMAGE_TAG); fi); \
|
||||
if [ -z "$(OCI_EXPORT_DIR)" ] && [ "$(PUSH)" = "1" ] && [ "$(PUBLISH_VERSIONED)" = "1" ]; then skopeo copy --all "$$SRC" docker://$(REGISTRY)/talos:$(TALOS_VERSION); fi; \
|
||||
if [ -z "$(OCI_EXPORT_DIR)" ] && [ "$(PUSH)" = "1" ] && [ "$(PUBLISH_FLOATING)" = "1" ]; then skopeo copy --all "$$SRC" docker://$(REGISTRY)/talos:latest; fi
|
||||
rm -rf $(ASSETS)/installer-oci
|
||||
|
||||
image-matchbox: $(call assets,kernel-%,$(BOOT_ARCHES)) $(call assets,initramfs-metal-%.xz,$(BOOT_ARCHES))
|
||||
|
||||
Reference in new issue
Block a user