fix(images): one router's extras must not cancel the whole lab

CHANGELOG.md

@@ -41,6 +41,36 @@ one waits for a major.
   context` asks the registry and falls back to the rootfs tarball, which is the
   path the published 25.12.5 images were already built through.
 
+### Fixed
+
+- **One router's `extra_packages` no longer cancels every other router.**
+  `docker compose build` puts the whole lab in one buildkit solve, and buildkit
+  cancels the solve on the first target that fails — so the extras step, which
+  ran under `set -eu` with no guard, turned one unusable file into a failed
+  lab. Measured 2026-09-04 on a three-router stand: one `.ipk` with an
+  unsatisfiable dependency exited 255 and took `#16 CANCELED`, `#13 CANCELED`
+  and `owlab: build failed: exit status 1` with it. Both release lines did it —
+  it is `set -eu`, not the package manager: the same stand with a `.apk` that
+  apk answers `unable to select packages` exited 27 and cancelled its
+  neighbour. There is nothing to configure around it, either: neither `docker
+  compose build` nor `docker buildx bake` has a `--keep-going`.
+
+  The extras step now installs the set as before, and only if that fails
+  installs each file on its own, in the staged order, so the good ones still
+  land. The successful path is byte-for-byte the run it was.
+
+- **A package that did not install is now said out loud instead of being
+  fatal.** Tolerating a failure silently would be worse than the cancelled
+  builds it replaces, so the retry names each file it could not install, writes
+  the list to `/etc/owlab/extras-failed`, and `owlab up` reads it back off each
+  running router and prints it under the ready table:
+  `! owrt2410 is running WITHOUT luci-app-example_1.0_all.ipk`. `owlab up`
+  exits non-zero when it prints that — the lab is up, and it is not what the
+  config describes — and `owlab test` gains an `extra_packages` step that fails
+  the router before any assertion runs against a box missing what it was told
+  to have. Read as-built at any time with
+  `owlab exec <router> -- cat /etc/owlab/extras-failed`.
+
 ## [0.5.6] - 2026-09-04
 
 ### Fixed

cmd/owlab/commands.go

@@ -1,8 +1,10 @@
 package main
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -94,13 +96,90 @@ func (a *app) up(ctx context.Context, args []string) error {
 		}
 	}
 
-	if *noWait {
-		return a.printReady(routers, nil)
+	var ready map[string]bool
+	if !*noWait {
+		ready = a.waitForLuCI(ctx, routers)
 	}
-	ready := a.waitForLuCI(ctx, routers)
-	return a.printReady(routers, ready)
+	if err := a.printReady(routers, ready); err != nil {
+		return err
+	}
+	// After the table, because it is what should be left on screen — and
+	// after the build, because the build no longer stops for it.
+	return a.reportMissingExtras(ctx, containers)
+}
+
+// extrasFailedPath is where an image build records the extra_packages it could
+// not install. See the extras layer in images/Dockerfile.
+//
+// Read back off the router rather than scraped out of the build log, for two
+// reasons: the log is thousands of lines of buildkit progress that nobody
+// reads, and a router started from a cached image never printed one at all.
+const extrasFailedPath = "/etc/owlab/extras-failed"
+
+// missingExtras asks one router which of its extra_packages did not install.
+//
+// Empty for almost every router: the file exists only when a build had
+// something to record.
+func missingExtras(ctx context.Context, run syncpkg.Exec) []string {
+	var buf bytes.Buffer
+	// A missing file is the normal case, not an error to propagate — and any
+	// other failure here (a router that stopped, an engine that went away) is
+	// already reported by whatever else is talking to the same router.
+	if err := run(ctx, "cat "+extrasFailedPath+" 2>/dev/null || true", nil, &buf); err != nil {
+		return nil
+	}
+	var out []string
+	for _, line := range strings.Split(buf.String(), "\n") {
+		if line = strings.TrimSpace(line); line != "" {
+			out = append(out, line)
+		}
+	}
+	return out
+}
+
+// reportMissingExtras names, per router, the extra_packages its image was
+// built without.
+//
+// This is the half of issue #12 that is not "stop cancelling the other
+// routers". One bad file no longer fails the build, and tolerance with no
+// report is silence — which is worse than the failure it replaced: the
+// developer named a file, the router does not have it, and nothing but a line
+// in the middle of a buildkit log would say so. Hence the non-zero exit as
+// well: a script that runs `owlab up` before its own checks must not read
+// "lab is up" as "lab is what the config describes".
+func (a *app) reportMissingExtras(ctx context.Context, routers []*config.Router) error {
+	var bad []string
+	for _, r := range routers {
+		if len(r.Extra) == 0 {
+			continue
+		}
+		run, err := a.execFor(r)
+		if err != nil {
+			continue
+		}
+		missing := missingExtras(ctx, run)
+		if len(missing) == 0 {
+			continue
+		}
+		bad = append(bad, r.ID)
+		fmt.Fprintf(os.Stderr, "\n! %s is running WITHOUT %s\n", r.ID, strings.Join(missing, ", "))
+	}
+	if len(bad) == 0 {
+		return nil
+	}
+	fmt.Fprintf(os.Stderr,
+		"!   Every other router built and started; only these packages are missing.\n"+
+			"!   The package manager said why during the build — one router at a time shows it again:\n"+
+			"!     owlab up --rebuild %s\n", bad[0])
+	return errExtrasMissing
 }
 
+// errExtrasMissing is the exit status of an `owlab up` that started every
+// router and could not put a named package on one of them. A plain sentinel:
+// the routers and packages have already been printed one line each, and an
+// "owlab: extra_packages missing" summary on top would bury which is which.
+var errExtrasMissing = errors.New("extra_packages missing")
+
 // resolveBaseImages asks the registry whether each router's upstream image
 // exists, and switches the ones that do not onto the rootfs tarball.
 //

cmd/owlab/extras_test.go

@@ -0,0 +1,58 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"io"
+	"strings"
+	"testing"
+)
+
+// The record a build leaves behind is the only thing standing between "one bad
+// package no longer fails the build" and "one bad package is never mentioned
+// again" (issue #12). Reading it wrong is silent in exactly the direction that
+// matters: an empty list means "the router has everything it was told to have".
+func TestMissingExtrasReadsOneNamePerLine(t *testing.T) {
+	cases := []struct {
+		name string
+		out  string
+		want []string
+	}{
+		// The overwhelming majority: `cat` of a file that is not there, which
+		// the script turns into empty output rather than a failure.
+		{"nothing recorded", "", nil},
+		{"one package", "01-example_1.0_all.ipk\n", []string{"01-example_1.0_all.ipk"}},
+		{
+			"several, with the blank line a trailing newline leaves",
+			"example-daemon_1.0_all.ipk\nluci-app-example_1.0_all.ipk\n\n",
+			[]string{"example-daemon_1.0_all.ipk", "luci-app-example_1.0_all.ipk"},
+		},
+		// docker exec hands back \r\n from a container with a tty attached.
+		{"carriage returns", "example_1.0_all.ipk\r\n", []string{"example_1.0_all.ipk"}},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			run := func(_ context.Context, _ string, _ []byte, out io.Writer) error {
+				_, err := io.WriteString(out, tc.out)
+				return err
+			}
+			got := missingExtras(t.Context(), run)
+			if strings.Join(got, ",") != strings.Join(tc.want, ",") {
+				t.Errorf("missingExtras = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}
+
+// A router that cannot be reached is not a router that is missing a package.
+// Reporting one as the other would put "is running WITHOUT" against a box that
+// was simply stopped, and send the developer looking at the wrong thing.
+func TestMissingExtrasReportsNothingWhenTheRouterCannotAnswer(t *testing.T) {
+	run := func(_ context.Context, _ string, _ []byte, out io.Writer) error {
+		_, _ = io.WriteString(out, "Error: No such container\n")
+		return errors.New("exit status 1")
+	}
+	if got := missingExtras(t.Context(), run); got != nil {
+		t.Errorf("missingExtras = %q, want none", got)
+	}
+}

cmd/owlab/main.go

@@ -154,10 +154,11 @@ func main() {
 		if errors.Is(err, context.Canceled) {
 			os.Exit(130)
 		}
-		// A failed assertion has already been printed, one line per failure.
-		// Repeating it as "owlab: test failed" would bury the reason under a
-		// summary of it.
-		if errors.Is(err, errTestFailed) {
+		// A failed assertion, and a router started without a package its
+		// config named, have both already been printed one line each.
+		// Repeating either as "owlab: test failed" would bury the reason
+		// under a summary of it.
+		if errors.Is(err, errTestFailed) || errors.Is(err, errExtrasMissing) {
 			os.Exit(1)
 		}
 		fmt.Fprintln(os.Stderr, "owlab: "+err.Error())

cmd/owlab/test.go

@@ -200,6 +200,15 @@ func (a *app) test(ctx context.Context, args []string) error {
 			if err != nil {
 				record(checkpkg.Result{Check: "reach", Kind: "up", Detail: err.Error()})
 			} else {
+				// Before anything is installed or asserted. A bad
+				// extra_packages file no longer fails the image build
+				// (issue #12), so this step is the only thing left that can
+				// fail a run whose router is missing a package the config
+				// named — and asserting against such a router would report a
+				// pass for something that was never on it.
+				if len(r.Extra) > 0 && r.Fidelity != config.VM {
+					record(extrasStep(ctx, run, r))
+				}
 				if len(files) > 0 || len(installs) > 0 {
 					record(a.testInstall(ctx, r, run, files, installs, feedSpec))
 				}
@@ -255,6 +264,21 @@ func (a *app) test(ctx context.Context, args []string) error {
 // errTestFailed reports assertion failures without printing anything more.
 var errTestFailed = errors.New("test failed")
 
+// extrasStep turns the image's own record of failed extra_packages into a test
+// step, so a run cannot pass on a router the build could not finish equipping.
+//
+// Only for the container tier: a VM installs its extra_packages over ssh
+// during provisioning, which fails as itself and never reaches here.
+func extrasStep(ctx context.Context, run syncpkg.Exec, r *config.Router) checkpkg.Result {
+	res := checkpkg.Result{Check: "extra_packages", Kind: checkpkg.KindPackage, OK: true}
+	if missing := missingExtras(ctx, run); len(missing) > 0 {
+		res.OK = false
+		res.Detail = "built without " + strings.Join(missing, ", ") +
+			" — `owlab up --rebuild " + r.ID + "` shows what the package manager said"
+	}
+	return res
+}
+
 // testReport is the --json document. The schema field is first because
 // consumers of a machine-readable output need a way to tell when it changed.
 type testReport struct {

docs/internals.md

@@ -435,7 +435,7 @@ the two package managers do not share a naming scheme: the same release
 publishes `luci-theme-footstrap-0.11.5-r1.apk` and
 `luci-theme-footstrap_0.11.5-r1_all.ipk`.
 
-Four things about how these are installed, each of which was a bug first.
+Five things about how these are installed, each of which was a bug first.
 
 **Downloaded on the host.** A stock OpenWrt rootfs has no `curl` and its
 busybox `wget` cannot do TLS, so an in-image download would depend on which
@@ -449,7 +449,8 @@ packages depend on each other — `luci-app-example` requires `example-daemon`,
 before it — which apk refuses outright with `unable to select packages`.
 
 **All of them in one command.** Handed the whole set, the package manager
-resolves among them and the ordering stops mattering at all.
+resolves among them and the ordering stops mattering — until the retry below,
+which is one file at a time and leans on the staged order again.
 
 **`--force-overwrite`.** Their dependencies routinely replace a file the stock
 image already owns:
@@ -464,9 +465,27 @@ exactly what was asked for. ImmortalWrt already ships `dnsmasq-full`, so
 without this the same config produced a different router on OpenWrt 24.10 than
 on the other three.
 
-**A failure here is fatal**, unlike a feed name. These are named by URL — the
-developer said "install this file" — and a build that reported success without
-it would hand them a router quietly missing the thing they are testing against.
+**A failure here is neither swallowed nor fatal**, unlike a feed name. These
+are named by URL — the developer said "install this file" — so a build that
+reported success without one would hand them a router quietly missing the thing
+they are testing against.
+
+Fatal was the first answer and cost more than it saved. `docker compose build`
+puts every router in one buildkit solve, and buildkit cancels the solve on the
+first target that fails, so one unusable file killed every other router in the
+lab. Measured 2026-09-04 on a three-router stand: opkg exits 255 on an
+unsatisfiable dependency, apk exits 27 on `unable to select packages`, and both
+left `CANCELED` neighbours — it is `set -eu`, not the package manager. Nor is
+there a knob: neither `docker compose build` nor `docker buildx bake` has a
+`--keep-going`.
+
+So the set is installed as one, then one file at a time if that failed, and
+what is still missing is written to `/etc/owlab/extras-failed` — which
+`owlab up` reads back off the running router and reports under its table before
+exiting non-zero, and which `owlab test` fails on as an `extra_packages` step.
+The record is what makes tolerance honest rather than silent: a build log
+scrolls past, and a router started from a cached image never printed one at
+all.
 
 These are installed **without signature verification**. Projects publishing
 this way usually sign with usign and ship a `.sig` beside the artifact, but

docs/reference.md

@@ -166,10 +166,20 @@ Downloads happen on the host and are cached in `.owlab/cache/`, so `up` does
 not re-fetch them. They have to: a stock OpenWrt rootfs has no `curl`, and its
 busybox `wget` cannot do TLS.
 
-A failure here is fatal, unlike a name in `packages:` that a particular feed
-happens not to carry. These are named by URL — you said "install this file" —
-and a build that reported success without it would hand you a router quietly
-missing the thing you are testing against. They are also installed with
+A failure here is neither passed over nor fatal, unlike a name in `packages:`
+that a particular feed happens not to carry. These are named by URL — you said
+"install this file" — so owlab will not report success without one. It will
+not fail the build either: every router in a lab is built in one buildkit
+solve, and one failing target cancels all the others, so a single package would
+take the whole lab down.
+
+What happens instead: the set is installed together, then one file at a time if
+that failed, so the good ones still land. Whatever is still missing is named by
+`owlab up` after its table, is failed on by `owlab test`, and is left on the
+router in `/etc/owlab/extras-failed`. `owlab up` exits non-zero when it has to
+print that — the lab is running, and it is not what your config describes.
+
+They are also installed with
 `--force-overwrite`, because their dependencies routinely replace a file the
 stock image already owns: `luci-app-openclash` pulls `dnsmasq-full`, which
 ships `/etc/init.d/dnsmasq` and collides with `dnsmasq`. Without it the same

docs/reference_ru.md

@@ -163,10 +163,20 @@ defaults:
 их заново. Иначе и нельзя: в стоковом rootfs нет `curl`, а его busybox `wget` не
 умеет TLS.
 
-Провал здесь фатален, в отличие от имени из `packages:`, которого конкретный
-фид может законно не иметь. Эти названы по URL — вы сказали «поставь этот файл»
-— и сборка, отрапортовавшая успех без него, отдала бы вам роутер, тихо лишённый
-того, против чего вы и тестируете. Ставятся с `--force-overwrite`, потому что их
+Провал здесь не проглатывается и не фатален, в отличие от имени из `packages:`,
+которого конкретный фид может законно не иметь. Эти названы по URL — вы сказали
+«поставь этот файл», — и об успехе без него owlab не отрапортует. Сборку он
+тоже не уронит: все роутеры лаборатории собираются в одном solve buildkit, и
+одна упавшая цель отменяет остальные, так что один пакет забрал бы с собой всю
+лабораторию.
+
+Вместо этого набор ставится целиком, а если не вышло — по одному файлу, чтобы
+исправные всё-таки встали. То, что так и не встало, называет `owlab up` после
+своей таблицы, на этом падает `owlab test`, и это же лежит на роутере в
+`/etc/owlab/extras-failed`. Напечатав такое, `owlab up` выходит с ненулевым
+кодом: лаборатория работает, но это не то, что описано в вашем конфиге.
+
+Ставятся с `--force-overwrite`, потому что их
 зависимости регулярно заменяют файл, которым уже владеет стоковый образ:
 `luci-app-openclash` тянет `dnsmasq-full`, который ставит `/etc/init.d/dnsmasq`
 и сталкивается с `dnsmasq`.