You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2023/06/01 10:18:35 UTC

[cloudstack] branch 4.18 updated: secondary-storage: delete backedup snapshot dir on delete (#7524)

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch 4.18
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/4.18 by this push:
     new 0941d01e97d secondary-storage: delete backedup snapshot dir on delete (#7524)
0941d01e97d is described below

commit 0941d01e97d07233d9ee534892df5aa4ac5dc7dc
Author: Abhishek Kumar <ab...@gmail.com>
AuthorDate: Thu Jun 1 15:48:28 2023 +0530

    secondary-storage: delete backedup snapshot dir on delete (#7524)
    
    * 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 <ab...@gmail.com>
    
    * refactor
    
    Signed-off-by: Abhishek Kumar <ab...@gmail.com>
    
    * resolve comment
    
    Signed-off-by: Abhishek Kumar <ab...@gmail.com>
    
    ---------
    
    Signed-off-by: Abhishek Kumar <ab...@gmail.com>
---
 .../storage/resource/NfsSecondaryStorageResource.java   | 12 +++++++++++-
 .../resource/NfsSecondaryStorageResourceTest.java       | 17 +++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

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
index 3300207a280..e16926e76dc 100644
--- 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 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         }
     }
 
+    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 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
                 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
index 5ca17b0638d..96afcce747b 100644
--- 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 class NfsSecondaryStorageResourceTest {
         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");
+    }
 }