From a1820bc331dc5fd3a1c2af5fd88ff2004c307188 Mon Sep 17 00:00:00 2001 From: Qu Xuan Date: Thu, 11 Mar 2021 17:48:31 +0800 Subject: [PATCH] fix: optimzed tag update and sync --- pkg/cloudprovider/fakeregion.go | 4 +- pkg/compute/guestdrivers/managedvirtual.go | 51 ++++++++++++------- pkg/compute/models/syncutils.go | 5 +- pkg/compute/regiondrivers/managedvirtual.go | 27 +++++++++- pkg/multicloud/aliyun/securitygroup.go | 3 -- pkg/multicloud/apsara/dbinstance.go | 2 +- .../apsara/elasticcache_instance.go | 2 +- pkg/multicloud/apsara/loadbalancer.go | 2 +- pkg/multicloud/apsara/securitygroup.go | 3 -- pkg/multicloud/aws/instance.go | 2 +- pkg/multicloud/qcloud/bucket.go | 5 +- .../qcloud/elasticcache_instance.go | 2 +- pkg/multicloud/qcloud/instance.go | 2 +- pkg/multicloud/qcloud/rds_mysql.go | 2 +- pkg/multicloud/resource_base.go | 8 ++- 15 files changed, 77 insertions(+), 43 deletions(-) diff --git a/pkg/cloudprovider/fakeregion.go b/pkg/cloudprovider/fakeregion.go index 2165755c78..25f53821a2 100644 --- a/pkg/cloudprovider/fakeregion.go +++ b/pkg/cloudprovider/fakeregion.go @@ -14,6 +14,8 @@ package cloudprovider +import "yunion.io/x/pkg/errors" + type SFakeOnPremiseRegion struct { } @@ -54,7 +56,7 @@ func (region *SFakeOnPremiseRegion) GetSysTags() map[string]string { } func (region *SFakeOnPremiseRegion) GetTags() (map[string]string, error) { - return nil, nil + return nil, errors.Wrap(ErrNotImplemented, "GetTags") } func (region *SFakeOnPremiseRegion) SetTags(tags map[string]string, replace bool) error { diff --git a/pkg/compute/guestdrivers/managedvirtual.go b/pkg/compute/guestdrivers/managedvirtual.go index 18f15b3d68..eebe84f90d 100644 --- a/pkg/compute/guestdrivers/managedvirtual.go +++ b/pkg/compute/guestdrivers/managedvirtual.go @@ -1229,26 +1229,39 @@ func (self *SManagedVirtualizedGuestDriver) RequestRemoteUpdate(ctx context.Cont if err != nil { return errors.Wrap(err, "guest.GetIVM") } - oldTags, err := iVM.GetTags() + + err = func() error { + oldTags, err := iVM.GetTags() + if err != nil { + if errors.Cause(err) == cloudprovider.ErrNotSupported || errors.Cause(err) == cloudprovider.ErrNotImplemented { + return nil + } + return errors.Wrap(err, "iVM.GetTags()") + } + tags, err := guest.GetAllUserMetadata() + if err != nil { + return errors.Wrapf(err, "GetAllUserMetadata") + } + tagsUpdateInfo := cloudprovider.TagsUpdateInfo{OldTags: oldTags, NewTags: tags} + err = iVM.SetTags(tags, replaceTags) + if err != nil { + if errors.Cause(err) == cloudprovider.ErrNotSupported || errors.Cause(err) == cloudprovider.ErrNotImplemented { + return nil + } + logclient.AddSimpleActionLog(guest, logclient.ACT_UPDATE_TAGS, err, userCred, false) + return errors.Wrap(err, "iVM.SetTags") + } + logclient.AddSimpleActionLog(guest, logclient.ACT_UPDATE_TAGS, tagsUpdateInfo, userCred, true) + // sync back cloud metadata + iVM.Refresh() + err = models.SyncVirtualResourceMetadata(ctx, userCred, guest, iVM) + if err != nil { + return errors.Wrap(err, "syncVirtualResourceMetadata") + } + return nil + }() if err != nil { - return errors.Wrap(err, "iVM.GetTags()") - } - tags, err := guest.GetAllUserMetadata() - if err != nil { - return errors.Wrapf(err, "GetAllUserMetadata") - } - tagsUpdateInfo := cloudprovider.TagsUpdateInfo{OldTags: oldTags, NewTags: tags} - err = iVM.SetTags(tags, replaceTags) - if err != nil { - logclient.AddSimpleActionLog(guest, logclient.ACT_UPDATE_TAGS, err, userCred, false) - return errors.Wrap(err, "iVM.SetTags") - } - logclient.AddSimpleActionLog(guest, logclient.ACT_UPDATE_TAGS, tagsUpdateInfo, userCred, true) - // sync back cloud metadata - iVM.Refresh() - err = models.SyncVirtualResourceMetadata(ctx, userCred, guest, iVM) - if err != nil { - return errors.Wrap(err, "syncVirtualResourceMetadata") + return err } err = iVM.UpdateVM(ctx, guest.Name) diff --git a/pkg/compute/models/syncutils.go b/pkg/compute/models/syncutils.go index beac0c7d48..f9a79c2f1f 100644 --- a/pkg/compute/models/syncutils.go +++ b/pkg/compute/models/syncutils.go @@ -18,7 +18,6 @@ import ( "context" "yunion.io/x/log" - "yunion.io/x/pkg/errors" "yunion.io/x/onecloud/pkg/cloudcommon/db" "yunion.io/x/onecloud/pkg/cloudprovider" @@ -42,7 +41,7 @@ func syncMetadata(ctx context.Context, userCred mcclient.TokenCredential, model model.SetSysCloudMetadataAll(ctx, sysStore, userCred) tags, err := remote.GetTags() - if err == nil || errors.Cause(err) == cloudprovider.ErrNotFound { + if err == nil { store := make(map[string]interface{}, 0) for key, value := range tags { store[db.CLOUD_TAG_PREFIX+key] = value @@ -71,7 +70,7 @@ func syncVirtualResourceMetadata(ctx context.Context, userCred mcclient.TokenCre model.SetSysCloudMetadataAll(ctx, sysStore, userCred) tags, err := remote.GetTags() - if err == nil || errors.Cause(err) == cloudprovider.ErrNotFound { + if err == nil { store := make(map[string]interface{}, 0) for key, value := range tags { store[db.CLOUD_TAG_PREFIX+key] = value diff --git a/pkg/compute/regiondrivers/managedvirtual.go b/pkg/compute/regiondrivers/managedvirtual.go index fa3512ce3c..e4014c9604 100644 --- a/pkg/compute/regiondrivers/managedvirtual.go +++ b/pkg/compute/regiondrivers/managedvirtual.go @@ -332,6 +332,9 @@ func (self *SManagedVirtualizationRegionDriver) RequestRemoteUpdateLoadbalancer( } oldTags, err := iLoadbalancer.GetTags() if err != nil { + if errors.Cause(err) == cloudprovider.ErrNotSupported || errors.Cause(err) == cloudprovider.ErrNotImplemented { + return nil, nil + } return nil, errors.Wrap(err, "iLoadbalancer.GetTags()") } tags, err := lb.GetAllUserMetadata() @@ -341,6 +344,9 @@ func (self *SManagedVirtualizationRegionDriver) RequestRemoteUpdateLoadbalancer( tagsUpdateInfo := cloudprovider.TagsUpdateInfo{OldTags: oldTags, NewTags: tags} err = iLoadbalancer.SetTags(tags, replaceTags) if err != nil { + if errors.Cause(err) == cloudprovider.ErrNotSupported || errors.Cause(err) == cloudprovider.ErrNotImplemented { + return nil, nil + } logclient.AddActionLogWithStartable(task, lb, logclient.ACT_UPDATE_TAGS, err, userCred, false) return nil, errors.Wrap(err, "iLoadbalancer.SetMetadata") } @@ -2628,7 +2634,10 @@ func (self *SManagedVirtualizationRegionDriver) RequestRemoteUpdateDBInstance(ct return nil, errors.Wrap(err, "instance.GetIDBInstance") } oldTags, err := iRds.GetTags() - if err != nil && errors.Cause(err) != cloudprovider.ErrNotFound { + if err != nil { + if errors.Cause(err) == cloudprovider.ErrNotSupported || errors.Cause(err) == cloudprovider.ErrNotImplemented { + return nil, nil + } return nil, errors.Wrap(err, "iRds.GetTags()") } tags, err := instance.GetAllUserMetadata() @@ -2638,6 +2647,9 @@ func (self *SManagedVirtualizationRegionDriver) RequestRemoteUpdateDBInstance(ct tagsUpdateInfo := cloudprovider.TagsUpdateInfo{OldTags: oldTags, NewTags: tags} err = iRds.SetTags(tags, replaceTags) if err != nil { + if errors.Cause(err) == cloudprovider.ErrNotSupported || errors.Cause(err) == cloudprovider.ErrNotImplemented { + return nil, nil + } logclient.AddActionLogWithStartable(task, instance, logclient.ACT_UPDATE_TAGS, err, userCred, false) return nil, errors.Wrap(err, "iRds.SetTags") } @@ -3026,8 +3038,15 @@ func (self *SManagedVirtualizationRegionDriver) RequestRemoteUpdateElasticcache( } iElasticcache, err := iRegion.GetIElasticcacheById(elasticcache.ExternalId) + if err != nil { + return nil, errors.Wrapf(err, "GetIElasticcacheById(%s)", elasticcache.ExternalId) + } + oldTags, err := iElasticcache.GetTags() - if err != nil && errors.Cause(err) != cloudprovider.ErrNotFound { + if err != nil { + if errors.Cause(err) == cloudprovider.ErrNotSupported || errors.Cause(err) == cloudprovider.ErrNotImplemented { + return nil, nil + } return nil, errors.Wrap(err, "iElasticcache.GetTags()") } tags, err := elasticcache.GetAllUserMetadata() @@ -3037,6 +3056,10 @@ func (self *SManagedVirtualizationRegionDriver) RequestRemoteUpdateElasticcache( tagsUpdateInfo := cloudprovider.TagsUpdateInfo{OldTags: oldTags, NewTags: tags} err = iElasticcache.SetTags(tags, replaceTags) if err != nil { + if errors.Cause(err) == cloudprovider.ErrNotSupported || errors.Cause(err) == cloudprovider.ErrNotImplemented { + return nil, nil + } + logclient.AddActionLogWithStartable(task, elasticcache, logclient.ACT_UPDATE_TAGS, err, userCred, false) return nil, errors.Wrap(err, "iElasticcache.SetTags") } diff --git a/pkg/multicloud/aliyun/securitygroup.go b/pkg/multicloud/aliyun/securitygroup.go index 1cbd180db5..da4c29d1e4 100644 --- a/pkg/multicloud/aliyun/securitygroup.go +++ b/pkg/multicloud/aliyun/securitygroup.go @@ -102,9 +102,6 @@ func (self *SSecurityGroup) GetMetadata() *jsonutils.JSONDict { } func (self *SSecurityGroup) GetTags() (map[string]string, error) { - if len(self.Tags.Tag) == 0 { - return nil, nil - } tags := map[string]string{} for _, value := range self.Tags.Tag { tags[value.TagKey] = value.TagValue diff --git a/pkg/multicloud/apsara/dbinstance.go b/pkg/multicloud/apsara/dbinstance.go index 2dde8a5f0e..b2c9b98f82 100644 --- a/pkg/multicloud/apsara/dbinstance.go +++ b/pkg/multicloud/apsara/dbinstance.go @@ -798,7 +798,7 @@ func (rds *SDBInstance) GetTags() (map[string]string, error) { return nil, errors.Wrap(err, "rds.region.ListResourceTags") } if _, ok := tags[rds.GetId()]; !ok { - return nil, cloudprovider.ErrNotFound + return map[string]string{}, nil } return *tags[rds.GetId()], nil } diff --git a/pkg/multicloud/apsara/elasticcache_instance.go b/pkg/multicloud/apsara/elasticcache_instance.go index a92752ab84..96c5b7fe2e 100644 --- a/pkg/multicloud/apsara/elasticcache_instance.go +++ b/pkg/multicloud/apsara/elasticcache_instance.go @@ -943,7 +943,7 @@ func (instance *SElasticcache) GetTags() (map[string]string, error) { return nil, errors.Wrap(err, "instance.region.ListResourceTags") } if _, ok := tags[instance.GetId()]; !ok { - return nil, cloudprovider.ErrNotFound + return map[string]string{}, nil } return *tags[instance.GetId()], nil } diff --git a/pkg/multicloud/apsara/loadbalancer.go b/pkg/multicloud/apsara/loadbalancer.go index 3d7fc6bbee..5ec2156900 100644 --- a/pkg/multicloud/apsara/loadbalancer.go +++ b/pkg/multicloud/apsara/loadbalancer.go @@ -122,7 +122,7 @@ func (lb *SLoadbalancer) GetTags() (map[string]string, error) { return nil, errors.Wrap(err, "lb.region.ListResourceTags") } if _, ok := tags[lb.GetId()]; !ok { - return nil, cloudprovider.ErrNotFound + return map[string]string{}, nil } return *tags[lb.GetId()], nil } diff --git a/pkg/multicloud/apsara/securitygroup.go b/pkg/multicloud/apsara/securitygroup.go index 82472ae42f..6d4d527a81 100644 --- a/pkg/multicloud/apsara/securitygroup.go +++ b/pkg/multicloud/apsara/securitygroup.go @@ -102,9 +102,6 @@ func (self *SSecurityGroup) GetMetadata() *jsonutils.JSONDict { } func (self *SSecurityGroup) GetTags() (map[string]string, error) { - if len(self.Tags.Tag) == 0 { - return nil, nil - } tags := map[string]string{} for _, value := range self.Tags.Tag { tags[value.TagKey] = value.TagValue diff --git a/pkg/multicloud/aws/instance.go b/pkg/multicloud/aws/instance.go index 6c50dc7923..887276c93f 100644 --- a/pkg/multicloud/aws/instance.go +++ b/pkg/multicloud/aws/instance.go @@ -286,7 +286,7 @@ func (self *SInstance) GetTags() (map[string]string, error) { } tags, err := FetchTags(ec2Client, self.InstanceId) if err != nil { - return nil, errors.Wrap(err, "FetchTags(self.host.zone.region.ec2Client, self.InstanceId)") + return nil, errors.Wrap(err, "FetchTags()") } data := map[string]string{} err = tags.Unmarshal(&data) diff --git a/pkg/multicloud/qcloud/bucket.go b/pkg/multicloud/qcloud/bucket.go index 2cd0458345..29a581c415 100644 --- a/pkg/multicloud/qcloud/bucket.go +++ b/pkg/multicloud/qcloud/bucket.go @@ -1087,8 +1087,7 @@ func (b *SBucket) DeletePolicy(id []string) ([]cloudprovider.SBucketPolicyStatem func (b *SBucket) GetTags() (map[string]string, error) { coscli, err := b.region.GetCosClient(b) if err != nil { - log.Errorf("GetCosClient fail %s", err) - return nil, errors.Wrap(err, "b.region.GetCosClient(b)") + return nil, errors.Wrap(err, "GetCosClient") } tagresult, _, err := coscli.Bucket.GetTagging(context.Background()) @@ -1096,7 +1095,7 @@ func (b *SBucket) GetTags() (map[string]string, error) { if strings.Contains(err.Error(), "404") { return nil, nil } - return nil, errors.Wrap(err, "coscli.Bucket.GetTagging(context.Background())") + return nil, errors.Wrap(err, "GetTagging") } result := map[string]string{} for i := range tagresult.TagSet { diff --git a/pkg/multicloud/qcloud/elasticcache_instance.go b/pkg/multicloud/qcloud/elasticcache_instance.go index d5b3acce68..b31663187c 100644 --- a/pkg/multicloud/qcloud/elasticcache_instance.go +++ b/pkg/multicloud/qcloud/elasticcache_instance.go @@ -266,7 +266,7 @@ func (self *SElasticcache) GetTags() (map[string]string, error) { return nil, errors.Wrap(err, "self.region.FetchResourceTags") } if _, ok := tags[self.GetId()]; !ok { - return nil, cloudprovider.ErrNotFound + return map[string]string{}, nil } return *tags[self.GetId()], nil } diff --git a/pkg/multicloud/qcloud/instance.go b/pkg/multicloud/qcloud/instance.go index ce1f653ee3..656b14711c 100644 --- a/pkg/multicloud/qcloud/instance.go +++ b/pkg/multicloud/qcloud/instance.go @@ -220,7 +220,7 @@ func (self *SInstance) GetTags() (map[string]string, error) { if tags, ok := mtags[self.InstanceId]; ok { return *tags, nil } - return nil, cloudprovider.ErrNotFound + return map[string]string{}, nil } func (self *SInstance) getCloudMetadata() (map[string]string, error) { diff --git a/pkg/multicloud/qcloud/rds_mysql.go b/pkg/multicloud/qcloud/rds_mysql.go index 4930d38eb8..8045df884b 100644 --- a/pkg/multicloud/qcloud/rds_mysql.go +++ b/pkg/multicloud/qcloud/rds_mysql.go @@ -903,7 +903,7 @@ func (self *SMySQLInstance) GetTags() (map[string]string, error) { return nil, errors.Wrap(err, "self.region.FetchResourceTags") } if _, ok := tags[self.GetId()]; !ok { - return nil, cloudprovider.ErrNotFound + return map[string]string{}, nil } return *tags[self.GetId()], nil } diff --git a/pkg/multicloud/resource_base.go b/pkg/multicloud/resource_base.go index cbc177d28e..d617762efb 100644 --- a/pkg/multicloud/resource_base.go +++ b/pkg/multicloud/resource_base.go @@ -14,7 +14,11 @@ package multicloud -import "yunion.io/x/onecloud/pkg/cloudprovider" +import ( + "yunion.io/x/pkg/errors" + + "yunion.io/x/onecloud/pkg/cloudprovider" +) type SResourceBase struct{} @@ -31,7 +35,7 @@ func (self *SResourceBase) GetSysTags() map[string]string { } func (self *SResourceBase) GetTags() (map[string]string, error) { - return nil, nil + return nil, errors.Wrapf(cloudprovider.ErrNotImplemented, "GetTags") } func (self *SResourceBase) SetTags(tags map[string]string, replace bool) error {