Skip to content

Commit

Permalink
fix: cloud account delete may fail due to inconsistent status
Browse files Browse the repository at this point in the history
  • Loading branch information
Qiu Jian committed Jul 25, 2020
1 parent 58ed07b commit f624ab1
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 33 deletions.
2 changes: 1 addition & 1 deletion pkg/apis/compute/cloudaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ type CloudaccountCreateInput struct {

// 自动根据云上项目或订阅创建本地项目
// default: false
AutoCreateProject bool `json:"auto_create_project"`
AutoCreateProject *bool `json:"auto_create_project"`

// 额外信息,例如账单的access key
Options *jsonutils.JSONDict `json:"options"`
Expand Down
30 changes: 13 additions & 17 deletions pkg/compute/models/cloudaccounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"yunion.io/x/onecloud/pkg/apis"
proxyapi "yunion.io/x/onecloud/pkg/apis/cloudcommon/proxy"
api "yunion.io/x/onecloud/pkg/apis/compute"
"yunion.io/x/onecloud/pkg/cloudcommon/consts"
"yunion.io/x/onecloud/pkg/cloudcommon/db"
"yunion.io/x/onecloud/pkg/cloudcommon/db/lockman"
"yunion.io/x/onecloud/pkg/cloudcommon/db/proxy"
Expand All @@ -49,7 +48,6 @@ import (
"yunion.io/x/onecloud/pkg/httperrors"
"yunion.io/x/onecloud/pkg/mcclient"
"yunion.io/x/onecloud/pkg/mcclient/auth"
"yunion.io/x/onecloud/pkg/mcclient/modules"
"yunion.io/x/onecloud/pkg/multicloud/esxi/vcenter"
"yunion.io/x/onecloud/pkg/util/choices"
"yunion.io/x/onecloud/pkg/util/httputils"
Expand Down Expand Up @@ -803,10 +801,22 @@ func (manager *SCloudaccountManager) ValidateCreateData(
}

if len(input.Project) > 0 {
_, input.ProjectizedResourceInput, err = db.ValidateProjectizedResourceInput(ctx, input.ProjectizedResourceInput)
var proj *db.STenant
proj, input.ProjectizedResourceInput, err = db.ValidateProjectizedResourceInput(ctx, input.ProjectizedResourceInput)
if err != nil {
return input, errors.Wrap(err, "db.ValidateProjectizedResourceInput")
}
if proj.DomainId != ownerId.GetProjectDomainId() {
return input, httperrors.NewInputParameterError("Project %s(%s) not belong to domain %s(%s)", proj.Name, proj.Id, ownerId.GetProjectDomain(), ownerId.GetProjectDomainId())
}
if input.AutoCreateProject != nil && *input.AutoCreateProject {
log.Warningf("project_id and auto_create_project should not be turned on at the same time")
}
input.AutoCreateProject = nil
} else if input.AutoCreateProject == nil || !*input.AutoCreateProject {
log.Warningf("auto_create_project is off while no project_id specified")
createProject := true
input.AutoCreateProject = &createProject
}

if !cloudprovider.IsSupported(input.Provider) {
Expand Down Expand Up @@ -890,20 +900,6 @@ func (manager *SCloudaccountManager) ValidateCreateData(
input.SyncIntervalSeconds = options.Options.MinimalSyncIntervalSeconds
}

if !input.AutoCreateProject {
if userCred.GetProjectDomainId() != ownerId.GetProjectDomainId() {
s := auth.GetAdminSession(ctx, consts.GetRegion(), "v1")
params := jsonutils.Marshal(map[string]string{"domain_id": ownerId.GetProjectDomainId()})
tenants, err := modules.Projects.List(s, params)
if err != nil {
return input, err
}
if tenants.Total == 0 {
return input, httperrors.NewInputParameterError("There is no projects under the domain %s", ownerId.GetProjectDomainId())
}
}
}

input.EnabledStatusInfrasResourceBaseCreateInput, err = manager.SEnabledStatusInfrasResourceBaseManager.ValidateCreateData(ctx, userCred, ownerId, query, input.EnabledStatusInfrasResourceBaseCreateInput)
if err != nil {
return input, err
Expand Down
35 changes: 35 additions & 0 deletions pkg/compute/models/cloudsync.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ func syncRegionVPCs(ctx context.Context, userCred mcclient.TokenCredential, sync
lockman.LockObject(ctx, &localVpcs[j])
defer lockman.ReleaseObject(ctx, &localVpcs[j])

if localVpcs[j].Deleted {
return
}

syncVpcWires(ctx, userCred, syncResults, provider, &localVpcs[j], remoteVpcs[j], syncRange)
if localRegion.GetDriver().IsSecurityGroupBelongVpc() || localRegion.GetDriver().IsSupportClassicSecurityGroup() || j == 0 { //有vpc属性的每次都同步,支持classic的vpc也同步,否则仅同步一次
syncVpcSecGroup(ctx, userCred, syncResults, provider, &localVpcs[j], remoteVpcs[j], syncRange)
Expand Down Expand Up @@ -311,6 +315,10 @@ func syncVpcNatgateways(ctx context.Context, userCred mcclient.TokenCredential,
lockman.LockObject(ctx, &localNatGateways[i])
defer lockman.ReleaseObject(ctx, &localNatGateways[i])

if localNatGateways[i].Deleted {
return
}

syncNatGatewayEips(ctx, userCred, provider, &localNatGateways[i], remoteNatGateways[i])
syncNatDTable(ctx, userCred, provider, &localNatGateways[i], remoteNatGateways[i])
syncNatSTable(ctx, userCred, provider, &localNatGateways[i], remoteNatGateways[i])
Expand Down Expand Up @@ -392,6 +400,9 @@ func syncVpcWires(ctx context.Context, userCred mcclient.TokenCredential, syncRe
lockman.LockObject(ctx, &localWires[i])
defer lockman.ReleaseObject(ctx, &localWires[i])

if localWires[i].Deleted {
return
}
syncWireNetworks(ctx, userCred, syncResults, provider, &localWires[i], remoteWires[i], syncRange)
}()
}
Expand Down Expand Up @@ -446,6 +457,10 @@ func syncZoneStorages(ctx context.Context, userCred mcclient.TokenCredential, sy
lockman.LockObject(ctx, &localStorages[i])
defer lockman.ReleaseObject(ctx, &localStorages[i])

if localStorages[i].Deleted {
return
}

if !isInCache(storageCachePairs, localStorages[i].StoragecacheId) && !isInCache(newCacheIds, localStorages[i].StoragecacheId) {
cachePair := syncStorageCaches(ctx, userCred, provider, &localStorages[i], remoteStorages[i])
if cachePair.remote != nil && cachePair.local != nil {
Expand Down Expand Up @@ -531,6 +546,10 @@ func syncZoneHosts(ctx context.Context, userCred mcclient.TokenCredential, syncR
lockman.LockObject(ctx, &localHosts[i])
defer lockman.ReleaseObject(ctx, &localHosts[i])

if localHosts[i].Deleted {
return
}

syncMetadata(ctx, userCred, &localHosts[i], remoteHosts[i])
newCachePairs = syncHostStorages(ctx, userCred, syncResults, provider, &localHosts[i], remoteHosts[i], storageCachePairs)
syncHostWires(ctx, userCred, syncResults, provider, &localHosts[i], remoteHosts[i])
Expand Down Expand Up @@ -624,6 +643,10 @@ func syncHostVMs(ctx context.Context, userCred mcclient.TokenCredential, syncRes
lockman.LockObject(ctx, syncVMPairs[i].Local)
defer lockman.ReleaseObject(ctx, syncVMPairs[i].Local)

if syncVMPairs[i].Local.Deleted || syncVMPairs[i].Local.PendingDeleted {
return
}

syncVMPeripherals(ctx, userCred, syncVMPairs[i].Local, syncVMPairs[i].Remote, localHost, provider, driver)
// syncMetadata(ctx, userCred, syncVMPairs[i].Local, syncVMPairs[i].Remote)
// syncVMNics(ctx, userCred, provider, localHost, syncVMPairs[i].Local, syncVMPairs[i].Remote)
Expand Down Expand Up @@ -773,6 +796,10 @@ func syncRegionDBInstances(ctx context.Context, userCred mcclient.TokenCredentia
lockman.LockObject(ctx, &localInstances[i])
defer lockman.ReleaseObject(ctx, &localInstances[i])

if localInstances[i].Deleted || localInstances[i].PendingDeleted {
return
}

syncDBInstanceNetwork(ctx, userCred, syncResults, &localInstances[i], remoteInstances[i])
syncDBInstanceParameters(ctx, userCred, syncResults, &localInstances[i], remoteInstances[i])
syncDBInstanceDatabases(ctx, userCred, syncResults, &localInstances[i], remoteInstances[i])
Expand Down Expand Up @@ -880,6 +907,10 @@ func syncDBInstanceAccounts(ctx context.Context, userCred mcclient.TokenCredenti
lockman.LockObject(ctx, &localAccounts[i])
defer lockman.ReleaseObject(ctx, &localAccounts[i])

if localAccounts[i].Deleted {
return
}

syncDBInstanceAccountPrivileges(ctx, userCred, syncResults, &localAccounts[i], remoteAccounts[i])

}()
Expand Down Expand Up @@ -963,6 +994,10 @@ func syncRegionNetworkInterfaces(ctx context.Context, userCred mcclient.TokenCre
lockman.LockObject(ctx, &localInterfaces[i])
defer lockman.ReleaseObject(ctx, &localInterfaces[i])

if localInterfaces[i].Deleted {
return
}

syncInterfaceAddresses(ctx, userCred, &localInterfaces[i], remoteInterfaces[i])
}()
}
Expand Down
10 changes: 7 additions & 3 deletions pkg/compute/models/hosts.go
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ func (self *SHost) GetHoststorages() []SHoststorage {
hoststorages := make([]SHoststorage, 0)
q := self.GetHoststoragesQuery()
err := db.FetchModelObjects(HoststorageManager, q, &hoststorages)
if err != nil {
if err != nil && errors.Cause(err) != sql.ErrNoRows {
log.Errorf("GetHoststorages error %s", err)
return nil
}
Expand All @@ -733,7 +733,9 @@ func (self *SHost) GetHoststorageOfId(storageId string) *SHoststorage {
hoststorage.SetModelManager(HoststorageManager, &hoststorage)
err := self.GetHoststoragesQuery().Equals("storage_id", storageId).First(&hoststorage)
if err != nil {
log.Errorf("GetHoststorageOfId fail %s", err)
if errors.Cause(err) != sql.ErrNoRows {
log.Errorf("GetHoststorageOfId fail %s", err)
}
return nil
}
return &hoststorage
Expand All @@ -752,7 +754,9 @@ func (self *SHost) GetHoststorageByExternalId(extId string) *SHoststorage {

err := q.First(&hoststorage)
if err != nil {
log.Errorf("GetHoststorageByExternalId fail %s", err)
if errors.Cause(err) != sql.ErrNoRows {
log.Errorf("GetHoststorageByExternalId fail %s", err)
}
return nil
}

Expand Down
43 changes: 32 additions & 11 deletions pkg/compute/tasks/cloud_account_sync_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,32 @@ func (self *CloudAccountSyncInfoTask) OnInit(ctx context.Context, obj db.IStanda
db.OpsLog.LogEvent(cloudaccount, db.ACT_SYNCING_HOST, "", self.UserCred)
// cloudaccount.MarkSyncing(self.UserCred)

// do sync
err := cloudaccount.SyncCallSyncAccountTask(ctx, self.UserCred)
self.SetStage("OnCloudaccountSyncReady", nil)

if err != nil {
if errors.Cause(err) != httperrors.ErrConflict {
log.Debugf("no other sync task, mark end sync for all cloudproviders")
cloudaccount.MarkEndSyncWithLock(ctx, self.UserCred)
taskman.LocalTaskRun(self, func() (jsonutils.JSONObject, error) {
// do sync
err := cloudaccount.SyncCallSyncAccountTask(ctx, self.UserCred)

if err != nil {
if errors.Cause(err) != httperrors.ErrConflict {
log.Debugf("no other sync task, mark end sync for all cloudproviders")
cloudaccount.MarkEndSyncWithLock(ctx, self.UserCred)
}
return nil, errors.Wrap(err, "SyncCallSyncAccountTask")
}
db.OpsLog.LogEvent(cloudaccount, db.ACT_SYNC_HOST_FAILED, err, self.UserCred)
self.SetStageFailed(ctx, err.Error())
logclient.AddActionLogWithStartable(self, cloudaccount, logclient.ACT_CLOUD_SYNC, err, self.UserCred, false)
return
}
return nil, nil
})
}

func (self *CloudAccountSyncInfoTask) OnCloudaccountSyncReadyFailed(ctx context.Context, obj db.IStandaloneModel, err jsonutils.JSONObject) {
cloudaccount := obj.(*models.SCloudaccount)
db.OpsLog.LogEvent(cloudaccount, db.ACT_SYNC_HOST_FAILED, err, self.UserCred)
self.SetStageFailed(ctx, err.String())
logclient.AddActionLogWithStartable(self, cloudaccount, logclient.ACT_CLOUD_SYNC, err, self.UserCred, false)
}

func (self *CloudAccountSyncInfoTask) OnCloudaccountSyncReady(ctx context.Context, obj db.IStandaloneModel, body jsonutils.JSONObject) {
cloudaccount := obj.(*models.SCloudaccount)

driver, err := cloudaccount.GetProvider()
if err != nil {
Expand Down Expand Up @@ -111,3 +124,11 @@ func (self *CloudAccountSyncInfoTask) OnCloudaccountSyncComplete(ctx context.Con
self.SetStageComplete(ctx, nil)
logclient.AddActionLogWithStartable(self, cloudaccount, logclient.ACT_CLOUD_SYNC, "", self.UserCred, true)
}

func (self *CloudAccountSyncInfoTask) OnCloudaccountSyncCompleteFailed(ctx context.Context, obj db.IStandaloneModel, err jsonutils.JSONObject) {
cloudaccount := obj.(*models.SCloudaccount)
cloudaccount.MarkEndSyncWithLock(ctx, self.UserCred)
db.OpsLog.LogEvent(cloudaccount, db.ACT_SYNC_HOST_FAILED, err, self.UserCred)
self.SetStageFailed(ctx, err.String())
logclient.AddActionLogWithStartable(self, cloudaccount, logclient.ACT_CLOUD_SYNC, err, self.UserCred, false)
}
1 change: 1 addition & 0 deletions pkg/mcclient/modules/mod_cloudaccounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func init() {
"Provider", "Brand",
"Enable_Auto_Sync", "Sync_Interval_Seconds",
"Share_Mode", "is_public", "public_scope",
"auto_create_project",
},
[]string{})

Expand Down
2 changes: 1 addition & 1 deletion pkg/mcclient/modules/mod_cloudproviderregions.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func init() {
"Enabled", "Sync_Status",
"Last_Sync", "Last_Sync_End_At", "Auto_Sync",
"last_deep_sync_at",
"Sync_Results"},
},
[]string{},
&Cloudproviders,
&Cloudregions)
Expand Down

0 comments on commit f624ab1

Please sign in to comment.