experiment: vendor setup-envtest (mirror of #516)#518
Closed
frobware wants to merge 4 commits into
Closed
Conversation
Add `tools/tools.go` with a blank import of `setup-envtest` so the tool is pulled into the vendor directory. Uses the `release-0.22` branch pseudo-version which requires Go 1.24 (compatible with the project's Go 1.25). Replace `go install` invocation of `setup-envtest` in the Makefile with `go run`, removing the `envtest` target and `ENVTEST` variable. Co-Authored-By: Claude Signed-off-by: Andrey Lebedev <alebedev@redhat.com>
The previous recipe used `KUBEBUILDER_ASSETS="$(shell go run sigs.k8s.io/...setup-envtest ...)"` to compute the assets path. GNU Make's `$(shell ...)` captures stdout only and silently discards the exit code, so when `go run` failed (for example with `cannot find module providing package ... import lookup disabled by -mod=vendor`) the variable expanded to the empty string and tests proceeded with `KUBEBUILDER_ASSETS=""`. Tests that do not currently consume the assets path then passed regardless, masking the real failure. Move the substitution into the recipe shell. After `go run`, assert that `KUBEBUILDER_ASSETS` is non-empty and exit with a clear message otherwise. Echo the resolved invocation before running it so the build log records the exact `go test` command, mirroring the readability of the previous one-liner. Signed-off-by: Andrew McDermott <amcdermo@redhat.com>
The "Check clean vendors" step previously ran `go mod vendor` and discarded the result, so a pull request could leave `vendor/`, `go.mod` or `go.sum` out of sync with the module graph and CI would still pass. In particular, when a contributor runs `go mod vendor` locally but forgets to `git add` newly created vendor directories, the resulting tree builds against the module proxy yet fails when later consumed with `-mod=vendor`. Run `go mod tidy` and then `go mod vendor`, and assert with `git status --porcelain` that no tracked changes and no untracked entries remain under `vendor/`, `go.mod` or `go.sum`. This catches two classes of drift: missing or stale `vendor/` source (forgot to commit a newly vendored directory), and `go.mod` or `go.sum` divergence from the actual import graph (forgot to tidy after manually editing requires). `git diff --exit-code` alone is insufficient because it ignores untracked files, which is precisely the failure mode we need to catch. On failure, print the offending paths and the diff to make the cause obvious in the workflow log. Co-authored-by: Andrey Lebedev <alebedev87@gmail.com> Signed-off-by: Andrew McDermott <amcdermo@redhat.com>
The earlier commit `Add setup-envtest to tools/tools.go for vendoring` updated `go.mod`, `go.sum` and `vendor/modules.txt` but did not `git add` the three top-level directories that `go mod vendor` materialises: vendor/github.com/spf13/afero/ vendor/golang.org/x/text/runes/ vendor/sigs.k8s.io/controller-runtime/tools/ `vendor/modules.txt` referenced packages whose source was absent on disk, so any build with `-mod=vendor` (the default when a `vendor` directory exists) reported `cannot find module providing package`. That failure was previously masked twice: first by `$(shell ...)` in the Makefile silently swallowing the non-zero exit, and second by the "Check clean vendors" CI step never asserting a clean tree after `go mod vendor`. Both layers have been removed in earlier commits on this branch; this commit removes the underlying breakage by committing the missing source. After this commit, `go run sigs.k8s.io/controller-runtime/tools/setup-envtest` resolves under `-mod=vendor`, the Test job runs to completion with a populated `KUBEBUILDER_ASSETS`, and the Verify job's vendor assertion passes because `go mod vendor` is a no-op against the committed tree. Signed-off-by: Andrew McDermott <amcdermo@redhat.com>
frobware
force-pushed
the
experiment/setup-envtest-vendoring
branch
from
abb1d9a to
3b982e6
Compare
Contributor
Author
|
Changes being pulled back into #516 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft. Not for merge. Stepwise demonstration that PR #516's masked-pass on CI is the result of three independent layers of masking, any one of which would have caught the broken vendor tree on its own.
CI logs on github.com expire (default ~90 days), so each stage's evidence is mirrored inline below.
Summary
c99d9daa04c7abc7$(shell ...)masking in Makefiled8088215abb1d9a3git add vendor/for the three missing dirsStage A -- cherry-pick of #516 verbatim (broken vendor)
c99d9daa(alebedev87, identical to PR Add setup-envtest to tools.go for vendoring #516 headab24401c)Smoking gun -- Test job log
Job: https://github.com/bpfman/bpfman-operator/actions/runs/25372315984/job/74398941940 (conclusion: success)
The recipe
KUBEBUILDER_ASSETS="$(shell go run sigs.k8s.io/...setup-envtest ...)"invokes setup-envtest, which fails because three top-level vendor directories were nevergit added by the original PR (afero, x/text/runes, controller-runtime/tools). GNU Make's$(shell ...)captures stdout and silently discards the exit code, so the variable expands to the empty string. Tests then run with no envtest assets and pass anyway, because no test in the suite consumesKUBEBUILDER_ASSETS.Verify job -- no-op vendor check
Job: https://github.com/bpfman/bpfman-operator/actions/runs/25372315984/job/74398941911 (conclusion: success)
The "Check clean vendors" step runs
go mod vendorand discards the result; nogit difforgit statusfollow-up. With the broken vendor tree,go mod vendormaterialises the missing directories at runtime, exits 0, and the step passes -- drift undetected.Three layers of masking
$(shell ...)swallows non-zero exits in MakeKUBEBUILDER_ASSETSStage B -- drop
$(shell ...)masking in Makefile04c7abc7failure)Smoking gun -- Test job log
Job: https://github.com/bpfman/bpfman-operator/actions/runs/25372937936/job/74400895857 (conclusion: failure)
Same
cannot find moduleerror as Stage A, but now the recipe propagates the failure: no silent fallback toKUBEBUILDER_ASSETS="", no spurious test pass. The Test job exits 2, the run conclusion isfailure, and the PR is correctly flagged.This proves layer 1 was masking a real, immediate runtime breakage. Layer 2 (the Verify job) is still passing -- it would not have caught this without Stage C.
Stage C -- assert vendor cleanliness in Verify job
d8088215(cherry-pick of PR ci: fail vendor check when vendor/, go.mod or go.sum drift #517's commita1ebb984)failure)Smoking gun -- Verify job log
Job: https://github.com/bpfman/bpfman-operator/actions/runs/25373577585/job/74403075256 (conclusion: failure)
The new check runs
go mod vendorand then asserts withgit status --porcelain vendor go.mod go.sumthat the working tree is unchanged. Because the broken tree was missing those three directories,go mod vendormaterialised them and the step exited 1, naming the offending paths in the workflow log. This is the cause of the breakage, not a downstream symptom -- and it is reported at the verify stage, before Test even runs.Test job log (still failing on the same Stage B reason)
Job: https://github.com/bpfman/bpfman-operator/actions/runs/25373577585/job/74403075196 (conclusion: failure)
Test continues to fail until Stage D commits the missing vendor source; Stage C only fixes detection, not the underlying breakage.
Stage D --
git add vendor/for the three missing dirsabb1d9a3Test job log -- envtest assets resolved
Job: https://github.com/bpfman/bpfman-operator/actions/runs/25374160435/job/74405070815 (conclusion: success)
KUBEBUILDER_ASSETSnow points at the real envtest binaries directory (Stage B's recipe asserts non-empty before runninggo test, and the assertion holds). Compare with Stage A and Stage C, where the same line readKUBEBUILDER_ASSETS="".Verify job -- vendor assertion passes
Job: https://github.com/bpfman/bpfman-operator/actions/runs/25374160435/job/74405070735 (conclusion: success)
The "Check clean vendors" step runs
go mod vendoragainst the now-complete tree.git status --porcelainreports no changes; noout of syncmessage is emitted; the step exits 0.Conclusions
KUBEBUILDER_ASSETS) is latent. The first test that actually needs envtest assets would have failed loudly on a Stage-A-shaped tree even with both upstream layers masking. None has been added yet.