Merge pull request #7995 from tb365/automated-cherry-pick-of-#7994-upstream-release-3.2

Automated cherry pick of #7994: ctyun sync secgroup fix & cloudaccount delete fix

pkg/compute/models/guestdisks.go

@@ -187,7 +187,7 @@ func (self *SGuestdisk) DoSave(driver string, cache string, mountpoint string) e
 func (self *SGuestdisk) GetDisk() *SDisk {
 	disk, err := DiskManager.FetchById(self.DiskId)
 	if err != nil {
-		log.Errorf("GetDisk fail: %s", err)
+		log.Errorf("GetDisk %s fail: %s", self.DiskId, err)
 		return nil
 	}
 	return disk.(*SDisk)
@@ -287,6 +287,10 @@ func (self *SGuestdisk) GetDetailedJson() *jsonutils.JSONDict {
 
 func (self *SGuestdisk) GetDetailedString() string {
 	disk := self.GetDisk()
+	if disk == nil {
+		return ""
+	}
+
 	var fs string
 	if len(disk.GetTemplateId()) > 0 {
 		fs = "root"

pkg/compute/models/guests.go

@@ -1974,8 +1974,10 @@ func (self *SGuest) getNetworksDetails() string {
 func (self *SGuest) getDisksDetails() string {
 	var buf bytes.Buffer
 	for _, disk := range self.GetDisks() {
-		buf.WriteString(disk.GetDetailedString())
-		buf.WriteString("\n")
+		if details := disk.GetDetailedString(); len(details) > 0 {
+			buf.WriteString(details)
+			buf.WriteString("\n")
+		}
 	}
 	return buf.String()
 }

pkg/compute/tasks/cloud_account_delete_task.go

@@ -16,8 +16,11 @@ package tasks
 
 import (
 	"context"
+	"database/sql"
+	"fmt"
 
 	"yunion.io/x/jsonutils"
+	"yunion.io/x/pkg/errors"
 
 	api "yunion.io/x/onecloud/pkg/apis/compute"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db"
@@ -62,6 +65,18 @@ func (self *CloudAccountDeleteTask) OnInit(ctx context.Context, obj db.IStandalo
 func (self *CloudAccountDeleteTask) OnAllCloudProviderDeleteComplete(ctx context.Context, obj db.IStandaloneModel, body jsonutils.JSONObject) {
 	account := obj.(*models.SCloudaccount)
 
+	// check providers deleted success
+	count, err := account.GetProviderCount()
+	if err != nil && errors.Cause(err) != sql.ErrNoRows {
+		self.OnAllCloudProviderDeleteCompleteFailed(ctx, obj, jsonutils.NewString(err.Error()))
+		return
+	}
+
+	if count > 0 {
+		self.OnAllCloudProviderDeleteCompleteFailed(ctx, obj, jsonutils.NewString(fmt.Sprintf("cloudprovider deleted failed count %d", count)))
+		return
+	}
+
 	account.RealDelete(ctx, self.UserCred)
 
 	self.SetStageComplete(ctx, nil)

pkg/multicloud/ctyun/instance.go

@@ -365,7 +365,7 @@ func (self *SInstance) GetInstanceType() string {
 
 func (self *SInstance) GetSecurityGroupIds() ([]string, error) {
 	if len(self.SecurityGroups) == 0 {
-		return nil, nil
+		return []string{}, nil
 	}
 
 	if len(self.MasterOrderId) > 0 {
@@ -423,8 +423,34 @@ func (self *SInstance) AssignSecurityGroup(secgroupId string) error {
 }
 
 func (self *SInstance) SetSecurityGroups(secgroupIds []string) error {
-	for i := 0; i < len(secgroupIds); i++ {
-		err := self.host.zone.region.AssignSecurityGroup(self.GetId(), secgroupIds[i])
+	currentIds, err := self.GetSecurityGroupIds()
+	if err != nil {
+		return errors.Wrap(err, "GetSecurityGroupIds")
+	}
+
+	adds := []string{}
+	for i := range secgroupIds {
+		if !utils.IsInStringArray(secgroupIds[i], currentIds) {
+			adds = append(adds, secgroupIds[i])
+		}
+	}
+
+	for i := range adds {
+		err := self.host.zone.region.AssignSecurityGroup(self.GetId(), adds[i])
+		if err != nil {
+			return errors.Wrap(err, "Instance.SetSecurityGroups")
+		}
+	}
+
+	removes := []string{}
+	for i := range currentIds {
+		if !utils.IsInStringArray(currentIds[i], secgroupIds) {
+			removes = append(removes, currentIds[i])
+		}
+	}
+
+	for i := range removes {
+		err := self.host.zone.region.UnsignSecurityGroup(self.GetId(), removes[i])
 		if err != nil {
 			return errors.Wrap(err, "Instance.SetSecurityGroups")
 		}