From 0d9548c815ff8b0ad8720b17b56d4fb762b1442c Mon Sep 17 00:00:00 2001 From: Yati Padia Date: Wed, 10 Feb 2021 13:49:40 +0530 Subject: [PATCH] Cephfs: Failed to delete snapshot Failed to delete voluesnapshot when backend subvolume (pvc) and ceph fs subvolume snapshot is deleted Fixes#1647 Signed-off-by: Yati Padia --- e2e/cephfs.go | 57 +++++++++++++++++++++++++++++ e2e/cephfs_helper.go | 46 ++++++++++++++++++++++- internal/cephfs/controllerserver.go | 12 ++++++ 3 files changed, 114 insertions(+), 1 deletion(-) diff --git a/e2e/cephfs.go b/e2e/cephfs.go index 0529fef7e..2b3103a43 100644 --- a/e2e/cephfs.go +++ b/e2e/cephfs.go @@ -571,6 +571,63 @@ var _ = Describe("cephfs", func() { } }) + By("Delete snapshot after deleting subvolume and snapshot from backend", func() { + // snapshot beta is only supported from v1.17+ + if k8sVersionGreaterEquals(f.ClientSet, 1, 17) { + err := createCephFSSnapshotClass(f) + if err != nil { + e2elog.Failf("failed to create CephFS snapshotclass with error %v", err) + } + pvc, err := loadPVC(pvcPath) + if err != nil { + e2elog.Failf("failed to load PVC with error %v", err) + } + + pvc.Namespace = f.UniqueName + err = createPVCAndvalidatePV(f.ClientSet, pvc, deployTimeout) + if err != nil { + e2elog.Failf("failed to create PVC with error %v", err) + } + + snap := getSnapshot(snapshotPath) + snap.Namespace = f.UniqueName + snap.Spec.Source.PersistentVolumeClaimName = &pvc.Name + // create snapshot + snap.Name = f.UniqueName + err = createSnapshot(&snap, deployTimeout) + if err != nil { + e2elog.Failf("failed to create snapshot (%s): %v", snap.Name, err) + } + + err = deleteBackingCephFSSubvolumeSnapshot(f, pvc, &snap) + if err != nil { + e2elog.Failf("failed to delete backing snapshot for snapname with error=%s", err) + } + + err = deleteBackingCephFSVolume(f, pvc) + if err != nil { + e2elog.Failf("failed to delete backing subvolume error=%s", err) + } + + err = deleteSnapshot(&snap, deployTimeout) + if err != nil { + e2elog.Failf("failed to delete snapshot with error=%s", err) + } else { + e2elog.Logf("successfully deleted snapshot") + } + + err = deletePVCAndValidatePV(f.ClientSet, pvc, deployTimeout) + if err != nil { + e2elog.Failf("failed to delete PVC with error %v", err) + } + + err = deleteResource(cephfsExamplePath + "snapshotclass.yaml") + if err != nil { + e2elog.Failf("failed to delete CephFS snapshotclass with error %v", err) + } + } + }) + By("Test snapshot retention feature", func() { // Delete the PVC after creating a snapshot, // this should work because of the snapshot diff --git a/e2e/cephfs_helper.go b/e2e/cephfs_helper.go index d24da5dd3..a9c1d7324 100644 --- a/e2e/cephfs_helper.go +++ b/e2e/cephfs_helper.go @@ -4,12 +4,15 @@ import ( "context" "encoding/json" "fmt" + "regexp" "strings" + snapapi "github.com/kubernetes-csi/external-snapshotter/v2/pkg/apis/volumesnapshot/v1beta1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/test/e2e/framework" + e2elog "k8s.io/kubernetes/test/e2e/framework/log" ) const ( @@ -104,7 +107,8 @@ func deleteBackingCephFSVolume(f *framework.Framework, pvc *v1.PersistentVolumeC return err } - _, stdErr, err := execCommandInToolBoxPod(f, "ceph fs subvolume rm myfs "+imageData.imageName+" "+subvolumegroup, rookNamespace) + cmd := fmt.Sprintf("ceph fs subvolume rm %s %s %s", fileSystemName, imageData.imageName, subvolumegroup) + _, stdErr, err := execCommandInToolBoxPod(f, cmd, rookNamespace) if err != nil { return err } @@ -147,3 +151,43 @@ func getSubvolumePath(f *framework.Framework, filesystem, subvolgrp, subvolume s } return strings.TrimSpace(stdOut), nil } + +func getSnapName(snapNamespace, snapName string) (string, error) { + sclient, err := newSnapshotClient() + if err != nil { + return "", err + } + snap, err := sclient.SnapshotV1beta1().VolumeSnapshots(snapNamespace).Get(context.TODO(), snapName, metav1.GetOptions{}) + if err != nil { + return "", err + } + sc, err := sclient.SnapshotV1beta1().VolumeSnapshotContents().Get(context.TODO(), *snap.Status.BoundVolumeSnapshotContentName, metav1.GetOptions{}) + if err != nil { + return "", err + } + snapIDRegex := regexp.MustCompile(`(\w+\-?){5}$`) + snapID := snapIDRegex.FindString(*sc.Status.SnapshotHandle) + snapshotName := fmt.Sprintf("csi-snap-%s", snapID) + e2elog.Logf("snapshotName= %s", snapshotName) + return snapshotName, nil +} + +func deleteBackingCephFSSubvolumeSnapshot(f *framework.Framework, pvc *v1.PersistentVolumeClaim, snap *snapapi.VolumeSnapshot) error { + snapshotName, err := getSnapName(snap.Namespace, snap.Name) + if err != nil { + return err + } + imageData, err := getImageInfoFromPVC(pvc.Namespace, pvc.Name, f) + if err != nil { + return err + } + cmd := fmt.Sprintf("ceph fs subvolume snapshot rm %s %s %s %s", fileSystemName, imageData.imageName, snapshotName, subvolumegroup) + _, stdErr, err := execCommandInToolBoxPod(f, cmd, rookNamespace) + if err != nil { + return err + } + if stdErr != "" { + return fmt.Errorf("error deleting backing snapshot %s %v", snapshotName, stdErr) + } + return nil +} diff --git a/internal/cephfs/controllerserver.go b/internal/cephfs/controllerserver.go index 4b2eec6d1..8a9f7e3cd 100644 --- a/internal/cephfs/controllerserver.go +++ b/internal/cephfs/controllerserver.go @@ -711,6 +711,18 @@ func (cs *ControllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteS } return &csi.DeleteSnapshotResponse{}, nil } + // if the error is ErrVolumeNotFound, the subvolume is already deleted + // from backend, Hence undo the omap entries and return success + if errors.Is(err, ErrVolumeNotFound) { + util.ErrorLog(ctx, "Volume not present") + err = undoSnapReservation(ctx, volOpt, *sid, sid.FsSnapshotName, cr) + if err != nil { + util.ErrorLog(ctx, "failed to remove reservation for snapname (%s) with backing snap (%s) (%s)", + sid.FsSubvolName, sid.FsSnapshotName, err) + return nil, status.Error(codes.Internal, err.Error()) + } + return &csi.DeleteSnapshotResponse{}, nil + } return nil, status.Error(codes.Internal, err.Error()) } defer volOpt.Destroy()