Merge pull request #10329 from ioito/hotfix/qx-bucket-tag-sync

fix: optimized bucket tag

pkg/cloudprovider/objectstore.go

@@ -273,8 +273,6 @@ type ICloudBucket interface {
 	SetPolicy(policy SBucketPolicyStatementInput) error
 	DeletePolicy(id []string) ([]SBucketPolicyStatement, error)
 
-	DeleteTags() error
-
 	ListMultipartUploads() ([]SBucketMultipartUploads, error)
 }
 
@@ -839,32 +837,18 @@ func DeleteBucketCORS(ibucket ICloudBucket, id []string) ([]SBucketCORSRule, err
 	return deletedRules, nil
 }
 
-func SetBucketMetadata(ibucket ICloudBucket, tags map[string]string, replace bool) error {
-	newTags := map[string]string{}
-	if replace {
-		newTags = tags
-	} else {
-		oldTags, err := ibucket.GetTags()
-		if err != nil {
-			return errors.Wrap(err, "b.getTags()")
+func SetBucketTags(iBucket ICloudBucket, tags map[string]string) (TagsUpdateInfo, error) {
+	ret := TagsUpdateInfo{}
+	old, err := iBucket.GetTags()
+	if err != nil {
+		if errors.Cause(err) == ErrNotImplemented || errors.Cause(err) == ErrNotSupported {
+			return ret, nil
 		}
-		for k, v := range oldTags {
-			if _, ok := tags[k]; !ok {
-				tags[k] = v
-			}
-		}
-		newTags = tags
+		return ret, errors.Wrapf(err, "iBucket.GetTags")
 	}
-	if len(newTags) == 0 {
-		err := ibucket.DeleteTags()
-		if err != nil {
-			return errors.Wrap(err, "b.DeleteTags()")
-		}
-	} else {
-		err := ibucket.SetTags(newTags, true)
-		if err != nil {
-			return errors.Wrapf(err, "b.setTags(%s)", jsonutils.Marshal(newTags).String())
-		}
+	ret.OldTags, ret.NewTags = old, tags
+	if !ret.IsChanged() {
+		return ret, nil
 	}
-	return nil
+	return ret, iBucket.SetTags(tags, true)
 }

pkg/cloudprovider/resourcetags.go

@@ -14,7 +14,13 @@
 
 package cloudprovider
 
+import "reflect"
+
 type TagsUpdateInfo struct {
 	OldTags map[string]string
 	NewTags map[string]string
 }
+
+func (t TagsUpdateInfo) IsChanged() bool {
+	return !reflect.DeepEqual(t.OldTags, t.NewTags)
+}

pkg/compute/models/buckets.go

@@ -480,6 +480,7 @@ func (bucket *SBucket) PostCreate(
 	query jsonutils.JSONObject,
 	data jsonutils.JSONObject,
 ) {
+	bucket.SSharableVirtualResourceBase.PostCreate(ctx, userCred, ownerId, query, data)
 	pendingUsage := SRegionQuota{Bucket: 1}
 	keys, err := bucket.GetQuotaKeys()
 	if err != nil {
@@ -495,10 +496,10 @@ func (bucket *SBucket) PostCreate(
 	bucket.SetStatus(userCred, api.BUCKET_STATUS_START_CREATE, "PostCreate")
 	task, err := taskman.TaskManager.NewTask(ctx, "BucketCreateTask", bucket, userCred, nil, "", "", nil)
 	if err != nil {
-		log.Errorf("BucketCreateTask newTask error %s", err)
-	} else {
-		task.ScheduleRun(nil)
+		bucket.SetStatus(userCred, api.BUCKET_STATUS_CREATE_FAIL, errors.Wrapf(err, "NewTask").Error())
+		return
 	}
+	task.ScheduleRun(nil)
 }
 
 func (bucket *SBucket) ValidateUpdateData(
@@ -538,15 +539,17 @@ func (bucket *SBucket) RemoteCreate(ctx context.Context, userCred mcclient.Token
 	if err != nil {
 		return errors.Wrap(err, "db.SetExternalId")
 	}
+	tags, _ := bucket.GetAllUserMetadata()
+	if len(tags) > 0 {
+		_, err = cloudprovider.SetBucketTags(extBucket, tags)
+		if err != nil {
+			logclient.AddSimpleActionLog(bucket, logclient.ACT_UPDATE_TAGS, err, userCred, false)
+		}
+	}
 	err = bucket.syncWithCloudBucket(ctx, userCred, extBucket, nil, false)
 	if err != nil {
 		return errors.Wrap(err, "bucket.syncWithCloudBucket")
 	}
-	tags, _ := bucket.GetAllUserMetadata()
-	err = cloudprovider.SetBucketMetadata(extBucket, tags, false)
-	if err != nil {
-		log.Errorf("iBucket.SetMetadata failed: %s", err)
-	}
 	return nil
 }
 
@@ -1891,26 +1894,20 @@ func (bucket *SBucket) OnMetadataUpdated(ctx context.Context, userCred mcclient.
 	}
 	iBucket, err := bucket.GetIBucket()
 	if err != nil {
-		log.Errorf("bucket.GetIBucket() failed: %s", err)
 		return
 	}
-	oldTags, err := iBucket.GetTags()
+	tags, err := bucket.GetAllUserMetadata()
+	if err != nil {
+		return
+	}
+	diff, err := cloudprovider.SetBucketTags(iBucket, tags)
 	if err != nil {
 		logclient.AddSimpleActionLog(bucket, logclient.ACT_UPDATE_TAGS, err, userCred, false)
-		log.Errorf("iBucket.GetTags failed: %s", err)
 		return
 	}
-	tags, _ := bucket.GetAllUserMetadata()
-	tagsUpdateInfo := cloudprovider.TagsUpdateInfo{OldTags: oldTags, NewTags: tags}
-
-	err = cloudprovider.SetBucketMetadata(iBucket, tags, true)
-	if err != nil {
-		logclient.AddSimpleActionLog(bucket, logclient.ACT_UPDATE_TAGS, err, userCred, false)
-		log.Errorf("iBucket.SetMetadata failed: %s", err)
-		return
+	if diff.IsChanged() {
+		logclient.AddSimpleActionLog(bucket, logclient.ACT_UPDATE_TAGS, diff, userCred, true)
 	}
-	syncMetadata(ctx, userCred, bucket, iBucket)
-	logclient.AddSimpleActionLog(bucket, logclient.ACT_UPDATE_TAGS, tagsUpdateInfo, userCred, true)
 }
 
 func (manager *SBucketManager) ListItemExportKeys(ctx context.Context,

pkg/multicloud/aliyun/bucket.go

@@ -724,27 +724,30 @@ func (b *SBucket) GetTags() (map[string]string, error) {
 
 	tagresult, err := osscli.GetBucketTagging(b.Name)
 	if err != nil {
-		if strings.Contains(err.Error(), "404") {
-			return nil, nil
-		}
-		return nil, errors.Wrapf(err, "osscli.GetBucketTagging(%s)", b.Name)
+		return nil, errors.Wrapf(err, "GetBucketTagging %s", b.Name)
 	}
 	result := map[string]string{}
-	for i := range tagresult.Tags {
-		result[tagresult.Tags[i].Key] = tagresult.Tags[i].Value
+	for _, tag := range tagresult.Tags {
+		result[tag.Key] = tag.Value
 	}
 	return result, nil
 }
 
 func (b *SBucket) SetTags(tags map[string]string, replace bool) error {
-	if !replace {
-		return cloudprovider.ErrNotSupported
-	}
 	osscli, err := b.region.GetOssClient()
 	if err != nil {
 		return errors.Wrap(err, "GetOssClient")
 	}
 
+	err = osscli.DeleteBucketTagging(b.Name)
+	if err != nil {
+		return errors.Wrapf(err, "DeleteBucketTagging(%s)", b.Name)
+	}
+
+	if len(tags) == 0 {
+		return nil
+	}
+
 	input := []oss.Tag{}
 	for k, v := range tags {
 		input = append(input, oss.Tag{Key: k, Value: v})
@@ -757,31 +760,6 @@ func (b *SBucket) SetTags(tags map[string]string, replace bool) error {
 	return nil
 }
 
-func (b *SBucket) DeleteTags() error {
-	osscli, err := b.region.GetOssClient()
-	if err != nil {
-		return errors.Wrap(err, "GetOssClient")
-	}
-	err = osscli.DeleteBucketTagging(b.Name)
-	if err != nil {
-		return errors.Wrapf(err, "osscli.DeleteBucketTagging(%s)", b.Name)
-	}
-	return nil
-}
-
-func (b *SBucket) GetMetadata() *jsonutils.JSONDict {
-	meta := jsonutils.NewDict()
-	tags, err := b.GetTags()
-	if err != nil {
-		log.Errorf("error:%s b.getTags()", err)
-		return meta
-	}
-	for k, v := range tags {
-		meta.Add(jsonutils.NewString(v), k)
-	}
-	return meta
-}
-
 func (b *SBucket) ListMultipartUploads() ([]cloudprovider.SBucketMultipartUploads, error) {
 	osscli, err := b.region.GetOssClient()
 	if err != nil {

pkg/multicloud/aws/bucket.go

@@ -713,6 +713,15 @@ func (b *SBucket) SetTags(tags map[string]string, replace bool) error {
 		return errors.Wrap(err, "GetS3Client")
 	}
 
+	_, err = s3cli.DeleteBucketTagging(&s3.DeleteBucketTaggingInput{Bucket: &b.Name})
+	if err != nil {
+		return errors.Wrapf(err, "DeleteBucketTagging")
+	}
+
+	if len(tags) == 0 {
+		return nil
+	}
+
 	input := s3.PutBucketTaggingInput{Tagging: &s3.Tagging{}}
 	input.Bucket = &b.Name
 	apiTagKeys := []string{}
@@ -733,31 +742,6 @@ func (b *SBucket) SetTags(tags map[string]string, replace bool) error {
 	return nil
 }
 
-func (b *SBucket) DeleteTags() error {
-	s3cli, err := b.region.GetS3Client()
-	if err != nil {
-		return errors.Wrap(err, "GetS3Client")
-	}
-	_, err = s3cli.DeleteBucketTagging(&s3.DeleteBucketTaggingInput{Bucket: &b.Name})
-	if err != nil {
-		return errors.Wrapf(err, "osscli.DeleteBucketTagging(%s)", b.Name)
-	}
-	return nil
-}
-
-func (b *SBucket) GetMetadata() *jsonutils.JSONDict {
-	meta := jsonutils.NewDict()
-	tags, err := b.GetTags()
-	if err != nil {
-		log.Errorf("error:%s b.getTags()", err)
-		return meta
-	}
-	for k, v := range tags {
-		meta.Add(jsonutils.NewString(v), k)
-	}
-	return meta
-}
-
 func (b *SBucket) ListMultipartUploads() ([]cloudprovider.SBucketMultipartUploads, error) {
 	s3cli, err := b.region.GetS3Client()
 	if err != nil {

pkg/multicloud/bucket_base.go

@@ -118,10 +118,6 @@ func (b *SBaseBucket) DeletePolicy(id []string) ([]cloudprovider.SBucketPolicySt
 	return nil, cloudprovider.ErrNotImplemented
 }
 
-func (b *SBaseBucket) DeleteTags() error {
-	return cloudprovider.ErrNotImplemented
-}
-
 func (b *SBaseBucket) ListMultipartUploads() ([]cloudprovider.SBucketMultipartUploads, error) {
 	return nil, cloudprovider.ErrNotImplemented
 }

pkg/multicloud/huawei/bucket.go

@@ -695,14 +695,20 @@ func (b *SBucket) GetTags() (map[string]string, error) {
 }
 
 func (b *SBucket) SetTags(tags map[string]string, replace bool) error {
-	if !replace {
-		return cloudprovider.ErrNotSupported
-	}
 	obscli, err := b.region.getOBSClient()
 	if err != nil {
 		return errors.Wrap(err, "GetOBSClient")
 	}
 
+	_, err = obscli.DeleteBucketTagging(b.Name)
+	if err != nil {
+		return errors.Wrapf(err, "DeleteBucketTagging")
+	}
+
+	if len(tags) == 0 {
+		return nil
+	}
+
 	input := obs.SetBucketTaggingInput{BucketTagging: obs.BucketTagging{}}
 	input.Bucket = b.Name
 	for k, v := range tags {
@@ -716,31 +722,6 @@ func (b *SBucket) SetTags(tags map[string]string, replace bool) error {
 	return nil
 }
 
-func (b *SBucket) DeleteTags() error {
-	obscli, err := b.region.getOBSClient()
-	if err != nil {
-		return errors.Wrap(err, "GetOBSClient")
-	}
-	_, err = obscli.DeleteBucketTagging(b.Name)
-	if err != nil {
-		return errors.Wrapf(err, "osscli.DeleteBucketTagging(%s)", b.Name)
-	}
-	return nil
-}
-
-func (b *SBucket) GetMetadata() *jsonutils.JSONDict {
-	meta := jsonutils.NewDict()
-	tags, err := b.GetTags()
-	if err != nil {
-		log.Errorf("error:%s b.getTags()", err)
-		return meta
-	}
-	for k, v := range tags {
-		meta.Add(jsonutils.NewString(v), k)
-	}
-	return meta
-}
-
 func (b *SBucket) ListMultipartUploads() ([]cloudprovider.SBucketMultipartUploads, error) {
 	obscli, err := b.region.getOBSClient()
 	if err != nil {

pkg/multicloud/objectstore/shell.go

@@ -666,12 +666,15 @@ func S3Shell() {
 	type BucketGetMetadata struct {
 		BUCKET string `help:"name of bucket to put object"`
 	}
-	shellutils.R(&BucketGetMetadata{}, "bucket-get-metadata", "get bucket metadata", func(cli cloudprovider.ICloudRegion, args *BucketGetMetadata) error {
+	shellutils.R(&BucketGetMetadata{}, "bucket-tag-list", "List bucket tag", func(cli cloudprovider.ICloudRegion, args *BucketGetMetadata) error {
 		bucket, err := cli.GetIBucketById(args.BUCKET)
 		if err != nil {
 			return err
 		}
-		meta, _ := bucket.GetTags()
+		meta, err := bucket.GetTags()
+		if err != nil {
+			return err
+		}
 		printObject(meta)
 		return nil
 	})
@@ -681,7 +684,7 @@ func S3Shell() {
 		Tags    []string `help:"Tags info, eg: hypervisor=aliyun、os_type=Linux、os_version"`
 		Replace bool
 	}
-	shellutils.R(&BucketSetMetadate{}, "bucket-set-metadata", "set bucket metadata", func(cli cloudprovider.ICloudRegion, args *BucketSetMetadate) error {
+	shellutils.R(&BucketSetMetadate{}, "bucket-set-tag", "set bucket tag", func(cli cloudprovider.ICloudRegion, args *BucketSetMetadate) error {
 		bucket, err := cli.GetIBucketById(args.BUCKET)
 		if err != nil {
 			return err
@@ -693,7 +696,7 @@ func S3Shell() {
 				tags[pair[0]] = pair[1]
 			}
 		}
-		err = cloudprovider.SetBucketMetadata(bucket, tags, args.Replace)
+		_, err = cloudprovider.SetBucketTags(bucket, tags)
 		if err != nil {
 			return err
 		}

pkg/multicloud/qcloud/bucket.go

@@ -1111,8 +1111,16 @@ func (b *SBucket) SetTags(tags map[string]string, replace bool) error {
 	}
 	coscli, err := b.region.GetCosClient(b)
 	if err != nil {
-		log.Errorf("GetCosClient fail %s", err)
-		return errors.Wrap(err, "b.region.GetCosClient(b)")
+		return errors.Wrapf(err, "b.region.GetCosClient(%s)", b.Name)
+	}
+
+	_, err = coscli.Bucket.DeleteTagging(context.Background())
+	if err != nil {
+		return errors.Wrapf(err, "DeleteTagging")
+	}
+
+	if len(tags) == 0 {
+		return nil
 	}
 
 	input := cos.BucketPutTaggingOptions{}
@@ -1127,32 +1135,6 @@ func (b *SBucket) SetTags(tags map[string]string, replace bool) error {
 	return nil
 }
 
-func (b *SBucket) DeleteTags() error {
-	coscli, err := b.region.GetCosClient(b)
-	if err != nil {
-		log.Errorf("GetCosClient fail %s", err)
-		return errors.Wrap(err, "b.region.GetCosClient(b)")
-	}
-	_, err = coscli.Bucket.DeleteTagging(context.Background())
-	if err != nil {
-		return errors.Wrap(err, "coscli.Bucket.DeleteTagging(context.Background())")
-	}
-	return nil
-}
-
-func (b *SBucket) GetMetadata() *jsonutils.JSONDict {
-	meta := jsonutils.NewDict()
-	tags, err := b.GetTags()
-	if err != nil {
-		log.Errorf("error:%s b.getTags()", err)
-		return meta
-	}
-	for k, v := range tags {
-		meta.Add(jsonutils.NewString(v), k)
-	}
-	return meta
-}
-
 func (b *SBucket) ListMultipartUploads() ([]cloudprovider.SBucketMultipartUploads, error) {
 	coscli, err := b.region.GetCosClient(b)
 	if err != nil {