secondary-storage: delete backedup snapshot dir on delete (#7524)

shwstppr · web-flow · commit 0941d01e97d0 · 2023-06-01T15:48:28.000+05:30
* secondary-storage: delete backedup snapshot dir on delete Fixes #7516 When a backed-up snapshot is deleted and the snapshot file is present in the same name directory (probably only for VMware), the whole directory can be deleted. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> * refactor Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> * resolve comment Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> --------- Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
@@ -1997,6 +1997,16 @@ private void savePostUploadPSK(String psk) {
         }
     }
 
+    protected String getSnapshotFilepathForDelete(String path, String snapshotName) {
+        if (!path.endsWith(snapshotName)) {
+            return path + "/*" + snapshotName + "*";
+        }
+        if (s_logger.isDebugEnabled()) {
+            s_logger.debug(String.format("Snapshot file %s is present in the same name directory %s. Deleting the directory", snapshotName, path));
+        }
+        return path;
+    }
+
     protected Answer deleteSnapshot(final DeleteCommand cmd) {
         DataTO obj = cmd.getData();
         DataStoreTO dstore = obj.getDataStore();
@@ -2033,7 +2043,7 @@ protected Answer deleteSnapshot(final DeleteCommand cmd) {
                 return new Answer(cmd, true, details);
             }
             // delete snapshot in the directory if exists
-            String lPath = absoluteSnapshotPath + "/*" + snapshotName + "*";
+            String lPath = getSnapshotFilepathForDelete(absoluteSnapshotPath, snapshotName);
             String result = deleteLocalFile(lPath);
             if (result != null) {
                 details = "failed to delete snapshot " + lPath + " , err=" + result;
diff --git a/services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java b/services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java
@@ -91,4 +91,21 @@ public void testCleanupStagingNfs() throws Exception{
         testLogAppender.assertMessagesLogged();
 
     }
+
+    private void performGetSnapshotFilepathForDeleteTest(String expected, String path, String name) {
+        Assert.assertEquals("Incorrect resultant snapshot delete path", expected, resource.getSnapshotFilepathForDelete(path, name));
+    }
+
+    @Test
+    public void testGetSnapshotFilepathForDelete() {
+        performGetSnapshotFilepathForDeleteTest("/snapshots/2/10/somename",
+                "/snapshots/2/10/somename",
+                "somename");
+        performGetSnapshotFilepathForDeleteTest("/snapshots/2/10/diffName/*diffname*",
+                "/snapshots/2/10/diffName",
+                "diffname");
+        performGetSnapshotFilepathForDeleteTest("/snapshots/2/10/*somename*",
+                "/snapshots/2/10",
+                "somename");
+    }
 }
