diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index cbbd559..f70d5c9 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -506,6 +506,10 @@ jobs: # it by digest rather than by tag: a tag can move between the build and # the smoke, and then the check reports on bytes nobody built here. digest: ${{ steps.build.outputs.digest }} + # What the channel tag already names. This is what smoke-web checks on a + # reuse hit — deliberately NOT folded into `digest`, which the :c- + # repoint reads and which must keep meaning "what this run built" (#4290). + published_digest: ${{ steps.reuse.outputs.published_digest }} # A plain `needs` — no `always()`. That expression existed to let a # SKIPPED sign-extension through on a tag push while still blocking a # FAILED one. With no tag trigger, sign-extension always runs, so the @@ -808,6 +812,16 @@ jobs: PUBLISHED=$(docker buildx imagetools inspect "$IMAGE:$T" \ --format '{{ index .Image.Config.Labels "fc.revision" }}' \ 2>/dev/null || echo "") + + # The manifest the channel tag names RIGHT NOW. Exported so the + # smoke has something to check on a reuse hit, when this job builds + # nothing and emits no digest of its own (#4323). Resolved here + # because this step is already resolving the tag to read its label — + # one lookup, one answer, rather than a second one that could name a + # different manifest if anything moved the tag in between. + PUBLISHED_DIGEST=$(docker buildx imagetools inspect "$IMAGE:$T" \ + --format '{{ .Manifest.Digest }}' 2>/dev/null || echo "") + echo "published_digest=$PUBLISHED_DIGEST" >> "$GITHUB_OUTPUT" echo "reuse: $IMAGE:$T carries fc.revision=${PUBLISHED:-}; derived=$DERIVED" if [ -z "$PUBLISHED" ] && docker buildx imagetools inspect "$IMAGE:$T" >/dev/null 2>&1; then # The tag resolves but carries no readable label. Expected exactly @@ -1172,9 +1186,29 @@ jobs: # shape the refresh already has — per-channel candidate tags, promote # learning its channel, and the :c- repoint moving after the gate. # That is a redesign of the production publish path and is its own task. - if: >- - needs.build-web.outputs.candidate == 'true' - || needs.build-web.outputs.digest != '' + # + # NO `if:` — this job always runs (#4323). It used to be gated on the + # build having published something, which skipped it on a reuse hit. That + # sounds like an optimisation and is a hole: this workflow file is in no + # artifact's path set (correctly — editing it changes no shipped byte), so + # a commit that touches ONLY the smoke moves no revision, hits reuse, + # emits no digest, and skips the smoke. The one commit whose purpose is + # changing this check was the one commit that could not run it. + # + # That is not hypothetical twice over. 5ca1058 added the egress sandbox + # and went green three times with this job SKIPPED. 7175ace fixed the + # bug that hid behind those greens (#4319) and needed a manual + # force_build to exercise at all. Both relied on someone remembering. + # + # It is also where the other historical failure lived: on run 5290 this + # expression read `env`, which a job `if:` cannot see, so it evaluated + # empty and skipped silently. Two skips, one expression. The expression + # goes. + # + # The cost is one pull and boot on a reuse-hit push, re-smoking bytes + # that were smoked when they were built. That is the price of a harness + # that tests itself, and it overlaps ci.yml's lanes, so little wall-clock + # moves. Not running is not the same as passing. runs-on: python-ci container: image: git.fabledsword.com/bvandeusen/ci-python:3.14 @@ -1216,6 +1250,8 @@ jobs: TOKEN: ${{ secrets.RELEASE_TOKEN }} ACTOR: ${{ github.actor }} BUILT_DIGEST: ${{ needs.build-web.outputs.digest }} + PUBLISHED_DIGEST: ${{ needs.build-web.outputs.published_digest }} + IS_CANDIDATE: ${{ needs.build-web.outputs.candidate }} run: | set -eux # Service discovery mirrors ci.yml's integration lane: these jobs run @@ -1245,14 +1281,32 @@ jobs: fi echo "$TOKEN" | docker login git.fabledsword.com -u "$ACTOR" --password-stdin - # A refresh publishes to the candidate tag; a push writes the - # channel tag directly and hands us its digest. Address the digest - # where we have one — it names the exact manifest this run built, - # which a tag stops doing the moment anything else moves it. - if [ -n "${BUILT_DIGEST:-}" ]; then - CANDIDATE="$IMAGE@$BUILT_DIGEST" - else + # WHAT GETS SMOKED, in order of preference — always a DIGEST, never + # a tag: a tag can move between the build and this job, and then the + # check reports on bytes nobody here decided to ship (#4290). + # + # 1. what this run built, when it built; + # 2. what the channel tag already names, on a reuse hit — the + # digest the reuse step resolved while reading its label. This + # is the case that makes the job able to verify a change to + # ITSELF (#4323), since such a change rebuilds nothing. + # + # A refresh always lands in (1); the candidate tag stays only as the + # last resort for one, and says so rather than being a silent else. + DIGEST="${BUILT_DIGEST:-}" + [ -n "$DIGEST" ] || DIGEST="${PUBLISHED_DIGEST:-}" + if [ -n "$DIGEST" ]; then + CANDIDATE="$IMAGE@$DIGEST" + elif [ "${IS_CANDIDATE:-}" = "true" ]; then CANDIDATE="$IMAGE:refresh-candidate" + else + # Nothing built and nothing published. There is no artifact this + # job could honestly report on, so it fails rather than passing + # quietly — a guard with nothing to check is not a passing guard. + echo "smoke: FAILED — no image to smoke. build-web neither built" >&2 + echo "smoke: one nor resolved a published digest, so there is" >&2 + echo "smoke: nothing here to verify." >&2 + exit 1 fi docker pull "$CANDIDATE" @@ -1284,9 +1338,55 @@ jobs: # They keep their original network too — that is fine, since what # must be offline is the APP container, and it is created with only # this network. - NET=smoke-noegress-$$ + # Named for the RUN, not for `$$`. The shell's pid is deterministic + # in this runner — every execution of this step got 157 — so `$$` + # produced one shared name, and the second run died on "network with + # name smoke-noegress-157 already exists". A pid is unique among + # LIVE processes, which is not the same as unique over time, and in + # a fresh container it is neither. + # The date fallback matters: if the runner does not set these, + # a literal default would put every run back on one shared name + # — the bug, with different letters. + NET="smoke-noegress-${GITHUB_RUN_ID:-$(date +%s)}-${GITHUB_RUN_ATTEMPT:-0}" + + # Sweep anything an earlier run left behind. Needed because the + # cleanup below used to be destroyed before it could fire (see the + # trap note), so every execution leaked its network. `rm` on one + # still in use fails, and `|| true` keeps that harmless — so a + # concurrent run's network survives this. + docker network ls --filter name=^smoke-noegress- -q \ + | while read -r stale; do + docker network rm "$stale" >/dev/null 2>&1 || true + done + docker network create --internal "$NET" - trap 'docker network rm "$NET" >/dev/null 2>&1 || true' EXIT + + # ONE exit trap, for everything. `trap ... EXIT` REPLACES the + # previous handler rather than adding to it, so the network's own + # trap used to be silently discarded the moment the container's was + # installed further down — and the network was never removed. That + # is invisible in a passing run and only ever surfaces on the NEXT + # one, as a name collision. + # + # CID is empty until the app container exists, so this is safe to + # arm now and still covers a failure before that point. + CID="" + cleanup() { + rc=$? + if [ -n "$CID" ]; then + # The log ONLY on failure — a boot that never answered must fail + # with the reason visible rather than as a bare timeout (rule + # 156), while a green run has nothing to say. + [ $rc -eq 0 ] || docker logs "$CID" 2>&1 | tail -40 + docker rm -f "$CID" >/dev/null 2>&1 || true + fi + docker network rm "$NET" >/dev/null 2>&1 || true + # Preserve the real status, which a trap ending on a successful + # `docker rm` would otherwise mask. + exit $rc + } + trap cleanup EXIT + docker network connect "$NET" "$PG" docker network connect "$NET" "$RD" # Re-read the addresses ON THIS NETWORK. The IPs discovered above @@ -1344,13 +1444,11 @@ jobs: # siblings directly and a published port would collide with # whatever else the runner is hosting. echo "smoke: web boots and answers /api/health" + # Assigning CID is all that is needed — the single EXIT trap armed + # beside the network already covers the container, and it checks CID + # for emptiness precisely so it can be installed before this line. + # Installing a second trap here is what used to discard the first. CID=$(docker run -d $ENVOPTS "$CANDIDATE" web) - # Clean up the container however this ends, and dump its log ONLY - # on failure — a boot that never answers must fail with the reason - # visible rather than as a bare timeout (rule 156), while a green run - # has nothing to say. `exit $rc` preserves the real status, which a - # trap that ends on a successful `docker rm` would otherwise mask. - trap 'rc=$?; [ $rc -eq 0 ] || docker logs "$CID" 2>&1 | tail -40; docker rm -f "$CID" >/dev/null 2>&1 || true; exit $rc' EXIT WEB_IP=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' "$CID") test -n "$WEB_IP"