diff --git a/backup_warning_test.go b/backup_warning_test.go deleted file mode 100644 index a45e78a..0000000 --- a/backup_warning_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package plaklet - -import ( - "strings" - "testing" - - "github.com/stretchr/testify/require" -) - -func TestBackupWarning(t *testing.T) { - // A clean backup produces no warning. - require.Nil(t, backupWarning("backup", &Report{Backup: &BackupReport{Errors: 0}})) - - // Nil report / nil backup section are safe no-ops. - require.Nil(t, backupWarning("backup", nil)) - require.Nil(t, backupWarning("backup", &Report{})) - - // A backup with per-file errors warns but does not fail — the caller still - // emits ReplySuccess. The message carries the actual error count. - w := backupWarning("backup", &Report{Backup: &BackupReport{Errors: 3}}) - require.NotNil(t, w) - require.Equal(t, ReplyWarning, w.Type) - require.True(t, strings.Contains(w.Message, "3"), "message should carry the error count: %q", w.Message) -} diff --git a/check.go b/check.go index d77c20b..dfd4936 100644 --- a/check.go +++ b/check.go @@ -69,5 +69,10 @@ func check(ctx *kcontext.KContext, input *ExecPayload) (*Report, error) { } ccr.Took = time.Since(start) - return &Report{Type: input.Op, Check: &ccr}, nil + + report := &Report{Type: input.Op, Check: &ccr} + if ccr.Errors != 0 { + return report, fmt.Errorf("check failed: %d of %d snapshot(s) had errors", ccr.Errors, len(snapshotIDs)) + } + return report, nil } diff --git a/plaklet.go b/plaklet.go index 9e3b931..4e84a29 100644 --- a/plaklet.go +++ b/plaklet.go @@ -170,15 +170,18 @@ func Main(args []string) int { ctx.Events().Close() listener.Wait() + // listener.Wait() has drained every event, so the final State is complete + // and race-free here — safe to fold event-only errors into the verdict. + if err == nil { + err = terminalError(input.Op, report, listener.State()) + } + if err != nil { send(&ExecReply{Type: ReplyFailure, Message: fmt.Sprintf("%s failed: %s", input.Op, err)}) return 0 } if report != nil { - if w := backupWarning(input.Op, report); w != nil { - send(w) - } if raw, merrr := json.Marshal(report); merrr == nil { send(&ExecReply{Type: ReplyReport, Report: raw}) } @@ -187,21 +190,35 @@ func Main(args []string) int { return 0 } -// backupWarning returns a warning reply when a backup committed a snapshot but -// couldn't read some files, or nil otherwise. Such a run is a -// success-with-warnings, not a failure (standard plakar semantics: the snapshot -// is still valid). The per-file error count lives in the report but nothing -// downstream inspects it, so we surface it as a warning the operator sees while -// the job still succeeds. -func backupWarning(op string, report *Report) *ExecReply { - if report == nil || report.Backup == nil || report.Backup.Errors <= 0 { +// terminalError decides whether a completed operation should fail the job, for +// the operations whose failures surface only after the fact rather than as a +// returned error. A backup or restore commits/returns nil even when it couldn't +// read or write some entries; check tallies snapshot errors in its own report +// and already returns an error itself. We treat any such error count as a job +// failure — a partial run is not a clean success. state carries the per-entry +// error counters folded from the (now fully drained) event bus, used for +// restore whose failures live only in events. Mutates report to record the +// count. Returns nil when the run is clean. +func terminalError(op string, report *Report, state State) error { + if report == nil { return nil } - return &ExecReply{ - Type: ReplyWarning, - Message: fmt.Sprintf("%s completed with %d file error(s); those files were skipped", - op, report.Backup.Errors), + switch op { + case "backup": + if report.Backup != nil && report.Backup.Errors > 0 { + return fmt.Errorf("backup failed: %d file(s) could not be read", report.Backup.Errors) + } + case "restore": + if report.Restore != nil { + entryErrors := state.Paths.Error + state.Files.Error + state.Dirs.Error + + state.Symlinks.Error + state.Xattrs.Error + if entryErrors != 0 { + report.Restore.Errors = entryErrors + return fmt.Errorf("restore failed: %d entries could not be restored", entryErrors) + } + } } + return nil } // dispatch routes an operation to its handler. Only the operations a remote edge diff --git a/report.go b/report.go index ddc61db..1e6a3a0 100644 --- a/report.go +++ b/report.go @@ -59,6 +59,7 @@ type RestoreReport struct { Took time.Duration `json:"took"` LogicalSize uint64 `json:"logical_size"` Content BackupContent `json:"content"` + Errors uint64 `json:"errors"` Store StoreIO `json:"store"` } diff --git a/terminal_error_test.go b/terminal_error_test.go new file mode 100644 index 0000000..bba3358 --- /dev/null +++ b/terminal_error_test.go @@ -0,0 +1,39 @@ +package plaklet + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestTerminalError(t *testing.T) { + // A clean backup/restore is not a failure. + require.NoError(t, terminalError("backup", &Report{Backup: &BackupReport{Errors: 0}}, State{})) + require.NoError(t, terminalError("restore", &Report{Restore: &RestoreReport{}}, State{})) + + // Nil report / nil section are safe no-ops. + require.NoError(t, terminalError("backup", nil, State{})) + require.NoError(t, terminalError("backup", &Report{}, State{})) + require.NoError(t, terminalError("restore", &Report{}, State{})) + + // A backup that couldn't read some files fails the job (same semantics as + // restore and check — a partial run is not a clean success). + err := terminalError("backup", &Report{Backup: &BackupReport{Errors: 3}}, State{}) + require.Error(t, err) + require.True(t, strings.Contains(err.Error(), "3"), "message should carry the error count: %q", err) + + // A restore whose entries failed to export fails the job; the per-entry + // counts come from the drained event State, and are folded into the report. + var st State + st.Files.Error = 2 + st.Symlinks.Error = 1 + rep := &Report{Restore: &RestoreReport{}} + err = terminalError("restore", rep, st) + require.Error(t, err) + require.Equal(t, uint64(3), rep.Restore.Errors) + require.True(t, strings.Contains(err.Error(), "3"), "message should carry the error count: %q", err) + + // Operations without a post-hoc failure notion (e.g. sync) are unaffected. + require.NoError(t, terminalError("sync", &Report{}, State{})) +}