Skip to content

packaging(ns8): fix manifest add arity, drop bad rm flag, and selftest the path - #139

Merged
shockalotti merged 2 commits into
mainfrom
packaging/ns8-manifest-selftest
Oct 14, 2026
Merged

shockalotti merged 2 commits into
mainfrom
packaging/ns8-manifest-selftest

Conversation

@shockalotti

Copy link
Copy Markdown
Contributor

Second failure of the main-branch publish after #138, both in push-manifest.sh.

1. buildah manifest add arity. It was passed three arguments (list, image, instance name). Buildah 5.x takes exactly two and failed with:

Error: too many arguments: expected list and image add to list

2. buildah manifest rm --all does not exist. manifest rm takes no flags, so the stale-manifest cleanup was silently doing nothing behind || true.

Both reproduced locally in a buildah container (buildah 5.39.3) rather than burning more release runs: manifest add <list> <image> yields an OCI image index containing amd64 and arm64, and manifest rm <name> works.

New invariant in the script. After assembly the script inspects the list and refuses to push unless it holds exactly the requested number of arches. An arm64 node handed an amd64-only image fails at podman run, so a partial manifest is worse than no publish at all. Verified locally that the guard fires.

New CI selftest. Runs the real push-manifest.sh against two scratch images and an oci: destination, so it needs no registry and no credentials. PR builds push nothing, which means the manifest step is skipped on PRs. That is the actual reason two broken versions of this file both reached main. Verified locally: the selftest passes, and the invariant fires when the list cannot match the request.

PUSH_DESTINATION is a new override defaulting to the docker:// target, added so the script can be driven without a registry.

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

…t the path

Second failure of the main-branch publish, both in push-manifest.sh:
- 'buildah manifest add' was passed three arguments (list, image, instance
  name). Buildah 5.x takes exactly two; it failed with 'too many arguments:
  expected list and image add to list'.
- 'buildah manifest rm --all' does not exist; rm takes no flags. The stale
  manifest cleanup was silently doing nothing behind '|| true'.

Reproduced both locally in a buildah container rather than burning release
runs: buildah 5.39.3, 'manifest add list image' yields an OCI image index
with amd64 and arm64, and 'manifest rm <name>' works.

Also add an invariant to the script itself: after assembly, inspect the list
and refuse to push unless it holds exactly the requested number of arches.
An arm64 node receiving an amd64-only image fails at podman run, so a partial
manifest is worse than no publish.

And add a CI selftest that runs the real push-manifest.sh against scratch
images and an oci: destination (no registry, no credentials). PR builds push
nothing, so the manifest step is skipped on PRs: that is why two broken
versions both reached main. Verified locally that the selftest passes and
that the new invariant fires.

PUSH_DESTINATION was added as an override so the script can be driven without
a registry; it defaults to the docker:// target.

@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

This PR restructures the validate job so that the job now fails on every run: the new selftest step was inserted mid-script and absorbs the two trailing checks, which then execute from the repo root instead of packaging/ns8. The exec-bit assertion aborts the step, so build is skipped and nothing republishes — including the :edge run this PR exists to unblock.

Reviewed changes

  • push-manifest.sh — manifest assembly fixed. Dropped the non-existent --all flag from buildah manifest rm, reduced manifest add to the two arguments buildah expects, added a PUSH_DESTINATION override (defaulting to docker://), and added an arch-count guard that refuses to push a partial list.
  • ns8-module.yml — selftest added. New Selftest the multi-arch manifest assembly step drives the real push-manifest.sh against scratch images and an oci: destination, so the manifest path actually runs on PRs.

The manifest logic itself checks out — I ran the selftest path against buildah in this environment and it produces a two-arch index at the oci: destination. The problem is only where the new step sits in the workflow.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash | 𝕏

Comment thread .github/workflows/ns8-module.yml
"${manifest}" \
"${repobase}/${reponame}:build-${arch}" \
"docker.io/calnode/ns8-calnode:${imagetag}-${arch}"
# Exactly two arguments: buildah 5.x takes only the list and the

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.

Nit: buildah releases are all 1.x (latest 1.45.2), so 5.x here reads as a transposition of the 1.39.x version quoted in the PR description. Worth correcting so the arity note points at a version that exists.

The previous commit's YAML edit spliced the new selftest step into the middle
of the existing validate step's script, so the placeholder and exec-bit checks
were orphaned into a step that runs from the repo root instead of
packaging/ns8. The selftest itself passed on the runner (Published
ocr-selftest/ns8-calnode:selftest (amd64 arm64)); the step then failed on the
orphaned 'grep imageroot/'.

Both sets of checks now sit in the step whose working-directory they need.

@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.

ℹ️ No new issues found. 52ae44b fixes the blocking workflow regression flagged in the previous review; no new findings.

Reviewed changes

  • Workflow checks restored to the correct step. The __CALNODE_APP_IMAGE__ grep and the exec-bit assertion moved back into the Validate action scripts and schemas step (which carries working-directory: packaging/ns8), and the manifest selftest is now its own step. validate no longer aborts on every run, so build is no longer skipped.
  • Selftest comment wording. "lived in this step" → "lived in that step", matching where the checks now live.

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

@shockalotti
shockalotti merged commit 3b2e655 into main Oct 14, 2026
7 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