From dd6ce5f9d8a68fd084824c8078495483ccb45dc0 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Jun 2017 15:58:23 +0200 Subject: [PATCH 1/7] Remove backend.Closer, use ioutil.NopCloser() instead --- src/restic/backend/mem/mem_backend.go | 3 +-- src/restic/backend/rest/rest.go | 5 ++--- src/restic/backend/utils.go | 10 ---------- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/restic/backend/mem/mem_backend.go b/src/restic/backend/mem/mem_backend.go index bbb4dbd1a..6c3296604 100644 --- a/src/restic/backend/mem/mem_backend.go +++ b/src/restic/backend/mem/mem_backend.go @@ -8,7 +8,6 @@ import ( "restic" "sync" - "restic/backend" "restic/errors" "restic/debug" @@ -114,7 +113,7 @@ func (be *MemoryBackend) Load(ctx context.Context, h restic.Handle, length int, buf = buf[:length] } - return backend.Closer{Reader: bytes.NewReader(buf)}, nil + return ioutil.NopCloser(bytes.NewReader(buf)), nil } // Stat returns information about a file in the backend. diff --git a/src/restic/backend/rest/rest.go b/src/restic/backend/rest/rest.go index 4145a2a32..337d8584c 100644 --- a/src/restic/backend/rest/rest.go +++ b/src/restic/backend/rest/rest.go @@ -108,9 +108,8 @@ func (b *restBackend) Save(ctx context.Context, h restic.Handle, rd io.Reader) ( ctx, cancel := context.WithCancel(ctx) defer cancel() - // make sure that client.Post() cannot close the reader by wrapping it in - // backend.Closer, which has a noop method. - rd = backend.Closer{Reader: rd} + // make sure that client.Post() cannot close the reader by wrapping it + rd = ioutil.NopCloser(rd) b.sem.GetToken() resp, err := ctxhttp.Post(ctx, b.client, b.Filename(h), "binary/octet-stream", rd) diff --git a/src/restic/backend/utils.go b/src/restic/backend/utils.go index a07c7e86e..76e9de569 100644 --- a/src/restic/backend/utils.go +++ b/src/restic/backend/utils.go @@ -29,16 +29,6 @@ func LoadAll(ctx context.Context, be restic.Backend, h restic.Handle) (buf []byt return ioutil.ReadAll(rd) } -// Closer wraps an io.Reader and adds a Close() method that does nothing. -type Closer struct { - io.Reader -} - -// Close is a no-op. -func (c Closer) Close() error { - return nil -} - // LimitedReadCloser wraps io.LimitedReader and exposes the Close() method. type LimitedReadCloser struct { io.ReadCloser From 9053b2000b86798cd661ecdd02c0ae22ee624de8 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Jun 2017 16:06:47 +0200 Subject: [PATCH 2/7] s3: Delete ignores error if the object doesn't exist --- src/restic/backend/s3/s3.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/restic/backend/s3/s3.go b/src/restic/backend/s3/s3.go index aa557e7e4..7f14e72cd 100644 --- a/src/restic/backend/s3/s3.go +++ b/src/restic/backend/s3/s3.go @@ -291,6 +291,11 @@ func (be *Backend) Remove(ctx context.Context, h restic.Handle) error { objName := be.Filename(h) err := be.client.RemoveObject(be.bucketname, objName) debug.Log("Remove(%v) at %v -> err %v", h, objName, err) + + if be.IsNotExist(err) { + err = nil + } + return errors.Wrap(err, "client.RemoveObject") } From 51877cecf74c3844befa20762c1b97d6f38f34b8 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Jun 2017 16:39:42 +0200 Subject: [PATCH 3/7] s3: Prevent closing of the reader for GCS --- src/restic/backend/s3/s3.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/restic/backend/s3/s3.go b/src/restic/backend/s3/s3.go index 7f14e72cd..beb557bdf 100644 --- a/src/restic/backend/s3/s3.go +++ b/src/restic/backend/s3/s3.go @@ -4,6 +4,8 @@ import ( "context" "fmt" "io" + "io/ioutil" + "net/url" "os" "path" "restic" @@ -14,6 +16,7 @@ import ( "restic/errors" "github.com/minio/minio-go" + "github.com/minio/minio-go/pkg/s3utils" "restic/debug" ) @@ -22,6 +25,7 @@ import ( type Backend struct { client *minio.Client sem *backend.Semaphore + cfg Config bucketname string prefix string backend.Layout @@ -50,6 +54,7 @@ func Open(cfg Config) (restic.Backend, error) { be := &Backend{ client: client, sem: sem, + cfg: cfg, bucketname: cfg.Bucket, prefix: cfg.Prefix, } @@ -157,6 +162,19 @@ func (be *Backend) Path() string { return be.prefix } +func (be *Backend) isGoogleCloudStorage() bool { + scheme := "https://" + if be.cfg.UseHTTP { + scheme = "http://" + } + url, err := url.Parse(scheme + be.cfg.Endpoint) + if err != nil { + panic(err) + } + + return s3utils.IsGoogleEndpoint(*url) +} + // Save stores data in the backend at the handle. func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err error) { debug.Log("Save %v", h) @@ -174,6 +192,11 @@ func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err return errors.New("key already exists") } + // prevent GCS from closing the file + if be.isGoogleCloudStorage() { + rd = ioutil.NopCloser(rd) + } + be.sem.GetToken() debug.Log("PutObject(%v, %v)", be.bucketname, objName) n, err := be.client.PutObject(be.bucketname, objName, rd, "application/octet-stream") From 98ae7b1210df224302df845147a48d6fb2855b7c Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Jun 2017 16:41:09 +0200 Subject: [PATCH 4/7] s3: Save config in backend --- src/restic/backend/s3/s3.go | 42 +++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/restic/backend/s3/s3.go b/src/restic/backend/s3/s3.go index beb557bdf..2fb88d2a6 100644 --- a/src/restic/backend/s3/s3.go +++ b/src/restic/backend/s3/s3.go @@ -23,11 +23,9 @@ import ( // Backend stores data on an S3 endpoint. type Backend struct { - client *minio.Client - sem *backend.Semaphore - cfg Config - bucketname string - prefix string + client *minio.Client + sem *backend.Semaphore + cfg Config backend.Layout } @@ -52,11 +50,9 @@ func Open(cfg Config) (restic.Backend, error) { } be := &Backend{ - client: client, - sem: sem, - cfg: cfg, - bucketname: cfg.Bucket, - prefix: cfg.Prefix, + client: client, + sem: sem, + cfg: cfg, } client.SetCustomTransport(backend.Transport()) @@ -123,7 +119,7 @@ func (be *Backend) ReadDir(dir string) (list []os.FileInfo, err error) { done := make(chan struct{}) defer close(done) - for obj := range be.client.ListObjects(be.bucketname, dir, false, done) { + for obj := range be.client.ListObjects(be.cfg.Bucket, dir, false, done) { if obj.Key == "" { continue } @@ -154,12 +150,12 @@ func (be *Backend) ReadDir(dir string) (list []os.FileInfo, err error) { // Location returns this backend's location (the bucket name). func (be *Backend) Location() string { - return be.Join(be.bucketname, be.prefix) + return be.Join(be.cfg.Bucket, be.cfg.Prefix) } // Path returns the path in the bucket that is used for this backend. func (be *Backend) Path() string { - return be.prefix + return be.cfg.Prefix } func (be *Backend) isGoogleCloudStorage() bool { @@ -186,7 +182,7 @@ func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err objName := be.Filename(h) // Check key does not already exist - _, err = be.client.StatObject(be.bucketname, objName) + _, err = be.client.StatObject(be.cfg.Bucket, objName) if err == nil { debug.Log("%v already exists", h) return errors.New("key already exists") @@ -198,8 +194,8 @@ func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err } be.sem.GetToken() - debug.Log("PutObject(%v, %v)", be.bucketname, objName) - n, err := be.client.PutObject(be.bucketname, objName, rd, "application/octet-stream") + debug.Log("PutObject(%v, %v)", be.cfg.Bucket, objName) + n, err := be.client.PutObject(be.cfg.Bucket, objName, rd, "application/octet-stream") be.sem.ReleaseToken() debug.Log("%v -> %v bytes, err %#v: %v", objName, n, err, err) @@ -249,7 +245,7 @@ func (be *Backend) Load(ctx context.Context, h restic.Handle, length int, offset debug.Log("Load(%v) send range %v", h, byteRange) coreClient := minio.Core{Client: be.client} - rd, _, err := coreClient.GetObject(be.bucketname, objName, headers) + rd, _, err := coreClient.GetObject(be.cfg.Bucket, objName, headers) if err != nil { be.sem.ReleaseToken() return nil, err @@ -273,7 +269,7 @@ func (be *Backend) Stat(ctx context.Context, h restic.Handle) (bi restic.FileInf objName := be.Filename(h) var obj *minio.Object - obj, err = be.client.GetObject(be.bucketname, objName) + obj, err = be.client.GetObject(be.cfg.Bucket, objName) if err != nil { debug.Log("GetObject() err %v", err) return restic.FileInfo{}, errors.Wrap(err, "client.GetObject") @@ -300,7 +296,7 @@ func (be *Backend) Stat(ctx context.Context, h restic.Handle) (bi restic.FileInf func (be *Backend) Test(ctx context.Context, h restic.Handle) (bool, error) { found := false objName := be.Filename(h) - _, err := be.client.StatObject(be.bucketname, objName) + _, err := be.client.StatObject(be.cfg.Bucket, objName) if err == nil { found = true } @@ -312,7 +308,7 @@ func (be *Backend) Test(ctx context.Context, h restic.Handle) (bool, error) { // Remove removes the blob with the given name and type. func (be *Backend) Remove(ctx context.Context, h restic.Handle) error { objName := be.Filename(h) - err := be.client.RemoveObject(be.bucketname, objName) + err := be.client.RemoveObject(be.cfg.Bucket, objName) debug.Log("Remove(%v) at %v -> err %v", h, objName, err) if be.IsNotExist(err) { @@ -336,7 +332,7 @@ func (be *Backend) List(ctx context.Context, t restic.FileType) <-chan string { prefix += "/" } - listresp := be.client.ListObjects(be.bucketname, prefix, true, ctx.Done()) + listresp := be.client.ListObjects(be.cfg.Bucket, prefix, true, ctx.Done()) go func() { defer close(ch) @@ -400,11 +396,11 @@ func (be *Backend) Rename(h restic.Handle, l backend.Layout) error { debug.Log(" %v -> %v", oldname, newname) coreClient := minio.Core{Client: be.client} - err := coreClient.CopyObject(be.bucketname, newname, path.Join(be.bucketname, oldname), minio.CopyConditions{}) + err := coreClient.CopyObject(be.cfg.Bucket, newname, path.Join(be.cfg.Bucket, oldname), minio.CopyConditions{}) if err != nil { debug.Log("copy failed: %v", err) return err } - return be.client.RemoveObject(be.bucketname, oldname) + return be.client.RemoveObject(be.cfg.Bucket, oldname) } From eb7fc12e01183616e07b88b0aee676cce1574513 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Jun 2017 19:41:07 +0200 Subject: [PATCH 5/7] backend tests: Delay listing for swift backend --- src/restic/backend/test/tests.go | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/restic/backend/test/tests.go b/src/restic/backend/test/tests.go index b6da7182d..a4d179dd9 100644 --- a/src/restic/backend/test/tests.go +++ b/src/restic/backend/test/tests.go @@ -453,6 +453,21 @@ func delayedRemove(b restic.Backend, h restic.Handle) error { return err } +func delayedList(t testing.TB, b restic.Backend, tpe restic.FileType, max int) restic.IDs { + list := restic.NewIDSet() + for i := 0; i < max; i++ { + for s := range b.List(context.TODO(), tpe) { + id := restic.TestParseID(s) + list.Insert(id) + } + if len(list) < max { + time.Sleep(100 * time.Millisecond) + } + } + + return list.List() +} + // TestBackend tests all functions of the backend. func (s *Suite) TestBackend(t *testing.T) { b := s.open(t) @@ -548,12 +563,7 @@ func (s *Suite) TestBackend(t *testing.T) { IDs = append(IDs, id) } - list := restic.IDs{} - - for s := range b.List(context.TODO(), tpe) { - list = append(list, restic.TestParseID(s)) - } - + list := delayedList(t, b, tpe, len(IDs)) if len(IDs) != len(list) { t.Fatalf("wrong number of IDs returned: want %d, got %d", len(IDs), len(list)) } From bbca31b661cb9f4ceb077e144b4d9cd907e81b8e Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Jun 2017 19:51:55 +0200 Subject: [PATCH 6/7] test/s3: Retry connection to Minio server --- src/restic/backend/s3/s3_test.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/restic/backend/s3/s3_test.go b/src/restic/backend/s3/s3_test.go index 12ca73a10..7d080063a 100644 --- a/src/restic/backend/s3/s3_test.go +++ b/src/restic/backend/s3/s3_test.go @@ -103,6 +103,21 @@ type MinioTestConfig struct { stopServer func() } +func openS3(t testing.TB, cfg MinioTestConfig) (be restic.Backend, err error) { + for i := 0; i < 10; i++ { + be, err = s3.Open(cfg.Config) + if err != nil { + t.Logf("s3 open: try %d: error %v", i, err) + time.Sleep(500 * time.Millisecond) + continue + } + + break + } + + return be, err +} + func newMinioTestSuite(ctx context.Context, t testing.TB) *test.Suite { return &test.Suite{ // NewConfig returns a config for a new temporary backend that will be used in tests. @@ -127,7 +142,7 @@ func newMinioTestSuite(ctx context.Context, t testing.TB) *test.Suite { Create: func(config interface{}) (restic.Backend, error) { cfg := config.(MinioTestConfig) - be, err := s3.Open(cfg.Config) + be, err := openS3(t, cfg) if err != nil { return nil, err } From 05365706c094be501b0765aa2f904c39bdcf4520 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Jun 2017 20:05:35 +0200 Subject: [PATCH 7/7] backend/tests: Correct error message and delayed remove --- src/restic/backend/test/tests.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/restic/backend/test/tests.go b/src/restic/backend/test/tests.go index a4d179dd9..4447064cf 100644 --- a/src/restic/backend/test/tests.go +++ b/src/restic/backend/test/tests.go @@ -446,9 +446,15 @@ func delayedRemove(b restic.Backend, h restic.Handle) error { found, err := b.Test(context.TODO(), h) for i := 0; found && i < 20; i++ { found, err = b.Test(context.TODO(), h) - if found { - time.Sleep(100 * time.Millisecond) + if err != nil { + return err } + + if !found { + break + } + + time.Sleep(100 * time.Millisecond) } return err } @@ -591,7 +597,7 @@ func (s *Suite) TestBackend(t *testing.T) { found, err = b.Test(context.TODO(), h) test.OK(t, err) - test.Assert(t, !found, fmt.Sprintf("id %q not found after removal", id)) + test.Assert(t, !found, fmt.Sprintf("id %q found after removal", id)) } } }