Repository: ceph/ceph-csi
Pull request #2134 (2021-06-04 11:30:25)

rbd: use DeepCopy to create a thick-provisioned clone
To create a full-allocated RBD image from a snapshot/clone DeepCopy()
can be used. This is needed when the parent of the new volume is
thick-provisioner, so that the new volume is independent of the parent
and thick-provisioned as well.

Things that need to be done:

- [x] manually tested and confirmed allocations with `rbd du`
- [x] rebase on top of the next go-ceph release (v0.10.0 #2147)
- [x] add e2e verification test

Fixes: #2077

---

<details>
<summary>Show available bot commands</summary>

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

- `/retest ci/centos/<job-name>`: retest the `<job-name>` after unrelated
  failure (please report the failure too!)
- `/retest all`: run this in case the CentOS CI failed to start/report any test
  progress or results

</details>


nixpanic: This PR depends on #2147, once that is merged, the **[DNM]** label can be dropped.
nixpanic: @mergifyio rebase
mergify[bot]: **Command `rebase`: success**
> **Branch has been successfully rebased**
> 

Madhu-1: tested it locally its failed to mount the clone PVC to application pod
```log
Events:
  Type     Reason                  Age                   From                     Message
  ----     ------                  ----                  ----                     -------
  Normal   Scheduled               5m29s                 default-scheduler        Successfully assigned default/csirbd-demo-pod to minicluster2
  Normal   SuccessfulAttachVolume  5m29s                 attachdetach-controller  AttachVolume.Attach succeeded for volume "pvc-16c4ed55-9f7a-487a-8040-caf9c56475f2"
  Warning  FailedMount             3m26s                 kubelet                  Unable to attach or mount volumes: unmounted volumes=[mypvc], unattached volumes=[mypvc default-token-glr45]: timed out waiting for the condition
  Warning  FailedMount             72s                   kubelet                  Unable to attach or mount volumes: unmounted volumes=[mypvc], unattached volumes=[default-token-glr45 mypvc]: timed out waiting for the condition
  Warning  FailedMount             57s (x10 over 5m13s)  kubelet                  MountVolume.MountDevice failed for volume "pvc-16c4ed55-9f7a-487a-8040-caf9c56475f2" : rpc error: code = Internal desc = an error (exit status 2) occurred while running ceph args: [rbd task add flatten replicapool/csi-vol-fdb11c26-cd8f-11eb-836d-0242ac110005 --id csi-rbd-node --keyfile=***stripped*** -m 192.168.50.61:6789]
```
Madhu-1: When No PVC is present on the kubernetes the images exists in the trash
and the ceph cluster is still using 16 Gib (PVC size was 15 Gib)

```
[root@dhcp53-170 rbd]# kubectl delete -f pvc-clone.yaml 
persistentvolumeclaim "rbd-pvc-clone" deleted
[root@dhcp53-170 rbd]# kubectl delete -f pvc.yaml 
persistentvolumeclaim "rbd-pvc" deleted
[root@dhcp53-170 rbd]# kubectl get pvc
No resources found in default namespace.
[root@dhcp53-170 rbd]# kubectl get pv
No resources found
```
```
# ceph -s
  cluster:
    id:     d6d71dda-7470-418a-9186-e63e55f71efc
    health: HEALTH_WARN
            mon is allowing insecure global_id reclaim
 
  services:
    mon: 1 daemons, quorum a (age 18m)
    mgr: a(active, since 18m)
    osd: 1 osds: 1 up (since 18m), 1 in (since 18m)
 
  data:
    pools:   2 pools, 33 pgs
    objects: 3.85k objects, 15 GiB
    usage:   16 GiB used, 34 GiB / 50 GiB avail
    pgs:     33 active+clean
 
  io:
    client:   681 B/s rd, 0 op/s rd, 0 op/s wr
 
sh-4.4# rbd ls --pool=replicapool
sh-4.4# 
sh-4.4# rbd trash ls --pool=replicapool
10be52dc3ad6 csi-vol-fdb11c26-cd8f-11eb-836d-0242ac110005-temp
10be534682a8 csi-vol-d6a94576-cd8f-11eb-836d-0242ac110005
```


Madhu-1: when provisioner pod is restarted when Thick clone is getting created the thick clone will not consume expected size on ceph cluster

```
[root@dhcp53-170 rbd]# kubectl get pvc
NAME            STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS      AGE
rbd-pvc         Bound    pvc-6ce1452f-689d-4903-a6b4-af4720151818   15Gi       RWO            rook-ceph-block   69s
rbd-pvc-clone   Bound    pvc-caff5817-86a2-45df-843f-19ad8cd81e8a   15Gi       RWO            rook-ceph-block   37s

sh-4.4# rbd du replicapool/csi-vol-d791f254-cd92-11eb-8352-0242ac110005
warning: fast-diff map is not enabled for csi-vol-d791f254-cd92-11eb-8352-0242ac110005. operation may be slow.
NAME                                          PROVISIONED  USED   
csi-vol-d791f254-cd92-11eb-8352-0242ac110005       15 GiB  936 MiB
sh-4.4# rbd du replicapool/csi-vol-c477b45f-cd92-11eb-8352-0242ac110005
warning: fast-diff map is not enabled for csi-vol-c477b45f-cd92-11eb-8352-0242ac110005. operation may be slow.
NAME                                          PROVISIONED  USED  
csi-vol-c477b45f-cd92-11eb-8352-0242ac110005       15 GiB  15 GiB
sh-4.4# rbd info replicapool/csi-vol-c477b45f-cd92-11eb-8352-0242ac110005
rbd image 'csi-vol-c477b45f-cd92-11eb-8352-0242ac110005':
	size 15 GiB in 3840 objects
	order 22 (4 MiB objects)
	snapshot_count: 1
	id: 124edb32d808
	block_name_prefix: rbd_data.124edb32d808
	format: 2
	features: layering, operations
	op_features: clone-parent, snap-trash
	flags: 
	create_timestamp: Tue Jun 15 04:33:03 2021
	access_timestamp: Tue Jun 15 04:33:03 2021
	modify_timestamp: Tue Jun 15 04:33:03 2021
sh-4.4# rbd info replicapool/csi-vol-d791f254-cd92-11eb-8352-0242ac110005
rbd image 'csi-vol-d791f254-cd92-11eb-8352-0242ac110005':
	size 15 GiB in 3840 objects
	order 22 (4 MiB objects)
	snapshot_count: 0
	id: 124e9cae7648
	block_name_prefix: rbd_data.124e9cae7648
	format: 2
	features: layering, deep-flatten
	op_features: 
	flags: 
	create_timestamp: Tue Jun 15 04:33:33 2021
	access_timestamp: Tue Jun 15 04:33:33 2021
	modify_timestamp: Tue Jun 15 04:33:33 2021
sh-4.4# 

```
nixpanic: > when provisioner pod is restarted when Thick clone is getting created the thick clone will not consume expected size on ceph cluster

In this case, `DeepCopy()` was aborted and needs to be restarted. This is still TODO at the moment.
Madhu-1: > > when provisioner pod is restarted when Thick clone is getting created the thick clone will not consume expected size on ceph cluster
> 
> In this case, `DeepCopy()` was aborted and needs to be restarted. This is still TODO at the moment.

:+1: This will be addressed in the next PR?
nixpanic: On Wed, Jun 16, 2021 at 05:19:42AM -0700, Humble Devassy Chirammal wrote:
> @humblec commented on this pull request.
> 
> 
> 
> >  		if err != nil {
> -			return fmt.Errorf("failed to copy encryption config for %q: %w", rv, err)
> +			return fmt.Errorf("failed to deep copy %q into %q: %w", parentVol, rv, err)
> +		}
> +	} else {
> +		// flatten clone
> +		errFlatten = tempClone.flattenRbdImage(ctx, rv.conn.Creds, false, rbdHardMaxCloneDepth, rbdSoftMaxCloneDepth)
> +		if errFlatten != nil {
> 
> @nixpanic if `Flatten` failed here, do we need to return error ? or can we move ahead without failing? I understand,  this is normal/general workflow we have so far/till now, so ideally should keep the same. But double confirming, is it mandatory to flatten before creating a snap from temporary clone image?. 

Sorry, I am not sure what you are asking...

The previous behaviour is not changed for non-thick-provisioned cloning.
Is that something you are concerned about?

For thick-provisioned cloning, the DeepCopy() function is expected to do
the flattening. I might have understood that incorrectly, as it seems
the 'deep-flatten' option is still enabled once DeepCopy() has finished.
In that case, additional flattening might still be required?


nixpanic: > > > when provisioner pod is restarted when Thick clone is getting created the thick clone will not consume expected size on ceph cluster
> > 
> > 
> > In this case, `DeepCopy()` was aborted and needs to be restarted. This is still TODO at the moment.
> 
> +1 This will be addressed in the next PR?

I am including that in this PR, with an additional commit.
nixpanic: Please review again :pray: Manual tests now succeed (including provisioner restart while cloning).
humblec: @nixpanic LGTM, a minor comment from me, but that can be ignored too.

Patch:
diff --git a/internal/rbd/clone.go b/internal/rbd/clone.go
index 1f4fb16a0..d7e77ba08 100644
--- a/internal/rbd/clone.go
+++ b/internal/rbd/clone.go
@@ -21,7 +21,6 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/ceph/ceph-csi/internal/journal"
 	"github.com/ceph/ceph-csi/internal/util"
 
 	librbd "github.com/ceph/go-ceph/rbd"
@@ -147,6 +146,57 @@ func (rv *rbdVolume) generateTempClone() *rbdVolume {
 }
 
 func (rv *rbdVolume) createCloneFromImage(ctx context.Context, parentVol *rbdVolume) error {
+	j, err := volJournal.Connect(rv.Monitors, rv.RadosNamespace, rv.conn.Creds)
+	if err != nil {
+		return status.Error(codes.Internal, err.Error())
+	}
+	defer j.Destroy()
+
+	err = rv.doSnapClone(ctx, parentVol)
+	if err != nil {
+		return status.Error(codes.Internal, err.Error())
+	}
+
+	err = rv.getImageID()
+	if err != nil {
+		util.ErrorLog(ctx, "failed to get volume id %s: %v", rv, err)
+		return err
+	}
+
+	if parentVol.isEncrypted() {
+		err = parentVol.copyEncryptionConfig(&rv.rbdImage)
+		if err != nil {
+			return fmt.Errorf("failed to copy encryption config for %q: %w", rv, err)
+		}
+	}
+
+	// TODO: copy thick provision config
+	thick, err := parentVol.isThickProvisioned()
+	if err != nil {
+		return fmt.Errorf("failed checking thick-provisioning of %q: %w", parentVol, err)
+	}
+
+	if thick {
+		err = rv.setThickProvisioned()
+		if err != nil {
+			return fmt.Errorf("failed mark %q thick-provisioned: %w", rv, err)
+		}
+	}
+
+	err = j.StoreImageID(ctx, rv.JournalPool, rv.ReservedID, rv.ImageID)
+	if err != nil {
+		util.ErrorLog(ctx, "failed to store volume %s: %v", rv, err)
+		return err
+	}
+	return nil
+}
+
+func (rv *rbdVolume) doSnapClone(ctx context.Context, parentVol *rbdVolume) error {
+	var (
+		errClone   error
+		errFlatten error
+	)
+
 	// generate temp cloned volume
 	tempClone := rv.generateTempClone()
 	// snapshot name is same as temporary cloned image, This helps to
@@ -160,21 +210,8 @@ func (rv *rbdVolume) createCloneFromImage(ctx context.Context, parentVol *rbdVol
 	cloneSnap.RbdSnapName = rv.RbdImageName
 	cloneSnap.Pool = rv.Pool
 
-	var (
-		errClone   error
-		errFlatten error
-		err        error
-	)
-	var j = &journal.Connection{}
-
-	j, err = volJournal.Connect(rv.Monitors, rv.RadosNamespace, rv.conn.Creds)
-	if err != nil {
-		return status.Error(codes.Internal, err.Error())
-	}
-	defer j.Destroy()
-
 	// create snapshot and temporary clone and delete snapshot
-	err = createRBDClone(ctx, parentVol, tempClone, tempSnap, rv.conn.Creds)
+	err := createRBDClone(ctx, parentVol, tempClone, tempSnap, rv.conn.Creds)
 	if err != nil {
 		return err
 	}
@@ -197,41 +234,39 @@ func (rv *rbdVolume) createCloneFromImage(ctx context.Context, parentVol *rbdVol
 			}
 		}
 	}()
-	// flatten clone
-	errFlatten = tempClone.flattenRbdImage(ctx, rv.conn.Creds, false, rbdHardMaxCloneDepth, rbdSoftMaxCloneDepth)
-	if errFlatten != nil {
-		return errFlatten
-	}
-	// create snap of temp clone from temporary cloned image
-	// create final clone
-	// delete snap of temp clone
-	errClone = createRBDClone(ctx, tempClone, rv, cloneSnap, rv.conn.Creds)
-	if errClone != nil {
-		// set errFlatten error to cleanup temporary snapshot and temporary clone
-		errFlatten = errors.New("failed to create user requested cloned image")
-		return errClone
-	}
-	err = rv.getImageID()
-	if err != nil {
-		util.ErrorLog(ctx, "failed to get volume id %s: %v", rv, err)
-		return err
-	}
 
-	if parentVol.isEncrypted() {
-		err = parentVol.copyEncryptionConfig(&rv.rbdImage)
+	if rv.ThickProvision {
+		err = tempClone.DeepCopy(rv)
 		if err != nil {
-			return fmt.Errorf("failed to copy encryption config for %q: %w", rv, err)
+			return fmt.Errorf("failed to deep copy %q into %q: %w", parentVol, rv, err)
+		}
+	} else {
+		// flatten clone
+		errFlatten = tempClone.flattenRbdImage(ctx, rv.conn.Creds, false, rbdHardMaxCloneDepth, rbdSoftMaxCloneDepth)
+		if errFlatten != nil {
+			return errFlatten
+		}
+
+		// create snap of temp clone from temporary cloned image
+		// create final clone
+		// delete snap of temp clone
+		errClone = createRBDClone(ctx, tempClone, rv, cloneSnap, rv.conn.Creds)
+		if errClone != nil {
+			// set errFlatten error to cleanup temporary snapshot and temporary clone
+			errFlatten = errors.New("failed to create user requested cloned image")
+			return errClone
 		}
 	}
-	err = j.StoreImageID(ctx, rv.JournalPool, rv.ReservedID, rv.ImageID)
-	if err != nil {
-		util.ErrorLog(ctx, "failed to store volume %s: %v", rv, err)
-		return err
-	}
+
 	return nil
 }
 
 func (rv *rbdVolume) flattenCloneImage(ctx context.Context) error {
+	if rv.ThickProvision {
+		// thick-provisioned images do not need flattening
+		return nil
+	}
+
 	tempClone := rv.generateTempClone()
 	// reducing the limit for cloned images to make sure the limit is in range,
 	// If the intermediate clone reaches the depth we may need to return ABORT
diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go
index 2fd0e2fb7..b20c439de 100644
--- a/internal/rbd/controllerserver.go
+++ b/internal/rbd/controllerserver.go
@@ -117,7 +117,7 @@ func (cs *ControllerServer) parseVolCreateRequest(ctx context.Context, req *csi.
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
-	rbdVol.ThickProvision = cs.isThickProvisionRequest(req)
+	rbdVol.ThickProvision = isThickProvisionRequest(req.GetParameters())
 
 	rbdVol.RequestName = req.GetName()
 
@@ -272,7 +272,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	if err != nil {
 		return nil, getGRPCErrorForCreateVolume(err)
 	} else if found {
-		return cs.repairExistingVolume(ctx, req, cr, rbdVol, rbdSnap)
+		return cs.repairExistingVolume(ctx, req, cr, rbdVol, parentVol, rbdSnap)
 	}
 
 	err = checkValidCreateVolumeRequest(rbdVol, parentVol, rbdSnap, cr)
@@ -335,7 +335,7 @@ func flattenParentImage(ctx context.Context, rbdVol *rbdVolume, cr *util.Credent
 // that the state is corrected to what was requested. It is needed to call this
 // when the process of creating a volume was interrupted.
 func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.CreateVolumeRequest,
-	cr *util.Credentials, rbdVol *rbdVolume, rbdSnap *rbdSnapshot) (*csi.CreateVolumeResponse, error) {
+	cr *util.Credentials, rbdVol, parentVol *rbdVolume, rbdSnap *rbdSnapshot) (*csi.CreateVolumeResponse, error) {
 	vcs := req.GetVolumeContentSource()
 
 	switch {
@@ -343,7 +343,7 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C
 	case vcs == nil:
 		// continue/restart allocating the volume in case it
 		// should be thick-provisioned
-		if cs.isThickProvisionRequest(req) {
+		if isThickProvisionRequest(req.GetParameters()) {
 			err := rbdVol.RepairThickProvision()
 			if err != nil {
 				return nil, status.Error(codes.Internal, err.Error())
@@ -364,9 +364,29 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C
 			return nil, err
 		}
 
-	// rbdVol is a clone from an other volume
+	// rbdVol is a clone from parentVol
 	case vcs.GetVolume() != nil:
-		// TODO: is there really nothing to repair on image clone?
+		// When cloning into a thick-provisioned volume was happening,
+		// the image should be marked as thick-provisioned, unless it
+		// was aborted in flight. In order to restart the
+		// thick-cloning, delete the volume and let the caller retry
+		// from the start.
+		if isThickProvisionRequest(req.GetParameters()) {
+			thick, err := rbdVol.isThickProvisioned()
+			if err != nil {
+				return nil, status.Errorf(codes.Aborted, "failed to verify thick-provisioned volume %q: %s", rbdVol, err)
+			} else if !thick {
+				err = cleanUpSnapshot(ctx, parentVol, rbdSnap, rbdVol, cr)
+				if err != nil {
+					return nil, status.Errorf(codes.Aborted, "failed to remove partially cloned volume %q: %s", rbdVol, err)
+				}
+				err = undoVolReservation(ctx, rbdVol, cr)
+				if err != nil {
+					return nil, status.Errorf(codes.Aborted, "failed to remove volume %q from journal: %s", rbdVol, err)
+				}
+				return nil, status.Errorf(codes.Aborted, "cloning thick-provisioned volume %q has been interrupted, please retry", rbdVol)
+			}
+		}
 	}
 
 	return buildCreateVolumeResponse(req, rbdVol), nil
@@ -379,6 +399,11 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C
 // are more than the `minSnapshotOnImage` Add a task to flatten all the
 // temporary cloned images.
 func flattenTemporaryClonedImages(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials) error {
+	if rbdVol.ThickProvision {
+		// thick-provisioned images do not need flattening
+		return nil
+	}
+
 	snaps, err := rbdVol.listSnapshots()
 	if err != nil {
 		if errors.Is(err, ErrImageNotFound) {
@@ -1171,10 +1196,10 @@ func (cs *ControllerServer) ControllerExpandVolume(ctx context.Context, req *csi
 
 // isThickProvisionRequest returns true in case the request contains the
 // `thickProvision` option set to `true`.
-func (cs *ControllerServer) isThickProvisionRequest(req *csi.CreateVolumeRequest) bool {
+func isThickProvisionRequest(parameters map[string]string) bool {
 	tp := "thickProvision"
 
-	thick, ok := req.GetParameters()[tp]
+	thick, ok := parameters[tp]
 	if !ok || thick == "" {
 		return false
 	}
diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go
index 4eaa6271a..1f1ce673b 100644
--- a/internal/rbd/nodeserver.go
+++ b/internal/rbd/nodeserver.go
@@ -171,6 +171,8 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
+	volOptions.ThickProvision = isThickProvisionRequest(req.GetVolumeContext())
+
 	// get rbd image name from the volume journal
 	// for static volumes, the image name is actually the volume ID itself
 	switch {
diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go
index 039109950..f2a6fbbc1 100644
--- a/internal/rbd/rbd_util.go
+++ b/internal/rbd/rbd_util.go
@@ -1508,6 +1508,49 @@ func (rv *rbdVolume) RepairThickProvision() error {
 	return nil
 }
 
+// DeepCopy creates an independent image (dest) from the source image. This
+// process may take some time when the image is large.
+func (rv *rbdVolume) DeepCopy(dest *rbdVolume) error {
+	opts := librbd.NewRbdImageOptions()
+	defer opts.Destroy()
+
+	// when doing DeepCopy, also flatten the new image
+	err := opts.SetUint64(librbd.ImageOptionFlatten, 1)
+	if err != nil {
+		return err
+	}
+
+	err = dest.openIoctx()
+	if err != nil {
+		return err
+	}
+
+	image, err := rv.open()
+	if err != nil {
+		return err
+	}
+	defer image.Close()
+
+	err = image.DeepCopy(dest.ioctx, dest.RbdImageName, opts)
+	if err != nil {
+		return err
+	}
+
+	// deep-flatten is not supported by all clients, so disable it
+	return dest.DisableDeepFlatten()
+}
+
+// DisableDeepFlatten removed the deep-flatten feature from the image.
+func (rv *rbdVolume) DisableDeepFlatten() error {
+	image, err := rv.open()
+	if err != nil {
+		return err
+	}
+	defer image.Close()
+
+	return image.UpdateFeatures(librbd.FeatureDeepFlatten, false)
+}
+
 func (rv *rbdVolume) listSnapshots() ([]librbd.SnapInfo, error) {
 	image, err := rv.open()
 	if err != nil {


Test patch:
diff --git a/e2e/rbd.go b/e2e/rbd.go
index 26abed41c..f65030653 100644
--- a/e2e/rbd.go
+++ b/e2e/rbd.go
@@ -761,7 +761,35 @@ var _ = Describe("RBD", func() {
 			By("create a PVC-PVC clone and bind it to an app", func() {
 				// pvc clone is only supported from v1.16+
 				if k8sVersionGreaterEquals(f.ClientSet, 1, 16) {
-					validatePVCClone(defaultCloneCount, pvcPath, appPath, pvcSmartClonePath, appSmartClonePath, false, f)
+					validatePVCClone(defaultCloneCount, pvcPath, appPath, pvcSmartClonePath, appSmartClonePath, noPVCValidation, f)
+				}
+			})
+
+			By("create a thick-provisioned PVC-PVC clone and bind it to an app", func() {
+				// pvc clone is only supported from v1.16+
+				if !k8sVersionGreaterEquals(f.ClientSet, 1, 16) {
+					Skip("pvc clone is only supported from v1.16+")
+				}
+
+				err := deleteResource(rbdExamplePath + "storageclass.yaml")
+				if err != nil {
+					e2elog.Failf("failed to delete storageclass with error %v", err)
+				}
+				err = createRBDStorageClass(f.ClientSet, f, defaultSCName, nil, map[string]string{
+					"thickProvision": "true"}, deletePolicy)
+				if err != nil {
+					e2elog.Failf("failed to create storageclass with error %v", err)
+				}
+
+				validatePVCClone(1, pvcPath, appPath, pvcSmartClonePath, appSmartClonePath, isThickPVC, f)
+
+				err = deleteResource(rbdExamplePath + "storageclass.yaml")
+				if err != nil {
+					e2elog.Failf("failed to delete storageclass with error %v", err)
+				}
+				err = createRBDStorageClass(f.ClientSet, f, defaultSCName, nil, nil, deletePolicy)
+				if err != nil {
+					e2elog.Failf("failed to create storageclass with error %v", err)
 				}
 			})
 
@@ -813,7 +841,7 @@ var _ = Describe("RBD", func() {
 					e2elog.Failf("failed to create storageclass with error %v", err)
 				}
 
-				validatePVCClone(1, pvcPath, appPath, pvcSmartClonePath, appSmartClonePath, true, f)
+				validatePVCClone(1, pvcPath, appPath, pvcSmartClonePath, appSmartClonePath, isEncryptedPVC, f)
 
 				err = deleteResource(rbdExamplePath + "storageclass.yaml")
 				if err != nil {
@@ -838,7 +866,7 @@ var _ = Describe("RBD", func() {
 				}
 				// pvc clone is only supported from v1.16+
 				if v.Major > "1" || (v.Major == "1" && v.Minor >= "16") {
-					validatePVCClone(defaultCloneCount, rawPvcPath, rawAppPath, pvcBlockSmartClonePath, appBlockSmartClonePath, false, f)
+					validatePVCClone(defaultCloneCount, rawPvcPath, rawAppPath, pvcBlockSmartClonePath, appBlockSmartClonePath, noPVCValidation, f)
 				}
 			})
 			By("create/delete multiple PVCs and Apps", func() {
diff --git a/e2e/rbd_helper.go b/e2e/rbd_helper.go
index c917a77cd..483c73ad1 100644
--- a/e2e/rbd_helper.go
+++ b/e2e/rbd_helper.go
@@ -414,7 +414,13 @@ func validateEncryptedPVCAndAppBinding(pvcPath, appPath, kms string, f *framewor
 	return nil
 }
 
-func validateEncryptedPVC(f *framework.Framework, pvc *v1.PersistentVolumeClaim, app *v1.Pod) error {
+type validateFunc func(f *framework.Framework, pvc *v1.PersistentVolumeClaim, app *v1.Pod) error
+
+// noPVCValidation can be used to pass to validatePVCClone when no extra
+// validation of the PVC is needed.
+var noPVCValidation validateFunc = nil
+
+func isEncryptedPVC(f *framework.Framework, pvc *v1.PersistentVolumeClaim, app *v1.Pod) error {
 	imageData, err := getImageInfoFromPVC(pvc.Namespace, pvc.Name, f)
 	if err != nil {
 		return err
@@ -424,6 +430,16 @@ func validateEncryptedPVC(f *framework.Framework, pvc *v1.PersistentVolumeClaim,
 	return validateEncryptedImage(f, rbdImageSpec, app)
 }
 
+func isThickPVC(f *framework.Framework, pvc *v1.PersistentVolumeClaim, app *v1.Pod) error {
+	du, err := getRbdDu(f, pvc)
+	if err != nil {
+		return fmt.Errorf("failed to get allocations of RBD image: %w", err)
+	} else if du.UsedSize == 0 || du.UsedSize != du.ProvisionedSize {
+		return fmt.Errorf("backing RBD image is not thick-provisioned (%d/%d)", du.UsedSize, du.ProvisionedSize)
+	}
+	return nil
+}
+
 // validateEncryptedImage verifies that the RBD image is encrypted. The
 // following checks are performed:
 // - Metadata of the image should be set with the encryption state;
diff --git a/e2e/utils.go b/e2e/utils.go
index 65dfc3a17..a9ea9c350 100644
--- a/e2e/utils.go
+++ b/e2e/utils.go
@@ -522,7 +522,7 @@ func writeDataAndCalChecksum(app *v1.Pod, opt *metav1.ListOptions, f *framework.
 }
 
 // nolint:gocyclo,gocognit // reduce complexity
-func validatePVCClone(totalCount int, sourcePvcPath, sourceAppPath, clonePvcPath, clonePvcAppPath string, validateEncryption bool, f *framework.Framework) {
+func validatePVCClone(totalCount int, sourcePvcPath, sourceAppPath, clonePvcPath, clonePvcAppPath string, validatePVC validateFunc, f *framework.Framework) {
 	var wg sync.WaitGroup
 	wgErrs := make([]error, totalCount)
 	chErrs := make([]error, totalCount)
@@ -598,8 +598,8 @@ func validatePVCClone(totalCount int, sourcePvcPath, sourceAppPath, clonePvcPath
 					e2elog.Logf("checksum didn't match. checksum=%s and checksumclone=%s", checkSum, checkSumClone)
 				}
 			}
-			if wgErrs[n] == nil && validateEncryption {
-				wgErrs[n] = validateEncryptedPVC(f, &p, &a)
+			if wgErrs[n] == nil && validatePVC != nil {
+				wgErrs[n] = validatePVC(f, &p, &a)
 			}
 			w.Done()
 		}(&wg, i, *pvcClone, *appClone)
@@ -791,7 +791,7 @@ func validatePVCSnapshot(totalCount int, pvcPath, appPath, snapshotPath, pvcClon
 				}
 			}
 			if wgErrs[n] == nil && kms != "" {
-				wgErrs[n] = validateEncryptedPVC(f, &p, &a)
+				wgErrs[n] = isEncryptedPVC(f, &p, &a)
 			}
 			w.Done()
 		}(&wg, i, *pvcClone, *appClone)
diff --git a/internal/rbd/controllerserver_test.go b/internal/rbd/controllerserver_test.go
index 32905a53f..509cadf42 100644
--- a/internal/rbd/controllerserver_test.go
+++ b/internal/rbd/controllerserver_test.go
@@ -23,7 +23,6 @@ import (
 )
 
 func TestIsThickProvisionRequest(t *testing.T) {
-	cs := &ControllerServer{}
 	req := &csi.CreateVolumeRequest{
 		Name: "fake",
 		Parameters: map[string]string{
@@ -32,38 +31,38 @@ func TestIsThickProvisionRequest(t *testing.T) {
 	}
 
 	// pass disabled/invalid values for "thickProvision" option
-	if cs.isThickProvisionRequest(req) {
+	if isThickProvisionRequest(req.GetParameters()) {
 		t.Error("request is not for thick-provisioning")
 	}
 
 	req.Parameters["thickProvision"] = ""
-	if cs.isThickProvisionRequest(req) {
+	if isThickProvisionRequest(req.GetParameters()) {
 		t.Errorf("request is not for thick-provisioning: %s", req.Parameters["thickProvision"])
 	}
 
 	req.Parameters["thickProvision"] = "false"
-	if cs.isThickProvisionRequest(req) {
+	if isThickProvisionRequest(req.GetParameters()) {
 		t.Errorf("request is not for thick-provisioning: %s", req.Parameters["thickProvision"])
 	}
 
 	req.Parameters["thickProvision"] = "off"
-	if cs.isThickProvisionRequest(req) {
+	if isThickProvisionRequest(req.GetParameters()) {
 		t.Errorf("request is not for thick-provisioning: %s", req.Parameters["thickProvision"])
 	}
 
 	req.Parameters["thickProvision"] = "no"
-	if cs.isThickProvisionRequest(req) {
+	if isThickProvisionRequest(req.GetParameters()) {
 		t.Errorf("request is not for thick-provisioning: %s", req.Parameters["thickProvision"])
 	}
 
 	req.Parameters["thickProvision"] = "**true**"
-	if cs.isThickProvisionRequest(req) {
+	if isThickProvisionRequest(req.GetParameters()) {
 		t.Errorf("request is not for thick-provisioning: %s", req.Parameters["thickProvision"])
 	}
 
 	// only "true" should enable thick provisioning
 	req.Parameters["thickProvision"] = "true"
-	if !cs.isThickProvisionRequest(req) {
+	if !isThickProvisionRequest(req.GetParameters()) {
 		t.Errorf("request should be for thick-provisioning: %s", req.Parameters["thickProvision"])
 	}
 }