install-blobs, flash-self: handle a failing blob_status

README.md

@@ -106,7 +106,10 @@ Firmware from 3.5.0 on keeps the iCE40 gateware and the ESP32-C3 Wi-Fi image in
 W25Q flash rather than in its image. After writing the firmware, `flash-self` waits for the
 pod's USB console and installs the ones it lacks from the same release (or from `blobs/`
 next to a local `.bin`); `install-blobs` does that step on its own, and `set-wifi` runs it
-first when the ESP32-C3 image is missing. `flash-self` also refuses an image that does not
+first when the ESP32-C3 image is missing. If the flashed pod answers but does not list its
+blob slots (its `blob_status` fails), `flash-self` still exits 0, since the firmware is in, but
+prints a warning that names the blobs it skipped; install them with `benchpod install-blobs`,
+which treats the same failure as an error. `flash-self` also refuses an image that does not
 fit the pod's flash (a 1 MB STM32H563 cannot take firmware built for the 2 MB part).
 
 Releases can publish a signed manifest, `<asset>.sig`, next to `bench_pod_stm32.bin` and each
@@ -1304,6 +1307,8 @@ Install the gateware and ESP32-C3 images the pod's firmware goes with (USB: over
 pod's USB network adapter when it is up, firmware 3.9.0+, else its console). By
 default they come from the release that matches the firmware the pod runs. Each
 blob's signature is reported; for now a missing or failing one never stops it.
+When the pod does not list its blob slots (its `blob_status` command fails), it installs
+nothing and exits non-zero with the pod's reason.
 
 | Flag        | Default         | Purpose                                                        |
 |-------------|-----------------|----------------------------------------------------------------|

cmd/benchpod-cli/blobs.go

@@ -5,6 +5,7 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -136,6 +137,43 @@ func blobsToInstall(m blobsManifest, slots []serialconsole.BlobSlot, force bool)
 	return out
 }
 
+// blobStatusError is a failed read of the pod's blob slots (blob_status, or the console's
+// `blobs`): nothing can be compared, so nothing is installed.
+type blobStatusError struct{ err error }
+
+func (e *blobStatusError) Error() string {
+	return fmt.Sprintf("the pod did not list its blob slots (blob_status: %v), so no blobs were installed", e.err)
+}
+
+func (e *blobStatusError) Unwrap() error { return e.err }
+
+// blobNamesAt names the blobs the manifest at src lists ("gw0, gw1, gw2, esp"), or a generic
+// phrase when the manifest cannot be read.
+func blobNamesAt(ctx context.Context, src blobSource) string {
+	m, err := loadBlobsManifest(ctx, src)
+	var names []string
+	if err == nil {
+		for _, e := range m.Blobs {
+			if e.Size > 0 && e.File != "" {
+				names = append(names, e.Name)
+			}
+		}
+	}
+	if len(names) == 0 {
+		return "the gateware and ESP32-C3 images that go with this firmware"
+	}
+	return strings.Join(names, ", ")
+}
+
+// blobStatusWarning is what flash-self prints when the flashed pod did not list its blob
+// slots: the firmware is in, the blobs are not, and how to retry.
+func blobStatusWarning(ctx context.Context, src blobSource, err error) string {
+	return fmt.Sprintf("flash-self: WARNING: the firmware is flashed, but reading the pod's blob slots failed (%v),\n"+
+		"  so these blobs were NOT installed: %s.\n"+
+		"  The pod may run old or missing gateware until they are. Install them with `benchpod install-blobs`.",
+		errors.Unwrap(err), blobNamesAt(ctx, src))
+}
+
 // installBlobs brings the pod's blob slots in line with the manifest at src, over the USB
 // console. only, when non-empty, limits it to those slot names.
 func installBlobs(ctx context.Context, console blobTarget, src blobSource, force bool, only []string) error {
@@ -147,7 +185,10 @@ func installBlobs(ctx context.Context, console blobTarget, src blobSource, force
 func installBlobsList(ctx context.Context, console blobTarget, src blobSource, force bool, only []string) ([]blobEntry, error) {
 	slots, err := console.Blobs(ctx)
 	if err != nil {
-		return nil, err
+		if errors.Is(err, serialconsole.ErrNoBlobSlots) {
+			return nil, err
+		}
+		return nil, &blobStatusError{err: err}
 	}
 	m, err := loadBlobsManifest(ctx, src)
 	if err != nil {
@@ -263,7 +304,10 @@ func newInstallBlobsCmd(g *globalFlags) *cobra.Command {
 			if strings.TrimSpace(only) != "" {
 				sel = strings.Split(only, ",")
 			}
-			before, _ := target.Blobs(ctx)
+			before, err := target.Blobs(ctx)
+			if err != nil && !errors.Is(err, serialconsole.ErrNoBlobSlots) {
+				return fmt.Errorf("install-blobs: %w", &blobStatusError{err: err})
+			}
 			installed, err := installBlobsList(ctx, target, src, force, sel)
 			if len(installed) > 0 {
 				g.reportFirmware(blobsEvent(ctx, target, before, installed))

cmd/benchpod-cli/flash_self.go

@@ -599,9 +599,16 @@ func installBlobsAfterFlash(g *globalFlags, src blobSource) (after podVersions,
 			fmt.Fprintln(os.Stderr, "flash-self: this firmware keeps its blobs built in; nothing to install")
 			return after, installed, nil
 		}
-		return after, installed, fmt.Errorf("flash-self: the firmware is flashed, but reading the pod's blob slots failed, so its blobs are not installed (retry with `benchpod install-blobs`): %w", err)
+		// The firmware is in: warn, naming what was skipped, rather than fail the flash.
+		fmt.Fprintln(os.Stderr, blobStatusWarning(ctx, src, &blobStatusError{err: err}))
+		return after, installed, nil
 	}
 	if installed, err = installBlobsList(ctx, target, src, false, nil); err != nil {
+		var bs *blobStatusError
+		if errors.As(err, &bs) {
+			fmt.Fprintln(os.Stderr, blobStatusWarning(ctx, src, bs))
+			return after, installed, nil
+		}
 		return after, installed, fmt.Errorf("flash-self: the firmware is flashed, but installing its blobs failed (retry with `benchpod install-blobs`): %w", err)
 	}
 	return after, installed, nil

cmd/benchpod-cli/flash_self_network.go

@@ -267,8 +267,11 @@ func runFlashSelfNetwork(g *globalFlags, f *flashSelfFlags, changed func(string)
 		if _, berr := target.Blobs(ctx); errors.Is(berr, serialconsole.ErrNoBlobSlots) {
 			fmt.Fprintln(os.Stderr, "flash-self: this firmware keeps its blobs built in; nothing to install")
 		} else if berr != nil {
-			err = fmt.Errorf("flash-self: the firmware is updated, but reading the pod's blob slots failed, so its blobs are not installed (retry with `benchpod install-blobs`): %w", berr)
-		} else if installed, berr = installBlobsList(ctx, target, src, false, nil); berr != nil {
+			// The firmware is in: warn, naming what was skipped, rather than fail the update.
+			fmt.Fprintln(os.Stderr, blobStatusWarning(ctx, src, &blobStatusError{err: berr}))
+		} else if installed, berr = installBlobsList(ctx, target, src, false, nil); errors.As(berr, new(*blobStatusError)) {
+			fmt.Fprintln(os.Stderr, blobStatusWarning(ctx, src, berr))
+		} else if berr != nil {
 			err = fmt.Errorf("flash-self: the firmware is updated, but installing its blobs failed (retry with `benchpod install-blobs`): %w", berr)
 		}
 	}

cmd/benchpod-cli/flash_self_network_test.go

@@ -2,10 +2,12 @@ package main
 
 import (
 	"bufio"
+	"context"
 	"crypto/sha256"
 	"encoding/base64"
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net"
 	"os"
@@ -14,6 +16,8 @@ import (
 	"sync"
 	"testing"
 	"time"
+
+	"github.com/embeddedci-com/benchpod-cli/internal/serialconsole"
 )
 
 // fwNetPod is a pod's network API as flash-self --network uses it: status, identity, ota_* for
@@ -25,6 +29,7 @@ type fwNetPod struct {
 	flashKB int
 	refuse  string // error reply to ota_begin
 	corrupt bool   // ota_end reports a hash mismatch
+	blobErr string // error reply to blob_status (default "unknown cmd": blobs built in)
 	staged  []byte
 	sha     string
 	state   string
@@ -77,6 +82,9 @@ func (p *fwNetPod) handle(req map[string]any) (string, bool) {
 	case "identity_public":
 		return okReply(map[string]any{"public": "pubkey-1"}), false
 	case "blob_status":
+		if p.blobErr != "" {
+			return errReply(p.blobErr), false
+		}
 		return errReply("unknown cmd"), false
 	case "ota_begin":
 		if p.refuse != "" {
@@ -214,3 +222,89 @@ func TestFlashSelfNetworkRefusesCloudAndDFUFlags(t *testing.T) {
 		t.Fatalf("enter-dfu err = %v", err)
 	}
 }
+
+// blobsDirWith writes a blobs manifest listing names (files need not exist: nothing is fetched
+// when the slots cannot be read).
+func blobsDirWith(t *testing.T, names ...string) string {
+	dir := t.TempDir()
+	var entries []string
+	for _, n := range names {
+		entries = append(entries, fmt.Sprintf(`{"name":%q,"file":"blob-%s.bin","size":10,"sha256":"00"}`, n, n))
+	}
+	m := `{"format":1,"blobs":[` + strings.Join(entries, ",") + `]}`
+	if err := os.WriteFile(filepath.Join(dir, blobsManifestName), []byte(m), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	return dir
+}
+
+// A pod that answers blob_status with an error (a dev build said "reply too large") must not
+// look like one with nothing to install: flash-self warns, naming the skipped blobs and the
+// retry, and keeps its exit status (the firmware itself is in).
+func TestFlashSelfNetworkWarnsWhenBlobStatusFails(t *testing.T) {
+	fastNetworkFlash(t)
+	pod := &fwNetPod{t: t, version: "3.9.0", flashKB: 2048, blobErr: "reply too large"}
+	addr := pod.start()
+	file, _ := fwImage(t, 2000, 1024)
+	dir := blobsDirWith(t, "gw0", "gw1", "gw2", "esp")
+	var err error
+	out := captureStderr(t, func() {
+		err = runFlashSelfNetwork(&globalFlags{connection: addr, noFirmwareReport: true},
+			&flashSelfFlags{network: true, blobsDir: dir}, noFlagChanged, []string{file})
+	})
+	if err != nil {
+		t.Fatalf("the firmware was flashed; want a warning, got err = %v", err)
+	}
+	for _, want := range []string{"WARNING", "reply too large", "NOT installed: gw0, gw1, gw2, esp", "benchpod install-blobs"} {
+		if !strings.Contains(out, want) {
+			t.Fatalf("stderr lacks %q:\n%s", want, out)
+		}
+	}
+	if strings.Contains(out, "already holds every blob") || strings.Contains(out, "built in") {
+		t.Fatalf("a failed blob_status read as nothing to install:\n%s", out)
+	}
+	if n := strings.Count(pod.cmds(), "ota_begin"); n != 1 {
+		t.Fatalf("installed blobs without a slot listing: %s", pod.cmds())
+	}
+}
+
+// install-blobs (installBlobsList) fails when the pod does not list its slots.
+func TestInstallBlobsFailsWhenBlobStatusFails(t *testing.T) {
+	pod := &fwNetPod{t: t, version: "3.10.3", flashKB: 2048, blobErr: "reply too large"}
+	addr := pod.start()
+	ctx := context.Background()
+	lan, err := openLAN(ctx, addr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer lan.s.Close()
+	target := lanBlobs{lan: lan, addr: addr}
+	installed, err := installBlobsList(ctx, target, blobSource{base: blobsDirWith(t, "gw0", "gw2")}, false, nil)
+	if err == nil || !strings.Contains(err.Error(), "reply too large") || !strings.Contains(err.Error(), "no blobs were installed") {
+		t.Fatalf("err = %v, want the blob_status failure", err)
+	}
+	var bs *blobStatusError
+	if !errors.As(err, &bs) || len(installed) != 0 {
+		t.Fatalf("err = %T %v, installed = %v", err, err, installed)
+	}
+	if strings.Contains(pod.cmds(), "ota_begin") {
+		t.Fatalf("sent a blob without a slot listing: %s", pod.cmds())
+	}
+}
+
+// "unknown cmd" still means firmware with its blobs built in, not a failure.
+func TestInstallBlobsBuiltInIsNotABlobStatusFailure(t *testing.T) {
+	pod := &fwNetPod{t: t, version: "3.4.0", flashKB: 2048}
+	addr := pod.start()
+	ctx := context.Background()
+	lan, err := openLAN(ctx, addr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer lan.s.Close()
+	_, err = installBlobsList(ctx, lanBlobs{lan: lan, addr: addr}, blobSource{base: blobsDirWith(t, "gw0")}, false, nil)
+	var bs *blobStatusError
+	if !errors.Is(err, serialconsole.ErrNoBlobSlots) || errors.As(err, &bs) {
+		t.Fatalf("err = %v, want ErrNoBlobSlots", err)
+	}
+}

cmd/benchpod-cli/flash_self_test.go

@@ -113,3 +113,29 @@ func TestInstallBlobsAfterFlashFailsWhenTheConsoleNeverReturns(t *testing.T) {
 		t.Fatalf("err = %v, want an error naming install-blobs", err)
 	}
 }
+
+// After a DFU flash, a pod that does not list its blob slots gets a warning naming the skipped
+// blobs and the retry; flash-self keeps its exit status (the firmware itself is in).
+func TestInstallBlobsAfterFlashWarnsWhenTheSlotsAreNotListed(t *testing.T) {
+	oldWait, oldRetry, oldOpen, oldSkip := flashedConsoleWait, flashedConsoleRetry, openFlashedConsole, skipUSBNetForBlobs
+	t.Cleanup(func() {
+		flashedConsoleWait, flashedConsoleRetry, openFlashedConsole, skipUSBNetForBlobs = oldWait, oldRetry, oldOpen, oldSkip
+	})
+	skipUSBNetForBlobs = true
+	openFlashedConsole = func(*globalFlags) (*serialconsole.Console, error) {
+		return serialconsole.NewConsole(&textConsole{status: "  device : benchpod\r\n"}), nil
+	}
+	var err error
+	var installed []blobEntry
+	out := captureStderr(t, func() {
+		_, installed, err = installBlobsAfterFlash(&globalFlags{}, blobSource{base: blobsDirWith(t, "gw0", "gw2", "esp")})
+	})
+	if err != nil || len(installed) != 0 {
+		t.Fatalf("err = %v, installed = %v; want a warning only", err, installed)
+	}
+	for _, want := range []string{"WARNING", "NOT installed: gw0, gw2, esp", "benchpod install-blobs"} {
+		if !strings.Contains(out, want) {
+			t.Fatalf("stderr lacks %q:\n%s", want, out)
+		}
+	}
+}