carlos / platform Public

console: a console without a registry has no fleets, not an unreadable one

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QCUCpfxbCyEAVQ2m51dH5
Paul Campbell pushed by paul@keymail.dev e33187465a748d8565e54010e15a1d9965e377bc parent deab7b6
2 files changed, +92 −13
  • internal/console/apisystem.go +43 −12
  • internal/console/apisystem_test.go +49 −1
diff --git a/internal/console/apisystem.go b/internal/console/apisystem.go
index e7befef93..ee8bfde86 100644
--- a/internal/console/apisystem.go
+++ b/internal/console/apisystem.go
@@ -194,13 +194,34 @@ func (s *Server) systemFeed() publicrelease.Feed {
// running is better served by the boxes that did answer plus an honest
// "partial" flag than by a blank page. The second return is that flag: true
// means some fleet's boxes are missing from the slice.
+//
+// NEITHER answer applies to a console with NO REGISTRY. cfg.Registry == ""
+// and a registry file that has not been created yet are legal, ordinary
+// shapes — a console before its first box, or one that serves no route
+// registry at all — and they mean this deployment has no fleets, full stop.
+// Treating them as "unreadable" would refuse every promote such a console
+// could ever make and pin its status at partial forever, which is what the
+// first cut of this function did. Only a registry that EXISTS and still
+// cannot be opened, or one whose fleets cannot be listed, is a fleet set this
+// console cannot see.
func (s *Server) systemBoxes(ctx context.Context, strict bool) ([]status.Box, bool, error) {
boxes, err := status.LoadBoxes(ctx, s.cfg.Releases)
if err != nil {
return nil, false, err
}
+ if s.cfg.Registry == "" {
+ return boxes, false, nil // this console serves no route registry: no fleets
+ }
+ // openPassRegistry reports the not-yet-created case by wrapping os.Stat's
+ // error, so errors.Is reaches it; the unconfigured case is the guard
+ // above, since that one is a plain message. Distinguished here rather
+ // than in openPassRegistry so its other callers keep the answers they
+ // already expect.
reg, err := s.openPassRegistry()
- if err != nil {
+ switch {
+ case errors.Is(err, os.ErrNotExist):
+ return boxes, false, nil // no registry file yet: no fleets
+ case err != nil:
return s.systemFleetsUnreadable(boxes, strict, err)
}
fleets, err := reg.ListFleets()
@@ -289,7 +310,7 @@ func (s *Server) handleAPISystemUpdate(w http.ResponseWriter, r *http.Request) {
})
return
}
- go s.runSystemUpdate(context.WithoutCancel(r.Context()), job, addr, in, spread)
+ go s.runSystemUpdate(context.WithoutCancel(r.Context()), id, job, addr, in, spread)
writeJSON(w, http.StatusOK, map[string]string{"job": id})
}
@@ -334,19 +355,27 @@ func (s *Server) startSystemJob(component, channel string) (id string, job *syst
return id, job, ""
}
-// releaseSystemChannel hands the slot back. It is called BEFORE the job is
-// marked done, so an operator who polls a job to completion and immediately
-// promotes again is never told the channel is busy by the job they just
-// watched finish.
-func (s *Server) releaseSystemChannel(component, channel string) {
+// releaseSystemChannel hands the slot back, and only if this job is still the
+// one holding it: a compare-and-delete on the id, so a late or repeated
+// release can never free a LATER promote's slot and let a third run beside
+// it. That makes it idempotent, which is what lets runSystemUpdate both defer
+// it (the panic path) and call it explicitly at the point the ordering
+// matters.
+func (s *Server) releaseSystemChannel(component, channel, id string) {
+ key := component + "/" + channel
s.systemMu.Lock()
- delete(s.systemRunning, component+"/"+channel)
+ if s.systemRunning[key] == id {
+ delete(s.systemRunning, key)
+ }
s.systemMu.Unlock()
}
// runSystemUpdate is the promote: resolve, ingest (or take the upload),
// mirror, sign, bump. Every step is idempotent, so a lost job is re-run.
-func (s *Server) runSystemUpdate(ctx context.Context, job *systemJob, actor string, in systemUpdateRequest, spread time.Duration) {
+func (s *Server) runSystemUpdate(ctx context.Context, id string, job *systemJob, actor string, in systemUpdateRequest, spread time.Duration) {
+ // Every exit hands the channel back, a panic included — without this a
+ // crashed promote would wedge its channel for the life of the process.
+ defer s.releaseSystemChannel(in.Component, in.Channel, id)
err := func() error {
var rel release.SystemRelease
if in.Upload != "" {
@@ -409,9 +438,11 @@ func (s *Server) runSystemUpdate(ctx context.Context, job *systemJob, actor stri
job.step("serial bumped: boxes on %s start their spread", in.Channel)
return nil
}()
- // Hand the channel back BEFORE marking the job done — see
- // releaseSystemChannel.
- s.releaseSystemChannel(in.Component, in.Channel)
+ // And hand it back BEFORE marking the job done, so an operator who polls
+ // this job to completion and immediately promotes again is never told the
+ // channel is busy by the job they just watched finish. The deferred
+ // release above then finds nothing to do; it is there for the panic path.
+ s.releaseSystemChannel(in.Component, in.Channel, id)
job.finish(err)
}
diff --git a/internal/console/apisystem_test.go b/internal/console/apisystem_test.go
index c78e1a90b..63c396000 100644
--- a/internal/console/apisystem_test.go
+++ b/internal/console/apisystem_test.go
@@ -12,6 +12,7 @@ import (
"mime/multipart"
"net/http"
"net/http/httptest"
+ "os"
"path/filepath"
"strings"
"sync"
@@ -267,7 +268,14 @@ func TestSystemUpdateRefusesWhenFleetsAreUnreadable(t *testing.T) {
systemFeed(t, fx, map[string][]byte{"carlos-linux-arm64": []byte("ELF arm64")})
seedBox(t, fx, "ie", "linux-arm64", "canary", "v0.17.0")
owner := cliToken(t, fx, "paul@keymail.dev", "code-sys-fleeterr")
- fx.srv.cfg.Registry = filepath.Join(t.TempDir(), "gone.db")
+ // A registry that EXISTS and cannot be opened — a directory where the
+ // database should be. Not a missing path: a path that is not there yet is
+ // an ordinary console with no fleets, which the sibling test covers.
+ broken := filepath.Join(t.TempDir(), "registry.db")
+ if err := os.Mkdir(broken, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ fx.srv.cfg.Registry = broken
st, out := postJSON(t, fx.ts.URL+"/api/cli/system/update", map[string]any{"channel": "canary"}, owner)
if st != http.StatusOK {
@@ -296,6 +304,46 @@ func TestSystemUpdateRefusesWhenFleetsAreUnreadable(t *testing.T) {
}
}
+// TestSystemUpdateWithNoRegistryHasNoFleets: a console with no route registry
+// — cfg.Registry unset, or the file not created yet — is an ordinary console
+// with no fleets, not one whose fleets are unreadable. It must be able to
+// promote, and its status must not claim to be short of anything.
+func TestSystemUpdateWithNoRegistryHasNoFleets(t *testing.T) {
+ for _, c := range []struct{ name, registry string }{
+ {"unset", ""},
+ {"not created yet", filepath.Join(t.TempDir(), "never-made.db")},
+ } {
+ t.Run(c.name, func(t *testing.T) {
+ fx := newConsoleFixture(t)
+ systemFeed(t, fx, map[string][]byte{"carlos-linux-arm64": []byte("ELF arm64")})
+ owner := cliToken(t, fx, "paul@keymail.dev", "code-sys-noreg-"+strings.ReplaceAll(c.name, " ", "-"))
+ fx.srv.cfg.Registry = c.registry
+
+ st, out := postJSON(t, fx.ts.URL+"/api/cli/system/update", map[string]any{"channel": "canary"}, owner)
+ if st != http.StatusOK {
+ t.Fatalf("update = %d %v", st, out)
+ }
+ if job := waitJob(t, fx, owner, str(t, out, "job")); job["error"] != "" {
+ t.Fatalf("job: %v", job)
+ }
+ p, _, err := release.GetSystemPointer(context.Background(), fx.rel, fx.pub, "router", "canary")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, ok := p.Current.Artifacts["linux-arm64"]; !ok {
+ t.Fatalf("artifacts = %+v, want linux-arm64 mirrored", p.Current.Artifacts)
+ }
+ st, out = getJSON(t, fx.ts.URL+"/api/cli/system/status", owner)
+ if st != http.StatusOK {
+ t.Fatalf("status = %d %v", st, out)
+ }
+ if out["partial"] != false {
+ t.Errorf("status partial = %v, want false — no registry is no fleets, not unreadable ones", out["partial"])
+ }
+ })
+ }
+}
+
// TestSystemStatusNamesAnUnverifiablePointer: null means "never promoted"
// and nothing else. A pointer that exists but does not verify is the one
// thing an operator most needs to see, so it must not render as the one