From 018d21c09b3354950f08d114d657ddbfdd54b87c Mon Sep 17 00:00:00 2001 From: ioito Date: Thu, 21 Apr 2022 16:42:40 +0800 Subject: [PATCH] fix(region): avoid panic --- pkg/compute/models/buckets.go | 5 +- pkg/compute/models/cloudaccounts.go | 16 +-- pkg/compute/models/cloudproviderregions.go | 65 +++++++----- pkg/compute/models/cloudproviders.go | 115 +++++++++++++-------- pkg/compute/models/cloudsync.go | 6 +- pkg/compute/models/disks.go | 4 +- pkg/compute/models/managedresource.go | 7 +- pkg/compute/models/quotas.go | 12 ++- pkg/scheduler/cache/candidate/base.go | 6 +- 9 files changed, 145 insertions(+), 91 deletions(-) diff --git a/pkg/compute/models/buckets.go b/pkg/compute/models/buckets.go index c5eadbbe97..8fad3ea5ef 100644 --- a/pkg/compute/models/buckets.go +++ b/pkg/compute/models/buckets.go @@ -1817,7 +1817,10 @@ func (bucket *SBucket) GetDetailsAccessInfo( if err != nil { return nil, err } - account := manager.GetCloudaccount() + account, err := manager.GetCloudaccount() + if err != nil { + return nil, err + } info.(*jsonutils.JSONDict).Add(jsonutils.NewString(account.Brand), "PROVIDER") return info, err } diff --git a/pkg/compute/models/cloudaccounts.go b/pkg/compute/models/cloudaccounts.go index 2509b721db..d86c1a368a 100644 --- a/pkg/compute/models/cloudaccounts.go +++ b/pkg/compute/models/cloudaccounts.go @@ -172,7 +172,7 @@ type SCloudaccount struct { SubAccounts *cloudprovider.SubAccounts `nullable:"true" get:"user" create:"optional"` // 缺失的权限，云账号操作资源时自动更新 - LakeOfPermissions api.SAccountPermissions `length:"medium" get:"user" list:"user"` + LakeOfPermissions *api.SAccountPermissions `length:"medium" get:"user" list:"user"` } func (self *SCloudaccount) GetCloudproviders() []SCloudprovider { @@ -970,20 +970,22 @@ func (self *SCloudaccount) UpdatePermission(ctx context.Context) func(string, st defer lockman.ReleaseRawObject(ctx, self.Id, key) db.Update(self, func() error { - if self.LakeOfPermissions == nil { - self.LakeOfPermissions = api.SAccountPermissions{} + data := api.SAccountPermissions{} + if self.LakeOfPermissions != nil { + data = *self.LakeOfPermissions } - _, ok := self.LakeOfPermissions[service] + _, ok := data[service] if !ok { - self.LakeOfPermissions[service] = api.SAccountPermission{} + data[service] = api.SAccountPermission{} } - permissions := self.LakeOfPermissions[service].Permissions + permissions := data[service].Permissions if !utils.IsInStringArray(permission, permissions) { permissions = append(permissions, permission) - self.LakeOfPermissions[service] = api.SAccountPermission{ + data[service] = api.SAccountPermission{ Permissions: permissions, } } + self.LakeOfPermissions = &data return nil }) } diff --git a/pkg/compute/models/cloudproviderregions.go b/pkg/compute/models/cloudproviderregions.go index f0c18289a8..11b368bb7a 100644 --- a/pkg/compute/models/cloudproviderregions.go +++ b/pkg/compute/models/cloudproviderregions.go @@ -30,6 +30,7 @@ import ( api "yunion.io/x/onecloud/pkg/apis/compute" "yunion.io/x/onecloud/pkg/cloudcommon/db" + "yunion.io/x/onecloud/pkg/compute/options" "yunion.io/x/onecloud/pkg/httperrors" "yunion.io/x/onecloud/pkg/mcclient" "yunion.io/x/onecloud/pkg/util/logclient" @@ -93,21 +94,20 @@ func (manager *SCloudproviderregionManager) GetSlaveFieldName() string { return "cloudregion_id" } -func (self *SCloudproviderregion) GetProvider() *SCloudprovider { +func (self *SCloudproviderregion) GetProvider() (*SCloudprovider, error) { providerObj, err := CloudproviderManager.FetchById(self.CloudproviderId) if err != nil { - log.Errorf("CloudproviderManager.FetchById fail %s", err) - return nil + return nil, errors.Wrapf(err, "CloudproviderManager.FetchById(%s)", self.CloudproviderId) } - return providerObj.(*SCloudprovider) + return providerObj.(*SCloudprovider), nil } -func (self *SCloudproviderregion) GetAccount() *SCloudaccount { - provider := self.GetProvider() - if provider != nil { - return provider.GetCloudaccount() +func (self *SCloudproviderregion) GetAccount() (*SCloudaccount, error) { + provider, err := self.GetProvider() + if err != nil { + return nil, err } - return nil + return provider.GetCloudaccount() } func (manager *SCloudproviderregionManager) FetchCustomizeColumns( @@ -142,9 +142,9 @@ func (manager *SCloudproviderregionManager) FetchCustomizeColumns( for i := range rows { if manager, ok := managers[managerIds[i]]; ok { rows[i].Cloudprovider = manager.Name - account := manager.GetCloudaccount() rows[i].EnableAutoSync = false rows[i].CloudproviderSyncStatus = manager.SyncStatus + account, _ := manager.GetCloudaccount() if account != nil { rows[i].CloudaccountId = account.Id rows[i].Cloudaccount = account.Name @@ -164,25 +164,31 @@ func (self *SCloudproviderregion) PostUpdate(ctx context.Context, userCred mccli self.SJointResourceBase.PostUpdate(ctx, userCred, query, data) if data.Contains("enabled") { enabled, _ := data.Bool("enabled") - provider := self.GetProvider() - action := logclient.ACT_DISABLE - if enabled { - action = logclient.ACT_ENABLE - } - region, err := self.GetRegion() - if err == nil { - notes := map[string]string{ - "region_name": region.Name, - "region_id": region.Id, + provider, _ := self.GetProvider() + if provider != nil { + action := logclient.ACT_DISABLE + if enabled { + action = logclient.ACT_ENABLE + } + region, err := self.GetRegion() + if err == nil { + notes := map[string]string{ + "region_name": region.Name, + "region_id": region.Id, + } + logclient.AddSimpleActionLog(provider, action, notes, userCred, true) } - logclient.AddSimpleActionLog(provider, action, notes, userCred, true) } } } func (self *SCloudproviderregion) getSyncIntervalSeconds(account *SCloudaccount) int { - if account == nil { - account = self.GetAccount() + if account != nil { + return account.getSyncIntervalSeconds() + } + account, err := self.GetAccount() + if err != nil { + return options.Options.MinimalSyncIntervalSeconds } return account.getSyncIntervalSeconds() } @@ -324,7 +330,11 @@ func (self *SCloudproviderregion) markEndSync(ctx context.Context, userCred mccl if err != nil { return errors.Wrapf(err, "markEndSyncInternal") } - err = self.GetProvider().markEndSyncWithLock(ctx, userCred) + provider, err := self.GetProvider() + if err != nil { + return errors.Wrapf(err, "GetProvider") + } + err = provider.markEndSyncWithLock(ctx, userCred) if err != nil { return errors.Wrapf(err, "markEndSyncWithLock") } @@ -417,7 +427,10 @@ func (self *SCloudproviderregion) DoSync(ctx context.Context, userCred mcclient. if err != nil { return errors.Wrapf(err, "GetRegion") } - provider := self.GetProvider() + provider, err := self.GetProvider() + if err != nil { + return errors.Wrapf(err, "GetProvider") + } self.markSyncing(userCred) @@ -505,7 +518,7 @@ func (cpr *SCloudproviderregion) needAutoSyncInternal() bool { if cpr.LastAutoSyncAt.IsZero() { return true } - account := cpr.GetAccount() + account, _ := cpr.GetAccount() intval := cpr.getSyncIntervalSeconds(account) if time.Now().Sub(cpr.LastSync) > time.Duration(intval)*time.Second { return true diff --git a/pkg/compute/models/cloudproviders.go b/pkg/compute/models/cloudproviders.go index 771e7c2c82..baf726fa78 100644 --- a/pkg/compute/models/cloudproviders.go +++ b/pkg/compute/models/cloudproviders.go @@ -374,13 +374,19 @@ func (self *SCloudprovider) getAccessUrl() string { if len(self.AccessUrl) > 0 { return self.AccessUrl } - account := self.GetCloudaccount() - return account.AccessUrl + account, _ := self.GetCloudaccount() + if account != nil { + return account.AccessUrl + } + return "" } func (self *SCloudprovider) getPassword() (string, error) { if len(self.Secret) == 0 { - account := self.GetCloudaccount() + account, err := self.GetCloudaccount() + if err != nil { + return "", errors.Wrapf(err, "GetCloudaccount") + } return account.getPassword() } return utils.DescryptAESBase64(self.Id, self.Secret) @@ -439,9 +445,9 @@ func (self *SCloudaccount) getOrCreateTenant(ctx context.Context, name, domainId } func (self *SCloudprovider) syncProject(ctx context.Context, userCred mcclient.TokenCredential) error { - account := self.GetCloudaccount() - if account == nil { - return errors.Error("no valid cloudaccount???") + account, err := self.GetCloudaccount() + if err != nil { + return errors.Wrapf(err, "GetCloudaccount") } desc := fmt.Sprintf("auto create from cloud provider %s (%s)", self.Name, self.Id) @@ -628,7 +634,10 @@ func (self *SCloudprovider) PerformSync(ctx context.Context, userCred mcclient.T if !self.GetEnabled() { return nil, httperrors.NewInvalidStatusError("Cloudprovider disabled") } - account := self.GetCloudaccount() + account, err := self.GetCloudaccount() + if err != nil { + return nil, errors.Wrapf(err, "GetCloudaccount") + } if !account.GetEnabled() { return nil, httperrors.NewInvalidStatusError("Cloudaccount disabled") } @@ -655,7 +664,7 @@ func (self *SCloudprovider) StartSyncCloudProviderInfoTask(ctx context.Context, log.Errorf("startSyncCloudProviderInfoTask newTask error %s", err) return err } - if cloudaccount := self.GetCloudaccount(); cloudaccount != nil { + if cloudaccount, _ := self.GetCloudaccount(); cloudaccount != nil { cloudaccount.markAutoSync(userCred) cloudaccount.MarkSyncing(userCred) } @@ -677,7 +686,10 @@ func (self *SCloudprovider) PerformChangeProject(ctx context.Context, userCred m return nil, nil } - account := self.GetCloudaccount() + account, err := self.GetCloudaccount() + if err != nil { + return nil, err + } if self.DomainId != tenant.DomainId { if !db.IsAdminAllowPerform(ctx, userCred, self, "change-project") { return nil, httperrors.NewForbiddenError("not allow to change project across domain") @@ -808,7 +820,10 @@ func (self *SCloudprovider) markEndSyncWithLock(ctx context.Context, userCred mc return err } - account := self.GetCloudaccount() + account, err := self.GetCloudaccount() + if err != nil { + return errors.Wrapf(err, "GetCloudaccount") + } return account.MarkEndSyncWithLock(ctx, userCred) } @@ -859,7 +874,10 @@ func (self *SCloudprovider) GetProvider(ctx context.Context) (cloudprovider.IClo return nil, err } - account := self.GetCloudaccount() + account, err := self.GetCloudaccount() + if err != nil { + return nil, errors.Wrapf(err, "GetCloudaccount") + } defaultRegion, _ := jsonutils.Marshal(account.Options).GetString("default_region") return cloudprovider.GetProvider(cloudprovider.ProviderConfig{ Id: self.Id, @@ -892,8 +910,12 @@ func (self *SCloudprovider) savePassword(secret string) error { return err } -func (self *SCloudprovider) GetCloudaccount() *SCloudaccount { - return CloudaccountManager.FetchCloudaccountById(self.CloudaccountId) +func (self *SCloudprovider) GetCloudaccount() (*SCloudaccount, error) { + obj, err := CloudaccountManager.FetchById(self.CloudaccountId) + if err != nil { + return nil, errors.Wrapf(err, "FetchById(%s)", self.CloudaccountId) + } + return obj.(*SCloudaccount), nil } func (manager *SCloudproviderManager) FetchCloudproviderById(providerId string) *SCloudprovider { @@ -919,7 +941,7 @@ func (manager *SCloudproviderManager) IsProviderAccountEnabled(providerId string if !providerObj.GetEnabled() { return false } - account := providerObj.GetCloudaccount() + account, _ := providerObj.GetCloudaccount() if account == nil { return false } @@ -1498,11 +1520,12 @@ func (self *SCloudprovider) PerformEnable(ctx context.Context, userCred mcclient if err != nil { return nil, err } - account := self.GetCloudaccount() - if account != nil { - if !account.GetEnabled() { - return account.enableAccountOnly(ctx, userCred, nil, input) - } + account, err := self.GetCloudaccount() + if err != nil { + return nil, err + } + if !account.GetEnabled() { + return account.enableAccountOnly(ctx, userCred, nil, input) } return nil, nil } @@ -1512,20 +1535,21 @@ func (self *SCloudprovider) PerformDisable(ctx context.Context, userCred mcclien if err != nil { return nil, err } - account := self.GetCloudaccount() - if account != nil { - allDisable := true - providers := account.GetCloudproviders() - for i := range providers { - if providers[i].GetEnabled() { - allDisable = false - break - } - } - if allDisable && account.GetEnabled() { - return account.PerformDisable(ctx, userCred, nil, input) + account, err := self.GetCloudaccount() + if err != nil { + return nil, err + } + allDisable := true + providers := account.GetCloudproviders() + for i := range providers { + if providers[i].GetEnabled() { + allDisable = false + break } } + if allDisable && account.GetEnabled() { + return account.PerformDisable(ctx, userCred, nil, input) + } return nil, nil } @@ -1624,13 +1648,12 @@ func (provider *SCloudprovider) GetDetailsClirc(ctx context.Context, userCred mc return nil, err } - account := provider.GetCloudaccount() - var options *jsonutils.JSONDict - if account != nil { - options = account.Options + account, err := provider.GetCloudaccount() + if err != nil { + return nil, err } - rc, err := cloudprovider.GetClientRC(provider.Name, accessUrl, provider.Account, passwd, provider.Provider, options) + rc, err := cloudprovider.GetClientRC(provider.Name, accessUrl, provider.Account, passwd, provider.Provider, account.Options) if err != nil { return nil, err } @@ -1689,12 +1712,15 @@ func (provider *SCloudprovider) GetDetailsCannedAcls( } func (provider *SCloudprovider) getAccountShareInfo() apis.SAccountShareInfo { - account := provider.GetCloudaccount() - return account.getAccountShareInfo() + account, _ := provider.GetCloudaccount() + if account != nil { + return account.getAccountShareInfo() + } + return apis.SAccountShareInfo{} } func (provider *SCloudprovider) IsSharable(reqUsrId mcclient.IIdentityProvider) bool { - account := provider.GetCloudaccount() + account, _ := provider.GetCloudaccount() if account != nil { if account.ShareMode == api.CLOUD_ACCOUNT_SHARE_MODE_SYSTEM { return account.IsSharable(reqUsrId) @@ -1708,7 +1734,10 @@ func (provider *SCloudprovider) GetDetailsChangeOwnerCandidateDomains(ctx contex } func (provider *SCloudprovider) GetChangeOwnerCandidateDomainIds() []string { - account := provider.GetCloudaccount() + account, _ := provider.GetCloudaccount() + if account == nil { + return []string{} + } if account.ShareMode == api.CLOUD_ACCOUNT_SHARE_MODE_ACCOUNT_DOMAIN { return []string{account.DomainId} } @@ -1721,9 +1750,9 @@ func (provider *SCloudprovider) GetChangeOwnerCandidateDomainIds() []string { } func (self *SCloudprovider) SyncProject(ctx context.Context, userCred mcclient.TokenCredential, id string) (string, error) { - account := self.GetCloudaccount() - if account == nil { - return "", fmt.Errorf("failed to get cloudprovider %s account", self.Name) + account, err := self.GetCloudaccount() + if err != nil { + return "", errors.Wrapf(err, "GetCloudaccount") } return account.SyncProject(ctx, userCred, id) } diff --git a/pkg/compute/models/cloudsync.go b/pkg/compute/models/cloudsync.go index e0af5db09a..2e3243792f 100644 --- a/pkg/compute/models/cloudsync.go +++ b/pkg/compute/models/cloudsync.go @@ -2079,9 +2079,9 @@ func SyncCloudProject(userCred mcclient.TokenCredential, model db.IVirtualModel, } return nil, errors.Wrapf(err, "GetProjectMapping") } - account := manager.GetCloudaccount() - if account == nil { - return nil, fmt.Errorf("can not find manager %s account", manager.Name) + account, err := manager.GetCloudaccount() + if err != nil { + return nil, errors.Wrapf(err, "GetCloudaccount") } if rm != nil && rm.Enabled.Bool() { extTags, err := extModel.GetTags() diff --git a/pkg/compute/models/disks.go b/pkg/compute/models/disks.go index caf0332e50..f0a6da5965 100644 --- a/pkg/compute/models/disks.go +++ b/pkg/compute/models/disks.go @@ -1138,7 +1138,7 @@ func (self *SDisk) ValidateDeleteCondition(ctx context.Context, info jsonutils.J return httperrors.NewNotSufficientPrivilegeError("cloud provider %s is not available", provider.GetName()) } - account := provider.GetCloudaccount() + account, _ := provider.GetCloudaccount() if account != nil && !account.IsAvailable() { return httperrors.NewNotSufficientPrivilegeError("cloud account %s is not available", account.GetName()) } @@ -1183,7 +1183,7 @@ func (self *SDisk) AllowDeleteItem(ctx context.Context, userCred mcclient.TokenC return false } - account := provider.GetCloudaccount() + account, _ := provider.GetCloudaccount() if account != nil && !account.IsAvailable() { return false } diff --git a/pkg/compute/models/managedresource.go b/pkg/compute/models/managedresource.go index b3c5d365f9..69477001bf 100644 --- a/pkg/compute/models/managedresource.go +++ b/pkg/compute/models/managedresource.go @@ -82,7 +82,8 @@ func (self *SManagedResourceBase) GetCloudaccount() *SCloudaccount { if cp == nil { return nil } - return cp.GetCloudaccount() + account, _ := cp.GetCloudaccount() + return account } func (self *SManagedResourceBase) GetRegionDriver() (IRegionDriver, error) { @@ -145,7 +146,7 @@ func (self *SManagedResourceBase) CanShareToDomain(domainId string) bool { if provider == nil { return true } - account := provider.GetCloudaccount() + account, _ := provider.GetCloudaccount() if account == nil { // no cloud account, can share to any domain return true @@ -765,7 +766,7 @@ func MakeCloudProviderInfo(region *SCloudregion, zone *SZone, provider *SCloudpr } } - account := provider.GetCloudaccount() + account, _ := provider.GetCloudaccount() if account != nil { info.Account = account.GetName() info.AccountId = account.GetId() diff --git a/pkg/compute/models/quotas.go b/pkg/compute/models/quotas.go index b65ff26d1a..4900733d4e 100644 --- a/pkg/compute/models/quotas.go +++ b/pkg/compute/models/quotas.go @@ -453,12 +453,14 @@ func fetchCloudQuotaKeys(scope rbacutils.TRbacScope, ownerId mcclient.IIdentityP keys := quotas.SCloudResourceKeys{} keys.SBaseProjectQuotaKeys = quotas.OwnerIdProjectQuotaKeys(scope, ownerId) if manager != nil { - account := manager.GetCloudaccount() - keys.Provider = account.Provider - keys.Brand = account.Brand - keys.CloudEnv = account.GetCloudEnv() - keys.AccountId = account.Id keys.ManagerId = manager.Id + account, _ := manager.GetCloudaccount() + if account != nil { + keys.Provider = account.Provider + keys.Brand = account.Brand + keys.CloudEnv = account.GetCloudEnv() + keys.AccountId = account.Id + } } else { keys.Provider = api.CLOUD_PROVIDER_ONECLOUD keys.Brand = api.ONECLOUD_BRAND_ONECLOUD diff --git a/pkg/scheduler/cache/candidate/base.go b/pkg/scheduler/cache/candidate/base.go index f10fbd1a1a..eb8a803a38 100644 --- a/pkg/scheduler/cache/candidate/base.go +++ b/pkg/scheduler/cache/candidate/base.go @@ -493,7 +493,11 @@ func (h *BaseHostDesc) fillIsolatedDevices(b *baseBuilder, host *computemodels.S func (b *BaseHostDesc) fillCloudProvider(host *computemodels.SHost) error { b.Cloudprovider = host.GetCloudprovider() if b.Cloudprovider != nil { - b.Cloudaccount = b.Cloudprovider.GetCloudaccount() + var err error + b.Cloudaccount, err = b.Cloudprovider.GetCloudaccount() + if err != nil { + return err + } } return nil }