Merge pull request #688 in YUNIONIO/onecloud from ~ZHOUYOUSONG/onecloud:bugfix/yousong-vpcs-errs to release/2.4.0

* commit '76f1ba405c417af042d6ebafc69e919e35589a91':
  hosts: GetHostsByManagerAndRegion(): sql调整
  tasks: VpcCreateTask: 风格调整
  vpcs: SyncWithCloudVpc(): 避免重复记日志
  vpcs: GetRegion(): 传递原始err信息

pkg/compute/models/hosts.go

@@ -1978,12 +1978,10 @@ func (self *SHost) GetDetailsIpmi(ctx context.Context, userCred mcclient.TokenCr
 }
 
 func (manager *SHostManager) GetHostsByManagerAndRegion(managerId string, regionId string) []SHost {
-	hosts := HostManager.Query().SubQuery()
-	zones := ZoneManager.Query().SubQuery()
-	q := hosts.Query()
-	q = q.Join(zones, sqlchemy.Equals(hosts.Field("zone_id"), zones.Field("id")))
-	q = q.Filter(sqlchemy.Equals(hosts.Field("manager_id"), managerId))
-	q = q.Filter(sqlchemy.Equals(zones.Field("cloudregion_id"), regionId))
+	zones := ZoneManager.Query().Equals("cloudregion_id", regionId).SubQuery()
+	hosts := HostManager.Query()
+	q := hosts.Equals("manager_id", managerId)
+	q = q.Join(zones, sqlchemy.Equals(zones.Field("id"), hosts.Field("zone_id")))
 	ret := make([]SHost, 0)
 	err := db.FetchModelObjects(HostManager, q, &ret)
 	if err != nil {

pkg/compute/models/routetables.go

@@ -142,9 +142,9 @@ func (man *SRouteTableManager) ValidateCreateData(ctx context.Context, userCred
 		return nil, err
 	}
 	vpc := vpcV.Model.(*SVpc)
-	cloudregion := vpc.GetRegion()
-	if cloudregion == nil {
-		return nil, httperrors.NewConflictError("failed fetching cloudregion of vpc %s(%s)", vpc.Name, vpc.Id)
+	cloudregion, err := vpc.GetRegion()
+	if err != nil {
+		return nil, httperrors.NewConflictError("failed getting region of vpc %s(%s)", vpc.Name, vpc.Id)
 	}
 	data.Set("cloudregion_id", jsonutils.NewString(cloudregion.Id))
 	return man.SVirtualResourceBaseManager.ValidateCreateData(ctx, userCred, ownerProjId, query, data)

pkg/compute/models/vpcs.go

@@ -153,23 +153,24 @@ func (self *SVpc) GetNetworkCount() int {
 func (self *SVpc) getMoreDetails(extra *jsonutils.JSONDict) *jsonutils.JSONDict {
 	extra.Add(jsonutils.NewInt(int64(self.GetWireCount())), "wire_count")
 	extra.Add(jsonutils.NewInt(int64(self.GetNetworkCount())), "network_count")
-	region := self.GetRegion()
-	if region != nil {
-		extra.Add(jsonutils.NewString(region.GetName()), "region")
-		if len(region.GetExternalId()) > 0 {
-			extra.Add(jsonutils.NewString(region.GetExternalId()), "region_external_id")
-		}
+	region, err := self.GetRegion()
+	if err != nil {
+		log.Errorf("failed getting region for vpc %s(%s)", self.Name, self.Id)
+		return extra
+	}
+	extra.Add(jsonutils.NewString(region.GetName()), "region")
+	if len(region.GetExternalId()) > 0 {
+		extra.Add(jsonutils.NewString(region.GetExternalId()), "region_external_id")
 	}
 	return extra
 }
 
-func (self *SVpc) GetRegion() *SCloudregion {
+func (self *SVpc) GetRegion() (*SCloudregion, error) {
 	region, err := CloudregionManager.FetchById(self.CloudregionId)
 	if err != nil {
-		log.Errorf("Get region error %s", err)
-		return nil
+		return nil, err
 	}
-	return region.(*SCloudregion)
+	return region.(*SCloudregion), nil
 }
 
 func (self *SVpc) GetCustomizeColumns(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject) *jsonutils.JSONDict {
@@ -287,9 +288,6 @@ func (self *SVpc) SyncWithCloudVpc(extVPC cloudprovider.ICloudVpc) error {
 
 		return nil
 	})
-	if err != nil {
-		log.Errorf("syncWithCloudVpc error %s", err)
-	}
 	return err
 }
 
@@ -410,14 +408,12 @@ func (self *SVpc) PostCreate(ctx context.Context, userCred mcclient.TokenCredent
 }
 
 func (self *SVpc) GetIRegion() (cloudprovider.ICloudRegion, error) {
-	region := self.GetRegion()
-	if region == nil {
-		log.Errorf("cannot find region for this vpc??")
-		return nil, fmt.Errorf("Cannot find region")
+	region, err := self.GetRegion()
+	if err != nil {
+		return nil, err
 	}
 	provider, err := self.GetDriver()
 	if err != nil {
-		log.Errorf("fail to find cloud provider")
 		return nil, err
 	}
 	return provider.GetIRegionById(region.GetExternalId())
@@ -433,11 +429,9 @@ func (self *SVpc) GetIVpc() (cloudprovider.ICloudVpc, error) {
 	if provider.IsOnPremiseInfrastructure() {
 		iregion, err = provider.GetOnPremiseIRegion()
 	} else {
-		region := self.GetRegion()
-		if region == nil {
-			msg := "fail to find region of host???"
-			log.Errorf(msg)
-			return nil, fmt.Errorf(msg)
+		region, err := self.GetRegion()
+		if err != nil {
+			return nil, err
 		}
 		iregion, err = provider.GetIRegionById(region.ExternalId)
 	}

pkg/compute/tasks/vpc_create_task.go

@@ -51,7 +51,6 @@ func (self *VpcCreateTask) OnInit(ctx context.Context, obj db.IStandaloneModel,
 
 	err = vpc.SyncWithCloudVpc(ivpc)
 	if err != nil {
-		log.Errorf("SyncWithCloudVpc fail: %s", err)
 		self.TaskFailed(ctx, vpc, err)
 		return
 	}
@@ -60,16 +59,13 @@ func (self *VpcCreateTask) OnInit(ctx context.Context, obj db.IStandaloneModel,
 	syncVpcWires(ctx, provider, self, vpc, ivpc, &models.SSyncRange{})
 
 	hosts := models.HostManager.GetHostsByManagerAndRegion(provider.Id, vpc.CloudregionId)
-	if hosts != nil {
-		for i := 0; i < len(hosts); i += 1 {
-			ihost, err := hosts[i].GetIHost()
-			if err != nil {
-				log.Errorf("getiHost fail %s", err)
-				self.TaskFailed(ctx, vpc, err)
-				return
-			}
-			syncHostWires(ctx, provider, self, &hosts[i], ihost)
+	for i := 0; i < len(hosts); i += 1 {
+		ihost, err := hosts[i].GetIHost()
+		if err != nil {
+			self.TaskFailed(ctx, vpc, err)
+			return
 		}
+		syncHostWires(ctx, provider, self, &hosts[i], ihost)
 	}
 
 	self.SetStageComplete(ctx, nil)