v0.64.0: additive storage discovery (A1) + internal-SSD label (A2)

A1: AutoDiscoverStoragePaths no longer bails on a non-empty registry;
registers only deployed-app paths missing from the registry. Never
mutates/removes existing entries, never re-adds or reactivates a path
present in ANY state (incl. Decommissioned), never flips IsDefault.
A2: InferStorageLabel maps base==felhom-data namespace dir to
'Belső SSD (rendszer)' to disambiguate the internal system volume.
Table-driven tests incl. a companion that fails without the
skip-by-presence guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-06-14 17:33:08 +02:00
parent 688ba0d2a5
commit 2d4d43203f
4 changed files with 341 additions and 38 deletions
+27
View File
@@ -1,5 +1,32 @@
## Changelog
### v0.64.0 — storage-lifecycle cleanups (2026-06-14)
Two settings-layer cleanups from the F9 storage-registration diagnosis
(`felhom.eu/documentation/backlog/DIAGNOSIS-f9-storage-registration-gap-2026-06-14.md`), trunk-based on
`main`, each with table-driven tests that fail on the pre-fix code.
- **A1 — `AutoDiscoverStoragePaths` is now ADDITIVE** (`internal/settings/settings.go`). It previously bailed
early (`if len(s.StoragePaths) > 0 { return }`), so a drive a deployed app referenced but that was missing
from the registry was never picked up after first run. It now registers only the discovered paths NOT
already present, while honouring strict invariants: never removes/modifies a manually-added path; SKIPS any
path already in the registry IN ANY STATE — including a `Decommissioned` soft-marked entry — so it can't
re-add or reactivate it; never flips `IsDefault` (a newly-discovered path becomes default ONLY if the
registry currently has no default at all, and only the first such new path). NOT auto-register-on-attach —
it only picks up paths deployed apps already reference (that recommendation was rejected; manual enrollment
is by design). New `internal/settings/storage_discovery_test.go` covers it, incl. a companion test that
FAILS if the skip-by-presence guard is removed (verified: removing the guard re-adds the decommissioned
path).
- **A2 — internal-SSD label disambiguation** (`InferStorageLabel`, `internal/settings/settings.go`). A path
whose basename is the `felhom-data` namespace dir (the internal system volume, e.g.
`/mnt/sys_drive/felhom-data`) previously labelled as `Tárhely (felhom-data)`, colliding with the per-drive
felhom-data namespace. It now reads **`Belső SSD (rendszer)`**. Discriminator is `base ==
appbackup.FelhomDataDir`; Model-A user drives register their MOUNT ROOT (e.g. `/mnt/felhom-usb`), never
`.../felhom-data`, so this can't mislabel a user drive. Still overridable via `SetStorageLabel`. The demo's
already-seeded `settings.json` label for that path on guest 9201 was updated out-of-band (the seeded value
doesn't auto-change). Separate host-metrics label in `web/agent_host_metrics_handler.go` was intentionally
left untouched.
### v0.63.0 — reflect agent F9/F20-BUG2 disk fields (2026-06-14)
Pass through two new fields the host agent (v0.31.0) now returns on `/disks`, so they reach
+11
View File
@@ -666,6 +666,17 @@ not just those with HDD data. Non-HDD apps can configure destination, method, an
> `FormatResult.{role,needs_confirmation,durable_id}`; `ErrNeedsConfirmation` (user-data) vs
> `ErrFormatRefused` (system/backup). `FormatResult.PendingOp.OpsignCommand()` for the operator path.
> - The **`StoragePath` registry** (`settings.go`: `AddStoragePath`/default/schedulable/label) is unchanged.
> - **(v0.64.0) `AutoDiscoverStoragePaths` is now ADDITIVE** — it no longer bails when the registry is
> non-empty; instead it registers only deployed-app paths missing from the registry. It never removes
> or mutates an existing entry, never re-adds or reactivates a path already present in ANY state
> (incl. `Decommissioned`), and never flips `IsDefault` (a new path defaults only if the registry has
> no default at all). This is NOT auto-register-on-attach (that recommendation was rejected — manual
> enrollment is by design); it only picks up drives that deployed apps already reference.
> - **(v0.64.0) `InferStorageLabel` disambiguates the internal SSD** — a path whose basename is the
> `felhom-data` namespace dir (the internal system volume, e.g. `/mnt/sys_drive/felhom-data`) now reads
> **"Belső SSD (rendszer)"** instead of the colliding "Tárhely (felhom-data)". Model-A user drives
> register their MOUNT ROOT (e.g. `/mnt/felhom-usb`), never `.../felhom-data`, so this can't mislabel a
> user drive. Still overridable via `SetStorageLabel`.
> - **Migration** (drive + per-stack) is **deferred** to its own slice (buttons disabled "Hamarosan").
>
> The privileged controller-side disk subsections **below are historical** (the `internal/storage/*` scan/
+74 -38
View File
@@ -9,6 +9,8 @@ import (
"strings"
"sync"
"time"
"gitea.dooplex.hu/admin/felhom-controller/internal/appbackup"
)
// Settings holds customer-modifiable overrides and cached state.
@@ -39,8 +41,8 @@ type Settings struct {
// Hub verification state
HubVerified bool `json:"hub_verified,omitempty"`
HubVerifiedAt string `json:"hub_verified_at,omitempty"` // RFC3339
HubLastCheck string `json:"hub_last_check,omitempty"` // RFC3339
HubVerifiedAt string `json:"hub_verified_at,omitempty"` // RFC3339
HubLastCheck string `json:"hub_last_check,omitempty"` // RFC3339
// Recovery credentials (saved from setup wizard input)
RetrievalPassword string `json:"retrieval_password,omitempty"`
@@ -59,7 +61,7 @@ type Settings struct {
type IntegrationState struct {
Enabled bool `json:"enabled"`
EnabledAt string `json:"enabled_at,omitempty"` // RFC3339
Status string `json:"status,omitempty"` // "active", "error", "disabled", "provider_stopped", "target_unavailable"
Status string `json:"status,omitempty"` // "active", "error", "disabled", "provider_stopped", "target_unavailable"
LastError string `json:"last_error,omitempty"`
}
@@ -80,8 +82,8 @@ type CrossDriveBackup struct {
Schedule string `json:"schedule"` // "daily", "weekly", "manual"
// Runtime state (updated by backup runner, persisted for display)
LastRun string `json:"last_run,omitempty"` // RFC3339
LastStatus string `json:"last_status,omitempty"` // "ok", "error", "running"
LastRun string `json:"last_run,omitempty"` // RFC3339
LastStatus string `json:"last_status,omitempty"` // "ok", "error", "running"
LastError string `json:"last_error,omitempty"`
LastDuration string `json:"last_duration,omitempty"` // "2m34s"
LastSizeHuman string `json:"last_size_human,omitempty"` // "1.2 GB"
@@ -95,17 +97,17 @@ type CrossDriveBackup struct {
// StoragePath represents a registered external storage location.
type StoragePath struct {
Path string `json:"path"` // e.g., "/mnt/hdd_1"
Label string `json:"label,omitempty"` // e.g., "Külső HDD 1TB"
IsDefault bool `json:"is_default,omitempty"` // new apps use this by default
Schedulable bool `json:"schedulable"` // whether new apps can be deployed here
AddedAt string `json:"added_at"` // RFC3339
Disconnected bool `json:"disconnected,omitempty"` // true when drive detected as disconnected
DisconnectedAt string `json:"disconnected_at,omitempty"` // RFC3339 timestamp of disconnect detection
StoppedStacks []string `json:"stopped_stacks,omitempty"` // stacks auto-stopped on disconnect
Decommissioned bool `json:"decommissioned,omitempty"` // true when drive data migrated to another
DecommissionedAt string `json:"decommissioned_at,omitempty"` // RFC3339 timestamp
MigratedTo string `json:"migrated_to,omitempty"` // path of target drive
Path string `json:"path"` // e.g., "/mnt/hdd_1"
Label string `json:"label,omitempty"` // e.g., "Külső HDD 1TB"
IsDefault bool `json:"is_default,omitempty"` // new apps use this by default
Schedulable bool `json:"schedulable"` // whether new apps can be deployed here
AddedAt string `json:"added_at"` // RFC3339
Disconnected bool `json:"disconnected,omitempty"` // true when drive detected as disconnected
DisconnectedAt string `json:"disconnected_at,omitempty"` // RFC3339 timestamp of disconnect detection
StoppedStacks []string `json:"stopped_stacks,omitempty"` // stacks auto-stopped on disconnect
Decommissioned bool `json:"decommissioned,omitempty"` // true when drive data migrated to another
DecommissionedAt string `json:"decommissioned_at,omitempty"` // RFC3339 timestamp
MigratedTo string `json:"migrated_to,omitempty"` // path of target drive
}
// NotificationPrefs holds customer notification preferences.
@@ -144,10 +146,10 @@ type GeoRestriction struct {
AppOverrides map[string]AppGeoOverride `json:"app_overrides,omitempty"`
// Sync state (updated by geo sync manager)
LastSync string `json:"last_sync,omitempty"` // RFC3339
LastSync string `json:"last_sync,omitempty"` // RFC3339
LastSyncError string `json:"last_sync_error,omitempty"`
ZoneID string `json:"zone_id,omitempty"` // cached Cloudflare zone ID
RulesetID string `json:"ruleset_id,omitempty"` // cached Cloudflare ruleset ID
ZoneID string `json:"zone_id,omitempty"` // cached Cloudflare zone ID
RulesetID string `json:"ruleset_id,omitempty"` // cached Cloudflare ruleset ID
}
// AppGeoOverride holds per-app country override.
@@ -157,7 +159,7 @@ type AppGeoOverride struct {
// DBValidationCache holds cached DB dump validation results.
type DBValidationCache struct {
ValidatedAt string `json:"validated_at"` // RFC3339
ValidatedAt string `json:"validated_at"` // RFC3339
TableCount int `json:"table_count"`
HasHeader bool `json:"has_header"`
Error string `json:"error,omitempty"`
@@ -578,9 +580,19 @@ func (s *Settings) SetStorageLabel(path, label string) error {
return fmt.Errorf("storage path %q not found", path)
}
// AutoDiscoverStoragePaths scans for HDD_PATH values and registers them if none exist.
// discoveredPaths are pre-scanned HDD_PATH values from deployed apps' app.yaml.
// fallbackHDDPath is the legacy controller.yaml paths.hdd_path (may be empty).
// AutoDiscoverStoragePaths scans for HDD_PATH values and registers any that are not
// already in the registry. It is ADDITIVE: pre-existing entries are never removed,
// modified, or reactivated.
// - discoveredPaths are pre-scanned HDD_PATH values from deployed apps' app.yaml.
// - fallbackHDDPath is the legacy controller.yaml paths.hdd_path (may be empty).
//
// Invariants:
// - A path already present in the registry IN ANY STATE (including a Decommissioned
// soft-marked entry) is SKIPPED — never re-added and never re-activated.
// - A manually-added path is never removed or modified.
// - IsDefault is never flipped on an existing entry. A newly-discovered path becomes
// default ONLY if the registry currently has no default at all (and then only the
// first such new path).
func (s *Settings) AutoDiscoverStoragePaths(discoveredPaths []string, fallbackHDDPath string, logger *log.Logger) {
s.mu.Lock()
defer s.mu.Unlock()
@@ -589,53 +601,77 @@ func (s *Settings) AutoDiscoverStoragePaths(discoveredPaths []string, fallbackHD
s.log.Printf("[DEBUG] [settings] AutoDiscoverStoragePaths discovered=%v fallback=%q existing=%d", discoveredPaths, fallbackHDDPath, len(s.StoragePaths))
}
if len(s.StoragePaths) > 0 {
return // already configured
// Index existing paths (in ANY state) and whether a default already exists.
existing := make(map[string]bool, len(s.StoragePaths))
hasDefault := false
for i := range s.StoragePaths {
existing[filepath.Clean(s.StoragePaths[i].Path)] = true
if s.StoragePaths[i].IsDefault {
hasDefault = true
}
}
// Build the de-duplicated, cleaned candidate list (discovered first, then fallback).
seen := make(map[string]bool)
var ordered []string
for _, p := range discoveredPaths {
cleaned := filepath.Clean(p)
if cleaned != "" && !seen[cleaned] {
if cleaned != "" && cleaned != "." && !seen[cleaned] {
seen[cleaned] = true
ordered = append(ordered, cleaned)
}
}
if fallbackHDDPath != "" {
cleaned := filepath.Clean(fallbackHDDPath)
if !seen[cleaned] {
if cleaned != "" && cleaned != "." && !seen[cleaned] {
seen[cleaned] = true
ordered = append(ordered, cleaned)
}
}
for i, path := range ordered {
added := 0
for _, path := range ordered {
if existing[path] {
continue // already registered in some state — never re-add or reactivate
}
sp := StoragePath{
Path: path,
Label: InferStorageLabel(path),
IsDefault: i == 0,
IsDefault: !hasDefault, // first newly-added path defaults only if none exists yet
Schedulable: true,
AddedAt: time.Now().UTC().Format(time.RFC3339),
}
if sp.IsDefault {
hasDefault = true // don't promote a second new path
}
s.StoragePaths = append(s.StoragePaths, sp)
existing[path] = true
added++
}
if len(s.StoragePaths) > 0 {
if err := s.save(); err != nil {
logger.Printf("[ERROR] [settings] Failed to save auto-discovered storage paths: %v", err)
return
}
logger.Printf("[INFO] [settings] Auto-discovered %d storage path(s)", len(s.StoragePaths))
for _, sp := range s.StoragePaths {
logger.Printf("[INFO] [settings] %s (%s) default=%v", sp.Path, sp.Label, sp.IsDefault)
}
if added == 0 {
return // nothing new to register
}
if err := s.save(); err != nil {
logger.Printf("[ERROR] [settings] Failed to save auto-discovered storage paths: %v", err)
return
}
logger.Printf("[INFO] [settings] Auto-discovered %d new storage path(s)", added)
for _, sp := range s.StoragePaths {
logger.Printf("[INFO] [settings] %s (%s) default=%v decommissioned=%v", sp.Path, sp.Label, sp.IsDefault, sp.Decommissioned)
}
}
// InferStorageLabel generates a human-readable label for a storage path.
func InferStorageLabel(path string) string {
base := filepath.Base(path)
// The internal system volume's data path ends in the felhom-data namespace dir
// (e.g. /mnt/sys_drive/felhom-data) — Model-A user drives register their MOUNT ROOT
// (e.g. /mnt/felhom-usb), never .../felhom-data, so this can't mislabel a user drive.
if base == appbackup.FelhomDataDir {
return "Belső SSD (rendszer)"
}
if strings.HasPrefix(base, "hdd") || strings.HasPrefix(base, "ssd") || strings.HasPrefix(base, "usb") {
return fmt.Sprintf("Külső tárhely (%s)", base)
}
@@ -0,0 +1,229 @@
package settings
import (
"io"
"log"
"path/filepath"
"reflect"
"testing"
)
// newTestSettings returns a Settings backed by a writable temp file so save() works.
func newTestSettings(t *testing.T, paths []StoragePath) *Settings {
t.Helper()
logger := log.New(io.Discard, "", 0)
s, err := Load(filepath.Join(t.TempDir(), "settings.json"), logger)
if err != nil {
t.Fatalf("Load: %v", err)
}
s.StoragePaths = paths
return s
}
// TestAutoDiscoverStoragePaths_Additive covers the additive registration behaviour and
// its invariants: existing entries are never mutated, decommissioned entries are never
// reactivated, and IsDefault is never promoted over an existing default.
func TestAutoDiscoverStoragePaths_Additive(t *testing.T) {
logger := log.New(io.Discard, "", 0)
tests := []struct {
name string
existing []StoragePath
discovered []string
fallback string
wantPaths []string // expected Path set after discovery (order-insensitive)
wantNewPath string // a path expected to be newly added (may be "")
// assertions run against the resulting registry
check func(t *testing.T, s *Settings, before []StoragePath)
}{
{
name: "non-empty registry registers only the missing deployed path",
existing: []StoragePath{
{Path: "/mnt/felhom-usb", Label: "Külső tárhely (felhom-usb)", IsDefault: true, Schedulable: true, AddedAt: "2026-01-01T00:00:00Z"},
},
discovered: []string{"/mnt/felhom-usb", "/mnt/hdd_2"}, // existing + a new one
wantPaths: []string{"/mnt/felhom-usb", "/mnt/hdd_2"},
wantNewPath: "/mnt/hdd_2",
check: func(t *testing.T, s *Settings, before []StoragePath) {
// pre-existing entry must be byte-identical (incl. IsDefault unchanged)
got := findPath(s, "/mnt/felhom-usb")
if got == nil {
t.Fatalf("pre-existing path vanished")
}
if !reflect.DeepEqual(*got, before[0]) {
t.Errorf("pre-existing entry mutated:\n before=%+v\n after =%+v", before[0], *got)
}
// the newly-added path must NOT have stolen default
nw := findPath(s, "/mnt/hdd_2")
if nw == nil {
t.Fatalf("new path /mnt/hdd_2 not registered")
}
if nw.IsDefault {
t.Errorf("new path promoted to default over existing default")
}
if !nw.Schedulable {
t.Errorf("new path should be schedulable")
}
},
},
{
name: "registry with no default lets first new path become default",
existing: []StoragePath{
{Path: "/mnt/felhom-usb", Label: "x", IsDefault: false, Schedulable: true, AddedAt: "2026-01-01T00:00:00Z"},
},
discovered: []string{"/mnt/hdd_2", "/mnt/hdd_3"},
wantPaths: []string{"/mnt/felhom-usb", "/mnt/hdd_2", "/mnt/hdd_3"},
wantNewPath: "/mnt/hdd_2",
check: func(t *testing.T, s *Settings, before []StoragePath) {
d2 := findPath(s, "/mnt/hdd_2")
d3 := findPath(s, "/mnt/hdd_3")
if d2 == nil || d3 == nil {
t.Fatalf("new paths not registered")
}
if !d2.IsDefault {
t.Errorf("first new path should become default when registry has none")
}
if d3.IsDefault {
t.Errorf("only one new path may become default")
}
},
},
{
name: "empty registry behaves like the original (first becomes default)",
existing: nil,
discovered: []string{"/mnt/felhom-usb", "/mnt/hdd_2"},
wantPaths: []string{"/mnt/felhom-usb", "/mnt/hdd_2"},
wantNewPath: "/mnt/felhom-usb",
check: func(t *testing.T, s *Settings, before []StoragePath) {
first := findPath(s, "/mnt/felhom-usb")
if first == nil || !first.IsDefault {
t.Errorf("first discovered path should be default in an empty registry")
}
},
},
{
name: "fallback path registered when missing",
existing: []StoragePath{
{Path: "/mnt/felhom-usb", Label: "x", IsDefault: true, Schedulable: true, AddedAt: "2026-01-01T00:00:00Z"},
},
discovered: nil,
fallback: "/mnt/legacy_hdd",
wantPaths: []string{"/mnt/felhom-usb", "/mnt/legacy_hdd"},
wantNewPath: "/mnt/legacy_hdd",
check: nil,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
s := newTestSettings(t, cloneStoragePaths(tc.existing))
before := cloneStoragePaths(tc.existing)
s.AutoDiscoverStoragePaths(tc.discovered, tc.fallback, logger)
// Normalize with filepath.Clean so comparisons hold on both Linux (the deploy
// target) and Windows (the dev machine), where Clean uses backslashes.
gotSet := map[string]bool{}
for _, sp := range s.StoragePaths {
gotSet[filepath.Clean(sp.Path)] = true
}
if len(gotSet) != len(tc.wantPaths) {
t.Fatalf("path count = %d, want %d (%v)", len(gotSet), len(tc.wantPaths), pathList(s))
}
for _, w := range tc.wantPaths {
if !gotSet[filepath.Clean(w)] {
t.Errorf("missing expected path %q (have %v)", w, pathList(s))
}
}
if tc.wantNewPath != "" && findPath(s, tc.wantNewPath) == nil {
t.Errorf("expected new path %q to be registered", tc.wantNewPath)
}
if tc.check != nil {
tc.check(t, s, before)
}
})
}
}
// TestAutoDiscoverStoragePaths_DecommissionedNotReactivated is the companion guard: a
// decommissioned path that is still referenced by a deployed app must NOT be re-added or
// reactivated. This test FAILS if the skip-by-presence guard is removed.
func TestAutoDiscoverStoragePaths_DecommissionedNotReactivated(t *testing.T) {
logger := log.New(io.Discard, "", 0)
existing := []StoragePath{
{Path: "/mnt/old_hdd", Label: "Külső tárhely (old_hdd)", IsDefault: false, Schedulable: true, AddedAt: "2026-01-01T00:00:00Z", Decommissioned: true, DecommissionedAt: "2026-02-01T00:00:00Z", MigratedTo: "/mnt/felhom-usb"},
{Path: "/mnt/felhom-usb", Label: "x", IsDefault: true, Schedulable: true, AddedAt: "2026-01-01T00:00:00Z"},
}
s := newTestSettings(t, cloneStoragePaths(existing))
before := cloneStoragePaths(existing)
// A deployed app still points at the decommissioned drive.
s.AutoDiscoverStoragePaths([]string{"/mnt/old_hdd", "/mnt/felhom-usb"}, "", logger)
if len(s.StoragePaths) != 2 {
t.Fatalf("path count changed: got %d want 2 (%v)", len(s.StoragePaths), pathList(s))
}
got := findPath(s, "/mnt/old_hdd")
if got == nil {
t.Fatalf("decommissioned path vanished")
}
if !got.Decommissioned {
t.Errorf("decommissioned path was REACTIVATED (skip-by-presence guard missing)")
}
if !reflect.DeepEqual(*got, before[0]) {
t.Errorf("decommissioned entry mutated:\n before=%+v\n after =%+v", before[0], *got)
}
}
func TestInferStorageLabel(t *testing.T) {
tests := []struct {
path string
want string
}{
{"/mnt/sys_drive/felhom-data", "Belső SSD (rendszer)"},
{"/var/lib/felhom/felhom-data", "Belső SSD (rendszer)"},
{"/mnt/felhom-usb", "Tárhely (felhom-usb)"}, // "felhom-usb" doesn't start with "usb"
{"/mnt/hdd_1", "Külső tárhely (hdd_1)"},
{"/mnt/ssd_data", "Külső tárhely (ssd_data)"},
{"/srv/backups", "Tárhely (backups)"},
}
for _, tc := range tests {
if got := InferStorageLabel(tc.path); got != tc.want {
t.Errorf("InferStorageLabel(%q) = %q, want %q", tc.path, got, tc.want)
}
}
}
// --- helpers ---
func cloneStoragePaths(in []StoragePath) []StoragePath {
if in == nil {
return nil
}
out := make([]StoragePath, len(in))
copy(out, in)
for i := range out {
if in[i].StoppedStacks != nil {
out[i].StoppedStacks = append([]string(nil), in[i].StoppedStacks...)
}
}
return out
}
func findPath(s *Settings, path string) *StoragePath {
want := filepath.Clean(path)
for i := range s.StoragePaths {
if filepath.Clean(s.StoragePaths[i].Path) == want {
return &s.StoragePaths[i]
}
}
return nil
}
func pathList(s *Settings) []string {
var out []string
for _, sp := range s.StoragePaths {
out = append(out, sp.Path)
}
return out
}