Merge pull request #2546 from Summerdave/fix-backup-error-code

Return an error when errors occured during backup

changelog/unreleased/pull-2546

@@ -0,0 +1,19 @@
+Change: Return exit code 3 when failing to backup all source data
+
+The backup command used to return a zero exit code as long as a snapshot
+could be created successfully, even if some of the source files could not
+be read (in which case the snapshot would contain the rest of the files).
+
+This made it hard for automation/scripts to detect failures/incomplete
+backups by looking at the exit code. Restic now returns the following exit
+codes for the backup command:
+
+ - 0 when the command was successful
+ - 1 when there was a fatal error (no snapshot created)
+ - 3 when some source data could not be read (incomplete snapshot created)
+
+https://github.com/restic/restic/pull/2546
+https://github.com/restic/restic/issues/956
+https://github.com/restic/restic/issues/2064
+https://github.com/restic/restic/issues/2526
+https://github.com/restic/restic/issues/2364

cmd/restic/cmd_backup.go

@@ -39,10 +39,9 @@ given as the arguments.
 EXIT STATUS
 ===========
 
-Exit status is 0 if the command was successful, and non-zero if there was any error.
+Exit status is 0 if the command was successful.
-
+Exit status is 1 if there was a fatal error (no snapshot created).
-Note that some issues such as unreadable or deleted files during backup
+Exit status is 3 if some source data could not be read (incomplete snapshot created).
-currently doesn't result in a non-zero error exit status.
 `,
 	PreRun: func(cmd *cobra.Command, args []string) {
 		if backupOptions.Host == "" {
@@ -99,6 +98,9 @@ type BackupOptions struct {
 
 var backupOptions BackupOptions
 
+// Error sentinel for invalid source data
+var InvalidSourceData = errors.New("Failed to read all source data during backup.")
+
 func init() {
 	cmdRoot.AddCommand(cmdBackup)
 
@@ -557,7 +559,11 @@ func runBackup(opts BackupOptions, gopts GlobalOptions, term *termstatus.Termina
 	arch.SelectByName = selectByNameFilter
 	arch.Select = selectFilter
 	arch.WithAtime = opts.WithAtime
-	arch.Error = p.Error
+	success := true
+	arch.Error = func(item string, fi os.FileInfo, err error) error {
+		success = false
+		return p.Error(item, fi, err)
+	}
 	arch.CompleteItem = p.CompleteItem
 	arch.StartFile = p.StartFile
 	arch.CompleteBlob = p.CompleteBlob
@@ -594,6 +600,9 @@ func runBackup(opts BackupOptions, gopts GlobalOptions, term *termstatus.Termina
 	if !gopts.JSON {
 		p.P("snapshot %s saved\n", id.Str())
 	}
+	if !success {
+		return InvalidSourceData
+	}
 
 	// Return error if any
 	return err

cmd/restic/integration_test.go

@@ -13,6 +13,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"runtime"
 	"strings"
 	"syscall"
 	"testing"
@@ -54,7 +55,7 @@ func testRunInit(t testing.TB, opts GlobalOptions) {
 	t.Logf("repository initialized at %v", opts.Repo)
 }
 
-func testRunBackup(t testing.TB, dir string, target []string, opts BackupOptions, gopts GlobalOptions) {
+func testRunBackupAssumeFailure(t testing.TB, dir string, target []string, opts BackupOptions, gopts GlobalOptions) error {
 	ctx, cancel := context.WithCancel(gopts.ctx)
 	defer cancel()
 
@@ -69,7 +70,7 @@ func testRunBackup(t testing.TB, dir string, target []string, opts BackupOptions
 		defer cleanup()
 	}
 
-	rtest.OK(t, runBackup(opts, gopts, term, target))
+	backupErr := runBackup(opts, gopts, term, target)
 
 	cancel()
 
@@ -77,6 +78,13 @@ func testRunBackup(t testing.TB, dir string, target []string, opts BackupOptions
 	if err != nil {
 		t.Fatal(err)
 	}
+
+	return backupErr
+}
+
+func testRunBackup(t testing.TB, dir string, target []string, opts BackupOptions, gopts GlobalOptions) {
+	err := testRunBackupAssumeFailure(t, dir, target, opts, gopts)
+	rtest.Assert(t, err == nil, "Error while backing up")
 }
 
 func testRunList(t testing.TB, tpe string, opts GlobalOptions) restic.IDs {
@@ -436,6 +444,36 @@ func TestBackupExclude(t *testing.T) {
 		"expected file %q not in first snapshot, but it's included", "passwords.txt")
 }
 
+func TestBackupErrors(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		return
+	}
+	env, cleanup := withTestEnvironment(t)
+	defer cleanup()
+
+	datafile := filepath.Join("testdata", "backup-data.tar.gz")
+
+	rtest.SetupTarTestFixture(t, env.testdata, datafile)
+
+	testRunInit(t, env.gopts)
+
+	// Assume failure
+	inaccessibleFile := filepath.Join(env.testdata, "0", "0", "9", "0")
+	os.Chmod(inaccessibleFile, 0000)
+	defer func() {
+		os.Chmod(inaccessibleFile, 0644)
+	}()
+	opts := BackupOptions{}
+	gopts := env.gopts
+	gopts.stderr = ioutil.Discard
+	err := testRunBackupAssumeFailure(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, gopts)
+	rtest.Assert(t, err != nil, "Assumed failure, but no error occured.")
+	rtest.Assert(t, err == InvalidSourceData, "Wrong error returned")
+	snapshotIDs := testRunList(t, "snapshots", env.gopts)
+	rtest.Assert(t, len(snapshotIDs) == 1,
+		"expected one snapshot, got %v", snapshotIDs)
+}
+
 const (
 	incrementalFirstWrite  = 10 * 1042 * 1024
 	incrementalSecondWrite = 1 * 1042 * 1024

cmd/restic/main.go

@@ -103,9 +103,13 @@ func main() {
 	}
 
 	var exitCode int
-	if err != nil {
+	switch err {
+	case nil:
+		exitCode = 0
+	case InvalidSourceData:
+		exitCode = 3
+	default:
 		exitCode = 1
 	}
-
 	Exit(exitCode)
 }

doc/040_backup.rst

@@ -366,7 +366,6 @@ created as it would only be written at the very (successful) end of
 the backup operation.  Previous snapshots will still be there and will still
 work.
 
-
 Environment Variables
 *********************
 
@@ -424,3 +423,26 @@ are taken into account for various operations:
  * ``$XDG_CACHE_HOME/restic``, ``$HOME/.cache/restic``: :ref:`caching`.
  * ``$TMPDIR``: :ref:`temporary_files`.
  * ``$PATH/fusermount``: Binary for ``restic mount``.
+
+Exit status codes
+*****************
+
+Restic returns one of the following exit status codes after the backup command is run:
+
+ * 0 when the backup was successful (snapshot with all source files created)
+ * 1 when there was a fatal error (no snapshot created)
+ * 3 when some source files could not be read (incomplete snapshot with remaining files created)
+
+Fatal errors occur for example when restic is unable to write to the backup destination, when
+there are network connectivity issues preventing successful communication, or when an invalid
+password or command line argument is provided. When restic returns this exit status code, one
+should not expect a snapshot to have been created.
+
+Source file read errors occur when restic fails to read one or more files or directories that
+it was asked to back up, e.g. due to permission problems. Restic displays the number of source
+file read errors that occurred while running the backup. If there are errors of this type,
+restic will still try to complete the backup run with all the other files, and create a
+snapshot that then contains all but the unreadable files.
+
+One can use these exit status codes in scripts and other automation tools, to make them aware of
+the outcome of the backup run. To manually inspect the exit code in e.g. Linux, run ``echo $?``.