Skip to content

Commit

Permalink
cherry pick #30674 to release-5.3
Browse files Browse the repository at this point in the history
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
  • Loading branch information
glorv authored and ti-srebot committed Dec 14, 2021
1 parent 83b273a commit b8d30b2
Show file tree
Hide file tree
Showing 11 changed files with 31 additions and 37 deletions.
2 changes: 0 additions & 2 deletions br/pkg/backup/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
13 changes: 13 additions & 0 deletions br/pkg/lightning/lightning.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 0 additions & 8 deletions br/pkg/storage/gcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
8 changes: 0 additions & 8 deletions br/pkg/storage/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 += "/"
}
Expand Down
3 changes: 1 addition & 2 deletions br/pkg/storage/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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/")
}
Expand Down
13 changes: 0 additions & 13 deletions br/pkg/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
})
}
Expand Down
1 change: 0 additions & 1 deletion br/pkg/task/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion br/pkg/task/backup_raw.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion br/pkg/task/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -548,7 +551,6 @@ func storageOpts(cfg *Config) *storage.ExternalStorageOptions {
return &storage.ExternalStorageOptions{
NoCredentials: cfg.NoCreds,
SendCredentials: cfg.SendCreds,
SkipCheckPath: cfg.SkipCheckPath,
}
}

Expand Down
1 change: 0 additions & 1 deletion br/pkg/task/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions br/tests/lightning_s3/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit b8d30b2

Please sign in to comment.