From b8d30b2aaed7a5f705e21be06b923319b87c8403 Mon Sep 17 00:00:00 2001 From: glorv Date: Tue, 14 Dec 2021 20:04:35 +0800 Subject: [PATCH] cherry pick #30674 to release-5.3 Signed-off-by: ti-srebot --- br/pkg/backup/client_test.go | 2 -- br/pkg/lightning/lightning.go | 13 +++++++++++++ br/pkg/storage/gcs.go | 8 -------- br/pkg/storage/s3.go | 8 -------- br/pkg/storage/s3_test.go | 3 +-- br/pkg/storage/storage.go | 13 ------------- br/pkg/task/backup.go | 1 - br/pkg/task/backup_raw.go | 1 - br/pkg/task/common.go | 4 +++- br/pkg/task/restore.go | 1 - br/tests/lightning_s3/run.sh | 14 ++++++++++++++ 11 files changed, 31 insertions(+), 37 deletions(-) diff --git a/br/pkg/backup/client_test.go b/br/pkg/backup/client_test.go index 3c3688f79bc9f..de4b6dfd2b588 100644 --- a/br/pkg/backup/client_test.go +++ b/br/pkg/backup/client_test.go @@ -258,7 +258,6 @@ func (r *testBackup) TestSendCreds(c *C) { c.Assert(err, IsNil) opts := &storage.ExternalStorageOptions{ SendCredentials: true, - SkipCheckPath: true, } _, err = storage.New(r.ctx, backend, opts) c.Assert(err, IsNil) @@ -277,7 +276,6 @@ func (r *testBackup) TestSendCreds(c *C) { c.Assert(err, IsNil) opts = &storage.ExternalStorageOptions{ SendCredentials: false, - SkipCheckPath: true, } _, err = storage.New(r.ctx, backend, opts) c.Assert(err, IsNil) diff --git a/br/pkg/lightning/lightning.go b/br/pkg/lightning/lightning.go index 0a9cf1585456a..723b73518fa03 100644 --- a/br/pkg/lightning/lightning.go +++ b/br/pkg/lightning/lightning.go @@ -297,6 +297,19 @@ func (l *Lightning) run(taskCtx context.Context, taskCfg *config.Config, g glue. return errors.Annotate(err, "create storage failed") } + // return expectedErr means at least meet one file + expectedErr := errors.New("Stop Iter") + walkErr := s.WalkDir(ctx, &storage.WalkOption{ListCount: 1}, func(string, int64) error { + // return an error when meet the first regular file to break the walk loop + return expectedErr + }) + if !errors.ErrorEqual(walkErr, expectedErr) { + if walkErr == nil { + return errors.Errorf("data-source-dir '%s' doesn't exist or contains no files", taskCfg.Mydumper.SourceDir) + } + return errors.Annotatef(walkErr, "visit data-source-dir '%s' failed", taskCfg.Mydumper.SourceDir) + } + loadTask := log.L().Begin(zap.InfoLevel, "load data source") var mdl *mydump.MDLoader mdl, err = mydump.NewMyDumpLoaderWithStore(ctx, taskCfg, s) diff --git a/br/pkg/storage/gcs.go b/br/pkg/storage/gcs.go index 07ce5c8a862b9..ae9a57f846bfd 100644 --- a/br/pkg/storage/gcs.go +++ b/br/pkg/storage/gcs.go @@ -281,14 +281,6 @@ func newGCSStorage(ctx context.Context, gcs *backuppb.GCS, opts *ExternalStorage // so we need find sst in slash directory gcs.Prefix += "//" } - // TODO remove it after BR remove cfg skip-check-path - if !opts.SkipCheckPath { - // check bucket exists - _, err = bucket.Attrs(ctx) - if err != nil { - return nil, errors.Annotatef(err, "gcs://%s/%s", gcs.Bucket, gcs.Prefix) - } - } return &gcsStorage{gcs: gcs, bucket: bucket}, nil } diff --git a/br/pkg/storage/s3.go b/br/pkg/storage/s3.go index 2c07b5af2cad0..6accafee7363d 100644 --- a/br/pkg/storage/s3.go +++ b/br/pkg/storage/s3.go @@ -283,14 +283,6 @@ func newS3Storage(backend *backuppb.S3, opts *ExternalStorageOptions) (*S3Storag } c := s3.New(ses) - // TODO remove it after BR remove cfg skip-check-path - if !opts.SkipCheckPath { - err = checkS3Bucket(c, &qs) - if err != nil { - return nil, errors.Annotatef(berrors.ErrStorageInvalidConfig, "Bucket %s is not accessible: %v", qs.Bucket, err) - } - } - if len(qs.Prefix) > 0 && !strings.HasSuffix(qs.Prefix, "/") { qs.Prefix += "/" } diff --git a/br/pkg/storage/s3_test.go b/br/pkg/storage/s3_test.go index 413f5e8881da1..cf30828b07c65 100644 --- a/br/pkg/storage/s3_test.go +++ b/br/pkg/storage/s3_test.go @@ -288,7 +288,6 @@ func (s *s3Suite) TestS3Storage(c *C) { _, err := New(ctx, s3, &ExternalStorageOptions{ SendCredentials: test.sendCredential, CheckPermissions: test.hackPermission, - SkipCheckPath: true, }) if test.errReturn { c.Assert(err, NotNil) @@ -414,7 +413,7 @@ func (s *s3Suite) TestS3Storage(c *C) { func (s *s3Suite) TestS3URI(c *C) { backend, err := ParseBackend("s3://bucket/prefix/", nil) c.Assert(err, IsNil) - storage, err := New(context.Background(), backend, &ExternalStorageOptions{SkipCheckPath: true}) + storage, err := New(context.Background(), backend, &ExternalStorageOptions{}) c.Assert(err, IsNil) c.Assert(storage.URI(), Equals, "s3://bucket/prefix/") } diff --git a/br/pkg/storage/storage.go b/br/pkg/storage/storage.go index af05abac398fa..177656fc378a0 100644 --- a/br/pkg/storage/storage.go +++ b/br/pkg/storage/storage.go @@ -121,18 +121,6 @@ type ExternalStorageOptions struct { // NoCredentials means that no cloud credentials are supplied to BR NoCredentials bool - // SkipCheckPath marks whether to skip checking path's existence. - // - // This should only be set to true in testing, to avoid interacting with the - // real world. - // When this field is false (i.e. path checking is enabled), the New() - // function will ensure the path referred by the backend exists by - // recursively creating the folders. This will also throw an error if such - // operation is impossible (e.g. when the bucket storing the path is missing). - - // deprecated: use checkPermissions and specify the checkPermission instead. - SkipCheckPath bool - // HTTPClient to use. The created storage may ignore this field if it is not // directly using HTTP (e.g. the local storage). HTTPClient *http.Client @@ -148,7 +136,6 @@ type ExternalStorageOptions struct { func Create(ctx context.Context, backend *backuppb.StorageBackend, sendCreds bool) (ExternalStorage, error) { return New(ctx, backend, &ExternalStorageOptions{ SendCredentials: sendCreds, - SkipCheckPath: false, HTTPClient: nil, }) } diff --git a/br/pkg/task/backup.go b/br/pkg/task/backup.go index 7a9037c20f80c..87461f53bab74 100644 --- a/br/pkg/task/backup.go +++ b/br/pkg/task/backup.go @@ -257,7 +257,6 @@ func RunBackup(c context.Context, g glue.Glue, cmdName string, cfg *BackupConfig opts := storage.ExternalStorageOptions{ NoCredentials: cfg.NoCreds, SendCredentials: cfg.SendCreds, - SkipCheckPath: cfg.SkipCheckPath, } if err = client.SetStorage(ctx, u, &opts); err != nil { return errors.Trace(err) diff --git a/br/pkg/task/backup_raw.go b/br/pkg/task/backup_raw.go index d8d11ea95c3a1..febe151218706 100644 --- a/br/pkg/task/backup_raw.go +++ b/br/pkg/task/backup_raw.go @@ -150,7 +150,6 @@ func RunBackupRaw(c context.Context, g glue.Glue, cmdName string, cfg *RawKvConf opts := storage.ExternalStorageOptions{ NoCredentials: cfg.NoCreds, SendCredentials: cfg.SendCreds, - SkipCheckPath: cfg.SkipCheckPath, } if err = client.SetStorage(ctx, u, &opts); err != nil { return errors.Trace(err) diff --git a/br/pkg/task/common.go b/br/pkg/task/common.go index 36de8583ea92e..fc1c8ecc5748d 100644 --- a/br/pkg/task/common.go +++ b/br/pkg/task/common.go @@ -485,6 +485,9 @@ func (cfg *Config) ParseFromFlags(flags *pflag.FlagSet) error { if cfg.SkipCheckPath, err = flags.GetBool(flagSkipCheckPath); err != nil { return errors.Trace(err) } + if cfg.SkipCheckPath { + log.L().Info("--skip-check-path is deprecated, need explicitly set it anymore") + } if err = cfg.parseCipherInfo(flags); err != nil { return errors.Trace(err) @@ -548,7 +551,6 @@ func storageOpts(cfg *Config) *storage.ExternalStorageOptions { return &storage.ExternalStorageOptions{ NoCredentials: cfg.NoCreds, SendCredentials: cfg.SendCreds, - SkipCheckPath: cfg.SkipCheckPath, } } diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index a80549d005905..ae46f15b1f6ce 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -249,7 +249,6 @@ func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf opts := storage.ExternalStorageOptions{ NoCredentials: cfg.NoCreds, SendCredentials: cfg.SendCreds, - SkipCheckPath: cfg.SkipCheckPath, } if err = client.SetStorage(ctx, u, &opts); err != nil { return errors.Trace(err) diff --git a/br/tests/lightning_s3/run.sh b/br/tests/lightning_s3/run.sh index 6fed0af2b81da..5b2973784fd7e 100755 --- a/br/tests/lightning_s3/run.sh +++ b/br/tests/lightning_s3/run.sh @@ -62,6 +62,20 @@ _EOF_ run_sql "DROP DATABASE IF EXISTS $DB;" run_sql "DROP TABLE IF EXISTS $DB.$TABLE;" +# test not exist path +rm -f $TEST_DIR/lightning.log +SOURCE_DIR="s3://$BUCKET/not-exist-path?endpoint=http%3A//127.0.0.1%3A9900&access_key=$MINIO_ACCESS_KEY&secret_access_key=$MINIO_SECRET_KEY&force_path_style=true" +! run_lightning -d $SOURCE_DIR --backend local 2> /dev/null +grep -Eq "data-source-dir .* doesn't exist or contains no files" $TEST_DIR/lightning.log + +# test empty dir +rm -f $TEST_DIR/lightning.log +emptyPath=empty-bucket/empty-path +mkdir -p $DBPATH/$emptyPath +SOURCE_DIR="s3://$emptyPath/not-exist-path?endpoint=http%3A//127.0.0.1%3A9900&access_key=$MINIO_ACCESS_KEY&secret_access_key=$MINIO_SECRET_KEY&force_path_style=true" +! run_lightning -d $SOURCE_DIR --backend local 2> /dev/null +grep -Eq "data-source-dir .* doesn't exist or contains no files" $TEST_DIR/lightning.log + SOURCE_DIR="s3://$BUCKET/?endpoint=http%3A//127.0.0.1%3A9900&access_key=$MINIO_ACCESS_KEY&secret_access_key=$MINIO_SECRET_KEY&force_path_style=true" run_lightning -d $SOURCE_DIR --backend local 2> /dev/null run_sql "SELECT count(*), sum(i) FROM \`$DB\`.$TABLE"