Skip to content

Commit 6764cb6

Browse files
deitchdeitch
authored
Merge pull request #398 from databacker/s3-prune-test
Fix S3 pruning and add s3 variants to prune tests
2 parents 6bdbc01 + 73e624b commit 6764cb6

File tree

2 files changed

+107
-53
lines changed

2 files changed

+107
-53
lines changed

‎pkg/core/prune.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"path"
78
"regexp"
89
"slices"
910
"strconv"
@@ -62,7 +63,7 @@ func pruneTarget(ctx context.Context, logger *logrus.Entry, target storage.Stora
6263
defer span.End()
6364

6465
logger.Debugf("pruning target %s", target.URL())
65-
files, err := target.ReadDir(ctx, ".", logger)
66+
files, err := target.ReadDir(ctx, "", logger)
6667
if err != nil {
6768
span.SetStatus(codes.Error, fmt.Sprintf("failed to read directory: %v", err))
6869
return fmt.Errorf("failed to read directory: %v", err)
@@ -73,7 +74,10 @@ func pruneTarget(ctx context.Context, logger *logrus.Entry, target storage.Stora
7374

7475
for _, fileInfo := range files {
7576
filename := fileInfo.Name()
76-
matches := filenameRE.FindStringSubmatch(filename)
77+
// this should be the basename, but sometimes it is a full path, like in S3, so we will be careful to trim
78+
// to basename. If it already is basename, nothing should be affected
79+
baseFilename := path.Base(filename)
80+
matches := filenameRE.FindStringSubmatch(baseFilename)
7781
if matches == nil {
7882
logger.Debugf("ignoring filename that is not standard backup pattern: %s", filename)
7983
ignored = append(ignored, filename)

‎pkg/core/prune_test.go‎

Lines changed: 101 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,18 @@ import (
44
"context"
55
"fmt"
66
"io"
7+
"net/http/httptest"
78
"os"
9+
"path"
810
"slices"
911
"testing"
1012
"time"
1113

1214
"github.com/databacker/mysql-backup/pkg/storage"
1315
"github.com/databacker/mysql-backup/pkg/storage/credentials"
16+
17+
"github.com/johannesboyne/gofakes3"
18+
"github.com/johannesboyne/gofakes3/backend/s3mem"
1419
log "github.com/sirupsen/logrus"
1520
"github.com/stretchr/testify/assert"
1621
)
@@ -77,7 +82,6 @@ func TestPrune(t *testing.T) {
7782
err error
7883
}{
7984
{"no targets", PruneOptions{Retention: "1h", Now: now}, nil, nil, fmt.Errorf("no targets")},
80-
// 1 hour - file[1] is 1h+30m = 1.5h, so it should be pruned
8185
{"invalid format", PruneOptions{Retention: "100x", Now: now}, filenames, filenames[0:1], fmt.Errorf("invalid retention string: 100x")},
8286
// 1 hour - file[1] is 1h+30m = 1.5h, so it should be pruned
8387
{"1 hour", PruneOptions{Retention: "1h", Now: now}, filenames, filenames[0:1], nil},
@@ -101,60 +105,106 @@ func TestPrune(t *testing.T) {
101105
// 2 most recent files
102106
{"2 most recent safe names", PruneOptions{Retention: "2c", Now: now}, safefilenames, safefilenames[0:2], nil},
103107
}
104-
for _, tt := range tests {
105-
t.Run(tt.name, func(t *testing.T) {
106-
ctx := context.Background()
107-
// create a temporary directory
108-
workDir := t.TempDir()
109-
// create beforeFiles in the directory and create a target, but only if there are beforeFiles
110-
// this lets us also test no targets, which should generate an error
111-
if len(tt.beforeFiles) > 0 {
112-
for _, filename := range tt.beforeFiles {
113-
if err := os.WriteFile(fmt.Sprintf("%s/%s", workDir, filename), nil, 0644); err != nil {
114-
t.Errorf("failed to create file %s: %v", filename, err)
115-
return
116-
}
117-
}
108+
for _, targetType := range []string{"file", "s3"} {
109+
t.Run(targetType, func(t *testing.T) {
110+
for _, tt := range tests {
111+
t.Run(tt.name, func(t *testing.T) {
112+
ctx := context.Background()
113+
logger := log.New()
114+
logger.Out = io.Discard
115+
// create a temporary directory
116+
// create beforeFiles in the directory and create a target, but only if there are beforeFiles
117+
// this lets us also test no targets, which should generate an error
118+
if len(tt.beforeFiles) > 0 {
119+
var (
120+
store storage.Storage
121+
err error
122+
)
123+
switch targetType {
124+
case "file":
125+
// add our tempdir as the target
126+
workDir := t.TempDir()
127+
store, err = storage.ParseURL(fmt.Sprintf("file://%s", workDir), credentials.Creds{})
128+
if err != nil {
129+
t.Errorf("failed to parse file url: %v", err)
130+
return
131+
}
132+
case "s3":
133+
bucketName := "mytestbucket"
134+
s3backend := s3mem.New()
135+
// create the bucket we will use for tests
136+
if err := s3backend.CreateBucket(bucketName); err != nil {
137+
t.Errorf("failed to create bucket: %v", err)
138+
return
139+
}
140+
s3 := gofakes3.New(s3backend)
141+
s3server := httptest.NewServer(s3.Server())
142+
defer s3server.Close()
143+
s3url := s3server.URL
144+
store, err = storage.ParseURL(fmt.Sprintf("s3://%s/%s", bucketName, bucketName), credentials.Creds{AWS: credentials.AWSCreds{
145+
Endpoint: s3url,
146+
AccessKeyID: "abcdefg",
147+
SecretAccessKey: "1234567",
148+
Region: "us-east-1",
149+
PathStyle: true,
150+
}})
151+
if err != nil {
152+
t.Errorf("failed to parse s3 url: %v", err)
153+
return
154+
}
155+
default:
156+
t.Errorf("unknown target type: %s", targetType)
157+
return
158+
}
118159

119-
// add our tempdir as the target
120-
store, err := storage.ParseURL(fmt.Sprintf("file://%s", workDir), credentials.Creds{})
121-
if err != nil {
122-
t.Errorf("failed to parse url: %v", err)
123-
return
124-
}
160+
tt.opts.Targets = append(tt.opts.Targets, store)
125161

126-
tt.opts.Targets = append(tt.opts.Targets, store)
127-
}
162+
for _, filename := range tt.beforeFiles {
163+
// we need an empty file to push
164+
srcDir := t.TempDir()
165+
srcFile := fmt.Sprintf("%s/%s", srcDir, "src")
166+
if err := os.WriteFile(srcFile, nil, 0644); err != nil {
167+
t.Errorf("failed to create file %s: %v", srcFile, err)
168+
return
169+
}
128170

129-
// run Prune
130-
logger := log.New()
131-
logger.Out = io.Discard
132-
executor := Executor{
133-
Logger: logger,
134-
}
135-
err := executor.Prune(ctx, tt.opts)
136-
switch {
137-
case (err == nil && tt.err != nil) || (err != nil && tt.err == nil):
138-
t.Errorf("expected error %v, got %v", tt.err, err)
139-
case err != nil && tt.err != nil && err.Error() != tt.err.Error():
140-
t.Errorf("expected error %v, got %v", tt.err, err)
141-
case err != nil:
142-
return
143-
}
144-
// check files match
145-
files, err := os.ReadDir(workDir)
146-
if err != nil {
147-
t.Errorf("failed to read directory: %v", err)
148-
return
149-
}
150-
var afterFiles []string
151-
for _, file := range files {
152-
afterFiles = append(afterFiles, file.Name())
171+
// now push that same empty file each time; we do not care about contents, only that the target file exists
172+
if _, err := store.Push(ctx, filename, srcFile, log.NewEntry(logger)); err != nil {
173+
t.Errorf("failed to create file %s: %v", filename, err)
174+
return
175+
}
176+
}
177+
}
178+
179+
// run Prune
180+
executor := Executor{
181+
Logger: logger,
182+
}
183+
err := executor.Prune(ctx, tt.opts)
184+
switch {
185+
case (err == nil && tt.err != nil) || (err != nil && tt.err == nil):
186+
t.Errorf("expected error %v, got %v", tt.err, err)
187+
case err != nil && tt.err != nil && err.Error() != tt.err.Error():
188+
t.Errorf("expected error %v, got %v", tt.err, err)
189+
case err != nil:
190+
return
191+
}
192+
// check files match
193+
files, err := tt.opts.Targets[0].ReadDir(ctx, "", log.NewEntry(logger))
194+
if err != nil {
195+
t.Errorf("failed to read directory: %v", err)
196+
return
197+
}
198+
var afterFiles []string
199+
for _, file := range files {
200+
afterFiles = append(afterFiles, path.Base(file.Name()))
201+
}
202+
afterFilesSorted, ttAfterFilesSorted := slices.Clone(afterFiles), slices.Clone(tt.afterFiles)
203+
slices.Sort(afterFilesSorted)
204+
slices.Sort(ttAfterFilesSorted)
205+
assert.ElementsMatch(t, ttAfterFilesSorted, afterFilesSorted)
206+
})
153207
}
154-
afterFilesSorted, ttAfterFilesSorted := slices.Clone(afterFiles), slices.Clone(tt.afterFiles)
155-
slices.Sort(afterFilesSorted)
156-
slices.Sort(ttAfterFilesSorted)
157-
assert.ElementsMatch(t, ttAfterFilesSorted, afterFilesSorted)
158208
})
159209
}
160210
}

0 commit comments

Comments
 (0)