From d6d25ac40ab4fcf158623a909e6b59b4dd5ab92d Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Mon, 10 Aug 2026 10:57:16 -0400 Subject: [PATCH 1/7] Make controller-gen/kustomize/golangci-lint version checks reliable Cherry-pick of the fix from oadp-dev. go-install-tool-branch only installs when the binary is missing, never verifying the pinned version against what's already on disk. Once a binary lands in bin//, it's reused forever across branch switches and version bumps since bin/ is gitignored and nothing else resets it. kustomize and controller-gen's targets also weren't fully .PHONY (only the wrapper name was, not the binary path), so Make's own mtime-based staleness check could skip their recipe entirely before any version check ran. Introduce go-install-tool-versioned, which compares a sidecar .version marker file against the pinned version instead of introspecting the binary's own --version output. Binary introspection isn't reliable for every tool installed this way: kustomize's `version` command depends on ldflags its own release process sets, which `go install` doesn't set, so identically-installed kustomize binaries were observed reporting "(devel)", an unexpanded `$Format:%H$` git-archive placeholder, or a correct version string depending on unrelated build-time factors. oadp-1.4's golangci-lint target didn't have any version-check at all (unlike oadp-dev's, which had a partial/broken one), so it's folded into the same fix here. Co-authored-by: Claude Signed-off-by: Tiger Kaovilai --- Makefile | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 8d3dabb8d18..4a21a267898 100644 --- a/Makefile +++ b/Makefile @@ -706,28 +706,51 @@ rm -rf $$TMP_DIR ;\ } endef -.PHONY: golangci-lint +# go-install-tool-versioned installs $2 to branch-specific path $1, but only if $1 is missing +# or $1.version doesn't match the pinned version $3. Uses a sidecar marker file instead of +# introspecting the binary's own --version output, because that output is unreliable for some +# tools when installed via `go install` (e.g. kustomize reports "(devel)" or an unexpanded +# `$$Format:%H$$` placeholder instead of its real version, depending on build-time factors). +define go-install-tool-versioned +@if [ -f $(1) ] && [ -f $(1).version ] && [ "$$(cat $(1).version)" = "$(3)" ]; then \ + echo "$(notdir $(1)) $(3) is already installed" ;\ +else \ + set -e ;\ + mkdir -p $(dir $(1)) ;\ + rm -f $(1) $(1).version ;\ + TMP_DIR=$$(mktemp -d) ;\ + cd $$TMP_DIR ;\ + go mod init tmp ;\ + echo "Installing $(notdir $(1)) $(3)" ;\ + GOBIN=$(dir $(1)) go install -mod=mod $(2) ;\ + cd - >/dev/null ;\ + rm -rf $$TMP_DIR ;\ + echo "$(3)" > $(1).version ;\ +fi +endef + +.PHONY: golangci-lint $(GOLANGCI_LINT) golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary. $(GOLANGCI_LINT): $(LOCALBIN) - $(call go-install-tool-branch,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)) + $(call go-install-tool-versioned,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION),$(GOLANGCI_LINT_VERSION)) @if [ -L "$(LOCALBIN)/golangci-lint" ]; then \ unlink "$(LOCALBIN)/golangci-lint"; \ fi @ln -sf "$(LOCALBIN)/$(BRANCH_VERSION)/golangci-lint" "$(LOCALBIN)/golangci-lint" -.PHONY: kustomize +.PHONY: kustomize $(KUSTOMIZE) kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary. If wrong version is installed, it will be removed before downloading. $(KUSTOMIZE): $(LOCALBIN) - $(call go-install-tool-branch,$(KUSTOMIZE),sigs.k8s.io/kustomize/kustomize/v4@$(KUSTOMIZE_VERSION)) + $(call go-install-tool-versioned,$(KUSTOMIZE),sigs.k8s.io/kustomize/kustomize/v4@$(KUSTOMIZE_VERSION),$(KUSTOMIZE_VERSION)) @if [ -L "$(LOCALBIN)/kustomize" ]; then \ unlink "$(LOCALBIN)/kustomize"; \ fi @ln -sf "$(LOCALBIN)/$(BRANCH_VERSION)/kustomize" "$(LOCALBIN)/kustomize" -.PHONY: controller-gen +.PHONY: controller-gen $(CONTROLLER_GEN) controller-gen: $(CONTROLLER_GEN) ## Download controller-gen locally if necessary. If wrong version is installed, it will be overwritten. $(CONTROLLER_GEN): $(LOCALBIN) - $(call go-install-tool-branch,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION)) + $(call go-install-tool-versioned,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION),$(CONTROLLER_TOOLS_VERSION)) @if [ -L "$(LOCALBIN)/controller-gen" ]; then \ unlink "$(LOCALBIN)/controller-gen"; \ fi From af5468b0883c730044aa99a6d913a6b0338bda2c Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Wed, 12 Aug 2026 00:17:06 -0400 Subject: [PATCH 2/7] Fix envtest arch-check to test exit code 126 specifically This branch's check-envtest-arch (landed via #2100, Feb 2026) has been unconditionally reinstalling setup-envtest on every single `make test`/`make envtest` invocation since it merged: it treats any `$(ENVTEST) --help` failure as "wrong architecture," but setup-envtest's own --help exits 2 by its own convention even on a perfectly working binary. Since check-envtest-arch is itself .PHONY and a prerequisite of $(ENVTEST), that always-nonzero exit forces a delete+reinstall on every invocation, unconditionally, defeating the point of caching this binary at all. Verified by cross-compiling a real linux/amd64 setup-envtest and running it on this darwin/arm64 host: the shell reports exit code 126 specifically (POSIX "found but cannot execute" / exec format error) for a genuinely incompatible binary, distinct from the tool's own exit codes. Check that instead. Co-authored-by: Claude Signed-off-by: Tiger Kaovilai --- Makefile | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 4a21a267898..d935a6aba61 100644 --- a/Makefile +++ b/Makefile @@ -304,11 +304,17 @@ ENVTESTPATH = $(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path) ifeq ($(shell $(ENVTEST) list | grep $(ENVTEST_K8S_VERSION)),) ENVTESTPATH = $(shell $(ENVTEST) --arch=amd64 use $(ENVTEST_K8S_VERSION) -p path) endif +# Checked via exit code 126 specifically (POSIX "found but cannot execute" / +# exec format error) rather than any nonzero exit, since setup-envtest's own +# --help exits 2 by its own convention on a perfectly working binary. .PHONY: check-envtest-arch check-envtest-arch: - @if [ -f $(ENVTEST) ] && ! $(ENVTEST) --help >/dev/null 2>&1; then \ - echo "$(ENVTEST) is not executable on this platform, removing and re-downloading"; \ - rm -f $(ENVTEST); \ + @if [ -f $(ENVTEST) ]; then \ + $(ENVTEST) --help >/dev/null 2>&1; \ + if [ $$? -eq 126 ]; then \ + echo "$(ENVTEST) is not executable on this platform, removing and re-downloading"; \ + rm -f $(ENVTEST); \ + fi; \ fi $(ENVTEST): check-envtest-arch ## Download envtest-setup locally if necessary. From ef5af152ac289836e24c57d3f74493ef174cea93 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Wed, 12 Aug 2026 09:55:05 -0400 Subject: [PATCH 3/7] Extend arch-check to controller-gen/kustomize/golangci-lint too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick of the same extension from oadp-dev. The version-marker check alone isn't enough: a binary can have a correct .version marker but still be the wrong architecture, e.g. if a containerized build with a different GOARCH bind-mounts the host's bin/ directory (this Makefile documents exactly that workflow for `make test`: `docker run --platform linux/amd64 -v $PWD:$PWD ...`). Anything that does `go install` in there writes onto the host's real bin/ tree since it's the same mounted path, not a copy. Not envtest-specific — it can happen to any of these four cached tool binaries. go-install-tool-versioned now also probes `$(1) --version` and checks specifically for exit code 126 (POSIX "found but cannot execute" / exec format error), same technique as the envtest fix already on this branch. Verified controller-gen and kustomize (this branch's v4.5.5 pin) correctly detect and repair a wrong-arch binary even when its .version marker already matches the pinned version: cross-compiled a real linux/amd64 binary, copied it over the working native binary, confirmed each was detected and replaced. golangci-lint shares the identical code path but couldn't be exercised directly on this branch (its v1.54.2 pin fails to build against this sandbox's Go 1.26 toolchain, a pre-existing environment issue unrelated to this change). Co-authored-by: Claude Signed-off-by: Tiger Kaovilai --- Makefile | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index d935a6aba61..a8c86e78506 100644 --- a/Makefile +++ b/Makefile @@ -712,15 +712,25 @@ rm -rf $$TMP_DIR ;\ } endef -# go-install-tool-versioned installs $2 to branch-specific path $1, but only if $1 is missing -# or $1.version doesn't match the pinned version $3. Uses a sidecar marker file instead of -# introspecting the binary's own --version output, because that output is unreliable for some -# tools when installed via `go install` (e.g. kustomize reports "(devel)" or an unexpanded -# `$$Format:%H$$` placeholder instead of its real version, depending on build-time factors). +# go-install-tool-versioned installs $2 to branch-specific path $1, but only if $1 is missing, +# $1.version doesn't match the pinned version $3, or $1 exists but cannot execute on this +# platform (checked via exit code 126, POSIX "found but cannot execute" / exec format error — +# this can happen when a containerized build with a different GOARCH bind-mounts the host's +# bin/ directory, e.g. `docker run --platform linux/amd64 -v $$PWD:$$PWD ... make manifests`). +# Uses a sidecar marker file for the version check instead of introspecting the binary's own +# --version output, because that output is unreliable for some tools when installed via +# `go install` (e.g. kustomize reports "(devel)" or an unexpanded `$$Format:%H$$` placeholder +# instead of its real version, depending on build-time factors). define go-install-tool-versioned -@if [ -f $(1) ] && [ -f $(1).version ] && [ "$$(cat $(1).version)" = "$(3)" ]; then \ +@ARCH_OK=1 ;\ +if [ -f $(1) ]; then \ + $(1) --version >/dev/null 2>&1 ;\ + if [ $$? -eq 126 ]; then ARCH_OK=0 ; fi ;\ +fi ;\ +if [ "$$ARCH_OK" = "1" ] && [ -f $(1) ] && [ -f $(1).version ] && [ "$$(cat $(1).version)" = "$(3)" ]; then \ echo "$(notdir $(1)) $(3) is already installed" ;\ else \ + if [ "$$ARCH_OK" = "0" ]; then echo "$(notdir $(1)) exists but cannot execute on this platform, removing and re-downloading" ; fi ;\ set -e ;\ mkdir -p $(dir $(1)) ;\ rm -f $(1) $(1).version ;\ From 0c97089c9b99fc433a85a002691d2496b88fe63f Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Wed, 12 Aug 2026 10:37:16 -0400 Subject: [PATCH 4/7] Fix a real crash: exit-126 arch check aborts under -e in modern Make MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick of the same fix from oadp-dev. An independent second-opinion review caught a severe bug in the previous commit: the exit-126 probe (`$(1) --version`, then a separate `if [ $? -eq 126 ]`) is a bare command outside any &&/||/if guard. Under `set -e` — which this Makefile's `.SHELLFLAGS = -ec` enables, honored by GNU Make 3.82+ — a bare failing command aborts the whole recipe immediately, before the exit-code check ever runs. My local macOS Make (3.81) silently ignores .SHELLFLAGS, so none of my testing could have caught this. Confirmed for real: installed GNU Make 4.4.1 via Homebrew and reproduced the crash directly on this branch too — `make kustomize` died with "Error 1" every time the binary already existed, because kustomize's own `--version` exits 1 by its own convention even on a perfectly healthy binary (this branch's kustomize v4.5.5 pin, same issue as v5.2.1 on oadp-dev). Replaced the whole exit-code-probe approach with `go version -m`, which reads a binary's embedded module version and GOOS/GOARCH directly from its build info, without executing it at all: - No execution means no exit-code heuristic to get wrong, and no -e hazard, for any of these four tools. - No execution also closes a gap the review surfaced: exit-126 detection cannot work at all inside a container with qemu-user-static/binfmt_misc registered (standard in multi-arch CI/build images), since a wrong-arch binary just runs under emulation and returns its own exit code instead of an exec-format-error. - Drops the sidecar `.version` marker file entirely — go version -m reads the real, authoritative module version already embedded in the binary. This branch's check-envtest-arch (a separate target here, unlike oadp-dev's inline check) is replaced outright by a plain call to the same go-install-tool-versioned macro used by the other three tools. Verified end-to-end with GNU Make 4.4.1: fresh install, idempotent re-run (previously the exact crash case for kustomize/envtest), and wrong-arch detection+repair, for controller-gen/kustomize/envtest. golangci-lint@v1.54.2 still can't be exercised directly on this branch (pre-existing build incompatibility with this sandbox's Go 1.26 toolchain, unrelated to this change — confirmed by testing whether a newer v1.x pin would help: it does fix the build, but its updated default linter set immediately surfaces ~20 pre-existing findings across unrelated files, which is real scope growth this PR shouldn't absorb; leaving the pin as-is). `make generate manifests bundle` zero diff, `make test` fully green. Co-authored-by: Claude Signed-off-by: Tiger Kaovilai --- Makefile | 73 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/Makefile b/Makefile index a8c86e78506..1131b315630 100644 --- a/Makefile +++ b/Makefile @@ -304,24 +304,17 @@ ENVTESTPATH = $(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path) ifeq ($(shell $(ENVTEST) list | grep $(ENVTEST_K8S_VERSION)),) ENVTESTPATH = $(shell $(ENVTEST) --arch=amd64 use $(ENVTEST_K8S_VERSION) -p path) endif -# Checked via exit code 126 specifically (POSIX "found but cannot execute" / -# exec format error) rather than any nonzero exit, since setup-envtest's own -# --help exits 2 by its own convention on a perfectly working binary. -.PHONY: check-envtest-arch -check-envtest-arch: - @if [ -f $(ENVTEST) ]; then \ - $(ENVTEST) --help >/dev/null 2>&1; \ - if [ $$? -eq 126 ]; then \ - echo "$(ENVTEST) is not executable on this platform, removing and re-downloading"; \ - rm -f $(ENVTEST); \ - fi; \ - fi - -$(ENVTEST): check-envtest-arch ## Download envtest-setup locally if necessary. - $(call go-install-tool,$(ENVTEST),sigs.k8s.io/controller-runtime/tools/setup-envtest@v0.0.0-20250308055145-5fe7bb3edc86) - -.PHONY: envtest -envtest: $(ENVTEST) +# Uses go-install-tool-versioned (see its doc comment above) instead of a bespoke +# check-envtest-arch target: an earlier version of this check ran `$(ENVTEST) --help` and +# treated any nonzero exit as "wrong architecture" — but setup-envtest's own --help exits 2 +# by its own convention even on a perfectly healthy binary, which under this Makefile's +# `.SHELLFLAGS = -ec` aborts the whole recipe (confirmed with real GNU Make 4.4.1) rather +# than being caught, so it was unconditionally reinstalling setup-envtest on every single +# invocation. +.PHONY: envtest $(ENVTEST) +envtest: $(ENVTEST) ## Download envtest-setup locally if necessary. +$(ENVTEST): $(LOCALBIN) + $(call go-install-tool-versioned,$(ENVTEST),sigs.k8s.io/controller-runtime/tools/setup-envtest@v0.0.0-20250308055145-5fe7bb3edc86,v0.0.0-20250308055145-5fe7bb3edc86) # If test results in prow are different, it is because the environment used. # You can simulate their env by running @@ -713,27 +706,38 @@ rm -rf $$TMP_DIR ;\ endef # go-install-tool-versioned installs $2 to branch-specific path $1, but only if $1 is missing, -# $1.version doesn't match the pinned version $3, or $1 exists but cannot execute on this -# platform (checked via exit code 126, POSIX "found but cannot execute" / exec format error — -# this can happen when a containerized build with a different GOARCH bind-mounts the host's -# bin/ directory, e.g. `docker run --platform linux/amd64 -v $$PWD:$$PWD ... make manifests`). -# Uses a sidecar marker file for the version check instead of introspecting the binary's own -# --version output, because that output is unreliable for some tools when installed via -# `go install` (e.g. kustomize reports "(devel)" or an unexpanded `$$Format:%H$$` placeholder -# instead of its real version, depending on build-time factors). +# doesn't have the pinned module version $3 embedded in it, or was built for a different +# GOOS/GOARCH than this host. Uses `go version -m` to read the binary's embedded build info +# directly (module version, GOOS, GOARCH) instead of executing it or trusting a sidecar marker +# file — this avoids two real failure modes found in earlier attempts at this check: +# - Introspecting the binary's own --version/--help output is unreliable: some tools report +# a version string that depends on ldflags their own release process sets, which `go +# install` doesn't set (e.g. kustomize reporting "(devel)" or an unexpanded `$$Format:%H$$` +# placeholder), and some tools exit nonzero on --version even when perfectly healthy (e.g. +# kustomize exits 1, setup-envtest's --help exits 2) — so a naive "nonzero exit means +# broken" check is wrong, and worse, a bare failing probe command under `set -e`/`-o +# pipefail` (this Makefile's .SHELLFLAGS, honored by GNU Make 3.82+ — silently ignored by +# the ancient Make 3.81 macOS ships, which is why this went unnoticed locally) aborts the +# entire recipe rather than being caught. Verified directly against real GNU Make 4.4.1: +# the previous exit-code-126 version of this macro reliably crashed `make kustomize` +# with "Error 1" every time the binary already existed. +# - Actually executing the binary to test compatibility (this macro's own earlier approach, +# and the same technique in #2152) can't detect a wrong-arch binary at all in a container +# with qemu-user-static/binfmt_misc registered (common in multi-arch CI/build images): +# the foreign-arch binary runs under emulation and returns its own exit code, not an +# exec-format-error, defeating the whole check silently in exactly the environments where +# it matters most. +# `go version -m` reads the embedded build info without ever executing the binary, sidestepping +# both problems at once. define go-install-tool-versioned -@ARCH_OK=1 ;\ -if [ -f $(1) ]; then \ - $(1) --version >/dev/null 2>&1 ;\ - if [ $$? -eq 126 ]; then ARCH_OK=0 ; fi ;\ -fi ;\ -if [ "$$ARCH_OK" = "1" ] && [ -f $(1) ] && [ -f $(1).version ] && [ "$$(cat $(1).version)" = "$(3)" ]; then \ +@BUILDINFO="$$(go version -m $(1) 2>/dev/null)" || BUILDINFO="" ;\ +MOD_VERSION="$$(printf '%s\n' "$$BUILDINFO" | awk '$$1=="mod"{print $$3; exit}')" ;\ +if [ -n "$$BUILDINFO" ] && [ "$$MOD_VERSION" = "$(3)" ] && printf '%s\n' "$$BUILDINFO" | grep -qF "GOOS=$$(go env GOOS)" && printf '%s\n' "$$BUILDINFO" | grep -qF "GOARCH=$$(go env GOARCH)"; then \ echo "$(notdir $(1)) $(3) is already installed" ;\ else \ - if [ "$$ARCH_OK" = "0" ]; then echo "$(notdir $(1)) exists but cannot execute on this platform, removing and re-downloading" ; fi ;\ set -e ;\ mkdir -p $(dir $(1)) ;\ - rm -f $(1) $(1).version ;\ + rm -f $(1) ;\ TMP_DIR=$$(mktemp -d) ;\ cd $$TMP_DIR ;\ go mod init tmp ;\ @@ -741,7 +745,6 @@ else \ GOBIN=$(dir $(1)) go install -mod=mod $(2) ;\ cd - >/dev/null ;\ rm -rf $$TMP_DIR ;\ - echo "$(3)" > $(1).version ;\ fi endef From 999e0fba8c84f1a8a8fa4e189844085c886e1493 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Wed, 12 Aug 2026 11:09:45 -0400 Subject: [PATCH 5/7] Fix ENVTESTPATH arch decided at Makefile-parse time on cold bin/ ifeq evaluates its $(shell ...) condition at parse time, before any target's prerequisites run. On a cold bin/ (setup-envtest not yet installed), `$(ENVTEST) list` fails silently, the grep finds nothing, and ENVTESTPATH gets permanently redefined to force --arch=amd64 -- regardless of host arch -- even though setup-envtest is correctly installed for the host's native arch by the time the `test` recipe actually runs. Move the fallback entirely into the shell expression itself (still a recursively-expanded variable, so it's only evaluated when referenced in the `test` recipe, after `envtest` has installed the right arch). Fixes #2377 Signed-off-by: Tiger Kaovilai --- Makefile | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 1131b315630..44bbb01c384 100644 --- a/Makefile +++ b/Makefile @@ -300,17 +300,14 @@ vet: check-go ## Run go vet against code. go vet -mod=mod ./... ENVTEST := $(shell pwd)/bin/setup-envtest -ENVTESTPATH = $(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path) -ifeq ($(shell $(ENVTEST) list | grep $(ENVTEST_K8S_VERSION)),) - ENVTESTPATH = $(shell $(ENVTEST) --arch=amd64 use $(ENVTEST_K8S_VERSION) -p path) -endif -# Uses go-install-tool-versioned (see its doc comment above) instead of a bespoke -# check-envtest-arch target: an earlier version of this check ran `$(ENVTEST) --help` and -# treated any nonzero exit as "wrong architecture" — but setup-envtest's own --help exits 2 -# by its own convention even on a perfectly healthy binary, which under this Makefile's -# `.SHELLFLAGS = -ec` aborts the whole recipe (confirmed with real GNU Make 4.4.1) rather -# than being caught, so it was unconditionally reinstalling setup-envtest on every single -# invocation. +# Native-arch resolution first, falling back to amd64 only if that fails (e.g. no +# native-arch envtest assets published for this k8s version). Done as a single shell +# expression (not a make ifeq) so it's decided when ENVTESTPATH is actually referenced +# in a recipe, after $(ENVTEST) is guaranteed installed for the host's real arch -- +# not at Makefile-parse time against a possibly-cold bin/ (see issue #2377). +ENVTESTPATH = $(shell out=$$($(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path 2>/dev/null); if [ -z "$$out" ]; then out=$$($(ENVTEST) --arch=amd64 use $(ENVTEST_K8S_VERSION) -p path 2>/dev/null); fi; echo "$$out") +# Uses go-install-tool-versioned (see its doc comment below) for both the version and +# architecture check, rather than a bespoke arch-only check here. .PHONY: envtest $(ENVTEST) envtest: $(ENVTEST) ## Download envtest-setup locally if necessary. $(ENVTEST): $(LOCALBIN) From ceed7ca78bfc99a2298bde274d1afbf3ae7de7f9 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Wed, 12 Aug 2026 11:15:51 -0400 Subject: [PATCH 6/7] Revert "Fix ENVTESTPATH arch decided at Makefile-parse time on cold bin/" This reverts commit 999e0fba8c84f1a8a8fa4e189844085c886e1493. Signed-off-by: Tiger Kaovilai --- Makefile | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 44bbb01c384..1131b315630 100644 --- a/Makefile +++ b/Makefile @@ -300,14 +300,17 @@ vet: check-go ## Run go vet against code. go vet -mod=mod ./... ENVTEST := $(shell pwd)/bin/setup-envtest -# Native-arch resolution first, falling back to amd64 only if that fails (e.g. no -# native-arch envtest assets published for this k8s version). Done as a single shell -# expression (not a make ifeq) so it's decided when ENVTESTPATH is actually referenced -# in a recipe, after $(ENVTEST) is guaranteed installed for the host's real arch -- -# not at Makefile-parse time against a possibly-cold bin/ (see issue #2377). -ENVTESTPATH = $(shell out=$$($(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path 2>/dev/null); if [ -z "$$out" ]; then out=$$($(ENVTEST) --arch=amd64 use $(ENVTEST_K8S_VERSION) -p path 2>/dev/null); fi; echo "$$out") -# Uses go-install-tool-versioned (see its doc comment below) for both the version and -# architecture check, rather than a bespoke arch-only check here. +ENVTESTPATH = $(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path) +ifeq ($(shell $(ENVTEST) list | grep $(ENVTEST_K8S_VERSION)),) + ENVTESTPATH = $(shell $(ENVTEST) --arch=amd64 use $(ENVTEST_K8S_VERSION) -p path) +endif +# Uses go-install-tool-versioned (see its doc comment above) instead of a bespoke +# check-envtest-arch target: an earlier version of this check ran `$(ENVTEST) --help` and +# treated any nonzero exit as "wrong architecture" — but setup-envtest's own --help exits 2 +# by its own convention even on a perfectly healthy binary, which under this Makefile's +# `.SHELLFLAGS = -ec` aborts the whole recipe (confirmed with real GNU Make 4.4.1) rather +# than being caught, so it was unconditionally reinstalling setup-envtest on every single +# invocation. .PHONY: envtest $(ENVTEST) envtest: $(ENVTEST) ## Download envtest-setup locally if necessary. $(ENVTEST): $(LOCALBIN) From d3ca37d75c0da71081c566c7a17b79bd07786a11 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Wed, 12 Aug 2026 11:57:47 -0400 Subject: [PATCH 7/7] Rewrite comments to describe current design, not review history Both go-install-tool-versioned's doc comment and the envtest section's note narrated the debugging process that led here (earlier attempts, "confirmed with real GNU Make 4.4.1", "the previous exit-code-126 version... reliably crashed", a specific past PR reference) rather than just stating why the current code is shaped this way. Keeps the durable WHY (unreliable tool self-report, the set -e hazard as a standing fact about this Makefile, the qemu/binfmt_misc blind spot) as present-tense design rationale instead. Co-authored-by: Claude Signed-off-by: Tiger Kaovilai --- Makefile | 48 +++++++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/Makefile b/Makefile index 1131b315630..4fab266d98e 100644 --- a/Makefile +++ b/Makefile @@ -304,13 +304,8 @@ ENVTESTPATH = $(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path) ifeq ($(shell $(ENVTEST) list | grep $(ENVTEST_K8S_VERSION)),) ENVTESTPATH = $(shell $(ENVTEST) --arch=amd64 use $(ENVTEST_K8S_VERSION) -p path) endif -# Uses go-install-tool-versioned (see its doc comment above) instead of a bespoke -# check-envtest-arch target: an earlier version of this check ran `$(ENVTEST) --help` and -# treated any nonzero exit as "wrong architecture" — but setup-envtest's own --help exits 2 -# by its own convention even on a perfectly healthy binary, which under this Makefile's -# `.SHELLFLAGS = -ec` aborts the whole recipe (confirmed with real GNU Make 4.4.1) rather -# than being caught, so it was unconditionally reinstalling setup-envtest on every single -# invocation. +# Uses go-install-tool-versioned (see its doc comment above) for both the version and +# architecture check, rather than a bespoke arch-only check here. .PHONY: envtest $(ENVTEST) envtest: $(ENVTEST) ## Download envtest-setup locally if necessary. $(ENVTEST): $(LOCALBIN) @@ -707,28 +702,23 @@ endef # go-install-tool-versioned installs $2 to branch-specific path $1, but only if $1 is missing, # doesn't have the pinned module version $3 embedded in it, or was built for a different -# GOOS/GOARCH than this host. Uses `go version -m` to read the binary's embedded build info -# directly (module version, GOOS, GOARCH) instead of executing it or trusting a sidecar marker -# file — this avoids two real failure modes found in earlier attempts at this check: -# - Introspecting the binary's own --version/--help output is unreliable: some tools report -# a version string that depends on ldflags their own release process sets, which `go -# install` doesn't set (e.g. kustomize reporting "(devel)" or an unexpanded `$$Format:%H$$` -# placeholder), and some tools exit nonzero on --version even when perfectly healthy (e.g. -# kustomize exits 1, setup-envtest's --help exits 2) — so a naive "nonzero exit means -# broken" check is wrong, and worse, a bare failing probe command under `set -e`/`-o -# pipefail` (this Makefile's .SHELLFLAGS, honored by GNU Make 3.82+ — silently ignored by -# the ancient Make 3.81 macOS ships, which is why this went unnoticed locally) aborts the -# entire recipe rather than being caught. Verified directly against real GNU Make 4.4.1: -# the previous exit-code-126 version of this macro reliably crashed `make kustomize` -# with "Error 1" every time the binary already existed. -# - Actually executing the binary to test compatibility (this macro's own earlier approach, -# and the same technique in #2152) can't detect a wrong-arch binary at all in a container -# with qemu-user-static/binfmt_misc registered (common in multi-arch CI/build images): -# the foreign-arch binary runs under emulation and returns its own exit code, not an -# exec-format-error, defeating the whole check silently in exactly the environments where -# it matters most. -# `go version -m` reads the embedded build info without ever executing the binary, sidestepping -# both problems at once. +# GOOS/GOARCH than this host. Uses `go version -m` to read a binary's embedded build info +# (module version, GOOS, GOARCH) instead of executing it or trusting a sidecar marker file: +# - A binary's own --version/--help output is not a reliable version or health signal. It +# can depend on ldflags a tool's own release process sets, which `go install` doesn't set +# (e.g. kustomize can report "(devel)" or an unexpanded `$$Format:%H$$` placeholder instead +# of its real version), and some tools exit nonzero on --version even when perfectly +# healthy (kustomize exits 1, setup-envtest's --help exits 2) — so "nonzero exit means +# broken" is the wrong signal. It's also dangerous under this Makefile's +# `.SHELLFLAGS = -ec` (enables `set -e`, honored by GNU Make 3.82+ but silently ignored by +# the Make 3.81 macOS ships): a bare probe command that exits nonzero aborts the whole +# recipe unless it's wrapped in an `if`/`||` guard. +# - Executing the binary to test compatibility can't detect a wrong-arch binary at all +# inside a container with qemu-user-static/binfmt_misc registered (common in multi-arch +# CI/build images): the foreign-arch binary runs under emulation and returns its own exit +# code rather than an exec-format-error. +# Reading embedded build info sidesteps both: no execution means no exit-code heuristic to +# get wrong and no qemu blind spot. define go-install-tool-versioned @BUILDINFO="$$(go version -m $(1) 2>/dev/null)" || BUILDINFO="" ;\ MOD_VERSION="$$(printf '%s\n' "$$BUILDINFO" | awk '$$1=="mod"{print $$3; exit}')" ;\