Skip to content
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f196c05
fix: honour cancellation in the PVE staged apply
tis24dev Sep 5, 2026
836cc67
fix: stop shadowing the components package in daemon diagnostics
tis24dev Sep 5, 2026
d204506
docs: state that --log-level never quiets an exit code
tis24dev Sep 5, 2026
bf3e494
fix: stop reporting an already-matching storage as an apply failure
tis24dev Sep 5, 2026
cdeee3c
fix: stop blaming the daemon for an unreadable configuration
tis24dev Sep 5, 2026
d8a96e0
fix: name the config the daemon-status screen could not read
tis24dev Sep 5, 2026
36c4b58
fix: limit the guest conf-file fallback to a schema refusal
tis24dev Sep 5, 2026
ac11e67
fix: name the kernel setting the accepted script ancestor rests on
tis24dev Sep 5, 2026
bc83359
fix: parse pvesh JSON from stdout, not from the merged stream
tis24dev Sep 5, 2026
b666e57
fix: converge the storage set fallback, and stop reading value errors…
tis24dev Sep 5, 2026
77d01b3
fix: let auto-yes mean auto-yes for the post-upgrade notes screen
tis24dev Sep 5, 2026
c10ba93
feat: retry a refused guest set instead of losing the whole config
tis24dev Sep 5, 2026
db718ed
feat: run the upgrade finalize inside the binary that was just installed
tis24dev Sep 5, 2026
151510d
docs: add the 0.36.0 what's-new entry
tis24dev Sep 5, 2026
7100490
fix: read parsed pvesh and PBS output from stdout
tis24dev Sep 5, 2026
dda5de1
fix: read the recipient lookup JSON from stdout
tis24dev Sep 5, 2026
93c6024
fix: read the rclone metadata manifest from stdout
tis24dev Sep 5, 2026
428b823
fix: read the pool list and the rclone listing from stdout
tis24dev Sep 5, 2026
8efe7a5
docs: widen the 0.36.0 locale highlight to what ships
tis24dev Sep 5, 2026
69eba4e
fix: address the four reviewer findings on PR 308
tis24dev Sep 5, 2026
0eb88c3
style: apply De Morgan in the finalize wiring assertion
tis24dev Sep 5, 2026
b9b6b34
fix: report an arm's cancellation as an abort, not a failed item
tis24dev Sep 5, 2026
d52170d
fix: keep the failed item name when an abort lands on a failing arm
tis24dev Sep 5, 2026
4035b6f
fix: propagate cancellation out of the aggregating apply arms
tis24dev Sep 5, 2026
7e249af
fix: stop the mount guard arm on cancellation
tis24dev Sep 5, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions cmd/proxsave/daemon_diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const (
personalScriptConfigurationDrift personalScriptSynchronization = "configuration-drift"
personalScriptPathStateChanged personalScriptSynchronization = "path-state-changed"
personalScriptRuntimeUnavailable personalScriptSynchronization = "runtime-state-unavailable"
personalScriptCurrentUnavailable personalScriptSynchronization = "current-configuration-unavailable"
personalScriptSyncNotApplicable personalScriptSynchronization = "not-applicable"
)

Expand Down Expand Up @@ -93,7 +94,12 @@ var (

// collectDaemonDiagnostics owns every probe and verdict used by the two
// daemon-status frontends. Renderers receive facts; they never recompute them.
func collectDaemonDiagnostics(ctx context.Context, cfg *config.Config, baseDir string) daemonDiagnostics {
//
// cfgErr is the error that PREVENTED cfg from existing, and it is separate from
// cfg being nil on purpose: a caller that never attempted a load passes nil for
// both, while the dashboard passes the loader's own words so the screen can name
// the file it could not read instead of only that it could not read one.
func collectDaemonDiagnostics(ctx context.Context, cfg *config.Config, cfgErr error, baseDir string) daemonDiagnostics {
mode := "unknown"
var interval time.Duration
if cfg != nil {
Expand Down Expand Up @@ -128,6 +134,9 @@ func collectDaemonDiagnostics(ctx context.Context, cfg *config.Config, baseDir s
level, keyword, explanation := daemonStatusStyle(state)
daemonUID := daemonUIDResolver(state)
currentScripts := personalScriptsInspector(cfg, daemonUID.Value)
if cfg == nil && cfgErr != nil {
currentScripts = unknownPersonalScripts(daemonUID.Value, cfgErr)
}
runtimeDiagnostic, runningScripts := resolveDaemonRuntime(state, baseDir)

currentConfigPath := ""
Expand Down Expand Up @@ -227,9 +236,11 @@ func personalScriptDiagnosticFromRuntime(
default:
return personalScriptDiagnostic{}, false
}
var components []personalScriptPathComponent
// Not named "components": this file imports internal/ui/components, and a local
// of that name shadows the package for the rest of the function.
var pathComponents []personalScriptPathComponent
for _, component := range in.Components {
components = append(components, personalScriptPathComponent{
pathComponents = append(pathComponents, personalScriptPathComponent{
Path: component.Path,
UID: component.UID,
Mode: os.FileMode(component.Mode),
Expand All @@ -241,7 +252,7 @@ func personalScriptDiagnosticFromRuntime(
State: state,
Reason: in.Reason,
DaemonUID: daemonUID,
Components: components,
Components: pathComponents,
}, true
}

Expand All @@ -252,6 +263,15 @@ func comparePersonalScript(
current personalScriptDiagnostic,
) personalScriptComparison {
comparison := personalScriptComparison{Running: running, Current: current}
// The CURRENT side is checked first and on purpose. With no configuration to
// read there is nothing to compare against, whatever the daemon is doing, and
// the config-path arm below would otherwise measure the daemon's real path
// against an empty one and blame the daemon for the operator's unreadable file.
if current.State == personalScriptUnknown {
comparison.Synchronization = personalScriptCurrentUnavailable
comparison.SyncReason = current.Reason
return comparison
}
if runtime.Availability == daemonRuntimeNotApplicable {
comparison.Synchronization = personalScriptSyncNotApplicable
comparison.SyncReason = "no live daemon exists; current configuration is prospective"
Expand Down Expand Up @@ -406,7 +426,7 @@ func logPersonalScriptSynchronization(logger *logging.Logger, comparison persona
logger.Warning(" Synchronization: OUT OF SYNC (%s)", reason)
case personalScriptPathStateChanged:
logger.Warning(" Synchronization: PATH STATE CHANGED SINCE STARTUP (%s)", reason)
case personalScriptRuntimeUnavailable:
case personalScriptRuntimeUnavailable, personalScriptCurrentUnavailable:
logger.Warning(" Synchronization: UNKNOWN (%s)", reason)
case personalScriptSyncNotApplicable:
logger.Info(" Synchronization: NOT APPLICABLE")
Expand All @@ -429,6 +449,8 @@ func logPersonalScriptDiagnostic(logger *logging.Logger, label string, diagnosti
return
}
logger.Warning("%s: REFUSED (%s): %s", label, path, reason)
case personalScriptUnknown:
logger.Warning("%s: UNKNOWN: %s", label, reason)
default:
logger.Info("%s: NOT CONFIGURED", label)
}
Expand Down
181 changes: 181 additions & 0 deletions cmd/proxsave/daemon_diagnostics_no_config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
package main

import (
"bytes"
"context"
"errors"
"strings"
"testing"
"time"

"github.com/tis24dev/proxsave/internal/health"
"github.com/tis24dev/proxsave/internal/logging"
"github.com/tis24dev/proxsave/internal/types"
)

// runDashboardDaemonStatus leaves cfg nil when the config file cannot be loaded,
// and the whole diagnostic then stated two falsehoods about a daemon that was
// fine: the current side read NOT CONFIGURED (which asserts the settings were
// empty), and the comparison measured the daemon's real config path against ""
// and told the operator to restart the daemon. The sibling screen twenty lines
// away already refuses to gloss this - runDashboardDaemonAdmin renders "CONFIG
// UNREADABLE" - so the status screen names it too.
func TestInspectPersonalScriptsWithoutAConfigIsUnknownNotEmpty(t *testing.T) {
got := inspectPersonalScripts(nil, 4242)
for _, tc := range []struct {
label string
diagnostic personalScriptDiagnostic
wantKey string
}{
{"pre", got.Pre, "PERSONAL_SCRIPT_PRE_RUN"},
{"post", got.Post, "PERSONAL_SCRIPT_POST_RUN"},
} {
if tc.diagnostic.State == personalScriptNotConfigured {
t.Fatalf("%s: an unreadable config must not claim the setting was empty: %+v", tc.label, tc.diagnostic)
}
if tc.diagnostic.State != personalScriptUnknown || tc.diagnostic.Reason != personalScriptConfigUnreadable {
t.Fatalf("%s: state/reason = %q/%q, want unknown with the shared reason", tc.label, tc.diagnostic.State, tc.diagnostic.Reason)
}
if tc.diagnostic.Key != tc.wantKey || tc.diagnostic.DaemonUID != 4242 || tc.diagnostic.Path != "" {
t.Fatalf("%s: evidence lost: %+v", tc.label, tc.diagnostic)
}
}
}

// The unknown current side wins over every runtime verdict, including the healthy
// one: with nothing to compare against, no synchronization statement is available,
// and the config-path arm would otherwise blame the daemon.
func TestComparePersonalScriptWithoutACurrentConfigNeverBlamesTheDaemon(t *testing.T) {
current := unknownPersonalScript("PERSONAL_SCRIPT_PRE_RUN", 0, nil)
running := personalScriptDiagnostic{Path: "/pre", State: personalScriptReady}
for _, availability := range []daemonRuntimeAvailability{
daemonRuntimeAvailable, daemonRuntimeMissing, daemonRuntimeStale,
daemonRuntimeInvalid, daemonRuntimeUnsupported, daemonRuntimeNotApplicable,
} {
t.Run(string(availability), func(t *testing.T) {
runtime := daemonRuntimeDiagnostic{Availability: availability, ConfigPath: "/daemon.env", Reason: "runtime reason"}
got := comparePersonalScript(runtime, "", running, current)
if got.Synchronization == personalScriptConfigurationDrift {
t.Fatalf("an unreadable config was reported as a daemon restart being due: %+v", got)
}
if got.Synchronization != personalScriptCurrentUnavailable {
t.Fatalf("synchronization = %q, want %q", got.Synchronization, personalScriptCurrentUnavailable)
}
if got.SyncReason != personalScriptConfigUnreadable {
t.Fatalf("sync reason = %q, want the unreadable-config reason", got.SyncReason)
}
})
}
}

// Both renderers share the state vocabulary; a state neither of them names falls
// through to the NOT CONFIGURED arm, which is the lie this fix removes.
func TestDaemonDiagnosticsRenderersNameTheUnknownCurrentState(t *testing.T) {
diagnostics := daemonDiagnostics{
Mode: "unknown",
Unit: "installed",
Active: "active",
Keyword: "running",
Runtime: daemonRuntimeDiagnostic{Availability: daemonRuntimeNotApplicable, Reason: "daemon process is not live"},
ScriptComparisons: personalScriptComparisons{
Pre: comparePersonalScript(daemonRuntimeDiagnostic{Availability: daemonRuntimeNotApplicable}, "", personalScriptDiagnostic{}, unknownPersonalScript("PERSONAL_SCRIPT_PRE_RUN", 0, nil)),
Post: comparePersonalScript(daemonRuntimeDiagnostic{Availability: daemonRuntimeNotApplicable}, "", personalScriptDiagnostic{}, unknownPersonalScript("PERSONAL_SCRIPT_POST_RUN", 0, nil)),
},
}

logger := logging.New(types.LogLevelDebug, false)
buf := &bytes.Buffer{}
logger.SetOutput(buf)
logDaemonDiagnostics(logger, diagnostics)
out := buf.String()

if strings.Contains(out, "Current configuration: NOT CONFIGURED") {
t.Fatalf("the CLI still claims the settings were empty:\n%s", out)
}
for _, want := range []string{
"Current configuration: UNKNOWN: " + personalScriptConfigUnreadable,
"Synchronization: UNKNOWN (" + personalScriptConfigUnreadable + ")",
} {
if !strings.Contains(out, want) {
t.Fatalf("missing %q:\n%s", want, out)
}
}

prompt := buildDaemonStatusPrompt(diagnostics)
if strings.Contains(prompt, "NOT CONFIGURED") {
t.Fatalf("the dashboard still claims the settings were empty:\n%s", prompt)
}
if strings.Count(prompt, personalScriptConfigUnreadable) != 4 {
t.Fatalf("reason count = %d, want 4 (state + synchronization, per script):\n%s",
strings.Count(prompt, personalScriptConfigUnreadable), prompt)
}
}

// Saying only that the configuration could not be read leaves the operator with
// no file to go and fix, so the loader's own words ride along. The dashboard is
// the one caller that reaches the collector without a config, and it is the one
// that holds the error.
func TestCollectDaemonDiagnosticsCarriesTheConfigLoadCause(t *testing.T) {
origInstalled := daemonStatusUnitInstalledProbe
origActive := daemonStatusActiveStateProbe
origPresence := daemonPresenceProbe
origNow := daemonStatusNow
t.Cleanup(func() {
daemonStatusUnitInstalledProbe = origInstalled
daemonStatusActiveStateProbe = origActive
daemonPresenceProbe = origPresence
daemonStatusNow = origNow
})
daemonStatusUnitInstalledProbe = func() bool { return true }
daemonStatusActiveStateProbe = func(context.Context) string { return "active" }
daemonPresenceProbe = func(context.Context) health.DaemonPresence {
return health.DaemonPresence{Probed: true, Installed: true, Active: true}
}
daemonStatusNow = func() time.Time { return time.Unix(1_700_000_000, 0) }

cause := errors.New("open /etc/proxsave/backup.env: permission denied")
got := collectDaemonDiagnostics(context.Background(), nil, cause, t.TempDir())

for _, tc := range []struct {
label string
comparison personalScriptComparison
}{
{"pre", got.ScriptComparisons.Pre},
{"post", got.ScriptComparisons.Post},
} {
if !strings.Contains(tc.comparison.Current.Reason, cause.Error()) {
t.Fatalf("%s: the current state drops the load cause: %q", tc.label, tc.comparison.Current.Reason)
}
if !strings.Contains(tc.comparison.SyncReason, cause.Error()) {
t.Fatalf("%s: the synchronization verdict drops the load cause: %q", tc.label, tc.comparison.SyncReason)
}
if tc.comparison.Synchronization != personalScriptCurrentUnavailable {
t.Fatalf("%s: synchronization = %q", tc.label, tc.comparison.Synchronization)
}
}

// A caller with a config, or one that never attempted a load, must be unchanged.
clean := collectDaemonDiagnostics(context.Background(), nil, nil, t.TempDir())
if clean.ScriptComparisons.Pre.Current.Reason != personalScriptConfigUnreadable {
t.Fatalf("a nil cause invented one: %q", clean.ScriptComparisons.Pre.Current.Reason)
}
}

// The daemon must never publish the unknown state: it is the reader's verdict
// about a missing config, not a state a running daemon can be in. A record
// carrying it is an invalid record.
func TestRuntimeRecordsMayNotCarryTheUnknownState(t *testing.T) {
record := func(state personalScriptState) health.DaemonRuntimeScript {
return health.DaemonRuntimeScript{Path: "/pre", State: string(state)}
}
if _, ok := personalScriptDiagnosticFromRuntime("PERSONAL_SCRIPT_PRE_RUN", 0, record(personalScriptUnknown)); ok {
t.Fatal("a runtime record claiming the unknown state was accepted")
}
for _, state := range []personalScriptState{
personalScriptNotConfigured, personalScriptReady, personalScriptReadyWithWarning, personalScriptRefused,
} {
if _, ok := personalScriptDiagnosticFromRuntime("PERSONAL_SCRIPT_PRE_RUN", 0, record(state)); !ok {
t.Fatalf("the published state %q is no longer accepted", state)
}
}
}
4 changes: 2 additions & 2 deletions cmd/proxsave/daemon_diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ func TestCollectDaemonDiagnosticsBuildsOneSharedSnapshot(t *testing.T) {
BaseDir: t.TempDir(),
HealthcheckHeartbeatInterval: time.Minute,
}
got := collectDaemonDiagnostics(context.Background(), cfg, cfg.BaseDir)
got := collectDaemonDiagnostics(context.Background(), cfg, nil, cfg.BaseDir)

if got.Mode != "daemon" || got.Unit != "installed" || got.Active != "active" {
t.Fatalf("incomplete shared snapshot: %+v", got)
Expand All @@ -320,7 +320,7 @@ func TestRunDaemonStatusUsesSharedDiagnosticsCollector(t *testing.T) {
t.Cleanup(func() { daemonDiagnosticsCollector = origCollector })

called := 0
daemonDiagnosticsCollector = func(context.Context, *config.Config, string) daemonDiagnostics {
daemonDiagnosticsCollector = func(context.Context, *config.Config, error, string) daemonDiagnostics {
called++
return daemonDiagnostics{
Mode: "daemon",
Expand Down
4 changes: 3 additions & 1 deletion cmd/proxsave/daemon_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ func cronModeRecordClause(revert cronRevertReport) string {
// alignment) - non-interactively, then exits. Exit 0 when the daemon is running and aligned,
// non-zero otherwise, so scripts can gate on it.
func runDaemonStatus(rt *appRuntime) int {
diagnostics := daemonDiagnosticsCollector(rt.ctx, rt.cfg, "")
// No cfgErr: the run bootstrap already exited on an unreadable config, so rt.cfg
// is never nil here and there is no failed attempt to report.
diagnostics := daemonDiagnosticsCollector(rt.ctx, rt.cfg, nil, "")
logDaemonDiagnostics(rt.logger, diagnostics)
if diagnostics.Level == orchestrator.HealthcheckSetupLevelOk {
return types.ExitSuccess.Int()
Expand Down
6 changes: 3 additions & 3 deletions cmd/proxsave/daemon_status_cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func TestCollectDaemonDiagnosticsInspectsScriptsWithResolvedUID(t *testing.T) {
PersonalScriptPreRun: "/pre",
PersonalScriptPostRun: "/post",
}
got := collectDaemonDiagnostics(context.Background(), cfg, cfg.BaseDir)
got := collectDaemonDiagnostics(context.Background(), cfg, nil, cfg.BaseDir)
if inspections != 1 {
t.Fatalf("script inspections = %d, want 1", inspections)
}
Expand Down Expand Up @@ -206,7 +206,7 @@ func TestCollectDaemonDiagnosticsComparesResidentReadyWithCurrentEmpty(t *testin
}); err != nil {
t.Fatal(err)
}
got := collectDaemonDiagnostics(context.Background(), cfg, "/ignored-base")
got := collectDaemonDiagnostics(context.Background(), cfg, nil, "/ignored-base")
if got.Runtime.Availability != daemonRuntimeAvailable || !got.State.ProcessAlive {
t.Fatalf("resident state unavailable: %+v", got)
}
Expand Down Expand Up @@ -303,7 +303,7 @@ func TestLogDaemonDiagnosticsUsesStandardEnvelopeAndSeverity(t *testing.T) {
func TestRunDaemonStatusExitRemainsBasedOnDaemonState(t *testing.T) {
origCollector := daemonDiagnosticsCollector
t.Cleanup(func() { daemonDiagnosticsCollector = origCollector })
daemonDiagnosticsCollector = func(context.Context, *config.Config, string) daemonDiagnostics {
daemonDiagnosticsCollector = func(context.Context, *config.Config, error, string) daemonDiagnostics {
return daemonDiagnostics{
Level: orchestrator.HealthcheckSetupLevelOk,
Keyword: "running",
Expand Down
16 changes: 13 additions & 3 deletions cmd/proxsave/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -903,14 +903,19 @@ const (
func runDashboardDaemonStatus(ctx context.Context, session *shell.Session, configPath, baseDir string) {
errDaemonStatusEsc := errors.New("daemon status: esc")
for {
// cfgErr is kept, not discarded: this is the ONE screen that reaches the
// collector without a configuration, and the personal-script block says so.
// Without the cause it could only report that the file was unreadable, never
// which file or why - the same half-answer runDashboardDaemonAdmin refuses.
var cfg *config.Config
if loaded, err := daemonStatusLoadConfig(configPath, baseDir); err == nil && loaded != nil {
loaded, cfgErr := daemonStatusLoadConfig(configPath, baseDir)
if cfgErr == nil && loaded != nil {
cfg = loaded
if strings.TrimSpace(loaded.BaseDir) != "" {
baseDir = loaded.BaseDir
}
}
diagnostics := daemonDiagnosticsCollector(ctx, cfg, baseDir)
diagnostics := daemonDiagnosticsCollector(ctx, cfg, cfgErr, baseDir)
// Dashboard "Status:" keywords are ALL-CAPS (the house convention across every
// dashboard Status screen). daemonStatusStyle is shared with the plain-text
// --daemon-status CLI line, so uppercase HERE (the graphical consumer) rather than at
Expand Down Expand Up @@ -1051,7 +1056,7 @@ func buildDashboardPersonalScriptSynchronization(comparison personalScriptCompar
return prefix + theme.WarningText.Render("OUT OF SYNC") + theme.Subtle.Render(" ("+reason+")")
case personalScriptPathStateChanged:
return prefix + theme.WarningText.Render("PATH STATE CHANGED SINCE STARTUP") + theme.Subtle.Render(" ("+reason+")")
case personalScriptRuntimeUnavailable:
case personalScriptRuntimeUnavailable, personalScriptCurrentUnavailable:
return prefix + theme.WarningText.Render("UNKNOWN") + theme.Subtle.Render(" ("+reason+")")
case personalScriptSyncNotApplicable:
return prefix + theme.Subtle.Render("NOT APPLICABLE")
Expand Down Expand Up @@ -1086,6 +1091,11 @@ func buildDashboardPersonalScriptLine(label string, diagnostic personalScriptDia
if reason != "" {
line += theme.Subtle.Render(": " + reason)
}
case personalScriptUnknown:
line += theme.WarningText.Render("UNKNOWN")
if reason != "" {
line += theme.Subtle.Render(": " + reason)
}
default:
line += theme.Subtle.Render("NOT CONFIGURED")
}
Expand Down
Loading
Loading