From fec89f95fb44725b15c6318bef01a1e87875f2c5 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Mon, 1 May 2017 10:13:03 +0200 Subject: [PATCH] Improve swift backend --- run_integration_tests.go | 7 ++ src/cmds/restic/global.go | 35 +----- src/restic/backend/swift/backend_test.go | 87 -------------- src/restic/backend/swift/config.go | 74 +++++++++--- src/restic/backend/swift/config_test.go | 31 ++--- src/restic/backend/swift/swift.go | 136 ++++++++++----------- src/restic/backend/swift/swift_test.go | 147 ++++++++++++++--------- src/restic/backend/test/tests.go | 6 +- src/restic/test/vars.go | 2 - 9 files changed, 245 insertions(+), 280 deletions(-) delete mode 100644 src/restic/backend/swift/backend_test.go diff --git a/run_integration_tests.go b/run_integration_tests.go index eca5ed82f..94c1c80c9 100644 --- a/run_integration_tests.go +++ b/run_integration_tests.go @@ -164,6 +164,13 @@ func (env *TravisEnvironment) RunTests() error { msg("S3 repository not available\n") } + // if the test swift service is available, make sure that the test is not skipped + if os.Getenv("RESTIC_TEST_SWIFT") != "" { + ensureTests = append(ensureTests, "restic/backend/swift.TestBackendSwift") + } else { + msg("Swift service not available\n") + } + env.env["RESTIC_TEST_DISALLOW_SKIP"] = strings.Join(ensureTests, ",") if *runCrossCompile { diff --git a/src/cmds/restic/global.go b/src/cmds/restic/global.go index f6aac00aa..92ab41137 100644 --- a/src/cmds/restic/global.go +++ b/src/cmds/restic/global.go @@ -360,39 +360,8 @@ func parseConfig(loc location.Location, opts options.Options) (interface{}, erro case "swift": cfg := loc.Config.(swift.Config) - for _, val := range []struct { - s *string - env string - }{ - // v2/v3 specific - {&cfg.UserName, "OS_USERNAME"}, - {&cfg.APIKey, "OS_PASSWORD"}, - {&cfg.Region, "OS_REGION_NAME"}, - {&cfg.AuthURL, "OS_AUTH_URL"}, - - // v3 specific - {&cfg.Domain, "OS_USER_DOMAIN_NAME"}, - {&cfg.Tenant, "OS_PROJECT_NAME"}, - {&cfg.TenantDomain, "OS_PROJECT_DOMAIN_NAME"}, - - // v2 specific - {&cfg.TenantID, "OS_TENANT_ID"}, - {&cfg.Tenant, "OS_TENANT_NAME"}, - - // v1 specific - {&cfg.AuthURL, "ST_AUTH"}, - {&cfg.UserName, "ST_USER"}, - {&cfg.APIKey, "ST_KEY"}, - - // Manual authentication - {&cfg.StorageURL, "OS_STORAGE_URL"}, - {&cfg.AuthToken, "OS_AUTH_TOKEN"}, - - {&cfg.DefaultContainerPolicy, "SWIFT_DEFAULT_CONTAINER_POLICY"}, - } { - if *val.s == "" { - *val.s = os.Getenv(val.env) - } + if err := swift.ApplyEnvironment("", &cfg); err != nil { + return nil, err } if err := opts.Apply(loc.Scheme, &cfg); err != nil { diff --git a/src/restic/backend/swift/backend_test.go b/src/restic/backend/swift/backend_test.go deleted file mode 100644 index b4409d145..000000000 --- a/src/restic/backend/swift/backend_test.go +++ /dev/null @@ -1,87 +0,0 @@ -// DO NOT EDIT, AUTOMATICALLY GENERATED -package swift_test - -import ( - "testing" - - "restic/backend/test" -) - -var SkipMessage string - -func TestSwiftBackendCreate(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestCreate(t) -} - -func TestSwiftBackendOpen(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestOpen(t) -} - -func TestSwiftBackendCreateWithConfig(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestCreateWithConfig(t) -} - -func TestSwiftBackendLocation(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLocation(t) -} - -func TestSwiftBackendConfig(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestConfig(t) -} - -func TestSwiftBackendLoad(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestLoad(t) -} - -func TestSwiftBackendSave(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestSave(t) -} - -func TestSwiftBackendSaveFilenames(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestSaveFilenames(t) -} - -func TestSwiftBackendBackend(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestBackend(t) -} - -func TestSwiftBackendDelete(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestDelete(t) -} - -func TestSwiftBackendCleanup(t *testing.T) { - if SkipMessage != "" { - t.Skip(SkipMessage) - } - test.TestCleanup(t) -} diff --git a/src/restic/backend/swift/config.go b/src/restic/backend/swift/config.go index 8178f00e3..78765e56d 100644 --- a/src/restic/backend/swift/config.go +++ b/src/restic/backend/swift/config.go @@ -1,13 +1,9 @@ package swift import ( - "net/url" - "regexp" + "os" "restic/errors" -) - -var ( - urlParser = regexp.MustCompile("^([^:]+):/(.*)$") + "strings" ) // Config contains basic configuration needed to specify swift location for a swift server @@ -32,21 +28,69 @@ type Config struct { // ParseConfig parses the string s and extract swift's container name and prefix. func ParseConfig(s string) (interface{}, error) { - - url, err := url.Parse(s) - if err != nil { - return nil, errors.Wrap(err, "url.Parse") + data := strings.SplitN(s, ":", 3) + if len(data) != 3 { + return nil, errors.New("invalid URL, expected: swift:container-name:/[prefix]") } - m := urlParser.FindStringSubmatch(url.Opaque) - if len(m) == 0 { - return nil, errors.New("swift: invalid URL, valid syntax is: 'swift:container-name:/[optional-prefix]'") + scheme, container, prefix := data[0], data[1], data[2] + if scheme != "swift" { + return nil, errors.Errorf("unexpected prefix: %s", data[0]) } + if len(prefix) == 0 { + return nil, errors.Errorf("prefix is empty") + } + + if prefix[0] != '/' { + return nil, errors.Errorf("prefix does not start with slash (/)") + } + prefix = prefix[1:] + cfg := Config{ - Container: m[1], - Prefix: m[2], + Container: container, + Prefix: prefix, } return cfg, nil } + +// ApplyEnvironment saves values from the environment to the config. +func ApplyEnvironment(prefix string, cfg interface{}) error { + c := cfg.(*Config) + for _, val := range []struct { + s *string + env string + }{ + // v2/v3 specific + {&c.UserName, prefix + "OS_USERNAME"}, + {&c.APIKey, prefix + "OS_PASSWORD"}, + {&c.Region, prefix + "OS_REGION_NAME"}, + {&c.AuthURL, prefix + "OS_AUTH_URL"}, + + // v3 specific + {&c.Domain, prefix + "OS_USER_DOMAIN_NAME"}, + {&c.Tenant, prefix + "OS_PROJECT_NAME"}, + {&c.TenantDomain, prefix + "OS_PROJECT_DOMAIN_NAME"}, + + // v2 specific + {&c.TenantID, prefix + "OS_TENANT_ID"}, + {&c.Tenant, prefix + "OS_TENANT_NAME"}, + + // v1 specific + {&c.AuthURL, prefix + "ST_AUTH"}, + {&c.UserName, prefix + "ST_USER"}, + {&c.APIKey, prefix + "ST_KEY"}, + + // Manual authentication + {&c.StorageURL, prefix + "OS_STORAGE_URL"}, + {&c.AuthToken, prefix + "OS_AUTH_TOKEN"}, + + {&c.DefaultContainerPolicy, prefix + "SWIFT_DEFAULT_CONTAINER_POLICY"}, + } { + if *val.s == "" { + *val.s = os.Getenv(val.env) + } + } + return nil +} diff --git a/src/restic/backend/swift/config_test.go b/src/restic/backend/swift/config_test.go index 97ec8d9c6..bd087bd47 100644 --- a/src/restic/backend/swift/config_test.go +++ b/src/restic/backend/swift/config_test.go @@ -9,23 +9,26 @@ var configTests = []struct { {"swift:cnt1:/", Config{Container: "cnt1", Prefix: ""}}, {"swift:cnt2:/prefix", Config{Container: "cnt2", Prefix: "prefix"}}, {"swift:cnt3:/prefix/longer", Config{Container: "cnt3", Prefix: "prefix/longer"}}, - {"swift:cnt4:/prefix?params", Config{Container: "cnt4", Prefix: "prefix"}}, - {"swift:cnt5:/prefix#params", Config{Container: "cnt5", Prefix: "prefix"}}, } -func TestParseConfigInternal(t *testing.T) { - for i, test := range configTests { - cfg, err := ParseConfig(test.s) - if err != nil { - t.Errorf("test %d:%s failed: %v", i, test.s, err) - continue - } +func TestParseConfig(t *testing.T) { + for _, test := range configTests { + t.Run("", func(t *testing.T) { + v, err := ParseConfig(test.s) + if err != nil { + t.Fatalf("parsing %q failed: %v", test.s, err) + } - if cfg != test.cfg { - t.Errorf("test %d:\ninput:\n %s\n wrong config, want:\n %v\ngot:\n %v", - i, test.s, test.cfg, cfg) - continue - } + cfg, ok := v.(Config) + if !ok { + t.Fatalf("wrong type returned, want Config, got %T", cfg) + } + + if cfg != test.cfg { + t.Fatalf("wrong output for %q, want:\n %#v\ngot:\n %#v", + test.s, test.cfg, cfg) + } + }) } } diff --git a/src/restic/backend/swift/swift.go b/src/restic/backend/swift/swift.go index 3d6bffe0f..733dc3221 100644 --- a/src/restic/backend/swift/swift.go +++ b/src/restic/backend/swift/swift.go @@ -1,8 +1,11 @@ package swift import ( + "fmt" "io" + "net/http" "path" + "path/filepath" "restic" "restic/backend" "restic/debug" @@ -21,11 +24,13 @@ type beSwift struct { connChan chan struct{} container string // Container name prefix string // Prefix of object names in the container + backend.Layout } // Open opens the swift backend at a container in region. The container is // created if it does not exist yet. func Open(cfg Config) (restic.Backend, error) { + debug.Log("config %#v", cfg) be := &beSwift{ conn: &swift.Connection{ @@ -42,9 +47,15 @@ func Open(cfg Config) (restic.Backend, error) { AuthToken: cfg.AuthToken, ConnectTimeout: time.Minute, Timeout: time.Minute, + + Transport: backend.Transport(), }, container: cfg.Container, prefix: cfg.Prefix, + Layout: &backend.DefaultLayout{ + Path: cfg.Prefix, + Join: path.Join, + }, } be.createConnections() @@ -70,32 +81,17 @@ func Open(cfg Config) (restic.Backend, error) { return nil, errors.Wrap(err, "conn.Container") } - return be, nil -} - -func (be *beSwift) swiftpath(h restic.Handle) string { - - var dir string - - switch h.Type { - case restic.ConfigFile: - dir = "" - h.Name = backend.Paths.Config - case restic.DataFile: - dir = backend.Paths.Data - case restic.SnapshotFile: - dir = backend.Paths.Snapshots - case restic.IndexFile: - dir = backend.Paths.Index - case restic.LockFile: - dir = backend.Paths.Locks - case restic.KeyFile: - dir = backend.Paths.Keys - default: - dir = string(h.Type) + // check that the server supports byte ranges + _, hdr, err := be.conn.Account() + if err != nil { + return nil, errors.Wrap(err, "Account()") } - return path.Join(be.prefix, dir, h.Name) + if hdr["Accept-Ranges"] != "bytes" { + return nil, errors.New("backend does not support byte range") + } + + return be, nil } func (be *beSwift) createConnections() { @@ -138,49 +134,33 @@ func (be *beSwift) Load(h restic.Handle, length int, offset int64) (io.ReadClose return nil, errors.Errorf("invalid length %d", length) } - objName := be.swiftpath(h) + objName := be.Filename(h) <-be.connChan defer func() { be.connChan <- struct{}{} }() - obj, _, err := be.conn.ObjectOpen(be.container, objName, false, nil) + headers := swift.Headers{} + if offset > 0 { + headers["Range"] = fmt.Sprintf("bytes=%d-", offset) + } + + if length > 0 { + headers["Range"] = fmt.Sprintf("bytes=%d-%d", offset, offset+int64(length)-1) + } + + if _, ok := headers["Range"]; ok { + debug.Log("Load(%v) send range %v", h, headers["Range"]) + } + + obj, _, err := be.conn.ObjectOpen(be.container, objName, false, headers) if err != nil { debug.Log(" err %v", err) return nil, errors.Wrap(err, "conn.ObjectOpen") } - // if we're going to read the whole object, just pass it on. - if length == 0 { - debug.Log("Load %v: pass on object", h) - _, err = obj.Seek(offset, 0) - if err != nil { - _ = obj.Close() - return nil, errors.Wrap(err, "obj.Seek") - } - - return obj, nil - } - - // otherwise pass a LimitReader - size, err := obj.Length() - if err != nil { - return nil, errors.Wrap(err, "obj.Length") - } - - if offset > size { - _ = obj.Close() - return nil, errors.Errorf("offset larger than file size") - } - - _, err = obj.Seek(offset, 0) - if err != nil { - _ = obj.Close() - return nil, errors.Wrap(err, "obj.Seek") - } - - return backend.LimitReadCloser(obj, int64(length)), nil + return obj, nil } // Save stores data in the backend at the handle. @@ -189,9 +169,9 @@ func (be *beSwift) Save(h restic.Handle, rd io.Reader) (err error) { return err } - debug.Log("Save %v", h) + objName := be.Filename(h) - objName := be.swiftpath(h) + debug.Log("Save %v at %v", h, objName) // Check key does not already exist switch _, _, err = be.conn.Object(be.container, objName); err { @@ -213,9 +193,7 @@ func (be *beSwift) Save(h restic.Handle, rd io.Reader) (err error) { encoding := "binary/octet-stream" - debug.Log("PutObject(%v, %v, %v)", - be.container, objName, encoding) - //err = be.conn.ObjectPutBytes(be.container, objName, p, encoding) + debug.Log("PutObject(%v, %v, %v)", be.container, objName, encoding) _, err = be.conn.ObjectPut(be.container, objName, rd, true, "", encoding, nil) debug.Log("%v, err %#v", objName, err) @@ -226,7 +204,7 @@ func (be *beSwift) Save(h restic.Handle, rd io.Reader) (err error) { func (be *beSwift) Stat(h restic.Handle) (bi restic.FileInfo, err error) { debug.Log("%v", h) - objName := be.swiftpath(h) + objName := be.Filename(h) obj, _, err := be.conn.Object(be.container, objName) if err != nil { @@ -239,7 +217,7 @@ func (be *beSwift) Stat(h restic.Handle) (bi restic.FileInfo, err error) { // Test returns true if a blob of the given type and name exists in the backend. func (be *beSwift) Test(h restic.Handle) (bool, error) { - objName := be.swiftpath(h) + objName := be.Filename(h) switch _, _, err := be.conn.Object(be.container, objName); err { case nil: return true, nil @@ -254,7 +232,7 @@ func (be *beSwift) Test(h restic.Handle) (bool, error) { // Remove removes the blob with the given name and type. func (be *beSwift) Remove(h restic.Handle) error { - objName := be.swiftpath(h) + objName := be.Filename(h) err := be.conn.ObjectDelete(be.container, objName) debug.Log("Remove(%v) -> err %v", h, err) return errors.Wrap(err, "conn.ObjectDelete") @@ -267,19 +245,19 @@ func (be *beSwift) List(t restic.FileType, done <-chan struct{}) <-chan string { debug.Log("listing %v", t) ch := make(chan string) - prefix := be.swiftpath(restic.Handle{Type: t}) + "/" + prefix := be.Filename(restic.Handle{Type: t}) + "/" go func() { defer close(ch) - be.conn.ObjectsWalk(be.container, &swift.ObjectsOpts{Prefix: prefix}, + err := be.conn.ObjectsWalk(be.container, &swift.ObjectsOpts{Prefix: prefix}, func(opts *swift.ObjectsOpts) (interface{}, error) { newObjects, err := be.conn.ObjectNames(be.container, opts) if err != nil { return nil, errors.Wrap(err, "conn.ObjectNames") } for _, obj := range newObjects { - m := strings.TrimPrefix(obj, prefix) + m := filepath.Base(strings.TrimPrefix(obj, prefix)) if m == "" { continue } @@ -292,6 +270,10 @@ func (be *beSwift) List(t restic.FileType, done <-chan struct{}) <-chan string { } return newObjects, nil }) + + if err != nil { + debug.Log("ObjectsWalk returned error: %v", err) + } }() return ch @@ -301,8 +283,8 @@ func (be *beSwift) List(t restic.FileType, done <-chan struct{}) <-chan string { func (be *beSwift) removeKeys(t restic.FileType) error { done := make(chan struct{}) defer close(done) - for key := range be.List(restic.DataFile, done) { - err := be.Remove(restic.Handle{Type: restic.DataFile, Name: key}) + for key := range be.List(t, done) { + err := be.Remove(restic.Handle{Type: t, Name: key}) if err != nil { return err } @@ -311,6 +293,15 @@ func (be *beSwift) removeKeys(t restic.FileType) error { return nil } +// IsNotExist returns true if the error is caused by a not existing file. +func (be *beSwift) IsNotExist(err error) bool { + if e, ok := errors.Cause(err).(*swift.Error); ok { + return e.StatusCode == http.StatusNotFound + } + + return false +} + // Delete removes all restic objects in the container. // It will not remove the container itself. func (be *beSwift) Delete() error { @@ -328,7 +319,12 @@ func (be *beSwift) Delete() error { } } - return be.Remove(restic.Handle{Type: restic.ConfigFile}) + err := be.Remove(restic.Handle{Type: restic.ConfigFile}) + if err != nil && !be.IsNotExist(err) { + return err + } + + return nil } // Close does nothing diff --git a/src/restic/backend/swift/swift_test.go b/src/restic/backend/swift/swift_test.go index 2f3976b0c..b53b7bb64 100644 --- a/src/restic/backend/swift/swift_test.go +++ b/src/restic/backend/swift/swift_test.go @@ -2,75 +2,106 @@ package swift_test import ( "fmt" - "math/rand" + "os" "restic" + "testing" "time" "restic/errors" + . "restic/test" "restic/backend/swift" "restic/backend/test" - . "restic/test" - - swiftclient "github.com/ncw/swift" ) -//go:generate go run ../test/generate_backend_tests.go +func newSwiftTestSuite(t testing.TB) *test.Suite { + return &test.Suite{ + // do not use excessive data + MinimalData: true, -func init() { - if TestSwiftServer == "" { - SkipMessage = "swift test server not available" + // NewConfig returns a config for a new temporary backend that will be used in tests. + NewConfig: func() (interface{}, error) { + swiftcfg, err := swift.ParseConfig(os.Getenv("RESTIC_TEST_SWIFT")) + if err != nil { + return nil, err + } + + cfg := swiftcfg.(swift.Config) + if err = swift.ApplyEnvironment("RESTIC_TEST_", &cfg); err != nil { + return nil, err + } + cfg.Prefix += fmt.Sprintf("/test-%d", time.Now().UnixNano()) + t.Logf("using prefix %v", cfg.Prefix) + return cfg, nil + }, + + // CreateFn is a function that creates a temporary repository for the tests. + Create: func(config interface{}) (restic.Backend, error) { + cfg := config.(swift.Config) + + be, err := swift.Open(cfg) + if err != nil { + return nil, err + } + + exists, err := be.Test(restic.Handle{Type: restic.ConfigFile}) + if err != nil { + return nil, err + } + + if exists { + return nil, errors.New("config already exists") + } + + return be, nil + }, + + // OpenFn is a function that opens a previously created temporary repository. + Open: func(config interface{}) (restic.Backend, error) { + cfg := config.(swift.Config) + return swift.Open(cfg) + }, + + // CleanupFn removes data created during the tests. + Cleanup: func(config interface{}) error { + cfg := config.(swift.Config) + + be, err := swift.Open(cfg) + if err != nil { + return err + } + + if err := be.(restic.Deleter).Delete(); err != nil { + return err + } + + return nil + }, + } +} + +func TestBackendSwift(t *testing.T) { + defer func() { + if t.Skipped() { + SkipDisallowed(t, "restic/backend/swift.TestBackendSwift") + } + }() + + if os.Getenv("RESTIC_TEST_SWIFT") == "" { + t.Skip("RESTIC_TEST_SWIFT unset, skipping test") return } - // Generate random container name to allow simultaneous test - // on the same swift backend - containerName := fmt.Sprintf( - "restictestcontainer_%d_%d", - time.Now().Unix(), - rand.Uint32(), - ) - - cfg := swift.Config{ - Container: containerName, - StorageURL: TestSwiftServer, - AuthToken: TestSwiftToken, - } - - test.CreateFn = func() (restic.Backend, error) { - be, err := swift.Open(cfg) - if err != nil { - return nil, err - } - - exists, err := be.Test(restic.Handle{Type: restic.ConfigFile}) - if err != nil { - return nil, err - } - - if exists { - return nil, errors.New("config already exists") - } - - return be, nil - } - - test.OpenFn = func() (restic.Backend, error) { - return swift.Open(cfg) - } - - test.CleanupFn = func() error { - client := swiftclient.Connection{ - StorageUrl: TestSwiftServer, - AuthToken: TestSwiftToken, - } - objects, err := client.ObjectsAll(containerName, nil) - if err != nil { - return err - } - for _, o := range objects { - client.ObjectDelete(containerName, o.Name) - } - return client.ContainerDelete(containerName) - } + t.Logf("run tests") + newSwiftTestSuite(t).RunTests(t) +} + +func BenchmarkBackendSwift(t *testing.B) { + if os.Getenv("RESTIC_TEST_SWIFT") == "" { + t.Skip("RESTIC_TEST_SWIFT unset, skipping test") + return + } + + t.Logf("run tests") + newSwiftTestSuite(t).RunBenchmarks(t) } diff --git a/src/restic/backend/test/tests.go b/src/restic/backend/test/tests.go index ba7ea20be..61136cea7 100644 --- a/src/restic/backend/test/tests.go +++ b/src/restic/backend/test/tests.go @@ -438,8 +438,12 @@ func delayedRemove(b restic.Backend, h restic.Handle) error { // Some backend (swift, I'm looking at you) may implement delayed // removal of data. Let's wait a bit if this happens. err := b.Remove(h) + if err != nil { + return err + } + found, err := b.Test(h) - for i := 0; found && i < 10; i++ { + for i := 0; found && i < 20; i++ { found, err = b.Test(h) if found { time.Sleep(100 * time.Millisecond) diff --git a/src/restic/test/vars.go b/src/restic/test/vars.go index f78c5ba6f..616f969f6 100644 --- a/src/restic/test/vars.go +++ b/src/restic/test/vars.go @@ -18,8 +18,6 @@ var ( BenchArchiveDirectory = getStringVar("RESTIC_BENCH_DIR", ".") TestS3Server = getStringVar("RESTIC_TEST_S3_SERVER", "") TestRESTServer = getStringVar("RESTIC_TEST_REST_SERVER", "") - TestSwiftServer = getStringVar("RESTIC_TEST_SWIFT_SERVER", "") - TestSwiftToken = getStringVar("RESTIC_TEST_SWIFT_TOKEN", "") TestIntegrationDisallowSkip = getStringVar("RESTIC_TEST_DISALLOW_SKIP", "") )