Merge pull request #1969 from swordqiu/hotfix/qj-server-create-eip-quota-check

fix: server create eip quota check error

pkg/compute/models/elasticips.go

@@ -350,17 +350,17 @@ func (self *SElasticip) syncRemoveCloudEip(ctx context.Context, userCred mcclien
 }
 
 func (self *SElasticip) SyncInstanceWithCloudEip(ctx context.Context, userCred mcclient.TokenCredential, ext cloudprovider.ICloudEIP) error {
-	vm := self.GetAssociateVM()
+	resource := self.GetAssociateResource()
 	vmExtId := ext.GetAssociationExternalId()
 
-	if vm == nil && len(vmExtId) == 0 {
+	if resource == nil && len(vmExtId) == 0 {
 		return nil
 	}
-	if vm != nil && vm.ExternalId == vmExtId {
+	if resource != nil && resource.(db.IExternalizedModel).GetExternalId() == vmExtId {
 		return nil
 	}
 
-	if vm != nil { // dissociate
+	if resource != nil { // dissociate
 		err := self.Dissociate(ctx, userCred)
 		if err != nil {
 			log.Errorf("fail to dissociate vm: %s", err)
@@ -369,12 +369,33 @@ func (self *SElasticip) SyncInstanceWithCloudEip(ctx context.Context, userCred m
 	}
 
 	if len(vmExtId) > 0 {
-		newVM, err := db.FetchByExternalId(GuestManager, vmExtId)
+		var manager db.IModelManager
+		switch ext.GetAssociationType() {
+		case api.EIP_ASSOCIATE_TYPE_SERVER:
+			manager = GuestManager
+		case api.EIP_ASSOCIATE_TYPE_NAT_GATEWAY:
+			manager = NatGatewayManager
+		case api.EIP_ASSOCIATE_TYPE_LOADBALANCER:
+			manager = LoadbalancerManager
+		default:
+			return errors.Error("unsupported association type")
+		}
+
+		extRes, err := db.FetchByExternalId(manager, vmExtId)
 		if err != nil {
 			log.Errorf("fail to find vm by external ID %s", vmExtId)
 			return err
 		}
-		err = self.AssociateVM(ctx, userCred, newVM.(*SGuest))
+		switch newRes := extRes.(type) {
+		case *SGuest:
+			err = self.AssociateVM(ctx, userCred, newRes)
+		case *SLoadbalancer:
+			err = self.AssociateLoadbalancer(ctx, userCred, newRes)
+		case *SNatGateway:
+			err = self.AssociateNatGateway(ctx, userCred, newRes)
+		default:
+			return errors.Error("unsupported association type")
+		}
 		if err != nil {
 			log.Errorf("fail to associate with new vm %s", err)
 			return err
@@ -490,6 +511,9 @@ func (self *SElasticip) IsAssociated() bool {
 	if self.GetAssociateVM() != nil {
 		return true
 	}
+	if self.GetAssociateLoadbalancer() != nil {
+		return true
+	}
 	if self.GetAssociateNatGateway() != nil {
 		return true
 	}
@@ -529,6 +553,19 @@ func (self *SElasticip) GetAssociateNatGateway() *SNatGateway {
 	return nil
 }
 
+func (self *SElasticip) GetAssociateResource() db.IModel {
+	if vm := self.GetAssociateVM(); vm != nil {
+		return vm
+	}
+	if lb := self.GetAssociateLoadbalancer(); lb != nil {
+		return lb
+	}
+	if nat := self.GetAssociateNatGateway(); nat != nil {
+		return nat
+	}
+	return nil
+}
+
 func (self *SElasticip) Dissociate(ctx context.Context, userCred mcclient.TokenCredential) error {
 	if len(self.AssociateType) == 0 {
 		return nil
@@ -729,17 +766,19 @@ func (manager *SElasticipManager) ValidateCreateData(ctx context.Context, userCr
 
 func (self *SElasticip) PostCreate(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, data jsonutils.JSONObject) {
 	self.SVirtualResourceBase.PostCreate(ctx, userCred, ownerId, query, data)
-	params := jsonutils.NewDict()
-	if data.Contains("ip") {
-		ip, _ := data.GetString("ip")
-		params.Add(jsonutils.NewString(ip), "ip")
-	}
+
+	quotaPlatform := self.GetQuotaPlatformID()
 	eipPendingUsage := &SQuota{Eip: 1}
-	self.startEipAllocateTask(ctx, userCred, params, eipPendingUsage)
+	err := QuotaManager.CancelPendingUsage(ctx, userCred, rbacutils.ScopeProject, ownerId, quotaPlatform, eipPendingUsage, eipPendingUsage)
+	if err != nil {
+		log.Errorf("SElasticip CancelPendingUsage error: %s", err)
+	}
+
+	self.startEipAllocateTask(ctx, userCred, data.(*jsonutils.JSONDict))
 }
 
-func (self *SElasticip) startEipAllocateTask(ctx context.Context, userCred mcclient.TokenCredential, params *jsonutils.JSONDict, pendingUsage quotas.IQuota) error {
-	task, err := taskman.TaskManager.NewTask(ctx, "EipAllocateTask", self, userCred, params, "", "", pendingUsage)
+func (self *SElasticip) startEipAllocateTask(ctx context.Context, userCred mcclient.TokenCredential, params *jsonutils.JSONDict) error {
+	task, err := taskman.TaskManager.NewTask(ctx, "EipAllocateTask", self, userCred, params, "", "", nil)
 	if err != nil {
 		log.Errorf("newtask EipAllocateTask fail %s", err)
 		return err
@@ -764,7 +803,7 @@ func (self *SElasticip) CustomizeDelete(ctx context.Context, userCred mcclient.T
 
 func (self *SElasticip) ValidateDeleteCondition(ctx context.Context) error {
 	if self.IsAssociated() {
-		return fmt.Errorf("eip is associated with instance")
+		return fmt.Errorf("eip is associated with resources")
 	}
 	return self.SVirtualResourceBase.ValidateDeleteCondition(ctx)
 }
@@ -1008,9 +1047,7 @@ func (self *SElasticip) getMoreDetails(extra *jsonutils.JSONDict) *jsonutils.JSO
 	return extra
 }
 
-func (manager *SElasticipManager) AllocateEipAndAssociateVM(ctx context.Context, userCred mcclient.TokenCredential, vm *SGuest, bw int, chargeType string, eipPendingUsage quotas.IQuota) error {
-
-	host := vm.GetHost()
+func (manager *SElasticipManager) NewEipForVMOnHost(ctx context.Context, userCred mcclient.TokenCredential, vm *SGuest, host *SHost, bw int, chargeType string, pendingUsage quotas.IQuota) (*SElasticip, error) {
 	region := host.GetRegion()
 
 	if len(chargeType) == 0 {
@@ -1035,9 +1072,16 @@ func (manager *SElasticipManager) AllocateEipAndAssociateVM(ctx context.Context,
 	err := manager.TableSpec().Insert(&eip)
 	if err != nil {
 		log.Errorf("create EIP record fail %s", err)
-		return err
+		return nil, err
 	}
 
+	eipPendingUsage := &SQuota{Eip: 1}
+	QuotaManager.CancelPendingUsage(ctx, userCred, rbacutils.ScopeProject, vm.GetOwnerId(), vm.GetQuotaPlatformID(), pendingUsage, eipPendingUsage)
+
+	return &eip, nil
+}
+
+func (eip *SElasticip) AllocateAndAssociateVM(ctx context.Context, userCred mcclient.TokenCredential, vm *SGuest) error {
 	params := jsonutils.NewDict()
 	params.Add(jsonutils.NewString(vm.ExternalId), "instance_external_id")
 	params.Add(jsonutils.NewString(vm.Id), "instance_id")
@@ -1045,7 +1089,7 @@ func (manager *SElasticipManager) AllocateEipAndAssociateVM(ctx context.Context,
 
 	vm.SetStatus(userCred, api.VM_ASSOCIATE_EIP, "allocate and associate EIP")
 
-	return eip.startEipAllocateTask(ctx, userCred, params, eipPendingUsage)
+	return eip.startEipAllocateTask(ctx, userCred, params)
 }
 
 func (self *SElasticip) AllowPerformChangeBandwidth(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, data jsonutils.JSONObject) bool {

pkg/compute/models/guest_actions.go

@@ -2589,9 +2589,13 @@ func (self *SGuest) PerformCreateEip(ctx context.Context, userCred mcclient.Toke
 		return nil, httperrors.NewOutOfQuotaError("Out of eip quota: %s", err)
 	}
 
-	err = ElasticipManager.AllocateEipAndAssociateVM(ctx, userCred, self, int(bw), chargeType, eipPendingUsage)
+	eip, err := ElasticipManager.NewEipForVMOnHost(ctx, userCred, self, host, int(bw), chargeType, eipPendingUsage)
+	if err != nil {
+		return nil, httperrors.NewGeneralError(err)
+	}
+
+	err = eip.AllocateAndAssociateVM(ctx, userCred, self)
 	if err != nil {
-		QuotaManager.CancelPendingUsage(ctx, userCred, rbacutils.ScopeProject, userCred, quotaPlatform, eipPendingUsage, eipPendingUsage)
 		return nil, httperrors.NewGeneralError(err)
 	}
 

pkg/compute/tasks/eip_allocate_task.go

@@ -18,8 +18,6 @@ import (
 	"context"
 	"fmt"
 
-	"yunion.io/x/onecloud/pkg/cloudprovider"
-
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/log"
 
@@ -27,9 +25,9 @@ import (
 	"yunion.io/x/onecloud/pkg/cloudcommon/db"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db/lockman"
 	"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"
-	"yunion.io/x/onecloud/pkg/util/rbacutils"
 )
 
 type EipAllocateTask struct {
@@ -41,7 +39,6 @@ func init() {
 }
 
 func (self *EipAllocateTask) onFailed(ctx context.Context, eip *models.SElasticip, reason string) {
-	self.finalReleasePendingUsage(ctx, eip)
 	self.setGuestAllocateEipFailed(eip, reason)
 	self.SetStageFailed(ctx, reason)
 }
@@ -63,16 +60,6 @@ func (self *EipAllocateTask) setGuestAllocateEipFailed(eip *models.SElasticip, r
 	}
 }
 
-func (self *EipAllocateTask) finalReleasePendingUsage(ctx context.Context, eip *models.SElasticip) {
-	pendingUsage := models.SQuota{}
-	if err := self.GetPendingUsage(&pendingUsage); err == nil && !pendingUsage.IsEmpty() {
-		quotaPlatform := eip.GetQuotaPlatformID()
-		if err := models.QuotaManager.CancelPendingUsage(ctx, self.UserCred, rbacutils.ScopeProject, self.UserCred, quotaPlatform, nil, &pendingUsage); err != nil {
-			log.Errorf("CancelPendingUsage error: %v", err)
-		}
-	}
-}
-
 func (self *EipAllocateTask) OnInit(ctx context.Context, obj db.IStandaloneModel, data jsonutils.JSONObject) {
 	eip := obj.(*models.SElasticip)
 
@@ -145,8 +132,6 @@ func (self *EipAllocateTask) OnInit(ctx context.Context, obj db.IStandaloneModel
 		return
 	}
 
-	self.finalReleasePendingUsage(ctx, eip)
-
 	if self.Params != nil && self.Params.Contains("instance_id") {
 		self.SetStage("on_eip_associate_complete", nil)
 		err = eip.StartEipAssociateTask(ctx, self.UserCred, self.Params, "")

pkg/compute/tasks/eip_dissociate_task.go

@@ -36,30 +36,44 @@ func init() {
 	taskman.RegisterTask(EipDissociateTask{})
 }
 
-func (self *EipDissociateTask) TaskFail(ctx context.Context, eip *models.SElasticip, msg string, vm *models.SGuest) {
+func (self *EipDissociateTask) TaskFail(ctx context.Context, eip *models.SElasticip, msg string, model db.IModel) {
 	eip.SetStatus(self.UserCred, api.EIP_STATUS_READY, msg)
 	self.SetStageFailed(ctx, msg)
-	if vm != nil {
-		vm.SetStatus(self.UserCred, api.VM_DISSOCIATE_EIP_FAILED, msg)
-		db.OpsLog.LogEvent(vm, db.ACT_EIP_DETACH, msg, self.GetUserCred())
-		logclient.AddActionLogWithStartable(self, vm, logclient.ACT_EIP_DISSOCIATE, msg, self.UserCred, false)
+	if model != nil {
+		switch srv := model.(type) {
+		case *models.SGuest:
+			srv.SetStatus(self.UserCred, api.VM_DISSOCIATE_EIP_FAILED, msg)
+		}
+		db.OpsLog.LogEvent(model, db.ACT_EIP_DETACH, msg, self.GetUserCred())
+		logclient.AddActionLogWithStartable(self, model, logclient.ACT_EIP_DISSOCIATE, msg, self.UserCred, false)
 	}
 }
 
 func (self *EipDissociateTask) OnInit(ctx context.Context, obj db.IStandaloneModel, data jsonutils.JSONObject) {
 	eip := obj.(*models.SElasticip)
 
-	server := eip.GetAssociateVM()
-	if server != nil {
+	if eip.IsAssociated() {
 
-		if server.Status != api.VM_DISSOCIATE_EIP {
-			server.SetStatus(self.UserCred, api.VM_DISSOCIATE_EIP, "dissociate eip")
+		var model db.IModel
+
+		if server := eip.GetAssociateVM(); server != nil {
+			if server.Status != api.VM_DISSOCIATE_EIP {
+				server.SetStatus(self.UserCred, api.VM_DISSOCIATE_EIP, "dissociate eip")
+			}
+			model = server
+		} else if lb := eip.GetAssociateLoadbalancer(); lb != nil {
+			model = lb
+		} else if nat := eip.GetAssociateNatGateway(); nat != nil {
+			model = nat
+		} else {
+			self.TaskFail(ctx, eip, "unsupported associate type", nil)
+			return
 		}
 
 		extEip, err := eip.GetIEip()
 		if err != nil && err != cloudprovider.ErrNotFound {
 			msg := fmt.Sprintf("fail to find iEIP for eip %s", err)
-			self.TaskFail(ctx, eip, msg, server)
+			self.TaskFail(ctx, eip, msg, model)
 			return
 		}
 
@@ -67,7 +81,7 @@ func (self *EipDissociateTask) OnInit(ctx context.Context, obj db.IStandaloneMod
 			err = extEip.Dissociate()
 			if err != nil {
 				msg := fmt.Sprintf("fail to remote dissociate eip %s", err)
-				self.TaskFail(ctx, eip, msg, server)
+				self.TaskFail(ctx, eip, msg, model)
 				return
 			}
 		}
@@ -75,14 +89,18 @@ func (self *EipDissociateTask) OnInit(ctx context.Context, obj db.IStandaloneMod
 		err = eip.Dissociate(ctx, self.UserCred)
 		if err != nil {
 			msg := fmt.Sprintf("fail to local dissociate eip %s", err)
-			self.TaskFail(ctx, eip, msg, server)
+			self.TaskFail(ctx, eip, msg, model)
 			return
 		}
 
 		eip.SetStatus(self.UserCred, api.EIP_STATUS_READY, "dissociate")
 
-		logclient.AddActionLogWithStartable(self, server, logclient.ACT_EIP_DISSOCIATE, nil, self.UserCred, true)
-		server.StartSyncstatus(ctx, self.UserCred, "")
+		logclient.AddActionLogWithStartable(self, model, logclient.ACT_EIP_DISSOCIATE, nil, self.UserCred, true)
+
+		switch srv := model.(type) {
+		case *models.SGuest:
+			srv.StartSyncstatus(ctx, self.UserCred, "")
+		}
 	}
 
 	self.SetStageComplete(ctx, nil)

pkg/compute/tasks/guest_batch_create_task.go

@@ -108,15 +108,29 @@ func (self *GuestBatchCreateTask) allocateGuestOnHost(ctx context.Context, guest
 		guest.SetStatus(self.UserCred, api.VM_CREATE_FAILED, err.Error())
 		return err
 	}
+
+	// allocate networks
 	err = guest.CreateNetworksOnHost(ctx, self.UserCred, host, input.Networks, &pendingUsage, candidate.Nets)
 	self.SetPendingUsage(&pendingUsage)
-
 	if err != nil {
 		log.Errorf("Network failed: %s", err)
 		guest.SetStatus(self.UserCred, api.VM_NETWORK_FAILED, err.Error())
 		return err
 	}
 
+	// allocate eips
+	if input.EipBw > 0 {
+		eip, err := models.ElasticipManager.NewEipForVMOnHost(ctx, self.UserCred, guest, host, input.EipBw, input.EipChargeType, &pendingUsage)
+		self.SetPendingUsage(&pendingUsage)
+		if err != nil {
+			log.Errorf("guest.CreateElasticipOnHost failed: %s", err)
+			guest.SetStatus(self.UserCred, api.VM_NETWORK_FAILED, err.Error())
+			return err
+		}
+		input.Eip = eip.Id
+	}
+
+	// allocate disks
 	guest.GetDriver().PrepareDiskRaidConfig(self.UserCred, host, input.BaremetalDiskConfigs)
 	var backupCandidateDisks []*schedapi.CandidateDisk
 	if candidate.BackupCandidate != nil {
@@ -132,9 +146,9 @@ func (self *GuestBatchCreateTask) allocateGuestOnHost(ctx context.Context, guest
 		return err
 	}
 
+	// allocate GPUs
 	err = guest.CreateIsolatedDeviceOnHost(ctx, self.UserCred, host, input.IsolatedDevices, &pendingUsage)
 	self.SetPendingUsage(&pendingUsage)
-
 	if err != nil {
 		log.Errorf("IsolatedDevices create failed: %s", err)
 		guest.SetStatus(self.UserCred, api.VM_DEVICE_FAILED, err.Error())

pkg/compute/tasks/guest_create_task.go

@@ -150,26 +150,24 @@ func (self *GuestCreateTask) TaskComplete(ctx context.Context, guest *models.SGu
 }
 
 func (self *GuestCreateTask) StartEipSubTask(ctx context.Context, guest *models.SGuest) {
-	eipId, _ := self.Params.GetString("eip_id")
+	eipId, _ := self.Params.GetString("eip")
 	if len(eipId) > 0 {
 		eipObj, err := models.ElasticipManager.FetchById(eipId)
 		if err != nil {
 			log.Errorf("fail to get eip %s %s", eipId, err)
 			return
 		}
+
 		eip := eipObj.(*models.SElasticip)
-		eip.StartEipAssociateInstanceTask(ctx, self.UserCred, guest, "")
+
+		eipBw, _ := self.Params.Int("eip_bw")
+		if eipBw > 0 {
+			// newly allocated eip, need allocation and associate
+			eip.AllocateAndAssociateVM(ctx, self.UserCred, guest)
+		} else {
+			// existing eip, association only
+			eip.StartEipAssociateInstanceTask(ctx, self.UserCred, guest, "")
+		}
 		return
 	}
-	eipBw, _ := self.Params.Int("eip_bw")
-	if eipBw > 0 {
-		pendingUsage := models.SQuota{}
-		err := self.GetPendingUsage(&pendingUsage)
-		if err != nil {
-			log.Errorf("GetPendingUsage fail %s", err)
-		}
-		eipChargeType, _ := self.Params.GetString("eip_charge_type")
-		models.ElasticipManager.AllocateEipAndAssociateVM(ctx, self.UserCred, guest, int(eipBw), eipChargeType, &pendingUsage)
-		self.SetPendingUsage(&pendingUsage)
-	}
 }

pkg/util/aliyun/eip.go

@@ -155,6 +155,8 @@ func (self *SEipAddress) GetAssociationType() string {
 		return api.EIP_ASSOCIATE_TYPE_SERVER
 	case EIP_INSTANCE_TYPE_NAT:
 		return api.EIP_ASSOCIATE_TYPE_NAT_GATEWAY
+	case EIP_INTANNCE_TYPE_SLB:
+		return api.EIP_ASSOCIATE_TYPE_LOADBALANCER
 	default:
 		log.Fatalf("unsupported type: %s", self.InstanceType)
 		return "unsupported"

pkg/util/qcloud/eip.go

@@ -137,11 +137,18 @@ func (self *SEipAddress) GetMode() string {
 
 func (self *SEipAddress) GetAssociationType() string {
 	if len(self.InstanceId) > 0 {
-		for prefix, instanceType := range map[string]string{"nat-": api.EIP_ASSOCIATE_TYPE_NAT_GATEWAY, "ins-": api.EIP_ASSOCIATE_TYPE_SERVER} {
+		for prefix, instanceType := range map[string]string{
+			"nat-": api.EIP_ASSOCIATE_TYPE_NAT_GATEWAY,
+			"ins-": api.EIP_ASSOCIATE_TYPE_SERVER,
+			"lb-":  api.EIP_ASSOCIATE_TYPE_LOADBALANCER,
+			"lbl-": api.EIP_ASSOCIATE_TYPE_LOADBALANCER,
+		} {
 			if strings.HasPrefix(prefix, self.InstanceId) {
 				return instanceType
 			}
 		}
+		log.Fatalf("unsupported type: %s", self.InstanceId)
+		return "unsupported"
 	}
 	return ""
 }