From adb56cf4ed06dfd8c785d9360aa4cd50aa6c17d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Thu, 23 Jul 2026 16:18:46 -0300 Subject: [PATCH 1/4] revert 71e001d7a0d0f92d3a77cb0738006186e2d7c16a --- .../cloudstack/backup/BackupProvider.java | 12 +- .../backup/InternalBackupProvider.java | 2 +- .../backup/CleanupKbossBackupErrorAnswer.java | 18 +- .../CleanupKbossBackupErrorCommand.java | 23 +- .../apache/cloudstack/storage/to/KbossTO.java | 25 +- .../cloudstack/storage/to/VolumeObjectTO.java | 9 - .../backup/InternalBackupJoinVO.java | 21 - .../backup/dao/InternalBackupJoinDao.java | 10 +- .../backup/dao/InternalBackupJoinDaoImpl.java | 34 +- .../dao/InternalBackupStoragePoolDao.java | 6 +- .../dao/InternalBackupStoragePoolDaoImpl.java | 18 +- .../db/views/cloud.internal_backup_view.sql | 8 +- ...KvmFileBasedStorageVmSnapshotStrategy.java | 140 ++--- .../backup/DummyBackupProvider.java | 2 +- .../backup/KbossBackupProvider.java | 544 +++++++----------- .../backup/KbossBackupProviderTest.java | 328 +++++++---- .../cloudstack/backup/NASBackupProvider.java | 2 +- .../backup/NASBackupProviderTest.java | 2 +- .../backup/NetworkerBackupProvider.java | 2 +- .../backup/VeeamBackupProvider.java | 2 +- ...irtCleanupKbossVmBackupCommandWrapper.java | 215 +++---- .../LibvirtRevertSnapshotCommandWrapper.java | 9 +- .../LibvirtTakeKbossBackupCommandWrapper.java | 7 +- .../kvm/storage/KVMStorageProcessor.java | 9 +- ...bvirTakeKbossBackupCommandWrapperTest.java | 2 +- .../cloudstack/backup/BackupManagerImpl.java | 31 +- .../backup/InternalBackupServiceImpl.java | 37 +- .../cloudstack/backup/BackupManagerTest.java | 47 +- .../backup/InternalBackupServiceImplTest.java | 79 ++- 29 files changed, 690 insertions(+), 954 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java b/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java index ffc32b6c4d08..66e4501c460e 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java +++ b/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java @@ -63,16 +63,6 @@ public interface BackupProvider { */ boolean removeVMFromBackupOffering(VirtualMachine vm); - /** - * Removes the specified backup schedule from a virtual machine. - * - * @param vm the virtual machine from which the schedule will be removed. - * @param backupSchedule the backup schedule to be removed. - * @return {@code true} if the operation was successful; {@code false} otherwise. - */ - default boolean removeVMBackupSchedule(VirtualMachine vm, BackupSchedule backupSchedule) { - return true; - } /** * Whether the provider will delete backups on removal of VM from the offering @@ -91,7 +81,7 @@ default boolean removeVMBackupSchedule(VirtualMachine vm, BackupSchedule backupS * @param isolated * @return the result and {code}Backup{code} {code}Object{code} */ - Pair takeBackup(VirtualMachine vm, Boolean quiesceVM, boolean isolated, Long backupScheduleId); + Pair takeBackup(VirtualMachine vm, Boolean quiesceVM, boolean isolated); /** * Delete an existing backup diff --git a/api/src/main/java/org/apache/cloudstack/backup/InternalBackupProvider.java b/api/src/main/java/org/apache/cloudstack/backup/InternalBackupProvider.java index efd28385a3e8..0543fb10af36 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/InternalBackupProvider.java +++ b/api/src/main/java/org/apache/cloudstack/backup/InternalBackupProvider.java @@ -136,7 +136,7 @@ default Set getSecondaryStorageUrls(UserVm userVm) { default void prepareVmForSnapshotRevert(VMSnapshot vmSnapshot, VirtualMachine virtualMachine) { } - default boolean finishBackupChains(VirtualMachine virtualMachine) { + default boolean finishBackupChain(VirtualMachine virtualMachine) { return false; } } diff --git a/core/src/main/java/org/apache/cloudstack/backup/CleanupKbossBackupErrorAnswer.java b/core/src/main/java/org/apache/cloudstack/backup/CleanupKbossBackupErrorAnswer.java index 042047b59358..34b65adc2467 100644 --- a/core/src/main/java/org/apache/cloudstack/backup/CleanupKbossBackupErrorAnswer.java +++ b/core/src/main/java/org/apache/cloudstack/backup/CleanupKbossBackupErrorAnswer.java @@ -18,26 +18,26 @@ */ package org.apache.cloudstack.backup; -import java.util.Map; +import java.util.List; -import org.apache.commons.collections4.MapUtils; +import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.commons.collections4.CollectionUtils; import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; -import com.cloud.utils.Pair; public class CleanupKbossBackupErrorAnswer extends Answer { - private Map> volumeIdToPathAndChainEnded; + private List volumeObjectTos; private boolean vmRunning; - public CleanupKbossBackupErrorAnswer(Command cmd, Map> volumeIdToPathAndChainEnded, boolean vmRunning) { - super(cmd, MapUtils.isNotEmpty(volumeIdToPathAndChainEnded), null); - this.volumeIdToPathAndChainEnded = volumeIdToPathAndChainEnded; + public CleanupKbossBackupErrorAnswer(Command cmd, List volumeObjectTos, boolean vmRunning) { + super(cmd, CollectionUtils.isNotEmpty(volumeObjectTos), null); + this.volumeObjectTos = volumeObjectTos; this.vmRunning = vmRunning; } - public Map> getVolumeIdToPathAndChainEnded() { - return volumeIdToPathAndChainEnded; + public List getVolumeObjectTos() { + return volumeObjectTos; } public boolean isVmRunning() { diff --git a/core/src/main/java/org/apache/cloudstack/backup/CleanupKbossBackupErrorCommand.java b/core/src/main/java/org/apache/cloudstack/backup/CleanupKbossBackupErrorCommand.java index e5cd5a7f8150..3257851d11ba 100644 --- a/core/src/main/java/org/apache/cloudstack/backup/CleanupKbossBackupErrorCommand.java +++ b/core/src/main/java/org/apache/cloudstack/backup/CleanupKbossBackupErrorCommand.java @@ -25,40 +25,19 @@ public class CleanupKbossBackupErrorCommand extends Command { private boolean runningVM; - private boolean errorOnCreate; - - private boolean endOfChain; - - private boolean isTopDelta; - private String vmName; private String imageStoreUrl; private List kbossTOS; - public CleanupKbossBackupErrorCommand(boolean runningVM, boolean errorOnCreate, boolean endOfChain, boolean isTopDelta, String vmName, String imageStoreUrl, List kbossTOS) { - this.errorOnCreate = errorOnCreate; + public CleanupKbossBackupErrorCommand(boolean runningVM, String vmName, String imageStoreUrl, List kbossTOS) { this.runningVM = runningVM; - this.endOfChain = endOfChain; - this.isTopDelta = isTopDelta; this.vmName = vmName; this.imageStoreUrl = imageStoreUrl; this.kbossTOS = kbossTOS; } - public boolean isErrorOnCreate() { - return errorOnCreate; - } - - public boolean isEndOfChain() { - return endOfChain; - } - - public boolean isTopDelta() { - return isTopDelta; - } - public boolean isRunningVM() { return runningVM; } diff --git a/core/src/main/java/org/apache/cloudstack/storage/to/KbossTO.java b/core/src/main/java/org/apache/cloudstack/storage/to/KbossTO.java index 4a3f53a6dad4..0280ffa27a1f 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/to/KbossTO.java +++ b/core/src/main/java/org/apache/cloudstack/storage/to/KbossTO.java @@ -16,9 +16,10 @@ // specific language governing permissions and limitations // under the License. -import java.util.LinkedList; import java.util.List; +import java.util.stream.Collectors; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; import org.apache.commons.lang3.builder.ReflectionToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; @@ -29,22 +30,20 @@ public class KbossTO { private String deltaPathOnPrimary; private String parentDeltaPathOnPrimary; private String deltaPathOnSecondary; - private String oldVolumePath; private DeltaMergeTreeTO deltaMergeTreeTO; - private List deltaPaths; + private List vmSnapshotDeltaPaths; - public KbossTO(VolumeObjectTO volumeObjectTO, LinkedList deltaPaths) { + public KbossTO(VolumeObjectTO volumeObjectTO, List snapshotDataStoreVOs) { this.volumeObjectTO = volumeObjectTO; - this.deltaPaths = deltaPaths; + this.vmSnapshotDeltaPaths = snapshotDataStoreVOs.stream().map(SnapshotDataStoreVO::getInstallPath).collect(Collectors.toList()); } - public KbossTO(VolumeObjectTO volumeObjectTO, String deltaPathOnPrimary, String deltaPathOnSecondary, LinkedList deltaPaths) { + public KbossTO(VolumeObjectTO volumeObjectTO, String deltaPathOnPrimary, String deltaPathOnSecondary) { this.volumeObjectTO = volumeObjectTO; this.deltaPathOnPrimary = deltaPathOnPrimary; this.deltaPathOnSecondary = deltaPathOnSecondary; - this.deltaPaths = deltaPaths; } public String getPathBackupParentOnSecondary() { @@ -59,8 +58,8 @@ public DeltaMergeTreeTO getDeltaMergeTreeTO() { return deltaMergeTreeTO; } - public List getDeltaPaths() { - return deltaPaths; + public List getVmSnapshotDeltaPaths() { + return vmSnapshotDeltaPaths; } public String getDeltaPathOnPrimary() { @@ -95,14 +94,6 @@ public void setDeltaPathOnSecondary(String deltaPathOnSecondary) { this.deltaPathOnSecondary = deltaPathOnSecondary; } - public String getOldVolumePath() { - return oldVolumePath; - } - - public void setOldVolumePath(String oldVolumePath) { - this.oldVolumePath = oldVolumePath; - } - @Override public String toString() { return ReflectionToStringBuilder.toString(this, ToStringStyle.JSON_STYLE); diff --git a/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java b/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java index 5b1d4c573b68..df98149faab6 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java +++ b/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java @@ -81,7 +81,6 @@ public class VolumeObjectTO extends DownloadableObjectTO implements DataTO, Seri private String encryptFormat; private List checkpointPaths; private Set checkpointImageStoreUrls; - private Set deltasToRemove; public VolumeObjectTO() { @@ -426,12 +425,4 @@ public Set getCheckpointImageStoreUrls() { public void setCheckpointImageStoreUrls(Set checkpointImageStoreUrls) { this.checkpointImageStoreUrls = checkpointImageStoreUrls; } - - public Set getDeltasToRemove() { - return deltasToRemove; - } - - public void setDeltasToRemove(Set deltasToRemove) { - this.deltasToRemove = deltasToRemove; - } } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/InternalBackupJoinVO.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/InternalBackupJoinVO.java index e9e232ef2023..acc54ccd41fa 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/InternalBackupJoinVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/InternalBackupJoinVO.java @@ -101,15 +101,6 @@ public class InternalBackupJoinVO { @Column(name = "isolated") private Boolean isolated; - @Column(name = "storage_pool_delta_path") - private String storagePoolDeltaPath; - - @Column(name = "storage_pool_parent_path") - private String storagePoolParentPath; - - @Column(name = "schedule_id") - private Long scheduleId; - public InternalBackupJoinVO() { } @@ -192,18 +183,6 @@ public Backup.CompressionStatus getCompressionStatus() { return compressionStatus; } - public String getStoragePoolDeltaPath() { - return storagePoolDeltaPath; - } - - public String getStoragePoolParentPath() { - return storagePoolParentPath; - } - - public Long getScheduleId() { - return scheduleId; - } - @Override public String toString() { return ReflectionToStringBuilder.toString(this, ToStringStyle.JSON_STYLE); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupJoinDao.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupJoinDao.java index 3cb6ea241041..2f9c3dd1ff8f 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupJoinDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupJoinDao.java @@ -24,15 +24,11 @@ public interface InternalBackupJoinDao extends GenericDao { - List listByBackedUpAndVmIdAndDateBeforeOrAfterOrderBy(long vmId, Long scheduleId, Date date, boolean before, boolean ascending); + List listByBackedUpAndVmIdAndDateBeforeOrAfterOrderBy(long vmId, Date date, boolean before, boolean ascending); - List listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(long vmId, Long scheduleId, Date beforeDate); + List listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(long vmId, Date beforeDate); - InternalBackupJoinVO findCurrent(long vmId, Long scheduleId); - - List listCurrents(long vmId, boolean descending); - - List listCurrentsByVolumeIdDesc(long volumeId); + InternalBackupJoinVO findCurrent(long vmId); InternalBackupJoinVO findByParentId(long parentId); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupJoinDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupJoinDaoImpl.java index bbc91db13e83..e8d823bb69c5 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupJoinDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupJoinDaoImpl.java @@ -39,8 +39,6 @@ public class InternalBackupJoinDaoImpl extends GenericDaoBase backupSearch; private SearchBuilder allBackupsSearch; @@ -54,7 +52,6 @@ protected void init() { backupSearch.and(CURRENT, backupSearch.entity().getCurrent(), SearchCriteria.Op.EQ); backupSearch.and(PARENT_ID, backupSearch.entity().getParentId(), SearchCriteria.Op.EQ); backupSearch.and(ISOLATED, backupSearch.entity().getIsolated(), SearchCriteria.Op.EQ); - backupSearch.and(SCHEDULE_ID, backupSearch.entity().getScheduleId(), SearchCriteria.Op.EQ); backupSearch.groupBy(backupSearch.entity().getId()); backupSearch.done(); @@ -63,14 +60,11 @@ protected void init() { allBackupsSearch.and(STATUS, allBackupsSearch.entity().getStatus(), SearchCriteria.Op.IN); allBackupsSearch.and(PARENT_ID, allBackupsSearch.entity().getParentId(), SearchCriteria.Op.EQ); allBackupsSearch.and(IMAGE_STORE_ID, allBackupsSearch.entity().getImageStoreId(), SearchCriteria.Op.EQ); - allBackupsSearch.and(VM_ID, allBackupsSearch.entity().getVmId(), SearchCriteria.Op.EQ); - allBackupsSearch.and(VOLUME_ID, allBackupsSearch.entity().getVolumeId(), SearchCriteria.Op.EQ); - allBackupsSearch.and(CURRENT, allBackupsSearch.entity().getCurrent(), SearchCriteria.Op.EQ); allBackupsSearch.done(); } @Override - public List listByBackedUpAndVmIdAndDateBeforeOrAfterOrderBy(long vmId, Long scheduleId, Date date, boolean before, boolean ascending) { + public List listByBackedUpAndVmIdAndDateBeforeOrAfterOrderBy(long vmId, Date date, boolean before, boolean ascending) { SearchCriteria sc = backupSearch.create(); sc.setParameters(VM_ID, vmId); sc.setParameters(STATUS, Backup.Status.BackedUp); @@ -80,29 +74,26 @@ public List listByBackedUpAndVmIdAndDateBeforeOrAfterOrder sc.setParameters(CREATED_AFTER, date); } sc.setParameters(ISOLATED, Boolean.FALSE.toString()); - sc.setParameters(SCHEDULE_ID, scheduleId); Filter filter = new Filter(InternalBackupJoinVO.class, "date", ascending); return new ArrayList<>(listBy(sc, filter)); } @Override - public List listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(long vmId, Long scheduleId, Date beforeDate) { + public List listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(long vmId, Date beforeDate) { SearchCriteria sc = backupSearch.create(); sc.setParameters(VM_ID, vmId); sc.setParameters(STATUS, Backup.Status.BackedUp, Backup.Status.Removed); sc.setParameters(CREATED_BEFORE, beforeDate); sc.setParameters(ISOLATED, Boolean.FALSE.toString()); - sc.setParameters(SCHEDULE_ID, scheduleId); Filter filter = new Filter(InternalBackupJoinVO.class, "date", false); return new ArrayList<>(listIncludingRemovedBy(sc, filter)); } @Override - public InternalBackupJoinVO findCurrent(long vmId, Long scheduleId) { + public InternalBackupJoinVO findCurrent(long vmId) { SearchCriteria sc = backupSearch.create(); sc.setParameters(VM_ID, vmId); sc.setParameters(CURRENT, Boolean.TRUE.toString()); - sc.setParameters(SCHEDULE_ID, scheduleId); return findOneBy(sc); } @@ -136,23 +127,4 @@ public List listByParentId(long parentId) { sc.setParameters(STATUS, Backup.Status.BackedUp); return listBy(sc); } - - @Override - public List listCurrents(long vmId, boolean descending) { - SearchCriteria sc = allBackupsSearch.create(); - sc.setParameters(VM_ID, vmId); - sc.setParameters(CURRENT, Boolean.TRUE.toString()); - Filter filter = new Filter(InternalBackupJoinVO.class, "date", !descending); - - return listBy(sc, filter); - } - - @Override - public List listCurrentsByVolumeIdDesc(long volumeId) { - SearchCriteria sc = allBackupsSearch.create(); - sc.setParameters(VOLUME_ID, volumeId); - sc.setParameters(CURRENT, Boolean.TRUE.toString()); - Filter filter = new Filter(InternalBackupJoinVO.class, "date", false); - return listBy(sc, filter); - } } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupStoragePoolDao.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupStoragePoolDao.java index e6628f78af8c..7e2ac5a249e4 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupStoragePoolDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupStoragePoolDao.java @@ -25,13 +25,9 @@ public interface InternalBackupStoragePoolDao extends GenericDao listByBackupId(long backupId); - List listByVolumeId(long volumeId); - - InternalBackupStoragePoolVO findOneByVolumeIdAndBackupId(long volumeId, long backupId); + InternalBackupStoragePoolVO findOneByVolumeId(long volumeId); void expungeByBackupId(long backupId); void expungeByVolumeId(long volumeId); - - void expungeByVolumeIdAndBackupId(long volumeId, long backupId); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupStoragePoolDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupStoragePoolDaoImpl.java index 41002aeb51d2..443ceed02e7d 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupStoragePoolDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/InternalBackupStoragePoolDaoImpl.java @@ -48,17 +48,9 @@ public List listByBackupId(long backupId) { } @Override - public List listByVolumeId(long volumeId) { + public InternalBackupStoragePoolVO findOneByVolumeId(long volumeId) { SearchCriteria sc = backupSearch.create(); sc.setParameters(VOLUME_ID, volumeId); - return listBy(sc); - } - - @Override - public InternalBackupStoragePoolVO findOneByVolumeIdAndBackupId(long volumeId, long backupId) { - SearchCriteria sc = backupSearch.create(); - sc.setParameters(VOLUME_ID, volumeId); - sc.setParameters(BACKUP_ID, backupId); return findOneBy(sc); } @@ -75,12 +67,4 @@ public void expungeByVolumeId(long volumeId) { sc.setParameters(VOLUME_ID, volumeId); expunge(sc); } - - @Override - public void expungeByVolumeIdAndBackupId(long volumeId, long backupId) { - SearchCriteria sc = backupSearch.create(); - sc.setParameters(VOLUME_ID, volumeId); - sc.setParameters(BACKUP_ID, backupId); - expunge(sc); - } } diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.internal_backup_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.internal_backup_view.sql index 9e6be1fc5b7f..a1fbd102630b 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.internal_backup_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.internal_backup_view.sql @@ -37,15 +37,11 @@ SELECT b.id, MAX(CASE WHEN bd.name = 'current' THEN bd.value END) current, COALESCE(MAX(CASE WHEN bd.name = 'isolated' THEN bd.value END), 'false') isolated, nbpr.volume_id, - nbpr.backup_delta_path storage_pool_delta_path, - nbpr.backup_parent_path storage_pool_parent_path, - nbsr.path image_store_path, - bs.id schedule_id + nbsr.path image_store_path FROM backups b LEFT JOIN backup_details bd ON b.id = bd.backup_id LEFT JOIN backup_offering bo ON b.backup_offering_id = bo.id LEFT JOIN internal_backup_store_ref nbsr ON b.id = nbsr.backup_id -LEFT JOIN internal_backup_pool_ref nbpr ON nbpr.volume_id = nbsr.volume_id and nbpr.backup_id = b.id -LEFT JOIN backup_schedule bs ON bs.id = b.backup_schedule_id +LEFT JOIN internal_backup_pool_ref nbpr ON nbpr.volume_id = nbsr.volume_id WHERE bo.provider='kboss' GROUP BY b.id, nbsr.volume_id; diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java index 15f16ae7b4ba..14e6e2b12edc 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java @@ -18,6 +18,40 @@ */ package org.apache.cloudstack.storage.vmsnapshot; +import java.util.ArrayList; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Objects; +import java.util.UUID; +import java.util.stream.Collectors; + +import javax.inject.Inject; + +import org.apache.cloudstack.backup.BackupManagerImpl; +import org.apache.cloudstack.backup.BackupOfferingVO; +import org.apache.cloudstack.backup.InternalBackupService; +import org.apache.cloudstack.backup.InternalBackupStoragePoolVO; +import org.apache.cloudstack.backup.dao.BackupOfferingDao; +import org.apache.cloudstack.backup.dao.InternalBackupStoragePoolDao; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider; +import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; +import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotOptions; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.snapshot.SnapshotObject; +import org.apache.cloudstack.storage.to.BackupDeltaTO; +import org.apache.cloudstack.storage.to.DeltaMergeTreeTO; +import org.apache.cloudstack.storage.to.SnapshotObjectTO; +import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.commons.collections.CollectionUtils; + import com.cloud.agent.api.Answer; import com.cloud.agent.api.VMSnapshotTO; import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotAnswer; @@ -49,40 +83,6 @@ import com.cloud.vm.snapshot.VMSnapshot; import com.cloud.vm.snapshot.VMSnapshotDetailsVO; import com.cloud.vm.snapshot.VMSnapshotVO; -import org.apache.cloudstack.backup.BackupManagerImpl; -import org.apache.cloudstack.backup.BackupOfferingVO; -import org.apache.cloudstack.backup.InternalBackupJoinVO; -import org.apache.cloudstack.backup.InternalBackupService; -import org.apache.cloudstack.backup.InternalBackupStoragePoolVO; -import org.apache.cloudstack.backup.dao.BackupOfferingDao; -import org.apache.cloudstack.backup.dao.InternalBackupJoinDao; -import org.apache.cloudstack.backup.dao.InternalBackupStoragePoolDao; -import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider; -import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; -import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; -import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; -import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotOptions; -import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; -import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; -import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; -import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.snapshot.SnapshotObject; -import org.apache.cloudstack.storage.to.BackupDeltaTO; -import org.apache.cloudstack.storage.to.DeltaMergeTreeTO; -import org.apache.cloudstack.storage.to.SnapshotObjectTO; -import org.apache.cloudstack.storage.to.VolumeObjectTO; -import org.apache.commons.collections.CollectionUtils; - -import javax.inject.Inject; -import java.util.ArrayList; -import java.util.Date; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.NoSuchElementException; -import java.util.Objects; -import java.util.UUID; -import java.util.stream.Collectors; public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStrategy { @@ -106,8 +106,6 @@ public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStra @Inject private SnapshotDao snapshotDao; - @Inject - private InternalBackupJoinDao internalBackupJoinDao; @Override public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { @@ -153,7 +151,7 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { List volumeSnapshotVos = new ArrayList<>(); if (isCurrent && numberOfChildren == 0) { - volumeSnapshotVos = mergeSucceedingDeltaOnSnapshot(vmSnapshotBeingDeleted, userVm, hostId, volumeTOs); + volumeSnapshotVos = mergeCurrentDeltaOnSnapshot(vmSnapshotBeingDeleted, userVm, hostId, volumeTOs); } else if (numberOfChildren == 0) { logger.debug("Deleting VM snapshot [{}] as no snapshots/volumes depend on it.", vmSnapshot.getUuid()); volumeSnapshotVos = deleteSnapshot(vmSnapshotBeingDeleted, hostId); @@ -278,7 +276,7 @@ private void mergeOldSiblingWithOldParentIfOldParentIsDead(VMSnapshotVO oldParen List snapshotVos; if (oldParent.getCurrent()) { - snapshotVos = mergeSucceedingDeltaOnSnapshot(oldParent, userVm, hostId, volumeTOs); + snapshotVos = mergeCurrentDeltaOnSnapshot(oldParent, userVm, hostId, volumeTOs); } else { List oldSiblings = vmSnapshotDao.listByParentAndStateIn(oldParent.getId(), VMSnapshot.State.Ready, VMSnapshot.State.Hidden); @@ -426,7 +424,7 @@ private List mergeSnapshots(VMSnapshotVO vmSnapshotVO, VMSnapshotVO SnapshotObjectTO parentTO = (SnapshotObjectTO) deltaMergeTreeTO.getParent(); if (childTO instanceof BackupDeltaTO) { - InternalBackupStoragePoolVO backupDelta = internalBackupStoragePoolDao.findOneByVolumeIdAndBackupId(parentTO.getVolume().getVolumeId(), childTO.getId()); + InternalBackupStoragePoolVO backupDelta = internalBackupStoragePoolDao.findOneByVolumeId(parentTO.getVolume().getVolumeId()); backupDelta.setBackupDeltaParentPath(parentTO.getPath()); logger.debug("The child was also a KBOSS backup delta, will update the backup delta metadata. Updating backupDeltaParentPath of backupDelta [{}] to [{}].", backupDelta.getId(), parentTO.getPath()); internalBackupStoragePoolDao.update(backupDelta.getId(), backupDelta); @@ -447,27 +445,26 @@ private List mergeSnapshots(VMSnapshotVO vmSnapshotVO, VMSnapshotVO return snapshotVOList; } - private List mergeSucceedingDeltaOnSnapshot(VMSnapshotVO vmSnapshotVo, UserVmVO userVmVO, Long hostId, List volumeObjectTOS) { - logger.debug(String.format("Merging VM snapshot [%s] with the succeeding delta.", vmSnapshotVo.getUuid())); + private List mergeCurrentDeltaOnSnapshot(VMSnapshotVO vmSnapshotVo, UserVmVO userVmVO, Long hostId, List volumeObjectTOS) { + logger.debug("Merging VM snapshot [{}] with the current volume delta.", vmSnapshotVo.getUuid()); List deltaMergeTreeTOs = new ArrayList<>(); List volumeSnapshots = vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(vmSnapshotVo.getId()); - Map volumeIdAndSucceedingBackupMap = getVolumeIdAndSucceedingBackupMap(vmSnapshotVo); for (VolumeObjectTO volumeObjectTO : volumeObjectTOS) { - Long volumeId = volumeObjectTO.getId(); - SnapshotDataStoreVO volumeParentSnapshot = volumeSnapshots.stream().filter(snapshot -> Objects.equals(snapshot.getVolumeId(), volumeId)) + SnapshotDataStoreVO volumeParentSnapshot = volumeSnapshots.stream().filter(snapshot -> Objects.equals(snapshot.getVolumeId(), volumeObjectTO.getId())) .findFirst() .orElseThrow(() -> new CloudRuntimeException(String.format("Failed to find volume snapshot for volume [%s].", volumeObjectTO.getUuid()))); DataTO parentSnapshot = snapshotDataFactory.getSnapshot(volumeParentSnapshot.getSnapshotId(), volumeParentSnapshot.getDataStoreId(), DataStoreRole.Primary).getTO(); - if (volumeIdAndSucceedingBackupMap.containsKey(volumeId)) { - InternalBackupJoinVO succeedingBackup = volumeIdAndSucceedingBackupMap.get(volumeId); - logger.debug("The succeeding delta is also a KNIB backup delta. Will merge the snapshot delta of volume [{}] with the parent backup delta at [{}].", - volumeObjectTO.getUuid(), succeedingBackup.getStoragePoolParentPath()); - BackupDeltaTO childTo = new BackupDeltaTO(succeedingBackup.getId(), volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, succeedingBackup.getStoragePoolParentPath()); + InternalBackupStoragePoolVO backupDelta = internalBackupStoragePoolDao.findOneByVolumeId(volumeObjectTO.getVolumeId()); + + if (backupDelta != null && backupDelta.getBackupDeltaPath().equals(volumeObjectTO.getPath())) { + logger.debug("The current volume delta is also a KBOSS backup delta. Will merge the snapshot delta of volume [{}] with the parent backup delta at [{}].", + volumeObjectTO.getUuid(), backupDelta.getBackupDeltaParentPath()); + BackupDeltaTO childTo = new BackupDeltaTO(volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, backupDelta.getBackupDeltaParentPath()); ArrayList grandChildren = new ArrayList<>(); if (userVmVO.getState().equals(VirtualMachine.State.Stopped)) { - grandChildren.add(new BackupDeltaTO(volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, succeedingBackup.getStoragePoolDeltaPath())); + grandChildren.add(new BackupDeltaTO(volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, backupDelta.getBackupDeltaPath())); } deltaMergeTreeTOs.add(new DeltaMergeTreeTO(volumeObjectTO, parentSnapshot, childTo, grandChildren)); } else { @@ -491,7 +488,7 @@ private List mergeSucceedingDeltaOnSnapshot(VMSnapshotVO vmSnapshotV if (dataTO instanceof BackupDeltaTO) { logger.debug("The child of deltaMergeTree [{}] is a backupDeltaTO, thus, we will update the backup delta metadata.", deltaMergeTreeTO); - InternalBackupStoragePoolVO backupDelta = internalBackupStoragePoolDao.findOneByVolumeIdAndBackupId(parentTO.getVolume().getVolumeId(), dataTO.getId()); + InternalBackupStoragePoolVO backupDelta = internalBackupStoragePoolDao.findOneByVolumeId(parentTO.getVolume().getVolumeId()); backupDelta.setBackupDeltaParentPath(parentTO.getPath()); internalBackupStoragePoolDao.update(backupDelta.getId(), backupDelta); } else { @@ -655,7 +652,6 @@ private List generateDeltaMergeTrees(VMSnapshotVO parent, VMSn List parentVolumeSnapshots = vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(parent.getId()); List childVolumeSnapshots = vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(child.getId()); List grandChildrenVolumeSnapshots = new ArrayList<>(); - Map volumeIdAndSucceedingBackupMap = getVolumeIdAndSucceedingBackupMap(parent); for (VMSnapshotVO grandChild : grandChildren) { grandChildrenVolumeSnapshots.addAll(vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(grandChild.getId())); @@ -664,14 +660,14 @@ private List generateDeltaMergeTrees(VMSnapshotVO parent, VMSn for (SnapshotDataStoreVO parentSnapshotDataStoreVO : parentVolumeSnapshots) { SnapshotObjectTO parentTO = (SnapshotObjectTO) snapshotDataFactory.getSnapshot(parentSnapshotDataStoreVO.getSnapshotId(), parentSnapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary).getTO(); VolumeObjectTO volumeObjectTO = parentTO.getVolume(); - InternalBackupJoinVO succeedingBackup = volumeIdAndSucceedingBackupMap.get(volumeObjectTO.getId()); SnapshotDataStoreVO childVO = childVolumeSnapshots.stream() .filter(childSnapshot -> Objects.equals(parentSnapshotDataStoreVO.getVolumeId(), childSnapshot.getVolumeId())) .findFirst().orElseThrow(() -> new CloudRuntimeException(String.format("Could not find child snapshot of parent [%s].", parentSnapshotDataStoreVO.getSnapshotId()))); + InternalBackupStoragePoolVO backupDelta = internalBackupStoragePoolDao.findOneByVolumeId(childVO.getVolumeId()); List grandChildrenTOList = new ArrayList<>(); - DataTO childTO = getChildAndGrandChildren(child, stoppedVm, parentSnapshotDataStoreVO, succeedingBackup, childVO, volumeObjectTO, grandChildrenTOList, + DataTO childTO = getChildAndGrandChildren(child, stoppedVm, parentSnapshotDataStoreVO, backupDelta, childVO, volumeObjectTO, grandChildrenTOList, grandChildrenVolumeSnapshots); snapshotMergeTrees.add(new DeltaMergeTreeTO(volumeObjectTO, parentTO, childTO, grandChildrenTOList)); @@ -684,16 +680,16 @@ private List generateDeltaMergeTrees(VMSnapshotVO parent, VMSn /** * Gets the correct children and grandchildren, taking KBOSS backups into account. * */ - private DataTO getChildAndGrandChildren(VMSnapshotVO childSnapshot, boolean stoppedVm, SnapshotDataStoreVO parentSnapshotDataStoreVO, InternalBackupJoinVO childBackup, + private DataTO getChildAndGrandChildren(VMSnapshotVO child, boolean stoppedVm, SnapshotDataStoreVO parentSnapshotDataStoreVO, InternalBackupStoragePoolVO backupDelta, SnapshotDataStoreVO childVO, VolumeObjectTO volumeObjectTO, List grandChildrenTOList, List grandChildrenVolumeSnapshots) { DataTO childTO; - if (childBackup != null && childBackup.getDate().before(childSnapshot.getCreated())) { + if (backupDelta != null && backupDelta.getBackupDeltaPath().equals(childVO.getInstallPath())) { logger.debug("The child snapshot delta is also a backup delta. We will set the backup delta parent path [{}] as the child and the backup delta path [{}] " + - "as the grand-child.", parentSnapshotDataStoreVO.getInstallPath(), childBackup.getStoragePoolDeltaPath()); - childTO = new BackupDeltaTO(childBackup.getId(), volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, childBackup.getStoragePoolParentPath()); - if (stoppedVm) { - grandChildrenTOList.add(new BackupDeltaTO(volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, childBackup.getStoragePoolDeltaPath())); + "as the grand-child.", backupDelta.getBackupDeltaParentPath(), backupDelta.getBackupDeltaPath()); + childTO = new BackupDeltaTO(volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, backupDelta.getBackupDeltaParentPath()); + if (!child.getCurrent() && stoppedVm) { + grandChildrenTOList.add(new BackupDeltaTO(volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, backupDelta.getBackupDeltaPath())); } } else { childTO = snapshotDataFactory.getSnapshot(childVO.getSnapshotId(), childVO.getDataStoreId(), DataStoreRole.Primary).getTO(); @@ -703,7 +699,7 @@ private DataTO getChildAndGrandChildren(VMSnapshotVO childSnapshot, boolean stop .collect(Collectors.toList())); } - if (childSnapshot.getCurrent() && stoppedVm && grandChildrenTOList.isEmpty()) { + if (child.getCurrent() && stoppedVm) { grandChildrenTOList.add(volumeObjectTO); } @@ -762,26 +758,4 @@ private void transitStateWithoutThrow(VMSnapshot vmSnapshot, VMSnapshot.Event ev throw new CloudRuntimeException(msg, e); } } - - - private Map getVolumeIdAndSucceedingBackupMap(VMSnapshotVO vmSnapshotVO) { - Map volumeIdAndSucceedingBackupMap = new HashMap<>(); - if (vmSnapshotVO == null) { - return volumeIdAndSucceedingBackupMap; - } - - List currents = internalBackupJoinDao.listCurrents(vmSnapshotVO.getVmId(), false) - .stream().filter(internalBackupJoinVO -> internalBackupJoinVO.getDate().after(vmSnapshotVO.getCreated())).collect(Collectors.toList()); - if (currents.isEmpty()) { - logger.debug("No backups created after the VM snapshot [{}] were found, returning.", vmSnapshotVO.getUuid()); - return volumeIdAndSucceedingBackupMap; - } - - InternalBackupJoinVO succeedingBackup = currents.get(0); - volumeIdAndSucceedingBackupMap = currents.stream().filter(b -> succeedingBackup.getId() == b.getId()) - .collect(Collectors.toMap(InternalBackupJoinVO::getVolumeId, internalBackupJoinVO -> internalBackupJoinVO)); - logger.debug("Found the following backups that succeeds the VM snapshot [{}]: [{}].", vmSnapshotVO.getUuid(), volumeIdAndSucceedingBackupMap.values()); - - return volumeIdAndSucceedingBackupMap; - } } diff --git a/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java b/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java index cf02de1f6c18..00bb353788f9 100644 --- a/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java +++ b/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java @@ -154,7 +154,7 @@ public boolean willDeleteBackupsOnOfferingRemoval() { } @Override - public Pair takeBackup(VirtualMachine vm, Boolean quiesceVM, boolean isolated, Long backupScheduleId) { + public Pair takeBackup(VirtualMachine vm, Boolean quiesceVM, boolean isolated) { logger.debug("Starting backup for VM {} on Dummy provider", vm); BackupVO backup = new BackupVO(); diff --git a/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java b/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java index 0569c318935e..d23182920a84 100644 --- a/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java +++ b/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java @@ -29,12 +29,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.Comparator; import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; @@ -131,7 +129,6 @@ import com.cloud.utils.DateUtil; import com.cloud.utils.Pair; import com.cloud.utils.Predicate; -import com.cloud.utils.Ternary; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.db.EntityManager; import com.cloud.utils.db.Transaction; @@ -334,23 +331,11 @@ public boolean removeVMFromBackupOffering(VirtualMachine vm) { logger.info("Removing VM [{}] from KBOSS backup offering.", vm.getUuid()); validateVmState(vm, "remove backup offering", VirtualMachine.State.Expunging, VirtualMachine.State.Destroyed); - List currents = internalBackupJoinDao.listCurrents(vm.getId(), true); - - return finishAllChains(vm, currents); - } - - @Override - public boolean removeVMBackupSchedule(VirtualMachine vm, BackupSchedule backupSchedule) { - logger.info("Removing VM [{}] from KBOSS backup schedule.", vm.getUuid()); - - if (endBackupChain(vm, backupSchedule.getId())) { + if (endBackupChain(vm)) { return true; } UserVmVO vmVO = userVmDao.findById(vm.getId()); - logger.error("Failed to merge deltas for VM [{}] during backup schedule removal process. Changing its state to [{}].", vm, VirtualMachine.State.BackupError); - BackupVO backupVO = backupDao.findById(internalBackupJoinDao.findCurrent(vm.getId(), backupSchedule.getId()).getId()); - backupVO.setStatus(Backup.Status.Error); - backupDao.update(backupVO.getId(), backupVO); + logger.error("Failed to merge deltas for VM [{}] during backup offering removal process. Changing its state to [{}].", vm, VirtualMachine.State.BackupError); vmInstanceDetailsDao.addDetail(vm.getId(), VmDetailConstants.LAST_KNOWN_STATE, vmVO.getState().name(), false); vmVO.setState(VirtualMachine.State.BackupError); userVmDao.update(vmVO.getId(), vmVO); @@ -364,9 +349,9 @@ public boolean willDeleteBackupsOnOfferingRemoval() { } @Override - public Pair takeBackup(VirtualMachine vm, Boolean quiesceVm, boolean isolated, Long backupScheduleId) { + public Pair takeBackup(VirtualMachine vm, Boolean quiesceVm, boolean isolated) { logger.debug("Queueing backup on VM [{}].", vm.getUuid()); - Outcome outcome = createBackupThroughJobQueue(vm, ObjectUtils.defaultIfNull(quiesceVm, false), isolated, backupScheduleId); + Outcome outcome = createBackupThroughJobQueue(vm, ObjectUtils.defaultIfNull(quiesceVm, false), isolated); try { outcome.get(); @@ -438,6 +423,13 @@ public Pair orchestrateTakeBackup(Backup backup, boolean quiesceV HashMap volumeUuidToDeltaPrimaryRef = new HashMap<>(); HashMap volumeUuidToDeltaSecondaryRef = new HashMap<>(); + if (!fullBackup) { + parentBackupDeltasOnPrimary = internalBackupStoragePoolDao.listByBackupId(parentBackup.getId()); + parentBackupDeltasOnSecondary = internalBackupDataStoreDao.listByBackupId(parentBackup.getId()); + + chainImageStoreUrls = getChainImageStoreUrls(backupChain); + } + boolean runningVm = userVm.getState() == VirtualMachine.State.Running; transitVmState(userVm, VirtualMachine.Event.BackupRequested, hostId); updateBackupStatusToBackingUp(volumeTOs, backupVO); @@ -445,24 +437,14 @@ public Pair orchestrateTakeBackup(Backup backup, boolean quiesceV DataStore imageStore = getImageStoreForBackup(userVm.getDataCenterId(), backupVO); createBasicBackupDetails(imageStore.getId(), fullBackup ? 0L : parentBackup.getId(), backupVO); - List succeedingBackupList = getSucceedingBackupList(parentBackup); - InternalBackupJoinVO succeedingBackup = succeedingBackupList.isEmpty() ? null : succeedingBackupList.get(0); - List succeedingVmSnapshotList = getSucceedingVmSnapshotList(parentBackup); VMSnapshotVO succeedingVmSnapshot = succeedingVmSnapshotList.isEmpty() ? null : succeedingVmSnapshotList.get(0); - if (!fullBackup) { - parentBackupDeltasOnPrimary = internalBackupStoragePoolDao.listByBackupId(parentBackup.getId()); - parentBackupDeltasOnSecondary = internalBackupDataStoreDao.listByBackupId(parentBackup.getId()); - - chainImageStoreUrls = getChainImageStoreUrls(backupChain); - } - - Map> volumeIdToSnapshotDataStoreAndBackupPathList = mapVolumesToVmSnapshotAndBackupReferences(volumeTOs, succeedingVmSnapshotList, succeedingBackupList); + Map> volumeIdToSnapshotDataStoreList = mapVolumesToVmSnapshotReferences(volumeTOs, succeedingVmSnapshotList); for (VolumeObjectTO volumeObjectTO : volumeTOs) { - KbossTO kbossTO = new KbossTO(volumeObjectTO, volumeIdToSnapshotDataStoreAndBackupPathList.getOrDefault(volumeObjectTO.getId(), new LinkedList<>())); + KbossTO kbossTO = new KbossTO(volumeObjectTO, volumeIdToSnapshotDataStoreList.getOrDefault(volumeObjectTO.getId(), new ArrayList<>())); kbossTOs.add(kbossTO); - createDeltaReferences(fullBackup, runningVm, backup, parentBackupDeltasOnSecondary, + createDeltaReferences(fullBackup, !succeedingVmSnapshotList.isEmpty(), runningVm, backup, parentBackupDeltasOnSecondary, parentBackupDeltasOnPrimary, volumeUuidToDeltaPrimaryRef, volumeUuidToDeltaSecondaryRef, succeedingVmSnapshot, kbossTO); } @@ -477,7 +459,7 @@ public Pair orchestrateTakeBackup(Backup backup, boolean quiesceV } processBackupSuccess(runningVm, volumeTOs, volumeUuidToDeltaPrimaryRef, volumeUuidToDeltaSecondaryRef, (TakeKbossBackupAnswer)answer, parentBackupDeltasOnPrimary, - succeedingVmSnapshot, backupVO, fullBackup, userVm, hostId, newBackupJoin.getEndOfChain(), isolated, succeedingBackup); + succeedingVmSnapshotList, backupVO, fullBackup, userVm, hostId, newBackupJoin.getEndOfChain(), isolated); if (!isolated) { updateCurrentBackup(newBackupJoin); @@ -577,7 +559,7 @@ public Boolean orchestrateDeleteBackup(Backup backup, boolean forced) { List removedBackupIds = backupParentsToBeRemovedAndLastAliveBackup.first().stream().map(InternalBackupJoinVO::getId).collect(Collectors.toList()); removedBackupIds.add(backup.getId()); - boolean isFailedSetEmpty = processRemoveBackupFailures(forced, deleteAnswers, removedBackupIds, backupJoinVO, virtualMachine); + boolean isFailedSetEmpty = processRemoveBackupFailures(forced, deleteAnswers, removedBackupIds, backupJoinVO); processRemovedBackups(removedBackupIds); @@ -625,10 +607,10 @@ public Boolean orchestrateRestoreVMFromBackup(Backup backup, VirtualMachine vm, } InternalBackupJoinVO backupJoinVO = internalBackupJoinDao.findById(backupId); - List currentBackups = sameVmAsBackup ? internalBackupJoinDao.listCurrents(vm.getId(), false) : List.of(); + InternalBackupJoinVO currentBackup = sameVmAsBackup ? internalBackupJoinDao.findCurrent(vm.getId()) : null; List deltasOnPrimary = new ArrayList<>(); - for (InternalBackupJoinVO currentBackup : currentBackups) { - deltasOnPrimary.addAll(0, internalBackupStoragePoolDao.listByBackupId(currentBackup.getId())); + if (currentBackup != null) { + deltasOnPrimary = internalBackupStoragePoolDao.listByBackupId(currentBackup.getId()); } List deltasOnSecondary = internalBackupDataStoreDao.listByBackupId(backupId); List volumeTOs = vmSnapshotHelper.getVolumeTOList(vm.getId()); @@ -685,7 +667,7 @@ public Boolean orchestrateRestoreVMFromBackup(Backup backup, VirtualMachine vm, updateVolumePathsAndSizeIfNeeded(vm, volumeTOs, volumeInfos, deltasToBeMerged, sameVmAsBackup); - for (InternalBackupJoinVO currentBackup : currentBackups) { + if (currentBackup != null) { internalBackupStoragePoolDao.expungeByBackupId(currentBackup.getId()); setEndOfChainAndRemoveCurrentForBackup(currentBackup); } @@ -901,17 +883,17 @@ public Pair restoreBackupToVM(VirtualMachine vm, Backup backup, } @Override - public boolean finishBackupChains(VirtualMachine virtualMachine) { - UserVmVO vm = userVmDao.findById(virtualMachine.getId()); - List currents = internalBackupJoinDao.listCurrents(vm.getId(), true); - if (allowedVmStates.contains(vm.getState())) { - return finishAllChains(vm, currents); - } - if (vm.getState() != VirtualMachine.State.BackupError) { - logger.error("VM [{}] is not in the right state to finish backup chain. It can only be in states [Running, Stopped and BackupError].", vm.getUuid()); + public boolean finishBackupChain(VirtualMachine virtualMachine) { + UserVmVO userVmVO = userVmDao.findById(virtualMachine.getId()); + if (allowedVmStates.contains(userVmVO.getState())) { + return endBackupChain(userVmVO); + } + if (userVmVO.getState() != VirtualMachine.State.BackupError) { + logger.error("VM [{}] is not in the right state to finish backup chain. It can only be in states [Running, Stopped and BackupError].", userVmVO.getUuid()); + return false; } - return normalizeBackupErrorAndFinishChain(vm); + return normalizeBackupErrorAndFinishChain(userVmVO); } @Override @@ -945,14 +927,14 @@ public boolean supportsMemoryVmSnapshot() { @Override public void prepareVolumeForDetach(Volume volume, VirtualMachine virtualMachine) { logger.info("Preparing volume [{}] for detach.", volume.getUuid()); - mergeCurrentDeltasIntoVolume(volume, virtualMachine, "detach", virtualMachine.getState().equals(VirtualMachine.State.Running)); + mergeCurrentDeltaIntoVolume(volume, virtualMachine, "detach", virtualMachine.getState().equals(VirtualMachine.State.Running)); } @Override public void prepareVolumeForMigration(Volume volume, VirtualMachine vm) { if (VirtualMachine.State.Migrating.equals(vm.getState())) { logger.info("Preparing volume [{}] for live migration.", volume.getUuid()); - mergeCurrentDeltasIntoVolume(volume, vm, "live migration", true); + mergeCurrentDeltaIntoVolume(volume, vm, "live migration", true); } } @@ -963,37 +945,31 @@ public void updateVolumeId(VirtualMachine virtualMachine, long oldVolumeId, long @Override public void prepareVmForSnapshotRevert(VMSnapshot vmSnapshot, VirtualMachine virtualMachine) { - List currentBackups = internalBackupJoinDao.listCurrents(virtualMachine.getId(), true); + InternalBackupJoinVO currentBackup = internalBackupJoinDao.findCurrent(virtualMachine.getId()); - if (currentBackups.isEmpty()) { + if (currentBackup == null) { logger.debug("There is no current backup delta, the VM [{}] is already prepared for VM snapshot revert.", virtualMachine.getUuid()); return; } - currentBackups = currentBackups.stream().filter(backup -> backup.getDate().after(vmSnapshot.getCreated())).collect(Collectors.toList()); - if (currentBackups.isEmpty()) { - logger.debug("Existing backup deltas [{}] were created before the target VM snapshot [{}]. No preparation needed for VM [{}].", - currentBackups, vmSnapshot.getCreated(), virtualMachine.getUuid()); + if (currentBackup.getDate().before(vmSnapshot.getCreated())) { + logger.debug("The current backup delta was taken before [{}] the VM snapshot being reverted [{}], no need to prepare the VM.", currentBackup.getDate(), + vmSnapshot.getCreated()); + return; } logger.debug("Preparing VM [{}] for VM snapshot reversion.", virtualMachine.getUuid()); List volumeObjectTOs = vmSnapshotHelper.getVolumeTOList(virtualMachine.getId()); + VMSnapshotVO vmSnapshotSucceedingCurrentBackup = getSucceedingVmSnapshot(currentBackup); List deltaMergeTreeTOList = new ArrayList<>(); Commands commands = new Commands(Command.OnError.Stop); List deletedDeltas = new ArrayList<>(); - Map backupVmSnapshotMap = new HashMap<>(); - - for (InternalBackupJoinVO currentBackup : currentBackups) { - VMSnapshotVO vmSnapshotSucceedingCurrentBackup = getSucceedingVmSnapshot(currentBackup); - - createDeleteCommandsAndMergeTrees(volumeObjectTOs, commands, deletedDeltas, vmSnapshotSucceedingCurrentBackup, deltaMergeTreeTOList, currentBackup); - backupVmSnapshotMap.put(currentBackup, vmSnapshotSucceedingCurrentBackup); - } + createDeleteCommandsAndMergeTrees(volumeObjectTOs, commands, deletedDeltas, vmSnapshotSucceedingCurrentBackup, deltaMergeTreeTOList); - if (CollectionUtils.isNotEmpty(deltaMergeTreeTOList)) { + if (!deltaMergeTreeTOList.isEmpty()) { commands.addCommand(new MergeDiskOnlyVmSnapshotCommand(deltaMergeTreeTOList, false, virtualMachine.getInstanceName())); } @@ -1012,18 +988,12 @@ public void prepareVmForSnapshotRevert(VMSnapshot vmSnapshot, VirtualMachine vir throw new CloudRuntimeException(String.format("Unable to prepare VM [%s] for VM snapshot reversion.", virtualMachine.getUuid())); } - for (Map.Entry backupAndVmSnapshot : backupVmSnapshotMap.entrySet()) { - InternalBackupJoinVO backup = backupAndVmSnapshot.getKey(); - VMSnapshotVO vmSnapshotSucceedingBackup = backupAndVmSnapshot.getValue(); - - List snapRefsSucceedingCurrentBackup = new ArrayList<>(); - - if (vmSnapshotSucceedingBackup != null) { - snapRefsSucceedingCurrentBackup = vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(vmSnapshotSucceedingBackup.getId()); - } - - updateReferencesAfterPrepareForSnapshotRevert(deltaMergeTreeTOList, snapRefsSucceedingCurrentBackup, deletedDeltas, backup); + List snapRefsSucceedingCurrentBackup = new ArrayList<>(); + if (vmSnapshotSucceedingCurrentBackup != null) { + snapRefsSucceedingCurrentBackup = vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(vmSnapshotSucceedingCurrentBackup.getId()); } + + updateReferencesAfterPrepareForSnapshotRevert(deltaMergeTreeTOList, snapRefsSucceedingCurrentBackup, deletedDeltas, currentBackup); } /** @@ -1064,14 +1034,14 @@ public ConfigKey[] getConfigKeys() { backupCompressionCoroutines}; } - protected Outcome createBackupThroughJobQueue(VirtualMachine vm, boolean quiesceVm, boolean isolated, Long backupScheduleId) { + protected Outcome createBackupThroughJobQueue(VirtualMachine vm, boolean quiesceVm, boolean isolated) { final CallContext context = CallContext.current(); long userId = context.getCallingUser().getId(); long accountId = context.getCallingAccount().getAccountId(); long vmId = vm.getId(); BackupVO backup = new BackupVO(String.format("%s-%s", vm.getHostName(), DateUtil.getDateInSystemTimeZone()), vmId, vm.getBackupOfferingId(), accountId, - vm.getDomainId(), vm.getDataCenterId(), 0, Backup.Status.Queued, backupScheduleId, + vm.getDomainId(), vm.getDataCenterId(), 0, Backup.Status.Queued, null, Backup.CompressionStatus.Uncompressed, Backup.ValidationStatus.NotValidated); VmWorkJobVO workJob = new VmWorkJobVO(AsyncJobExecutionContext.getOriginJobId(), userId, accountId, VmWorkTakeBackup.class.getName(), vmId, VirtualMachine.Type.Instance, @@ -1274,9 +1244,9 @@ protected void endBackupChainIfConfigured(BackupVO backupVO) { // Get updated record InternalBackupJoinVO backupJoinVO = internalBackupJoinDao.findById(backupVO.getId()); if (backupJoinVO.getCurrent() || (!backupChildren.isEmpty() && backupChildren.get(backupChildren.size() - 1).getCurrent())) { - logger.info("As [{}] is true, we are ending the backup chain of schedule [{}] for VM [{}]. The next backup will be a full backup.", - backupVO.getBackupScheduleId(), BackupValidationServiceJobController.backupValidationEndChainOnFail.toString()); - endBackupChain(userVmDao.findById(backupVO.getVmId()), backupVO.getBackupScheduleId()); + logger.info("As [{}] is true, we are ending the backup chain for VM [{}]. The next backup will be a full backup.", + BackupValidationServiceJobController.backupValidationEndChainOnFail.toString()); + endBackupChain(userVmDao.findById(backupVO.getVmId())); } } @@ -1292,20 +1262,12 @@ protected boolean normalizeBackupErrorAndFinishChain(UserVmVO userVmVO) { boolean runningVM = detail == null || VirtualMachine.State.valueOf(detail.getValue()) == VirtualMachine.State.Running; BackupVO backupVO = backupDao.findLatestByStatusAndVmId(Backup.Status.Error, userVmVO.getId()); - InternalBackupJoinVO currentOnThisChain = internalBackupJoinDao.findCurrent(userVmVO.getId(), backupVO.getBackupScheduleId()); - InternalBackupJoinVO errorBackup = internalBackupJoinDao.findById(backupVO.getId()); - - boolean errorOnBackupCreation = currentOnThisChain == null || currentOnThisChain.getId() != errorBackup.getId(); - - List succeedingBackupList = getSucceedingBackupList(currentOnThisChain); - List succeedingVmSnapshotList = getSucceedingVmSnapshotList(currentOnThisChain); - List volumeTOs = vmSnapshotHelper.getVolumeTOList(userVmVO.getId()); - - Map> volumeToDeltasAfterCurrent = mapVolumesToVmSnapshotAndBackupReferences(volumeTOs, succeedingVmSnapshotList, succeedingBackupList); + InternalBackupJoinVO internalBackupJoinVO = internalBackupJoinDao.findById(backupVO.getId()); + ImageStoreVO imageStoreVO = imageStoreDao.findById(internalBackupJoinVO.getImageStoreId()); List kbossTOS = new ArrayList<>(); - List deltasOnPrimary = internalBackupStoragePoolDao.listByBackupId(errorBackup.getId()); - InternalBackupJoinVO parent = internalBackupJoinDao.findById(errorBackup.getParentId()); + List deltasOnPrimary = internalBackupStoragePoolDao.listByBackupId(internalBackupJoinVO.getId()); + InternalBackupJoinVO parent = internalBackupJoinDao.findById(internalBackupJoinVO.getParentId()); // There is a possibility that the cleanup step of the backup creation was executed, and thus we would have to merge with the old parent's parent List parentDeltasOnPrimary = new ArrayList<>(); @@ -1313,11 +1275,9 @@ protected boolean normalizeBackupErrorAndFinishChain(UserVmVO userVmVO) { parentDeltasOnPrimary = internalBackupStoragePoolDao.listByBackupId(parent.getId()); } - List deltasOnSecondary = internalBackupDataStoreDao.listByBackupId(errorBackup.getId()); - ImageStoreVO imageStoreVO = imageStoreDao.findById(errorBackup.getImageStoreId()); - configureKbossTosForCleanup(userVmVO, deltasOnPrimary, volumeToDeltasAfterCurrent, deltasOnSecondary, parentDeltasOnPrimary, kbossTOS, errorOnBackupCreation); - CleanupKbossBackupErrorCommand command = new CleanupKbossBackupErrorCommand(runningVM, errorOnBackupCreation, errorBackup.getEndOfChain(), succeedingBackupList.isEmpty(), - userVmVO.getInstanceName(), imageStoreVO.getUrl(), kbossTOS); + List deltasOnSecondary = internalBackupDataStoreDao.listByBackupId(internalBackupJoinVO.getId()); + configureKbossTosForCleanup(userVmVO, deltasOnPrimary, deltasOnSecondary, runningVM, parentDeltasOnPrimary, kbossTOS); + CleanupKbossBackupErrorCommand command = new CleanupKbossBackupErrorCommand(runningVM, userVmVO.getInstanceName(), imageStoreVO.getUrl(), kbossTOS); long hostId = userVmVO.getHostId() != null ? userVmVO.getHostId() : vmSnapshotHelper.pickRunningHost(userVmVO.getId()); Answer answer = sendBackupCommand(hostId, command); @@ -1326,40 +1286,32 @@ protected boolean normalizeBackupErrorAndFinishChain(UserVmVO userVmVO) { return false; } - boolean chainAlreadyEnded = processCleanupBackupErrorAnswer(userVmVO, answer, errorBackup, currentOnThisChain, succeedingBackupList); + boolean chainAlreadyEnded = processCleanupBackupErrorAnswer(userVmVO, answer); if (!chainAlreadyEnded) { - mergeCurrentBackupDeltas(errorBackup); - } - - if (currentOnThisChain != null) { - internalBackupStoragePoolDao.expungeByBackupId(currentOnThisChain.getId()); - setEndOfChainAndRemoveCurrentForBackup(currentOnThisChain); + return endBackupChain(userVmVO); } + InternalBackupJoinVO current = internalBackupJoinDao.findCurrent(userVmVO.getId()); + internalBackupStoragePoolDao.expungeByBackupId(current.getId()); + setEndOfChainAndRemoveCurrentForBackup(current); - return finishBackupChains(userVmVO); + return true; } - protected boolean processCleanupBackupErrorAnswer(UserVmVO userVmVO, Answer answer, InternalBackupJoinVO errorBackup, InternalBackupJoinVO currentBackup, - List succeedingBackups) { + protected boolean processCleanupBackupErrorAnswer(UserVmVO userVmVO, Answer answer) { boolean runningVM; CleanupKbossBackupErrorAnswer cleanAnswer = (CleanupKbossBackupErrorAnswer) answer; logger.info("Successfully finished chain for VM [{}] and normalizing the BackupError state. Cleaning up metadata.", userVmVO.getUuid()); - boolean chainAlreadyEnded = true; - for (Map.Entry> entry : cleanAnswer.getVolumeIdToPathAndChainEnded().entrySet()) { - VolumeVO volumeVO = volumeDao.findByUuid(entry.getKey()); - if (!entry.getValue().first().equals(volumeVO.getPath())) { - volumeVO.setPath(entry.getValue().first()); + boolean chainAlreadyEnded = false; + for (VolumeObjectTO volumeObjectTO : cleanAnswer.getVolumeObjectTos()) { + VolumeVO volumeVO = volumeDao.findById(volumeObjectTO.getId()); + if (!volumeObjectTO.getPath().equals(volumeVO.getPath())) { + volumeVO.setPath(volumeObjectTO.getPath()); volumeDao.update(volumeVO.getId(), volumeVO); - if (!entry.getValue().second()) { - chainAlreadyEnded = false; - continue; - } - internalBackupStoragePoolDao.expungeByVolumeIdAndBackupId(volumeVO.getId(), errorBackup.getId()); + chainAlreadyEnded = true; } } - updateSucceedingBackupIfNeeded(currentBackup, succeedingBackups); runningVM = cleanAnswer.isVmRunning(); userVmVO.setState(runningVM ? VirtualMachine.State.Running : VirtualMachine.State.Stopped); @@ -1368,20 +1320,6 @@ protected boolean processCleanupBackupErrorAnswer(UserVmVO userVmVO, Answer answ return chainAlreadyEnded; } - private void updateSucceedingBackupIfNeeded(InternalBackupJoinVO currentBackup, List succeedingBackups) { - if (currentBackup == null || succeedingBackups.isEmpty()) { - return; - } - InternalBackupJoinVO succeedingBackup = succeedingBackups.get(0); - for (InternalBackupStoragePoolVO deltaRef : internalBackupStoragePoolDao.listByBackupId(currentBackup.getId())) { - InternalBackupStoragePoolVO succeedingDelta = internalBackupStoragePoolDao.findOneByVolumeIdAndBackupId(deltaRef.getVolumeId(), succeedingBackup.getId()); - if (succeedingDelta != null) { - succeedingDelta.setBackupDeltaParentPath(deltaRef.getBackupDeltaParentPath()); - internalBackupStoragePoolDao.update(succeedingDelta.getId(), succeedingDelta); - } - } - } - protected void calculateAndSaveHash(Set> backupDeltaAndVolumePairs, BackupVO backupVO, long hostId) { TakeBackupHashCommand cmd = new TakeBackupHashCommand(backupDeltaAndVolumePairs.stream().map(Pair::first).collect(Collectors.toList()), backupVO.getUuid()); Answer answer = sendBackupCommand(hostId, cmd); @@ -1585,10 +1523,51 @@ protected boolean deleteFailedBackup(BackupVO backupVO) { return true; } + /** + * Merges the current delta on primary storage, if any, into the given volume. If the backup has no more deltas on primary storage, will set the backup as end_of_chain. + * */ + protected void mergeCurrentDeltaIntoVolume(Volume volume, VirtualMachine virtualMachine, String operation, boolean isVmRunning) { + InternalBackupStoragePoolVO delta = internalBackupStoragePoolDao.findOneByVolumeId(volume.getId()); + if (delta == null) { + logger.debug("Volume [{}] has no deltas to merge, doing nothing.", volume.getUuid()); + return; + } + InternalBackupJoinVO internalBackupJoinVO = internalBackupJoinDao.findById(delta.getBackupId()); + VMSnapshotVO succeedingVmSnapshotVO = getSucceedingVmSnapshot(internalBackupJoinVO); + + DataStore store = dataStoreManager.getDataStore(volume.getPoolId(), DataStoreRole.Primary); + VolumeObject volumeObject = VolumeObject.getVolumeObject(store, (VolumeVO)volume); + + DeltaMergeTreeTO deltaMergeTreeTO = createDeltaMergeTree(succeedingVmSnapshotVO == null, isVmRunning, delta, (VolumeObjectTO)volumeObject.getTO(), succeedingVmSnapshotVO); + MergeDiskOnlyVmSnapshotCommand cmd = new MergeDiskOnlyVmSnapshotCommand(List.of(deltaMergeTreeTO), isVmRunning, virtualMachine.getInstanceName()); + + Answer answer = sendBackupCommand(vmSnapshotHelper.pickRunningHost(virtualMachine.getId()), cmd); + + if (answer == null || !answer.getResult()) { + logger.error("Error while trying to prepare volume [{}] for {}. Got [{}] as answer from host.", volume.getUuid(), operation, answer != null ? answer.getDetails() : null); + throw new CloudRuntimeException(String.format("Unable to prepare volume [%s] for [%s].", volume.getUuid(), operation)); + } + + if (succeedingVmSnapshotVO == null) { + VolumeVO volumeVO = volumeDao.findById(volumeObject.getId()); + volumeVO.setPath(deltaMergeTreeTO.getParent().getPath()); + volumeDao.update(volumeVO.getId(), volumeVO); + } + + expungeOldDeltasAndUpdateVmSnapshotIfNeeded(List.of(delta), succeedingVmSnapshotVO); + + List backupDeltas = internalBackupStoragePoolDao.listByBackupId(delta.getBackupId()); + if (backupDeltas.isEmpty()) { + logger.debug("Backup [{}] has no more deltas on primary storage due to prepare volume [{}] for {} operation. Will set it as end of chain and not current.", + internalBackupJoinVO.getUuid(), volume.getUuid(), operation); + setEndOfChainAndRemoveCurrentForBackup(internalBackupJoinVO); + } + } + /** * Creates the necessary delta references on both primary and secondary storage. Also maps the volume to the parent delta backup and create the delta merge tree. * */ - protected void createDeltaReferences(boolean fullBackup, boolean runningVm, Backup backup, + protected void createDeltaReferences(boolean fullBackup, boolean hasVmSnapshotSucceedingLastBackup, boolean runningVm, Backup backup, List parentBackupDeltasOnSecondary, List parentBackupDeltasOnPrimary, HashMap volumeUuidToDeltaPrimaryRef, HashMap volumeUuidToDeltaSecondaryRef, VMSnapshotVO succeedingVmSnapshot, KbossTO kbossTO) { @@ -1603,8 +1582,7 @@ protected void createDeltaReferences(boolean fullBackup, boolean runningVm, Back InternalBackupDataStoreVO deltaSecondaryRef = new InternalBackupDataStoreVO(backup.getId(), volumeObjectTO.getVolumeId(), volumeObjectTO.getDeviceId(), relativePathOnSecondary); if (!fullBackup) { - InternalBackupStoragePoolVO parentDeltaOnPrimary = createDeltaMergeTreeForVolume(false, runningVm, parentBackupDeltasOnPrimary, succeedingVmSnapshot, kbossTO, - new ArrayList<>()); + InternalBackupStoragePoolVO parentDeltaOnPrimary = createDeltaMergeTreeForVolume(false, runningVm, parentBackupDeltasOnPrimary, succeedingVmSnapshot, kbossTO); findAndSetParentBackupPath(parentBackupDeltasOnSecondary, parentDeltaOnPrimary, kbossTO); } @@ -1615,8 +1593,10 @@ protected void createDeltaReferences(boolean fullBackup, boolean runningVm, Back InternalBackupStoragePoolVO deltaPrimaryRef = new InternalBackupStoragePoolVO(backup.getId(), volumeObjectTO.getPoolId(), volumeObjectTO.getVolumeId(), filename, volumeObjectTO.getPath()); - if (kbossTO.getDeltaMergeTreeTO() != null && CollectionUtils.isEmpty(kbossTO.getDeltaPaths())) { + if (kbossTO.getDeltaMergeTreeTO() != null && !hasVmSnapshotSucceedingLastBackup) { deltaPrimaryRef.setBackupDeltaParentPath(kbossTO.getDeltaMergeTreeTO().getParent().getPath()); + } else if (hasVmSnapshotSucceedingLastBackup) { + deltaPrimaryRef.setBackupDeltaParentPath(volumeObjectTO.getPath()); } InternalBackupStoragePoolVO referenceOnPrimary = internalBackupStoragePoolDao.persist(deltaPrimaryRef); @@ -1624,49 +1604,6 @@ protected void createDeltaReferences(boolean fullBackup, boolean runningVm, Back volumeUuidToDeltaPrimaryRef.put(volumeObjectTO.getUuid(), referenceOnPrimary); } - /** - * Merges the current delta on primary storage, if any, into the given volume. If the backup has no more deltas on primary storage, will set the backup as end_of_chain. - * */ - protected void mergeCurrentDeltasIntoVolume(Volume volume, VirtualMachine virtualMachine, String operation, boolean isVmRunning) { - List currents = internalBackupJoinDao.listCurrentsByVolumeIdDesc(volume.getId()); - if (currents.isEmpty()) { - logger.debug("Volume [{}] has no deltas to merge, doing nothing.", volume.getUuid()); - return; - } - - for (InternalBackupJoinVO current : currents) { - InternalBackupStoragePoolVO delta = internalBackupStoragePoolDao.findOneByVolumeIdAndBackupId(volume.getId(), current.getId()); - - DataStore store = dataStoreManager.getDataStore(volume.getPoolId(), DataStoreRole.Primary); - VolumeObject volumeObject = VolumeObject.getVolumeObject(store, (VolumeVO)volume); - - DeltaMergeTreeTO deltaMergeTreeTO = createDeltaMergeTree(true, isVmRunning, delta, (VolumeObjectTO)volumeObject.getTO(), null, new ArrayList<>()); - MergeDiskOnlyVmSnapshotCommand cmd = new MergeDiskOnlyVmSnapshotCommand(List.of(deltaMergeTreeTO), isVmRunning, virtualMachine.getInstanceName()); - - Answer answer = sendBackupCommand(vmSnapshotHelper.pickRunningHost(virtualMachine.getId()), cmd); - - if (answer == null || !answer.getResult()) { - logger.error("Error while trying to prepare volume [{}] for {}. Got [{}] as answer from host.", volume.getUuid(), operation, answer != null ? answer.getDetails() : null); - throw new CloudRuntimeException(String.format("Unable to prepare volume [%s] for [%s].", volume.getUuid(), operation)); - } - VolumeVO volumeVO = volumeDao.findById(volumeObject.getId()); - volumeVO.setPath(deltaMergeTreeTO.getParent().getPath()); - volumeDao.update(volumeVO.getId(), volumeVO); - - volume = volumeVO; - - List deltaOnPrimary = List.of(delta); - expungeOldDeltasAndUpdateVmSnapshotOrBackup(deltaOnPrimary, null, null); - - List backupDeltas = internalBackupStoragePoolDao.listByBackupId(delta.getBackupId()); - if (backupDeltas.isEmpty()) { - logger.debug("Backup [{}] has no more deltas on primary storage due to prepare volume [{}] for {} operation. Will set it as end of chain and not current.", - current.getUuid(), volume.getUuid(), operation); - setEndOfChainAndRemoveCurrentForBackup(current); - } - } - } - protected HostVO getHostToRestore(VirtualMachine vm, boolean quickRestore, Long hostId) throws AgentUnavailableException { HostVO host; if (quickRestore) { @@ -1732,59 +1669,54 @@ protected VMSnapshotVO getSucceedingVmSnapshot(InternalBackupJoinVO backup) { } /** - * Returns ordered list of backups taken after the last backup. The list is ordered from oldest to newest. + * Given a VM snapshot, returns a map of volume id to list of snapshot references of the children of the VM snapshot. * */ - protected List getSucceedingBackupList(InternalBackupJoinVO backup) { - List internalBackupJoinVOS = new ArrayList<>(); - if (backup == null) { - return internalBackupJoinVOS; - } - - List currentBackups = internalBackupJoinDao.listCurrents(backup.getVmId(), false); - if (currentBackups.isEmpty()) { - return internalBackupJoinVOS; + protected Map> gatherSnapshotReferencesOfChildrenSnapshot(List volumeObjectTOs, VMSnapshot vmSnapshotVO) { + Map> volumeToSnapshotRefs = new HashMap<>(); + if (vmSnapshotVO == null) { + return volumeToSnapshotRefs; + } + + List snapshotChildren = vmSnapshotDao.listByParent(vmSnapshotVO.getId()); + if (CollectionUtils.isEmpty(snapshotChildren)) { + return volumeToSnapshotRefs; + } + + List snapshotDataStoreVOS = new ArrayList<>(); + snapshotChildren.stream() + .map(snapshotVo -> vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(snapshotVo.getId())) + .forEach(snapshotDataStoreVOS::addAll); + mapVolumesToSnapshotReferences(volumeObjectTOs, snapshotDataStoreVOS, volumeToSnapshotRefs); + if (logger.isDebugEnabled()) { + StringBuilder log = new StringBuilder(String.format("Found the following snapshot references that succeed the VM snapshot [%s].", vmSnapshotVO.getUuid())); + for (VolumeObjectTO volumeObjectTO : volumeObjectTOs) { + log.append(String.format(" Volume [%s]; Snapshot references [%s].", volumeObjectTO.getUuid(), volumeToSnapshotRefs.get(volumeObjectTO.getId()))); + } + logger.debug(log.toString()); } - internalBackupJoinVOS = currentBackups.stream().filter(internalBackupJoinVO -> internalBackupJoinVO.getDate().after(backup.getDate())).collect(Collectors.toList()); - logger.debug("Found the following backups that succeed the backup [{}]: [{}].", backup.getUuid(), internalBackupJoinVOS); - - return internalBackupJoinVOS; + return volumeToSnapshotRefs; } /** - * Given a list of volumes and VM snapshots/backups, maps the volumes to the delta references of the VM snapshots/backups. + * Given a list of volumes and VM snapshots, maps the volumes to the snapshot references of the VM snapshots. * */ - protected Map> mapVolumesToVmSnapshotAndBackupReferences(List volumeObjectTOs, List vmSnapshotVOList, List internalBackupJoinVOList) { - Map> volumeToSnapshotAndBackupRefs = new HashMap<>(); - if (vmSnapshotVOList.isEmpty() && internalBackupJoinVOList.isEmpty()) { - logger.trace("No VM snapshot nor backup to map to any volume, returning."); - return volumeToSnapshotAndBackupRefs; - } - - List> volumeIdAndResourcePathAndCreatedDateList = new ArrayList<>(); - for (InternalBackupJoinVO internalBackupJoinVO : internalBackupJoinVOList) { - volumeIdAndResourcePathAndCreatedDateList.add(new Ternary<>(internalBackupJoinVO.getVolumeId(), internalBackupJoinVO.getStoragePoolDeltaPath(), internalBackupJoinVO.getDate())); + protected Map> mapVolumesToVmSnapshotReferences(List volumeObjectTOs, List vmSnapshotVOList) { + Map> volumeToSnapshotRefs = new HashMap<>(); + if (vmSnapshotVOList.isEmpty()) { + logger.trace("No VM snapshot to map to any volume, returning."); + return volumeToSnapshotRefs; } + ArrayList allRefs = new ArrayList<>(); for (VMSnapshotVO vmSnapshotVO : vmSnapshotVOList) { - vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(vmSnapshotVO.getId()) - .forEach(snapshotDataStoreVO -> volumeIdAndResourcePathAndCreatedDateList.add(new Ternary<>(snapshotDataStoreVO.getVolumeId(), snapshotDataStoreVO.getInstallPath(), snapshotDataStoreVO.getCreated()))); - } - - volumeIdAndResourcePathAndCreatedDateList.sort(Comparator.comparing(Ternary::third)); - - for (Ternary volumeIdAndResourcePathAndCreatedDate : volumeIdAndResourcePathAndCreatedDateList) { - long volumeId = volumeIdAndResourcePathAndCreatedDate.first(); - String resourcePath = volumeIdAndResourcePathAndCreatedDate.second(); - - volumeToSnapshotAndBackupRefs.computeIfAbsent(volumeId, k -> new LinkedList<>()).addLast(resourcePath); + allRefs.addAll(vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(vmSnapshotVO.getId())); } - - logger.trace("Given volume objects [{}], VM snapshots [{}] and backups [{}], created the following map [{}].", volumeObjectTOs, vmSnapshotVOList, internalBackupJoinVOList, volumeToSnapshotAndBackupRefs); - return volumeToSnapshotAndBackupRefs; + mapVolumesToSnapshotReferences(volumeObjectTOs, allRefs, volumeToSnapshotRefs); + logger.trace("Given volume objects [{}] and VM snapshots [{}], created the following map [{}].", volumeObjectTOs, vmSnapshotVOList, volumeToSnapshotRefs); + return volumeToSnapshotRefs; } - protected void mapVolumesToSnapshotReferences(List volumeObjectTOs, List snapshotDataStoreVOS, Map> volumeToSnapshotRefs) { for (VolumeObjectTO volumeObjectTO : volumeObjectTOs) { List associatedSnapshots = snapshotDataStoreVOS.stream() @@ -1827,68 +1759,30 @@ protected long updateDeltaReferencesAndCalculateBackupPhysicalSize(VolumeObjectT } /** - * Expunge the old backup deltas and if there were disk-only VM snapshot or backup deltas after the last backup, update their paths. + * Expunge the old backup deltas and if there were disk-only VM snapshot deltas after the last backup, update their paths. * */ - protected void expungeOldDeltasAndUpdateVmSnapshotOrBackupIfNeeded(List oldDeltasOnPrimary, VMSnapshot vmSnapshot, - InternalBackupJoinVO lastBackup) { + protected void expungeOldDeltasAndUpdateVmSnapshotIfNeeded(List oldDeltasOnPrimary, VMSnapshot vmSnapshot) { List snapshotRefs = vmSnapshot == null ? List.of() : vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(vmSnapshot.getId()); - List newBackupDeltas = new ArrayList<>(); - Map volumeIdNewBackupDeltaMap = new HashMap<>(); - - if (lastBackup != null) { - newBackupDeltas = internalBackupStoragePoolDao.listByBackupId(lastBackup.getId()); - volumeIdNewBackupDeltaMap = newBackupDeltas.stream().collect(Collectors.toMap(InternalBackupStoragePoolVO::getVolumeId, nbsp -> nbsp)); - } - for (InternalBackupStoragePoolVO oldBackupDelta : oldDeltasOnPrimary) { logger.trace("Expunging old backup delta [{}].", oldBackupDelta); internalBackupStoragePoolDao.expunge(oldBackupDelta.getId()); SnapshotDataStoreVO snapshotDataStoreVO = snapshotRefs.stream().filter(ref -> ref.getVolumeId() == oldBackupDelta.getVolumeId()).findFirst().orElse(null); - if (snapshotDataStoreVO != null) { - snapshotDataStoreVO.setInstallPath(oldBackupDelta.getBackupDeltaParentPath()); - logger.debug("Updating snapshot delta [{}] path to [{}].", snapshotDataStoreVO.getId(), oldBackupDelta.getBackupDeltaParentPath()); - snapshotDataStoreDao.update(snapshotDataStoreVO.getId(), snapshotDataStoreVO); + if (snapshotDataStoreVO == null) { continue; } - if (lastBackup != null) { - InternalBackupStoragePoolVO newBackupDelta = volumeIdNewBackupDeltaMap.get(oldBackupDelta.getVolumeId()); - newBackupDelta.setBackupDeltaParentPath(oldBackupDelta.getBackupDeltaParentPath()); - logger.debug("Updating backup delta [{}] path to [{}].", newBackupDelta.getId(), oldBackupDelta.getBackupDeltaParentPath()); - internalBackupStoragePoolDao.update(newBackupDelta.getId(), newBackupDelta); - } + snapshotDataStoreVO.setInstallPath(oldBackupDelta.getBackupDeltaParentPath()); + logger.debug("Updating snapshot delta [{}] path to [{}].", snapshotDataStoreVO.getId(), oldBackupDelta.getBackupDeltaParentPath()); + snapshotDataStoreDao.update(snapshotDataStoreVO.getId(), snapshotDataStoreVO); } } - /** - * Expunges old deltas on primary storage and updates the metadata for either - * the succeeding VM snapshot or the succeeding backup based on their chronological order. - * If only one (or neither) is provided, it proceeds with the available entities. - * - * @param oldDeltasOnPrimary The list of delta references on the primary storage to be removed; - * @param succeedingVmSnapshotVO The VM snapshot that follows the deltas being expunged; - * @param succeedingBackup The backup entity that follows the deltas being expunged. - */ - protected void expungeOldDeltasAndUpdateVmSnapshotOrBackup(List oldDeltasOnPrimary, VMSnapshot succeedingVmSnapshotVO, - InternalBackupJoinVO succeedingBackup) { - if (ObjectUtils.allNotNull(succeedingVmSnapshotVO, succeedingBackup)) { - if (succeedingVmSnapshotVO.getCreated().before(succeedingBackup.getDate())) { - expungeOldDeltasAndUpdateVmSnapshotOrBackupIfNeeded(oldDeltasOnPrimary, succeedingVmSnapshotVO, null); - } else { - expungeOldDeltasAndUpdateVmSnapshotOrBackupIfNeeded(oldDeltasOnPrimary, null, succeedingBackup); - } - } else { - expungeOldDeltasAndUpdateVmSnapshotOrBackupIfNeeded(oldDeltasOnPrimary, succeedingVmSnapshotVO, succeedingBackup); - } - } - - /** * Create a {@link DeltaMergeTreeTO} for the volume if it has a delta on primary and add it to the list. * * @return the delta on primary of the volume. Null if no delta. * */ protected InternalBackupStoragePoolVO createDeltaMergeTreeForVolume(boolean childIsVolume, boolean runningVm, List deltasOnPrimary, VMSnapshotVO succeedingVmSnapshot, - KbossTO kbossTO, List succeedingBackupList) { + KbossTO kbossTO) { VolumeObjectTO volumeObjectTO = kbossTO.getVolumeObjectTO(); InternalBackupStoragePoolVO deltaOnPrimary = deltasOnPrimary.stream() @@ -1902,12 +1796,12 @@ protected InternalBackupStoragePoolVO createDeltaMergeTreeForVolume(boolean chil logger.debug("Volume [{}] has a backup delta on primary storage [{}].", volumeObjectTO.getUuid(), deltaOnPrimary); - kbossTO.setDeltaMergeTreeTO(createDeltaMergeTree(childIsVolume, runningVm, deltaOnPrimary, volumeObjectTO, succeedingVmSnapshot, succeedingBackupList)); + kbossTO.setDeltaMergeTreeTO(createDeltaMergeTree(childIsVolume, runningVm, deltaOnPrimary, volumeObjectTO, succeedingVmSnapshot)); return deltaOnPrimary; } protected DeltaMergeTreeTO createDeltaMergeTree(boolean childIsVolume, boolean runningVm, InternalBackupStoragePoolVO deltaOnPrimary, - VolumeObjectTO volumeObjectTO, VMSnapshotVO succeedingVmSnapshot, List succeedingBackupsList) { + VolumeObjectTO volumeObjectTO, VMSnapshotVO succeedingVmSnapshot) { DataStore store = dataStoreManager.getDataStore(deltaOnPrimary.getStoragePoolId(), DataStoreRole.Primary); DataTO deltaChild; if (childIsVolume) { @@ -1917,12 +1811,11 @@ protected DeltaMergeTreeTO createDeltaMergeTree(boolean childIsVolume, boolean r } BackupDeltaTO deltaParent = new BackupDeltaTO(store.getTO(), Hypervisor.HypervisorType.KVM, deltaOnPrimary.getBackupDeltaParentPath()); - List succeedingSnapshotList = succeedingVmSnapshot != null ? vmSnapshotDao.listByParent(succeedingVmSnapshot.getId()) : new ArrayList<>(); List succeedingDeltaPaths = new ArrayList<>(); - if (succeedingVmSnapshot != null || CollectionUtils.isNotEmpty(succeedingBackupsList)) { - succeedingDeltaPaths = mapVolumesToVmSnapshotAndBackupReferences(List.of(volumeObjectTO), succeedingSnapshotList, succeedingBackupsList) - .getOrDefault(volumeObjectTO.getVolumeId(), new LinkedList<>()); + if (succeedingVmSnapshot != null) { + succeedingDeltaPaths = gatherSnapshotReferencesOfChildrenSnapshot(List.of(volumeObjectTO), succeedingVmSnapshot).getOrDefault(volumeObjectTO.getVolumeId(), List.of()) + .stream().map(SnapshotDataStoreVO::getInstallPath).collect(Collectors.toList()); if (!childIsVolume && !runningVm && succeedingDeltaPaths.isEmpty()) { succeedingDeltaPaths = List.of(volumeObjectTO.getPath()); @@ -2071,7 +1964,7 @@ protected List populateDeltasToRemoveAndToMergeAndUpdateVolume VolumeObjectTO volumeObjectTO = optional.get(); if (volumesNotPartOfTheBackupBeingRestored.contains(volumeObjectTO)) { - deltasToBeMerged.add(createDeltaMergeTree(true, false, deltaOnPrimary, volumeObjectTO, null, new ArrayList<>())); + deltasToBeMerged.add(createDeltaMergeTree(true, false, deltaOnPrimary, volumeObjectTO, null)); continue; } @@ -2189,8 +2082,7 @@ protected List getVolumesThatAreNotPartOfTheBackup(List volumeTOs, HashMap volumeUuidToDeltaPrimaryRef, HashMap volumeUuidToDeltaSecondaryRef, TakeKbossBackupAnswer answer, List parentBackupDeltasOnPrimary, - VMSnapshotVO succeedingVmSnapshot, BackupVO backupVO, boolean fullBackup, VirtualMachine userVm, Long hostId, boolean endChain, boolean isolated, - InternalBackupJoinVO succeedingBackup) { + List succeedingVmSnapshots, BackupVO backupVO, boolean fullBackup, VirtualMachine userVm, Long hostId, boolean endChain, boolean isolated) { long physicalBackupSize = 0; logger.debug("Processing backup [{}] success.", backupVO.getUuid()); for (VolumeObjectTO volumeObjectTO : volumeTOs) { @@ -2198,7 +2090,7 @@ protected void processBackupSuccess(boolean runningVm, List volu physicalBackupSize, endChain, isolated, backupVO); } - expungeOldDeltasAndUpdateVmSnapshotOrBackup(parentBackupDeltasOnPrimary, succeedingVmSnapshot, succeedingBackup); + expungeOldDeltasAndUpdateVmSnapshotIfNeeded(parentBackupDeltasOnPrimary, succeedingVmSnapshots.isEmpty() ? null : succeedingVmSnapshots.get(0)); backupVO.setSize(physicalBackupSize); backupVO.setStatus(Backup.Status.BackedUp); @@ -2241,7 +2133,7 @@ protected void processRemovedBackups(List removedBackupIds) { * For every backup, except for the one which the command was issued, will set them as Expunged regardless and hope operators will look * at the logs. For the current one, if forced=false, will set it as error, otherwise, will set it as Expunged as well. * */ - protected boolean processRemoveBackupFailures(boolean forced, Answer[] deleteAnswers, List removedBackupIds, InternalBackupJoinVO backupJoinVO, VirtualMachine vm) { + protected boolean processRemoveBackupFailures(boolean forced, Answer[] deleteAnswers, List removedBackupIds, InternalBackupJoinVO backupJoinVO) { List failures = Arrays.stream(deleteAnswers).filter(answer -> !answer.getResult()).collect(Collectors.toList()); Set failedToRemoveBackupIdSet = new HashSet<>(); if (CollectionUtils.isNotEmpty(failures)) { @@ -2262,7 +2154,6 @@ protected boolean processRemoveBackupFailures(boolean forced, Answer[] deleteAns logger.info("Since backup delete command was not forced, will not set the main backup [{}] as Expunged, will set it as error instead.", failedVO.getUuid()); failedVO.setStatus(Backup.Status.Error); backupDao.update(failedVO.getId(), failedVO); - vmInstanceDetailsDao.addDetail(vm.getId(), VmDetailConstants.LAST_KNOWN_STATE, vm.getState().name(), false); } for (Long failedToRemove : failedToRemoveBackupIdSet) { @@ -2369,41 +2260,17 @@ protected void handleRestoreException(Backup backup, VirtualMachine vm, Object j } else if (jobResult instanceof BackupProviderException) { throw (BackupProviderException) jobResult; } - throw new CloudRuntimeException(String.format("Exception while restoring KVM internal incremental backup [%s]. Check the logs for more information.", backup.getUuid()), ((Throwable)jobResult).getCause()); - } - - protected boolean finishAllChains(VirtualMachine vm, List currents) { - if (currents.isEmpty()) { - logger.debug("There is no current active chain, no need to do anything."); - return true; - } - - for (InternalBackupJoinVO current : currents) { - if (!mergeCurrentBackupDeltas(current)) { - UserVmVO vmVO = userVmDao.findById(vm.getId()); - logger.error("Failed to merge deltas for VM [{}] during backup offering removal process. Changing its state to [{}].", vm, VirtualMachine.State.BackupError); - BackupVO backupVO = backupDao.findById(current.getId()); - backupVO.setStatus(Backup.Status.Error); - backupDao.update(backupVO.getId(), backupVO); - vmVO.setState(VirtualMachine.State.BackupError); - userVmDao.update(vmVO.getId(), vmVO); - - return false; - } - setEndOfChainAndRemoveCurrentForBackup(current); - } - return true; + throw new CloudRuntimeException(String.format("Exception while restoring KVM internal incremental backup [%s]. Check the logs for more information.", backup.getUuid()), + ((Throwable)jobResult).getCause()); } - protected boolean endBackupChain(VirtualMachine vm, Long backupScheduleId) { - InternalBackupJoinVO current = internalBackupJoinDao.findCurrent(vm.getId(), backupScheduleId); + protected boolean endBackupChain(VirtualMachine vm) { + InternalBackupJoinVO current = internalBackupJoinDao.findCurrent(vm.getId()); if (current == null) { logger.debug("There is no current active chain, no need to do anything."); return true; } - validateVmState(vm, "end backup chain"); - if (mergeCurrentBackupDeltas(current)) { setEndOfChainAndRemoveCurrentForBackup(current); return true; @@ -2418,11 +2285,8 @@ protected boolean endBackupChain(VirtualMachine vm, Long backupScheduleId) { * */ protected boolean mergeCurrentBackupDeltas(InternalBackupJoinVO backupJoinVO) { VirtualMachine userVm = userVmDao.findById(backupJoinVO.getVmId()); - - List succeedingBackupList = getSucceedingBackupList(backupJoinVO); - InternalBackupJoinVO succeedingBackup = succeedingBackupList.isEmpty() ? null : succeedingBackupList.get(0); VMSnapshotVO succeedingVmSnapshot = getSucceedingVmSnapshot(backupJoinVO); - MergeDiskOnlyVmSnapshotCommand cmd = buildMergeDiskOnlyVmSnapshotCommandForCurrentBackup(backupJoinVO, userVm, succeedingVmSnapshot, succeedingBackupList); + MergeDiskOnlyVmSnapshotCommand cmd = buildMergeDiskOnlyVmSnapshotCommandForCurrentBackup(backupJoinVO, userVm, succeedingVmSnapshot); Long hostId = vmSnapshotHelper.pickRunningHost(backupJoinVO.getVmId()); Answer answer = sendBackupCommand(hostId, cmd); @@ -2432,10 +2296,9 @@ protected boolean mergeCurrentBackupDeltas(InternalBackupJoinVO backupJoinVO) { return false; } - List deltasOnPrimary = internalBackupStoragePoolDao.listByBackupId(backupJoinVO.getId()); - expungeOldDeltasAndUpdateVmSnapshotOrBackup(deltasOnPrimary, succeedingVmSnapshot, succeedingBackup); + expungeOldDeltasAndUpdateVmSnapshotIfNeeded(internalBackupStoragePoolDao.listByBackupId(backupJoinVO.getId()), succeedingVmSnapshot); - if (ObjectUtils.anyNotNull(succeedingVmSnapshot, succeedingBackup)) { + if (succeedingVmSnapshot != null) { return true; } @@ -2450,18 +2313,18 @@ protected boolean mergeCurrentBackupDeltas(InternalBackupJoinVO backupJoinVO) { } protected void createDeleteCommandsAndMergeTrees(List volumeObjectTOs, Commands commands, List deletedDeltas, - VMSnapshotVO vmSnapshotSucceedingCurrentBackup, List deltaMergeTreeTOList, InternalBackupJoinVO currentBackup) { + VMSnapshotVO vmSnapshotSucceedingCurrentBackup, List deltaMergeTreeTOList) { for (VolumeObjectTO volumeObjectTO : volumeObjectTOs) { - InternalBackupStoragePoolVO delta = internalBackupStoragePoolDao.findOneByVolumeIdAndBackupId(volumeObjectTO.getVolumeId(), currentBackup.getId()); + InternalBackupStoragePoolVO delta = internalBackupStoragePoolDao.findOneByVolumeId(volumeObjectTO.getVolumeId()); if (delta == null) { continue; } - if (vmSnapshotSucceedingCurrentBackup == null) { + if (delta.getBackupDeltaPath().equals(volumeObjectTO.getPath())) { commands.addCommand(new DeleteCommand(new BackupDeltaTO(volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, delta.getBackupDeltaParentPath()))); deletedDeltas.add(delta); logger.debug("Volume [{}] has a backup delta that will be deleted as part of the preparation to revert a VM snapshot.", volumeObjectTO.getUuid()); } else { - deltaMergeTreeTOList.add(createDeltaMergeTree(false, false, delta, volumeObjectTO, vmSnapshotSucceedingCurrentBackup, new ArrayList<>())); + deltaMergeTreeTOList.add(createDeltaMergeTree(false, false, delta, volumeObjectTO, vmSnapshotSucceedingCurrentBackup)); } } } @@ -2495,17 +2358,16 @@ protected Pair, InternalBackupJoinVO> getParentsToBeE return new Pair<>(backupParentsToBeExpunged, lastAliveBackup); } - private MergeDiskOnlyVmSnapshotCommand buildMergeDiskOnlyVmSnapshotCommandForCurrentBackup(InternalBackupJoinVO backupJoinVO, VirtualMachine userVm, VMSnapshotVO vmSnapshot, - List succeedingBackupList) { + protected MergeDiskOnlyVmSnapshotCommand buildMergeDiskOnlyVmSnapshotCommandForCurrentBackup(InternalBackupJoinVO backupJoinVO, VirtualMachine userVm, VMSnapshotVO vmSnapshot) { List deltaMergeTreeTOs = new ArrayList<>(); List volumeTOs = vmSnapshotHelper.getVolumeTOList(backupJoinVO.getVmId()); + Map> volumeIdToSnapshotDataStoreList = gatherSnapshotReferencesOfChildrenSnapshot(volumeTOs, vmSnapshot); List deltasOnPrimary = internalBackupStoragePoolDao.listByBackupId(backupJoinVO.getId()); for (VolumeObjectTO volumeObjectTO : volumeTOs) { - KbossTO kbossTO = new KbossTO(volumeObjectTO, new LinkedList<>()); - boolean childIsVolume = vmSnapshot == null && succeedingBackupList.isEmpty(); - createDeltaMergeTreeForVolume(childIsVolume, userVm.getState() == VirtualMachine.State.Running, deltasOnPrimary, vmSnapshot, kbossTO, succeedingBackupList); + KbossTO kbossTO = new KbossTO(volumeObjectTO, volumeIdToSnapshotDataStoreList.getOrDefault(volumeObjectTO.getId(), new ArrayList<>())); + createDeltaMergeTreeForVolume(vmSnapshot == null, userVm.getState() == VirtualMachine.State.Running, deltasOnPrimary, vmSnapshot, kbossTO); if (kbossTO.getDeltaMergeTreeTO() != null) { deltaMergeTreeTOs.add(kbossTO.getDeltaMergeTreeTO()); } else { @@ -2558,11 +2420,9 @@ protected List getBackupJoinParents(BackupVO backupVO, boo List ancestorBackups; if (includeRemoved) { - ancestorBackups = internalBackupJoinDao.listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(backupVO.getVmId(), backupVO.getBackupScheduleId(), - backupVO.getDate()); + ancestorBackups = internalBackupJoinDao.listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(backupVO.getVmId(), backupVO.getDate()); } else { - ancestorBackups = internalBackupJoinDao.listByBackedUpAndVmIdAndDateBeforeOrAfterOrderBy(backupVO.getVmId(), backupVO.getBackupScheduleId(), backupVO.getDate(), true, - false); + ancestorBackups = internalBackupJoinDao.listByBackedUpAndVmIdAndDateBeforeOrAfterOrderBy(backupVO.getVmId(), backupVO.getDate(), true, false); } for (int i = 0; i < ancestorBackups.size(); i++) { @@ -2589,8 +2449,7 @@ protected int getChainSizeForBackup(BackupOfferingVO offering, long zoneId) { * @return list of children, or and empty list if no children found. * */ protected List getBackupJoinChildren(BackupVO backupVO) { - List children = internalBackupJoinDao.listByBackedUpAndVmIdAndDateBeforeOrAfterOrderBy(backupVO.getVmId(), backupVO.getBackupScheduleId(), - backupVO.getDate(), false, true); + List children = internalBackupJoinDao.listByBackedUpAndVmIdAndDateBeforeOrAfterOrderBy(backupVO.getVmId(), backupVO.getDate(), false, true); long parentId = backupVO.getId(); for (int i = 0; i < children.size(); i++) { @@ -2632,7 +2491,7 @@ protected void updateBackupStatusToBackingUp(List volumeTOs, Bac * Retrieves the current backup and removes the CURRENT detail. If the informed backup is not the end of chain, sets it as the new CURRENT * */ protected void updateCurrentBackup(InternalBackupJoinVO backup) { - InternalBackupJoinVO current = internalBackupJoinDao.findCurrent(backup.getVmId(), backup.getScheduleId()); + InternalBackupJoinVO current = internalBackupJoinDao.findCurrent(backup.getVmId()); if (current != null) { backupDetailDao.removeDetail(current.getId(), CURRENT); @@ -2673,29 +2532,20 @@ protected void setBackupAsInvalidAndSendAlert(BackupVO backupVO, String msg) { backupVO.getName()), msg); } - protected void configureKbossTosForCleanup(UserVmVO userVmVO, List deltasOnPrimary, Map> volumeIdToDeltasAfterCurrent, - List deltasOnSecondary, List parentDeltasOnPrimary, List kbossTOS, boolean errorOnCreation) { + protected void configureKbossTosForCleanup(UserVmVO userVmVO, List deltasOnPrimary, List deltasOnSecondary, boolean runningVM, + List parentDeltasOnPrimary, List kbossTOS) { for (VolumeObjectTO volumeObjectTO : vmSnapshotHelper.getVolumeTOList(userVmVO.getId())) { InternalBackupStoragePoolVO deltaOnPrimary = deltasOnPrimary.stream() - .filter(delta -> delta.getVolumeId() == volumeObjectTO.getVolumeId()).findFirst().orElseThrow(); + .filter(delta -> delta.getVolumeId() == volumeObjectTO.getVolumeId()).findFirst().orElseThrow(); + volumeObjectTO.setPath(deltaOnPrimary.getBackupDeltaPath()); + InternalBackupDataStoreVO deltaOnSecondary = deltasOnSecondary.stream().filter(delta -> delta.getVolumeId() == volumeObjectTO.getVolumeId()).findFirst().orElseThrow(); - KbossTO kbossTO; - if (errorOnCreation) { - InternalBackupStoragePoolVO parent = parentDeltasOnPrimary.stream().filter(delta -> delta.getVolumeId() == volumeObjectTO.getVolumeId()).findFirst().orElse(null); - kbossTO = new KbossTO(volumeObjectTO, parent == null ? deltaOnPrimary.getBackupDeltaParentPath() : parent.getBackupDeltaPath(), deltaOnSecondary.getBackupPath(), - volumeIdToDeltasAfterCurrent.get(volumeObjectTO.getId())); - if (parent != null) { - kbossTO.setParentDeltaPathOnPrimary(parent.getBackupDeltaParentPath()); - } - kbossTO.setOldVolumePath(volumeObjectTO.getPath()); - volumeObjectTO.setPath(deltaOnPrimary.getBackupDeltaPath()); - } else { - kbossTO = new KbossTO(volumeObjectTO, deltaOnPrimary.getBackupDeltaPath(), deltaOnSecondary.getBackupPath(), - volumeIdToDeltasAfterCurrent.get(volumeObjectTO.getId())); - kbossTO.setParentDeltaPathOnPrimary(deltaOnPrimary.getBackupDeltaParentPath()); - } + KbossTO kbossTO = new KbossTO(volumeObjectTO, deltaOnPrimary.getBackupDeltaParentPath(), deltaOnSecondary.getBackupPath()); + parentDeltasOnPrimary.stream() + .filter(delta -> delta.getVolumeId() == volumeObjectTO.getVolumeId()).findFirst() + .ifPresent(parentDelta -> kbossTO.setParentDeltaPathOnPrimary(parentDelta.getBackupDeltaParentPath())); kbossTOS.add(kbossTO); } } @@ -2761,11 +2611,11 @@ protected void updateReferencesAfterPrepareForSnapshotRevert(List backupChainSize; + @Mock private DataStoreManager dataStoreManagerMock; @@ -287,7 +299,7 @@ public class KbossBackupProviderTest { private long vmId = 319832; private long volumeId = 41; - private Long backupId = 312L; + Long backupId = 312L; @Before public void setup() { @@ -359,7 +371,7 @@ public void removeVMFromBackupOfferingTestNoActiveChain() { @Test public void removeVMFromBackupOfferingTestWithActiveChain() { - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listCurrents(vmId, true); + doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findCurrent(vmId); doReturn(true).when(kbossBackupProviderSpy).mergeCurrentBackupDeltas(any()); doReturn(VirtualMachine.State.Stopped).when(virtualMachineMock).getState(); @@ -369,12 +381,25 @@ public void removeVMFromBackupOfferingTestWithActiveChain() { assertTrue(result); } + @Test + public void removeVMFromBackupOfferingTestFailedToEndChain() { + doReturn(VirtualMachine.State.Stopped).when(userVmVOMock).getState(); + doReturn(false).when(kbossBackupProviderSpy).endBackupChain(any()); + doReturn(userVmVOMock).when(userVmDaoMock).findById(any()); + doNothing().when(vmInstanceDetailsDaoMock).addDetail(Mockito.anyLong(), any(), any(), Mockito.anyBoolean()); + + boolean result = kbossBackupProviderSpy.removeVMFromBackupOffering(userVmVOMock); + + verify(vmInstanceDetailsDaoMock, Mockito.times(1)).addDetail(Mockito.anyLong(), any(), any(), Mockito.anyBoolean()); + verify(userVmDaoMock, Mockito.times(1)).update(Mockito.anyLong(), any()); + assertFalse(result); + } + @Test public void getBackupJoinParentsTestIncludeRemovedEmptyList() { Date date = DateUtil.now(); doReturn(date).when(backupVoMock).getDate(); - doReturn(null).when(backupVoMock).getBackupScheduleId(); - doReturn(new ArrayList<>()).when(internalBackupJoinDaoMock).listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(vmId, null, date); + doReturn(new ArrayList<>()).when(internalBackupJoinDaoMock).listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(vmId, date); List result = kbossBackupProviderSpy.getBackupJoinParents(backupVoMock, true); @@ -385,9 +410,8 @@ public void getBackupJoinParentsTestIncludeRemovedEmptyList() { public void getBackupJoinParentsTestIncludeRemovedAncestorIsEndOfChain() { Date date = DateUtil.now(); doReturn(date).when(backupVoMock).getDate(); - doReturn(null).when(backupVoMock).getBackupScheduleId(); doReturn(true).when(internalBackupJoinVoMock).getEndOfChain(); - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(vmId, null, date); + doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(vmId, date); List result = kbossBackupProviderSpy.getBackupJoinParents(backupVoMock, true); @@ -398,13 +422,12 @@ public void getBackupJoinParentsTestIncludeRemovedAncestorIsEndOfChain() { public void getBackupJoinParentsTestIncludeRemovedAncestorMultipleAncestors() { Date date = DateUtil.now(); doReturn(date).when(backupVoMock).getDate(); - doReturn(null).when(backupVoMock).getBackupScheduleId(); InternalBackupJoinVO internalBackupJoinVoMock1 = Mockito.mock(InternalBackupJoinVO.class); doReturn(false).when(internalBackupJoinVoMock1).getEndOfChain(); InternalBackupJoinVO internalBackupJoinVoMock2 = Mockito.mock(InternalBackupJoinVO.class); doReturn(false).when(internalBackupJoinVoMock2).getEndOfChain(); doReturn(true).when(internalBackupJoinVoMock).getEndOfChain(); - doReturn(List.of(internalBackupJoinVoMock1, internalBackupJoinVoMock2, internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(vmId, null, date); + doReturn(List.of(internalBackupJoinVoMock1, internalBackupJoinVoMock2, internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(vmId, date); List result = kbossBackupProviderSpy.getBackupJoinParents(backupVoMock, true); @@ -415,13 +438,12 @@ public void getBackupJoinParentsTestIncludeRemovedAncestorMultipleAncestors() { public void getBackupJoinParentsTestIncludeRemovedAncestorMultipleAncestorsNoEndOfChain() { Date date = DateUtil.now(); doReturn(date).when(backupVoMock).getDate(); - doReturn(null).when(backupVoMock).getBackupScheduleId(); InternalBackupJoinVO internalBackupJoinVoMock1 = Mockito.mock(InternalBackupJoinVO.class); doReturn(false).when(internalBackupJoinVoMock1).getEndOfChain(); InternalBackupJoinVO internalBackupJoinVoMock2 = Mockito.mock(InternalBackupJoinVO.class); doReturn(false).when(internalBackupJoinVoMock2).getEndOfChain(); doReturn(false).when(internalBackupJoinVoMock).getEndOfChain(); - doReturn(List.of(internalBackupJoinVoMock1, internalBackupJoinVoMock2, internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(vmId, null, date); + doReturn(List.of(internalBackupJoinVoMock1, internalBackupJoinVoMock2, internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listIncludingRemovedByVmIdAndBeforeDateOrderByCreatedDesc(vmId, date); List result = kbossBackupProviderSpy.getBackupJoinParents(backupVoMock, true); @@ -432,13 +454,12 @@ public void getBackupJoinParentsTestIncludeRemovedAncestorMultipleAncestorsNoEnd public void getBackupJoinParentsTestNoRemovedAncestorMultipleAncestorsNoEndOfChain() { Date date = DateUtil.now(); doReturn(date).when(backupVoMock).getDate(); - doReturn(null).when(backupVoMock).getBackupScheduleId(); InternalBackupJoinVO internalBackupJoinVoMock1 = Mockito.mock(InternalBackupJoinVO.class); doReturn(false).when(internalBackupJoinVoMock1).getEndOfChain(); InternalBackupJoinVO internalBackupJoinVoMock2 = Mockito.mock(InternalBackupJoinVO.class); doReturn(false).when(internalBackupJoinVoMock2).getEndOfChain(); doReturn(false).when(internalBackupJoinVoMock).getEndOfChain(); - doReturn(List.of(internalBackupJoinVoMock1, internalBackupJoinVoMock2, internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listByBackedUpAndVmIdAndDateBeforeOrAfterOrderBy(vmId, null, date, true, + doReturn(List.of(internalBackupJoinVoMock1, internalBackupJoinVoMock2, internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listByBackedUpAndVmIdAndDateBeforeOrAfterOrderBy(vmId, date, true, false); List result = kbossBackupProviderSpy.getBackupJoinParents(backupVoMock, false); @@ -647,30 +668,32 @@ public void getSucceedingVmSnapshotListTestCurrentVmSnapshotHasParentsCreatedBef } @Test - public void mapVolumesToVmSnapshotReferencesTestVmSnapshotAndBackupVOListIsEmpty() { - kbossBackupProviderSpy.mapVolumesToVmSnapshotAndBackupReferences(List.of(), List.of(), List.of()); + public void mapVolumesToVmSnapshotReferencesTestVmSnapshotVOListIsEmpty() { + kbossBackupProviderSpy.mapVolumesToVmSnapshotReferences(List.of(), List.of()); verify(vmSnapshotHelperMock, Mockito.never()).getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(1); } @Test - public void mapVolumesToVmSnapshotAndBackupReferencesTestVmSnapshotAndBackupVOListHasTwoElements() { + public void mapVolumesToVmSnapshotReferencesTestVmSnapshotVOListHasTwoElements() { VMSnapshotVO vmSnapshotVoMock1 = Mockito.mock(VMSnapshotVO.class); doReturn(1L).when(vmSnapshotVoMock).getId(); doReturn(2L).when(vmSnapshotVoMock1).getId(); + doNothing().when(kbossBackupProviderSpy).mapVolumesToSnapshotReferences(Mockito.anyList(), Mockito.anyList(), anyMap()); - kbossBackupProviderSpy.mapVolumesToVmSnapshotAndBackupReferences(List.of(), List.of(vmSnapshotVoMock, vmSnapshotVoMock1), List.of()); + kbossBackupProviderSpy.mapVolumesToVmSnapshotReferences(List.of(), List.of(vmSnapshotVoMock, vmSnapshotVoMock1)); verify(vmSnapshotHelperMock, times(1)).getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(1); verify(vmSnapshotHelperMock, times(1)).getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(2); + verify(kbossBackupProviderSpy, times(1)).mapVolumesToSnapshotReferences(Mockito.anyList(), Mockito.anyList(), anyMap()); } @Test public void createDeltaReferencesTestFullBackupEndOfChain() { doReturn(internalBackupDataStoreVoMock).when(internalBackupDataStoreDaoMock).persist(any()); - kbossBackupProviderSpy.createDeltaReferences(true, true, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, new KbossTO(volumeObjectToMock, - new LinkedList<>())); + kbossBackupProviderSpy.createDeltaReferences(true, + true, true, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, new KbossTO(volumeObjectToMock, List.of())); verify(internalBackupDataStoreDaoMock, Mockito.times(1)).persist(any()); } @@ -679,8 +702,8 @@ public void createDeltaReferencesTestFullBackupEndOfChain() { public void createDeltaReferencesTestIsolatedBackup() { doReturn(internalBackupDataStoreVoMock).when(internalBackupDataStoreDaoMock).persist(any()); - kbossBackupProviderSpy.createDeltaReferences(true, true, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, new KbossTO(volumeObjectToMock, - new LinkedList<>())); + kbossBackupProviderSpy.createDeltaReferences(true, + true, true, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, new KbossTO(volumeObjectToMock, List.of())); verify(internalBackupDataStoreDaoMock, Mockito.times(1)).persist(any()); verify(kbossBackupProviderSpy, Mockito.times(0)).findAndSetParentBackupPath(any(), any(), any()); @@ -691,11 +714,11 @@ public void createDeltaReferencesTestIsolatedBackup() { @Test public void createDeltaReferencesTestNotFullBackupEndOfChain() { doReturn(internalBackupDataStoreVoMock).when(internalBackupDataStoreDaoMock).persist(any()); - KbossTO kbossTO = new KbossTO(volumeObjectToMock, new LinkedList<>()); - doReturn(null).when(kbossBackupProviderSpy).createDeltaMergeTreeForVolume(false, true, List.of(), null, kbossTO, List.of()); + KbossTO kbossTO = new KbossTO(volumeObjectToMock, List.of()); + doReturn(null).when(kbossBackupProviderSpy).createDeltaMergeTreeForVolume(false, true, List.of(), null, kbossTO); doNothing().when(kbossBackupProviderSpy).findAndSetParentBackupPath(List.of(), null, kbossTO); - kbossBackupProviderSpy.createDeltaReferences(false, true, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, kbossTO); + kbossBackupProviderSpy.createDeltaReferences(false, true, true, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, kbossTO); verify(internalBackupDataStoreDaoMock, Mockito.times(1)).persist(any()); verify(kbossBackupProviderSpy, Mockito.times(1)).findAndSetParentBackupPath(List.of(), null, kbossTO); @@ -705,8 +728,8 @@ public void createDeltaReferencesTestNotFullBackupEndOfChain() { public void createDeltaReferencesTestFullBackupNotEndOfChainDoesNotHaveVmSnapshotSucceedingLastBackup() { doReturn(internalBackupDataStoreVoMock).when(internalBackupDataStoreDaoMock).persist(any()); - kbossBackupProviderSpy.createDeltaReferences(true, true, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, new KbossTO(volumeObjectToMock, - new LinkedList<>())); + kbossBackupProviderSpy.createDeltaReferences(true, + false, true, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, new KbossTO(volumeObjectToMock, List.of())); verify(internalBackupDataStoreDaoMock, Mockito.times(1)).persist(any()); } @@ -769,7 +792,7 @@ public void orchestrateTakeBackupTestIsolatedBackupFailed() { assertFalse(result.first()); assertNull(result.second()); verify(kbossBackupProviderSpy, Mockito.times(1)).setBackupAsIsolated(backupVoMock); - verify(kbossBackupProviderSpy, Mockito.times(2)).createDeltaReferences(Mockito.anyBoolean(), Mockito.anyBoolean(), any(), any(), any(), any(), any(), any(), any()); + verify(kbossBackupProviderSpy, Mockito.times(2)).createDeltaReferences(Mockito.anyBoolean(), Mockito.anyBoolean(), Mockito.anyBoolean(), any(), any(), any(), any(), any(), any(), any()); verify(kbossBackupProviderSpy, Mockito.times(1)).processBackupFailure(any(), any(), Mockito.anyLong(), Mockito.anyBoolean(), any()); } @@ -792,7 +815,7 @@ public void orchestrateTakeBackupTestIsolatedBackupSuccessWithCompression() { doReturn(takeKbossBackupAnswerMock).when(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); doReturn(true).when(takeKbossBackupAnswerMock).getResult(); doNothing().when(kbossBackupProviderSpy).processBackupSuccess(anyBoolean(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), any(), - anyLong(), anyBoolean(), anyBoolean(), any()); + anyLong(), anyBoolean(), anyBoolean()); doReturn(true).when(kbossBackupProviderSpy).offeringSupportsCompression(internalBackupJoinVoMock); doNothing().when(kbossBackupProviderSpy).compressBackupAsync(internalBackupJoinVoMock, 0, 0); @@ -800,9 +823,9 @@ public void orchestrateTakeBackupTestIsolatedBackupSuccessWithCompression() { assertTrue(result.first()); assertEquals(backupId, result.second()); verify(kbossBackupProviderSpy, Mockito.times(1)).setBackupAsIsolated(backupVoMock); - verify(kbossBackupProviderSpy, Mockito.times(2)).createDeltaReferences(Mockito.anyBoolean(), Mockito.anyBoolean(), any(), any(), any(), any(), any(), any(), any()); + verify(kbossBackupProviderSpy, Mockito.times(2)).createDeltaReferences(Mockito.anyBoolean(), Mockito.anyBoolean(), Mockito.anyBoolean(), any(), any(), any(), any(), any(), any(), any()); verify(kbossBackupProviderSpy, Mockito.times(1)).processBackupSuccess(anyBoolean(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), any(), - anyLong(), anyBoolean(), anyBoolean(), any()); + anyLong(), anyBoolean(), anyBoolean()); verify(kbossBackupProviderSpy, Mockito.times(1)).compressBackupAsync(internalBackupJoinVoMock, 0, 0); } @@ -827,7 +850,7 @@ public void orchestrateTakeBackupTestBackupSuccessWithValidation() { doReturn(takeKbossBackupAnswerMock).when(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); doReturn(true).when(takeKbossBackupAnswerMock).getResult(); doNothing().when(kbossBackupProviderSpy).processBackupSuccess(anyBoolean(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), any(), - anyLong(), anyBoolean(), anyBoolean(), any()); + anyLong(), anyBoolean(), anyBoolean()); doReturn(false).when(kbossBackupProviderSpy).offeringSupportsCompression(internalBackupJoinVoMock); doNothing().when(kbossBackupProviderSpy).validateBackupAsyncIfHasOfferingSupport(any(), anyLong(), anyLong()); @@ -836,9 +859,9 @@ public void orchestrateTakeBackupTestBackupSuccessWithValidation() { assertEquals(backupId, result.second()); verify(internalBackupStoragePoolDaoMock).listByBackupId(0); verify(internalBackupDataStoreDaoMock).listByBackupId(0); - verify(kbossBackupProviderSpy, Mockito.times(2)).createDeltaReferences(Mockito.anyBoolean(), Mockito.anyBoolean(), any(), any(), any(), any(), any(), any(), any()); + verify(kbossBackupProviderSpy, Mockito.times(2)).createDeltaReferences(Mockito.anyBoolean(), Mockito.anyBoolean(), Mockito.anyBoolean(), any(), any(), any(), any(), any(), any(), any()); verify(kbossBackupProviderSpy, Mockito.times(1)).processBackupSuccess(anyBoolean(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), any(), - anyLong(), anyBoolean(), anyBoolean(), any()); + anyLong(), anyBoolean(), anyBoolean()); verify(kbossBackupProviderSpy, Mockito.times(1)).validateBackupAsyncIfHasOfferingSupport(internalBackupJoinVoMock, 0, 0); } @@ -975,7 +998,7 @@ public void orchestrateDeleteBackupTestDeleteCurrentBackupWithNoChildrenWithPare doReturn(new Pair<>(List.of(), parentVo)).when(kbossBackupProviderSpy).getParentsToBeExpungedWithBackupAndAddThemToListOfDeleteCommands(any(), any()); doReturn(endPointMock).when(endPointSelectorMock).select((DataStore)null); doReturn(null).when(kbossBackupProviderSpy).sendBackupCommands(anyLong(), any()); - doReturn(false).when(kbossBackupProviderSpy).processRemoveBackupFailures(anyBoolean(), any(), any(), any(), any()); + doReturn(false).when(kbossBackupProviderSpy).processRemoveBackupFailures(anyBoolean(), any(), any(), any()); doNothing().when(kbossBackupProviderSpy).processRemovedBackups(any()); @@ -1005,7 +1028,7 @@ public void orchestrateDeleteBackupTestDeleteCurrentBackupWithNoChildrenWithPare doReturn(new Pair<>(List.of(), parentVo)).when(kbossBackupProviderSpy).getParentsToBeExpungedWithBackupAndAddThemToListOfDeleteCommands(any(), any()); doReturn(endPointMock).when(endPointSelectorMock).select((DataStore)null); doReturn(null).when(kbossBackupProviderSpy).sendBackupCommands(anyLong(), any()); - doReturn(true).when(kbossBackupProviderSpy).processRemoveBackupFailures(anyBoolean(), any(), any(), any(), any()); + doReturn(true).when(kbossBackupProviderSpy).processRemoveBackupFailures(anyBoolean(), any(), any(), any()); doNothing().when(kbossBackupProviderSpy).processRemovedBackups(any()); @@ -1048,6 +1071,8 @@ public void orchestrateRestoreVMFromBackupTestSameVmCurrentBackupTimeOut() throw doReturn(new Pair<>(true, backupVoMock)).when(kbossBackupProviderSpy).validateCompressionStateForRestoreAndGetBackup(backupId); long currentBackupId = 39; InternalBackupJoinVO currentBackup = Mockito.mock(InternalBackupJoinVO.class); + doReturn(currentBackupId).when(currentBackup).getId(); + doReturn(currentBackup).when(internalBackupJoinDaoMock).findCurrent(vmId); doReturn(hostVOMock).when(kbossBackupProviderSpy).getHostToRestore(virtualMachineMock, false, null); doNothing().when(kbossBackupProviderSpy).createAndAttachVolumes(any(), any(), any(), any()); doReturn(Set.of()).when(kbossBackupProviderSpy).generateBackupAndVolumePairsToRestore(any(), any(), any(), anyBoolean()); @@ -1070,6 +1095,8 @@ public void orchestrateRestoreVMFromBackupTestSameVmCurrentBackupNullAnswers() t doReturn(new Pair<>(true, backupVoMock)).when(kbossBackupProviderSpy).validateCompressionStateForRestoreAndGetBackup(backupId); long currentBackupId = 39; InternalBackupJoinVO currentBackup = Mockito.mock(InternalBackupJoinVO.class); + doReturn(currentBackupId).when(currentBackup).getId(); + doReturn(currentBackup).when(internalBackupJoinDaoMock).findCurrent(vmId); doReturn(hostVOMock).when(kbossBackupProviderSpy).getHostToRestore(virtualMachineMock, false, null); doNothing().when(kbossBackupProviderSpy).createAndAttachVolumes(any(), any(), any(), any()); doReturn(Set.of()).when(kbossBackupProviderSpy).generateBackupAndVolumePairsToRestore(any(), any(), any(), anyBoolean()); @@ -1080,6 +1107,7 @@ public void orchestrateRestoreVMFromBackupTestSameVmCurrentBackupNullAnswers() t boolean result = kbossBackupProviderSpy.orchestrateRestoreVMFromBackup(backupVoMock, virtualMachineMock, false, null, true); + verify(internalBackupStoragePoolDaoMock).listByBackupId(currentBackupId); verify(kbossBackupProviderSpy).createAndAttachVolumes(any(), any(), any(), any()); verify(kbossBackupProviderSpy).populateDeltasToRemoveAndToMergeAndUpdateVolumePaths(any(), any(), any(), any(), any()); assertFalse(result); @@ -1092,6 +1120,8 @@ public void orchestrateRestoreVMFromBackupTestSameVmCurrentBackupAnswerFalse() t doReturn(new Pair<>(true, backupVoMock)).when(kbossBackupProviderSpy).validateCompressionStateForRestoreAndGetBackup(backupId); long currentBackupId = 39; InternalBackupJoinVO currentBackup = Mockito.mock(InternalBackupJoinVO.class); + doReturn(currentBackupId).when(currentBackup).getId(); + doReturn(currentBackup).when(internalBackupJoinDaoMock).findCurrent(vmId); doReturn(hostVOMock).when(kbossBackupProviderSpy).getHostToRestore(virtualMachineMock, false, null); doNothing().when(kbossBackupProviderSpy).createAndAttachVolumes(any(), any(), any(), any()); doReturn(Set.of()).when(kbossBackupProviderSpy).generateBackupAndVolumePairsToRestore(any(), any(), any(), anyBoolean()); @@ -1103,6 +1133,7 @@ public void orchestrateRestoreVMFromBackupTestSameVmCurrentBackupAnswerFalse() t boolean result = kbossBackupProviderSpy.orchestrateRestoreVMFromBackup(backupVoMock, virtualMachineMock, false, null, true); + verify(internalBackupStoragePoolDaoMock).listByBackupId(currentBackupId); verify(kbossBackupProviderSpy).createAndAttachVolumes(any(), any(), any(), any()); verify(kbossBackupProviderSpy).populateDeltasToRemoveAndToMergeAndUpdateVolumePaths(any(), any(), any(), any(), any()); assertFalse(result); @@ -1115,6 +1146,8 @@ public void orchestrateRestoreVMFromBackupTestSameVmQuickRestoreCurrentBackupAns doReturn(new Pair<>(true, backupVoMock)).when(kbossBackupProviderSpy).validateCompressionStateForRestoreAndGetBackup(backupId); long currentBackupId = 39; InternalBackupJoinVO currentBackup = Mockito.mock(InternalBackupJoinVO.class); + doReturn(currentBackupId).when(currentBackup).getId(); + doReturn(currentBackup).when(internalBackupJoinDaoMock).findCurrent(vmId); doReturn(hostVOMock).when(kbossBackupProviderSpy).getHostToRestore(virtualMachineMock, true, null); doNothing().when(kbossBackupProviderSpy).createAndAttachVolumes(any(), any(), any(), any()); doReturn(Set.of()).when(kbossBackupProviderSpy).generateBackupAndVolumePairsToRestore(any(), any(), any(), anyBoolean()); @@ -1123,14 +1156,18 @@ public void orchestrateRestoreVMFromBackupTestSameVmQuickRestoreCurrentBackupAns doReturn(VirtualMachine.State.Stopped).when(virtualMachineMock).getState(); doReturn(new Answer[]{Mockito.mock(Answer.class)}).when(kbossBackupProviderSpy).sendBackupCommands(anyLong(), any()); doReturn(true).when(kbossBackupProviderSpy).processRestoreAnswers(any(), any(), anyBoolean()); + doNothing().when(kbossBackupProviderSpy).setEndOfChainAndRemoveCurrentForBackup(currentBackup); doReturn(List.of()).when(kbossBackupProviderSpy).getVolumesToConsolidate(any(), any(), any(), anyLong(), anyBoolean()); doReturn(true).when(kbossBackupProviderSpy).finalizeQuickRestore(any(), anyList(), anyLong()); boolean result = kbossBackupProviderSpy.orchestrateRestoreVMFromBackup(backupVoMock, virtualMachineMock, true, null, true); + verify(internalBackupStoragePoolDaoMock).listByBackupId(currentBackupId); verify(kbossBackupProviderSpy).createAndAttachVolumes(any(), any(), any(), any()); verify(kbossBackupProviderSpy).populateDeltasToRemoveAndToMergeAndUpdateVolumePaths(any(), any(), any(), any(), any()); verify(kbossBackupProviderSpy).updateVolumePathsAndSizeIfNeeded(any(), any(), anyList(), anyList(), anyBoolean()); + verify(internalBackupStoragePoolDaoMock).expungeByBackupId(currentBackupId); + verify(kbossBackupProviderSpy).setEndOfChainAndRemoveCurrentForBackup(currentBackup); verify(kbossBackupProviderSpy).finalizeQuickRestore(any(), anyList(), anyLong()); assertTrue(result); } @@ -1340,25 +1377,25 @@ public void validateBackupTestValidateWithValidationVm() { } @Test - public void finishBackupChainsTestInvalidState() { + public void finishBackupChainTestInvalidState() { doReturn(userVmVOMock).when(userVmDaoMock).findById(vmId); doReturn(VirtualMachine.State.Migrating).when(userVmVOMock).getState(); - boolean result = kbossBackupProviderSpy.finishBackupChains(virtualMachineMock); + boolean result = kbossBackupProviderSpy.finishBackupChain(virtualMachineMock); assertFalse(result); } @Test - public void finishBackupChainsTestRunningVm() { + public void finishBackupChainTestRunningVm() { doReturn(userVmVOMock).when(userVmDaoMock).findById(vmId); doReturn(VirtualMachine.State.Running).when(userVmVOMock).getState(); - doReturn(true).when(kbossBackupProviderSpy).finishAllChains(eq(userVmVOMock), any()); + doReturn(true).when(kbossBackupProviderSpy).endBackupChain(userVmVOMock); - boolean result = kbossBackupProviderSpy.finishBackupChains(virtualMachineMock); + boolean result = kbossBackupProviderSpy.finishBackupChain(virtualMachineMock); assertTrue(result); - verify(kbossBackupProviderSpy).finishAllChains(eq(userVmVOMock), any()); + verify(kbossBackupProviderSpy).endBackupChain(userVmVOMock); } @Test @@ -1367,7 +1404,7 @@ public void finishBackupChainTestBackupError() { doReturn(VirtualMachine.State.BackupError).when(userVmVOMock).getState(); doReturn(true).when(kbossBackupProviderSpy).normalizeBackupErrorAndFinishChain(userVmVOMock); - boolean result = kbossBackupProviderSpy.finishBackupChains(virtualMachineMock); + boolean result = kbossBackupProviderSpy.finishBackupChain(virtualMachineMock); assertTrue(result); verify(kbossBackupProviderSpy).normalizeBackupErrorAndFinishChain(userVmVOMock); @@ -1375,6 +1412,8 @@ public void finishBackupChainTestBackupError() { @Test public void prepareVmForSnapshotRevertTestNoCurrentBackup() { + doReturn(null).when(internalBackupJoinDaoMock).findCurrent(vmId); + kbossBackupProviderSpy.prepareVmForSnapshotRevert(vmSnapshotVoMock, virtualMachineMock); verify(kbossBackupProviderSpy, never()).getSucceedingVmSnapshot(any()); @@ -1382,7 +1421,7 @@ public void prepareVmForSnapshotRevertTestNoCurrentBackup() { @Test public void prepareVmForSnapshotRevertTestCurrentBackupBeforeVmSnapshot() { - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listCurrents(anyLong(), anyBoolean()); + doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findCurrent(vmId); doReturn(Date.from(Instant.EPOCH)).when(internalBackupJoinVoMock).getDate(); doReturn(Date.from(Instant.now())).when(vmSnapshotVoMock).getCreated(); @@ -1393,12 +1432,12 @@ public void prepareVmForSnapshotRevertTestCurrentBackupBeforeVmSnapshot() { @Test (expected = CloudRuntimeException.class) public void prepareVmForSnapshotRevertTestCurrentBackupAfterVmSnapshotTimeout() throws OperationTimedoutException, AgentUnavailableException { - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listCurrents(anyLong(), anyBoolean()); + doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findCurrent(vmId); doReturn(Date.from(Instant.now())).when(internalBackupJoinVoMock).getDate(); doReturn(Date.from(Instant.EPOCH)).when(vmSnapshotVoMock).getCreated(); doReturn(List.of()).when(vmSnapshotHelperMock).getVolumeTOList(vmId); doReturn(vmSnapshotVoMock).when(kbossBackupProviderSpy).getSucceedingVmSnapshot(internalBackupJoinVoMock); - doNothing().when(kbossBackupProviderSpy).createDeleteCommandsAndMergeTrees(any(), any(), any(), any(), anyList(), any()); + doNothing().when(kbossBackupProviderSpy).createDeleteCommandsAndMergeTrees(any(), any(), any(), any(), anyList()); doThrow(OperationTimedoutException.class).when(kbossBackupProviderSpy).sendBackupCommands(any(), any()); kbossBackupProviderSpy.prepareVmForSnapshotRevert(vmSnapshotVoMock, virtualMachineMock); @@ -1408,12 +1447,12 @@ public void prepareVmForSnapshotRevertTestCurrentBackupAfterVmSnapshotTimeout() @Test (expected = CloudRuntimeException.class) public void prepareVmForSnapshotRevertTestCurrentBackupAfterVmSnapshotNullAnswer() throws OperationTimedoutException, AgentUnavailableException { - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listCurrents(anyLong(), anyBoolean()); + doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findCurrent(vmId); doReturn(Date.from(Instant.now())).when(internalBackupJoinVoMock).getDate(); doReturn(Date.from(Instant.EPOCH)).when(vmSnapshotVoMock).getCreated(); doReturn(List.of()).when(vmSnapshotHelperMock).getVolumeTOList(vmId); doReturn(vmSnapshotVoMock).when(kbossBackupProviderSpy).getSucceedingVmSnapshot(internalBackupJoinVoMock); - doNothing().when(kbossBackupProviderSpy).createDeleteCommandsAndMergeTrees(any(), any(), any(), any(), anyList(), any()); + doNothing().when(kbossBackupProviderSpy).createDeleteCommandsAndMergeTrees(any(), any(), any(), any(), anyList()); doReturn(null).when(kbossBackupProviderSpy).sendBackupCommands(any(), any()); kbossBackupProviderSpy.prepareVmForSnapshotRevert(vmSnapshotVoMock, virtualMachineMock); @@ -1423,12 +1462,12 @@ public void prepareVmForSnapshotRevertTestCurrentBackupAfterVmSnapshotNullAnswer @Test public void prepareVmForSnapshotRevertTestCurrentBackupAfterVmSnapshotSuccess() throws OperationTimedoutException, AgentUnavailableException { - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listCurrents(anyLong(), anyBoolean()); + doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findCurrent(vmId); doReturn(Date.from(Instant.now())).when(internalBackupJoinVoMock).getDate(); doReturn(Date.from(Instant.EPOCH)).when(vmSnapshotVoMock).getCreated(); doReturn(List.of()).when(vmSnapshotHelperMock).getVolumeTOList(vmId); doReturn(vmSnapshotVoMock).when(kbossBackupProviderSpy).getSucceedingVmSnapshot(internalBackupJoinVoMock); - doNothing().when(kbossBackupProviderSpy).createDeleteCommandsAndMergeTrees(any(), any(), any(), any(), anyList(), any()); + doNothing().when(kbossBackupProviderSpy).createDeleteCommandsAndMergeTrees(any(), any(), any(), any(), anyList()); doReturn(new Answer[]{}).when(kbossBackupProviderSpy).sendBackupCommands(any(), any()); doNothing().when(kbossBackupProviderSpy).updateReferencesAfterPrepareForSnapshotRevert(any(), any(), any(), any()); @@ -1628,7 +1667,7 @@ public void endBackupChainIfConfiguredTestFeatureDisabled() { kbossBackupProviderSpy.endBackupChainIfConfigured(backupVoMock); - verify(kbossBackupProviderSpy, never()).endBackupChain(any(), any()); + verify(kbossBackupProviderSpy, never()).endBackupChain(any()); } @Test @@ -1642,7 +1681,7 @@ public void endBackupChainIfConfiguredTestNotCurrentAndNoCurrentChildren() { kbossBackupProviderSpy.endBackupChainIfConfigured(backupVoMock); - verify(kbossBackupProviderSpy, never()).endBackupChain(any(), any()); + verify(kbossBackupProviderSpy, never()).endBackupChain(any()); } @Test @@ -1652,11 +1691,11 @@ public void endBackupChainIfConfiguredTestBackupIsCurrent() { doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findById(anyLong()); doReturn(List.of()).when(kbossBackupProviderSpy).getBackupJoinChildren(any()); doReturn(userVmVOMock).when(userVmDaoMock).findById(anyLong()); - doReturn(true).when(kbossBackupProviderSpy).endBackupChain(any(), any()); + doReturn(true).when(kbossBackupProviderSpy).endBackupChain(any()); kbossBackupProviderSpy.endBackupChainIfConfigured(backupVoMock); - verify(kbossBackupProviderSpy, times(1)).endBackupChain(eq(userVmVOMock), anyLong()); + verify(kbossBackupProviderSpy, times(1)).endBackupChain(userVmVOMock); } @Test @@ -1668,11 +1707,11 @@ public void endBackupChainIfConfiguredTestLastChildIsCurrent() { doReturn(true).when(child).getCurrent(); doReturn(List.of(child)).when(kbossBackupProviderSpy).getBackupJoinChildren(any()); doReturn(userVmVOMock).when(userVmDaoMock).findById(anyLong()); - doReturn(true).when(kbossBackupProviderSpy).endBackupChain(any(), any()); + doReturn(true).when(kbossBackupProviderSpy).endBackupChain(any()); kbossBackupProviderSpy.endBackupChainIfConfigured(backupVoMock); - verify(kbossBackupProviderSpy, times(1)).endBackupChain(eq(userVmVOMock), anyLong()); + verify(kbossBackupProviderSpy, times(1)).endBackupChain(userVmVOMock); } @@ -1687,7 +1726,7 @@ public void normalizeBackupErrorAndFinishChainTestAnswerNull() { doReturn(parentId).when(internalBackupJoinVoMock).getParentId(); doReturn(null).when(internalBackupJoinDaoMock).findById(parentId); doReturn(List.of()).when(internalBackupDataStoreDaoMock).listByBackupId(anyLong()); - doNothing().when(kbossBackupProviderSpy).configureKbossTosForCleanup(any(), any(), any(), any(), any(), any(),anyBoolean()); + doNothing().when(kbossBackupProviderSpy).configureKbossTosForCleanup(any(), any(), any(), anyBoolean(), any(), any()); doReturn(null).when(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); boolean result = kbossBackupProviderSpy.normalizeBackupErrorAndFinishChain(userVmVOMock); @@ -1706,7 +1745,7 @@ public void normalizeBackupErrorAndFinishChainTestAnswerFailed() { doReturn(parentId).when(internalBackupJoinVoMock).getParentId(); doReturn(null).when(internalBackupJoinDaoMock).findById(parentId); doReturn(List.of()).when(internalBackupDataStoreDaoMock).listByBackupId(anyLong()); - doNothing().when(kbossBackupProviderSpy).configureKbossTosForCleanup(any(), any(), any(), any(), any(), any(), anyBoolean()); + doNothing().when(kbossBackupProviderSpy).configureKbossTosForCleanup(any(), any(), any(), anyBoolean(), any(), any()); doReturn(answerMock).when(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); doReturn(false).when(answerMock).getResult(); @@ -1717,8 +1756,6 @@ public void normalizeBackupErrorAndFinishChainTestAnswerFailed() { @Test public void normalizeBackupErrorAndFinishChainTestSuccessCallsEndChain() { - doReturn(userVmVOMock).when(userVmDaoMock).findById(any()); - doReturn(VirtualMachine.State.Running).when(userVmVOMock).getState(); doReturn(null).when(vmInstanceDetailsDaoMock).findDetail(anyLong(), any()); doReturn(backupVoMock).when(backupDaoMock).findLatestByStatusAndVmId(any(), anyLong()); doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findById(anyLong()); @@ -1728,23 +1765,21 @@ public void normalizeBackupErrorAndFinishChainTestSuccessCallsEndChain() { doReturn(parentId).when(internalBackupJoinVoMock).getParentId(); doReturn(null).when(internalBackupJoinDaoMock).findById(parentId); doReturn(List.of()).when(internalBackupDataStoreDaoMock).listByBackupId(anyLong()); - doNothing().when(kbossBackupProviderSpy).configureKbossTosForCleanup(any(), any(), any(), any(), any(), any(), anyBoolean()); + doNothing().when(kbossBackupProviderSpy).configureKbossTosForCleanup(any(), any(), any(), anyBoolean(), any(), any()); doReturn(answerMock).when(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); doReturn(true).when(answerMock).getResult(); - doReturn(false).when(kbossBackupProviderSpy).processCleanupBackupErrorAnswer(any(), any(), any(), any(), any()); + doReturn(false).when(kbossBackupProviderSpy).processCleanupBackupErrorAnswer(any(), any()); + doReturn(true).when(kbossBackupProviderSpy).endBackupChain(any()); boolean result = kbossBackupProviderSpy.normalizeBackupErrorAndFinishChain(userVmVOMock); assertTrue(result); - verify(kbossBackupProviderSpy).mergeCurrentBackupDeltas(internalBackupJoinVoMock); - verify(kbossBackupProviderSpy).finishBackupChains(userVmVOMock); + verify(kbossBackupProviderSpy).endBackupChain(userVmVOMock); } @Test public void normalizeBackupErrorAndFinishChainTestChainAlreadyEnded() { - doReturn(userVmVOMock).when(userVmDaoMock).findById(any()); - doReturn(VirtualMachine.State.Running).when(userVmVOMock).getState(); doReturn(null).when(vmInstanceDetailsDaoMock).findDetail(anyLong(), any()); doReturn(backupVoMock).when(backupDaoMock).findLatestByStatusAndVmId(any(), anyLong()); doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findById(anyLong()); @@ -1754,12 +1789,12 @@ public void normalizeBackupErrorAndFinishChainTestChainAlreadyEnded() { doReturn(parentId).when(internalBackupJoinVoMock).getParentId(); doReturn(null).when(internalBackupJoinDaoMock).findById(parentId); doReturn(List.of()).when(internalBackupDataStoreDaoMock).listByBackupId(anyLong()); - doNothing().when(kbossBackupProviderSpy).configureKbossTosForCleanup(any(), any(), any(), any(), any(), any(), anyBoolean()); + doNothing().when(kbossBackupProviderSpy).configureKbossTosForCleanup(any(), any(), any(), anyBoolean(), any(), any()); doReturn(answerMock).when(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); doReturn(true).when(answerMock).getResult(); - doReturn(true).when(kbossBackupProviderSpy).processCleanupBackupErrorAnswer(any(), any(), any(), any(), any()); + doReturn(true).when(kbossBackupProviderSpy).processCleanupBackupErrorAnswer(any(), any()); InternalBackupJoinVO current = mock(InternalBackupJoinVO.class); - doReturn(current).when(internalBackupJoinDaoMock).findCurrent(anyLong(), any()); + doReturn(current).when(internalBackupJoinDaoMock).findCurrent(anyLong()); doNothing().when(internalBackupStoragePoolDaoMock).expungeByBackupId(anyLong()); doNothing().when(kbossBackupProviderSpy).setEndOfChainAndRemoveCurrentForBackup(any()); @@ -1992,53 +2027,57 @@ public void deleteFailedBackupTestNonFailedBackupDoesNothing() { @Test public void mergeCurrentDeltaIntoVolumeTestNoDeltaDoesNothing() { doReturn(volumeId).when(volumeVoMock).getId(); - doReturn(List.of()).when(internalBackupJoinDaoMock).listCurrentsByVolumeIdDesc(volumeId); + doReturn(null).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(volumeId); - kbossBackupProviderSpy.mergeCurrentDeltasIntoVolume(volumeVoMock, virtualMachineMock, "detach", true); + kbossBackupProviderSpy.mergeCurrentDeltaIntoVolume(volumeVoMock, virtualMachineMock, "detach", true); - verify(internalBackupJoinDaoMock, times(1)).listCurrentsByVolumeIdDesc(volumeId); + verify(internalBackupStoragePoolDaoMock, times(1)).findOneByVolumeId(volumeId); verify(internalBackupJoinDaoMock, never()).findById(anyLong()); } @Test (expected = CloudRuntimeException.class) public void mergeCurrentDeltaIntoVolumeTestNullAnswer() { doReturn(volumeId).when(volumeVoMock).getId(); - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listCurrentsByVolumeIdDesc(volumeId); - doReturn(deltaMergeTreeToMock).when(kbossBackupProviderSpy).createDeltaMergeTree(anyBoolean(), anyBoolean(), any(), any(), any(), any()); + doReturn(internalBackupStoragePoolVoMock).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(volumeId); + doReturn(backupId).when(internalBackupStoragePoolVoMock).getBackupId(); + doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findById(backupId); + doReturn(null).when(kbossBackupProviderSpy).getSucceedingVmSnapshot(internalBackupJoinVoMock); + doReturn(deltaMergeTreeToMock).when(kbossBackupProviderSpy).createDeltaMergeTree(anyBoolean(), anyBoolean(), any(), any(), any()); doReturn(null).when(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); try (MockedStatic volumeObjectMockedStatic = Mockito.mockStatic(VolumeObject.class)) { when(VolumeObject.getVolumeObject(any(), any())).thenReturn(volumeObjectMock); - kbossBackupProviderSpy.mergeCurrentDeltasIntoVolume(volumeVoMock, virtualMachineMock, "detach", true); + kbossBackupProviderSpy.mergeCurrentDeltaIntoVolume(volumeVoMock, virtualMachineMock, "detach", true); verify(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); - verify(kbossBackupProviderSpy, never()).expungeOldDeltasAndUpdateVmSnapshotOrBackup(anyList(), any(), any()); + verify(kbossBackupProviderSpy, never()).expungeOldDeltasAndUpdateVmSnapshotIfNeeded(anyList(), any()); } } @Test public void mergeCurrentDeltaIntoVolumeTestNoSucceedingSnapshot() { doReturn(volumeId).when(volumeVoMock).getId(); - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listCurrentsByVolumeIdDesc(volumeId); + doReturn(internalBackupStoragePoolVoMock).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(volumeId); doReturn(backupId).when(internalBackupStoragePoolVoMock).getBackupId(); - doReturn(deltaMergeTreeToMock).when(kbossBackupProviderSpy).createDeltaMergeTree(anyBoolean(), anyBoolean(), any(), any(), any(), any()); + doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findById(backupId); + doReturn(null).when(kbossBackupProviderSpy).getSucceedingVmSnapshot(internalBackupJoinVoMock); + doReturn(deltaMergeTreeToMock).when(kbossBackupProviderSpy).createDeltaMergeTree(anyBoolean(), anyBoolean(), any(), any(), any()); doReturn(answerMock).when(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); doReturn(true).when(answerMock).getResult(); doReturn(volumeVoMock).when(volumeDaoMock).findById(anyLong()); doReturn(backupDeltaToMock).when(deltaMergeTreeToMock).getParent(); - doNothing().when(kbossBackupProviderSpy).expungeOldDeltasAndUpdateVmSnapshotOrBackup(anyList(), any(), any()); + doNothing().when(kbossBackupProviderSpy).expungeOldDeltasAndUpdateVmSnapshotIfNeeded(anyList(), any()); doReturn(List.of()).when(internalBackupStoragePoolDaoMock).listByBackupId(backupId); doNothing().when(kbossBackupProviderSpy).setEndOfChainAndRemoveCurrentForBackup(any()); - doReturn(internalBackupStoragePoolVoMock).when(internalBackupStoragePoolDaoMock).findOneByVolumeIdAndBackupId(anyLong(), anyLong()); try (MockedStatic volumeObjectMockedStatic = Mockito.mockStatic(VolumeObject.class)) { when(VolumeObject.getVolumeObject(any(), any())).thenReturn(volumeObjectMock); - kbossBackupProviderSpy.mergeCurrentDeltasIntoVolume(volumeVoMock, virtualMachineMock, "detach", true); + kbossBackupProviderSpy.mergeCurrentDeltaIntoVolume(volumeVoMock, virtualMachineMock, "detach", true); verify(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); verify(volumeDaoMock).update(volumeId, volumeVoMock); - verify(kbossBackupProviderSpy).expungeOldDeltasAndUpdateVmSnapshotOrBackup(anyList(), any(), any()); + verify(kbossBackupProviderSpy).expungeOldDeltasAndUpdateVmSnapshotIfNeeded(anyList(), any()); verify(kbossBackupProviderSpy).setEndOfChainAndRemoveCurrentForBackup(any()); } } @@ -2046,24 +2085,24 @@ public void mergeCurrentDeltaIntoVolumeTestNoSucceedingSnapshot() { @Test public void mergeCurrentDeltaIntoVolumeTestWithSucceedingSnapshotWithMoreDeltas() { doReturn(volumeId).when(volumeVoMock).getId(); - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listCurrentsByVolumeIdDesc(volumeId); + doReturn(internalBackupStoragePoolVoMock).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(volumeId); doReturn(backupId).when(internalBackupStoragePoolVoMock).getBackupId(); - doReturn(deltaMergeTreeToMock).when(kbossBackupProviderSpy).createDeltaMergeTree(anyBoolean(), anyBoolean(), any(), any(), any(), any()); - doReturn(backupDeltaToMock).when(deltaMergeTreeToMock).getParent(); + doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findById(backupId); + doReturn(vmSnapshotVoMock).when(kbossBackupProviderSpy).getSucceedingVmSnapshot(internalBackupJoinVoMock); + doReturn(deltaMergeTreeToMock).when(kbossBackupProviderSpy).createDeltaMergeTree(anyBoolean(), anyBoolean(), any(), any(), any()); doReturn(answerMock).when(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); doReturn(true).when(answerMock).getResult(); - doNothing().when(kbossBackupProviderSpy).expungeOldDeltasAndUpdateVmSnapshotOrBackup(anyList(), any(), any()); + doNothing().when(kbossBackupProviderSpy).expungeOldDeltasAndUpdateVmSnapshotIfNeeded(anyList(), any()); doReturn(List.of(internalBackupStoragePoolVoMock)).when(internalBackupStoragePoolDaoMock).listByBackupId(backupId); - doReturn(volumeVoMock).when(volumeDaoMock).findById(anyLong()); - doReturn(internalBackupStoragePoolVoMock).when(internalBackupStoragePoolDaoMock).findOneByVolumeIdAndBackupId(anyLong(), anyLong()); try (MockedStatic volumeObjectMockedStatic = Mockito.mockStatic(VolumeObject.class)) { when(VolumeObject.getVolumeObject(any(), any())).thenReturn(volumeObjectMock); - kbossBackupProviderSpy.mergeCurrentDeltasIntoVolume(volumeVoMock, virtualMachineMock, "detach", true); + kbossBackupProviderSpy.mergeCurrentDeltaIntoVolume(volumeVoMock, virtualMachineMock, "detach", true); verify(kbossBackupProviderSpy).sendBackupCommand(anyLong(), any()); - verify(kbossBackupProviderSpy).expungeOldDeltasAndUpdateVmSnapshotOrBackup(anyList(), any(), any()); + verify(volumeDaoMock, never()).update(volumeId, volumeVoMock); + verify(kbossBackupProviderSpy).expungeOldDeltasAndUpdateVmSnapshotIfNeeded(anyList(), any()); verify(kbossBackupProviderSpy, never()).setEndOfChainAndRemoveCurrentForBackup(any()); } } @@ -2143,13 +2182,90 @@ public void getHostToRestoreTestQuickRestoreWithHostDisabledThrows() throws Agen kbossBackupProviderSpy.getHostToRestore(virtualMachineMock, true, 55L); } + @Test + public void gatherSnapshotReferencesOfChildrenSnapshotTestVmSnapshotIsNull() { + List volumeObjectTOs = List.of(volumeObjectToMock); + + Map> result = kbossBackupProviderSpy.gatherSnapshotReferencesOfChildrenSnapshot(volumeObjectTOs, null); + + assertTrue(result.isEmpty()); + verify(vmSnapshotDaoMock, never()).listByParent(anyLong()); + verify(vmSnapshotHelperMock, never()).getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(anyLong()); + verify(kbossBackupProviderSpy, never()).mapVolumesToSnapshotReferences(anyList(), anyList(), anyMap()); + } + + @Test + public void gatherSnapshotReferencesOfChildrenSnapshotTestChildrenListIsEmpty() { + doReturn(100L).when(vmSnapshotVoMock).getId(); + doReturn(List.of()).when(vmSnapshotDaoMock).listByParent(100L); + + List volumeObjectTOs = List.of(volumeObjectToMock); + + Map> result = kbossBackupProviderSpy.gatherSnapshotReferencesOfChildrenSnapshot(volumeObjectTOs, vmSnapshotVoMock); + + assertTrue(result.isEmpty()); + verify(vmSnapshotDaoMock, times(1)).listByParent(100L); + verify(vmSnapshotHelperMock, never()).getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(anyLong()); + verify(kbossBackupProviderSpy, never()).mapVolumesToSnapshotReferences(anyList(), anyList(), anyMap()); + } + + @Test + public void gatherSnapshotReferencesOfChildrenSnapshotTestSingleChildWithSingleSnapshotReference() { + VMSnapshotVO childSnapshot = Mockito.mock(VMSnapshotVO.class); + SnapshotDataStoreVO snapshotDataStoreVO = Mockito.mock(SnapshotDataStoreVO.class); + + doReturn(100L).when(vmSnapshotVoMock).getId(); + doReturn(List.of(childSnapshot)).when(vmSnapshotDaoMock).listByParent(100L); + doReturn(200L).when(childSnapshot).getId(); + doReturn(List.of(snapshotDataStoreVO)).when(vmSnapshotHelperMock).getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(200L); + doNothing().when(kbossBackupProviderSpy).mapVolumesToSnapshotReferences(anyList(), anyList(), anyMap()); + + List volumeObjectTOs = List.of(volumeObjectToMock); + + Map> result = + kbossBackupProviderSpy.gatherSnapshotReferencesOfChildrenSnapshot(volumeObjectTOs, vmSnapshotVoMock); + + assertTrue(result.isEmpty()); + verify(vmSnapshotDaoMock, times(1)).listByParent(100L); + verify(vmSnapshotHelperMock, times(1)).getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(200L); + verify(kbossBackupProviderSpy, times(1)).mapVolumesToSnapshotReferences(eq(volumeObjectTOs), anyList(), anyMap()); + } + + @Test + public void gatherSnapshotReferencesOfChildrenSnapshotTestMultipleChildrenAggregatesSnapshotReferences() { + VMSnapshotVO childSnapshot1 = Mockito.mock(VMSnapshotVO.class); + VMSnapshotVO childSnapshot2 = Mockito.mock(VMSnapshotVO.class); + SnapshotDataStoreVO snapshotDataStoreVO1 = Mockito.mock(SnapshotDataStoreVO.class); + SnapshotDataStoreVO snapshotDataStoreVO2 = Mockito.mock(SnapshotDataStoreVO.class); + + doReturn(100L).when(vmSnapshotVoMock).getId(); + doReturn(List.of(childSnapshot1, childSnapshot2)).when(vmSnapshotDaoMock).listByParent(100L); + doReturn(201L).when(childSnapshot1).getId(); + doReturn(202L).when(childSnapshot2).getId(); + doReturn(List.of(snapshotDataStoreVO1)).when(vmSnapshotHelperMock) + .getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(201L); + doReturn(List.of(snapshotDataStoreVO2)).when(vmSnapshotHelperMock) + .getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(202L); + doNothing().when(kbossBackupProviderSpy).mapVolumesToSnapshotReferences(anyList(), anyList(), anyMap()); + + List volumeObjectTOs = List.of(volumeObjectToMock); + + Map> result = + kbossBackupProviderSpy.gatherSnapshotReferencesOfChildrenSnapshot(volumeObjectTOs, vmSnapshotVoMock); + + assertTrue(result.isEmpty()); + verify(vmSnapshotHelperMock, times(1)).getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(201L); + verify(vmSnapshotHelperMock, times(1)).getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(202L); + verify(kbossBackupProviderSpy, times(1)).mapVolumesToSnapshotReferences(eq(volumeObjectTOs), anyList(), anyMap()); + } + @Test public void createDeltaMergeTreeTestChildIsVolumeWithoutSucceedingSnapshot() { doReturn(dataStoreMock).when(dataStoreManagerMock).getDataStore(anyLong(), eq(DataStoreRole.Primary)); doReturn("parent-path").when(internalBackupStoragePoolVoMock).getBackupDeltaParentPath(); DeltaMergeTreeTO result = kbossBackupProviderSpy.createDeltaMergeTree(true, true, internalBackupStoragePoolVoMock, - volumeObjectToMock, null, null); + volumeObjectToMock, null); assertEquals(volumeObjectToMock, result.getVolumeObjectTO()); assertTrue(result.getGrandChildren().isEmpty()); @@ -2164,7 +2280,7 @@ public void createDeltaMergeTreeTestChildIsDeltaWithoutSucceedingSnapshot() { doReturn("child-path").when(internalBackupStoragePoolVoMock).getBackupDeltaPath(); DeltaMergeTreeTO result = kbossBackupProviderSpy.createDeltaMergeTree(false, true, internalBackupStoragePoolVoMock, - volumeObjectToMock, null, null); + volumeObjectToMock, null); assertEquals(volumeObjectToMock, result.getVolumeObjectTO()); assertEquals("parent-path", result.getParent().getPath()); @@ -2180,14 +2296,16 @@ public void createDeltaMergeTreeTestChildIsDeltaWithSucceedingSnapshotReferences doReturn("parent-path").when(internalBackupStoragePoolVoMock).getBackupDeltaParentPath(); doReturn("child-path").when(internalBackupStoragePoolVoMock).getBackupDeltaPath(); doReturn(volumeId).when(volumeObjectToMock).getVolumeId(); - doReturn("path").when(volumeObjectToMock).getPath(); + doReturn("snapshot-grandchild").when(snapshotRefMock).getInstallPath(); - DeltaMergeTreeTO result = kbossBackupProviderSpy.createDeltaMergeTree(false, false, internalBackupStoragePoolVoMock, - volumeObjectToMock, vmSnapshotVoMock, List.of()); + doReturn(Map.of(volumeId, List.of(snapshotRefMock))).when(kbossBackupProviderSpy).gatherSnapshotReferencesOfChildrenSnapshot(List.of(volumeObjectToMock), vmSnapshotVoMock); + + DeltaMergeTreeTO result = kbossBackupProviderSpy.createDeltaMergeTree(false, true, internalBackupStoragePoolVoMock, + volumeObjectToMock, vmSnapshotVoMock); assertEquals("child-path", result.getChild().getPath()); assertEquals(1, result.getGrandChildren().size()); - assertEquals("path", result.getGrandChildren().get(0).getPath()); + assertEquals("snapshot-grandchild", result.getGrandChildren().get(0).getPath()); } @Test @@ -2197,8 +2315,11 @@ public void createDeltaMergeTreeTestChildIsDeltaWithSucceedingSnapshotButNoRefer doReturn("child-path").when(internalBackupStoragePoolVoMock).getBackupDeltaPath(); doReturn("/volume/path").when(volumeObjectToMock).getPath(); + doReturn(Map.of()).when(kbossBackupProviderSpy) + .gatherSnapshotReferencesOfChildrenSnapshot(List.of(volumeObjectToMock), vmSnapshotVoMock); + DeltaMergeTreeTO result = kbossBackupProviderSpy.createDeltaMergeTree(false, false, internalBackupStoragePoolVoMock, - volumeObjectToMock, vmSnapshotVoMock, List.of()); + volumeObjectToMock, vmSnapshotVoMock); assertEquals(1, result.getGrandChildren().size()); assertEquals("/volume/path", result.getGrandChildren().get(0).getPath()); @@ -2275,16 +2396,14 @@ public void populateDeltasToRemoveAndToMergeAndUpdateVolumePathsTestVolumeIsPart Set deltasToRemove = new java.util.HashSet<>(); - doReturn(deltaMergeTreeToMock).when(kbossBackupProviderSpy).createDeltaMergeTree(eq(true), eq(false), eq(internalBackupStoragePoolVoMock), eq(volumeObjectToMock), eq(null), - eq(new ArrayList<>())); + doReturn(deltaMergeTreeToMock).when(kbossBackupProviderSpy).createDeltaMergeTree(true, false, internalBackupStoragePoolVoMock, volumeObjectToMock, null); List result = kbossBackupProviderSpy.populateDeltasToRemoveAndToMergeAndUpdateVolumePaths(List.of(internalBackupStoragePoolVoMock), deltasToRemove, List.of(volumeObjectToMock), List.of(volumeObjectToMock), "vm-uuid"); assertEquals(List.of(deltaMergeTreeToMock), result); assertTrue(deltasToRemove.isEmpty()); - verify(kbossBackupProviderSpy, times(1)).createDeltaMergeTree(eq(true), eq(false), eq(internalBackupStoragePoolVoMock), eq(volumeObjectToMock), eq(null), - eq(new ArrayList<>())); + verify(kbossBackupProviderSpy, times(1)).createDeltaMergeTree(true, false, internalBackupStoragePoolVoMock, volumeObjectToMock, null); verify(dataStoreManagerMock, never()).getDataStore(anyLong(), eq(DataStoreRole.Primary)); } @@ -2384,7 +2503,7 @@ public void processRemoveBackupFailuresTestNoFailuresReturnsTrueAndRemovesNothin List removedBackupIds = new ArrayList<>(List.of(backupId, 200L)); - boolean result = kbossBackupProviderSpy.processRemoveBackupFailures(false, deleteAnswers, removedBackupIds, internalBackupJoinVoMock, virtualMachineMock); + boolean result = kbossBackupProviderSpy.processRemoveBackupFailures(false, deleteAnswers, removedBackupIds, internalBackupJoinVoMock); assertTrue(result); assertEquals(List.of(backupId, 200L), removedBackupIds); @@ -2403,11 +2522,10 @@ public void processRemoveBackupFailuresTestFailureOnCurrentBackupNotForcedSetsEr doReturn(backupId).when(backupVoMock).getId(); doReturn(backupVoMock).when(backupDaoMock).findByIdIncludingRemoved(backupId); - doReturn(VirtualMachine.State.Stopped).when(virtualMachineMock).getState(); List removedBackupIds = new ArrayList<>(List.of(backupId, 200L)); - boolean result = kbossBackupProviderSpy.processRemoveBackupFailures(false, new Answer[]{failedCurrentBackupAnswer}, removedBackupIds, internalBackupJoinVoMock, virtualMachineMock); + boolean result = kbossBackupProviderSpy.processRemoveBackupFailures(false, new Answer[]{failedCurrentBackupAnswer}, removedBackupIds, internalBackupJoinVoMock); assertFalse(result); assertEquals(List.of(200L), removedBackupIds); @@ -2426,7 +2544,7 @@ public void processRemoveBackupFailuresTestFailureOnCurrentBackupForcedSetBackup List removedBackupIds = new ArrayList<>(List.of(backupId, 200L)); - boolean result = kbossBackupProviderSpy.processRemoveBackupFailures(true, new Answer[]{failedCurrentBackupAnswer}, removedBackupIds, internalBackupJoinVoMock, virtualMachineMock); + boolean result = kbossBackupProviderSpy.processRemoveBackupFailures(true, new Answer[]{failedCurrentBackupAnswer}, removedBackupIds, internalBackupJoinVoMock); assertFalse(result); assertEquals(List.of(200L), removedBackupIds); @@ -2449,7 +2567,7 @@ public void processRemoveBackupFailuresTestFailureOnOtherBackupMarksItExpunged() List removedBackupIds = new ArrayList<>(List.of(backupId, 200L)); - boolean result = kbossBackupProviderSpy.processRemoveBackupFailures(false, new Answer[]{failedOtherBackupAnswer}, removedBackupIds, internalBackupJoinVoMock, virtualMachineMock); + boolean result = kbossBackupProviderSpy.processRemoveBackupFailures(false, new Answer[]{failedOtherBackupAnswer}, removedBackupIds, internalBackupJoinVoMock); assertFalse(result); assertEquals(List.of(backupId), removedBackupIds); diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java index d73efc51be1a..8e6d33c4e668 100644 --- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java +++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java @@ -550,7 +550,7 @@ protected Host getVMHypervisorHostForBackup(VirtualMachine vm) { } @Override - public Pair takeBackup(final VirtualMachine vm, Boolean quiesceVM, boolean isolated, Long scheduleId) { + public Pair takeBackup(final VirtualMachine vm, Boolean quiesceVM, boolean isolated) { final Host host = getVMHypervisorHostForBackup(vm); final BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId()); diff --git a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java index 09eb877e0ab1..f1d5613ab7fc 100644 --- a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java +++ b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java @@ -251,7 +251,7 @@ public void takeBackupSuccessfully() throws AgentUnavailableException, Operation Mockito.when(backupDao.persist(Mockito.any(BackupVO.class))).thenAnswer(invocation -> invocation.getArgument(0)); Mockito.when(backupDao.update(Mockito.anyLong(), Mockito.any(BackupVO.class))).thenReturn(true); - Pair result = nasBackupProvider.takeBackup(vm, false, false, null); + Pair result = nasBackupProvider.takeBackup(vm, false, false); Assert.assertTrue(result.first()); Assert.assertNotNull(result.second()); diff --git a/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java b/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java index 31186385d578..1cf962edae51 100644 --- a/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java +++ b/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java @@ -492,7 +492,7 @@ public Pair restoreBackedUpVolume(Backup backup, Backup.VolumeI } @Override - public Pair takeBackup(VirtualMachine vm, Boolean quiesceVM, boolean isolated, Long scheduleId) { + public Pair takeBackup(VirtualMachine vm, Boolean quiesceVM, boolean isolated) { String networkerServer; String clusterName; diff --git a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java index 361b3349b011..9b34af2d6f49 100644 --- a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java +++ b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java @@ -219,7 +219,7 @@ public boolean willDeleteBackupsOnOfferingRemoval() { } @Override - public Pair takeBackup(final VirtualMachine vm, Boolean quiesceVM, boolean isolated, Long scheduleId) { + public Pair takeBackup(final VirtualMachine vm, Boolean quiesceVM, boolean isolated) { final VeeamClient client = getClient(vm.getDataCenterId()); Boolean result = client.startBackupJob(vm.getBackupExternalId()); return new Pair<>(result, null); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupKbossVmBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupKbossVmBackupCommandWrapper.java index 8ca17fc0c6cf..430d1c6ea3c2 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupKbossVmBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupKbossVmBackupCommandWrapper.java @@ -16,16 +16,17 @@ // under the License. package com.cloud.hypervisor.kvm.resource.wrapper; -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - +import com.cloud.agent.api.Answer; +import com.cloud.hypervisor.Hypervisor; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser; +import com.cloud.hypervisor.kvm.resource.LibvirtVMDef; +import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk; +import com.cloud.hypervisor.kvm.storage.KVMStoragePool; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; +import com.cloud.utils.Pair; import org.apache.cloudstack.backup.CleanupKbossBackupErrorAnswer; import org.apache.cloudstack.backup.CleanupKbossBackupErrorCommand; import org.apache.cloudstack.storage.to.BackupDeltaTO; @@ -39,19 +40,12 @@ import org.libvirt.Error; import org.libvirt.LibvirtException; -import com.cloud.agent.api.Answer; -import com.cloud.agent.api.to.DataTO; -import com.cloud.hypervisor.Hypervisor; -import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; -import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser; -import com.cloud.hypervisor.kvm.resource.LibvirtVMDef; -import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk; -import com.cloud.hypervisor.kvm.storage.KVMStoragePool; -import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; -import com.cloud.resource.CommandWrapper; -import com.cloud.resource.ResourceWrapper; -import com.cloud.utils.Pair; -import com.cloud.utils.exception.CloudRuntimeException; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; @ResourceWrapper(handles = CleanupKbossBackupErrorCommand.class) public class LibvirtCleanupKbossVmBackupCommandWrapper extends CommandWrapper { @@ -64,14 +58,14 @@ public Answer execute(CleanupKbossBackupErrorCommand command, LibvirtComputingRe cleanupBackupDeltasOnSecondary(command, storagePoolManager, kbossTOS); if (command.isRunningVM()) { - Pair>, Boolean> volumeTosAndIsVmRunning = cleanupRunningVm(command, serverResource); + Pair, Boolean> volumeTosAndIsVmRunning = cleanupRunningVm(command, serverResource); return new CleanupKbossBackupErrorAnswer(command, volumeTosAndIsVmRunning.first(), volumeTosAndIsVmRunning.second()); } return new CleanupKbossBackupErrorAnswer(command, mergeDeltasForStoppedVmIfNeeded(command, serverResource), false); } - private Pair>, Boolean> cleanupRunningVm(CleanupKbossBackupErrorCommand command, LibvirtComputingResource serverResource) { + private Pair, Boolean> cleanupRunningVm(CleanupKbossBackupErrorCommand command, LibvirtComputingResource serverResource) { Domain dm = null; try { dm = serverResource.getDomain(serverResource.getLibvirtUtilitiesHelper().getConnection(), command.getVmName()); @@ -81,7 +75,7 @@ private Pair>, Boolean> cleanupRunningVm(Cleanu return new Pair<>(mergeDeltasForStoppedVmIfNeeded(command, serverResource), false); } logger.error("Error while trying to get VM [{}]. Aborting the process.", command.getVmName(), e); - return new Pair<>(Map.of(), false); + return new Pair<>(List.of(), false); } finally { if (dm != null) { try { @@ -93,140 +87,87 @@ private Pair>, Boolean> cleanupRunningVm(Cleanu } } - private Map> mergeDeltasForStoppedVmIfNeeded(CleanupKbossBackupErrorCommand command, LibvirtComputingResource serverResource) { - HashMap> volumeToChainEnded = new HashMap<>(); + private List mergeDeltasForStoppedVmIfNeeded(CleanupKbossBackupErrorCommand command, LibvirtComputingResource serverResource) { + List volumeObjectTOList = new ArrayList<>(); for (KbossTO kbossTO : command.getKbossTOs()) { VolumeObjectTO volumeObjectTO = kbossTO.getVolumeObjectTO(); PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO)volumeObjectTO.getDataStore(); KVMStoragePool kvmStoragePool = serverResource.getStoragePoolMgr().getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid()); - boolean volumePathMissing = !Files.exists(Path.of(kvmStoragePool.getLocalPathFor(volumeObjectTO.getPath()))); - boolean deltaPathMissing = !Files.exists(Path.of(kvmStoragePool.getLocalPathFor(kbossTO.getDeltaPathOnPrimary()))); - boolean basePathMissing = kbossTO.getParentDeltaPathOnPrimary() != null && !Files.exists(Path.of(kvmStoragePool.getLocalPathFor(kbossTO.getParentDeltaPathOnPrimary()))); - List grandchildren = kbossTO.getDeltaPaths().isEmpty() ? List.of() : List.of(new BackupDeltaTO(volumeObjectTO.getDataStore(), - Hypervisor.HypervisorType.KVM, kbossTO.getDeltaPaths().get(0))); - - Boolean chainEnded = mergeDeltaIfNeeded(serverResource, kbossTO, volumeObjectTO, grandchildren, volumePathMissing, deltaPathMissing, basePathMissing, - command.isErrorOnCreate(), false, command.isTopDelta(), command.isEndOfChain()); - volumeToChainEnded.put(volumeObjectTO.getUuid(), new Pair<>(volumeObjectTO.getPath(), chainEnded)); + boolean backupErrorDeltaExists = Files.exists(Path.of(kvmStoragePool.getLocalPathFor(volumeObjectTO.getPath()))); + boolean parentBackupDeltaExists = Files.exists(Path.of(kvmStoragePool.getLocalPathFor(kbossTO.getDeltaPathOnPrimary()))); + boolean shouldBaseDeltaExist = kbossTO.getParentDeltaPathOnPrimary() != null; + boolean baseDeltaExists = shouldBaseDeltaExist && Files.exists(Path.of(kvmStoragePool.getLocalPathFor(kbossTO.getParentDeltaPathOnPrimary()))); + + if(!mergeDeltaIfNeeded(serverResource, kbossTO, backupErrorDeltaExists, parentBackupDeltaExists, shouldBaseDeltaExist, baseDeltaExists, + false, volumeObjectTO, volumeObjectTOList)) { + return List.of(); + } } - return volumeToChainEnded; + return volumeObjectTOList; } - private Map> mergeDeltasForRunningVmIfNeeded(CleanupKbossBackupErrorCommand command, LibvirtComputingResource serverResource, Domain dm) throws LibvirtException { - HashMap> volumeIdToPathAndChainEnded = new HashMap<>(); + private List mergeDeltasForRunningVmIfNeeded(CleanupKbossBackupErrorCommand command, LibvirtComputingResource serverResource, Domain dm) throws LibvirtException { String xmlDesc = dm.getXMLDesc(0); LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser(); parser.parseDomainXML(xmlDesc); + List volumeObjectTOList = new ArrayList<>(); for (KbossTO kbossTO : command.getKbossTOs()) { - VolumeObjectTO volumeObjectTO = kbossTO.getVolumeObjectTO(); - String volumePath = volumeObjectTO.getPath(); LibvirtVMDef.DiskDef diskDef = parser.getDisks().stream() - .filter(disk -> hasPath(disk, volumePath, kbossTO.getDeltaPathOnPrimary(), kbossTO.getParentDeltaPathOnPrimary())) - .findFirst().orElse(null); + .filter(disk -> StringUtils.contains(disk.getDiskPath(), kbossTO.getVolumeObjectTO().getPath()) || + StringUtils.contains(disk.getDiskPath(), kbossTO.getDeltaPathOnPrimary()) || + StringUtils.contains(disk.getDiskPath(), kbossTO.getParentDeltaPathOnPrimary())).findFirst().orElse(null); if (diskDef == null) { - logger.warn("Volume [{}] does not match any record we have. This must be manually normalized.", volumeObjectTO.getUuid()); - return Map.of(); + logger.warn("Volume [{}] does not match any record we have. This must be manually normalized.", kbossTO.getVolumeObjectTO().getUuid()); + return List.of(); } - List backingStoreList = diskDef.getBackingStoreList(); - backingStoreList.add(0, diskDef.getDiskPath()); - - boolean volumePathMissing = true; - boolean deltaPathMissing = true; - boolean basePathMissing = kbossTO.getParentDeltaPathOnPrimary() != null; - for (String delta : backingStoreList) { - if (StringUtils.contains(delta, volumePath)) { - volumePathMissing = false; - } - if (StringUtils.contains(delta, kbossTO.getDeltaPathOnPrimary())) { - deltaPathMissing = false; - } - if (StringUtils.contains(delta, kbossTO.getParentDeltaPathOnPrimary())) { - basePathMissing = false; - } - } + boolean backupErrorDeltaExists = diskDef.getDiskPath().contains(kbossTO.getVolumeObjectTO().getPath()); + boolean parentBackupDeltaExists = diskDef.getDiskPath().contains(kbossTO.getDeltaPathOnPrimary()) || + diskDef.getBackingStoreList().stream().anyMatch(path -> path.contains(kbossTO.getDeltaPathOnPrimary())); + boolean shouldBaseDeltaExist = kbossTO.getParentDeltaPathOnPrimary() != null; + boolean baseDeltaExists = shouldBaseDeltaExist && StringUtils.contains(diskDef.getDiskPath(), kbossTO.getParentDeltaPathOnPrimary()) || + diskDef.getBackingStoreList().stream().anyMatch(path -> StringUtils.contains(path, kbossTO.getParentDeltaPathOnPrimary())); - Boolean chainEnded = mergeDeltaIfNeeded(serverResource, kbossTO, volumeObjectTO, List.of(), volumePathMissing, deltaPathMissing, basePathMissing, - command.isErrorOnCreate(), true, command.isTopDelta(), command.isEndOfChain()); - volumeIdToPathAndChainEnded.put(volumeObjectTO.getUuid(), new Pair<>(volumeObjectTO.getPath(), chainEnded)); + mergeDeltaIfNeeded(serverResource, kbossTO, backupErrorDeltaExists, parentBackupDeltaExists, shouldBaseDeltaExist, baseDeltaExists, true, + kbossTO.getVolumeObjectTO(), volumeObjectTOList); } - return volumeIdToPathAndChainEnded; + return volumeObjectTOList; } - private boolean hasPath(LibvirtVMDef.DiskDef diskDef, String... paths) { - List chain = diskDef.getBackingStoreList(); - chain = chain != null ? chain : new ArrayList<>(); - chain.add(diskDef.getDiskPath()); - for (String delta : chain) { - if (Arrays.stream(paths).anyMatch(path -> StringUtils.contains(delta, path))) { + private boolean mergeDeltaIfNeeded(LibvirtComputingResource serverResource, KbossTO kbossTO, boolean backupErrorDeltaExists, + boolean parentBackupDeltaExists, boolean shouldBaseDeltaExist, boolean baseDeltaExists, boolean runningVm, VolumeObjectTO volumeObjectTO, + List volumeObjectTOList) { + DeltaMergeTreeTO deltaMergeTreeTO; + if (!backupErrorDeltaExists) { + if (parentBackupDeltaExists && (!shouldBaseDeltaExist || baseDeltaExists)) { + volumeObjectTO.setPath(kbossTO.getDeltaPathOnPrimary()); + logger.debug("Volume [{}] is already consistent. Its path is [{}].", volumeObjectTO.getUuid(), volumeObjectTO.getPath()); + volumeObjectTOList.add(volumeObjectTO); return true; - } - } - return false; - } - - /** - * @return True if error chain is already ended, false otherwise. - * */ - private boolean mergeDeltaIfNeeded(LibvirtComputingResource serverResource, KbossTO kbossTO, VolumeObjectTO volumeObjectTO, List grandChildren, - boolean volumePathMissing, boolean deltaPathMissing, boolean basePathMissing, boolean errorOnCreate, boolean runningVm, boolean isTopDelta, boolean isEndOfChain) { - String errorMessage = String.format("Volume [%s] is inconsistent in an anomalous way. We cannot normalize it automatically.", volumeObjectTO.getUuid()); - if (!errorOnCreate) { - // Base should never be missing if it is not an error from creation. If the volume path is missing and it is not the delta that was being removed, it is an anomaly as well. - if (basePathMissing || (volumePathMissing && !isTopDelta)) { - logger.warn(errorMessage); - throw new CloudRuntimeException(String.format ("Unable to find the base delta or the volume path was not found. We cannot normalize it automatically. At least " + - "one of these should exist: volume [%s]; base path [%s].", volumeObjectTO.getPath(), kbossTO.getParentDeltaPathOnPrimary())); - } - // This means that the delta merge likely succeeded but the host was unable to reply to the Management Server - if (deltaPathMissing) { - // This is if the delta being merged was the top delta. Then we must update its path. - if (volumePathMissing) { - volumeObjectTO.setPath(kbossTO.getParentDeltaPathOnPrimary()); - } + } else if (baseDeltaExists) { + volumeObjectTO.setPath(kbossTO.getParentDeltaPathOnPrimary()); logger.debug("Volume [{}] is already consistent. Its path is [{}].", volumeObjectTO.getUuid(), volumeObjectTO.getPath()); + volumeObjectTOList.add(volumeObjectTO); return true; + } else { + logger.warn("Volume [{}] is inconsistent in an anomalous way. We cannot normalize it automatically.", volumeObjectTO.getUuid()); + return false; } - return false; - } - - DeltaMergeTreeTO deltaMergeTreeTO; - boolean errorChainFinished; - if (volumePathMissing && !deltaPathMissing) { // The process was not started for this volume - DataTO child; - // If it is the top delta, we should set the volume path as the delta path on primary, as it is the real path. This will get updated later after being merged. - if (isTopDelta) { - volumeObjectTO.setPath(kbossTO.getDeltaPathOnPrimary()); - child = volumeObjectTO; - } else { // Otherwise, we set it as the old path of the volume. In this case, this will be its final path. - volumeObjectTO.setPath(kbossTO.getOldVolumePath()); - child = new BackupDeltaTO(volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, kbossTO.getDeltaPathOnPrimary()); - } - logger.debug("Volume [{}] is consistent, the backup process for it was not started. Its current path is [{}]. We will merge the old backup chain.", - volumeObjectTO.getUuid(), volumeObjectTO.getPath()); + } else if (parentBackupDeltaExists) { + logger.debug("Volume [{}] is inconsistent, but we can normalize it. We will merge the delta created by this backup with the delta created by the previous " + + "backup.", volumeObjectTO.getUuid()); deltaMergeTreeTO = new DeltaMergeTreeTO(volumeObjectTO, new BackupDeltaTO(volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, - kbossTO.getParentDeltaPathOnPrimary()), child, grandChildren); - errorChainFinished = true; - } else if (!isEndOfChain && !volumePathMissing && (deltaPathMissing || kbossTO.getParentDeltaPathOnPrimary() == null)) { // The process was completed for this volume - logger.debug("Volume [{}] is consistent, the backup process was completed for it. Its current path is [{}].", volumeObjectTO.getUuid(), volumeObjectTO.getPath()); - return false; - } else if (isEndOfChain && volumePathMissing && !basePathMissing) { // The process was completed for this volume - volumeObjectTO.setPath(kbossTO.getParentDeltaPathOnPrimary()); - logger.debug("Volume [{}] is consistent, the backup process was completed for it. Its current path is [{}].", volumeObjectTO.getUuid(), volumeObjectTO.getPath()); - return true; - } else if (!volumePathMissing && !deltaPathMissing) { // The process stopped midway - logger.debug("Volume [{}] is inconsistent, but we can normalize it. We will merge the delta created by the last backup with the base volume.", + kbossTO.getDeltaPathOnPrimary()), volumeObjectTO, List.of()); + } else if (baseDeltaExists) { + logger.debug("Volume [{}] is inconsistent, but we can normalize it. We will merge the delta created by this backup with the base volume.", volumeObjectTO.getUuid()); deltaMergeTreeTO = new DeltaMergeTreeTO(volumeObjectTO, new BackupDeltaTO(volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, - kbossTO.getParentDeltaPathOnPrimary()), new BackupDeltaTO(volumeObjectTO.getDataStore(), Hypervisor.HypervisorType.KVM, kbossTO.getDeltaPathOnPrimary()), - grandChildren); - errorChainFinished = false; - isTopDelta = false; + kbossTO.getParentDeltaPathOnPrimary()), volumeObjectTO, List.of()); } else { - logger.warn(errorMessage); - throw new CloudRuntimeException(errorMessage + " Maybe it is a good idea to open an issue to get help on this."); + logger.warn("Volume [{}] is inconsistent in an anomalous way. We cannot normalize it automatically.", volumeObjectTO.getUuid()); + return false; } try { @@ -235,19 +176,15 @@ private boolean mergeDeltaIfNeeded(LibvirtComputingResource serverResource, Kbos } else { serverResource.mergeDeltaForStoppedVm(deltaMergeTreeTO); } - if (isTopDelta) { - volumeObjectTO.setPath(deltaMergeTreeTO.getParent().getPath()); - } - return errorChainFinished; + volumeObjectTO.setPath(deltaMergeTreeTO.getParent().getPath()); + volumeObjectTOList.add(volumeObjectTO); + return true; } catch (QemuImgException | IOException | LibvirtException ex) { logger.error("Got an exception while trying to merge delta for volume [{}].", volumeObjectTO.getUuid(), ex); - throw new CloudRuntimeException(ex); + return false; } } - /** - * Checks if the VM is really stopped by checking if its root volume has had any writes on the last 30 seconds. - * */ private boolean isVmReallyStopped(CleanupKbossBackupErrorCommand command, LibvirtComputingResource serverResource) { VolumeObjectTO volume = command.getKbossTOs().stream() .filter(kbossTO -> kbossTO.getVolumeObjectTO().getDeviceId() == 0) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java index 865d2bfb1e50..507744fdc316 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java @@ -57,7 +57,6 @@ import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; -import org.apache.commons.collections4.CollectionUtils; import org.libvirt.LibvirtException; import static com.cloud.hypervisor.kvm.storage.KVMStorageProcessor.poolTypesToDeleteChainInfo; @@ -182,12 +181,10 @@ protected void revertVolumeToSnapshot(KVMStoragePool kvmStoragePoolSecondary, Sn try { replaceVolumeWithSnapshot(volumePath, snapshotPath); - if (CollectionUtils.isNotEmpty(volumeObjectTo.getDeltasToRemove()) && poolTypesToDeleteChainInfo.contains(kvmStoragePoolPrimary.getType()) && + if (volumeObjectTo.getChainInfo() != null && poolTypesToDeleteChainInfo.contains(kvmStoragePoolPrimary.getType()) && volumeObjectTo.getFormat() == Storage.ImageFormat.QCOW2 && deleteChain) { - for (String deltaPath : volumeObjectTo.getDeltasToRemove()) { - logger.debug("Deleting leftover backup delta at [{}].", deltaPath); - kvmStoragePoolPrimary.deletePhysicalDisk(deltaPath, volumeObjectTo.getFormat()); - } + logger.debug("Deleting leftover backup delta at [{}].", volumeObjectTo.getChainInfo()); + kvmStoragePoolPrimary.deletePhysicalDisk(volumeObjectTo.getChainInfo(), volumeObjectTo.getFormat()); } logger.debug(String.format("Successfully reverted volume [%s] to snapshot [%s].", volumeObjectTo, snapshotToPrint)); } catch (LibvirtException | QemuImgException ex) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeKbossBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeKbossBackupCommandWrapper.java index 5ff9bbaaad0f..d2332f4f99b1 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeKbossBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeKbossBackupCommandWrapper.java @@ -40,7 +40,6 @@ import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; import org.apache.commons.collections4.CollectionUtils; -import org.apache.commons.lang3.ObjectUtils; import org.libvirt.LibvirtException; import java.io.File; @@ -144,8 +143,8 @@ protected void cleanupVm(TakeKbossBackupCommand command, LibvirtComputingResourc volumeObjectTO.setPath(kbossTO.getDeltaPathOnPrimary()); if (deltaMergeTreeTO != null) { - List snapshotDataStoreVos = kbossTO.getDeltaPaths(); - mergeBackupDelta(resource, deltaMergeTreeTO, volumeObjectTO, vmName, runningVM, volumeUuid, CollectionUtils.isEmpty(snapshotDataStoreVos)); + List snapshotDataStoreVos = kbossTO.getVmSnapshotDeltaPaths(); + mergeBackupDelta(resource, deltaMergeTreeTO, volumeObjectTO, vmName, runningVM, volumeUuid, snapshotDataStoreVos.isEmpty()); } if (command.isEndChain() || command.isIsolated()) { @@ -169,7 +168,7 @@ protected Pair copyBackupDeltaToSecondary(KVMStoragePoolManager st int waitInMillis) { VolumeObjectTO delta = kbossTO.getVolumeObjectTO(); String parentDeltaPathOnSecondary = kbossTO.getPathBackupParentOnSecondary(); - List deltaPathsToCopy = ObjectUtils.defaultIfNull(kbossTO.getDeltaPaths(), new ArrayList<>()); + List deltaPathsToCopy = CollectionUtils.isEmpty(kbossTO.getVmSnapshotDeltaPaths()) ? new ArrayList<>() : new ArrayList<>(kbossTO.getVmSnapshotDeltaPaths()); deltaPathsToCopy.add(delta.getPath()); KVMStoragePool parentImagePool = null; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index b1d43286b725..11acb9546b53 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -99,7 +99,6 @@ import org.apache.cloudstack.utils.qemu.QemuObject.EncryptFormat; import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.commons.collections.MapUtils; -import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.BooleanUtils; @@ -2907,11 +2906,9 @@ public Answer deleteVolume(final DeleteCommand cmd) { } } pool.deletePhysicalDisk(vol.getPath(), vol.getFormat()); - if (CollectionUtils.isNotEmpty(vol.getDeltasToRemove()) && poolTypesToDeleteChainInfo.contains(pool.getType()) && vol.getFormat() == ImageFormat.QCOW2 && cmd.isDeleteChain()) { - for (String deltaPath : vol.getDeltasToRemove()) { - logger.debug("Deleting leftover backup delta at [{}].", deltaPath); - pool.deletePhysicalDisk(deltaPath, vol.getFormat()); - } + if (vol.getChainInfo() != null && poolTypesToDeleteChainInfo.contains(pool.getType()) && vol.getFormat() == ImageFormat.QCOW2 && cmd.isDeleteChain()) { + logger.debug("Deleting leftover backup delta at [{}].", vol.getChainInfo()); + pool.deletePhysicalDisk(vol.getChainInfo(), vol.getFormat()); } return new Answer(null); } catch (final CloudRuntimeException e) { diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirTakeKbossBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirTakeKbossBackupCommandWrapperTest.java index b726b92c7424..8354993e61a1 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirTakeKbossBackupCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirTakeKbossBackupCommandWrapperTest.java @@ -266,7 +266,7 @@ public void copyBackupDeltaToSecondaryTest() throws LibvirtException, QemuImgExc doReturn(volumePath).when(volumeObjectToMock1).getPath(); doReturn(volUuid1).when(volumeObjectToMock1).getUuid(); doReturn(parentPath).when(kbossTO1).getPathBackupParentOnSecondary(); - doReturn(new ArrayList<>(List.of(deltaPath2))).when(kbossTO1).getDeltaPaths(); + doReturn(new ArrayList<>(List.of(deltaPath2))).when(kbossTO1).getVmSnapshotDeltaPaths(); doReturn(deltaPath1).when(kbossTO1).getDeltaPathOnSecondary(); doReturn(kvmStoragePool1).when(kvmStoragePoolManagerMock).getStoragePoolByURI(secondaryUrl); doReturn(kvmStoragePool2).when(kvmStoragePoolManagerMock).getStoragePoolByURI(secondaryUrl2); diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index 58bad20e4f1a..58e435b6406d 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -971,7 +971,6 @@ public boolean deleteBackupSchedule(DeleteBackupScheduleCmd cmd) { throw new InvalidParameterValueException("Could not find the requested backup schedule."); } checkCallerAccessToBackupScheduleVm(schedule.getVmId()); - finalizeBackupScheduleIfNeeded(schedule); return backupScheduleDao.remove(schedule.getId()); } @@ -979,33 +978,6 @@ public boolean deleteBackupSchedule(DeleteBackupScheduleCmd cmd) { return deleteAllVmBackupSchedules(vmId); } - /** - * Terminates the backup schedule if necessary. - * - * @param backupSchedule the backup schedule to be processed for termination. - * @throws CloudRuntimeException if the backup offering associated with the - * virtual machine was not found or if the backup provider could not finalize - * the backup schedule. - */ - protected void finalizeBackupScheduleIfNeeded(BackupSchedule backupSchedule) { - VMInstanceVO vm = findVmById(backupSchedule.getVmId()); - - if (vm.getBackupOfferingId() == null) { - logger.debug("The virtual machine {} backup offering has already been removed; therefore, it is not necessary to finalize the backup schedule.", vm.getUuid()); - return; - } - - BackupOfferingVO backupOffering = backupOfferingDao.findById(vm.getBackupOfferingId()); - if (backupOffering == null) { - throw new CloudRuntimeException("Could not find the backup offering of the backup schedule virtual machine."); - } - - BackupProvider backupProvider = getBackupProvider(backupOffering.getProvider()); - if (!backupProvider.removeVMBackupSchedule(vm, backupSchedule)) { - throw new CloudRuntimeException(String.format("Failed to finalize VM backup schedule with ID [%s].", backupSchedule.getUuid())); - } - } - /** * Checks if the backup framework is enabled for the zone in which the VM with specified ID is allocated and * if the caller has access to the VM. @@ -1030,7 +1002,6 @@ protected boolean deleteAllVmBackupSchedules(long vmId) { List vmBackupSchedules = backupScheduleDao.listByVM(vmId); boolean success = true; for (BackupScheduleVO vmBackupSchedule : vmBackupSchedules) { - finalizeBackupScheduleIfNeeded(vmBackupSchedule); success = success && backupScheduleDao.remove(vmBackupSchedule.getId()); } return success; @@ -1096,7 +1067,7 @@ private void createCheckedBackup(CreateBackupCmd cmd, Account owner, boolean isS CheckedReservation backupStorageReservation = new CheckedReservation(owner, Resource.ResourceType.backup_storage, backupSize, reservationDao, resourceLimitMgr)) { - Pair result = backupProvider.takeBackup(vm, cmd.getQuiesceVM(), cmd.isIsolated(), backupScheduleId); + Pair result = backupProvider.takeBackup(vm, cmd.getQuiesceVM(), cmd.isIsolated()); if (!result.first()) { throw new CloudRuntimeException("Failed to create Instance Backup"); } diff --git a/server/src/main/java/org/apache/cloudstack/backup/InternalBackupServiceImpl.java b/server/src/main/java/org/apache/cloudstack/backup/InternalBackupServiceImpl.java index 5c30188a8c41..12088c76de3c 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/InternalBackupServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/InternalBackupServiceImpl.java @@ -70,7 +70,6 @@ import java.util.HashMap; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; public class InternalBackupServiceImpl extends ComponentLifecycleBase implements InternalBackupService, VmWorkJobHandler { protected Logger logger = LogManager.getLogger(getClass()); @@ -127,17 +126,16 @@ public void configureChainInfo(DataTO volumeTo, Command cmd) { return; } VolumeObjectTO volumeObjectTO = (VolumeObjectTO) volumeTo; - List backupDeltas = internalBackupStoragePoolDao.listByVolumeId(volumeObjectTO.getVolumeId()); - if (backupDeltas.isEmpty()) { + InternalBackupStoragePoolVO backupDelta = internalBackupStoragePoolDao.findOneByVolumeId(volumeObjectTO.getVolumeId()); + if (backupDelta == null) { return; } - volumeObjectTO.setDeltasToRemove(backupDeltas.stream().map(InternalBackupStoragePoolVO::getBackupDeltaParentPath).collect(Collectors.toSet())); + volumeObjectTO.setChainInfo(backupDelta.getBackupDeltaParentPath()); if (cmd instanceof DeleteCommand) { ((DeleteCommand) cmd).setDeleteChain(true); - } else if (cmd instanceof RevertSnapshotCommand) { + } + if (cmd instanceof RevertSnapshotCommand) { ((RevertSnapshotCommand) cmd).setDeleteChain(true); - } else { - return; } logger.debug("Configured chain info for volume [{}]. Set it as [{}].", volumeObjectTO.getUuid(), volumeObjectTO.getChainInfo()); } @@ -145,23 +143,20 @@ public void configureChainInfo(DataTO volumeTo, Command cmd) { @Override public void cleanupBackupMetadata(long volumeId) { logger.debug("Cleaning up backup metadata for volume [{}].", volumeId); - List currents = internalBackupJoinDao.listCurrentsByVolumeIdDesc(volumeId); - if (currents.isEmpty()) { + InternalBackupStoragePoolVO delta = internalBackupStoragePoolDao.findOneByVolumeId(volumeId); + if (delta == null) { return; } internalBackupStoragePoolDao.expungeByVolumeId(volumeId); - for (InternalBackupJoinVO current : currents) { - if (CollectionUtils.isNotEmpty(internalBackupStoragePoolDao.listByBackupId(current.getId()))) { - continue; - } - - logger.debug("Volume [{}] was the last volume with deltas in backup [{}]. Setting the backup as END_OF_CHAIN and not current.", volumeId, current.getUuid()); - backupDetailDao.removeDetail(current.getId(), BackupDetailsDao.CURRENT); - if (!current.getEndOfChain()) { - backupDetailDao.persist(new BackupDetailVO(current.getId(), BackupDetailsDao.END_OF_CHAIN, Boolean.TRUE.toString(), true)); - } + if (CollectionUtils.isNotEmpty(internalBackupStoragePoolDao.listByBackupId(delta.getBackupId()))) { + return; + } + InternalBackupJoinVO joinVO = internalBackupJoinDao.findById(delta.getBackupId()); + logger.debug("Volume [{}] was the last volume with deltas in backup [{}]. Setting the backup as not current and not END_OF_CHAIN.", volumeId, joinVO.getUuid()); + backupDetailDao.removeDetail(joinVO.getId(), BackupDetailsDao.CURRENT); + if (!joinVO.getEndOfChain()) { + backupDetailDao.persist(new BackupDetailVO(joinVO.getId(), BackupDetailsDao.END_OF_CHAIN, Boolean.TRUE.toString(), true)); } - } @@ -307,7 +302,7 @@ public boolean finishBackupChain(long vmId) { if (internalBackupProvider == null) { return false; } - return internalBackupProvider.finishBackupChains(vm); + return internalBackupProvider.finishBackupChain(vm); } @Override diff --git a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java index 927b2831c6a7..04bd03670079 100644 --- a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java +++ b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java @@ -725,7 +725,7 @@ public void createBackupTestCreateScheduledBackup() throws ResourceAllocationExc when(backup.getId()).thenReturn(backupId); when(backup.getSize()).thenReturn(newBackupSize); when(backupProvider.getName()).thenReturn("testbackupprovider"); - when(backupProvider.takeBackup(vmInstanceVOMock, null, false, scheduleId)).thenReturn(new Pair<>(true, backup)); + when(backupProvider.takeBackup(vmInstanceVOMock, null, false)).thenReturn(new Pair<>(true, backup)); Map backupProvidersMap = new HashMap<>(); backupProvidersMap.put(backupProvider.getName().toLowerCase(), backupProvider); ReflectionTestUtils.setField(backupManager, "backupProvidersMap", backupProvidersMap); @@ -955,7 +955,6 @@ public void deleteAllVmBackupSchedulesTestReturnSuccessWhenAllSchedulesAreDelete Mockito.when(backupSchedules.get(0).getId()).thenReturn(2L); Mockito.when(backupSchedules.get(1).getId()).thenReturn(3L); Mockito.when(backupScheduleDao.remove(Mockito.anyLong())).thenReturn(true); - Mockito.doNothing().when(backupManager).finalizeBackupScheduleIfNeeded(Mockito.any()); boolean success = backupManager.deleteAllVmBackupSchedules(vmId); assertTrue(success); @@ -971,7 +970,6 @@ public void deleteAllVmBackupSchedulesTestReturnFalseWhenAnyDeletionFails() { Mockito.when(backupSchedules.get(1).getId()).thenReturn(3L); Mockito.when(backupScheduleDao.remove(2L)).thenReturn(true); Mockito.when(backupScheduleDao.remove(3L)).thenReturn(false); - Mockito.doNothing().when(backupManager).finalizeBackupScheduleIfNeeded(Mockito.any()); boolean success = backupManager.deleteAllVmBackupSchedules(vmId); assertFalse(success); @@ -1017,7 +1015,6 @@ public void deleteBackupScheduleTestDeleteSpecificScheduleWhenItsIdIsSpecified() Mockito.doNothing().when(backupManager).checkCallerAccessToBackupScheduleVm(vmId); when(backupScheduleVOMock.getId()).thenReturn(id); when(backupScheduleDao.remove(id)).thenReturn(true); - Mockito.doNothing().when(backupManager).finalizeBackupScheduleIfNeeded(Mockito.any()); boolean success = backupManager.deleteBackupSchedule(deleteBackupScheduleCmdMock); assertTrue(success); @@ -1332,7 +1329,6 @@ public void testDeleteBackupScheduleByVmId() { when(schedule.getId()).thenReturn(scheduleId); when(backupScheduleDao.listByVM(vmId)).thenReturn(List.of(schedule)); when(backupScheduleDao.remove(scheduleId)).thenReturn(true); - doNothing().when(backupManager).finalizeBackupScheduleIfNeeded(any()); boolean result = backupManager.deleteBackupSchedule(cmd); assertTrue(result); @@ -2878,45 +2874,4 @@ public void createBackupOfferingTestAddsNoDetails() { verify(backupOfferingDao).persist(any()); verify(backupOfferingDetailsDao, never()).saveDetails(any()); } - - @Test(expected = CloudRuntimeException.class) - public void endScheduleBackupChainIfNeededTestInvalidVirtualMachineThrowCloudRuntimeException() { - Mockito.doReturn(1L).when(backupScheduleVOMock).getVmId(); - - backupManager.finalizeBackupScheduleIfNeeded(backupScheduleVOMock); - } - - @Test(expected = CloudRuntimeException.class) - public void endScheduleBackupChainIfNeededTestInvalidBackupOfferingThrowCloudRuntimeException() { - Mockito.doReturn(1L).when(backupScheduleVOMock).getVmId(); - Mockito.doReturn(vmInstanceVOMock).when(vmInstanceDao).findById(1L); - - backupManager.finalizeBackupScheduleIfNeeded(backupScheduleVOMock); - } - - @Test - public void endScheduleBackupChainIfNeededTestBackupProviderSuccessDoesNotThrowException() { - Mockito.doReturn(1L).when(backupScheduleVOMock).getVmId(); - Mockito.doReturn(vmInstanceVOMock).when(vmInstanceDao).findById(1L); - Mockito.doReturn(2L).when(vmInstanceVOMock).getBackupOfferingId(); - Mockito.doReturn(backupOfferingVOMock).when(backupOfferingDao).findById(2L); - Mockito.doReturn(BackupManagerImpl.KBOSS_BACKUP_PROVIDER).when(backupOfferingVOMock).getProvider(); - Mockito.doReturn(backupProvider).when(backupManager).getBackupProvider(BackupManagerImpl.KBOSS_BACKUP_PROVIDER); - Mockito.doReturn(true).when(backupProvider).removeVMBackupSchedule(vmInstanceVOMock, backupScheduleVOMock); - - backupManager.finalizeBackupScheduleIfNeeded(backupScheduleVOMock); - } - - @Test(expected = CloudRuntimeException.class) - public void endScheduleBackupChainIfNeededTestBackupProviderFailThrowCloudRuntimeException() { - Mockito.doReturn(1L).when(backupScheduleVOMock).getVmId(); - Mockito.doReturn(vmInstanceVOMock).when(vmInstanceDao).findById(1L); - Mockito.doReturn(2L).when(vmInstanceVOMock).getBackupOfferingId(); - Mockito.doReturn(backupOfferingVOMock).when(backupOfferingDao).findById(2L); - Mockito.doReturn(BackupManagerImpl.KBOSS_BACKUP_PROVIDER).when(backupOfferingVOMock).getProvider(); - Mockito.doReturn(backupProvider).when(backupManager).getBackupProvider(BackupManagerImpl.KBOSS_BACKUP_PROVIDER); - Mockito.doReturn(false).when(backupProvider).removeVMBackupSchedule(vmInstanceVOMock, backupScheduleVOMock); - - backupManager.finalizeBackupScheduleIfNeeded(backupScheduleVOMock); - } } diff --git a/server/src/test/java/org/apache/cloudstack/backup/InternalBackupServiceImplTest.java b/server/src/test/java/org/apache/cloudstack/backup/InternalBackupServiceImplTest.java index 5ad0aabaf825..008a590c3e5a 100644 --- a/server/src/test/java/org/apache/cloudstack/backup/InternalBackupServiceImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/backup/InternalBackupServiceImplTest.java @@ -17,7 +17,9 @@ package org.apache.cloudstack.backup; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.doNothing; @@ -33,6 +35,8 @@ import org.apache.cloudstack.backup.dao.InternalBackupJoinDao; import org.apache.cloudstack.backup.dao.InternalBackupStoragePoolDao; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.storage.command.DeleteCommand; +import org.apache.cloudstack.storage.command.RevertSnapshotCommand; import org.apache.cloudstack.storage.datastore.db.ImageStoreObjectDownloadDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreObjectDownloadVO; import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity; @@ -140,26 +144,83 @@ public void configureChainInfoTestNonVolumeObjectReturnsImmediately() { internalBackupServiceImplSpy.configureChainInfo(dataToMock, cmdMock); - verify(internalBackupStoragePoolDaoMock, never()).listByVolumeId(anyLong()); + verify(internalBackupStoragePoolDaoMock, never()).findOneByVolumeId(anyLong()); + } + + @Test + public void configureChainInfoTestVolumeWithoutBackupDeltaReturnsImmediately() { + doReturn(VOLUME_ID).when(volumeObjectToMock).getVolumeId(); + doReturn(null).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); + + internalBackupServiceImplSpy.configureChainInfo(volumeObjectToMock, mock(Command.class)); + + verify(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); + verify(volumeObjectToMock, never()).setChainInfo(anyString()); + } + + @Test + public void configureChainInfoTestSetsChainInfoForGenericCommand() { + doReturn(VOLUME_ID).when(volumeObjectToMock).getVolumeId(); + doReturn(internalBackupStoragePoolVoMock).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); + doReturn("/path/to/parent").when(internalBackupStoragePoolVoMock).getBackupDeltaParentPath(); + + Command cmdMock = mock(Command.class); + + internalBackupServiceImplSpy.configureChainInfo(volumeObjectToMock, cmdMock); + + verify(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); + verify(volumeObjectToMock).setChainInfo("/path/to/parent"); + } + + @Test + public void configureChainInfoTestSetsDeleteChainForDeleteCommand() { + doReturn(VOLUME_ID).when(volumeObjectToMock).getVolumeId(); + doReturn(internalBackupStoragePoolVoMock).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); + doReturn("/path/to/parent").when(internalBackupStoragePoolVoMock).getBackupDeltaParentPath(); + + DeleteCommand deleteCommand = new DeleteCommand(volumeObjectToMock); + + internalBackupServiceImplSpy.configureChainInfo(volumeObjectToMock, deleteCommand); + + verify(volumeObjectToMock).setChainInfo("/path/to/parent"); + assertTrue(deleteCommand.isDeleteChain()); + } + + @Test + public void configureChainInfoTestSetsDeleteChainForRevertSnapshotCommand() { + doReturn(VOLUME_ID).when(volumeObjectToMock).getVolumeId(); + doReturn(internalBackupStoragePoolVoMock).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); + doReturn("/path/to/parent").when(internalBackupStoragePoolVoMock).getBackupDeltaParentPath(); + + RevertSnapshotCommand revertSnapshotCommand = new RevertSnapshotCommand(snapshotObjectToMock, snapshotObjectToMock); + + internalBackupServiceImplSpy.configureChainInfo(volumeObjectToMock, revertSnapshotCommand); + + verify(volumeObjectToMock).setChainInfo("/path/to/parent"); + assertTrue(revertSnapshotCommand.isDeleteChain()); } @Test public void cleanupBackupMetadataTestNoDeltaReturnsImmediately() { + doReturn(null).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); + internalBackupServiceImplSpy.cleanupBackupMetadata(VOLUME_ID); + verify(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); verify(internalBackupStoragePoolDaoMock, never()).expungeByVolumeId(VOLUME_ID); verify(internalBackupJoinDaoMock, never()).findById(anyLong()); } @Test public void cleanupBackupMetadataTestDeltaExistsButOtherDeltasRemainReturnsImmediately() { - doReturn(BACKUP_ID).when(internalBackupJoinVoMock).getId(); - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listCurrentsByVolumeIdDesc(VOLUME_ID); + doReturn(BACKUP_ID).when(internalBackupStoragePoolVoMock).getBackupId(); + doReturn(internalBackupStoragePoolVoMock).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); doReturn(List.of(internalBackupStoragePoolVoMock, mock(InternalBackupStoragePoolVO.class))) .when(internalBackupStoragePoolDaoMock).listByBackupId(BACKUP_ID); internalBackupServiceImplSpy.cleanupBackupMetadata(VOLUME_ID); + verify(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); verify(internalBackupStoragePoolDaoMock).expungeByVolumeId(VOLUME_ID); verify(internalBackupStoragePoolDaoMock).listByBackupId(BACKUP_ID); verify(internalBackupJoinDaoMock, never()).findById(anyLong()); @@ -167,30 +228,38 @@ public void cleanupBackupMetadataTestDeltaExistsButOtherDeltasRemainReturnsImmed @Test public void cleanupBackupMetadataTestLastDeltaAndEndOfChainTrue() { - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listCurrentsByVolumeIdDesc(VOLUME_ID); + doReturn(BACKUP_ID).when(internalBackupStoragePoolVoMock).getBackupId(); + doReturn(internalBackupStoragePoolVoMock).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); doReturn(List.of()).when(internalBackupStoragePoolDaoMock).listByBackupId(BACKUP_ID); + doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findById(BACKUP_ID); doReturn(BACKUP_ID).when(internalBackupJoinVoMock).getId(); doReturn(true).when(internalBackupJoinVoMock).getEndOfChain(); internalBackupServiceImplSpy.cleanupBackupMetadata(VOLUME_ID); + verify(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); verify(internalBackupStoragePoolDaoMock).expungeByVolumeId(VOLUME_ID); verify(internalBackupStoragePoolDaoMock).listByBackupId(BACKUP_ID); + verify(internalBackupJoinDaoMock).findById(BACKUP_ID); verify(backupDetailDaoMock).removeDetail(BACKUP_ID, BackupDetailsDao.CURRENT); verify(backupDetailDaoMock, never()).persist(any()); } @Test public void cleanupBackupMetadataTestLastDeltaAndEndOfChainFalse() { - doReturn(List.of(internalBackupJoinVoMock)).when(internalBackupJoinDaoMock).listCurrentsByVolumeIdDesc(VOLUME_ID); + doReturn(BACKUP_ID).when(internalBackupStoragePoolVoMock).getBackupId(); + doReturn(internalBackupStoragePoolVoMock).when(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); doReturn(List.of()).when(internalBackupStoragePoolDaoMock).listByBackupId(BACKUP_ID); + doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findById(BACKUP_ID); doReturn(BACKUP_ID).when(internalBackupJoinVoMock).getId(); doReturn(false).when(internalBackupJoinVoMock).getEndOfChain(); internalBackupServiceImplSpy.cleanupBackupMetadata(VOLUME_ID); + verify(internalBackupStoragePoolDaoMock).findOneByVolumeId(VOLUME_ID); verify(internalBackupStoragePoolDaoMock).expungeByVolumeId(VOLUME_ID); verify(internalBackupStoragePoolDaoMock).listByBackupId(BACKUP_ID); + verify(internalBackupJoinDaoMock).findById(BACKUP_ID); verify(backupDetailDaoMock).removeDetail(BACKUP_ID, BackupDetailsDao.CURRENT); verify(backupDetailDaoMock).persist(any(BackupDetailVO.class)); } From 4dc723b2bdeb5835cb0eb587d26bd0440204f244 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Fri, 24 Jul 2026 08:42:25 -0300 Subject: [PATCH 2/4] address reviews --- .../org/apache/cloudstack/backup/KbossBackupProvider.java | 2 ++ .../org/apache/cloudstack/backup/KbossBackupProviderTest.java | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java b/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java index d23182920a84..7575f820d633 100644 --- a/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java +++ b/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java @@ -1239,6 +1239,8 @@ protected void endBackupChainIfConfigured(BackupVO backupVO) { if (!getValidationEndChainOnFail(backupVO)) { return; } + VirtualMachine vm = userVmDao.findByIdIncludingRemoved(backupVO.getVmId()); + validateVmState(vm, "end backup chain", VirtualMachine.State.Expunging, VirtualMachine.State.Destroyed); List backupChildren = getBackupJoinChildren(backupVO); // Get updated record diff --git a/plugins/backup/kboss/src/test/java/org/apache/cloudstack/backup/KbossBackupProviderTest.java b/plugins/backup/kboss/src/test/java/org/apache/cloudstack/backup/KbossBackupProviderTest.java index 390f17d42531..db242cf08b21 100644 --- a/plugins/backup/kboss/src/test/java/org/apache/cloudstack/backup/KbossBackupProviderTest.java +++ b/plugins/backup/kboss/src/test/java/org/apache/cloudstack/backup/KbossBackupProviderTest.java @@ -1604,6 +1604,7 @@ public void validateWithValidationVmTestValidateBackupFails() throws NoTransitio doReturn(virtualMachineToMock).when(hypervisorGuruMock).implement(any()); doReturn(false).when(kbossBackupProviderSpy).validateBackup(anyLong(), any(), any(), any(), any(), any()); doNothing().when(kbossBackupProviderSpy).sendCleanupFailedEmail(any(), any()); + doNothing().when(kbossBackupProviderSpy).validateVmState(any(), any(), any(), any()); boolean result = kbossBackupProviderSpy.validateWithValidationVm(backupId, 2L, backupVoMock); @@ -1678,6 +1679,7 @@ public void endBackupChainIfConfiguredTestNotCurrentAndNoCurrentChildren() { InternalBackupJoinVO child = mock(InternalBackupJoinVO.class); doReturn(false).when(child).getCurrent(); doReturn(List.of(child)).when(kbossBackupProviderSpy).getBackupJoinChildren(any()); + doNothing().when(kbossBackupProviderSpy).validateVmState(any(), any(), any(), any()); kbossBackupProviderSpy.endBackupChainIfConfigured(backupVoMock); @@ -1692,6 +1694,7 @@ public void endBackupChainIfConfiguredTestBackupIsCurrent() { doReturn(List.of()).when(kbossBackupProviderSpy).getBackupJoinChildren(any()); doReturn(userVmVOMock).when(userVmDaoMock).findById(anyLong()); doReturn(true).when(kbossBackupProviderSpy).endBackupChain(any()); + doNothing().when(kbossBackupProviderSpy).validateVmState(any(), any(), any(), any()); kbossBackupProviderSpy.endBackupChainIfConfigured(backupVoMock); @@ -1708,6 +1711,7 @@ public void endBackupChainIfConfiguredTestLastChildIsCurrent() { doReturn(List.of(child)).when(kbossBackupProviderSpy).getBackupJoinChildren(any()); doReturn(userVmVOMock).when(userVmDaoMock).findById(anyLong()); doReturn(true).when(kbossBackupProviderSpy).endBackupChain(any()); + doNothing().when(kbossBackupProviderSpy).validateVmState(any(), any(), any(), any()); kbossBackupProviderSpy.endBackupChainIfConfigured(backupVoMock); From 47134fa7a355f5eb808acc7f923c5402791e1d33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Fri, 24 Jul 2026 08:46:39 -0300 Subject: [PATCH 3/4] minor nit --- .../java/org/apache/cloudstack/backup/KbossBackupProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java b/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java index 7575f820d633..596dc62b7206 100644 --- a/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java +++ b/plugins/backup/kboss/src/main/java/org/apache/cloudstack/backup/KbossBackupProvider.java @@ -1248,7 +1248,7 @@ protected void endBackupChainIfConfigured(BackupVO backupVO) { if (backupJoinVO.getCurrent() || (!backupChildren.isEmpty() && backupChildren.get(backupChildren.size() - 1).getCurrent())) { logger.info("As [{}] is true, we are ending the backup chain for VM [{}]. The next backup will be a full backup.", BackupValidationServiceJobController.backupValidationEndChainOnFail.toString()); - endBackupChain(userVmDao.findById(backupVO.getVmId())); + endBackupChain(vm); } } From 5d22bc756ebe544471b1488c174ede710d83b0ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Fri, 24 Jul 2026 09:34:07 -0300 Subject: [PATCH 4/4] fix test --- .../org/apache/cloudstack/backup/KbossBackupProviderTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/backup/kboss/src/test/java/org/apache/cloudstack/backup/KbossBackupProviderTest.java b/plugins/backup/kboss/src/test/java/org/apache/cloudstack/backup/KbossBackupProviderTest.java index db242cf08b21..d16e0bd948fc 100644 --- a/plugins/backup/kboss/src/test/java/org/apache/cloudstack/backup/KbossBackupProviderTest.java +++ b/plugins/backup/kboss/src/test/java/org/apache/cloudstack/backup/KbossBackupProviderTest.java @@ -1692,7 +1692,7 @@ public void endBackupChainIfConfiguredTestBackupIsCurrent() { doReturn(true).when(internalBackupJoinVoMock).getCurrent(); doReturn(internalBackupJoinVoMock).when(internalBackupJoinDaoMock).findById(anyLong()); doReturn(List.of()).when(kbossBackupProviderSpy).getBackupJoinChildren(any()); - doReturn(userVmVOMock).when(userVmDaoMock).findById(anyLong()); + doReturn(userVmVOMock).when(userVmDaoMock).findByIdIncludingRemoved(anyLong()); doReturn(true).when(kbossBackupProviderSpy).endBackupChain(any()); doNothing().when(kbossBackupProviderSpy).validateVmState(any(), any(), any(), any()); @@ -1709,7 +1709,7 @@ public void endBackupChainIfConfiguredTestLastChildIsCurrent() { InternalBackupJoinVO child = mock(InternalBackupJoinVO.class); doReturn(true).when(child).getCurrent(); doReturn(List.of(child)).when(kbossBackupProviderSpy).getBackupJoinChildren(any()); - doReturn(userVmVOMock).when(userVmDaoMock).findById(anyLong()); + doReturn(userVmVOMock).when(userVmDaoMock).findByIdIncludingRemoved(anyLong()); doReturn(true).when(kbossBackupProviderSpy).endBackupChain(any()); doNothing().when(kbossBackupProviderSpy).validateVmState(any(), any(), any(), any());