Pruning method logs nonsensical configuration values (#301)

* Pruning method logs nonsensical configuration values

* Adjust test assertion about log output
This commit is contained in:
Frederik Ring 2023-11-04 12:19:44 +01:00 committed by GitHub
parent d3e1d1531b
commit 0325889ac4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 38 additions and 46 deletions

View File

@ -172,11 +172,11 @@ func newScript() (*script, error) {
CACert: s.c.AwsEndpointCACert.Cert, CACert: s.c.AwsEndpointCACert.Cert,
PartSize: s.c.AwsPartSize, 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) 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 != "" { if s.c.WebdavUrl != "" {
@ -187,11 +187,11 @@ func newScript() (*script, error) {
Password: s.c.WebdavPassword, Password: s.c.WebdavPassword,
RemotePath: s.c.WebdavPath, 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) 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 != "" { if s.c.SSHHostName != "" {
@ -204,11 +204,11 @@ func newScript() (*script, error) {
IdentityPassphrase: s.c.SSHIdentityPassphrase, IdentityPassphrase: s.c.SSHIdentityPassphrase,
RemotePath: s.c.SSHRemotePath, 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) 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) { if _, err := os.Stat(s.c.BackupArchive); !os.IsNotExist(err) {

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -4,6 +4,7 @@
package storage package storage
import ( import (
"fmt"
"time" "time"
) )
@ -17,7 +18,6 @@ type Backend interface {
// StorageBackend is a generic type of storage. Everything here are common properties of all storage types. // StorageBackend is a generic type of storage. Everything here are common properties of all storage types.
type StorageBackend struct { type StorageBackend struct {
DestinationPath string DestinationPath string
RetentionDays int
Log Log Log Log
} }
@ -39,16 +39,21 @@ type PruneStats struct {
// DoPrune holds general control flow that applies to any kind of storage. // 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. // 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 lenMatches != 0 && lenMatches != lenCandidates {
if err := doRemoveFiles(); err != nil { if err := doRemoveFiles(); err != nil {
return err return err
} }
formattedDeadline, err := deadline.Local().MarshalText()
if err != nil {
return fmt.Errorf("(*StorageBackend).DoPrune: error marshaling deadline: %w", err)
}
b.Log(LogLevelInfo, context, 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, lenMatches,
lenCandidates, lenCandidates,
b.RetentionDays, string(formattedDeadline),
) )
} else if lenMatches != 0 && lenMatches == lenCandidates { } else if lenMatches != 0 && lenMatches == lenCandidates {
b.Log(LogLevelWarning, context, "The current configuration would delete all %d existing backups.", lenMatches) b.Log(LogLevelWarning, context, "The current configuration would delete all %d existing backups.", lenMatches)

View File

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

View File

@ -50,7 +50,7 @@ info "Create second backup and prune"
logs=$(docker compose exec -T backup backup) logs=$(docker compose exec -T backup backup)
echo "$logs" 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." pass "Old remote backup has been pruned, new one is still present."
elif echo "$logs" | grep -q "ERROR"; then elif echo "$logs" | grep -q "ERROR"; then
fail "Pruning failed, errors reported: $logs" fail "Pruning failed, errors reported: $logs"