From 466f6e77b6b01f09443043f75156de4ca2892141 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Thu, 23 Sep 2021 16:34:46 +0800 Subject: [PATCH] br.storage: add delete api (#27237) (#27766) --- br/pkg/mock/storage/storage.go | 14 +++++++++++ br/pkg/storage/gcs.go | 7 ++++++ br/pkg/storage/gcs_test.go | 19 ++++++++++++++ br/pkg/storage/local.go | 5 ++++ br/pkg/storage/local_test.go | 28 ++++++++++++++++++++- br/pkg/storage/noop.go | 5 ++++ br/pkg/storage/s3.go | 11 ++++++++ br/pkg/storage/s3_test.go | 46 ++++++++++++++++++++++++++++++++++ br/pkg/storage/storage.go | 2 ++ 9 files changed, 136 insertions(+), 1 deletion(-) diff --git a/br/pkg/mock/storage/storage.go b/br/pkg/mock/storage/storage.go index 4827ddb865c6c..71b2b72f67b13 100644 --- a/br/pkg/mock/storage/storage.go +++ b/br/pkg/mock/storage/storage.go @@ -138,3 +138,17 @@ func (mr *MockExternalStorageMockRecorder) WriteFile(arg0, arg1, arg2 interface{ mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WriteFile", reflect.TypeOf((*MockExternalStorage)(nil).WriteFile), arg0, arg1, arg2) } + +// DeleteFile mocks base method. +func (m *MockExternalStorage) DeleteFile(ctx context.Context, name string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteFile", ctx, name) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteFile indicates an expected call of DeleteFile. +func (mr *MockExternalStorageMockRecorder) DeleteFile(ctx, name interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteFile", reflect.TypeOf((*MockExternalStorage)(nil).DeleteFile), ctx, name) +} diff --git a/br/pkg/storage/gcs.go b/br/pkg/storage/gcs.go index 193411f8e020e..07ce5c8a862b9 100644 --- a/br/pkg/storage/gcs.go +++ b/br/pkg/storage/gcs.go @@ -88,6 +88,13 @@ type gcsStorage struct { bucket *storage.BucketHandle } +// DeleteFile delete the file in storage +func (s *gcsStorage) DeleteFile(ctx context.Context, name string) error { + object := s.objectName(name) + err := s.bucket.Object(object).Delete(ctx) + return errors.Trace(err) +} + func (s *gcsStorage) objectName(name string) string { return path.Join(s.gcs.Prefix, name) } diff --git a/br/pkg/storage/gcs_test.go b/br/pkg/storage/gcs_test.go index 66167953492f4..c3e63d6d410a2 100644 --- a/br/pkg/storage/gcs_test.go +++ b/br/pkg/storage/gcs_test.go @@ -65,6 +65,25 @@ func (r *testStorageSuite) TestGCS(c *C) { c.Assert(err, IsNil) c.Assert(exist, IsFalse) + keyDelete := "key_delete" + exist, err = stg.FileExists(ctx, keyDelete) + c.Assert(err, IsNil) + c.Assert(exist, IsFalse) + + err = stg.WriteFile(ctx, keyDelete, []byte("data")) + c.Assert(err, IsNil) + + exist, err = stg.FileExists(ctx, keyDelete) + c.Assert(err, IsNil) + c.Assert(exist, IsTrue) + + err = stg.DeleteFile(ctx, keyDelete) + c.Assert(err, IsNil) + + exist, err = stg.FileExists(ctx, keyDelete) + c.Assert(err, IsNil) + c.Assert(exist, IsFalse) + list := "" var totalSize int64 = 0 err = stg.WalkDir(ctx, nil, func(name string, size int64) error { diff --git a/br/pkg/storage/local.go b/br/pkg/storage/local.go index 297d08a3c5c49..8894edd1d7acf 100644 --- a/br/pkg/storage/local.go +++ b/br/pkg/storage/local.go @@ -25,6 +25,11 @@ type LocalStorage struct { base string } +func (l *LocalStorage) DeleteFile(ctx context.Context, name string) error { + path := filepath.Join(l.base, name) + return os.Remove(path) +} + // WriteFile writes data to a file to storage. func (l *LocalStorage) WriteFile(ctx context.Context, name string, data []byte) error { path := filepath.Join(l.base, name) diff --git a/br/pkg/storage/local_test.go b/br/pkg/storage/local_test.go index c69fa74dfab04..54eed83cd47bd 100644 --- a/br/pkg/storage/local_test.go +++ b/br/pkg/storage/local_test.go @@ -15,6 +15,33 @@ type testLocalSuite struct{} var _ = Suite(&testLocalSuite{}) +func (r *testStorageSuite) TestDeleteFile(c *C) { + dir := c.MkDir() + sb, err := ParseBackend("file://"+filepath.ToSlash(dir), &BackendOptions{}) + c.Assert(err, IsNil) + store, err := Create(context.TODO(), sb, true) + c.Assert(err, IsNil) + + name := "test_delete" + ret, err := store.FileExists(context.Background(), name) + c.Assert(err, IsNil) + c.Assert(ret, Equals, false) + + _, err = store.Create(context.Background(), name) + c.Assert(err, IsNil) + + ret, err = store.FileExists(context.Background(), name) + c.Assert(err, IsNil) + c.Assert(ret, Equals, true) + + err = store.DeleteFile(context.Background(), name) + c.Assert(err, IsNil) + + ret, err = store.FileExists(context.Background(), name) + c.Assert(err, IsNil) + c.Assert(ret, Equals, false) +} + func (r *testStorageSuite) TestWalkDirWithSoftLinkFile(c *C) { if runtime.GOOS == "windows" { // skip the test on windows. typically windows users don't have symlink permission. @@ -26,7 +53,6 @@ func (r *testStorageSuite) TestWalkDirWithSoftLinkFile(c *C) { path1 := filepath.Join(dir1, name1) f1, err := os.Create(path1) c.Assert(err, IsNil) - data := "/* whatever pragmas */;" + "INSERT INTO `namespaced`.`table` (columns, more, columns) VALUES (1,-2, 3),\n(4,5., 6);" + "INSERT `namespaced`.`table` (x,y,z) VALUES (7,8,9);" + diff --git a/br/pkg/storage/noop.go b/br/pkg/storage/noop.go index 48d47e06ac23c..ead3c9ed706af 100644 --- a/br/pkg/storage/noop.go +++ b/br/pkg/storage/noop.go @@ -8,6 +8,11 @@ import ( type noopStorage struct{} +// DeleteFile delete the file in storage +func (s *noopStorage) DeleteFile(ctx context.Context, name string) error { + return nil +} + // WriteFile file to storage. func (*noopStorage) WriteFile(ctx context.Context, name string, data []byte) error { return nil diff --git a/br/pkg/storage/s3.go b/br/pkg/storage/s3.go index 06b4963e7d222..8fe1b6b3ae24c 100644 --- a/br/pkg/storage/s3.go +++ b/br/pkg/storage/s3.go @@ -403,6 +403,17 @@ func (rs *S3Storage) ReadFile(ctx context.Context, file string) ([]byte, error) return data, nil } +// DeleteFile delete the file in s3 storage +func (rs *S3Storage) DeleteFile(ctx context.Context, file string) error { + input := &s3.DeleteObjectInput{ + Bucket: aws.String(rs.options.Bucket), + Key: aws.String(rs.options.Prefix + file), + } + + _, err := rs.svc.DeleteObjectWithContext(ctx, input) + return errors.Trace(err) +} + // FileExists check if file exists on s3 storage. func (rs *S3Storage) FileExists(ctx context.Context, file string) (bool, error) { input := &s3.HeadObjectInput{ diff --git a/br/pkg/storage/s3_test.go b/br/pkg/storage/s3_test.go index 87894bc174ae1..f1a4e42221afa 100644 --- a/br/pkg/storage/s3_test.go +++ b/br/pkg/storage/s3_test.go @@ -508,6 +508,52 @@ func (s *s3Suite) TestFileExistsNoError(c *C) { c.Assert(exists, IsTrue) } +func (s *s3Suite) TestDeleteFileNoError(c *C) { + s.setUpTest(c) + defer s.tearDownTest() + ctx := aws.BackgroundContext() + + s.s3.EXPECT(). + DeleteObjectWithContext(ctx, gomock.Any()). + DoAndReturn(func(_ context.Context, input *s3.DeleteObjectInput, opt ...request.Option) (*s3.DeleteObjectInput, error) { + c.Assert(aws.StringValue(input.Bucket), Equals, "bucket") + c.Assert(aws.StringValue(input.Key), Equals, "prefix/file") + return &s3.DeleteObjectInput{}, nil + }) + + err := s.storage.DeleteFile(ctx, "file") + c.Assert(err, IsNil) +} + +func (s *s3Suite) TestDeleteFileMissing(c *C) { + s.setUpTest(c) + defer s.tearDownTest() + ctx := aws.BackgroundContext() + + awserr := awserr.New(s3.ErrCodeNoSuchKey, "no such key", nil) + s.s3.EXPECT(). + DeleteObjectWithContext(ctx, gomock.Any()). + Return(nil, awserr) + + err := s.storage.DeleteFile(ctx, "file-missing") + c.Assert(err, ErrorMatches, awserr.Error()) +} + +func (s *s3Suite) TestDeleteFileError(c *C) { + s.setUpTest(c) + defer s.tearDownTest() + ctx := aws.BackgroundContext() + + expectedErr := errors.New("just some unrelated error") + + s.s3.EXPECT(). + DeleteObjectWithContext(ctx, gomock.Any()). + Return(nil, expectedErr) + + err := s.storage.DeleteFile(ctx, "file3") + c.Assert(err, ErrorMatches, `\Q`+expectedErr.Error()+`\E`) +} + // TestFileExistsNoSuckKey ensures FileExists API reports file missing if S3's // HeadObject request replied NoSuchKey. func (s *s3Suite) TestFileExistsMissing(c *C) { diff --git a/br/pkg/storage/storage.go b/br/pkg/storage/storage.go index 14e9fc2ad73d8..427e625795533 100644 --- a/br/pkg/storage/storage.go +++ b/br/pkg/storage/storage.go @@ -77,6 +77,8 @@ type ExternalStorage interface { ReadFile(ctx context.Context, name string) ([]byte, error) // FileExists return true if file exists FileExists(ctx context.Context, name string) (bool, error) + // DeleteFile delete the file in storage + DeleteFile(ctx context.Context, name string) error // Open a Reader by file path. path is relative path to storage base path Open(ctx context.Context, path string) (ExternalFileReader, error) // WalkDir traverse all the files in a dir.