fix: fail to delete sub images of composed guest image (#22281)

Co-authored-by: Qiu Jian <qiujian@yunionyun.com>

pkg/apis/image/consts.go

@@ -41,6 +41,8 @@ const (
 	IMAGE_STATUS_DELETED        = "deleted"
 	IMAGE_STATUS_PENDING_DELETE = "pending_delete"
 
+	IMAGE_STATUS_UNKNOWN = "unkown"
+
 	IMAGE_STATUS_SYNC_CLASS_METADATA_FAILEd = "sync_class_metadata_failed"
 
 	ImageTypeTemplate = TImageType("image")

pkg/image/models/image_guest.go

@@ -276,7 +276,7 @@ func (gi *SGuestImage) RealDelete(ctx context.Context, userCred mcclient.TokenCr
 func (gi *SGuestImage) CustomizeDelete(ctx context.Context, userCred mcclient.TokenCredential,
 	query jsonutils.JSONObject, data jsonutils.JSONObject) error {
 
-	images, err := GuestImageJointManager.GetImagesByGuestImageId(gi.Id)
+	images, err := GuestImageJointManager.GetByGuestImageId(gi.Id)
 	if err != nil {
 		return errors.Wrap(err, "get images of guest images failed")
 	}
@@ -334,12 +334,16 @@ func (gi *SGuestImage) PerformCancelDelete(ctx context.Context, userCred mcclien
 }
 
 func (gi *SGuestImage) DoCancelPendingDelete(ctx context.Context, userCred mcclient.TokenCredential) error {
-	subImages, err := GuestImageJointManager.GetImagesByGuestImageId(gi.Id)
+	subImages, err := GuestImageJointManager.GetByGuestImageId(gi.Id)
 	if err != nil {
 		return errors.Wrap(err, "GetImagesByGuestImageId")
 	}
 	for i := range subImages {
-		err = subImages[i].DoCancelPendingDelete(ctx, userCred)
+		image, err := subImages[i].GetImage()
+		if err != nil {
+			return errors.Wrapf(err, "subImages[%d].GetImage", i)
+		}
+		err = image.DoCancelPendingDelete(ctx, userCred)
 		if err != nil {
 			return errors.Wrapf(err, "subimage %s cancel delete error", subImages[i].GetId())
 		}
@@ -355,14 +359,14 @@ func (gi *SGuestImage) DoCancelPendingDelete(ctx context.Context, userCred mccli
 	return errors.Wrap(err, "guest image cancel delete error")
 }
 
-func (self *SGuestImage) getMoreDetails(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject,
+func (guestImage *SGuestImage) getMoreDetails(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject,
 	out api.GuestImageDetails) api.GuestImageDetails {
 
-	if self.Status != api.IMAGE_STATUS_ACTIVE {
-		self.checkStatus(ctx, userCred)
-		out.Status = self.Status
+	if guestImage.Status != api.IMAGE_STATUS_ACTIVE {
+		guestImage.checkStatus(ctx, userCred)
+		out.Status = guestImage.Status
 	}
-	images, err := GuestImageJointManager.GetImagesByGuestImageId(self.Id)
+	images, err := GuestImageJointManager.GetByGuestImageId(guestImage.Id)
 	if err != nil {
 		return out
 	}
@@ -374,7 +378,11 @@ func (self *SGuestImage) getMoreDetails(ctx context.Context, userCred mcclient.T
 	dataImages := make([]api.SubImageInfo, 0, len(images)-1)
 	var rootImage *api.SubImageInfo
 	for i := range images {
-		image := images[i]
+		image, err := images[i].GetImage()
+		if err != nil {
+			log.Errorf("subimage[%d].GetImage fail %s", i, err)
+			continue
+		}
 		size += image.Size
 		if !image.IsData.IsTrue() && rootImage == nil {
 			rootImage = &api.SubImageInfo{
@@ -425,7 +433,7 @@ func (self *SGuestImage) getMoreDetails(ctx context.Context, userCred mcclient.T
 		}
 		out.Properties = propJson
 	}
-	out.DisableDelete = self.Protected.Bool()
+	out.DisableDelete = guestImage.Protected.Bool()
 	return out
 }
 
@@ -454,39 +462,44 @@ func (manager *SGuestImageManager) FetchCustomizeColumns(
 	return rows
 }
 
-func (self *SGuestImage) PostUpdate(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject,
+func (guestImage *SGuestImage) PostUpdate(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject,
 	data jsonutils.JSONObject) {
 
 	lockman.LockClass(ctx, ImageManager, db.GetLockClassKey(ImageManager, userCred))
 	defer lockman.ReleaseClass(ctx, ImageManager, db.GetLockClassKey(ImageManager, userCred))
 
-	err := self.UpdateSubImage(ctx, userCred, data)
+	err := guestImage.UpdateSubImage(ctx, userCred, data)
 	if err != nil {
-		logclient.AddSimpleActionLog(self, logclient.ACT_UPDATE, nil, userCred, false)
+		logclient.AddSimpleActionLog(guestImage, logclient.ACT_UPDATE, nil, userCred, false)
 	}
 
 }
 
-func (self *SGuestImage) UpdateSubImage(ctx context.Context, userCred mcclient.TokenCredential,
+func (guestImage *SGuestImage) UpdateSubImage(ctx context.Context, userCred mcclient.TokenCredential,
 	data jsonutils.JSONObject) error {
 
-	subImages, err := GuestImageJointManager.GetImagesByFilter(self.GetId(), func(q *sqlchemy.SQuery) *sqlchemy.SQuery {
-		return q.Asc("name")
-	})
+	subImages, err := GuestImageJointManager.GetByGuestImageId(guestImage.Id)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "GetByGuestImageId")
 	}
 	dict := data.(*jsonutils.JSONDict)
 	var g errgroup.Group
 	for i := range subImages {
-		if f, ok := self.genUpdateImage(ctx, userCred, &subImages[i], i, dict); ok {
+		img, err := subImages[i].GetImage()
+		if err != nil {
+			g.Go(func() error {
+				return err
+			})
+			continue
+		}
+		if f, ok := guestImage.genUpdateImage(ctx, userCred, img, i, dict); ok {
 			g.Go(f)
 		}
 	}
 	return g.Wait()
 }
 
-func (self *SGuestImage) genUpdateImage(ctx context.Context, userCred mcclient.TokenCredential, image *SImage,
+func (guestImage *SGuestImage) genUpdateImage(ctx context.Context, userCred mcclient.TokenCredential, image *SImage,
 	index int, dict *jsonutils.JSONDict) (func() error, bool) {
 	if image.IsGuestImage.IsFalse() {
 		return nil, false
@@ -553,12 +566,13 @@ var checkStatus = map[string]int{
 	api.IMAGE_STATUS_SAVING:      3,
 	api.IMAGE_STATUS_DEACTIVATED: 4,
 	api.IMAGE_STATUS_KILLED:      5,
+	api.IMAGE_STATUS_UNKNOWN:     6,
 }
 
-func (self *SGuestImage) checkStatus(ctx context.Context, userCred mcclient.TokenCredential) error {
-	images, err := GuestImageJointManager.GetImagesByGuestImageId(self.Id)
+func (guestImage *SGuestImage) checkStatus(ctx context.Context, userCred mcclient.TokenCredential) error {
+	images, err := GuestImageJointManager.GetByGuestImageId(guestImage.Id)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "GetByGuestImageId")
 	}
 	if len(images) == 0 {
 		return nil
@@ -567,38 +581,49 @@ func (self *SGuestImage) checkStatus(ctx context.Context, userCred mcclient.Toke
 	status := api.IMAGE_STATUS_ACTIVE
 
 	for i := range images {
-		if checkStatus[images[i].Status] > checkStatus[status] {
-			status = images[i].Status
+		imgStatus := ""
+		image, err := images[i].GetImage()
+		if err != nil {
+			log.Errorf("subimage %d GetImage fail %s", i, err)
+			imgStatus = api.IMAGE_STATUS_UNKNOWN
+		} else {
+			imgStatus = image.Status
+		}
+		if checkStatus[imgStatus] > checkStatus[status] {
+			status = imgStatus
 		}
 	}
-	if self.Status != status {
-		self.SetStatus(ctx, userCred, status, "")
-		self.Status = status
+	if guestImage.Status != status {
+		guestImage.SetStatus(ctx, userCred, status, "")
+		guestImage.Status = status
 	}
 	return nil
 }
 
-func (self *SGuestImage) getSize(ctx context.Context, userCred mcclient.TokenCredential) (int64, error) {
-	images, err := GuestImageJointManager.GetImagesByGuestImageId(self.Id)
+func (guestImage *SGuestImage) getSize(ctx context.Context, userCred mcclient.TokenCredential) (int64, error) {
+	images, err := GuestImageJointManager.GetByGuestImageId(guestImage.Id)
 	if err != nil {
 		return 0, err
 	}
 	var size int64 = 0
 	for i := range images {
-		size += images[i].Size
+		image, _ := images[i].GetImage()
+		if image != nil {
+			size += image.Size
+		}
 	}
 	return size, nil
 }
 
-func (self *SGuestImageManager) getExpiredPendingDeleteImages() []SGuestImage {
+func (guestImage *SGuestImageManager) getExpiredPendingDeleteImages() []SGuestImage {
 	deadline := time.Now().Add(time.Duration(-options.Options.PendingDeleteExpireSeconds) * time.Second)
 
 	// there are so many common images of one guest image, so that batch shrink three times
-	q := self.Query().IsTrue("pending_deleted").LT("pending_deleted_at",
+	q := guestImage.Query().IsTrue("pending_deleted").LT("pending_deleted_at",
 		deadline).Limit(options.Options.PendingDeleteMaxCleanBatchSize / 3)
 
 	images := make([]SGuestImage, 0)
-	err := db.FetchModelObjects(self, q, &images)
+	err := db.FetchModelObjects(guestImage, q, &images)
 	if err != nil {
 		log.Errorf("fetch guest images error %s", err)
 		return nil
@@ -606,10 +631,10 @@ func (self *SGuestImageManager) getExpiredPendingDeleteImages() []SGuestImage {
 	return images
 }
 
-func (self *SGuestImageManager) CleanPendingDeleteImages(ctx context.Context, userCred mcclient.TokenCredential,
+func (guestImage *SGuestImageManager) CleanPendingDeleteImages(ctx context.Context, userCred mcclient.TokenCredential,
 	isStart bool) {
 
-	images := self.getExpiredPendingDeleteImages()
+	images := guestImage.getExpiredPendingDeleteImages()
 	if images == nil {
 		return
 	}
@@ -618,42 +643,50 @@ func (self *SGuestImageManager) CleanPendingDeleteImages(ctx context.Context, us
 	}
 }
 
-func (self *SGuestImage) PerformPublic(
+func (guestImage *SGuestImage) PerformPublic(
 	ctx context.Context,
 	userCred mcclient.TokenCredential,
 	query jsonutils.JSONObject,
 	input apis.PerformPublicProjectInput,
 ) (jsonutils.JSONObject, error) {
-	images, err := GuestImageJointManager.GetImagesByGuestImageId(self.Id)
+	images, err := GuestImageJointManager.GetByGuestImageId(guestImage.Id)
 	if err != nil {
 		return nil, errors.Wrap(err, "fail to fetch subimages of guest image")
 	}
 	for i := range images {
-		_, err := images[i].performPublic(ctx, userCred, query, input)
+		image, _ := images[i].GetImage()
+		if image == nil {
+			continue
+		}
+		_, err := image.performPublic(ctx, userCred, query, input)
 		if err != nil {
 			return nil, errors.Wrapf(err, "fail to public subimage %s", images[i].GetId())
 		}
 	}
-	return self.SSharableVirtualResourceBase.PerformPublic(ctx, userCred, query, input)
+	return guestImage.SSharableVirtualResourceBase.PerformPublic(ctx, userCred, query, input)
 }
 
-func (self *SGuestImage) PerformPrivate(
+func (guestImage *SGuestImage) PerformPrivate(
 	ctx context.Context,
 	userCred mcclient.TokenCredential,
 	query jsonutils.JSONObject,
 	input apis.PerformPrivateInput,
 ) (jsonutils.JSONObject, error) {
-	images, err := GuestImageJointManager.GetImagesByGuestImageId(self.Id)
+	images, err := GuestImageJointManager.GetByGuestImageId(guestImage.Id)
 	if err != nil {
 		return nil, errors.Wrap(err, "fail to fetch subimages of guest image")
 	}
 	for i := range images {
-		_, err := images[i].performPrivate(ctx, userCred, query, input)
+		image, _ := images[i].GetImage()
+		if image == nil {
+			continue
+		}
+		_, err := image.performPrivate(ctx, userCred, query, input)
 		if err != nil {
 			return nil, errors.Wrapf(err, "fail to private subimage %s", images[i].GetId())
 		}
 	}
-	return self.SSharableVirtualResourceBase.PerformPrivate(ctx, userCred, query, input)
+	return guestImage.SSharableVirtualResourceBase.PerformPrivate(ctx, userCred, query, input)
 }
 
 // 主机镜像列表
@@ -738,13 +771,18 @@ func (gi *SGuestImage) GetUsages() []db.IUsage {
 	if gi.PendingDeleted || gi.Deleted {
 		return nil
 	}
-	images, err := GuestImageJointManager.GetImagesByGuestImageId(gi.Id)
+	images, err := GuestImageJointManager.GetByGuestImageId(gi.Id)
 	if err != nil {
 		return nil
 	}
 	usages := make([]db.IUsage, 0)
 	for i := range images {
-		ui := images[i].GetUsages()
+		image, err := images[i].GetImage()
+		if err != nil {
+			log.Errorf("subimage %d GetImage fail %s", i, err)
+			continue
+		}
+		ui := image.GetUsages()
 		if len(ui) > 0 {
 			usages = append(usages, ui...)
 		}
@@ -757,12 +795,17 @@ func (img *SGuestImage) PerformSetClassMetadata(ctx context.Context, userCred mc
 	if err != nil {
 		return ret, err
 	}
-	images, err := GuestImageJointManager.GetImagesByGuestImageId(img.Id)
+	images, err := GuestImageJointManager.GetByGuestImageId(img.Id)
 	if err != nil {
 		return nil, err
 	}
 	for i := range images {
-		_, err := images[i].PerformSetClassMetadata(ctx, userCred, query, input)
+		image, err := images[i].GetImage()
+		if err != nil {
+			log.Errorf("subimage %d GetImage fail %s", i, err)
+			continue
+		}
+		_, err = image.PerformSetClassMetadata(ctx, userCred, query, input)
 		if err != nil {
 			return nil, errors.Wrapf(err, "unable to PerformSetClassMetadata for image %s", images[i].GetId())
 		}

pkg/image/models/image_guest_joint.go

@@ -24,6 +24,7 @@ import (
 
 	"yunion.io/x/onecloud/pkg/cloudcommon/db"
 	"yunion.io/x/onecloud/pkg/mcclient"
+	"yunion.io/x/onecloud/pkg/mcclient/auth"
 )
 
 // +onecloud:swagger-gen-ignore
@@ -54,6 +55,22 @@ func init() {
 	GuestImageJointManager.SetVirtualObject(GuestImageJointManager)
 }
 
+func (manager *SGuestImageJointManager) InitializeData() error {
+	q := manager.Query()
+	guestImageQ := GuestImageManager.RawQuery().IsTrue("deleted").SubQuery()
+	q = q.Join(guestImageQ, sqlchemy.Equals(q.Field("guest_image_id"), guestImageQ.Field("id")))
+
+	guestImageJoints := make([]SGuestImageJoint, 0)
+	err := db.FetchModelObjects(manager, q, &guestImageJoints)
+	if err != nil {
+		return errors.Wrap(err, "FetchModelObjects")
+	}
+	for i := range guestImageJoints {
+		guestImageJoints[i].Delete(context.Background(), auth.AdminCredential())
+	}
+	return nil
+}
+
 func (gm *SGuestImageJointManager) GetByGuestImageId(guestImageId string) ([]SGuestImageJoint, error) {
 	q := gm.Query().Equals("guest_image_id", guestImageId).Asc("row_id") // order by row_id ascending
 	ret := make([]SGuestImageJoint, 0, 1)
@@ -86,7 +103,7 @@ func (gm *SGuestImageJointManager) GetByImageId(imageId string) ([]SGuestImageJo
 	return model.(*SGuestImage), nil
 }*/
 
-func (gm *SGuestImageJointManager) GetImagesByFilter(guestImageId string,
+/*func (gm *SGuestImageJointManager) GetImagesByFilter(guestImageId string,
 	filter func(q *sqlchemy.SQuery) *sqlchemy.SQuery) ([]SImage, error) {
 
 	giJoints, err := gm.GetByGuestImageId(guestImageId)
@@ -114,7 +131,7 @@ func (gm *SGuestImageJointManager) GetImagesByGuestImageId(guestImageId string)
 	return gm.GetImagesByFilter(guestImageId, func(q *sqlchemy.SQuery) *sqlchemy.SQuery {
 		return q
 	})
-}
+}*/
 
 func (gt *SGuestImageJoint) GetId() string {
 	return fmt.Sprintf("guestimage-%s-image-%s", gt.GuestImageId, gt.ImageId)
@@ -124,8 +141,11 @@ func (gt *SGuestImageJoint) RealDelete(ctx context.Context, userCred mcclient.To
 	return db.DeleteModel(ctx, userCred, gt)
 }
 
-func (gt *SGuestImageJointManager) CreateGuestImageJoint(ctx context.Context, guestImageId,
-	imageId string) (*SGuestImageJoint, error) {
+func (gt *SGuestImageJointManager) CreateGuestImageJoint(
+	ctx context.Context,
+	guestImageId,
+	imageId string,
+) (*SGuestImageJoint, error) {
 
 	gi := SGuestImageJoint{}
 	gi.GuestImageId = guestImageId
@@ -138,3 +158,14 @@ func (gt *SGuestImageJointManager) CreateGuestImageJoint(ctx context.Context, gu
 	gi.SetVirtualObject(gt)
 	return &gi, nil
 }
+
+func (gt *SGuestImageJoint) GetImage() (*SImage, error) {
+	imgObj, err := ImageManager.FetchById(gt.ImageId)
+	if err != nil {
+		if errors.Cause(err) == sql.ErrNoRows {
+			err = errors.ErrNotFound
+		}
+		return nil, errors.Wrapf(err, "FetchByImageId %s", gt.ImageId)
+	}
+	return imgObj.(*SImage), nil
+}

pkg/image/models/initdb.go

@@ -30,6 +30,7 @@ func InitDB() error {
 		db.Metadata,
 
 		ImageManager,
+		GuestImageJointManager,
 		QuotaManager,
 	} {
 		err := manager.InitializeData()

pkg/image/tasks/guest_image_delete_task.go

@@ -54,16 +54,21 @@ func (task *GuestImageDeleteTask) OnInit(ctx context.Context, obj db.IStandalone
 }
 
 func (task *GuestImageDeleteTask) startPendingDelete(ctx context.Context, guestImage *models.SGuestImage) {
-	images, err := models.GuestImageJointManager.GetImagesByGuestImageId(guestImage.GetId())
+	images, err := models.GuestImageJointManager.GetByGuestImageId(guestImage.GetId())
 	if err != nil {
 		task.taskFailed(ctx, guestImage, jsonutils.NewString(err.Error()))
 	}
 	for i := range images {
-		if !images[i].IsGuestImage.IsTrue() {
+		image, err := images[i].GetImage()
+		if err != nil {
+			log.Errorf("subimage %d GetImage fail %s", i, err)
 			continue
 		}
-		images[i].StopTorrents()
-		err := images[i].DoPendingDelete(ctx, task.UserCred)
+		if !image.IsGuestImage.IsTrue() {
+			continue
+		}
+		image.StopTorrents()
+		err = image.DoPendingDelete(ctx, task.UserCred)
 		if err != nil {
 			task.taskFailed(ctx, guestImage, jsonutils.NewString(fmt.Sprintf("image %s pending delete failed", images[i].GetId())))
 			return
@@ -78,28 +83,39 @@ func (task *GuestImageDeleteTask) startPendingDelete(ctx context.Context, guestI
 }
 
 func (task *GuestImageDeleteTask) startDelete(ctx context.Context, guestImage *models.SGuestImage) {
-	images, err := models.GuestImageJointManager.GetImagesByGuestImageId(guestImage.GetId())
+	subImages, err := models.GuestImageJointManager.GetByGuestImageId(guestImage.GetId())
 	if err != nil {
 		task.taskFailed(ctx, guestImage, jsonutils.NewString(err.Error()))
 		return
 	}
-	for i := range images {
-		if !images[i].IsGuestImage.IsTrue() {
+	for i := range subImages {
+		image, err := subImages[i].GetImage()
+		if err != nil {
+			log.Errorf("subimage %d GetImage fail %s", i, err)
 			continue
 		}
-		err := images[i].Remove(ctx, task.UserCred)
+		// delete guestImageJoint
+		err = subImages[i].Delete(ctx, task.UserCred)
 		if err != nil {
-			task.taskFailed(ctx, guestImage, jsonutils.NewString(fmt.Sprintf("fail to remove %s: %s", images[i].GetPath(""), err)))
+			task.taskFailed(ctx, guestImage, jsonutils.NewString(fmt.Sprintf("fail to remove GuestImageJoin %d: %s", i, err)))
 			return
 		}
-		err = images[i].SetStatus(ctx, task.UserCred, api.IMAGE_STATUS_DELETED, "delete")
+		if !image.IsGuestImage.IsTrue() {
+			continue
+		}
+		err = image.Remove(ctx, task.UserCred)
 		if err != nil {
-			task.taskFailed(ctx, guestImage, jsonutils.NewString(fmt.Sprintf("fail to set image %s status ", images[i].GetId())))
+			task.taskFailed(ctx, guestImage, jsonutils.NewString(fmt.Sprintf("fail to remove %s: %s", image.GetPath(""), err)))
 			return
 		}
-		err = images[i].RealDelete(ctx, task.UserCred)
+		err = image.SetStatus(ctx, task.UserCred, api.IMAGE_STATUS_DELETED, "delete")
 		if err != nil {
-			task.taskFailed(ctx, guestImage, jsonutils.NewString(fmt.Sprintf("fail to real delete image %s", images[i].GetId())))
+			task.taskFailed(ctx, guestImage, jsonutils.NewString(fmt.Sprintf("fail to set image %s status ", image.GetId())))
+			return
+		}
+		err = image.RealDelete(ctx, task.UserCred)
+		if err != nil {
+			task.taskFailed(ctx, guestImage, jsonutils.NewString(fmt.Sprintf("fail to real delete image %s", image.GetId())))
 			return
 		}
 	}