Merge pull request #9593 from ioito/hotfix/qx-private-cloud-sku

fix(region): private cloud sku create

pkg/cloudprovider/resources.go

@@ -114,7 +114,7 @@ type ICloudRegion interface {
 	CreateILoadBalancerCertificate(cert *SLoadbalancerCertificate) (ICloudLoadbalancerCertificate, error)
 
 	GetISkus() ([]ICloudSku, error)
-	CreateISku(name string, vCpu int, memoryMb int) error
+	CreateISku(opts *SServerSkuCreateOption) (ICloudSku, error)
 
 	GetINetworkInterfaces() ([]ICloudNetworkInterface, error)
 

pkg/cloudprovider/serversku.go

@@ -19,3 +19,11 @@ type SServerSku struct {
 	CpuCoreCount int
 	MemorySizeMB int
 }
+
+type SServerSkuCreateOption struct {
+	Name             string
+	CpuCount         int
+	VmemSizeMb       int
+	SysDiskMinSizeGb int
+	SysDiskMaxSizeGb int
+}

pkg/compute/models/cloudregions.go

@@ -323,7 +323,7 @@ func (manager *SCloudregionManager) FetchCustomizeColumns(
 	return rows
 }
 
-func (self *SCloudregion) GetSkus() ([]SServerSku, error) {
+func (self *SCloudregion) GetServerSkus() ([]SServerSku, error) {
 	skus := []SServerSku{}
 	q := ServerSkuManager.Query().Equals("cloudregion_id", self.Id)
 	err := db.FetchModelObjects(ServerSkuManager, q, &skus)
@@ -1089,3 +1089,14 @@ func (manager *SCloudregionManager) AllowGetPropertySyncTasks(ctx context.Contex
 func (manager *SCloudregionManager) GetPropertySyncTasks(ctx context.Context, userCred mcclient.TokenCredential, query api.SkuTaskQueryInput) (jsonutils.JSONObject, error) {
 	return GetPropertySkusSyncTasks(ctx, userCred, query)
 }
+
+func (self *SCloudregion) GetCloudprovider() (*SCloudprovider, error) {
+	if len(self.ManagerId) == 0 {
+		return nil, sql.ErrNoRows
+	}
+	provider, err := CloudproviderManager.FetchById(self.ManagerId)
+	if err != nil {
+		return nil, errors.Wrapf(err, "FetchByI(%s)", self.ManagerId)
+	}
+	return provider.(*SCloudprovider), nil
+}

pkg/compute/models/cloudsync.go

@@ -792,20 +792,20 @@ func syncVMSecgroups(ctx context.Context, userCred mcclient.TokenCredential, pro
 	return localVM.SyncVMSecgroups(ctx, userCred, secgroupIds)
 }
 
-func syncSkusFromPrivateCloud(ctx context.Context, userCred mcclient.TokenCredential, syncResults SSyncResultSet, provider *SCloudprovider, remoteRegion cloudprovider.ICloudRegion) {
+func syncSkusFromPrivateCloud(ctx context.Context, userCred mcclient.TokenCredential, syncResults SSyncResultSet, region *SCloudregion, remoteRegion cloudprovider.ICloudRegion) {
 	skus, err := remoteRegion.GetISkus()
 	if err != nil {
-		msg := fmt.Sprintf("GetISkus for provider %s(%s) failed %v", provider.Name, provider.Id, err)
+		msg := fmt.Sprintf("GetISkus for region %s(%s) failed %v", region.Name, region.Id, err)
 		log.Errorf(msg)
 		return
 	}
 
-	result := ServerSkuManager.SyncPrivateCloudSkus(ctx, userCred, provider, skus)
+	result := ServerSkuManager.SyncPrivateCloudSkus(ctx, userCred, region, skus)
 
 	syncResults.Add(ServerSkuManager, result)
 
 	msg := result.Result()
-	log.Infof("SyncCloudSkusByRegion for provider %s result: %s", provider.Name, msg)
+	log.Infof("SyncCloudSkusByRegion for region %s result: %s", region.Name, msg)
 	if result.IsError() {
 		return
 	}
@@ -1143,7 +1143,7 @@ func syncPublicCloudProviderInfo(
 		syncRegionSkus(ctx, userCred, localRegion)
 		SyncRegionDBInstanceSkus(ctx, userCred, localRegion.Id, true)
 	} else {
-		syncSkusFromPrivateCloud(ctx, userCred, syncResults, provider, remoteRegion)
+		syncSkusFromPrivateCloud(ctx, userCred, syncResults, localRegion, remoteRegion)
 	}
 
 	// no need to lock public cloud region as cloud region for public cloud is readonly

pkg/compute/models/skus.go

@@ -116,6 +116,19 @@ type SServerSku struct {
 	Provider string `width:"64" charset:"ascii" nullable:"true" list:"user" default:"OneCloud" create:"admin_optional"`
 }
 
+func (manager *SServerSkuManager) FetchUniqValues(ctx context.Context, data jsonutils.JSONObject) jsonutils.JSONObject {
+	regionId, _ := data.GetString("cloudregion_id")
+	return jsonutils.Marshal(map[string]string{"cloudregion_id": regionId})
+}
+
+func (manager *SServerSkuManager) FilterByUniqValues(q *sqlchemy.SQuery, values jsonutils.JSONObject) *sqlchemy.SQuery {
+	regionId, _ := values.GetString("cloudregion_id")
+	if len(regionId) > 0 {
+		q = q.Equals("cloudregion_id", regionId)
+	}
+	return q
+}
+
 type SInstanceSpecQueryParams struct {
 	Provider       string
 	PublicCloud    bool
@@ -277,12 +290,13 @@ func (manager *SServerSkuManager) AllowCreateItem(ctx context.Context, userCred
 }
 
 func (self *SServerSkuManager) ValidateCreateData(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, input api.ServerSkuCreateInput) (api.ServerSkuCreateInput, error) {
-	var err error
+	var region *SCloudregion
 	if len(input.CloudregionId) > 0 {
-		_, err = validators.ValidateModel(userCred, CloudregionManager, &input.CloudregionId)
+		_region, err := validators.ValidateModel(userCred, CloudregionManager, &input.CloudregionId)
 		if err != nil {
 			return input, err
 		}
+		region = _region.(*SCloudregion)
 	}
 
 	if len(input.ZoneId) > 0 {
@@ -297,6 +311,7 @@ func (self *SServerSkuManager) ValidateCreateData(ctx context.Context, userCred
 		if input.CloudregionId != zone.CloudregionId {
 			return input, httperrors.NewConflictError("zone %s not in cloudregion %s", zone.Name, input.CloudregionId)
 		}
+		region = zone.GetRegion()
 	}
 
 	if input.CpuCoreCount < 1 || input.CpuCoreCount > 256 {
@@ -322,10 +337,20 @@ func (self *SServerSkuManager) ValidateCreateData(ctx context.Context, userCred
 
 	input.Provider = api.CLOUD_PROVIDER_ONECLOUD
 	input.Status = api.SkuStatusReady
+	if region != nil {
+		input.Provider = region.Provider
+	}
+	if input.Provider == api.CLOUD_PROVIDER_ONECLOUD {
+	} else if utils.IsInStringArray(input.Provider, api.PRIVATE_CLOUD_PROVIDERS) {
+		input.Status = api.SkuStatusCreating
+	} else {
+		return input, httperrors.NewUnsupportOperationError("Not support create public cloud sku")
+	}
 
 	input.LocalCategory = input.InstanceTypeCategory
 	input.InstanceTypeFamily = api.InstanceFamilies[input.InstanceTypeCategory]
 
+	var err error
 	if len(input.Name) == 0 {
 		// 格式 ecs.g1.c1m1
 		input.Name, err = genInstanceType(input.InstanceTypeFamily, input.CpuCoreCount, input.MemorySizeMB)
@@ -333,6 +358,9 @@ func (self *SServerSkuManager) ValidateCreateData(ctx context.Context, userCred
 			return input, httperrors.NewInputParameterError("%v", err)
 		}
 		q := self.Query().Equals("name", input.Name)
+		if len(input.CloudregionId) > 0 {
+			q = q.Equals("cloudregion_id", input.CloudregionId)
+		}
 		count, err := q.CountWithError()
 		if err != nil {
 			return input, httperrors.NewInternalServerError("checkout server sku name duplicate error: %v", err)
@@ -352,17 +380,19 @@ func (self *SServerSkuManager) ValidateCreateData(ctx context.Context, userCred
 func (self *SServerSku) PostCreate(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, data jsonutils.JSONObject) {
 	self.SEnabledStatusStandaloneResourceBase.PostCreate(ctx, userCred, ownerId, query, data)
 	if self.Provider != api.CLOUD_PROVIDER_ONECLOUD {
-		_, err := db.Update(self, func() error {
-			self.CloudregionId = ""
-			self.ZoneId = ""
-			return nil
-		})
-		if err != nil {
-			log.Warningf("failed to update %s sku %s cloudregion and zone info: %v", self.Provider, self.Name, err)
-		}
+		self.StartSkuCreateTask(ctx, userCred)
 	}
 }
 
+func (self *SServerSku) StartSkuCreateTask(ctx context.Context, userCred mcclient.TokenCredential) error {
+	task, err := taskman.TaskManager.NewTask(ctx, "ServerSkuCreateTask", self, userCred, nil, "", "", nil)
+	if err != nil {
+		return errors.Wrapf(err, "NewTask")
+	}
+	task.ScheduleRun(nil)
+	return nil
+}
+
 func (self *SServerSku) GetPrivateCloudproviders() ([]SCloudprovider, error) {
 	providers := []SCloudprovider{}
 	q := CloudproviderManager.Query().In("provider", CloudproviderManager.GetPrivateProviderProvidersQuery())
@@ -1000,67 +1030,65 @@ func (manager *SServerSkuManager) PendingDeleteInvalidSku() error {
 	return nil
 }
 
-func (self *SServerSku) AllowPerformCacheSku(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, data jsonutils.JSONObject) bool {
-	return db.IsAdminAllowPerform(userCred, self, "cache-sku")
-}
-
-func (self *SServerSku) PerformCacheSku(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, data jsonutils.JSONObject) (jsonutils.JSONObject, error) {
-	cloudregionV := validators.NewModelIdOrNameValidator("cloudregion", "cloudregion", nil)
-	err := cloudregionV.Validate(data.(*jsonutils.JSONDict))
-	if err != nil {
-		return nil, err
-	}
-	cloudregion := cloudregionV.Model.(*SCloudregion)
-	if len(cloudregion.ManagerId) == 0 {
-		return nil, httperrors.NewInputParameterError("Only support cache sku for private cloud")
-	}
-
-	cloudprovider := cloudregion.GetCloudprovider()
-	if cloudprovider == nil {
-		return nil, httperrors.NewInputParameterError("failed to get cloudprovider for region %s(%s)", cloudregion.Name, cloudregion.Id)
-	}
-
-	if !cloudprovider.GetEnabled() {
-		return nil, httperrors.NewInputParameterError("cloudprovider %s(%s) disabled", cloudprovider.Name, cloudprovider.Id)
-	}
-
-	return nil, self.StartServerSkuCacheTask(ctx, userCred, cloudregion.Id, "")
-}
-
-func (self *SServerSku) StartServerSkuCacheTask(ctx context.Context, userCred mcclient.TokenCredential, cloudregionId string, parentTaskId string) error {
-	params := jsonutils.NewDict()
-	params.Add(jsonutils.NewString(cloudregionId), "cloudregion_id")
-	task, err := taskman.TaskManager.NewTask(ctx, "ServerSkuCacheTask", self, userCred, params, parentTaskId, "", nil)
-	if err != nil {
-		return err
-	}
-	task.ScheduleRun(nil)
-	return nil
-}
-
-func (manager *SServerSkuManager) SyncPrivateCloudSkus(ctx context.Context, userCred mcclient.TokenCredential, provider *SCloudprovider, skus []cloudprovider.ICloudSku) compare.SyncResult {
+func (manager *SServerSkuManager) SyncPrivateCloudSkus(ctx context.Context, userCred mcclient.TokenCredential, region *SCloudregion, skus []cloudprovider.ICloudSku) compare.SyncResult {
 	lockman.LockClass(ctx, manager, db.GetLockClassKey(manager, userCred))
 	defer lockman.ReleaseClass(ctx, manager, db.GetLockClassKey(manager, userCred))
 
-	syncResult := compare.SyncResult{}
-	localskus, err := manager.GetOneCloudSkus()
+	result := compare.SyncResult{}
+
+	dbSkus, err := region.GetServerSkus()
 	if err != nil {
-		syncResult.Error(err)
-		return syncResult
+		result.Error(errors.Wrapf(err, "GetServerSkus"))
+		return result
 	}
 
-	for _, sku := range skus {
-		if !utils.IsInStringArray(fmt.Sprintf("%d/%d", sku.GetCpuCoreCount(), sku.GetMemorySizeMB()), localskus) {
-			err := manager.newFromCloudSku(ctx, userCred, sku)
-			if err != nil {
-				syncResult.AddError(err)
-			} else {
-				syncResult.Add()
-			}
+	removed := make([]SServerSku, 0)
+	commondb := make([]SServerSku, 0)
+	commonext := make([]cloudprovider.ICloudSku, 0)
+	added := make([]cloudprovider.ICloudSku, 0)
+
+	err = compare.CompareSets(dbSkus, skus, &removed, &commondb, &commonext, &added)
+	if err != nil {
+		result.Error(errors.Wrapf(err, "CompareSets"))
+		return result
+	}
+
+	for i := 0; i < len(removed); i += 1 {
+		err = removed[i].RealDelete(ctx, userCred)
+		if err != nil {
+			result.DeleteError(err)
+			continue
 		}
+		result.Delete()
 	}
 
-	return syncResult
+	for i := 0; i < len(commondb); i += 1 {
+		err = commondb[i].SyncWithPrivateCloudSku(ctx, userCred, commonext[i])
+		if err != nil {
+			result.UpdateError(err)
+		}
+		result.Update()
+	}
+
+	for i := 0; i < len(added); i += 1 {
+		err := manager.newFromCloudSku(ctx, userCred, region, added[i])
+		if err != nil {
+			result.AddError(err)
+			continue
+		}
+		result.Add()
+	}
+
+	return result
+}
+
+func (self *SServerSku) SyncWithPrivateCloudSku(ctx context.Context, userCred mcclient.TokenCredential, sku cloudprovider.ICloudSku) error {
+	_, err := db.Update(self, func() error {
+		self.Status = api.SkuStatusAvailable
+		self.constructSku(sku)
+		return nil
+	})
+	return err
 }
 
 func (self *SServerSku) constructSku(extSku cloudprovider.ICloudSku) {
@@ -1112,23 +1140,17 @@ func (self *SServerSku) setPrepaidPostpaidStatus(userCred mcclient.TokenCredenti
 	return nil
 }
 
-func (manager *SServerSkuManager) newFromCloudSku(ctx context.Context, userCred mcclient.TokenCredential, extSku cloudprovider.ICloudSku) error {
-	sku := &SServerSku{Provider: api.CLOUD_PROVIDER_ONECLOUD}
+func (manager *SServerSkuManager) newFromCloudSku(ctx context.Context, userCred mcclient.TokenCredential, region *SCloudregion, extSku cloudprovider.ICloudSku) error {
+	sku := &SServerSku{Provider: region.Provider}
 	sku.SetModelManager(manager, sku)
 	// 第一次同步新建的套餐是启用状态
 	sku.Enabled = tristate.True
 	sku.Status = api.SkuStatusReady
 	sku.constructSku(extSku)
 
-	var err error
-	sku.Name, err = db.GenerateName(manager, userCred, extSku.GetName())
-	if err != nil {
-		return errors.Wrap(err, "db.GenerateName")
-	}
-
-	sku.CloudregionId = api.DEFAULT_REGION_ID
+	sku.CloudregionId = region.Id
 	sku.SetModelManager(manager, sku)
-	err = manager.TableSpec().Insert(ctx, sku)
+	err := manager.TableSpec().Insert(ctx, sku)
 	if err != nil {
 		return errors.Wrapf(err, "newFromCloudSku.Insert")
 	}
@@ -1303,31 +1325,6 @@ func (manager *SServerSkuManager) FetchAllAvailableSkuIdByZoneId(zoneId string)
 	return ids, nil
 }
 
-func (manager *SServerSkuManager) initializeSkuProvider() error {
-	skus := []SServerSku{}
-	q := manager.Query()
-	q = q.Filter(sqlchemy.OR(
-		sqlchemy.In(q.Field("provider"), CloudproviderManager.GetPrivateProviderProvidersQuery()),
-		sqlchemy.IsNullOrEmpty(q.Field("provider")),
-	))
-	err := db.FetchModelObjects(manager, q, &skus)
-	if err != nil {
-		return errors.Wrapf(err, "initializeSkuProvider.FetchModelObjects")
-	}
-	for _, sku := range skus {
-		_, err = db.Update(&sku, func() error {
-			sku.Provider = api.CLOUD_PROVIDER_ONECLOUD
-			sku.CloudregionId = api.DEFAULT_REGION_ID
-			sku.ZoneId = ""
-			return nil
-		})
-		if err != nil {
-			return errors.Wrapf(err, "sku.Update")
-		}
-	}
-	return nil
-}
-
 func (manager *SServerSkuManager) initializeSkuStatus() error {
 	skus := []SServerSku{}
 	q := manager.Query().NotEquals("status", api.SkuStatusReady)
@@ -1347,101 +1344,44 @@ func (manager *SServerSkuManager) initializeSkuStatus() error {
 	return nil
 }
 
-func (manager *SServerSkuManager) initializeSkuEnableField() error {
-	skus := []SServerSku{}
-	q := manager.Query().IsNull("enabled")
-	err := db.FetchModelObjects(manager, q, &skus)
-	if err != nil {
-		return errors.Wrapf(err, "FetchModelObjects")
-	}
-	for _, sku := range skus {
-		_, err = db.Update(&sku, func() error {
-			sku.Status = api.SkuStatusReady
-			sku.Enabled = tristate.True
-			return nil
-		})
-		if err != nil {
-			return errors.Wrapf(err, "sku.Update")
-		}
-	}
-	return nil
-}
-
 func (manager *SServerSkuManager) InitializeData() error {
 	count, err := manager.Query().Equals("cloudregion_id", api.DEFAULT_REGION_ID).IsNullOrEmpty("zone_id").CountWithError()
-	if err == nil {
-		if count == 0 {
-			type Item struct {
-				CPU   int
-				MemMB int
-			}
+	if err != nil {
+		return errors.Wrapf(err, "fetch default region skus")
+	}
+	if count == 0 {
+		skus := []struct {
+			cpu   int
+			memGb int
+		}{
+			{1, 1}, {1, 2}, {1, 4}, {1, 8},
+			{2, 2}, {2, 4}, {2, 8}, {2, 12}, {2, 16},
+			{4, 4}, {4, 12}, {4, 16}, {4, 24}, {4, 32},
+			{8, 8}, {8, 16}, {8, 24}, {8, 32}, {8, 64},
+			{12, 12}, {12, 16}, {12, 24}, {12, 32}, {12, 64},
+			{16, 16}, {16, 24}, {16, 32}, {16, 48}, {16, 64},
+			{24, 24}, {24, 32}, {24, 48}, {24, 64}, {24, 128},
+			{32, 32}, {32, 48}, {32, 64}, {32, 128},
+		}
 
-			items := []Item{
-				{1, 1 * 1024},
-				{1, 2 * 1024},
-				{1, 4 * 1024},
-				{1, 8 * 1024},
-				{2, 2 * 1024},
-				{2, 4 * 1024},
-				{2, 8 * 1024},
-				{2, 12 * 1024},
-				{2, 16 * 1024},
-				{4, 4 * 1024},
-				{4, 8 * 1024},
-				{4, 12 * 1024},
-				{4, 16 * 1024},
-				{4, 24 * 1024},
-				{4, 32 * 1024},
-				{8, 8 * 1024},
-				{8, 12 * 1024},
-				{8, 16 * 1024},
-				{8, 24 * 1024},
-				{8, 32 * 1024},
-				{8, 64 * 1024},
-				{12, 12 * 1024},
-				{12, 16 * 1024},
-				{12, 24 * 1024},
-				{12, 32 * 1024},
-				{12, 64 * 1024},
-				{16, 16 * 1024},
-				{16, 24 * 1024},
-				{16, 32 * 1024},
-				{16, 48 * 1024},
-				{16, 64 * 1024},
-				{24, 24 * 1024},
-				{24, 32 * 1024},
-				{24, 48 * 1024},
-				{24, 64 * 1024},
-				{24, 128 * 1024},
-				{32, 32 * 1024},
-				{32, 48 * 1024},
-				{32, 64 * 1024},
-				{32, 128 * 1024},
-			}
-
-			for i := range items {
-				item := items[i]
-				sku := &SServerSku{}
-				sku.CloudregionId = api.DEFAULT_REGION_ID
-				sku.CpuCoreCount = item.CPU
-				sku.MemorySizeMB = item.MemMB
-				sku.IsEmulated = false
-				sku.Enabled = tristate.True
-				sku.InstanceTypeCategory = api.SkuCategoryGeneralPurpose
-				sku.LocalCategory = api.SkuCategoryGeneralPurpose
-				sku.InstanceTypeFamily = api.InstanceFamilies[api.SkuCategoryGeneralPurpose]
-				name, _ := genInstanceType(sku.InstanceTypeFamily, int64(item.CPU), int64(item.MemMB))
-				sku.Name = name
-				sku.PrepaidStatus = api.SkuStatusAvailable
-				sku.PostpaidStatus = api.SkuStatusAvailable
-				err := manager.TableSpec().Insert(context.TODO(), sku)
-				if err != nil {
-					log.Errorf("ServerSkuManager Initialize local sku %s", err)
-				}
+		for _, item := range skus {
+			sku := &SServerSku{}
+			sku.CloudregionId = api.DEFAULT_REGION_ID
+			sku.CpuCoreCount = item.cpu
+			sku.MemorySizeMB = item.memGb * 1024
+			sku.IsEmulated = false
+			sku.Enabled = tristate.True
+			sku.InstanceTypeCategory = api.SkuCategoryGeneralPurpose
+			sku.LocalCategory = api.SkuCategoryGeneralPurpose
+			sku.InstanceTypeFamily = api.InstanceFamilies[api.SkuCategoryGeneralPurpose]
+			sku.Name, _ = genInstanceType(sku.InstanceTypeFamily, int64(item.cpu), int64(item.memGb*1024))
+			sku.PrepaidStatus = api.SkuStatusAvailable
+			sku.PostpaidStatus = api.SkuStatusAvailable
+			err := manager.TableSpec().Insert(context.TODO(), sku)
+			if err != nil {
+				log.Errorf("ServerSkuManager Initialize local sku %s", err)
 			}
 		}
-	} else {
-		log.Errorf("ServerSkuManager InitializeData %s", err)
 	}
 
 	privateSkus := make([]SServerSku, 0)
@@ -1460,16 +1400,6 @@ func (manager *SServerSkuManager) InitializeData() error {
 		}
 	}
 
-	err = manager.initializeSkuEnableField()
-	if err != nil {
-		return errors.Wrap(err, "initializeSkuEnableField")
-	}
-
-	err = manager.initializeSkuProvider()
-	if err != nil {
-		return errors.Wrap(err, "InitializeProvider")
-	}
-
 	return manager.initializeSkuStatus()
 }
 
@@ -1515,3 +1445,38 @@ func (manager *SServerSkuManager) AllowGetPropertySyncTasks(ctx context.Context,
 func (manager *SServerSkuManager) GetPropertySyncTasks(ctx context.Context, userCred mcclient.TokenCredential, query api.SkuTaskQueryInput) (jsonutils.JSONObject, error) {
 	return GetPropertySkusSyncTasks(ctx, userCred, query)
 }
+
+func (self *SServerSku) GetICloudSku() (cloudprovider.ICloudSku, error) {
+	region, err := self.GetRegion()
+	if err != nil {
+		if errors.Cause(err) == sql.ErrNoRows {
+			return nil, errors.Wrapf(cloudprovider.ErrNotFound, "GetRegion")
+		}
+		return nil, errors.Wrapf(err, "GetRegion")
+	}
+	provider, err := region.GetCloudprovider()
+	if err != nil {
+		if errors.Cause(err) == sql.ErrNoRows {
+			return nil, errors.Wrapf(cloudprovider.ErrNotFound, "GetCloudprovider")
+		}
+		return nil, errors.Wrapf(err, "GetCloudprovider")
+	}
+	driver, err := provider.GetProvider()
+	if err != nil {
+		return nil, errors.Wrapf(err, "GetDriver()")
+	}
+	iRegion, err := driver.GetIRegionById(region.ExternalId)
+	if err != nil {
+		return nil, errors.Wrapf(err, "GetIRegionById(%s)", region.ExternalId)
+	}
+	skus, err := iRegion.GetISkus()
+	if err != nil {
+		return nil, errors.Wrapf(err, "GetICloudSku")
+	}
+	for i := range skus {
+		if skus[i].GetGlobalId() == self.ExternalId {
+			return skus[i], nil
+		}
+	}
+	return nil, errors.Wrapf(cloudprovider.ErrNotFound, self.ExternalId)
+}

pkg/compute/tasks/server_sku_cache_task.go

@@ -1,99 +0,0 @@
-// Copyright 2019 Yunion
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package tasks
-
-import (
-	"context"
-	"fmt"
-
-	"yunion.io/x/jsonutils"
-	"yunion.io/x/pkg/errors"
-
-	"yunion.io/x/onecloud/pkg/cloudcommon/db"
-	"yunion.io/x/onecloud/pkg/cloudcommon/db/taskman"
-	"yunion.io/x/onecloud/pkg/compute/models"
-)
-
-type ServerSkuCacheTask struct {
-	taskman.STask
-}
-
-func init() {
-	taskman.RegisterTask(ServerSkuCacheTask{})
-}
-
-func (self *ServerSkuCacheTask) taskFailed(ctx context.Context, sku *models.SServerSku, err jsonutils.JSONObject) {
-	self.SetStageFailed(ctx, err)
-}
-
-func (self *ServerSkuCacheTask) getCloudregion() (*models.SCloudregion, error) {
-	cloudregionId, _ := self.GetParams().GetString("cloudregion_id")
-	if len(cloudregionId) == 0 {
-		return nil, fmt.Errorf("Missing cloudregion_id params")
-	}
-	region, err := models.CloudregionManager.FetchById(cloudregionId)
-	if err != nil {
-		return nil, errors.Wrap(err, "CloudregionManager.FetchById")
-	}
-	return region.(*models.SCloudregion), nil
-}
-
-func (self *ServerSkuCacheTask) OnInit(ctx context.Context, obj db.IStandaloneModel, data jsonutils.JSONObject) {
-	sku := obj.(*models.SServerSku)
-
-	cloudregion, err := self.getCloudregion()
-	if err != nil {
-		self.taskFailed(ctx, sku, jsonutils.NewString(errors.Wrap(err, "self.getCloudregion()").Error()))
-		return
-	}
-
-	cloudprovider := cloudregion.GetCloudprovider()
-	if cloudprovider == nil {
-		self.taskFailed(ctx, sku, jsonutils.NewString(fmt.Sprintf("failed to found cloudprovider for cloudregion %s(%s)", cloudregion.Name, cloudregion.Id)))
-		return
-	}
-
-	provider, err := cloudprovider.GetProvider()
-	if err != nil {
-		self.taskFailed(ctx, sku, jsonutils.NewString(errors.Wrap(err, "cloudprovider.GetProvider").Error()))
-		return
-	}
-	iRegion, err := provider.GetIRegionById(cloudregion.ExternalId)
-	if err != nil {
-		self.taskFailed(ctx, sku, jsonutils.NewString(errors.Wrap(err, "provider.GetIRegionById").Error()))
-		return
-	}
-
-	iskus, err := iRegion.GetISkus()
-	if err != nil {
-		self.taskFailed(ctx, sku, jsonutils.NewString(errors.Wrap(err, "provider.GetIRegionById").Error()))
-		return
-	}
-
-	for i := 0; i < len(iskus); i++ {
-		if iskus[i].GetName() == sku.Name && iskus[i].GetMemorySizeMB() == sku.MemorySizeMB && iskus[i].GetCpuCoreCount() == sku.CpuCoreCount {
-			self.SetStageComplete(ctx, nil)
-			return
-		}
-	}
-
-	err = iRegion.CreateISku(sku.Name, sku.CpuCoreCount, sku.MemorySizeMB)
-	if err != nil {
-		self.taskFailed(ctx, sku, jsonutils.NewString(errors.Wrap(err, "provider.GetIRegionById").Error()))
-		return
-	}
-	self.SetStageComplete(ctx, nil)
-	return
-}

pkg/compute/tasks/server_sku_create_task.go

@@ -0,0 +1,88 @@
+// Copyright 2019 Yunion
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package tasks
+
+import (
+	"context"
+
+	"yunion.io/x/jsonutils"
+	"yunion.io/x/pkg/errors"
+
+	api "yunion.io/x/onecloud/pkg/apis/compute"
+	"yunion.io/x/onecloud/pkg/cloudcommon/db"
+	"yunion.io/x/onecloud/pkg/cloudcommon/db/taskman"
+	"yunion.io/x/onecloud/pkg/cloudprovider"
+	"yunion.io/x/onecloud/pkg/compute/models"
+	"yunion.io/x/onecloud/pkg/util/logclient"
+)
+
+type ServerSkuCreateTask struct {
+	taskman.STask
+}
+
+func init() {
+	taskman.RegisterTask(ServerSkuCreateTask{})
+}
+
+func (self *ServerSkuCreateTask) taskFail(ctx context.Context, sku *models.SServerSku, err error) {
+	sku.SetStatus(self.UserCred, api.SkuStatusCreatFailed, err.Error())
+	db.OpsLog.LogEvent(sku, db.ACT_ALLOCATE, err, self.GetUserCred())
+	logclient.AddActionLogWithStartable(self, sku, logclient.ACT_ALLOCATE, err, self.UserCred, false)
+	self.SetStageFailed(ctx, jsonutils.NewString(err.Error()))
+}
+
+func (self *ServerSkuCreateTask) OnInit(ctx context.Context, obj db.IStandaloneModel, data jsonutils.JSONObject) {
+	sku := obj.(*models.SServerSku)
+	region, err := sku.GetRegion()
+	if err != nil {
+		self.taskFail(ctx, sku, errors.Wrapf(err, "GetRegion"))
+		return
+	}
+
+	provider, err := region.GetCloudprovider()
+	if err != nil {
+		self.taskFail(ctx, sku, errors.Wrapf(err, "GetCloudprovider"))
+		return
+	}
+
+	driver, err := provider.GetProvider()
+	if err != nil {
+		self.taskFail(ctx, sku, errors.Wrapf(err, "GetDriver"))
+		return
+	}
+
+	iRegion, err := driver.GetIRegionById(region.ExternalId)
+	if err != nil {
+		self.taskFail(ctx, sku, errors.Wrapf(err, "GetIRegionById"))
+		return
+	}
+
+	opts := cloudprovider.SServerSkuCreateOption{
+		Name:             sku.Name,
+		CpuCount:         sku.CpuCoreCount,
+		VmemSizeMb:       sku.MemorySizeMB,
+		SysDiskMinSizeGb: sku.SysDiskMinSizeGB,
+		SysDiskMaxSizeGb: sku.SysDiskMaxSizeGB,
+	}
+
+	iSku, err := iRegion.CreateISku(&opts)
+	if err != nil {
+		self.taskFail(ctx, sku, errors.Wrapf(err, "CreateISku"))
+		return
+	}
+
+	sku.SyncWithPrivateCloudSku(ctx, self.GetUserCred(), iSku)
+	self.SetStageComplete(ctx, nil)
+}

pkg/compute/tasks/server_sku_delete_task.go

@@ -20,11 +20,12 @@ import (
 	"github.com/pkg/errors"
 
 	"yunion.io/x/jsonutils"
-	"yunion.io/x/log"
+	"yunion.io/x/pkg/utils"
 
 	api "yunion.io/x/onecloud/pkg/apis/compute"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db/taskman"
+	"yunion.io/x/onecloud/pkg/cloudprovider"
 	"yunion.io/x/onecloud/pkg/compute/models"
 	"yunion.io/x/onecloud/pkg/util/logclient"
 )
@@ -37,55 +38,41 @@ func init() {
 	taskman.RegisterTask(ServerSkuDeleteTask{})
 }
 
-func (self *ServerSkuDeleteTask) taskFail(ctx context.Context, sku *models.SServerSku, msg jsonutils.JSONObject) {
-	sku.SetStatus(self.UserCred, api.SkuStatusDeleteFailed, msg.String())
-	db.OpsLog.LogEvent(sku, db.ACT_DELOCATE, msg, self.GetUserCred())
-	logclient.AddActionLogWithStartable(self, sku, logclient.ACT_DELETE, msg, self.UserCred, false)
-	self.SetStageFailed(ctx, msg)
+func (self *ServerSkuDeleteTask) taskFail(ctx context.Context, sku *models.SServerSku, err error) {
+	sku.SetStatus(self.UserCred, api.SkuStatusDeleteFailed, err.Error())
+	db.OpsLog.LogEvent(sku, db.ACT_DELOCATE, err, self.GetUserCred())
+	logclient.AddActionLogWithStartable(self, sku, logclient.ACT_DELETE, err, self.UserCred, false)
+	self.SetStageFailed(ctx, jsonutils.NewString(err.Error()))
+}
+
+func (self *ServerSkuDeleteTask) taskComplete(ctx context.Context, sku *models.SServerSku) {
+	err := sku.RealDelete(ctx, self.UserCred)
+	if err != nil {
+		self.taskFail(ctx, sku, errors.Wrapf(err, "RealDelete"))
+		return
+	}
+	logclient.AddActionLogWithStartable(self, sku, logclient.ACT_DELETE, nil, self.UserCred, true)
+	self.SetStageComplete(ctx, nil)
 }
 
 func (self *ServerSkuDeleteTask) OnInit(ctx context.Context, obj db.IStandaloneModel, data jsonutils.JSONObject) {
 	sku := obj.(*models.SServerSku)
-
-	if !jsonutils.QueryBoolean(self.Params, "purge", false) {
-		cloudproviders, err := sku.GetPrivateCloudproviders()
+	purge := jsonutils.QueryBoolean(self.GetParams(), "purge", false)
+	if !purge && utils.IsInStringArray(sku.Provider, api.PRIVATE_CLOUD_PROVIDERS) {
+		iSku, err := sku.GetICloudSku()
 		if err != nil {
-			self.taskFail(ctx, sku, jsonutils.NewString(err.Error()))
+			if errors.Cause(err) == cloudprovider.ErrNotFound {
+				self.taskComplete(ctx, sku)
+				return
+			}
+			self.taskFail(ctx, sku, errors.Wrapf(err, "GetICloudSku"))
 			return
 		}
-
-		for _, cloudprovider := range cloudproviders {
-			provider, err := cloudprovider.GetProvider()
-			if err != nil {
-				log.Warningf("failed to get provider for cloudprovider %s error: %v", cloudprovider.Name, err)
-				continue
-			}
-			regions := provider.GetIRegions()
-			for _, region := range regions {
-				iskus, err := region.GetISkus()
-				if err != nil {
-					log.Warningf("failed to get region %s skus", region.GetName())
-					continue
-				}
-				for _, isku := range iskus {
-					if isku.GetName() == sku.Name && isku.GetCpuCoreCount() == sku.CpuCoreCount && isku.GetMemorySizeMB() == sku.MemorySizeMB {
-						err = isku.Delete()
-						if err != nil {
-							log.Warningf("failed to delete sku %s %dC%dM", sku.Name, sku.CpuCoreCount, sku.MemorySizeMB)
-						}
-					}
-				}
-			}
+		err = iSku.Delete()
+		if err != nil {
+			self.taskFail(ctx, sku, errors.Wrapf(err, "iSku.Delete"))
+			return
 		}
 	}
-
-	err := sku.RealDelete(ctx, self.UserCred)
-	if err != nil {
-		err = errors.Wrapf(err, "sku.RealDelete")
-		self.taskFail(ctx, sku, jsonutils.NewString(err.Error()))
-		return
-	}
-
-	logclient.AddActionLogWithStartable(self, sku, logclient.ACT_DELETE, nil, self.UserCred, true)
-	self.SetStageComplete(ctx, nil)
+	self.taskComplete(ctx, sku)
 }

pkg/mcclient/options/serverskus.go

@@ -59,7 +59,7 @@ type ServerSkusCreateOptions struct {
 	InstanceTypeCategory *string `help:"instance type category" choices:"general_purpose|compute_optimized|memory_optimized|storage_optimized|hardware_accelerated|high_memory|high_storage"`
 
 	SysDiskResizable *bool   `help:"system disk is resizable"`
-	SysDiskType      *string `help:"system disk type" default:"local" choices:"local"`
+	SysDiskType      *string `help:"system disk type" choices:"local"`
 	SysDiskMaxSizeGB *int    `help:"system disk maximal size in gb"`
 
 	AttachedDiskType   *string `help:"attached data disk type"`
@@ -75,10 +75,10 @@ type ServerSkusCreateOptions struct {
 	GPUCount      *int    `help:"GPU count"`
 	GPUAttachable *bool   `help:"Allow attach GPU"`
 
-	Zone        *string `help:"Zone ID or name"`
-	Cloudregion *string `help:"Cloudregion ID or name"`
-	Provider    string  `help:"provider"`
-	Brand       string  `help:"brand"`
+	ZoneId        string `help:"Zone ID or name"`
+	CloudregionId string `help:"Cloudregion ID or name"`
+	Provider      string `help:"provider"`
+	Brand         string `help:"brand"`
 }
 
 func (opts *ServerSkusCreateOptions) Params() (jsonutils.JSONObject, error) {
@@ -118,12 +118,3 @@ type ServerSkusUpdateOptions struct {
 func (opts *ServerSkusUpdateOptions) Params() (jsonutils.JSONObject, error) {
 	return StructToParams(opts)
 }
-
-type ServerSkusCacheOptions struct {
-	ServerSkusIdOptions
-	REGION string `help:"Private cloud region"`
-}
-
-func (opts *ServerSkusCacheOptions) Params() (jsonutils.JSONObject, error) {
-	return jsonutils.Marshal(map[string]string{"cloudregion": opts.REGION}), nil
-}

pkg/multicloud/objectstore/objectstore.go

@@ -352,8 +352,8 @@ func (cli *SObjectStoreClient) GetISkus() ([]cloudprovider.ICloudSku, error) {
 	return nil, cloudprovider.ErrNotSupported
 }
 
-func (cli *SObjectStoreClient) CreateISku(name string, vCpu int, memoryMb int) error {
-	return cloudprovider.ErrNotSupported
+func (cli *SObjectStoreClient) CreateISku(opts *cloudprovider.SServerSkuCreateOption) (cloudprovider.ICloudSku, error) {
+	return nil, cloudprovider.ErrNotSupported
 }
 
 ////////////////////////////////// S3 API ///////////////////////////////////

pkg/multicloud/openstack/flavor.go

@@ -23,6 +23,7 @@ import (
 	"yunion.io/x/jsonutils"
 
 	api "yunion.io/x/onecloud/pkg/apis/compute"
+	"yunion.io/x/onecloud/pkg/cloudprovider"
 	"yunion.io/x/onecloud/pkg/multicloud"
 )
 
@@ -122,9 +123,12 @@ func (region *SRegion) syncFlavor(name string, cpu, memoryMb, diskGB int) (*SFla
 	return flavor, nil
 }
 
-func (region *SRegion) CreateISku(name string, vCpu int, memoryMb int) error {
-	_, err := region.CreateFlavor(name, vCpu, memoryMb, 30)
-	return err
+func (region *SRegion) CreateISku(opts *cloudprovider.SServerSkuCreateOption) (cloudprovider.ICloudSku, error) {
+	flavor, err := region.CreateFlavor(opts.Name, opts.CpuCount, opts.VmemSizeMb, opts.SysDiskMaxSizeGb)
+	if err != nil {
+		return nil, errors.Wrapf(err, "CreateFlavor")
+	}
+	return flavor, nil
 }
 
 func (region *SRegion) CreateFlavor(name string, cpu int, memoryMb int, diskGB int) (*SFlavor, error) {

pkg/multicloud/region_base.go

@@ -96,8 +96,8 @@ func (self *SRegion) GetISkus() ([]cloudprovider.ICloudSku, error) {
 	return nil, errors.Wrapf(cloudprovider.ErrNotSupported, "GetISkus")
 }
 
-func (self *SRegion) CreateISku(name string, vCpu int, memoryMb int) error {
-	return errors.Wrapf(cloudprovider.ErrNotImplemented, "CreateISku")
+func (self *SRegion) CreateISku(opts *cloudprovider.SServerSkuCreateOption) (cloudprovider.ICloudSku, error) {
+	return nil, errors.Wrapf(cloudprovider.ErrNotImplemented, "CreateISku")
 }
 
 func (self *SRegion) GetINetworkInterfaces() ([]cloudprovider.ICloudNetworkInterface, error) {

pkg/multicloud/zstack/offering.go

@@ -58,9 +58,12 @@ func (region *SRegion) GetInstanceOfferingByType(instanceType string) (*SInstanc
 	return nil, cloudprovider.ErrNotFound
 }
 
-func (region *SRegion) CreateISku(name string, vCpu int, memoryMb int) error {
-	_, err := region.CreateInstanceOffering(name, vCpu, memoryMb, "UserVm")
-	return err
+func (region *SRegion) CreateISku(opts *cloudprovider.SServerSkuCreateOption) (cloudprovider.ICloudSku, error) {
+	sku, err := region.CreateInstanceOffering(opts.Name, opts.CpuCount, opts.VmemSizeMb, "UserVm")
+	if err != nil {
+		return nil, errors.Wrapf(err, "CreateISku")
+	}
+	return sku, nil
 }
 
 func (region *SRegion) CreateInstanceOffering(name string, cpu int, memoryMb int, offeringType string) (*SInstanceOffering, error) {