Skip to content

packaging(ns8): fix buildah subcommand typo that broke manifest assembly - #138

Merged
shockalotti merged 1 commit into
mainfrom
packaging/ns8-manifest-guard
Oct 14, 2026
Merged

shockalotti merged 1 commit into
mainfrom
packaging/ns8-manifest-guard

Conversation

@shockalotti

Copy link
Copy Markdown
Contributor

Fixes the first main-branch publish after #137.

What happened. The run failed at 'Assemble and push the multi-arch manifest'. The per-arch half was fine: :build-amd64 and :build-arm64 are both in the registry. The failure was the preflight check in push-manifest.sh:

if ! buildah image exists ...

There is no singular image subcommand. buildah printed Error: unknown command "image" for "buildah", the script exited 1, and no manifest was assembled. Correct form is buildah images -q, which prints the image ID when present and nothing when absent.

Why PR CI did not catch it. PR builds push nothing, so the manifest step is skipped and a green PR run says nothing about the publish path. This only surfaced on a real main-branch publish.

Guard added. A validate step now asserts every buildah <subcommand> used by the build scripts is real, using the buildah already present on the runner. Verified locally that the extracted list is exactly: add, commit, config, containers, from, images, manifest, push, run. This class of typo now fails in seconds on a PR instead of minutes into a release.

After merge, the main-branch run republishes :edge; that run is the end-to-end proof and :edge should then resolve to an OCI image index with amd64 and arm64.

The first main-branch publish after #137 failed at 'Assemble and push the
multi-arch manifest'. The per-arch builds and pushes both succeeded (the
:build-amd64 and :build-arm64 tags are in the registry); the failure was the
preflight check in push-manifest.sh using 'buildah image exists'. There is
no singular image subcommand, so buildah errored out and the script bailed
before assembling anything.

Use 'buildah images -q', which prints the image ID when the tag is present
and nothing when it is absent.

This was only ever caught because the publish path was verified end to end:
PR builds push nothing, so the manifest step is skipped there and a green PR
run proves nothing about it.

Also add a validate step that asserts every buildah subcommand the build
scripts invoke actually exists, so this class of typo fails in seconds on a
PR instead of minutes into a release publish.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

The preflight fix is correct and verified, but the manifest loop still fails: push-manifest.sh passes a third positional argument to buildah manifest add, which buildah rejects. Now that the preflight no longer aborts early, execution reaches that line and set -e kills the script — so this PR does not yet get the publish working. This is the same second root cause flagged in the #137 review; it remains unaddressed.

Reviewed changes

  • Preflight fix — packaging/ns8/push-manifest.sh:39-41 replaces the nonexistent buildah image exists <ref> with if [ -z "$(buildah images -q <ref>)" ]. Confirmed correct on buildah 1.33.7: a missing ref yields empty stdout and -z is true; a present ref prints its ID, and set -e does not abort from the failing substitution inside the test.
  • Subcommand guard — .github/workflows/ns8-module.yml:40-49 extracts buildah <sub> from both build scripts and asserts each is real via buildah <sub> --help. Extraction yields exactly add commit config containers from images manifest push run, and all nine pass on the installed buildah.

🚨 buildah manifest add still passes a rejected third argument

packaging/ns8/push-manifest.sh:54-57 calls buildah manifest add "${manifest}" "${repobase}/${reponame}:build-${arch}" "docker.io/calnode/ns8-calnode:${imagetag}-${arch}". buildah manifest add takes exactly two positional arguments (<list> <image>); the third makes it exit non-zero (At least two arguments are necessary: list and image to add to list, verified on the runner's buildah 1.33.7), and because push-manifest.sh runs under set -e the whole assembly aborts. The per-arch image config already records the architecture (stamped by buildah from --arch + buildah commit), so the platform is correct without it — either drop the third argument, or use --arch/--os if an explicit override is ever wanted.

Technical details
# `buildah manifest add` rejects the third positional argument

## Affected sites
- packaging/ns8/push-manifest.sh:54-57 — `buildah manifest add` invoked with `<list> <image> <docker-ref>`; the extra arg aborts the script before any manifest is produced.

## Evidence
- `buildah manifest add list img1 img2` → exit 125, `Error: At least two arguments are necessary: list and image to add to list` (buildah 1.33.7, the runner's version; the check is still present in buildah `main`).
- `cmd/buildah/manifest.go` `manifestAddCmd`: non-`--artifact` path returns `"too many arguments: expected list and image add to list"` when `len(args) > 2`.
- Prior pullfrog review on #137 raised this same issue alongside the `buildah image exists` typo; only the latter was fixed here.

## Required outcome
- `manifest add` receives exactly the list and the image; the platform is taken from the image config.

ℹ️ Stale-manifest cleanup is silently skipped

packaging/ns8/push-manifest.sh:51 runs buildah manifest rm --all "${manifest}" 2>/dev/null || true, but manifest rm defines no --all flag (Error: unknown flag: --all, exit 125 — verified), so the || true swallows the failure and the removal never happens. It is harmless on a fresh CI runner, where no local manifest exists, but on a re-run locally (or any host where the tag already exists) the following buildah manifest create errors on the duplicate name because it is not passed --amend/--replace. buildah manifest rm "${manifest}" already removes the whole list, so dropping --all is sufficient.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash | 𝕏

@shockalotti
shockalotti merged commit 24f1cc8 into main Oct 14, 2026
8 of 9 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant