config: an absent exhibit directory warns and is skipped, not fatal
commit bd278b033654b00c9f3f58f4eae6f551c0455a24 · parent bd3b610b14
· browse this tree
Ximianth, 2026-08-09 16:59:07 UTC
Came around to the realization that the server does perfectly reasonable things (errors: but reasonably) when a dir is removed when the server's already running. Therefore, having the server reject that condition at launch makes little sense. (And specifically, can be an operational surprise because the server could refuse to re-launch on a situation that it was previously running happily with.)
diff --git a/README.md b/README.md index cb2b1ed..1bbf98c 100644 --- a/README.md +++ b/README.md @@ -41,13 +41,14 @@ exhibits: A bad config (parse error, unknown key, missing path, or zero exhibits) is a startup error: vitrine refuses to run rather than serve the wrong thing. +An exhibit whose directory is absent is not a bad config, though: +that exhibit is skipped with a warning and the rest serve normally, +so one config can describe trees that only some hosts happen to have. + Config changes apply live: vitrine watches the config file (inotify for immediate pickup, plus a 5-second stat poll as backstop, so a change lands within ~5s worst case, usually instantly). -At runtime the failure mode flips from fail-loud to fail-stable: -a reload that fails to parse, or names a directory that can't be opened, -logs a loud REJECTED line and the previous exhibits keep serving untouched. -Edit freely; a typo mid-edit can't take the server dark. +Edit freely; a typo mid-edit logs a rejection and leaves the running server untouched. Theming ------- diff --git a/main.go b/main.go index f1003a5..d1c9ca0 100644 --- a/main.go +++ b/main.go @@ -38,14 +38,12 @@ func main() { if err != nil { log.Fatal(err) } - srv, err := newServer(cfg.Exhibits, newTheme(seed)) - if err != nil { - log.Fatal(err) - } + srv := newServer(cfg.Exhibits, newTheme(seed)) // Config changes apply live: reloads are fail-stable - // (a bad config logs loudly and the old exhibits keep serving), - // in contrast to the fail-loud startup above. + // (a malformed config logs loudly and the old exhibits keep serving). + // A well-formed config naming an absent directory is not an error at all: + // that exhibit is skipped with a warning, at startup and on reload alike. watchConfig(*configPath, 5*time.Second, func() { reloadConfig(srv, *configPath) }) @@ -55,18 +53,16 @@ func main() { } // reloadConfig loads the config file and swaps the server's exhibits to it. -// Any failure (parse, validation, unopenable exhibit dir) logs loudly -// and leaves the previously served exhibits untouched: -// a fleet of vitrines must not go dark from one typo mid-edit. +// A malformed config (parse or validation failure) logs loudly and leaves the +// previously served exhibits untouched: a fleet of vitrines must not go dark +// from one typo mid-edit. A well-formed config whose directories are partly +// absent applies normally; the missing ones are skipped with a warning. func reloadConfig(srv *server, path string) { cfg, err := loadConfig(path) if err != nil { log.Printf("config reload REJECTED, keeping previous exhibits: %v", err) return } - if err := srv.setExhibits(cfg.Exhibits); err != nil { - log.Printf("config reload REJECTED, keeping previous exhibits: %v", err) - return - } + srv.setExhibits(cfg.Exhibits) log.Printf("config reloaded: %d exhibits", len(cfg.Exhibits)) } diff --git a/serve.go b/serve.go index 01bd142..6225ed3 100644 --- a/serve.go +++ b/serve.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "io/fs" "log" "net/http" @@ -35,18 +34,18 @@ type tree struct { exposeGit bool } -func newServer(exhibits map[string]exhibit, th theme) (*server, error) { +func newServer(exhibits map[string]exhibit, th theme) *server { s := &server{theme: th} - if err := s.setExhibits(exhibits); err != nil { - return nil, err - } - return s, nil + s.setExhibits(exhibits) + return s } // setExhibits builds a fresh mount set from the given exhibits and publishes it atomically. -// On any error (an exhibit dir that fails to open) nothing is published: -// at startup that is fail-loud (newServer returns the error), -// and on reload it is fail-stable (the previous mount set keeps serving untouched). +// An exhibit whose directory cannot be opened (typically because it does not exist) +// is logged and skipped, not fatal: a config may legitimately name a tree +// that is absent on this particular host, and the rest should still serve. +// Malformed config is rejected earlier, in loadConfig; by here the config is well-formed +// and the only question is which of its directories are actually present. // // The previous mount set's os.Root handles are deliberately not closed here. // An in-flight request may have loaded the old set just before the swap @@ -56,31 +55,26 @@ func newServer(exhibits map[string]exhibit, th theme) (*server, error) { // That is the simplest scheme that is actually correct: // refcounting would buy only promptness, // and a few directory fds lingering until the next GC cycle cost nothing. -func (s *server) setExhibits(exhibits map[string]exhibit) error { +func (s *server) setExhibits(exhibits map[string]exhibit) { mounts := make(mountSet, len(exhibits)) for name, ex := range exhibits { root, err := os.OpenRoot(ex.Path) if err != nil { - // Nothing from this attempt was published, so closing is safe here. - for _, tr := range mounts { - tr.root.Close() - } - return fmt.Errorf("opening exhibit %q: %w", name, err) + log.Printf("exhibit %q skipped: %v", name, err) + continue } mounts[name] = tree{root: root, exposeGit: ex.ExposeGit} } - // Run update-server-info only once the whole set has validated: - // a rejected config should leave no trace, not even freshened git metadata. + // Run update-server-info only for exhibits that actually opened above. // Re-running on every successful reload is deliberate and idempotent. // (Watching the git repos themselves for new commits is possible future work; // today a commit becomes clonable at the next config reload or restart.) - for name, ex := range exhibits { - if ex.ExposeGit { - updateGitServerInfo(name, ex.Path) + for name, tr := range mounts { + if tr.exposeGit { + updateGitServerInfo(name, exhibits[name].Path) } } s.mounts.Store(&mounts) - return nil } // updateGitServerInfo runs `git update-server-info` in the exhibit directory, diff --git a/serve_test.go b/serve_test.go index a6436f7..fa07d3c 100644 --- a/serve_test.go +++ b/serve_test.go @@ -29,10 +29,7 @@ func newTestServer(t *testing.T) *server { t.Fatal(err) } } - srv, err := newServer(map[string]exhibit{"demo": {Path: dir}}, newTheme("testhost")) - if err != nil { - t.Fatal(err) - } + srv := newServer(map[string]exhibit{"demo": {Path: dir}}, newTheme("testhost")) return srv } @@ -133,12 +130,9 @@ func newNestedTestServer(t *testing.T) *server { t.Fatal(err) } } - srv, err := newServer(map[string]exhibit{ + srv := newServer(map[string]exhibit{ "meta/research/latest": {Path: filepath.Join(dir, "research/latest")}, }, theme{}) - if err != nil { - t.Fatal(err) - } return srv } @@ -222,10 +216,7 @@ func newExposeGitTestServer(t *testing.T) (*server, string) { t.Fatalf("git %v: %v: %s", args, err, out) } } - srv, err := newServer(map[string]exhibit{"repo": {Path: dir, ExposeGit: true}}, theme{}) - if err != nil { - t.Fatal(err) - } + srv := newServer(map[string]exhibit{"repo": {Path: dir, ExposeGit: true}}, theme{}) return srv, dir } diff --git a/watch_test.go b/watch_test.go index c6d00e8..c7e8ac4 100644 --- a/watch_test.go +++ b/watch_test.go @@ -40,10 +40,7 @@ func newWatchFixture(t *testing.T, pollInterval time.Duration) *watchFixture { if err != nil { t.Fatal(err) } - fx.srv, err = newServer(cfg.Exhibits, theme{}) - if err != nil { - t.Fatal(err) - } + fx.srv = newServer(cfg.Exhibits, theme{}) watchConfig(fx.cfgPath, pollInterval, func() { reloadConfig(fx.srv, fx.cfgPath) fx.checked.Add(1) @@ -91,28 +88,40 @@ func TestHotSwapServesNewMountsAndDropsOld(t *testing.T) { } func TestRejectedReloadKeepsOldConfigServing(t *testing.T) { - // Two flavors of bad: YAML that doesn't parse, - // and a clean parse whose exhibit dir fails to open (the setExhibits path). - for name, bad := range map[string]string{ - "parse-error": "exhibits: [unclosed\n", - "unopenable-dir": "exhibits:\n ghost:\n path: no/such/dir\n", - } { - t.Run(name, func(t *testing.T) { - fx := newWatchFixture(t, 2*time.Millisecond) - before := fx.checked.Load() - fx.writeConfig(t, bad) - // Happens-after gate: wait for a reload attempt to complete, - // then assert it changed nothing. - waitFor(t, "a reload attempt on the bad config", func() bool { - return fx.checked.Load() > before - }) - if code := fx.status(t, "/one/f.md"); code != http.StatusOK { - t.Errorf("old config stopped serving after rejected reload: status = %d", code) - } - if code := fx.status(t, "/ghost/"); code != http.StatusNotFound { - t.Errorf("rejected config partially applied: /ghost/ status = %d", code) - } - }) + // A malformed config (here: YAML that does not parse) is rejected wholesale, + // and the previously served exhibits keep serving untouched. + fx := newWatchFixture(t, 2*time.Millisecond) + before := fx.checked.Load() + fx.writeConfig(t, "exhibits: [unclosed\n") + // Happens-after gate: wait for a reload attempt to complete, + // then assert it changed nothing. + waitFor(t, "a reload attempt on the bad config", func() bool { + return fx.checked.Load() > before + }) + if code := fx.status(t, "/one/f.md"); code != http.StatusOK { + t.Errorf("old config stopped serving after rejected reload: status = %d", code) + } +} + +func TestReloadSkipsAbsentDir(t *testing.T) { + // A well-formed config naming an absent directory is not a rejection: + // the reload applies, the absent exhibit is skipped with a warning, + // and the present ones serve. The old "one" mount is gone afterward, + // which shows the new set was published rather than the reload being refused. + fx := newWatchFixture(t, 2*time.Millisecond) + before := fx.checked.Load() + fx.writeConfig(t, "exhibits:\n two:\n path: two\n ghost:\n path: no/such/dir\n") + waitFor(t, "the reload to apply", func() bool { + return fx.status(t, "/two/g.md") == http.StatusOK + }) + if fx.checked.Load() == before { + t.Error("reload never ran") + } + if code := fx.status(t, "/ghost/"); code != http.StatusNotFound { + t.Errorf("absent exhibit should be skipped, not served: /ghost/ status = %d", code) + } + if code := fx.status(t, "/one/f.md"); code != http.StatusNotFound { + t.Errorf("old mount still serving, so the reload was not applied: status = %d", code) } }