CSI snapshots of an erasure-coded-backed volume stores snapshot data in the metadata pool

[Champ-Goblem]

For reference, I asked about the following in the Ceph Slack: https://ceph-storage.slack.com/archives/C05522L7P60/p1723555634369239

Describe the bug

When creating snapshots of erasure-coded RBD volumes the data is stored in the replicated metadata pool.

From the investigation into the ceph-csi code, the data pool option is not set during CreateSnapshot->GenVolFromVolID.
This instance of rbdVolume is passed to doSnapshotClone, which calls createRBDClone.
During the clone process:

• a snapshot is created of the original volume
• a new rbd image is created from the snapshot of the original image <- issue happens here
• the original snapshot is deleted

cloneRbdImageFromSnapshot copies the RBD image options from the cloneRbdVol, created based on the rbdVolume generated from GenVolFromVolID. These options are passed to librbd.CloneImage and thus copies the data into the erasure-coded metadata pool, rather than keeping the data in the original erasure-coded pool.

This can be seen when inspecting the image in rbd with rbd info, the image is missing the data_pool field:

rbd info -p ec-metadatapool-us-east-1b csi-snap-12f5524f-de0d-4c21-bc4f-af843960337b
rbd image 'csi-snap-12f5524f-de0d-4c21-bc4f-af843960337b':
	size 30 GiB in 7680 objects
	order 22 (4 MiB objects)
	snapshot_count: 1
	id: 1933d7e8dc4d58
	block_name_prefix: rbd_data.1933d7e8dc4d58
	format: 2
	features: layering, deep-flatten, operations
	op_features: clone-child
	flags: 
	create_timestamp: Sat Aug 10 09:40:32 2024
	access_timestamp: Sat Aug 10 09:40:32 2024
	modify_timestamp: Sat Aug 10 09:40:32 2024
	parent: ec-metadatapool-us-east-1b/csi-vol-5aea1d4e-6575-492e-ad9d-f1c378d9f21c@a0684712-eadf-40b9-8d33-e46711a19fc2
	overlap: 30 GiB

Compare this to the original image:

rbd image 'csi-vol-5aea1d4e-6575-492e-ad9d-f1c378d9f21c':
	size 30 GiB in 7680 objects
	order 22 (4 MiB objects)
	snapshot_count: 88
	id: 17e84e87be3a0c
	data_pool: ec-blockpool-us-east-1b
	block_name_prefix: rbd_data.17.17e84e87be3a0c
	format: 2
	features: layering, exclusive-lock, object-map, fast-diff, deep-flatten, data-pool, operations
	op_features: clone-parent, snap-trash
	flags: 
	create_timestamp: Thu Aug  8 14:44:59 2024
	access_timestamp: Mon Aug 12 10:24:16 2024
	modify_timestamp: Thu Aug  8 14:44:59 2024

Steps to reproduce

Steps to reproduce the behavior: Snapshot an erasure-coded volume

Expected behavior

The snapshot should remain within the original erasure-coded pool and not have data copied to the replicated metadata pool.

[Champ-Goblem]

I have temporarily patched this problem internally so we can test if the change helps with our OSD disk usage.
I have generated a patch for the changes I made and I will upload it here for reference, in case it helps with the final fix.

From f3262f9fd84237c8617cd78f2a8792b871bc9ff2 Mon Sep 17 00:00:00 2001
From: Champ-Goblem <cameron@northflank.com>
Date: Wed, 14 Aug 2024 12:13:10 +0100
Subject: [PATCH] csi-snapshot: fetch data-pool info from rbd_get_data_pool_id
 api call

---
 internal/rbd/controllerserver.go          |  2 ++
 internal/rbd/rbd_util.go                  | 23 +++++++++++++++++++++++
 vendor/github.com/ceph/go-ceph/rbd/rbd.go |  8 ++++++++
 3 files changed, 33 insertions(+)

diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go
index 4f07ce5f0..8e7d7b301 100644
--- a/internal/rbd/controllerserver.go
+++ b/internal/rbd/controllerserver.go
@@ -1328,6 +1328,8 @@ func (cs *ControllerServer) doSnapshotClone(
 	f := []string{librbd.FeatureNameLayering, librbd.FeatureNameDeepFlatten}
 	cloneRbd.ImageFeatureSet = librbd.FeatureSetFromNames(f)
 
+	cloneRbd.DataPool = parentVol.DataPool
+
 	err := cloneRbd.Connect(cr)
 	if err != nil {
 		return cloneRbd, err
diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go
index 31b330b4d..ad3e07ce1 100644
--- a/internal/rbd/rbd_util.go
+++ b/internal/rbd/rbd_util.go
@@ -561,6 +561,17 @@ func (ri *rbdImage) isInUse() (bool, error) {
 	return len(watchers) > defaultWatchers, nil
 }
 
+func (ri *rbdImage) GetDataPoolId() (int64, error) {
+	image, err := ri.open()
+	if err != nil {
+		return -1, err
+	}
+	defer image.Close()
+
+	id, err := image.GetDataPoolId()
+	return id, err
+}
+
 // checkValidImageFeatures check presence of imageFeatures parameter. It returns false when
 // there imageFeatures is present and empty.
 func checkValidImageFeatures(imageFeatures string, ok bool) bool {
@@ -1158,6 +1169,18 @@ func generateVolumeFromVolumeID(
 	}
 	err = rbdVol.getImageInfo()
 
+	dataPoolId, getDataPoolErr := rbdVol.GetDataPoolId()
+	if dataPoolId != util.InvalidPoolID && getDataPoolErr == nil {
+		dataPoolName, getDataPoolErr := util.GetPoolName(rbdVol.Monitors, cr, dataPoolId)
+		if getDataPoolErr == nil {
+			rbdVol.DataPool = dataPoolName
+		} else {
+			log.ErrorLog(ctx, "failed to get data pool name: %w", getDataPoolErr)
+		}
+	} else if getDataPoolErr != nil {
+		log.ErrorLog(ctx, "failed to get data pool id: %w", getDataPoolErr)
+	}
+
 	return rbdVol, err
 }
 
diff --git a/vendor/github.com/ceph/go-ceph/rbd/rbd.go b/vendor/github.com/ceph/go-ceph/rbd/rbd.go
index b05b14397..2dea6c02f 100644
--- a/vendor/github.com/ceph/go-ceph/rbd/rbd.go
+++ b/vendor/github.com/ceph/go-ceph/rbd/rbd.go
@@ -1008,6 +1008,14 @@ func (image *Image) SetSnapshot(snapname string) error {
 	return getError(C.rbd_snap_set(image.image, cSnapName))
 }
 
+func (image *Image) GetDataPoolId() (int64, error) {
+	if err := image.validate(imageIsOpen); err != nil {
+		return -1, err
+	}
+
+	return int64(C.rbd_get_data_pool_id(image.image)), nil
+}
+
 // GetTrashList returns a slice of TrashInfo structs, containing information about all RBD images
 // currently residing in the trash.
 func GetTrashList(ioctx *rados.IOContext) ([]TrashInfo, error) {
-- 
2.46.0

[Champ-Goblem]

@iPraveenParihar any update on this?

[iPraveenParihar]

@Champ-Goblem, Thanks for opening up the issue.

Since DataPool is not set (should be in generateVolumeFromVolumeID()?) when doing snapshot/clone operations the images will not be in the required data pool.

Can we use GetString(RbdImageOptionDataPool) to get datapool value as we are setting the datapool value when creating an image?

ceph-csi/internal/rbd/rbd_util.go

Lines 1553 to 1559 in e19c289

	if rv.DataPool != "" {
	logMsg += ", data pool %s" + rv.DataPool
	err = options.SetString(librbd.RbdImageOptionDataPool, rv.DataPool)
	if err != nil {
	return fmt.Errorf("failed to set data pool: %w", err)
	}
	}

@nixpanic WDYT?

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[Champ-Goblem]

Is it possible to prioritise this change - we are looking to start upgrading our Ceph clusters in the next month and ideally this fix is part of that release

[iPraveenParihar]

Hey @Champ-Goblem,
I did get a chance to verify the issue -

bash-5.1$ rbd ls -p $p
csi-snap-27b1facb-0db5-4aa2-9fe3-4738e1cd051d
csi-snap-f3ffc3de-e183-4865-8a17-73ac69b4fd26
csi-vol-cba90bc4-3137-47fe-a0b5-d8ec92191034

bash-5.1$ rbd info $p/csi-vol-cba90bc4-3137-47fe-a0b5-d8ec92191034
rbd image 'csi-vol-cba90bc4-3137-47fe-a0b5-d8ec92191034':
	size 1 GiB in 256 objects
	order 22 (4 MiB objects)
	snapshot_count: 1
	id: 3a51669e0ecd
	data_pool: ec-pool
	block_name_prefix: rbd_data.1.3a51669e0ecd
	format: 2
	features: layering, exclusive-lock, object-map, fast-diff, deep-flatten, data-pool, operations
	op_features: clone-parent, snap-trash
	flags:
	create_timestamp: Thu Feb  6 14:40:13 2025
	access_timestamp: Thu Feb  6 15:07:44 2025
	modify_timestamp: Thu Feb  6 14:40:13 2025

bash-5.1$ rados ls -p ec-pool | grep "rbd_data.1.3a51669e0ecd"
rbd_data.1.3a51669e0ecd.0000000000000060
rbd_data.1.3a51669e0ecd.0000000000000004
rbd_data.1.3a51669e0ecd.0000000000000000
rbd_data.1.3a51669e0ecd.0000000000000080
rbd_data.1.3a51669e0ecd.00000000000000a0
rbd_data.1.3a51669e0ecd.00000000000000e0
rbd_data.1.3a51669e0ecd.0000000000000020
	
bash-5.1$ rbd children $p/csi-vol-cba90bc4-3137-47fe-a0b5-d8ec92191034
ocs-storagecluster-cephblockpool/csi-snap-f3ffc3de-e183-4865-8a17-73ac69b4fd26

bash-5.1$ rbd info $p/csi-snap-27b1facb-0db5-4aa2-9fe3-4738e1cd051d
rbd image 'csi-snap-27b1facb-0db5-4aa2-9fe3-4738e1cd051d':
	size 1 GiB in 256 objects
	order 22 (4 MiB objects)
	snapshot_count: 1
	id: 3a51ad764093
	block_name_prefix: rbd_data.3a51ad764093
	format: 2
	features: layering, deep-flatten, operations
	op_features: clone-parent
	flags:
	create_timestamp: Thu Feb  6 14:49:26 2025
	access_timestamp: Thu Feb  6 14:49:26 2025
	modify_timestamp: Thu Feb  6 14:49:26 2025
	
bash-5.1$ rados ls -p $p | grep "rbd_data.3a51ad764093"
rbd_data.3a51ad764093.00000000000000e0
rbd_data.3a51ad764093.0000000000000004
rbd_data.3a51ad764093.0000000000000060
rbd_data.3a51ad764093.0000000000000000
rbd_data.3a51ad764093.0000000000000020
rbd_data.3a51ad764093.0000000000000080
rbd_data.3a51ad764093.00000000000000a0

built a image from your changes, and it looks good

bash-5.1$ rbd info $p/csi-vol-435b8f40-9baf-4951-82e9-aa8118ed1378
rbd image 'csi-vol-435b8f40-9baf-4951-82e9-aa8118ed1378':
	size 1 GiB in 256 objects
	order 22 (4 MiB objects)
	snapshot_count: 3
	id: 6a8bf8e2cf03
	data_pool: ec-pool
	block_name_prefix: rbd_data.1.6a8bf8e2cf03
	format: 2
	features: layering, exclusive-lock, object-map, fast-diff, deep-flatten, data-pool, operations
	op_features: clone-parent, snap-trash
	flags:
	create_timestamp: Fri Feb  7 06:41:53 2025
	access_timestamp: Fri Feb  7 06:45:52 2025
	modify_timestamp: Fri Feb  7 06:41:53 2025
	
bash-5.1$ rbd info $p/csi-snap-38007f65-3ed5-4176-81dc-4a535dc56172
rbd image 'csi-snap-38007f65-3ed5-4176-81dc-4a535dc56172':
	size 1 GiB in 256 objects
	order 22 (4 MiB objects)
	snapshot_count: 1
	id: 6bbdc01977b4
	data_pool: ec-pool
	block_name_prefix: rbd_data.1.6bbdc01977b4
	format: 2
	features: layering, deep-flatten, data-pool
	op_features:
	flags:
	create_timestamp: Fri Feb  7 06:45:37 2025
	access_timestamp: Fri Feb  7 06:45:37 2025
	modify_timestamp: Fri Feb  7 06:45:37 2025

bash-5.1$ rados ls -p ec-pool | grep "rbd_data.1.6bbdc01977b4"
rbd_data.1.6bbdc01977b4.0000000000000020
rbd_data.1.6bbdc01977b4.00000000000000e0
rbd_data.1.6bbdc01977b4.0000000000000080
rbd_data.1.6bbdc01977b4.0000000000000060
rbd_data.1.6bbdc01977b4.0000000000000000
rbd_data.1.6bbdc01977b4.00000000000000a0
rbd_data.1.6bbdc01977b4.0000000000000004

would you like to raise a PR for this 😄? Also a PR in go-ceph for this change.

diff --git a/vendor/github.com/ceph/go-ceph/rbd/rbd.go b/vendor/github.com/ceph/go-ceph/rbd/rbd.go
index b05b14397..2dea6c02f 100644
--- a/vendor/github.com/ceph/go-ceph/rbd/rbd.go
+++ b/vendor/github.com/ceph/go-ceph/rbd/rbd.go
@@ -1008,6 +1008,14 @@ func (image *Image) SetSnapshot(snapname string) error {
 	return getError(C.rbd_snap_set(image.image, cSnapName))
 }
 
+func (image *Image) GetDataPoolId() (int64, error) {
+	if err := image.validate(imageIsOpen); err != nil {
+		return -1, err
+	}
+
+	return int64(C.rbd_get_data_pool_id(image.image)), nil
+}
+