secondary-storage: delete backedup snapshot dir on delete#7524
secondary-storage: delete backedup snapshot dir on delete#7524yadvr merged 3 commits intoapache:4.18from
Conversation
Fixes apache#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>
Codecov Report
@@ Coverage Diff @@
## 4.18 #7524 +/- ##
=========================================
Coverage 12.93% 12.93%
- Complexity 8941 8948 +7
=========================================
Files 2716 2716
Lines 256108 256144 +36
Branches 39939 39947 +8
=========================================
+ Hits 33127 33139 +12
- Misses 218822 218845 +23
- Partials 4159 4160 +1 see 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@blueorangutan package |
|
@shwstppr a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6069 |
| s_logger.debug(String.format("Snapshot file %s is present in the same name directory %s. Deleting the directory", snapshotName, path)); | ||
| } | ||
| return path; | ||
| } |
There was a problem hiding this comment.
@shwstppr
would it be safer to check if the path is a directory and there is no files in the directory before deletion ?
There was a problem hiding this comment.
@weizhouapache directory check be added but we always expect it to be a directory anyway as we append the `/SNAPSHOT_NAME
And there will always be files (I checked VMware, Xen and KVM. Snapshots are placed inside the same name folder only for VMWare). Example
[root@sl-nestednfs snapshots]# tree 2/10/
2/10/
└── fd57cb44-0559-490c-998d-5313eec52cef
├── fd57cb44-0559-490c-998d-5313eec52cef-disk0.vmdk
├── fd57cb44-0559-490c-998d-5313eec52cef-disk1.nvram
└── fd57cb44-0559-490c-998d-5313eec52cef.ovf
There was a problem hiding this comment.
I suppose we need to cover both cases, to check if we're deleting files inside a folder (like in vmware), but also for kvm where snapshots arne't in a folder. XenServer/XCP-ng we need to check.
There was a problem hiding this comment.
@weizhouapache @shwstppr , we are dealing with a String, not a File/directory. I think this is ok, in this place. checking for existence should not happen in this method.
|
|
||
| @Test | ||
| public void testGetSnapshotFilepathForDelete() { | ||
| performGetSnapshotFilepathForDeleteTest("/snapshots/2/10/somename", |
|
@blueorangutan test rocky8 vmware-67u3 |
|
@rohityadavcloud a [LL] Trillian-Jenkins test job (rocky8 mgmt + vmware-67u3) has been kicked to run smoke tests |
|
Trillian test result (tid-6509)
|
|
@blueorangutan package |
|
@shwstppr a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6102 |
|
@blueorangutan test rocky8 vmware-67u3 |
|
@shwstppr a [SF] Trillian-Jenkins test job (rocky8 mgmt + vmware-67u3) has been kicked to run smoke tests |
kiranchavala
left a comment
There was a problem hiding this comment.
LGTM, Tested on vmware, kvm, xenserver and the snapshot directory/file gets deleted
Steps
- Deploy a VM and create volume snapshot of its disks.
- Delete the volume snapshot once they are backed up.
- On the secondary storage, the snapshots are stored at the path convention:
snapshots/<volume ID>/<snapshot ID>/<snapshots files or folders>.
Before deletion of the snapshots, you should be able to see snapshots files in the snapshots directory for the specific volume/snapshot. - Post-deletion of the snapshots, the directory of /snapshot files are removed.
|
Trillian test result (tid-6545)
|
|
@blueorangutan LLtest rocky8 vmware-67u3 |
|
@DaanHoogland a [LL] Trillian-Jenkins test job (rocky8 mgmt + vmware-67u3) has been kicked to run smoke tests |
...server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
Outdated
Show resolved
Hide resolved
| s_logger.debug(String.format("Snapshot file %s is present in the same name directory %s. Deleting the directory", snapshotName, path)); | ||
| } | ||
| return path; | ||
| } |
There was a problem hiding this comment.
@weizhouapache @shwstppr , we are dealing with a String, not a File/directory. I think this is ok, in this place. checking for existence should not happen in this method.
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
|
@blueorangutan LLpackage |
|
@shwstppr a [LL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [LL]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6056 |
|
[LL]Trillian test result (tid-6538)
|
|
@blueorangutan LLpackage |
|
@shwstppr a [LL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [LL]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6057 |
|
@blueorangutan LLtest matrix |
|
@shwstppr a [LL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests |
|
[LL] Trillian Build Failed (tid-6556) |
|
[LL]Trillian test result (tid-6555)
|
|
[LL]Trillian test result (tid-6557)
|
|
@blueorangutan test matrix |
|
@rohityadavcloud a [SF] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-6633)
|
|
[SF] Trillian test result (tid-6635)
|
|
[SF] Trillian test result (tid-6634)
|
Description
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.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Tested deleting a backedup volume snapshot on VMware
Logs:
Storage
before deletion:
after deletion: