Merge pull request #2369 from swordqiu/hotfix/qj-qcloud-upload-duplicate-image

fix: avoid duplicate image uploading if multiple identical instances

pkg/compute/hostdrivers/managedvirtual.go

@@ -63,6 +63,8 @@ func (self *SManagedVirtualizationHostDriver) CheckAndSetCacheImage(ctx context.
 		lockman.LockRawObject(ctx, "cachedimages", fmt.Sprintf("%s-%s", storageCache.Id, image.ImageId))
 		defer lockman.ReleaseRawObject(ctx, "cachedimages", fmt.Sprintf("%s-%s", storageCache.Id, image.ImageId))
 
+		log.Debugf("XXX Hold lockman key %p cachedimages %s-%s", ctx, storageCache.Id, image.ImageId)
+
 		scimg := models.StoragecachedimageManager.Register(ctx, task.GetUserCred(), storageCache.Id, image.ImageId, "")
 
 		cachedImage := scimg.GetCachedimage()
@@ -91,7 +93,10 @@ func (self *SManagedVirtualizationHostDriver) CheckAndSetCacheImage(ctx context.
 			return nil, err
 		}
 
-		// scimg.SetExternalId(extImgId)
+		// should record the externalId immediately
+		// so the waiting goroutine could pick the new externalId
+		// and avoid duplicate uploading
+		scimg.SetExternalId(image.ExternalId)
 
 		ret := jsonutils.NewDict()
 		ret.Add(jsonutils.NewString(image.ExternalId), "image_id")
@@ -337,7 +342,7 @@ func (self *SManagedVirtualizationHostDriver) RequestRebuildDiskOnStorage(ctx co
 func (driver *SManagedVirtualizationHostDriver) IsReachStoragecacheCapacityLimit(host *models.SHost, cachedImages []models.SCachedimage) bool {
 	quota := host.GetHostDriver().GetStoragecacheQuota(host)
 	log.Debugf("Cached image total: %d quota: %d", len(cachedImages), quota)
-	if quota > 0 && len(cachedImages)+1 >= quota {
+	if quota > 0 && len(cachedImages) >= quota {
 		return true
 	}
 	return false

pkg/compute/models/storagecaches.go

@@ -263,7 +263,7 @@ func (self *SStoragecache) GetCustomizeColumns(ctx context.Context, userCred mcc
 	return extra
 }
 
-func (self *SStoragecache) getCachedImageList(excludeIds []string, imageType string) []SCachedimage {
+func (self *SStoragecache) getCachedImageList(excludeIds []string, imageType string, status []string) []SCachedimage {
 	images := make([]SCachedimage, 0)
 
 	cachedImages := CachedimageManager.Query().SubQuery()
@@ -279,6 +279,9 @@ func (self *SStoragecache) getCachedImageList(excludeIds []string, imageType str
 	if len(imageType) > 0 {
 		q = q.Filter(sqlchemy.Equals(cachedImages.Field("image_type"), imageType))
 	}
+	if len(status) > 0 {
+		q = q.Filter(sqlchemy.In(storagecachedImages.Field("status"), status))
+	}
 
 	err := db.FetchModelObjects(CachedimageManager, q, &images)
 	if err != nil {
@@ -584,20 +587,29 @@ func (cache *SStoragecache) SyncCloudImages(
 func (self *SStoragecache) IsReachCapacityLimit(imageId string) bool {
 	imgObj, _ := CachedimageManager.FetchById(imageId)
 	if imgObj == nil {
+		log.Debugf("no such cached image %s", imageId)
 		return false
 	}
 	cachedImage := imgObj.(*SCachedimage)
 	if cachedImage.ImageType != cloudprovider.CachedImageTypeCustomized {
 		// no need to cache
+		log.Debugf("image %s is not a customized image, no need to cache", imageId)
 		return false
 	}
-	cachedImages := self.getCachedImageList([]string{imageId}, cloudprovider.CachedImageTypeCustomized)
+	cachedImages := self.getCachedImageList(nil, cloudprovider.CachedImageTypeCustomized, []string{api.CACHED_IMAGE_STATUS_READY})
+	for i := range cachedImages {
+		if cachedImages[i].Id == imageId {
+			// already cached
+			log.Debugf("image %s has been cached in storage cache %s(%s)", imageId, self.Id, self.Name)
+			return false
+		}
+	}
 	host, _ := self.GetHost()
 	return host.GetHostDriver().IsReachStoragecacheCapacityLimit(host, cachedImages)
 }
 
 func (self *SStoragecache) StartRelinquishLeastUsedCachedImageTask(ctx context.Context, userCred mcclient.TokenCredential, imageId string, parentTaskId string) error {
-	cachedImages := self.getCachedImageList([]string{imageId}, cloudprovider.CachedImageTypeCustomized)
+	cachedImages := self.getCachedImageList([]string{imageId}, cloudprovider.CachedImageTypeCustomized, []string{api.CACHED_IMAGE_STATUS_READY})
 	leastUsedIdx := -1
 	leastRefCount := -1
 	for i := range cachedImages {

pkg/multicloud/aliyun/latitud_and_longitude.go

@@ -28,6 +28,7 @@ var LatitudeAndLongitude = map[string]cloudprovider.SGeographicInfo{
 	"cn-shanghai":    {Latitude: 31.230391, Longitude: 121.473701, City: api.CITY_SHANG_HAI, CountryCode: api.COUNTRY_CODE_CN},
 	"cn-shenzhen":    {Latitude: 22.543097, Longitude: 114.057861, City: api.CITY_SHEN_ZHEN, CountryCode: api.COUNTRY_CODE_CN},
 	"cn-hongkong":    {Latitude: 22.396427, Longitude: 114.109497, City: api.CITY_HONG_KONG, CountryCode: api.COUNTRY_CODE_CN},
+	"cn-chengdu":     {Latitude: 30.572815, Longitude: 104.066803, City: api.CITY_CHENG_DU, CountryCode: api.COUNTRY_CODE_CN},
 	"ap-northeast-1": {Latitude: 35.709026, Longitude: 139.731995, City: api.CITY_TOKYO, CountryCode: api.COUNTRY_CODE_JP},
 	"ap-southeast-1": {Latitude: 1.352083, Longitude: 103.819839, City: api.CITY_SINGAPORE, CountryCode: api.COUNTRY_CODE_SG},
 	"ap-southeast-2": {Latitude: -33.868820, Longitude: 151.209290, City: api.CITY_SYDNEY, CountryCode: api.COUNTRY_CODE_AU},

pkg/multicloud/aliyun/storagecache.go

@@ -115,12 +115,11 @@ func (self *SStoragecache) GetPath() string {
 func (self *SStoragecache) UploadImage(ctx context.Context, userCred mcclient.TokenCredential, image *cloudprovider.SImageCreateOption, isForce bool) (string, error) {
 
 	if len(image.ExternalId) > 0 {
-		log.Debugf("UploadImage: Image external ID exists %s", image.ExternalId)
-
 		status, err := self.region.GetImageStatus(image.ExternalId)
 		if err != nil {
 			log.Errorf("GetImageStatus error %s", err)
 		}
+		log.Debugf("UploadImage: Image external ID %s exists, status %s", image.ExternalId, status)
 		if status == ImageStatusAvailable && !isForce {
 			return image.ExternalId, nil
 		}

pkg/multicloud/qcloud/instance.go

@@ -570,9 +570,11 @@ func (self *SRegion) doStartVM(instanceId string) error {
 func (self *SRegion) doStopVM(instanceId string, isForce bool) error {
 	params := make(map[string]string)
 	if isForce {
-		params["ForceStop"] = "true"
+		params["ForceStop"] = "TRUE"
+		params["StopType"] = "HARD"
 	} else {
-		params["ForceStop"] = "false"
+		params["ForceStop"] = "FALSE"
+		params["StopType"] = "SOFT"
 	}
 	return self.instanceOperation(instanceId, "StopInstances", params, true)
 }

pkg/multicloud/qcloud/storagecache.go

@@ -137,18 +137,17 @@ func (self *SStoragecache) GetPath() string {
 
 func (self *SStoragecache) UploadImage(ctx context.Context, userCred mcclient.TokenCredential, image *cloudprovider.SImageCreateOption, isForce bool) (string, error) {
 	if len(image.ExternalId) > 0 {
-		log.Debugf("UploadImage: Image external ID exists %s", image.ExternalId)
-
 		status, err := self.region.GetImageStatus(image.ExternalId)
 		if err != nil {
 			log.Errorf("GetImageStatus error %s", err)
 		}
+		log.Debugf("ctx %p UploadImage: Image external ID %s exists, status %s", ctx, image.ExternalId, status)
 		if (status == ImageStatusNormal || status == ImageStatusUsing) && !isForce {
 			return image.ExternalId, nil
 		}
 		log.Debugf("image status: %s isForce: %v", status, isForce)
 	} else {
-		log.Debugf("UploadImage: no external ID")
+		log.Debugf("ctx %s UploadImage: no external ID", ctx)
 	}
 	return self.uploadImage(ctx, userCred, image, isForce)
 }