diff --git a/src/yangerd/cmd/yangerd/main.go b/src/yangerd/cmd/yangerd/main.go index 0b5f25b1..16577ad1 100644 --- a/src/yangerd/cmd/yangerd/main.go +++ b/src/yangerd/cmd/yangerd/main.go @@ -316,8 +316,14 @@ func main() { Debounce: 200 * time.Millisecond, UseMerge: true, } + // Watch the parent directory, not the file: fw_setenv (U-Boot) and + // grub-editenv may rewrite the env via a temp file + rename, which + // gives it a new inode that a direct file watch never sees. Watching + // the directory catches the Create/Rename (and still catches in-place + // writes), so a boot-order change after a RAUC install is reflected + // without waiting for a reboot. for _, path := range []string{"/mnt/aux/grub/grubenv", "/mnt/aux/uboot.env"} { - if err := fsw.Watch(path, bootOrderHandler); err != nil { + if err := fsw.WatchSymlink(path, bootOrderHandler); err != nil { slogLog.Debug("fswatcher boot-order watch skipped", "path", path, "err", err) } } diff --git a/src/yangerd/internal/collector/containers.go b/src/yangerd/internal/collector/containers.go index c1905b5b..08008232 100644 --- a/src/yangerd/internal/collector/containers.go +++ b/src/yangerd/internal/collector/containers.go @@ -38,10 +38,20 @@ func (c *ContainerCollector) Interval() time.Duration { return c.interval } // "infix-containers:containers". func (c *ContainerCollector) Collect(ctx context.Context, t *tree.Tree) error { data := c.collectJSON(ctx) + if data == nil { + // No containers: remove the key rather than leaving a bare + // "containers" node, so clients see the feature as absent. + t.Delete("infix-containers:containers") + return nil + } t.Set("infix-containers:containers", data) return nil } +// collectJSON returns the operational containers subtree, or nil when no +// container exists. Returning nil (not an empty {"container":[]}) lets the +// caller drop the key entirely so an enabled-but-idle container feature does +// not surface as operational data. func (c *ContainerCollector) collectJSON(ctx context.Context) json.RawMessage { containers := []interface{}{} @@ -53,13 +63,17 @@ func (c *ContainerCollector) collectJSON(ctx context.Context) json.RawMessage { } } + if len(containers) == 0 { + return nil + } + out := map[string]interface{}{ "container": containers, } data, err := json.Marshal(out) if err != nil { - return json.RawMessage(`{"container":[]}`) + return nil } return data } diff --git a/src/yangerd/internal/collector/containers_test.go b/src/yangerd/internal/collector/containers_test.go index e867e27b..2ee3abc5 100644 --- a/src/yangerd/internal/collector/containers_test.go +++ b/src/yangerd/internal/collector/containers_test.go @@ -436,10 +436,17 @@ func TestContainerGracefulDegradation(t *testing.T) { } fs := &testutil.MockFileReader{Files: map[string][]byte{}, Globs: map[string][]string{}} - out := collectContainers(t, runner, fs) - containers := containerList(t, out) - if len(containers) != 0 { - t.Fatalf("expected no containers when podman ps fails, got %d", len(containers)) + c := NewContainerCollector(runner, fs, 30*time.Second) + tr := tree.New() + if err := c.Collect(context.Background(), tr); err != nil { + t.Fatalf("Collect failed: %v", err) + } + + // With no containers the key must be absent, not a bare + // {"container":[]} node -- otherwise an enabled-but-idle container + // feature surfaces as operational data. + if raw := tr.Get("infix-containers:containers"); raw != nil { + t.Fatalf("expected no containers key when podman ps fails, got %s", raw) } } diff --git a/src/yangerd/internal/fswatcher/fswatcher.go b/src/yangerd/internal/fswatcher/fswatcher.go index c81d9164..f70c1789 100644 --- a/src/yangerd/internal/fswatcher/fswatcher.go +++ b/src/yangerd/internal/fswatcher/fswatcher.go @@ -117,11 +117,7 @@ func (fw *FSWatcher) InitialRead() { fw.log.Warn("fswatcher: initial read failed", "path", path, "err", err) continue } - if handler.UseMerge { - fw.tree.Merge(handler.TreeKey, data) - } else { - fw.tree.Set(handler.TreeKey, data) - } + fw.apply(handler, data) fw.log.Debug("fswatcher: initial read", "path", path, "key", handler.TreeKey) } for dir, handler := range fw.dirHandlers { @@ -130,11 +126,7 @@ func (fw *FSWatcher) InitialRead() { fw.log.Warn("fswatcher: initial read failed", "path", dir, "err", err) continue } - if handler.UseMerge { - fw.tree.Merge(handler.TreeKey, data) - } else { - fw.tree.Set(handler.TreeKey, data) - } + fw.apply(handler, data) fw.log.Debug("fswatcher: initial read", "path", dir, "key", handler.TreeKey) } } @@ -246,10 +238,23 @@ func (fw *FSWatcher) fireHandler(path string, handler WatchHandler) { fw.log.Warn("fswatcher: read failed", "path", path, "err", err) return } - if handler.UseMerge { - fw.tree.Merge(handler.TreeKey, data) - } else { - fw.tree.Set(handler.TreeKey, data) - } + fw.apply(handler, data) fw.log.Debug("fswatcher: updated", "path", path, "key", handler.TreeKey) } + +// apply writes a ReadFunc result into the tree. For merge handlers it +// merges; for plain handlers an empty result deletes the key rather than +// writing an empty node -- a collector that has "nothing" to report (e.g. +// the containers feature is enabled but no container is running) must not +// leave a bare subtree behind, or clients would see operational data where +// the feature is effectively absent. +func (fw *FSWatcher) apply(handler WatchHandler, data json.RawMessage) { + switch { + case handler.UseMerge: + fw.tree.Merge(handler.TreeKey, data) + case len(data) == 0: + fw.tree.Delete(handler.TreeKey) + default: + fw.tree.Set(handler.TreeKey, data) + } +} diff --git a/src/yangerd/internal/fswatcher/fswatcher_test.go b/src/yangerd/internal/fswatcher/fswatcher_test.go index 55ba59b0..0c8cc917 100644 --- a/src/yangerd/internal/fswatcher/fswatcher_test.go +++ b/src/yangerd/internal/fswatcher/fswatcher_test.go @@ -233,6 +233,27 @@ func TestFireHandlerReadError(t *testing.T) { } } +// A plain (non-merge) handler that returns an empty result must delete its +// key, not leave a stale or empty node behind -- e.g. the containers +// collector returns nil when no container is running. +func TestFireHandlerEmptyDeletes(t *testing.T) { + fw, tr := newTestFSWatcher(t) + + tr.Set("fire/gone", json.RawMessage(`{"container":[{"name":"old"}]}`)) + + empty := json.RawMessage(nil) + handler := WatchHandler{ + TreeKey: "fire/gone", + ReadFunc: func(string) (json.RawMessage, error) { return empty, nil }, + } + + fw.fireHandler("/fake/path", handler) + + if got := tr.Get("fire/gone"); got != nil { + t.Errorf("expected key deleted on empty result, got %s", got) + } +} + func TestDebounce(t *testing.T) { fw, tr := newTestFSWatcher(t) @@ -511,6 +532,54 @@ func TestWatchSymlinkReplace(t *testing.T) { cancel() } +// fw_setenv (U-Boot) and grub-editenv rewrite the env via a temp file + +// atomic rename, so the env gets a new inode. A direct file watch misses +// that; the parent-directory watch used for boot-order must catch it, +// otherwise operational boot-order stays stale until the next reboot. +func TestWatchSymlinkAtomicRename(t *testing.T) { + fw, tr := newTestFSWatcher(t) + + tmp := t.TempDir() + env := filepath.Join(tmp, "uboot.env") + os.WriteFile(env, []byte("BOOT_ORDER=net\n"), 0644) + + fw.WatchSymlink(env, WatchHandler{ + TreeKey: "boot/env", + ReadFunc: func(p string) (json.RawMessage, error) { + data, _ := os.ReadFile(p) + return json.RawMessage(fmt.Sprintf("%q", strings.TrimSpace(string(data)))), nil + }, + }) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + go fw.Run(ctx) + time.Sleep(50 * time.Millisecond) + + // Replace the file the way fw_setenv does: write a temp, rename over. + tmpEnv := env + ".tmp" + os.WriteFile(tmpEnv, []byte("BOOT_ORDER=primary net\n"), 0644) + if err := os.Rename(tmpEnv, env); err != nil { + t.Fatalf("rename: %v", err) + } + + deadline := time.After(2 * time.Second) + for { + if got := tr.Get("boot/env"); got != nil && strings.Contains(string(got), "primary net") { + break + } + select { + case <-deadline: + t.Fatalf("timed out waiting for atomic-rename event; tree = %s", tr.Get("boot/env")) + default: + time.Sleep(10 * time.Millisecond) + } + } + + cancel() +} + func TestWatchDir(t *testing.T) { fw, tr := newTestFSWatcher(t)