Merge pull request #25 from offen/delete-on-failure

Ensure script always tries to remove local artifacts even when backup failed

README.md

@@ -3,11 +3,13 @@
 Backup Docker volumes locally or to any S3 compatible storage.
 
 The [offen/docker-volume-backup](https://hub.docker.com/r/offen/docker-volume-backup) Docker image can be used as a lightweight (below 15MB) sidecar container to an existing Docker setup.
-It handles __recurring or one-off backups of Docker volumes__ to a __local directory__ or __any S3 compatible storage__ (or both), and __rotates away old backups__ if configured. It also supports __encrypting your backups using GPG__.
+It handles __recurring or one-off backups of Docker volumes__ to a __local directory__ or __any S3 compatible storage__ (or both), and __rotates away old backups__ if configured. It also supports __encrypting your backups using GPG__ and __sending notifications for failed backup runs__.
 
 <!-- MarkdownTOC -->
 
 - [Quickstart](#quickstart)
+  - [Recurring backups in a compose setup](#recurring-backups-in-a-compose-setup)
+  - [One-off backups using Docker CLI](#one-off-backups-using-docker-cli)
 - [Configuration reference](#configuration-reference)
 - [How to](#how-to)
   - [Stopping containers during backup](#stopping-containers-during-backup)
@@ -38,6 +40,8 @@ Code and documentation for `v1` versions are found on [this branch][v1-branch].
 
 ## Quickstart
 
+### Recurring backups in a compose setup
+
 Add a `backup` service to your compose setup and mount the volumes you would like to see backed up:
 
 ```yml
@@ -78,6 +82,22 @@ volumes:
   data:
 ```
 
+### One-off backups using Docker CLI
+
+To run a one time backup, mount the volume you would like to see backed up into a container and run the `backup` command:
+
+```console
+docker run --rm \
+  -v data:/backup/data \
+  --env AWS_ACCESS_KEY_ID="<xxx>" \
+  --env AWS_SECRET_ACCESS_KEY="<xxx>" \
+  --env AWS_S3_BUCKET_NAME="<xxx>" \
+  --entrypoint backup \
+  offen/docker-volume-backup:latest
+```
+
+Alternatively, pass a `--env-file` in order to use a full config as described below.
+
 ## Configuration reference
 
 Backup targets, schedule and retention are configured in environment variables.
@@ -210,20 +230,12 @@ You can populate below template according to your requirements and use it as you
 
 # EMAIL_NOTIFICATION_SENDER="no-reply@example.com"
 
-# The hostname of your SMTP server.
+# Configuration and credentials for the SMTP server to be used.
+# EMAIL_SMTP_PORT defaults to 587.
 
 # EMAIL_SMTP_HOST="posteo.de"
-
-# The SMTP password.
-
 # EMAIL_SMTP_PASSWORD="<xxx>"
-
-# The SMTP username.
-
 # EMAIL_SMTP_USERNAME="no-reply@example.com"
-
-The port used when communicating with the server. Defaults to 587.
-
 # EMAIL_SMTP_PORT="<port>"
 ```
 

cmd/backup/main.go

@@ -39,6 +39,15 @@ func main() {
 		panic(err)
 	}
 
+	defer func() {
+		if err := recover(); err != nil {
+			if e, ok := err.(error); ok && strings.Contains(e.Error(), msgBackupFailed) {
+				os.Exit(1)
+			}
+			panic(err)
+		}
+	}()
+
 	s.must(func() error {
 		restartContainers, err := s.stopContainers()
 		defer func() {
@@ -50,9 +59,14 @@ func main() {
 		return s.takeBackup()
 	}())
 
-	s.must(s.encryptBackup())
-	s.must(s.copyBackup())
-	s.must(s.removeArtifacts())
+	s.must(func() error {
+		defer func() {
+			s.must(s.removeArtifacts())
+		}()
+		s.must(s.encryptBackup())
+		return s.copyBackup()
+	}())
+
 	s.must(s.pruneOldBackups())
 	s.logger.Info("Finished running backup tasks.")
 }
@@ -60,10 +74,10 @@ func main() {
 // script holds all the stateful information required to orchestrate a
 // single backup run.
 type script struct {
-	cli        *client.Client
-	mc         *minio.Client
-	logger     *logrus.Logger
-	errorHooks []errorHook
+	cli    *client.Client
+	mc     *minio.Client
+	logger *logrus.Logger
+	hooks  []hook
 
 	start  time.Time
 	file   string
@@ -72,8 +86,6 @@ type script struct {
 	c *config
 }
 
-type errorHook func(err error, start time.Time, logOutput string) error
-
 type config struct {
 	BackupSources              string        `split_words:"true" default:"/backup"`
 	BackupFilename             string        `split_words:"true" default:"backup-%Y-%m-%dT%H-%M-%S.tar.gz"`
@@ -97,6 +109,8 @@ type config struct {
 	EmailSMTPPassword          string        `envconfig:"EMAIL_SMTP_PASSWORD"`
 }
 
+var msgBackupFailed = "backup run failed"
+
 // newScript creates all resources needed for the script to perform actions against
 // remote resources like the Docker engine or remote storage locations. All
 // reading from env vars or other configuration sources is expected to happen
@@ -146,7 +160,7 @@ func newScript() (*script, error) {
 	}
 
 	if s.c.EmailNotificationRecipient != "" {
-		s.errorHooks = append(s.errorHooks, func(err error, start time.Time, logOutput string) error {
+		s.hooks = append(s.hooks, hook{hookLevelFailure, func(err error, start time.Time, logOutput string) error {
 			mailer := gomail.NewDialer(
 				s.c.EmailSMTPHost, s.c.EmailSMTPPort, s.c.EmailSMTPUsername, s.c.EmailSMTPPassword,
 			)
@@ -155,7 +169,7 @@ func newScript() (*script, error) {
 				"Failure running docker-volume-backup at %s", start.Format(time.RFC3339),
 			)
 			body := fmt.Sprintf(
-				"Running docker-volume-backup failed with error: %s\n\nLog output before the error occurred:\n\n%s\n", err, logOutput,
+				"Running docker-volume-backup failed with error: %s\n\nLog output of the failed run was:\n\n%s\n", err, logOutput,
 			)
 
 			message := gomail.NewMessage()
@@ -164,7 +178,7 @@ func newScript() (*script, error) {
 			message.SetHeader("Subject", subject)
 			message.SetBody("text/plain", body)
 			return mailer.DialAndSend(message)
-		})
+		}})
 	}
 
 	return s, nil
@@ -362,10 +376,17 @@ func (s *script) copyBackup() error {
 
 // removeArtifacts removes the backup file from disk.
 func (s *script) removeArtifacts() error {
-	if err := os.Remove(s.file); err != nil {
-		return fmt.Errorf("removeArtifacts: error removing file: %w", err)
+	_, err := os.Stat(s.file)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return fmt.Errorf("removeArtifacts: error calling stat on file %s: %w", s.file, err)
 	}
-	s.logger.Info("Removed local artifacts.")
+	if err := os.Remove(s.file); err != nil {
+		return fmt.Errorf("removeArtifacts: error removing file %s: %w", s.file, err)
+	}
+	s.logger.Infof("Removed local artifacts %s.", s.file)
 	return nil
 }
 
@@ -504,17 +525,35 @@ func (s *script) pruneOldBackups() error {
 	return nil
 }
 
-// must exits the script run non-zero and prematurely in case the given error
-// is non-nil. If error hooks are present on the script object, they
+// runHooks runs all hooks that have been registered using the
+// given level. In case executing a hook returns an error, the following
+// hooks will still be run before the function returns an error.
+func (s *script) runHooks(err error, targetLevel string) error {
+	var actionErrors []error
+	for _, hook := range s.hooks {
+		if hook.level != targetLevel {
+			continue
+		}
+		if err := hook.action(err, s.start, s.output.String()); err != nil {
+			actionErrors = append(actionErrors, err)
+		}
+	}
+	if len(actionErrors) != 0 {
+		return join(actionErrors...)
+	}
+	return nil
+}
+
+// must exits the script run prematurely in case the given error
+// is non-nil. If failure hooks have been registered on the script object, they
 // will be called, passing the failure and previous log output.
 func (s *script) must(err error) {
 	if err != nil {
-		for _, hook := range s.errorHooks {
-			if hookErr := hook(err, s.start, s.output.String()); hookErr != nil {
-				s.logger.Errorf("An error occurred calling an error hook: %s", hookErr)
-			}
+		s.logger.Errorf("Fatal error running backup: %s", err)
+		if hookErr := s.runHooks(err, hookLevelFailure); hookErr != nil {
+			s.logger.Errorf("An error occurred calling the registered failure hooks: %s", hookErr)
 		}
-		s.logger.Fatalf("Fatal error running backup: %s", err)
+		panic(errors.New(msgBackupFailed))
 	}
 }
 
@@ -587,3 +626,14 @@ func (b *bufferingWriter) Write(p []byte) (n int, err error) {
 	}
 	return b.writer.Write(p)
 }
+
+// hook contains a queued action that can be trigger them when the script
+// reaches a certain point (e.g. unsuccessful backup)
+type hook struct {
+	level  string
+	action func(err error, start time.Time, logOutput string) error
+}
+
+const (
+	hookLevelFailure = "failure"
+)

test/cli/run.sh

@@ -29,8 +29,7 @@ docker run -d \
 
 sleep 10
 
-docker run -d \
-  --name backup \
+docker run --rm \
   --network test_network \
   -v app_data:/backup/app_data \
   -v /var/run/docker.sock:/var/run/docker.sock \
@@ -40,18 +39,16 @@ docker run -d \
   --env AWS_ENDPOINT_PROTO=http \
   --env AWS_S3_BUCKET_NAME=backup \
   --env BACKUP_FILENAME=test.tar.gz \
-  --env BACKUP_CRON_EXPRESSION="0 0 5 31 2 ?" \
+  --entrypoint backup \
   offen/docker-volume-backup:$TEST_VERSION
 
-docker exec backup backup
-
 docker run --rm -it \
   -v backup_data:/data alpine \
   ash -c 'tar -xvf /data/backup/test.tar.gz && test -f /backup/app_data/offen.db'
 
 echo "[TEST:PASS] Found relevant files in untared backup."
 
-if [ "$(docker ps -q | wc -l)" != "3" ]; then
+if [ "$(docker ps -q | wc -l)" != "2" ]; then
   echo "[TEST:FAIL] Expected all containers to be running post backup, instead seen:"
   docker ps
   exit 1