diff --git a/TASK.md b/TASK.md index 8b7d347..b0e8062 100644 --- a/TASK.md +++ b/TASK.md @@ -1,200 +1,192 @@ -# 0.12.1 Bug Fixes +# 0.12.2 - Bug 4: Restore Section ("Visszaállítás") Simplification — TASK.md -## Bug 1: Version displays "vv0.12.0" +## Current State -**Severity:** Low (cosmetic) -**Location:** Two templates with hardcoded `v` prefix +### Backup architecture (what goes where) -The sidebar and settings page templates both prepend `v` to `{{.Version}}`: -``` -v{{.Version}} ← line 17845 (layout.html) -v{{.Version}} ← line 19061 (settings.html) -``` +**Restic snapshots** (daily, on SSD): +- `/opt/docker/stacks/` — all Docker configs, compose files, app.yaml +- `/opt/docker/db-dumps/` — DB dump SQL files +- `/opt/docker/felhom-controller/controller.yaml` +- App HDD data paths (if backup enabled) — e.g., `/mnt/hdd_1/storage/immich/` -`build.sh` takes VERSION as its first argument (`VERSION="${1:-dev}"`), and Viktor passes `v0.12.0` → the template renders `vv0.12.0`. +**Cross-drive backups** (per-app, rsync/restic to secondary drive): +- Copies app user data between drives (e.g., HDD→SSD or SSD→HDD) +- Separate from restic — not part of snapshot restore -**Fix (template side — recommended):** -Replace `v{{.Version}}` with `{{.Version}}` in both locations. This way the version displayed matches the git tag exactly. The `v` prefix comes from the tag/build arg naturally. +**Key detail:** `SnapshotInfo.Paths []string` from `restic snapshots --json` includes the backup source paths. Snapshots created BEFORE an app's HDD backup was enabled won't have that app's HDD paths listed. This can be used for filtering. -**Affected files:** -- `internal/web/templates/layout.html` — sidebar footer -- `internal/web/templates/settings.html` — version info row +### Current restore UI flow (backups.html Section 7) +1. **App dropdown** — only apps with `HasHDDData && BackupEnabled` (filters out auto-only apps) +2. **Snapshot dropdown** — loads ALL snapshots from `/api/backup/snapshots` (no filtering by app) +3. **Path display** — shows HDD paths that will be restored (technical, confusing for customers) +4. **Warning block** — "FELÜLÍRJA a jelenlegi adatait... NEM vonható vissza!... Javasoljuk az alkalmazás leállítását" +5. **Confirmation checkbox** — "Megértettem, visszaállítás saját felelősségre" +6. **Restore button** → POST `/backup/restore` with `stack_name` + `snapshot_id` + +### Current restore backend (`RestoreApp` in `restore.go`) +- Validates backup enabled + snapshot exists +- Resolves app's HDD paths via `GetStackHDDMounts()` +- Runs `restic restore {snapshot} --target / --include {path1} --include {path2}` +- Only restores HDD data — does NOT restore Docker configs or DB dumps --- -## Bug 2: "Külső HDD" stats show same values as SSD +## Problems -**Severity:** Medium (wrong data displayed) -**Location:** Dashboard and Backup page storage bars; also metrics collector in `main.go` +1. **Snapshot selector shows ALL snapshots** — user can pick one that predates their app's backup setup → restore fails or does nothing +2. **Path display is confusing** — customers don't need to see `/mnt/hdd_1/storage/immich/` +3. **Warning is intimidating** — multiple strong statements, feels dangerous +4. **No auto-stop** — just "recommended" to stop the app, user can forget +5. **DB-only restore** is implicitly possible (pick old snapshot) but not clear what it does -**Setup on demo-felhom.eu:** -- Internal SSD (512G): system disk, mounted at `/`. `/mnt/hdd_placeholder` is a normal folder on this. -- External USB HDD (1TB): mounted at `/mnt/hdd_1` (ext4, `/host-dev/sdb1`) -- Two storage paths registered (visible in Settings > Adattárolók): - - `/mnt/hdd_placeholder` — "Rendszermeghajtón" badge, Aktív, 19.8 GB / 460.2 GB - - `/mnt/hdd_1` — "Alapértelmezett" + "Aktív" badge, 0.0 GB / 915.8 GB -- No `hdd_path` in `controller.yaml`, no `HDD_PATH` in any `app.yaml` -- The Settings page (Adattárolók section) shows **correct** values for both paths +--- -**Current behavior:** -- Dashboard: "Külső HDD" shows 19.8 GB / 460 GB — same as SSD (wrong!) -- Backup page: Same duplicate in "Tárhely áttekintés" +## Proposed Design -**Root cause:** -`primaryHDDPath()` (line 15164) returns `paths[0].Path` — the **first** element in the slice, which happens to be `/mnt/hdd_placeholder` (on the SSD). Then `system.GetInfo("/mnt/hdd_placeholder", ...)` calls `syscall.Statfs` on a directory that lives on the root filesystem → returns SSD stats. +### Simplified UI (3 elements instead of 6) +``` +┌─────────────────────────────────────────────────┐ +│ Visszaállítás │ +│ │ +│ Alkalmazás: [▼ Immich ] │ +│ Pillanatkép: [▼ 2026-02-17 03:00 (a3f2b1) ] │ +│ │ +│ ⚠ A visszaállítás felülírja az alkalmazás │ +│ jelenlegi adatait a kiválasztott mentés │ +│ állapotával. Az alkalmazás a folyamat során │ +│ automatikusan leáll és újraindul. │ +│ │ +│ ☐ Megértettem, visszaállítás indítása │ +│ │ +│ [ Visszaállítás indítása ] │ +└─────────────────────────────────────────────────┘ +``` + +### Changes + +| # | What | Details | +|---|------|---------| +| 1 | **Filter snapshots by app** | When app selected, `/api/backup/snapshots?stack={name}` returns only snapshots whose `Paths` contain at least one of the app's HDD paths. Show snapshot time in human-friendly format, no raw IDs | +| 2 | **Remove path display** | Customer doesn't need to see mount paths. The restore handler knows what to include | +| 3 | **Single calm warning** | Replace 3-line warning + separate checkbox label with one concise block | +| 4 | **Auto-stop + restart** | Restore handler stops the app's containers before restore, restarts after. Eliminates "javasoljuk leállítását" advice | +| 5 | **Hide DB-only restore** | Not exposed in UI — support contacts Viktor for CLI-level `restic restore` | + +--- + +## Implementation Plan + +### Backend changes + +**1. Filtered snapshots API** (`internal/web/api_router.go` or `router.go`) + +Extend `GET /api/backup/snapshots` with optional `?stack={name}` query param: ```go -// current — broken -func (s *Server) primaryHDDPath() string { - if paths := s.settings.GetStoragePaths(); len(paths) > 0 { - return paths[0].Path // ← returns first element, NOT the default! +func (r *Router) snapshotsHandler(w http.ResponseWriter, req *http.Request) { + snapshots, err := r.backupMgr.ListSnapshots(50) + // ... existing error handling ... + + // Filter by stack if requested + if stackName := req.URL.Query().Get("stack"); stackName != "" { + hddMounts := r.stackProvider.GetStackHDDMounts(stackName) + if len(hddMounts) > 0 { + snapshots = filterSnapshotsByPaths(snapshots, hddMounts) + } } - return s.cfg.Paths.HDDPath + + writeJSON(w, http.StatusOK, apiResponse{OK: true, Data: snapshots}) } -``` -But `GetDefaultStoragePath()` (line 8946) already exists and returns the path with `IsDefault: true` — which is `/mnt/hdd_1` (the actual HDD). - -**Same bug in `main.go`** (line 2403-2405) for the metrics collector: -```go -metricsHDDPath := cfg.Paths.HDDPath -if paths := sett.GetStoragePaths(); len(paths) > 0 { - metricsHDDPath = paths[0].Path // ← same bug -} -``` - -**Fix:** -1. `primaryHDDPath()` → use `s.settings.GetDefaultStoragePath()` instead of `paths[0].Path` -2. `main.go` metrics collector → use `sett.GetDefaultStoragePath()` instead of `paths[0].Path` -3. Fallback: if no default is set, fall back to `cfg.Paths.HDDPath` (existing behavior) - -```go -// fixed -func (s *Server) primaryHDDPath() string { - if p := s.settings.GetDefaultStoragePath(); p != "" { - return p +func filterSnapshotsByPaths(snapshots []backup.SnapshotInfo, requiredPaths []string) []backup.SnapshotInfo { + var filtered []backup.SnapshotInfo + for _, snap := range snapshots { + for _, req := range requiredPaths { + for _, sp := range snap.Paths { + if strings.HasPrefix(req, sp) || strings.HasPrefix(sp, req) { + filtered = append(filtered, snap) + goto next + } + } + } + next: } - return s.cfg.Paths.HDDPath + return filtered } ``` -**Affected files:** -- `internal/web/handlers.go` — `primaryHDDPath()` function -- `cmd/controller/main.go` — metrics collector HDD path selection +**2. Auto-stop/restart in RestoreApp** (`internal/backup/restore.go`) ---- - -## Bug 3: Gokapi/Mealie show gray ("auto") status — should be green - -**Severity:** Medium (misleading status) -**Location:** `buildAppBackupRows()` in `internal/web/handlers.go` (line 14302-14367) - -Apps without HDD data (like Gokapi, Mealie) always get `status = "auto"` (gray dot). From the user's perspective, these apps ARE fully backed up — their Docker volumes are included in the restic snapshot, and their configs are under `/opt/docker/stacks/`. The gray dot misleadingly suggests they're not backed up at all. - -**Current logic (line 14365-14368):** ```go -} else { - // No HDD data — fully automatic - row.Status = "auto" - row.StatusText = "Automatikus mentés (nincs felhasználói adat)" -} -``` - -**Desired behavior:** -- Apps with no HDD data whose Docker volumes are successfully backed up → **green** dot with text "Mentés rendben" -- Apps with no HDD data where last volume backup failed → **yellow** dot -- Keep "auto" only if there's genuinely no backup info available (edge case) - -**Fix approach:** -Replace the `else` block (line 14365-14368): -```go -} else { - // No HDD data — everything backed up automatically via restic - if volumeLastStatus == "ok" { - row.Status = "green" - row.StatusText = "Mentés rendben" - } else if volumeLastStatus == "error" { - row.Status = "yellow" - row.StatusText = "Kötetek mentése sikertelen" - } else { - row.Status = "auto" - row.StatusText = "Automatikus mentés" +func (m *Manager) RestoreApp(stackName, snapshotID string) error { + // ... existing validation ... + + // Stop the app's containers before restore + if m.stackProvider != nil { + m.logger.Printf("[INFO] Stopping %s for restore...", stackName) + if err := m.stackProvider.StopStack(stackName); err != nil { + m.logger.Printf("[WARN] Could not stop %s: %v (proceeding anyway)", stackName, err) + } } + + // Execute restore (existing logic) + if err := m.restic.RestoreAppData(snapshotID, hddMounts); err != nil { + // Try to restart even on failure + m.stackProvider.StartStack(stackName) + return err + } + + // Restart after successful restore + if m.stackProvider != nil { + m.logger.Printf("[INFO] Restarting %s after restore...", stackName) + m.stackProvider.StartStack(stackName) + } + + return nil } ``` -Additionally, if the app has a DB and the DB backup was successful, that should also contribute to green status. The existing DB error check at line 14372 already degrades to yellow, so this is covered. +### Frontend changes -**Affected files:** -- `internal/web/handlers.go` — `buildAppBackupRows()` status logic +**3. Template simplification** (`backups.html` Section 7) +- Remove `restore-paths` div entirely +- Replace warning text with single concise block +- Keep app dropdown + snapshot dropdown + confirm checkbox + button +- Update `onRestoreAppChange()` JS to call `/api/backup/snapshots?stack={name}` + +**4. Snapshot display format** +- Show: `2026-02-17 vasárnap 03:00` instead of `a3f2b1 — 2026.02.17. 3:00:00` +- Keep snapshot ID in `option.value`, show human time in label --- -## Bug 4: Restore section ("Visszaállítás") UX rethink +## Design Decisions -**Severity:** Medium (UX complexity) -**Location:** Backup page restore section in `backups.html` +**D1: Snapshot dropdown format** → Show `date (id)` — human-friendly time with short ID in parentheses for support debugging. E.g. `2026-02-17 vasárnap 03:00 (a3f2b1)` -Now that user data folders are also backed up via cross-drive (rsync), the restore section needs rethinking. Currently: -- DB backup is a separate snapshot tied to user data -- Multiple warnings accumulate, making the UI intimidating -- User must understand the difference between DB-only and full restore +**D2: Zero filtered snapshots** → Show inline message instead of empty dropdown: "Még nincs mentés felhasználói adattal." (shortened from original) -**Decision (from Viktor):** -- If user data is backed up, the "Pillanatkép" selector should only show snapshots that include user data → restores everything together (most failsafe) -- Daily DB-only backups remain useful as intermediate recovery points, but DB-only restore is an advanced task → contact support -- This simplifies the UI: one dropdown, one action, one clear warning +**D3: Auto-stop/restart** → Yes, stacks must be stopped during restore. Check if StackProvider interface already exposes Stop/Start; extend it if not. -**Implementation plan:** -1. Snapshot selector: Filter to show only snapshots containing user data paths when cross-drive is configured -2. Restore action: Restores both Docker volumes/configs AND user data in one operation -3. Simplify warnings: Single clear warning about data overwrite, remove redundant ones -4. DB-only restore: Not exposed in UI — support-level operation via CLI +**D4: Progress indication** → Keep current simple behavior (button text change + page redirect) for v0.13.0. Async polling (like backup progress) is a future enhancement. -**Note:** This is a design change, not a bugfix. Needs more detailed specification before implementation. Possibly v0.13.0 scope. - -**Affected files:** -- `internal/web/handlers.go` — restore handler logic -- `internal/web/templates/backups.html` — restore section template -- `internal/backup/restore.go` — restore logic (if separate) +**D5: Restore scope** → User data (HDD paths) only. Docker configs and DB dumps are NOT restored — that's a disaster recovery task handled via CLI/support. --- -## Bug 5: Restic key buttons barely readable +## Files to modify -**Severity:** Low (cosmetic) -**Location:** Backup page, "Titkosítási kulcs" section (line 16660-16661 in `backups.html`) +| File | Changes | +|------|---------| +| `internal/web/api_router.go` (or `router.go`) | Add `?stack=` filter to snapshots endpoint | +| `internal/backup/restore.go` | Add auto-stop/restart logic | +| `internal/backup/types.go` | Add `filterSnapshotsByPaths` helper if needed | +| `internal/web/templates/backups.html` | Simplify restore section HTML | +| `internal/web/templates/backups.html` (JS) | Update `onRestoreAppChange()` to pass stack param | +| `internal/stacks/` (maybe) | Ensure StackProvider exposes Stop/Start if needed for Q3 | -The "Megjelenítés" and "Másolás" buttons use `class="btn btn-sm"` without a color variant. The `.btn` base class sets `color: #fff` (white text) but has **no background color**. The browser default background (light gray) makes white text nearly invisible on the dark theme. - -**Current HTML:** -```html - - -``` - -**Fix options:** -A. **Add a default background to `.btn` base class** — e.g. `background: var(--card-bg);` or `background: rgba(255,255,255,0.1);` — This fixes all unclassed buttons globally -B. **Add variant class to these specific buttons** — e.g. `class="btn btn-sm btn-secondary"` with a defined `.btn-secondary` style - -**Recommended: Option A** — add a sensible dark default to `.btn`: -```css -.btn { - /* existing properties... */ - background: rgba(255,255,255,0.1); /* subtle dark default */ -} -``` -This ensures any `btn` without a specific variant is still readable on dark backgrounds. - -**Affected files:** -- `internal/web/templates/style.css` (or wherever `.btn` is defined) — add default background - ---- - -## Priority / Implementation Order - -1. **Bug 1** (version `vv`) — trivial template fix, ~2 min -2. **Bug 5** (button contrast) — trivial CSS fix, ~2 min -3. **Bug 2** (HDD stats wrong path) — 2-line fix in `primaryHDDPath()` + `main.go`, ~5 min -4. **Bug 3** (gray → green status) — small logic change, ~10 min -5. **Bug 4** (restore rethink) — design decision needed, deferred to v0.13.0 \ No newline at end of file +## Estimated effort +- Backend: ~2 hours (snapshot filtering + auto-stop/restart + interface extension) +- Frontend: ~1 hour (template simplification + JS update) +- Testing: ~1 hour (restore flow with different apps/snapshots) \ No newline at end of file