Merge pull request #80 from cybozu-go/use-trash-mv

use trash to remove RBD images asynchronously

internal/ceph/ceph.go

@@ -5,10 +5,15 @@ import (
 	"time"
 )
 
-type RBDInfo struct {
-	ParentPool  string
-	ParentImage string
-	ParentSnap  string
+type RBDImageInfo struct {
+	ID     string              `json:"id"`
+	Parent *RBDImageInfoParent `json:"parent,omitempty"`
+}
+
+type RBDImageInfoParent struct {
+	Pool     string `json:"pool"`
+	Image    string `json:"image"`
+	Snapshot string `json:"snapshot"`
 }
 
 type RBDTimeStamp struct {
@@ -35,9 +40,11 @@ type RBDSnapshot struct {
 
 type CephCmd interface {
 	RBDClone(pool, srcImage, srcSnap, dstImage, features string) error
-	RBDInfo(pool, image string) (*RBDInfo, error)
+	RBDInfo(pool, image string) (*RBDImageInfo, error)
 	RBDLs(pool string) ([]string, error)
 	RBDRm(pool, image string) error
+	RBDTrashMv(pool, image string) error
+	CephRBDTaskAddTrashRemove(pool, image string) error
 	RBDSnapCreate(pool, image, snap string) error
 	RBDSnapLs(pool, image string) ([]RBDSnapshot, error)
 	RBDSnapRm(pool, image, snap string) error

internal/ceph/rbd.go

@@ -5,16 +5,6 @@ import (
 	"fmt"
 )
 
-type rbdInfoParentJS struct {
-	Pool     string `json:"pool"`
-	Image    string `json:"image"`
-	Snapshot string `json:"snapshot"`
-}
-
-type rbdInfoJS struct {
-	Parent *rbdInfoParentJS `json:"parent,omitempty"`
-}
-
 // RBDClone clones an RBD image from a snapshot with specified features.
 func (c *cephCmdImpl) RBDClone(pool, srcImage, srcSnap, dstImage, features string) error {
 	src := fmt.Sprintf("%s/%s@%s", pool, srcImage, srcSnap)
@@ -32,28 +22,18 @@ func (c *cephCmdImpl) RBDClone(pool, srcImage, srcSnap, dstImage, features strin
 }
 
 // RBDInfo gets information about an RBD image.
-func (c *cephCmdImpl) RBDInfo(pool, image string) (*RBDInfo, error) {
+func (c *cephCmdImpl) RBDInfo(pool, image string) (*RBDImageInfo, error) {
 	out, err := c.command.execute("rbd", "info", "--format", "json", fmt.Sprintf("%s/%s", pool, image))
 	if err != nil {
 		return nil, fmt.Errorf("failed to get RBD info: %v", err)
 	}
 
-	infoJS := &rbdInfoJS{}
-	err = json.Unmarshal(out, infoJS)
+	imageInfo := &RBDImageInfo{}
+	err = json.Unmarshal(out, imageInfo)
 	if err != nil {
 		return nil, fmt.Errorf("failed to unmarshal RBD info: %v", err)
 	}
-
-	if infoJS.Parent == nil {
-		return nil, fmt.Errorf("RBD info parent field is empty")
-	}
-	info := &RBDInfo{
-		ParentPool:  infoJS.Parent.Pool,
-		ParentImage: infoJS.Parent.Image,
-		ParentSnap:  infoJS.Parent.Snapshot,
-	}
-
-	return info, nil
+	return imageInfo, nil
 }
 
 // RBDLs lists RBD images in a pool.
@@ -82,6 +62,24 @@ func (c *cephCmdImpl) RBDRm(pool, image string) error {
 	return nil
 }
 
+// RBDTrashMv removes an RBD image asynchronously.
+func (c *cephCmdImpl) RBDTrashMv(pool, image string) error {
+	_, err := c.command.execute("rbd", "trash", "mv", fmt.Sprintf("%s/%s", pool, image))
+	if err != nil {
+		return fmt.Errorf("failed to move RBD image to trash: %w", err)
+	}
+	return nil
+}
+
+// CephRBDTaskTrashRemove adds a task to remove the image from trash.
+func (c *cephCmdImpl) CephRBDTaskAddTrashRemove(pool, imageID string) error {
+	_, err := c.command.execute("ceph", "rbd", "task", "add", "trash", "remove", fmt.Sprintf("%s/%s", pool, imageID))
+	if err != nil {
+		return fmt.Errorf("failed to add task to remove the image from trash: %w", err)
+	}
+	return nil
+}
+
 // RBDSnapCreate creates an RBD snapshot.
 func (c *cephCmdImpl) RBDSnapCreate(pool, image, snap string) error {
 	_, err := c.command.execute("rbd", "snap", "create", fmt.Sprintf("%s/%s@%s", pool, image, snap))

internal/ceph/rbd_test.go

@@ -97,9 +97,10 @@ var _ = Describe("CephCmd.RBDInfo", func() {
 		cmd := mockedCephCmd(m)
 		info, err := cmd.RBDInfo("pool", "image")
 		Expect(err).ToNot(HaveOccurred())
-		Expect(info.ParentPool).To(Equal("pool"))
-		Expect(info.ParentImage).To(Equal("csi-vol-39ca122a-88e1-44b6-aa2b-cae64fb383db"))
-		Expect(info.ParentSnap).To(Equal("test-snap"))
+		Expect(info.Parent).NotTo(BeNil())
+		Expect(info.Parent.Pool).To(Equal("pool"))
+		Expect(info.Parent.Image).To(Equal("csi-vol-39ca122a-88e1-44b6-aa2b-cae64fb383db"))
+		Expect(info.Parent.Snapshot).To(Equal("test-snap"))
 	})
 
 	It("should return an error, if the command failed", func() {

internal/controller/internal/testutil/fake_rbd.go

@@ -26,7 +26,7 @@ func (f *fakeRBD) RBDClone(pool, srcImage, srcSnap, dstImage, features string) e
 	return nil
 }
 
-func (f *fakeRBD) RBDInfo(pool, image string) (*ceph.RBDInfo, error) {
+func (f *fakeRBD) RBDInfo(pool, image string) (*ceph.RBDImageInfo, error) {
 	return nil, nil
 }
 
@@ -38,6 +38,14 @@ func (f *fakeRBD) RBDRm(pool, image string) error {
 	return nil
 }
 
+func (f *fakeRBD) RBDTrashMv(pool, image string) error {
+	return nil
+}
+
+func (f *fakeRBD) CephRBDTaskAddTrashRemove(pool, image string) error {
+	return nil
+}
+
 func (f *fakeRBD) RBDSnapCreate(pool, image, snap string) error {
 	key := pool + "/" + image
 

internal/controller/mantlerestore_controller.go

@@ -224,8 +224,11 @@ func (r *MantleRestoreReconciler) cloneImageFromBackup(ctx context.Context, rest
 		if err != nil {
 			return fmt.Errorf("failed to get RBD info: %v", err)
 		}
+		if info.Parent == nil {
+			return fmt.Errorf("failed to get RBD info: parent field is empty")
+		}
 
-		if info.ParentPool == restore.Status.Pool && info.ParentImage == bkImage && info.ParentSnap == backup.Name {
+		if info.Parent.Pool == restore.Status.Pool && info.Parent.Image == bkImage && info.Parent.Snapshot == backup.Name {
 			logger.Info("image already exists", "image", r.restoringRBDImageName(restore))
 			return nil
 		} else {

internal/controller/persistentvolume_controller.go

@@ -132,12 +132,25 @@ func (r *PersistentVolumeReconciler) removeRBDImage(ctx context.Context, pv *cor
 
 	images, err := r.ceph.RBDLs(pool)
 	if err != nil {
-		return fmt.Errorf("failed to list RBD images: %v", err)
+		return fmt.Errorf("failed to list RBD images: %w", err)
 	}
 
 	if !slices.Contains(images, image) {
 		return nil
 	}
 
-	return r.ceph.RBDRm(pool, image)
+	imageInfo, err := r.ceph.RBDInfo(pool, image)
+	if err != nil {
+		return fmt.Errorf("failed to get info about the RBD image: %s/%s: %w", pool, image, err)
+	}
+
+	if err := r.ceph.RBDTrashMv(pool, image); err != nil {
+		return fmt.Errorf("failed to move the RBD image to trash: %s/%s: %w", pool, image, err)
+	}
+
+	if err := r.ceph.CephRBDTaskAddTrashRemove(pool, imageInfo.ID); err != nil {
+		return fmt.Errorf("failed to add task to remove the RBD image from trash: %s/%s: %w", pool, image, err)
+	}
+
+	return nil
 }