gian/cloudstack-mirror
1
0
Fork 0
mirror of https://github.com/apache/cloudstack synced 2026-08-02 05:26:35 +00:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

Incremental nas backup fixes (#13571)

Browse Source 
* Avoid concurrent deletes modifying the same backup chain in parallel - Backup deletes are serialized by using a per-VM GlobalLock
Backup delete checks if it has a live dependent backup not just a live immediate child.
This commit is contained in:
Abhisar Sinha 2026-07-10 11:50:19 +05:30 committed by GitHub
parent de3656c819
commit ba837fb8a8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 334 additions and 135 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

189
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
View File

@ -38,6 +38,7 @@ import com.cloud.storage.dao.StoragePoolHostDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.utils.Pair;
import com.cloud.utils.component.AdapterBase;
import com.cloud.utils.db.GlobalLock;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.user.ResourceLimitService;
import com.cloud.vm.VMInstanceDetailVO;
@ -83,6 +84,8 @@ import static org.apache.cloudstack.backup.BackupManager.BackupFrameworkEnabled;
public class NASBackupProvider extends AdapterBase implements BackupProvider, Configurable {
private static final Logger LOG = LogManager.getLogger(NASBackupProvider.class);
private static final int CHAIN_LOCK_TIMEOUT_SECONDS = 300;
ConfigKey<Integer> NASBackupRestoreMountTimeout = new ConfigKey<>("Advanced", Integer.class,
"nas.backup.restore.mount.timeout",
"30",
@ -639,7 +642,7 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
}
} else {
logger.error("Failed to take backup for VM {}: {}", vm.getInstanceName(), answer != null ? answer.getDetails() : "No answer received");
if (answer.getNeedsCleanup()) {
if (answer != null && answer.getNeedsCleanup()) {
logger.error("Backup cleanup failed for VM {}. Leaving the backup in Error state. Backup should be manually deleted to free up the space", vm.getInstanceName());
backupVO.setStatus(Backup.Status.Error);
backupDao.update(backupVO.getId(), backupVO);
@ -912,26 +915,58 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
return deleteBackupFileAndRow(backup, backupRepository, host);
}
// Snapshot-style cascade: defer the on-NAS rm + DB row while there are live children,
// mark this backup as delete-pending, and let the leaf's deletion sweep it up later.
// See DefaultSnapshotStrategy#deleteSnapshotChain for the same pattern on incremental
// snapshots. forced=true means the caller wants the entire subtree gone right now.
if (forced) {
return cascadeDeleteSubtree(backup, backupRepository, host);
}
// Concurrent deletes can mutate the same chain concurrently resulting in inconsistent states.
// take a per-VM lock before modifying the chain.
final GlobalLock chainLock = acquireChainDeleteLock(backup.getVmId());
try {
Backup current = backupDao.findById(backup.getId());
if (current == null) {
LOG.debug("Backup {} was already removed by a concurrent chain delete", backup.getUuid());
return true;
}
backup = current;
List<Backup> liveChildren = findLiveChildren(backup);
if (!liveChildren.isEmpty()) {
markDeletePending(backup);
LOG.debug("Backup {} has {} live child backup(s); marking as delete-pending. The on-NAS file " +
"and DB row will be removed once the last descendant is gone, or pass forced=true.",
backup.getUuid(), liveChildren.size());
return true;
}
// Snapshot-style cascade: defer the on-NAS rm + DB row while there are live children,
// mark this backup as delete-pending, and let the leaf's deletion sweep it up later.
// See DefaultSnapshotStrategy#deleteSnapshotChain for the same pattern on incremental
// snapshots. forced=true means the caller wants the entire subtree gone right now.
if (forced) {
return cascadeDeleteSubtree(backup, backupRepository, host);
}
// No live children — physically delete this backup, then walk up the chain and
// collect any ancestors that were left in delete-pending state.
return deleteLeafBackupAndSweepPendingAncestors(backup, backupRepository, host);
if (hasLiveChildren(backup)) {
markDeletePending(backup);
LOG.debug("Backup {} has live descendants in its chain; setting status as Hidden. " +
"The on-NAS file and DB row will be removed once the last descendant is gone, " +
"or pass forced=true.",
backup.getUuid());
return true;
}
// No live children — physically delete this backup, then walk up the chain and
// collect any ancestors that were left in Hidden state.
return deleteLeafBackupAndSweepPendingAncestors(backup, backupRepository, host);
} finally {
releaseChainDeleteLock(chainLock);
}
}
/**
* Take the per-VM lock that serializes chain-mutating backup deletes.
*/
protected GlobalLock acquireChainDeleteLock(long vmId) {
GlobalLock lock = GlobalLock.getInternLock("nas.backup.chain.vm." + vmId);
if (!lock.lock(CHAIN_LOCK_TIMEOUT_SECONDS)) {
lock.releaseRef();
throw new CloudRuntimeException(String.format(
"Timed out waiting for concurrent backup chain operations on VM %d to finish, please try again", vmId));
}
return lock;
}
protected void releaseChainDeleteLock(GlobalLock lock) {
lock.unlock();
lock.releaseRef();
}
/**
@ -951,7 +986,7 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
throw new CloudRuntimeException("Operation to delete backup timed out, please try again");
}
if (answer == null || !answer.getResult()) {
logger.warn("Failed to delete backup file for {} ({}); leaving DB row intact",
logger.error("Failed to delete backup file for {} ({}); leaving DB row intact",
backup.getUuid(), backup.getExternalId());
return false;
}
@ -1000,53 +1035,24 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
}
/**
* Return the live (not delete-pending, not Removed) children of {@code parent} within the
* same chain. Equivalent to "incrementals whose parent_backup_id points at parent".
* Whether there are any live (not tombstoned/Hidden, not Removed) descendants of {@code backup} within the
* same chain. This uses {@code CHAIN_POSITION} to detect dependents deeper in the chain.
*/
private List<Backup> findLiveChildren(Backup parent) {
String parentUuid = parent.getUuid();
String chainId = readDetail(parent, NASBackupChainKeys.CHAIN_ID);
if (parentUuid == null || chainId == null) {
return Collections.emptyList();
}
List<Backup> children = new ArrayList<>();
for (Backup b : backupDao.listByVmId(null, parent.getVmId())) {
if (b.getId() == parent.getId()) {
continue;
}
if (!chainId.equals(readDetail(b, NASBackupChainKeys.CHAIN_ID))) {
continue;
}
if (!parentUuid.equals(readDetail(b, NASBackupChainKeys.PARENT_BACKUP_ID))) {
continue;
}
if (isDeletePending(b)) {
// Tombstoned children don't keep us alive — they're already on the way out.
continue;
}
children.add(b);
}
return children;
}
/**
* Look up this backup's immediate parent in the chain (by {@code PARENT_BACKUP_ID}).
* Returns {@code null} if this is the full (no parent) or the parent row is gone.
*
* <p>Prefer {@link #getChainOrderedLeafToRoot(Backup)} when walking the whole chain —
* this method hits the DB on each call and is O(N²) when used in a loop.
*/
private Backup findChainParent(Backup backup) {
String parentUuid = readDetail(backup, NASBackupChainKeys.PARENT_BACKUP_ID);
if (parentUuid == null || parentUuid.isEmpty()) {
return null;
private boolean hasLiveChildren(Backup backup) {
String chainId = readDetail(backup, NASBackupChainKeys.CHAIN_ID);
if (chainId == null) {
return false;
}
int position = chainPosition(backup);
for (Backup b : backupDao.listByVmId(null, backup.getVmId())) {
if (parentUuid.equals(b.getUuid())) {
return b;
if (b.getId() == backup.getId() || !chainId.equals(readDetail(b, NASBackupChainKeys.CHAIN_ID))) {
continue;
}
if (!isDeletePending(b) && chainPosition(b) > position) {
return true;
}
}
return null;
return false;
}
/**
@ -1078,7 +1084,7 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
/**
* Physically delete the leaf {@code backup}, then walk up the chain while each ancestor
* is in delete-pending state. Mirrors the snapshot subsystem pattern: once a leaf is
* is in Hidden state. Mirrors the snapshot subsystem pattern: once a leaf is
* gone, garbage-collect any tombstoned parents.
*
* <p>Caller must guarantee {@code backup} is a leaf (no live children). Each tombstoned
@ -1092,23 +1098,14 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
if (!deleteBackupFileAndRow(backup, repo, host)) {
return false;
}
// Walk the snapshot from leaf+1 upward, deleting tombstoned ancestors until a live
// one is reached or the root is past.
int leafIdx = indexOfBackupById(chain, backup.getId());
if (leafIdx < 0) {
// Leaf wasn't in its own CHAIN_ID list — degenerate case, nothing more to sweep.
return true;
}
for (int i = leafIdx + 1; i < chain.size(); i++) {
Backup ancestor = chain.get(i);
if (!isDeletePending(ancestor)) {
for (Backup member : chain) {
if (member.getId() == backup.getId()) {
continue;
}
if (!isDeletePending(member)) {
break;
}
if (!deleteBackupFileAndRow(ancestor, repo, host)) {
// Stop the sweep; the rest of the tombstoned chain will be collected on a
// future delete that re-runs the sweep.
return true;
}
deleteBackupFileAndRow(member, repo, host);
}
return true;
}
@ -1127,42 +1124,6 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
return true;
}
private static int indexOfBackupById(List<Backup> chain, long id) {
for (int i = 0; i < chain.size(); i++) {
if (chain.get(i).getId() == id) {
return i;
}
}
return -1;
}
/**
* Return the backup with the highest {@code CHAIN_POSITION} sharing {@code root}'s
* {@code CHAIN_ID}. Returns {@code root} if it has no chain metadata or is itself the tail.
*/
private Backup findChainTail(Backup root) {
String chainId = readDetail(root, NASBackupChainKeys.CHAIN_ID);
if (chainId == null) {
return root;
}
Backup tail = root;
int tailPos = chainPosition(root);
for (Backup b : backupDao.listByVmId(null, root.getVmId())) {
if (b.getId() == root.getId()) {
continue;
}
if (!chainId.equals(readDetail(b, NASBackupChainKeys.CHAIN_ID))) {
continue;
}
int pos = chainPosition(b);
if (pos > tailPos) {
tail = b;
tailPos = pos;
}
}
return tail;
}
private int chainPosition(Backup b) {
String s = readDetail(b, NASBackupChainKeys.CHAIN_POSITION);
if (s == null) {

243
plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java
View File

@ -55,6 +55,7 @@ import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.user.ResourceLimitService;
import com.cloud.utils.Pair;
import com.cloud.utils.db.GlobalLock;
import com.cloud.vm.VMInstanceDetailVO;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine;
@ -679,9 +680,11 @@ public class NASBackupProviderTest {
// CHAIN_ID on the parent => not the no-chain fast path.
BackupDetailVO chainIdDetail = new BackupDetailVO(50L, NASBackupChainKeys.CHAIN_ID, "chain-1", true);
BackupDetailVO chainPosDetail = new BackupDetailVO(50L, NASBackupChainKeys.CHAIN_POSITION, "0", true);
Mockito.when(backupDetailsDao.findDetail(50L, NASBackupChainKeys.CHAIN_ID)).thenReturn(chainIdDetail);
Mockito.when(backupDetailsDao.findDetail(50L, NASBackupChainKeys.CHAIN_POSITION)).thenReturn(chainPosDetail);
// A live child references parent-uuid via PARENT_BACKUP_ID.
// A live child sits deeper in the chain (higher CHAIN_POSITION).
BackupVO child = new BackupVO();
child.setVmId(vmId);
child.setBackupOfferingId(offeringId);
@ -692,13 +695,14 @@ public class NASBackupProviderTest {
ReflectionTestUtils.setField(child, "uuid", "child-uuid");
BackupDetailVO childChainId = new BackupDetailVO(51L, NASBackupChainKeys.CHAIN_ID, "chain-1", true);
BackupDetailVO childParent = new BackupDetailVO(51L, NASBackupChainKeys.PARENT_BACKUP_ID, "parent-uuid", true);
BackupDetailVO childChainPos = new BackupDetailVO(51L, NASBackupChainKeys.CHAIN_POSITION, "1", true);
Mockito.when(backupDetailsDao.findDetail(51L, NASBackupChainKeys.CHAIN_ID)).thenReturn(childChainId);
Mockito.when(backupDetailsDao.findDetail(51L, NASBackupChainKeys.PARENT_BACKUP_ID)).thenReturn(childParent);
Mockito.when(backupDetailsDao.findDetail(51L, NASBackupChainKeys.CHAIN_POSITION)).thenReturn(childChainPos);
Mockito.when(backupDao.listByVmId(null, vmId)).thenReturn(List.of(parent, child));
// markDeletePending loads the row to flip its status to Hidden.
// Re-read under the chain lock + markDeletePending both load the row by id.
Mockito.when(backupDao.findById(50L)).thenReturn(parent);
Mockito.doReturn(mock(GlobalLock.class)).when(nasBackupProvider).acquireChainDeleteLock(vmId);
boolean result = nasBackupProvider.deleteBackup(parent, false);
Assert.assertTrue(result);
@ -771,13 +775,13 @@ public class NASBackupProviderTest {
parent.setStatus(Backup.Status.Hidden);
Mockito.when(backupDetailsDao.findDetail(50L, NASBackupChainKeys.CHAIN_ID)).thenReturn(parentChainId);
Mockito.when(backupDetailsDao.findDetail(50L, NASBackupChainKeys.CHAIN_POSITION)).thenReturn(parentChainPos);
// Parent has no parent of its own (it's the full anchor).
Mockito.when(backupDetailsDao.findDetail(50L, NASBackupChainKeys.PARENT_BACKUP_ID)).thenReturn(null);
// listByVmId is called once now (chain snapshot taken before the leaf delete).
// We still use a mutable list + remove() answer so the DAO contract is realistic.
java.util.List<Backup> liveBackups = new java.util.ArrayList<>(List.of(parent, leaf));
Mockito.when(backupDao.listByVmId(null, vmId)).thenAnswer(inv -> new java.util.ArrayList<>(liveBackups));
// The target row is re-read under the chain lock before any chain decision.
Mockito.when(backupDao.findById(51L)).thenReturn(leaf);
Mockito.doReturn(mock(GlobalLock.class)).when(nasBackupProvider).acquireChainDeleteLock(vmId);
// Agent acknowledges every delete.
Mockito.when(agentManager.send(Mockito.anyLong(), Mockito.any(DeleteBackupCommand.class)))
@ -805,4 +809,229 @@ public class NASBackupProviderTest {
Mockito.verify(resourceLimitMgr, Mockito.times(2))
.decrementResourceCount(Mockito.anyLong(), Mockito.eq(Resource.ResourceType.backup_storage), Mockito.any());
}
@Test
public void deletingLastLiveMemberCollectsDeeperOrphanTombstones()
throws AgentUnavailableException, OperationTimedoutException {
Long zoneId = 1L;
Long vmId = 2L;
Long hostId = 3L;
Long offeringId = 4L;
BackupVO full = new BackupVO();
full.setVmId(vmId);
full.setBackupOfferingId(offeringId);
full.setExternalId("i-2-2-VM/2026.05.10.10.00.00");
full.setZoneId(zoneId);
full.setStatus(Backup.Status.BackedUp);
ReflectionTestUtils.setField(full, "id", 50L);
// The last live incremental — the one being deleted.
BackupVO inc1 = new BackupVO();
inc1.setVmId(vmId);
inc1.setBackupOfferingId(offeringId);
inc1.setExternalId("i-2-2-VM/2026.05.10.10.30.00");
inc1.setZoneId(zoneId);
inc1.setStatus(Backup.Status.BackedUp);
ReflectionTestUtils.setField(inc1, "id", 51L);
// A stranded tombstone deeper in the chain: its own descendants are long gone, so
// only a sweep triggered by an ancestor's deletion can ever collect it.
BackupVO orphan = new BackupVO();
orphan.setVmId(vmId);
orphan.setBackupOfferingId(offeringId);
orphan.setExternalId("i-2-2-VM/2026.05.10.11.00.00");
orphan.setZoneId(zoneId);
orphan.setStatus(Backup.Status.Hidden);
ReflectionTestUtils.setField(orphan, "id", 52L);
VMInstanceVO vm = mock(VMInstanceVO.class);
Mockito.when(vm.getLastHostId()).thenReturn(hostId);
HostVO host = mock(HostVO.class);
Mockito.when(host.getStatus()).thenReturn(Status.Up);
Mockito.when(host.getId()).thenReturn(hostId);
Mockito.when(hostDao.findById(hostId)).thenReturn(host);
BackupRepositoryVO repo = new BackupRepositoryVO(1L, "nas", "test-repo",
"nfs", "address", "sync", 1024L, null);
Mockito.when(backupRepositoryDao.findByBackupOfferingId(offeringId)).thenReturn(repo);
Mockito.when(vmInstanceDao.findByIdIncludingRemoved(vmId)).thenReturn(vm);
Mockito.when(backupDetailsDao.findDetail(50L, NASBackupChainKeys.CHAIN_ID))
.thenReturn(new BackupDetailVO(50L, NASBackupChainKeys.CHAIN_ID, "chain-1", true));
Mockito.when(backupDetailsDao.findDetail(50L, NASBackupChainKeys.CHAIN_POSITION))
.thenReturn(new BackupDetailVO(50L, NASBackupChainKeys.CHAIN_POSITION, "0", true));
Mockito.when(backupDetailsDao.findDetail(51L, NASBackupChainKeys.CHAIN_ID))
.thenReturn(new BackupDetailVO(51L, NASBackupChainKeys.CHAIN_ID, "chain-1", true));
Mockito.when(backupDetailsDao.findDetail(51L, NASBackupChainKeys.CHAIN_POSITION))
.thenReturn(new BackupDetailVO(51L, NASBackupChainKeys.CHAIN_POSITION, "1", true));
Mockito.when(backupDetailsDao.findDetail(52L, NASBackupChainKeys.CHAIN_ID))
.thenReturn(new BackupDetailVO(52L, NASBackupChainKeys.CHAIN_ID, "chain-1", true));
Mockito.when(backupDetailsDao.findDetail(52L, NASBackupChainKeys.CHAIN_POSITION))
.thenReturn(new BackupDetailVO(52L, NASBackupChainKeys.CHAIN_POSITION, "2", true));
Mockito.when(backupDao.listByVmId(null, vmId)).thenReturn(List.of(full, inc1, orphan));
Mockito.when(backupDao.findById(51L)).thenReturn(inc1);
Mockito.doReturn(mock(GlobalLock.class)).when(nasBackupProvider).acquireChainDeleteLock(vmId);
Mockito.when(agentManager.send(Mockito.anyLong(), Mockito.any(DeleteBackupCommand.class)))
.thenReturn(new BackupAnswer(new DeleteBackupCommand(null, null, null, null), true, "ok"));
Assert.assertTrue(nasBackupProvider.deleteBackup(inc1, false));
// inc1 and the deeper orphan tombstone are physically removed; the live full survives.
Mockito.verify(agentManager, Mockito.times(2))
.send(Mockito.anyLong(), Mockito.any(DeleteBackupCommand.class));
Mockito.verify(backupDao).remove(51L);
Mockito.verify(backupDao).remove(52L);
Mockito.verify(backupDao, Mockito.never()).remove(50L);
}
@Test
public void deletingAncestorOfTombstoneWithLiveDescendantTombstonesIt()
throws AgentUnavailableException, OperationTimedoutException {
Long zoneId = 1L;
Long vmId = 2L;
Long hostId = 3L;
Long offeringId = 4L;
BackupVO full = new BackupVO();
full.setVmId(vmId);
full.setBackupOfferingId(offeringId);
full.setExternalId("i-2-2-VM/2026.05.10.10.00.00");
full.setZoneId(zoneId);
full.setStatus(Backup.Status.BackedUp);
ReflectionTestUtils.setField(full, "id", 50L);
BackupVO inc1 = new BackupVO();
inc1.setVmId(vmId);
inc1.setBackupOfferingId(offeringId);
inc1.setExternalId("i-2-2-VM/2026.05.10.10.30.00");
inc1.setZoneId(zoneId);
inc1.setStatus(Backup.Status.Hidden);
ReflectionTestUtils.setField(inc1, "id", 51L);
BackupVO inc2 = new BackupVO();
inc2.setVmId(vmId);
inc2.setBackupOfferingId(offeringId);
inc2.setExternalId("i-2-2-VM/2026.05.10.11.00.00");
inc2.setZoneId(zoneId);
inc2.setStatus(Backup.Status.BackedUp);
ReflectionTestUtils.setField(inc2, "id", 52L);
VMInstanceVO vm = mock(VMInstanceVO.class);
Mockito.when(vm.getLastHostId()).thenReturn(hostId);
HostVO host = mock(HostVO.class);
Mockito.when(host.getStatus()).thenReturn(Status.Up);
Mockito.when(hostDao.findById(hostId)).thenReturn(host);
BackupRepositoryVO repo = new BackupRepositoryVO(1L, "nas", "test-repo",
"nfs", "address", "sync", 1024L, null);
Mockito.when(backupRepositoryDao.findByBackupOfferingId(offeringId)).thenReturn(repo);
Mockito.when(vmInstanceDao.findByIdIncludingRemoved(vmId)).thenReturn(vm);
Mockito.when(backupDetailsDao.findDetail(50L, NASBackupChainKeys.CHAIN_ID))
.thenReturn(new BackupDetailVO(50L, NASBackupChainKeys.CHAIN_ID, "chain-1", true));
Mockito.when(backupDetailsDao.findDetail(50L, NASBackupChainKeys.CHAIN_POSITION))
.thenReturn(new BackupDetailVO(50L, NASBackupChainKeys.CHAIN_POSITION, "0", true));
Mockito.when(backupDetailsDao.findDetail(51L, NASBackupChainKeys.CHAIN_ID))
.thenReturn(new BackupDetailVO(51L, NASBackupChainKeys.CHAIN_ID, "chain-1", true));
Mockito.when(backupDetailsDao.findDetail(52L, NASBackupChainKeys.CHAIN_ID))
.thenReturn(new BackupDetailVO(52L, NASBackupChainKeys.CHAIN_ID, "chain-1", true));
Mockito.when(backupDetailsDao.findDetail(52L, NASBackupChainKeys.CHAIN_POSITION))
.thenReturn(new BackupDetailVO(52L, NASBackupChainKeys.CHAIN_POSITION, "2", true));
Mockito.when(backupDao.listByVmId(null, vmId)).thenReturn(List.of(full, inc1, inc2));
Mockito.when(backupDao.findById(50L)).thenReturn(full);
Mockito.doReturn(mock(GlobalLock.class)).when(nasBackupProvider).acquireChainDeleteLock(vmId);
Assert.assertTrue(nasBackupProvider.deleteBackup(full, false));
// The full anchor must NOT be physically deleted — inc2 (live) still restores
// through inc1's and full's files. It becomes a tombstone instead.
Mockito.verify(agentManager, Mockito.never()).send(Mockito.anyLong(), Mockito.any(DeleteBackupCommand.class));
Mockito.verify(backupDao, Mockito.never()).remove(Mockito.anyLong());
ArgumentCaptor<BackupVO> captor = ArgumentCaptor.forClass(BackupVO.class);
Mockito.verify(backupDao).update(Mockito.eq(50L), captor.capture());
Assert.assertEquals(Backup.Status.Hidden, captor.getValue().getStatus());
}
@Test
public void sweepContinuesPastFailedTombstoneDelete()
throws AgentUnavailableException, OperationTimedoutException {
Long zoneId = 1L;
Long vmId = 2L;
Long hostId = 3L;
Long offeringId = 4L;
BackupVO full = new BackupVO();
full.setVmId(vmId);
full.setBackupOfferingId(offeringId);
full.setExternalId("i-2-2-VM/2026.05.10.10.00.00");
full.setZoneId(zoneId);
full.setStatus(Backup.Status.Hidden);
ReflectionTestUtils.setField(full, "id", 50L);
BackupVO inc1 = new BackupVO();
inc1.setVmId(vmId);
inc1.setBackupOfferingId(offeringId);
inc1.setExternalId("i-2-2-VM/2026.05.10.10.30.00");
inc1.setZoneId(zoneId);
inc1.setStatus(Backup.Status.Hidden);
ReflectionTestUtils.setField(inc1, "id", 51L);
// The last live member — deleting it triggers the sweep of both tombstones.
BackupVO inc2 = new BackupVO();
inc2.setVmId(vmId);
inc2.setBackupOfferingId(offeringId);
inc2.setExternalId("i-2-2-VM/2026.05.10.11.00.00");
inc2.setZoneId(zoneId);
inc2.setStatus(Backup.Status.BackedUp);
ReflectionTestUtils.setField(inc2, "id", 52L);
VMInstanceVO vm = mock(VMInstanceVO.class);
Mockito.when(vm.getLastHostId()).thenReturn(hostId);
HostVO host = mock(HostVO.class);
Mockito.when(host.getStatus()).thenReturn(Status.Up);
Mockito.when(host.getId()).thenReturn(hostId);
Mockito.when(hostDao.findById(hostId)).thenReturn(host);
BackupRepositoryVO repo = new BackupRepositoryVO(1L, "nas", "test-repo",
"nfs", "address", "sync", 1024L, null);
Mockito.when(backupRepositoryDao.findByBackupOfferingId(offeringId)).thenReturn(repo);
Mockito.when(vmInstanceDao.findByIdIncludingRemoved(vmId)).thenReturn(vm);
Mockito.when(backupDetailsDao.findDetail(50L, NASBackupChainKeys.CHAIN_ID))
.thenReturn(new BackupDetailVO(50L, NASBackupChainKeys.CHAIN_ID, "chain-1", true));
Mockito.when(backupDetailsDao.findDetail(50L, NASBackupChainKeys.CHAIN_POSITION))
.thenReturn(new BackupDetailVO(50L, NASBackupChainKeys.CHAIN_POSITION, "0", true));
Mockito.when(backupDetailsDao.findDetail(51L, NASBackupChainKeys.CHAIN_ID))
.thenReturn(new BackupDetailVO(51L, NASBackupChainKeys.CHAIN_ID, "chain-1", true));
Mockito.when(backupDetailsDao.findDetail(51L, NASBackupChainKeys.CHAIN_POSITION))
.thenReturn(new BackupDetailVO(51L, NASBackupChainKeys.CHAIN_POSITION, "1", true));
Mockito.when(backupDetailsDao.findDetail(52L, NASBackupChainKeys.CHAIN_ID))
.thenReturn(new BackupDetailVO(52L, NASBackupChainKeys.CHAIN_ID, "chain-1", true));
Mockito.when(backupDetailsDao.findDetail(52L, NASBackupChainKeys.CHAIN_POSITION))
.thenReturn(new BackupDetailVO(52L, NASBackupChainKeys.CHAIN_POSITION, "2", true));
Mockito.when(backupDao.listByVmId(null, vmId)).thenReturn(List.of(full, inc1, inc2));
Mockito.when(backupDao.findById(52L)).thenReturn(inc2);
Mockito.doReturn(mock(GlobalLock.class)).when(nasBackupProvider).acquireChainDeleteLock(vmId);
// Deletes run leaf-to-root: inc2 succeeds, inc1's rm FAILS, full succeeds.
DeleteBackupCommand dummy = new DeleteBackupCommand(null, null, null, null);
Mockito.when(agentManager.send(Mockito.anyLong(), Mockito.any(DeleteBackupCommand.class)))
.thenReturn(new BackupAnswer(dummy, true, "ok"),
new BackupAnswer(dummy, false, "rm failed"),
new BackupAnswer(dummy, true, "ok"));
Assert.assertTrue(nasBackupProvider.deleteBackup(inc2, false));
// All three members were attempted; the failed inc1 keeps its row, full is collected.
Mockito.verify(agentManager, Mockito.times(3))
.send(Mockito.anyLong(), Mockito.any(DeleteBackupCommand.class));
Mockito.verify(backupDao).remove(52L);
Mockito.verify(backupDao, Mockito.never()).remove(51L);
Mockito.verify(backupDao).remove(50L);
}
}

37
test/integration/smoke/test_backup_recovery_nas.py
View File

@ -56,13 +56,17 @@ class TestNASBackupAndRecovery(cloudstackTestCase):
# Check backup configuration values, set them to enable the nas provider
backup_enabled_cfg = Configurations.list(cls.api_client, name='backup.framework.enabled')
backup_provider_cfg = Configurations.list(cls.api_client, name='backup.framework.provider.plugin')
incremental_backup_enabled_cfg = Configurations.list(cls.api_client, name='nas.backup.incremental.enabled')
cls.backup_enabled = backup_enabled_cfg[0].value
cls.backup_provider = backup_provider_cfg[0].value
cls.incremental_backup_enabled = incremental_backup_enabled_cfg[0].value
if cls.backup_enabled == "false":
cls.skipTest(cls, reason="Test can be run only if the config backup.framework.enabled is true")
if cls.backup_provider != "nas":
Configurations.update(cls.api_client, 'backup.framework.provider.plugin', value='nas')
if cls.incremental_backup_enabled == "false":
Configurations.update(cls.api_client, 'nas.backup.incremental.enabled', value='true')
cls.account = Account.create(cls.api_client, cls.services["account"], domainid=cls.domain.id)
@ -94,6 +98,8 @@ class TestNASBackupAndRecovery(cloudstackTestCase):
if cls.backup_provider != "nas":
Configurations.update(cls.api_client, 'backup.framework.provider.plugin', value=cls.backup_provider)
if cls.incremental_backup_enabled == "false":
Configurations.update(cls.api_client, 'nas.backup.incremental.enabled', value="false")
except Exception as e:
raise Exception("Warning: Exception during cleanup : %s" % e)
@ -519,10 +525,11 @@ class TestNASBackupAndRecovery(cloudstackTestCase):
self.backup_offering.removeOffering(self.apiclient, self.vm.id)
@attr(tags=["advanced", "backup"], required_hardware="true")
def test_refuse_delete_full_with_children(self):
def test_delete_full_with_children_is_deferred(self):
"""
Deleting a FULL that has surviving incrementals must fail without forced=true.
With forced=true it must succeed and remove the entire chain.
Deleting a FULL that has surviving incrementals succeeds but is deferred: the
FULL is hidden from the backup list while its child survives, and it is
physically swept once the last descendant is deleted.
"""
self.backup_offering.assignOffering(self.apiclient, self.vm.id)
original_full_every = self._get_full_every()
@ -535,19 +542,21 @@ class TestNASBackupAndRecovery(cloudstackTestCase):
backups = Backup.list(self.apiclient, self.vm.id)
backups.sort(key=lambda b: b.created)
full = backups[0]
full, inc = backups[0], backups[1]
failed = False
try:
Backup.delete(self.apiclient, full.id)
except Exception:
failed = True
self.assertTrue(failed, "Deleting a FULL with children should be refused without forced=true")
# Forced delete should succeed and clear the whole chain
Backup.delete(self.apiclient, full.id, forced=True)
# Non-forced delete of the FULL is accepted but deferred: the FULL is
# tombstoned (Hidden) and disappears from the list, the child survives.
Backup.delete(self.apiclient, full.id)
remaining = Backup.list(self.apiclient, self.vm.id)
self.assertIsNone(remaining, "Forced delete of FULL should remove the entire chain")
self.assertEqual(len(remaining), 1,
"Deleting a FULL with children should hide the FULL and keep the child")
self.assertEqual(remaining[0].id, inc.id,
"The surviving backup should be the incremental child")
# Deleting the last descendant sweeps the delete-pending FULL with it.
Backup.delete(self.apiclient, inc.id)
remaining = Backup.list(self.apiclient, self.vm.id)
self.assertIsNone(remaining, "Deleting the leaf should sweep the hidden FULL, removing the chain")
finally:
self._set_full_every(original_full_every)
self.backup_offering.removeOffering(self.apiclient, self.vm.id)