From 3721f30a48f3ff799e2ac27434f778e739261629 Mon Sep 17 00:00:00 2001 From: Rakshith R Date: Thu, 14 Nov 2024 14:10:36 +0530 Subject: [PATCH 1/2] rbd: remove checkFlatten() function CephCSI should not flatten image that can be mounted for use by the user. `checkFlatten()` was called in a recovery code flow of PVC restored from snapshot and was missed while refractoring in https://github.com/ceph/ceph-csi/pull/2900 refer: #2900 Signed-off-by: Rakshith R --- internal/rbd/controllerserver.go | 39 +++----------------------------- 1 file changed, 3 insertions(+), 36 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index e721bc7c51f..5a499041004 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -389,7 +389,7 @@ func (cs *ControllerServer) CreateVolume( if err != nil { return nil, getGRPCErrorForCreateVolume(err) } else if found { - return cs.repairExistingVolume(ctx, req, cr, rbdVol, rbdSnap) + return cs.repairExistingVolume(ctx, req, rbdVol, rbdSnap) } err = checkValidCreateVolumeRequest(rbdVol, parentVol, rbdSnap) @@ -516,21 +516,14 @@ func flattenParentImage( // 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, + rbdVol *rbdVolume, rbdSnap *rbdSnapshot, ) (*csi.CreateVolumeResponse, error) { vcs := req.GetVolumeContentSource() switch { // rbdVol is a restore from snapshot, rbdSnap is passed case vcs.GetSnapshot() != nil: - // restore from snapshot implies rbdSnap != nil - // check if image depth is reached limit and requires flatten - err := checkFlatten(ctx, rbdVol, cr) - if err != nil { - return nil, err - } - - err = rbdSnap.repairEncryptionConfig(ctx, &rbdVol.rbdImage) + err := rbdSnap.repairEncryptionConfig(ctx, &rbdVol.rbdImage) if err != nil { return nil, err } @@ -636,32 +629,6 @@ func flattenTemporaryClonedImages(ctx context.Context, rbdVol *rbdVolume, cr *ut return nil } -// checkFlatten ensures that the image chain depth is not reached -// hardlimit or softlimit. if the softlimit is reached it adds a task and -// return success,the hardlimit is reached it starts a task to flatten the -// image and return Aborted. -func checkFlatten(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials) error { - err := rbdVol.flattenRbdImage(ctx, false, rbdHardMaxCloneDepth, rbdSoftMaxCloneDepth) - if err != nil { - if errors.Is(err, ErrFlattenInProgress) { - return status.Error(codes.Aborted, err.Error()) - } - if errDefer := rbdVol.Delete(ctx); errDefer != nil { - log.ErrorLog(ctx, "failed to delete rbd image: %s with error: %v", rbdVol, errDefer) - - return status.Error(codes.Internal, err.Error()) - } - errDefer := undoVolReservation(ctx, rbdVol, cr) - if errDefer != nil { - log.WarningLog(ctx, "failed undoing reservation of volume: %s (%s)", rbdVol.RequestName, errDefer) - } - - return status.Error(codes.Internal, err.Error()) - } - - return nil -} - func (cs *ControllerServer) createVolumeFromSnapshot( ctx context.Context, cr *util.Credentials, From 1bf7f0a5e1264c8df806a97e6f01e4376b3ec299 Mon Sep 17 00:00:00 2001 From: Rakshith R Date: Thu, 14 Nov 2024 14:15:16 +0530 Subject: [PATCH 2/2] rbd: set depthToAvoidFlatten to 3 during PVC-PVC clone During PVC-PVC clone creation, parent of the datasource image is flattened after checking for clone depth. We need to account for data source image as well since we're calculating depth from the parent image. depthToAvoidFlatten = 3(datasource image + temp + final clone) Signed-off-by: Rakshith R --- internal/rbd/controllerserver.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 5a499041004..3ec067c3ecf 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -461,9 +461,9 @@ func flattenParentImage( hardLimit := rbdHardMaxCloneDepth softLimit := rbdSoftMaxCloneDepth if rbdVol != nil { - // choosing 2, since cloning image creates a temp clone and a final clone which - // will add a total depth of 2. - const depthToAvoidFlatten = 2 + // choosing 3, since cloning image creates a temp clone and a final clone which + // will add a total depth of 2 and the parent image itself adds one depth. + const depthToAvoidFlatten = 3 if rbdHardMaxCloneDepth > depthToAvoidFlatten { hardLimit = rbdHardMaxCloneDepth - depthToAvoidFlatten }