Pruning method logs nonsensical configuration values (#301)

* Pruning method logs nonsensical configuration values

* Adjust test assertion about log output

cmd/backup/script.go

@@ -172,11 +172,11 @@ func newScript() (*script, error) {
 			CACert:           s.c.AwsEndpointCACert.Cert,
 			PartSize:         s.c.AwsPartSize,
 		}
-		if s3Backend, err := s3.NewStorageBackend(s3Config, logFunc); err != nil {
+		s3Backend, err := s3.NewStorageBackend(s3Config, logFunc)
+		if err != nil {
 			return nil, fmt.Errorf("newScript: error creating s3 storage backend: %w", err)
-		} else {
-			s.storages = append(s.storages, s3Backend)
 		}
+		s.storages = append(s.storages, s3Backend)
 	}
 
 	if s.c.WebdavUrl != "" {
@@ -187,11 +187,11 @@ func newScript() (*script, error) {
 			Password:    s.c.WebdavPassword,
 			RemotePath:  s.c.WebdavPath,
 		}
-		if webdavBackend, err := webdav.NewStorageBackend(webDavConfig, logFunc); err != nil {
+		webdavBackend, err := webdav.NewStorageBackend(webDavConfig, logFunc)
+		if err != nil {
 			return nil, fmt.Errorf("newScript: error creating webdav storage backend: %w", err)
-		} else {
-			s.storages = append(s.storages, webdavBackend)
 		}
+		s.storages = append(s.storages, webdavBackend)
 	}
 
 	if s.c.SSHHostName != "" {
@@ -204,11 +204,11 @@ func newScript() (*script, error) {
 			IdentityPassphrase: s.c.SSHIdentityPassphrase,
 			RemotePath:         s.c.SSHRemotePath,
 		}
-		if sshBackend, err := ssh.NewStorageBackend(sshConfig, logFunc); err != nil {
+		sshBackend, err := ssh.NewStorageBackend(sshConfig, logFunc)
+		if err != nil {
 			return nil, fmt.Errorf("newScript: error creating ssh storage backend: %w", err)
-		} else {
-			s.storages = append(s.storages, sshBackend)
 		}
+		s.storages = append(s.storages, sshBackend)
 	}
 
 	if _, err := os.Stat(s.c.BackupArchive); !os.IsNotExist(err) {

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.2.0
 	github.com/containrrr/shoutrrr v0.7.1
 	github.com/cosiner/argv v0.1.0
-	github.com/docker/docker v24.0.5+incompatible
+	github.com/docker/docker v24.0.7+incompatible
 	github.com/gofrs/flock v0.8.1
 	github.com/klauspost/compress v1.17.2
 	github.com/leekchan/timeutil v0.0.0-20150802142658-28917288c48d

go.sum

@@ -258,6 +258,8 @@ github.com/docker/distribution v2.8.2+incompatible h1:T3de5rq0dB1j30rp0sA2rER+m3
 github.com/docker/distribution v2.8.2+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
 github.com/docker/docker v24.0.5+incompatible h1:WmgcE4fxyI6EEXxBRxsHnZXrO1pQ3smi0k/jho4HLeY=
 github.com/docker/docker v24.0.5+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
+github.com/docker/docker v24.0.7+incompatible h1:Wo6l37AuwP3JaMnZa226lzVXGA3F9Ig1seQen0cKYlM=
+github.com/docker/docker v24.0.7+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
 github.com/docker/go-connections v0.4.0 h1:El9xVISelRB7BuFusrZozjnkIM5YnzCViNKohAFqRJQ=
 github.com/docker/go-connections v0.4.0/go.mod h1:Gbd7IOopHjR8Iph03tsViu4nIes5XhDvyHbTtUxmeec=
 github.com/docker/go-units v0.4.0 h1:3uh0PgVws3nIA0Q+MwDC8yjEPf9zjRfZZWXZYDct3Tw=

internal/storage/azure/azure.go

@@ -127,12 +127,12 @@ func (b *azureBlobStorage) Prune(deadline time.Time, pruningPrefix string) (*sto
 		}
 	}
 
-	stats := storage.PruneStats{
+	stats := &storage.PruneStats{
 		Total:  totalCount,
 		Pruned: uint(len(matches)),
 	}
 
-	if err := b.DoPrune(b.Name(), len(matches), int(totalCount), func() error {
+	pruneErr := b.DoPrune(b.Name(), len(matches), int(totalCount), deadline, func() error {
 		wg := sync.WaitGroup{}
 		wg.Add(len(matches))
 		var errs []error
@@ -152,9 +152,7 @@ func (b *azureBlobStorage) Prune(deadline time.Time, pruningPrefix string) (*sto
 			return errors.Join(errs...)
 		}
 		return nil
-	}); err != nil {
-		return &stats, err
-	}
+	})
 
-	return &stats, nil
+	return stats, pruneErr
 }

internal/storage/dropbox/dropbox.go

@@ -245,16 +245,14 @@ func (b *dropboxStorage) Prune(deadline time.Time, pruningPrefix string) (*stora
 		Pruned: uint(len(matches)),
 	}
 
-	if err := b.DoPrune(b.Name(), len(matches), lenCandidates, func() error {
+	pruneErr := b.DoPrune(b.Name(), len(matches), lenCandidates, deadline, func() error {
 		for _, match := range matches {
 			if _, err := b.client.DeleteV2(files.NewDeleteArg(filepath.Join(b.DestinationPath, match.Name))); err != nil {
 				return fmt.Errorf("(*dropboxStorage).Prune: Error removing file from Dropbox storage: %w", err)
 			}
 		}
 		return nil
-	}); err != nil {
-		return stats, err
-	}
+	})
 
-	return stats, nil
+	return stats, pruneErr
 }

internal/storage/local/local.go

@@ -116,7 +116,7 @@ func (b *localStorage) Prune(deadline time.Time, pruningPrefix string) (*storage
 		Pruned: uint(len(matches)),
 	}
 
-	if err := b.DoPrune(b.Name(), len(matches), len(candidates), func() error {
+	pruneErr := b.DoPrune(b.Name(), len(matches), len(candidates), deadline, func() error {
 		var removeErrors []error
 		for _, match := range matches {
 			if err := os.Remove(match); err != nil {
@@ -131,11 +131,9 @@ func (b *localStorage) Prune(deadline time.Time, pruningPrefix string) (*storage
 			)
 		}
 		return nil
-	}); err != nil {
-		return stats, err
-	}
+	})
 
-	return stats, nil
+	return stats, pruneErr
 }
 
 // copy creates a copy of the file located at `dst` at `src`.

internal/storage/s3/s3.go

@@ -167,7 +167,7 @@ func (b *s3Storage) Prune(deadline time.Time, pruningPrefix string) (*storage.Pr
 		Pruned: uint(len(matches)),
 	}
 
-	if err := b.DoPrune(b.Name(), len(matches), lenCandidates, func() error {
+	pruneErr := b.DoPrune(b.Name(), len(matches), lenCandidates, deadline, func() error {
 		objectsCh := make(chan minio.ObjectInfo)
 		go func() {
 			for _, match := range matches {
@@ -186,9 +186,7 @@ func (b *s3Storage) Prune(deadline time.Time, pruningPrefix string) (*storage.Pr
 			return errors.Join(removeErrors...)
 		}
 		return nil
-	}); err != nil {
-		return stats, err
-	}
+	})
 
-	return stats, nil
+	return stats, pruneErr
 }

internal/storage/ssh/ssh.go

@@ -174,16 +174,14 @@ func (b *sshStorage) Prune(deadline time.Time, pruningPrefix string) (*storage.P
 		Pruned: uint(len(matches)),
 	}
 
-	if err := b.DoPrune(b.Name(), len(matches), len(candidates), func() error {
+	pruneErr := b.DoPrune(b.Name(), len(matches), len(candidates), deadline, func() error {
 		for _, match := range matches {
 			if err := b.sftpClient.Remove(filepath.Join(b.DestinationPath, match)); err != nil {
 				return fmt.Errorf("(*sshStorage).Prune: error removing file: %w", err)
 			}
 		}
 		return nil
-	}); err != nil {
-		return stats, err
-	}
+	})
 
-	return stats, nil
+	return stats, pruneErr
 }

internal/storage/storage.go

@@ -4,6 +4,7 @@
 package storage
 
 import (
+	"fmt"
 	"time"
 )
 
@@ -17,7 +18,6 @@ type Backend interface {
 // StorageBackend is a generic type of storage. Everything here are common properties of all storage types.
 type StorageBackend struct {
 	DestinationPath string
-	RetentionDays   int
 	Log             Log
 }
 
@@ -39,16 +39,21 @@ type PruneStats struct {
 
 // DoPrune holds general control flow that applies to any kind of storage.
 // Callers can pass in a thunk that performs the actual deletion of files.
-func (b *StorageBackend) DoPrune(context string, lenMatches, lenCandidates int, doRemoveFiles func() error) error {
+func (b *StorageBackend) DoPrune(context string, lenMatches, lenCandidates int, deadline time.Time, doRemoveFiles func() error) error {
 	if lenMatches != 0 && lenMatches != lenCandidates {
 		if err := doRemoveFiles(); err != nil {
 			return err
 		}
+
+		formattedDeadline, err := deadline.Local().MarshalText()
+		if err != nil {
+			return fmt.Errorf("(*StorageBackend).DoPrune: error marshaling deadline: %w", err)
+		}
 		b.Log(LogLevelInfo, context,
-			"Pruned %d out of %d backups as their age exceeded the configured retention period of %d days.",
+			"Pruned %d out of %d backups as they were older than the given deadline of %s.",
 			lenMatches,
 			lenCandidates,
-			b.RetentionDays,
+			string(formattedDeadline),
 		)
 	} else if lenMatches != 0 && lenMatches == lenCandidates {
 		b.Log(LogLevelWarning, context, "The current configuration would delete all %d existing backups.", lenMatches)

internal/storage/webdav/webdav.go

@@ -108,16 +108,13 @@ func (b *webDavStorage) Prune(deadline time.Time, pruningPrefix string) (*storag
 		Pruned: uint(len(matches)),
 	}
 
-	if err := b.DoPrune(b.Name(), len(matches), lenCandidates, func() error {
+	pruneErr := b.DoPrune(b.Name(), len(matches), lenCandidates, deadline, func() error {
 		for _, match := range matches {
 			if err := b.client.Remove(filepath.Join(b.DestinationPath, match.Name())); err != nil {
 				return fmt.Errorf("(*webDavStorage).Prune: error removing file: %w", err)
 			}
 		}
 		return nil
-	}); err != nil {
-		return stats, err
-	}
-
-	return stats, nil
+	})
+	return stats, pruneErr
 }

test/dropbox/run.sh

@@ -50,7 +50,7 @@ info "Create second backup and prune"
 logs=$(docker compose exec -T backup backup)
 
 echo "$logs"
-if echo "$logs" | grep -q "Pruned 1 out of 2 backups as their age exceeded the configured retention period"; then
+if echo "$logs" | grep -q "Pruned 1 out of 2 backups as they were older"; then
   pass "Old remote backup has been pruned, new one is still present."
 elif echo "$logs" | grep -q "ERROR"; then
   fail "Pruning failed, errors reported: $logs"