From: Kotresh HR <khiremat@redhat.com>
Date: Tue, 29 Mar 2022 05:07:36 +0000 (+0530)
Subject: mgr/volumes: Fix idempotent subvolume rm
X-Git-Tag: v17.2.1~41^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=30bff864ec46d711ce0d3ebe333a2f973021de6b;p=ceph.git

mgr/volumes: Fix idempotent subvolume rm

The subvolume deletion of a subvolume which is already deleted
with retain snapshots option fails with 'EAGAIN: clone in progress'
error. After subvolume deletion with retain snapshots, the subvolume
exists until the trash directory (resides inside subvolume) is
cleaned up. The subvolume deletion issued while the trash directory is not empty, should
pass. This patch fixes the same.

Credit: Issue discovery and fix suggestion to John Mulligan <jmulligan@redhat.com>
Fixes: https://tracker.ceph.com/issues/54625
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 15a2ab4e263b86a77af42c88247a1cb5fb016f94)
---

diff --git a/qa/tasks/cephfs/test_volumes.py b/qa/tasks/cephfs/test_volumes.py
index 102d1f8b9e4..68f1f0501cd 100644
--- a/qa/tasks/cephfs/test_volumes.py
+++ b/qa/tasks/cephfs/test_volumes.py
@@ -2448,6 +2448,41 @@ class TestSubvolumes(TestVolumesHelper):
         # verify trash dir is clean
         self._wait_for_trash_empty()
 
+    def test_subvolume_retain_snapshot_rm_idempotency(self):
+        """
+        ensure subvolume deletion of a subvolume which is already deleted with retain snapshots option passes.
+        After subvolume deletion with retain snapshots, the subvolume exists until the trash directory (resides inside subvolume)
+        is cleaned up. The subvolume deletion issued while the trash directory is not empty, should pass and should
+        not error out with EAGAIN.
+        """
+        subvolume = self._generate_random_subvolume_name()
+        snapshot = self._generate_random_snapshot_name()
+
+        # create subvolume
+        self._fs_cmd("subvolume", "create", self.volname, subvolume, "--mode=777")
+
+        # do some IO
+        self._do_subvolume_io(subvolume, number_of_files=256)
+
+        # snapshot subvolume
+        self._fs_cmd("subvolume", "snapshot", "create", self.volname, subvolume, snapshot)
+
+        # remove with snapshot retention
+        self._fs_cmd("subvolume", "rm", self.volname, subvolume, "--retain-snapshots")
+
+        # remove snapshots (removes retained volume)
+        self._fs_cmd("subvolume", "snapshot", "rm", self.volname, subvolume, snapshot)
+
+        # remove subvolume (check idempotency)
+        try:
+            self._fs_cmd("subvolume", "rm", self.volname, subvolume)
+        except CommandFailedError as ce:
+            if ce.exitstatus != errno.ENOENT:
+                self.fail(f"expected subvolume rm to pass with error: {os.strerror(ce.exitstatus)}")
+
+        # verify trash dir is clean
+        self._wait_for_trash_empty()
+
 
 class TestSubvolumeGroupSnapshots(TestVolumesHelper):
     """Tests for FS subvolume group snapshot operations."""
diff --git a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py
index a827bb7a00e..3971d31b1aa 100644
--- a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py
+++ b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py
@@ -352,7 +352,7 @@ class SubvolumeV2(SubvolumeV1):
         # machine which removes the entry from the index. Hence, it's safe to removed clone with
         # force option for both.
         acceptable_rm_clone_states = [SubvolumeStates.STATE_COMPLETE, SubvolumeStates.STATE_CANCELED,
-                                      SubvolumeStates.STATE_FAILED]
+                                      SubvolumeStates.STATE_FAILED, SubvolumeStates.STATE_RETAINED]
         if subvol_state not in acceptable_rm_clone_states:
             return False
         return True