181 lines
6.5 KiB
Markdown
181 lines
6.5 KiB
Markdown
# TASK.md — Cross-Drive Backup Validation Fix (v0.12.5)
|
||
|
||
## Prompt (copy-paste this into Claude Code)
|
||
|
||
```
|
||
Read TASK.md for the full plan. Apply all code changes described, then build and deploy.
|
||
After all fixes are done:
|
||
1. Run `go build ./...` and `go vet ./...` from the controller/ directory — fix any errors
|
||
2. Update CHANGELOG.md with a new entry at the top (session 41, v0.12.5)
|
||
3. Commit, build, and deploy following the workflow in CLAUDE.md
|
||
```
|
||
|
||
---
|
||
|
||
## Context
|
||
|
||
The cross-drive backup for Immich failed last night with:
|
||
```
|
||
Hiba: destination /mnt/hdd_placeholder is not a mount point (0s)
|
||
```
|
||
|
||
**Root cause:** `ValidateDestination()` in `crossdrive.go` hard-blocked non-mount-point
|
||
destinations. The `/mnt/hdd_placeholder` folder is on the internal SSD (not a separate mount),
|
||
so the device-ID check in `IsMountPoint()` returned false.
|
||
|
||
**Already fixed (in current working tree):** The mount-point check was changed from a hard
|
||
block to a logged warning (lines 172–174 of crossdrive.go). The backup will now proceed
|
||
for system-drive destinations.
|
||
|
||
**Remaining work:** Improve the disk space validation to be smarter about system-drive
|
||
destinations (don't fill up the OS drive).
|
||
|
||
---
|
||
|
||
## Fix 1: Smarter space checks in `ValidateDestination` (crossdrive.go)
|
||
|
||
**File:** `internal/backup/crossdrive.go`, lines 161–183
|
||
|
||
**Current code (already patched with the mount-point warning):**
|
||
```go
|
||
func (r *CrossDriveRunner) ValidateDestination(path string) error {
|
||
if path == "" {
|
||
return fmt.Errorf("destination path is empty")
|
||
}
|
||
if _, err := os.Stat(path); os.IsNotExist(err) {
|
||
return fmt.Errorf("destination %s does not exist", path)
|
||
}
|
||
if !system.IsMountPoint(path) {
|
||
r.logger.Printf("[WARN] Destination %s is not a separate mount point (system drive) — backup will proceed but data is not protected against drive failure", path)
|
||
}
|
||
if !system.IsWritable(path) {
|
||
return fmt.Errorf("destination %s is not writable", path)
|
||
}
|
||
di := system.GetDiskUsage(path)
|
||
if di != nil && di.AvailGB < 0.1 {
|
||
return fmt.Errorf("destination %s has insufficient free space (%.1f GB)", path, di.AvailGB)
|
||
}
|
||
return nil
|
||
}
|
||
```
|
||
|
||
**Required change:** Replace the flat 100MB space check (lines 178–181) with drive-type-aware logic:
|
||
|
||
```go
|
||
func (r *CrossDriveRunner) ValidateDestination(path string) error {
|
||
if path == "" {
|
||
return fmt.Errorf("destination path is empty")
|
||
}
|
||
if _, err := os.Stat(path); os.IsNotExist(err) {
|
||
return fmt.Errorf("destination %s does not exist", path)
|
||
}
|
||
onSystemDrive := !system.IsMountPoint(path)
|
||
if onSystemDrive {
|
||
r.logger.Printf("[WARN] Destination %s is not a separate mount point (system drive) — backup will proceed but data is not protected against drive failure", path)
|
||
}
|
||
if !system.IsWritable(path) {
|
||
return fmt.Errorf("destination %s is not writable", path)
|
||
}
|
||
if di := system.GetDiskUsage(path); di != nil {
|
||
if onSystemDrive {
|
||
// System drive: protect OS stability — require ≥10 GB free and <90% used
|
||
if di.AvailGB < 10 {
|
||
return fmt.Errorf("destination %s is on the system drive with only %.1f GB free — at least 10 GB required to protect OS stability", path, di.AvailGB)
|
||
}
|
||
if di.UsedPercent >= 90 {
|
||
return fmt.Errorf("destination %s is on the system drive at %.0f%% capacity — maximum 90%% allowed", path, di.UsedPercent)
|
||
}
|
||
} else {
|
||
// External drive: just ensure it's not completely full
|
||
if di.AvailGB < 0.1 {
|
||
return fmt.Errorf("destination %s has insufficient free space (%.1f GB free)", path, di.AvailGB)
|
||
}
|
||
}
|
||
}
|
||
return nil
|
||
}
|
||
```
|
||
|
||
**Update the function comment (lines 161–164) to match:**
|
||
```go
|
||
// ValidateDestination checks that the destination path exists, is writable,
|
||
// and has sufficient free space. System-drive destinations get stricter limits
|
||
// (≥10 GB free, <90% used) to protect OS stability; external drives just need
|
||
// ≥100 MB. Non-mount-point destinations are allowed with a logged warning.
|
||
```
|
||
|
||
---
|
||
|
||
## Fix 2: Align `CheckBackupDestination` thresholds for system drives (mounts_linux.go)
|
||
|
||
**File:** `internal/system/mounts_linux.go`, lines 134–186
|
||
|
||
The web UI's `CheckBackupDestination` currently applies the same disk thresholds (90% warn,
|
||
95% block) regardless of drive type. For system drives, it should use the same stricter
|
||
thresholds as the runner (90% block, 10 GB minimum) so the UI warning matches what the
|
||
runner will actually enforce.
|
||
|
||
**Required change:** In the Tier 4 block (lines 171–183), add system-drive-specific checks
|
||
BEFORE the generic percentage checks. The logic should be:
|
||
|
||
```go
|
||
// Tier 4: disk usage checks
|
||
if di := GetDiskUsage(path); di != nil {
|
||
h.UsedPercent = di.UsedPercent
|
||
h.FreeGB = di.AvailGB
|
||
if h.SystemDrive {
|
||
// System drive: stricter limits to protect OS stability
|
||
if di.AvailGB < 10 {
|
||
h.Warning = fmt.Sprintf("A rendszermeghajtón csak %.1f GB szabad — legalább 10 GB szükséges a rendszer stabilitásához!", di.AvailGB)
|
||
h.Blocked = true
|
||
h.Severity = "critical"
|
||
} else if di.UsedPercent >= 90 {
|
||
h.Warning = fmt.Sprintf("A rendszermeghajtó %.0f%%-ban megtelt — maximum 90%% megengedett.", di.UsedPercent)
|
||
h.Blocked = true
|
||
h.Severity = "critical"
|
||
}
|
||
// If neither triggers, keep the Tier 3 system-drive warning
|
||
} else {
|
||
// External drive: original thresholds
|
||
if di.UsedPercent >= 95 {
|
||
h.Warning = fmt.Sprintf("A mentési meghajtó megtelt (%.0f%% használt)!", di.UsedPercent)
|
||
h.Blocked = true
|
||
h.Severity = "critical"
|
||
} else if di.UsedPercent >= 90 {
|
||
h.Warning = fmt.Sprintf("A mentési meghajtó majdnem megtelt (%.0f%% használt).", di.UsedPercent)
|
||
h.Severity = "warning"
|
||
}
|
||
}
|
||
}
|
||
```
|
||
|
||
Note: the `else if di.UsedPercent >= 90 && h.Severity == "ok"` condition in the original
|
||
was preventing the 90% warning from overriding the system-drive warning. The new code
|
||
separates the branches cleanly — system drive gets its own block, external drive gets its own.
|
||
|
||
---
|
||
|
||
## Summary of thresholds
|
||
|
||
| Condition | System drive | External drive |
|
||
|-----------|-------------|----------------|
|
||
| Free space < 10 GB | **Block** | — |
|
||
| Usage ≥ 90% | **Block** | Warning |
|
||
| Usage ≥ 95% | (caught by 90%) | **Block** |
|
||
| Free space < 100 MB | (caught by 10GB) | **Block** |
|
||
|
||
---
|
||
|
||
## Files to modify
|
||
|
||
1. `internal/backup/crossdrive.go` — `ValidateDestination()` (Fix 1)
|
||
2. `internal/system/mounts_linux.go` — `CheckBackupDestination()` (Fix 2)
|
||
|
||
## Post-fix checklist
|
||
|
||
- [ ] `go build ./...` passes
|
||
- [ ] `go vet ./...` passes
|
||
- [ ] Update `CHANGELOG.md` — session 41, version **v0.12.5**, describe both fixes
|
||
- [ ] Commit, build on 192.168.0.180, deploy on 192.168.0.162
|
||
- [ ] Verify with `docker ps` and `docker logs`
|