Skip to content

Commit 65c0446

Browse files
committed
NAS backup: keep the Failed row when the agent reports a failed backup
#13180 made takeBackup keep the backup row in Failed state instead of deleting it. The forward merge of 4.22 after #14305 resolved the failed-answer branch to the 4.22 version, which deletes the row again. Restore the update and return the row to the caller as the Error branch does, so the schedule, name and resource counts are recorded on it.
1 parent 1abb539 commit 65c0446

2 files changed

Lines changed: 7 additions & 5 deletions

File tree

‎plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -669,8 +669,8 @@ public Pair<Boolean, Backup> takeBackup(final VirtualMachine vm, Boolean quiesce
669669
return new Pair<>(false, backupVO);
670670
}
671671
backupVO.setStatus(Backup.Status.Failed);
672-
backupDao.remove(backupVO.getId());
673-
return new Pair<>(false, null);
672+
backupDao.update(backupVO.getId(), backupVO);
673+
return new Pair<>(false, backupVO);
674674
}
675675
}
676676

‎plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ public void takeBackupFailureNeedingCleanupReturnsErrorBackup() throws AgentUnav
311311
}
312312

313313
@Test
314-
public void takeBackupFailureWithoutCleanupRemovesBackup() throws AgentUnavailableException, OperationTimedoutException {
314+
public void takeBackupFailureWithoutCleanupKeepsFailedBackup() throws AgentUnavailableException, OperationTimedoutException {
315315
VMInstanceVO vm = mockRunningVmForBackup(1L, 2L, 3L, 4L, 5L, 6L);
316316

317317
BackupAnswer answer = mock(BackupAnswer.class);
@@ -324,8 +324,10 @@ public void takeBackupFailureWithoutCleanupRemovesBackup() throws AgentUnavailab
324324
Pair<Boolean, Backup> result = nasBackupProvider.takeBackup(vm, false, false);
325325

326326
Assert.assertFalse(result.first());
327-
Assert.assertNull(result.second());
328-
Mockito.verify(backupDao).remove(Mockito.anyLong());
327+
Assert.assertNotNull(result.second());
328+
Assert.assertEquals(Backup.Status.Failed, result.second().getStatus());
329+
Mockito.verify(backupDao).update(Mockito.anyLong(), Mockito.any(BackupVO.class));
330+
Mockito.verify(backupDao, Mockito.never()).remove(Mockito.anyLong());
329331
Mockito.verifyNoInteractions(alertManager);
330332
}
331333

0 commit comments

Comments
 (0)