0.12.2 - Bug 4: Restore Section ("Visszaállítás") Simplification — TASK.md
This commit is contained in:
@@ -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}}`:
|
||||
```
|
||||
<span class="version">v{{.Version}}</span> ← line 17845 (layout.html)
|
||||
<span class="settings-value mono">v{{.Version}}</span> ← 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
|
||||
}
|
||||
```
|
||||
|
||||
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
|
||||
}
|
||||
return s.cfg.Paths.HDDPath
|
||||
|
||||
writeJSON(w, http.StatusOK, apiResponse{OK: true, Data: snapshots})
|
||||
}
|
||||
```
|
||||
|
||||
**Affected files:**
|
||||
- `internal/web/handlers.go` — `primaryHDDPath()` function
|
||||
- `cmd/controller/main.go` — metrics collector HDD path selection
|
||||
|
||||
---
|
||||
|
||||
## 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 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 filtered
|
||||
}
|
||||
```
|
||||
|
||||
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.
|
||||
**2. Auto-stop/restart in RestoreApp** (`internal/backup/restore.go`)
|
||||
|
||||
**Affected files:**
|
||||
- `internal/web/handlers.go` — `buildAppBackupRows()` status logic
|
||||
```go
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
## Bug 4: Restore section ("Visszaállítás") UX rethink
|
||||
// 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
|
||||
}
|
||||
|
||||
**Severity:** Medium (UX complexity)
|
||||
**Location:** Backup page restore section in `backups.html`
|
||||
// Restart after successful restore
|
||||
if m.stackProvider != nil {
|
||||
m.logger.Printf("[INFO] Restarting %s after restore...", stackName)
|
||||
m.stackProvider.StartStack(stackName)
|
||||
}
|
||||
|
||||
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
|
||||
|
||||
**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
|
||||
|
||||
**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
|
||||
|
||||
**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)
|
||||
|
||||
---
|
||||
|
||||
## Bug 5: Restic key buttons barely readable
|
||||
|
||||
**Severity:** Low (cosmetic)
|
||||
**Location:** Backup page, "Titkosítási kulcs" section (line 16660-16661 in `backups.html`)
|
||||
|
||||
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
|
||||
<button type="button" class="btn btn-sm" onclick="toggleResticPw()">Megjelenítés</button>
|
||||
<button type="button" class="btn btn-sm" onclick="copyResticPw()">Másolás</button>
|
||||
```
|
||||
|
||||
**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 */
|
||||
return nil
|
||||
}
|
||||
```
|
||||
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
|
||||
### Frontend changes
|
||||
|
||||
**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
|
||||
|
||||
---
|
||||
|
||||
## Priority / Implementation Order
|
||||
## Design Decisions
|
||||
|
||||
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
|
||||
**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)`
|
||||
|
||||
**D2: Zero filtered snapshots** → Show inline message instead of empty dropdown: "Még nincs mentés felhasználói adattal." (shortened from original)
|
||||
|
||||
**D3: Auto-stop/restart** → Yes, stacks must be stopped during restore. Check if StackProvider interface already exposes Stop/Start; extend it if not.
|
||||
|
||||
**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.
|
||||
|
||||
**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.
|
||||
|
||||
---
|
||||
|
||||
## Files to modify
|
||||
|
||||
| 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 |
|
||||
|
||||
## 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)
|
||||
Reference in New Issue
Block a user