RBD: Do flattening only the temporary cloned images not the images given to user for mapping

[Madhu-1]

In the case of CreateVolume from snapshot and CreateVolume from Volume, when the soft or hard limit is reached we do flattening on the image. and we need to ensure that we only flatten the intermediate clones we create not the volumes which might get mapped when the application mounts the PVC. flattening on the images can affect the read/write performance of the image due to some internal locking.

[Madhu-1]

This needs some testing and may or may not code changes but it's good to validate this case.

[humblec]

As discussed we have to continue the analysis and get into the completion in upcoming releases. Moving this out of 3.4.0 release.

[Rakshith-R]

In current implementation of CephCSI rbd driver, flattening is triggered for two reasons hitting snaps and CloneDepth limit:
Ceph CSI rbd processes(highly summarized):

• k8s snapshot = PVC->snap->tmp-snap-img->snap
• k8s PVC-restore= Snapshot's snap->clone-img
• k8s PVC-PVC-clone= PVC -> snap->temp-clone-img->snap->clone-img

(snap refers to rbd image snapshot)

With respect to snap limit:

• rbd image snap is taken during k8s snapshot and pvc-pvc cloning.
• Thus flattening due to hitting snap limit can occur only on rbd temp image underlying snapshot and temp clone image underlying PVC-clone.

With respect to CloneDepth limit:
To be analysed.

Every k8s PVC's underlying image if it has a parent rbd image, it's either a k8s snapshot's image(PVC-restore) or temporary-clone image(PVC-PVC-clone).

[Rakshith-R]

ceph-csi/internal/rbd/clone.go

Lines 258 to 290 in 60c2afb

	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
	// error message as it need to be flatten before continuing, this may leak
	// omap entries and stale temporary snapshots in corner cases, if we reduce
	// the limit and check for the depth of the parent image clain itself we
	// can flatten the parent images before used to avoid the stale omap entries.
	hardLimit := rbdHardMaxCloneDepth
	softLimit := rbdSoftMaxCloneDepth
	// choosing 2 so that we don't need to flatten the image in the request.
	const depthToAvoidFlatten = 2
	if rbdHardMaxCloneDepth > depthToAvoidFlatten {
	hardLimit = rbdHardMaxCloneDepth - depthToAvoidFlatten
	}
	if rbdSoftMaxCloneDepth > depthToAvoidFlatten {
	softLimit = rbdSoftMaxCloneDepth - depthToAvoidFlatten
	}
	err := tempClone.getImageInfo()
	if err == nil {
	return tempClone.flattenRbdImage(ctx, tempClone.conn.Creds, false, hardLimit, softLimit)
	}
	if !errors.Is(err, ErrImageNotFound) {
	return err
	}
	return rv.flattenRbdImage(ctx, rv.conn.Creds, false, hardLimit, softLimit)
	}

If a temporary clone does not exist for given image ,
we need to find the parent image here, which would be a rbd image image under k8s snapshot and flatten it instead.

[Madhu-1]

ceph-csi/internal/rbd/clone.go

Lines 258 to 290 in 60c2afb

	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
	// error message as it need to be flatten before continuing, this may leak
	// omap entries and stale temporary snapshots in corner cases, if we reduce
	// the limit and check for the depth of the parent image clain itself we
	// can flatten the parent images before used to avoid the stale omap entries.
	hardLimit := rbdHardMaxCloneDepth
	softLimit := rbdSoftMaxCloneDepth
	// choosing 2 so that we don't need to flatten the image in the request.
	const depthToAvoidFlatten = 2
	if rbdHardMaxCloneDepth > depthToAvoidFlatten {
	hardLimit = rbdHardMaxCloneDepth - depthToAvoidFlatten
	}
	if rbdSoftMaxCloneDepth > depthToAvoidFlatten {
	softLimit = rbdSoftMaxCloneDepth - depthToAvoidFlatten
	}
	err := tempClone.getImageInfo()
	if err == nil {
	return tempClone.flattenRbdImage(ctx, tempClone.conn.Creds, false, hardLimit, softLimit)
	}
	if !errors.Is(err, ErrImageNotFound) {
	return err
	}
	return rv.flattenRbdImage(ctx, rv.conn.Creds, false, hardLimit, softLimit)
	}

If a temporary clone does not exist for a given image ,

In which case the temporary clone does not exists?

from PVC-PVC clone case both image and the temporary image will be deleted when we delete the image.

we need to find the parent image here, which would be a rbd image image under k8s snapshot and flatten it instead.

Yes, the snapshot needs to be flattened in case of PVC restoration from snapshot operation.

[Rakshith-R]

In which case the temporary clone does not exists?

I meant ErrImageNotFound for temp clone -> for a snapshot restored parent PVC.

[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.

[Rakshith-R]

@Yuggupta27 , I was planning to work on this.
If you did understand the issue and the fix discussed above in the comments, then its fine
else let me know, I can continue and send a pr for this soon.

[Rakshith-R]

ceph-csi/internal/rbd/controllerserver.go

Lines 588 to 595 in 9c50e25

	if rbdSnap != nil {
	err = rbdVol.flattenRbdImage(ctx, false, rbdHardMaxCloneDepth, rbdSoftMaxCloneDepth)
	if err != nil {
	log.ErrorLog(ctx, "failed to flatten image %s: %v", rbdVol, err)
	return err
	}

one more place where we are flattening the image which will be given to user for mapping.
The parent underlying snapshot image can be flattened instead.