From 75864527c68045156e4d5052d6629d3dbeb7ffd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= Date: Thu, 14 May 2026 10:57:03 -0300 Subject: [PATCH 1/3] Multiple Backup Schedules --- .../apache/cloudstack/api/ApiConstants.java | 1 + .../admin/backup/ImportBackupOfferingCmd.java | 14 ++ .../admin/backup/UpdateBackupOfferingCmd.java | 18 +- .../user/backup/CreateBackupOfferingCmd.java | 15 +- .../user/backup/CreateBackupScheduleCmd.java | 21 +- .../user/backup/UpdateBackupScheduleCmd.java | 133 +++++++++- .../cloudstack/backup/BackupManager.java | 8 + .../cloudstack/backup/BackupScheduleVO.java | 2 +- .../backup/dao/BackupScheduleDao.java | 2 +- .../backup/dao/BackupScheduleDaoImpl.java | 4 +- .../cloudstack/backup/BackupManagerImpl.java | 178 ++++++++++--- .../cloudstack/backup/BackupManagerTest.java | 10 +- ui/public/locales/en.json | 3 + ui/public/locales/pt_BR.json | 3 + ui/src/config/section/offering.js | 3 +- ui/src/utils/plugins.js | 2 +- ui/src/views/compute/BackupScheduleWizard.vue | 10 +- .../views/compute/backup/BackupSchedule.vue | 53 +++- ui/src/views/compute/backup/FormSchedule.vue | 131 +++++++--- .../views/offering/CreateBackupOffering.vue | 79 +++++- .../views/offering/ImportBackupOffering.vue | 79 +++++- .../views/offering/UpdateBackupOffering.vue | 233 ++++++++++++++++++ 22 files changed, 911 insertions(+), 91 deletions(-) create mode 100644 ui/src/views/offering/UpdateBackupOffering.vue diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index ac6acdf42516..8007565f0ec3 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -1489,6 +1489,7 @@ public class ApiConstants { public static final String SCHEDULED = "scheduled"; public static final String SCHEDULED_DATE = "scheduleddate"; public static final String BACKUP_PROVIDER = "backupprovider"; + public static final String MAX_SCHEDULES = "maxschedules"; /** * This enum specifies IO Drivers, each option controls specific policies on I/O. diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupOfferingCmd.java index 4cf27c561508..6c595b7aa545 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupOfferingCmd.java @@ -42,9 +42,11 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections4.MapUtils; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.Set; @APICommand(name = "importBackupOffering", @@ -90,6 +92,11 @@ public class ImportBackupOfferingCmd extends BaseAsyncCmd { since = "4.23.0") private List domainIds; + @Parameter(name = ApiConstants.MAX_SCHEDULES, type = CommandType.MAP, + description = "Maximum number of schedules, values lower than 0 disable the limit. By default, all allowed schedule types have a limit of 1. Accepts a map of " + + "schedule type to maximum amount. Example: maxschedules[0].DAILY=2&maxschedules[0].WEEKLY=5. Please note that all values should be on the same index ('0').") + private Map maxSchedules; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -123,6 +130,13 @@ public List getDomainIds() { return domainIds; } + public Map getMaxSchedules() { + if (MapUtils.isEmpty(maxSchedules)) { + return null; + } + return (Map)maxSchedules.values().iterator().next(); + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java index 2f0dd6acd0e1..6a553de848d8 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java @@ -31,6 +31,8 @@ import org.apache.cloudstack.backup.BackupOffering; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections4.MapUtils; +import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import com.cloud.exception.InvalidParameterValueException; @@ -40,6 +42,8 @@ import java.util.List; import java.util.function.LongFunction; +import java.util.Map; + @APICommand(name = "updateBackupOffering", description = "Updates a backup offering.", responseObject = BackupOfferingResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.16.0") public class UpdateBackupOfferingCmd extends BaseCmd implements DomainAndZoneIdResolver { @@ -69,6 +73,11 @@ public class UpdateBackupOfferingCmd extends BaseCmd implements DomainAndZoneIdR length = 4096) private String domainIds; + @Parameter(name = ApiConstants.MAX_SCHEDULES, type = CommandType.MAP, + description = "Maximum number of schedules, values lower than 0 disable the limit. By default, all allowed schedule types have a limit of 1. Accepts a map of " + + "schedule type to maximum amount. Example: maxschedules[0].DAILY=2&maxschedules[0].WEEKLY=5. Please note that all values should be on the same index ('0').") + private Map maxSchedules; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -88,13 +97,20 @@ public Boolean getAllowUserDrivenBackups() { return allowUserDrivenBackups; } + public Map getMaxSchedules() { + if (MapUtils.isEmpty(maxSchedules)) { + return null; + } + return (Map)maxSchedules.values().iterator().next(); + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @Override public void execute() { try { - if (StringUtils.isAllEmpty(getName(), getDescription()) && getAllowUserDrivenBackups() == null && CollectionUtils.isEmpty(getDomainIds())) { + if (StringUtils.isAllEmpty(getName(), getDescription()) && getAllowUserDrivenBackups() == null && CollectionUtils.isEmpty(getDomainIds()) && ObjectUtils.allNull(getAllowUserDrivenBackups(), getMaxSchedules())) { throw new InvalidParameterValueException(String.format("Can't update Backup Offering [id: %s] because there are no parameters to be updated, at least one of the", "following should be informed: name, description or allowUserDrivenBackups.", id)); } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupOfferingCmd.java index c5d29b615439..9a4db8f20def 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupOfferingCmd.java @@ -34,10 +34,11 @@ import org.apache.cloudstack.backup.Backup; import org.apache.cloudstack.backup.BackupManager; import org.apache.cloudstack.backup.BackupOffering; - +import org.apache.commons.collections4.MapUtils; import javax.inject.Inject; import java.util.List; +import java.util.Map; @APICommand(name = "createBackupOffering", description = "Creates a backup offering", responseObject = BackupOfferingResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, authorized = {RoleType.Admin}, since = "4.23.0") @@ -92,6 +93,11 @@ public class CreateBackupOfferingCmd extends BaseCmd { description = "Restrict the backup offering to the Domains identified by these IDs.") private List domainIds; + @Parameter(name = ApiConstants.MAX_SCHEDULES, type = CommandType.MAP, + description = "Maximum number of schedules, values lower than 0 disable the limit. By default, all allowed schedule types have a limit of 1. Accepts a map of " + + "schedule type to maximum amount. Example: maxschedules[0].DAILY=2&maxschedules[0].WEEKLY=5. Please note that all values should be on the same index ('0').") + private Map maxSchedules; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -166,6 +172,13 @@ public Boolean getUserDrivenBackups() { return userDrivenBackups; } + public Map getMaxSchedules() { + if (MapUtils.isEmpty(maxSchedules)) { + return null; + } + return (Map)maxSchedules.values().iterator().next(); + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupScheduleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupScheduleCmd.java index 41c530471da5..f8e946f56175 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupScheduleCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupScheduleCmd.java @@ -43,7 +43,7 @@ public class CreateBackupScheduleCmd extends BaseCmd { @Inject - private BackupManager backupManager; + protected BackupManager backupManager; ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// @@ -92,7 +92,7 @@ public class CreateBackupScheduleCmd extends BaseCmd { type = CommandType.BOOLEAN, description = ApiConstants.PARAMETER_DESCRIPTION_ISOLATED_BACKUPS, since = "4.23.0") - private boolean isolated; + private Boolean isolated; ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// @@ -118,11 +118,11 @@ public Integer getMaxBackups() { return maxBackups; } - public Boolean getQuiesceVM() { + public Boolean isQuiesceVM() { return quiesceVM; } - public boolean isIsolated() { + public Boolean isIsolated() { return isolated; } @@ -160,4 +160,17 @@ public Long getApiResourceId() { public ApiCommandResourceType getApiResourceType() { return ApiCommandResourceType.VirtualMachine; } + + public CreateBackupScheduleCmd() { + } + + public CreateBackupScheduleCmd(UpdateBackupScheduleCmd that) { + this.maxBackups = that.getMaxBackups(); + this.schedule = that.getSchedule(); + this.intervalType = that.intervalType; + this.quiesceVM = that.isQuiesceVm(); + this.isolated = that.isIsolated(); + this.vmId = that.getVmId(); + this.timezone = that.getTimezone(); + } } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/UpdateBackupScheduleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/UpdateBackupScheduleCmd.java index f4938ee0f87d..0deb81c48e2a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/UpdateBackupScheduleCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/UpdateBackupScheduleCmd.java @@ -17,13 +17,144 @@ package org.apache.cloudstack.api.command.user.backup; +import com.cloud.utils.DateUtil; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.BackupResponse; +import org.apache.cloudstack.api.response.BackupScheduleResponse; +import org.apache.cloudstack.api.response.UserVmResponse; +import org.apache.cloudstack.backup.BackupManager; +import org.apache.cloudstack.backup.BackupSchedule; +import org.apache.cloudstack.context.CallContext; + +import javax.inject.Inject; @APICommand(name = "updateBackupSchedule", description = "Updates a User-defined Instance backup schedule", responseObject = BackupResponse.class, since = "4.14.0", authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) -public class UpdateBackupScheduleCmd extends CreateBackupScheduleCmd { +public class UpdateBackupScheduleCmd extends BaseCmd { + + @Inject + protected BackupManager backupManager; + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @Parameter(name = ApiConstants.ID, + type = CommandType.UUID, + entityType = BackupScheduleResponse.class, + required = false, + description = "ID of the schedule which should be updated. This parameter takes precedence over the virtualmachineid parameter.") + private Long id; + + @Parameter(name = ApiConstants.VIRTUAL_MACHINE_ID, + type = CommandType.UUID, + entityType = UserVmResponse.class, + required = false, + description = "ID of the VM for which schedule is to be defined") + private Long vmId; + + @Parameter(name = ApiConstants.INTERVAL_TYPE, + type = CommandType.STRING, + required = false, + description = "valid values are HOURLY, DAILY, WEEKLY, and MONTHLY") + protected String intervalType; + + @Parameter(name = ApiConstants.SCHEDULE, + type = CommandType.STRING, + required = false, + description = "custom backup schedule, the format is:" + + "for HOURLY MM*, for DAILY MM:HH*, for WEEKLY MM:HH:DD (1-7)*, for MONTHLY MM:HH:DD (1-28)") + private String schedule; + + @Parameter(name = ApiConstants.TIMEZONE, + type = CommandType.STRING, + required = false, + description = "Specifies a timezone for this command. For more information on the timezone parameter, see TimeZone Format.") + private String timezone; + + @Parameter(name = ApiConstants.VM_SNAPSHOT_QUIESCEVM, + type = CommandType.BOOLEAN, + description = "Whether the VM's file systems should be frozen for the scheduled backups. Currently only supported for the KNIB backup provider.") + private Boolean quiesceVm; + + @Parameter(name = ApiConstants.MAX_BACKUPS, type = CommandType.INTEGER, + description = ApiConstants.PARAMETER_DESCRIPTION_MAX_BACKUPS, since = "4.24.0") + private Integer maxBackups; + + @Parameter(name = ApiConstants.ISOLATED, + type = CommandType.BOOLEAN, + description = ApiConstants.PARAMETER_DESCRIPTION_ISOLATED_BACKUPS, + since = "4.24.0") + private Boolean isolated; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public Long getId() { + return id; + } + + public Long getVmId() { + return vmId; + } + + public DateUtil.IntervalType getIntervalType() { + return DateUtil.IntervalType.getIntervalType(intervalType); + } + + public String getSchedule() { + return schedule; + } + + public String getTimezone() { + return timezone; + } + + public Boolean isQuiesceVm() { + return quiesceVm; + } + + public Integer getMaxBackups() { + return maxBackups; + } + + public Boolean isIsolated() { + return isolated; + } + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + @Override + public void execute() throws ServerApiException { + try { + BackupSchedule schedule = backupManager.configureBackupSchedule(this); + + if (schedule != null) { + BackupScheduleResponse response = _responseGenerator.createBackupScheduleResponse(schedule); + response.setResponseName(getCommandName()); + setResponseObject(response); + } else { + throw new CloudRuntimeException("Error while updating backup schedule of VM."); + } + } catch (Exception e) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage()); + } + } + + @Override + public long getEntityOwnerId() { + return CallContext.current().getCallingAccount().getId(); + } } diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java index 30fc2bbec40d..82dbb1f23d6a 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java +++ b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java @@ -34,6 +34,7 @@ import org.apache.cloudstack.api.command.user.backup.ListBackupScheduleCmd; import org.apache.cloudstack.api.command.user.backup.ListBackupsCmd; import org.apache.cloudstack.api.command.user.backup.CreateBackupOfferingCmd; +import org.apache.cloudstack.api.command.user.backup.UpdateBackupScheduleCmd; import org.apache.cloudstack.api.response.BackupResponse; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; @@ -187,6 +188,13 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer */ BackupSchedule configureBackupSchedule(CreateBackupScheduleCmd cmd); + /** + * Updates a VM backup schedule + * @param cmd + * @return the updated backup schedule + */ + BackupSchedule configureBackupSchedule(UpdateBackupScheduleCmd cmd); + /** * Lists VM backup schedule for a VM * @param vmId diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupScheduleVO.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupScheduleVO.java index aa02bb077163..f73ff70ea788 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupScheduleVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupScheduleVO.java @@ -172,7 +172,7 @@ public void setQuiesceVM(Boolean quiesceVM) { } public Boolean getQuiesceVM() { - return quiesceVM; + return this.quiesceVM; } @Override diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupScheduleDao.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupScheduleDao.java index 87b7dab1ff7b..0d6893610e15 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupScheduleDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupScheduleDao.java @@ -28,7 +28,7 @@ public interface BackupScheduleDao extends GenericDao { List listByVM(Long vmId); - BackupScheduleVO findByVMAndIntervalType(Long vmId, DateUtil.IntervalType intervalType); + List listByVMAndIntervalType(Long vmId, DateUtil.IntervalType intervalType); List getSchedulesToExecute(Date currentTimestamp); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupScheduleDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupScheduleDaoImpl.java index 972af73391af..3ffbb91789f8 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupScheduleDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupScheduleDaoImpl.java @@ -62,11 +62,11 @@ public List listByVM(Long vmId) { } @Override - public BackupScheduleVO findByVMAndIntervalType(Long vmId, DateUtil.IntervalType intervalType) { + public List listByVMAndIntervalType(Long vmId, DateUtil.IntervalType intervalType) { SearchCriteria sc = backupScheduleSearch.create(); sc.setParameters("vm_id", vmId); sc.setParameters("interval_type", intervalType.ordinal()); - return findOneBy(sc); + return listBy(sc); } @Override 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 9be4c7ea083c..0df2d6788aa2 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -35,19 +35,9 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import com.cloud.host.Host; -import com.cloud.storage.VolumeApiService; -import com.cloud.utils.exception.BackupProviderException; -import com.cloud.utils.fsm.NoTransitionException; -import com.cloud.vm.VirtualMachineManager; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.vm.VmDiskInfo; -import com.google.gson.GsonBuilder; -import com.google.gson.reflect.TypeToken; -import com.cloud.utils.DomainHelper; -import com.cloud.utils.EnumUtils; import org.apache.cloudstack.api.ApiCommandResourceType; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.InternalIdentity; @@ -60,20 +50,20 @@ import org.apache.cloudstack.api.command.admin.vm.CreateVMFromBackupCmdByAdmin; import org.apache.cloudstack.api.command.user.backup.AssignVirtualMachineToBackupOfferingCmd; import org.apache.cloudstack.api.command.user.backup.CreateBackupCmd; +import org.apache.cloudstack.api.command.user.backup.CreateBackupOfferingCmd; import org.apache.cloudstack.api.command.user.backup.CreateBackupScheduleCmd; import org.apache.cloudstack.api.command.user.backup.DeleteBackupCmd; import org.apache.cloudstack.api.command.user.backup.DeleteBackupScheduleCmd; import org.apache.cloudstack.api.command.user.backup.DownloadValidationScreenshotCmd; import org.apache.cloudstack.api.command.user.backup.FinishBackupChainCmd; -import org.apache.cloudstack.api.command.user.backup.ListBackupServiceJobsCmd; import org.apache.cloudstack.api.command.user.backup.ListBackupOfferingsCmd; import org.apache.cloudstack.api.command.user.backup.ListBackupScheduleCmd; +import org.apache.cloudstack.api.command.user.backup.ListBackupServiceJobsCmd; import org.apache.cloudstack.api.command.user.backup.ListBackupsCmd; import org.apache.cloudstack.api.command.user.backup.RemoveVirtualMachineFromBackupOfferingCmd; import org.apache.cloudstack.api.command.user.backup.RestoreBackupCmd; import org.apache.cloudstack.api.command.user.backup.RestoreVolumeFromBackupAndAttachToVMCmd; import org.apache.cloudstack.api.command.user.backup.UpdateBackupScheduleCmd; -import org.apache.cloudstack.api.command.user.backup.CreateBackupOfferingCmd; import org.apache.cloudstack.api.command.user.backup.repository.AddBackupRepositoryCmd; import org.apache.cloudstack.api.command.user.backup.repository.DeleteBackupRepositoryCmd; import org.apache.cloudstack.api.command.user.backup.repository.ListBackupRepositoriesCmd; @@ -99,6 +89,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections4.MapUtils; import org.apache.commons.lang.math.NumberUtils; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.ObjectUtils; @@ -127,6 +118,7 @@ import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceAllocationException; +import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor; @@ -146,6 +138,7 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; import com.cloud.storage.Volume; +import com.cloud.storage.VolumeApiService; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.GuestOSDao; @@ -161,6 +154,8 @@ import com.cloud.user.User; import com.cloud.user.dao.AccountDao; import com.cloud.utils.DateUtil; +import com.cloud.utils.DomainHelper; +import com.cloud.utils.EnumUtils; import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.component.ComponentContext; @@ -176,14 +171,20 @@ import com.cloud.utils.db.TransactionCallbackNoReturn; import com.cloud.utils.db.TransactionLegacy; import com.cloud.utils.db.TransactionStatus; +import com.cloud.utils.exception.BackupProviderException; import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.fsm.NoTransitionException; import com.cloud.vm.VMInstanceDetailVO; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VirtualMachineManager; +import com.cloud.vm.VmDiskInfo; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; import com.cloud.vm.dao.VMInstanceDetailsDao; import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.reflect.TypeToken; public class BackupManagerImpl extends ManagerBase implements BackupManager { @@ -325,6 +326,8 @@ public BackupOffering importBackupOffering(final ImportBackupOfferingCmd cmd) { throw new CloudRuntimeException("Backup offering '" + cmd.getExternalId() + "' does not exist on provider " + provider.getName() + " on zone " + cmd.getZoneId()); } + Map intervalTypeToMaxAmountMap = parseIntervalTypeToMaxAmountMap(cmd.getMaxSchedules()); + final BackupOfferingVO offering = new BackupOfferingVO(cmd.getZoneId(), cmd.getExternalId(), provider.getName(), cmd.getName(), cmd.getDescription(), cmd.getUserDrivenBackups()); @@ -341,6 +344,11 @@ public BackupOffering importBackupOffering(final ImportBackupOfferingCmd cmd) { backupOfferingDetailsDao.saveDetails(detailsVOList); } } + + for (DateUtil.IntervalType intervalType : intervalTypeToMaxAmountMap.keySet()) { + backupOfferingDetailsDao.addDetail(savedOffering.getId(), intervalType.name(), String.valueOf(intervalTypeToMaxAmountMap.get(intervalType)), true); + } + logger.debug("Successfully created backup offering " + cmd.getName() + " mapped to backup provider offering " + cmd.getExternalId()); return savedOffering; } @@ -415,6 +423,10 @@ public BackupOffering createBackupOffering(CreateBackupOfferingCmd cmd) { if (cmd.getValidationSteps() != null) { detailsVOList.add(new BackupOfferingDetailsVO(savedOffering.getId(), ApiConstants.VALIDATION_STEPS, cmd.getValidationSteps(), true)); } + Map intervalTypeToMaxAmountMap = parseIntervalTypeToMaxAmountMap(cmd.getMaxSchedules()); + for (DateUtil.IntervalType intervalType : intervalTypeToMaxAmountMap.keySet()) { + detailsVOList.add(new BackupOfferingDetailsVO(savedOffering.getId(), intervalType.name(), String.valueOf(intervalTypeToMaxAmountMap.get(intervalType)), true)); + } if (!detailsVOList.isEmpty()) { backupOfferingDetailsDao.saveDetails(detailsVOList); @@ -800,13 +812,14 @@ public boolean removeVMFromBackupOffering(final Long vmId, final boolean forced) } @Override - @ActionEvent(eventType = EventTypes.EVENT_VM_BACKUP_SCHEDULE_CONFIGURE, eventDescription = "configuring Instance Backup Schedule") + @ActionEvent(eventType = EventTypes.EVENT_VM_BACKUP_SCHEDULE_CONFIGURE, eventDescription = "creating Instance Backup Schedule") public BackupSchedule configureBackupSchedule(CreateBackupScheduleCmd cmd) { final Long vmId = cmd.getVmId(); final DateUtil.IntervalType intervalType = cmd.getIntervalType(); final String scheduleString = cmd.getSchedule(); final TimeZone timeZone = TimeZone.getTimeZone(cmd.getTimezone()); - boolean isolated = cmd.isIsolated(); + boolean quiesceVm = BooleanUtils.isTrue(cmd.isQuiesceVM()); + boolean isolated = BooleanUtils.isTrue(cmd.isIsolated()); if (intervalType == null) { throw new CloudRuntimeException("Invalid interval type provided"); @@ -826,44 +839,99 @@ public BackupSchedule configureBackupSchedule(CreateBackupScheduleCmd cmd) { } final int maxBackups = validateAndGetDefaultBackupRetentionIfRequired(cmd.getMaxBackups(), offering, vm); + validateQuiesceAndIsolated(offering, quiesceVm, isolated); - if (isolated && !KBOSS_BACKUP_PROVIDER.equals(offering.getProvider())) { - throw new InvalidParameterValueException("Isolated backups are only supported by KBOSS backup provider."); - } - - if (!quiesceSupported.contains(offering.getProvider()) && cmd.getQuiesceVM() != null) { - throw new InvalidParameterValueException("Quiesce VM option is supported only by NAS and KBOSS backup providers."); - } + validateMaxScheduleForIntervalType(offering, intervalType, vm); + validateDifferentScheduleFromExistingSchedules(vm, intervalType, scheduleString); final String timezoneId = timeZone.getID(); if (!timezoneId.equals(cmd.getTimezone())) { logger.warn("Using timezone: " + timezoneId + " for running this snapshot policy as an equivalent of " + cmd.getTimezone()); } - Date nextDateTime = null; + Date nextDateTime; try { nextDateTime = DateUtil.getNextRunTime(intervalType, cmd.getSchedule(), timezoneId, null); } catch (Exception e) { throw new InvalidParameterValueException("Invalid schedule: " + cmd.getSchedule() + " for interval type: " + cmd.getIntervalType()); } - final BackupScheduleVO schedule = backupScheduleDao.findByVMAndIntervalType(vmId, intervalType); - if (schedule == null) { - return backupScheduleDao.persist(new BackupScheduleVO(vmId, intervalType, scheduleString, timezoneId, nextDateTime, maxBackups, cmd.getQuiesceVM(), vm.getAccountId(), + return backupScheduleDao.persist(new BackupScheduleVO(vmId, intervalType, scheduleString, timezoneId, nextDateTime, maxBackups, cmd.isQuiesceVM(), vm.getAccountId(), vm.getDomainId(), isolated)); + } + + @Override + @ActionEvent(eventType = EventTypes.EVENT_VM_BACKUP_SCHEDULE_CONFIGURE, eventDescription = "updating VM backup schedule") + public BackupSchedule configureBackupSchedule(UpdateBackupScheduleCmd cmd) { + BackupScheduleVO schedule; + DateUtil.IntervalType intervalType = cmd.getIntervalType(); + Long vmId = cmd.getVmId(); + VMInstanceVO vm; + if (cmd.getId() != null) { + schedule = backupScheduleDao.findById(cmd.getId()); + } else if (vmId != null && intervalType != null) { + List schedules = backupScheduleDao.listByVMAndIntervalType(vmId, intervalType); + if (schedules.size() > 1) { + vm = findVmById(vmId); + throw new InvalidParameterValueException(String.format("There is more than one schedule with type [%s] for VM [%s]. Please inform the schedule ID to update.", + intervalType, vm.getUuid())); + } + if (schedules.isEmpty()) { + return configureBackupSchedule(new CreateBackupScheduleCmd(cmd)); + } + schedule = schedules.get(0); + } else { + throw new InvalidParameterValueException("You must either inform the schedule ID to update, or the VM ID and interval type."); + } + + vm = findVmById(schedule.getVmId()); + validateBackupForZone(vm.getDataCenterId()); + accountManager.checkAccess(CallContext.current().getCallingAccount(), null, true, vm); + + final BackupOffering offering = backupOfferingDao.findById(vm.getBackupOfferingId()); + + boolean quiesceVm = ObjectUtils.defaultIfNull(cmd.isQuiesceVm(), Boolean.TRUE.equals(schedule.getQuiesceVM())); + boolean isolated = ObjectUtils.defaultIfNull(cmd.isIsolated(), schedule.isIsolated()); + validateQuiesceAndIsolated(offering, quiesceVm, isolated); + + String timeZoneString = ObjectUtils.defaultIfNull(cmd.getTimezone(), schedule.getTimezone()); + TimeZone timeZone = TimeZone.getTimeZone(timeZoneString); + intervalType = ObjectUtils.defaultIfNull(intervalType, schedule.getScheduleType()); + String scheduleString = ObjectUtils.defaultIfNull(cmd.getSchedule(), schedule.getSchedule()); + + Date nextDateTime; + try { + nextDateTime = DateUtil.getNextRunTime(intervalType, scheduleString, timeZone.getID(), null); + } catch (Exception e) { + throw new InvalidParameterValueException("Invalid schedule: " + cmd.getSchedule() + " for interval type: " + intervalType); + } + + int maxBackups = schedule.getMaxBackups(); + if (cmd.getMaxBackups() != null) { + maxBackups = validateAndGetDefaultBackupRetentionIfRequired(cmd.getMaxBackups(), offering, vm); } schedule.setScheduleType((short) intervalType.ordinal()); schedule.setSchedule(scheduleString); - schedule.setTimezone(timezoneId); + schedule.setTimezone(timeZone.getID()); schedule.setScheduledTimestamp(nextDateTime); schedule.setMaxBackups(maxBackups); - schedule.setQuiesceVM(cmd.getQuiesceVM()); + schedule.setQuiesceVM(quiesceVm); schedule.setIsolated(isolated); backupScheduleDao.update(schedule.getId(), schedule); return backupScheduleDao.findById(schedule.getId()); } + protected void validateQuiesceAndIsolated(BackupOffering offering, boolean isQuiesceVm, boolean isIsolated) { + if (isQuiesceVm && !quiesceSupported.contains(offering.getProvider())) { + throw new InvalidParameterValueException("Quiesce VM option is supported only by NAS and KBOSS backup providers."); + } + + if (isIsolated && !KBOSS_BACKUP_PROVIDER.equals(offering.getProvider())) { + throw new InvalidParameterValueException("Isolated backups are only supported by KBOSS backup provider."); + } + } + /** * Validates the provided backup retention value and returns 0 as the default value if required. * @@ -2545,7 +2613,7 @@ public BackupOffering updateBackupOffering(UpdateBackupOfferingCmd updateBackupO } - if (allowUserDrivenBackups != null){ + if (allowUserDrivenBackups != null) { offering.setUserDrivenBackupAllowed(allowUserDrivenBackups); fields.add("allowUserDrivenBackups: " + allowUserDrivenBackups); } @@ -2571,6 +2639,20 @@ public BackupOffering updateBackupOffering(UpdateBackupOfferingCmd updateBackupO updateBackupOfferingDomainDetails(id, filteredDomainIds, existingDomainIds); } + Map maxSchedules = updateBackupOfferingCmd.getMaxSchedules(); + if (MapUtils.isNotEmpty(maxSchedules)) { + for (String intervalTypeString : maxSchedules.keySet()) { + DateUtil.IntervalType intervalType = DateUtil.IntervalType.getIntervalType(intervalTypeString); + if (intervalType == null) { + String message = String.format("Received invalid interval type name. Accepted values are [%s].", List.of(DateUtil.IntervalType.values())); + logger.warn(message); + throw new InvalidParameterValueException(message); + } + Integer maxAmount = Integer.valueOf(maxSchedules.get(intervalTypeString)); + backupOfferingDetailsDao.addDetail(id, intervalType.name(), String.valueOf(maxAmount), true); + } + } + BackupOfferingVO response = backupOfferingDao.findById(id); CallContext.current().setEventDetails(String.format("Backup Offering updated [%s].", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(response, "id", "name", "description", "userDrivenBackupAllowed", "externalId"))); @@ -2623,6 +2705,46 @@ Map getDetailsFromBackupDetails(Long backupId) { return details; } + private Map parseIntervalTypeToMaxAmountMap(Map maxSchedules) { + Map intervalTypeToMaxAmountMap = new HashMap<>(); + if (MapUtils.isEmpty(maxSchedules)) { + return intervalTypeToMaxAmountMap; + } + + for (String intervalTypeString : maxSchedules.keySet()) { + DateUtil.IntervalType intervalType = DateUtil.IntervalType.getIntervalType(intervalTypeString); + if (intervalType == null) { + String message = String.format("Received invalid interval type name. Accepted values are [%s].", List.of(DateUtil.IntervalType.values())); + logger.warn(message); + throw new InvalidParameterValueException(message); + } + Integer maxAmount = Integer.valueOf(maxSchedules.get(intervalTypeString)); + intervalTypeToMaxAmountMap.put(intervalType, maxAmount); + } + return intervalTypeToMaxAmountMap; + } + + protected void validateMaxScheduleForIntervalType(BackupOffering offering, DateUtil.IntervalType intervalType, VMInstanceVO vm) { + BackupOfferingDetailsVO maxScheduleForType = backupOfferingDetailsDao.findDetail(offering.getId(), intervalType.name()); + if (maxScheduleForType == null || Integer.parseInt(maxScheduleForType.getValue()) < 0) { + return; + } + List existingBackupSchedules = backupScheduleDao.listByVMAndIntervalType(vm.getId(), intervalType); + + if (existingBackupSchedules.size() >= Integer.parseInt(maxScheduleForType.getValue())) { + throw new CloudRuntimeException(String.format("VM [%s] already has the max allowed schedules of type [%s] for backup offering [%s].", vm.getUuid(), + intervalType.name(), offering.getUuid())); + } + } + + private void validateDifferentScheduleFromExistingSchedules(VMInstanceVO vm, DateUtil.IntervalType intervalType, String schedule) { + List existingBackupSchedules = backupScheduleDao.listByVMAndIntervalType(vm.getId(), intervalType); + if (existingBackupSchedules.stream().anyMatch(existingSchedule -> existingSchedule.getSchedule().equals(schedule))) { + throw new CloudRuntimeException(String.format("VM [%s] already has a [%s] schedule at [%s]. Cannot have multiple schedules of the same type at the same time.", + vm.getUuid(), intervalType.name(), schedule)); + } + } + @Override public BackupResponse createBackupResponse(Backup backup, Boolean listVmDetails) { VMInstanceVO vm = vmInstanceDao.findByIdIncludingRemoved(backup.getVmId()); 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..603f8a830e1f 100644 --- a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java +++ b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java @@ -53,6 +53,7 @@ import org.apache.cloudstack.api.command.user.backup.DeleteBackupScheduleCmd; import org.apache.cloudstack.api.command.user.backup.ListBackupOfferingsCmd; import org.apache.cloudstack.api.command.user.backup.ListBackupScheduleCmd; +import org.apache.cloudstack.api.command.user.backup.UpdateBackupScheduleCmd; import org.apache.cloudstack.api.response.BackupResponse; import org.apache.cloudstack.backup.dao.BackupDao; import org.apache.cloudstack.backup.dao.BackupDetailsDao; @@ -609,13 +610,14 @@ public void testConfigureBackupSchedule() { Long domainId = 4L; Long backupOfferingId = 5L; - CreateBackupScheduleCmd cmd = Mockito.mock(CreateBackupScheduleCmd.class); + UpdateBackupScheduleCmd cmd = Mockito.mock(UpdateBackupScheduleCmd.class); when(cmd.getVmId()).thenReturn(vmId); when(cmd.getTimezone()).thenReturn("GMT"); when(cmd.getIntervalType()).thenReturn(DateUtil.IntervalType.DAILY); when(cmd.getMaxBackups()).thenReturn(8); when(cmd.getSchedule()).thenReturn("00:00:00"); - when(cmd.getQuiesceVM()).thenReturn(null); + when(cmd.isQuiesceVm()).thenReturn(null); + doReturn(null).when(cmd).getId(); VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); when(vmInstanceDao.findById(vmId)).thenReturn(vm); @@ -635,11 +637,11 @@ public void testConfigureBackupSchedule() { BackupOfferingVO offering = Mockito.mock(BackupOfferingVO.class); when(backupOfferingDao.findById(backupOfferingId)).thenReturn(offering); - when(offering.isUserDrivenBackupAllowed()).thenReturn(true); when(offering.getProvider()).thenReturn("test"); BackupScheduleVO schedule = mock(BackupScheduleVO.class); - when(backupScheduleDao.findByVMAndIntervalType(vmId, DateUtil.IntervalType.DAILY)).thenReturn(schedule); + doReturn(vmId).when(schedule).getVmId(); + when(backupScheduleDao.listByVMAndIntervalType(vmId, DateUtil.IntervalType.DAILY)).thenReturn(List.of(schedule)); backupManager.configureBackupSchedule(cmd); diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 775de26103a0..7c0a2b280e5d 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -478,6 +478,7 @@ "label.backingup": "BackingUp", "label.backup.isolated": "Isolated", "label.backup.attach.restore": "Restore and attach backup volume", +"label.backup.edit.schedule": "Edit Backup Schedule", "label.backup.configure.schedule": "Configure Backup Schedule", "label.backup.chain.finish": "Finish backup chain", "label.backupchainsize": "Backup chain size", @@ -1058,6 +1059,7 @@ "label.edit.acl.rule": "Edit ACL rule", "label.edit.autoscale.vmprofile": "Edit AutoScale Instance Profile", "label.edit.nic": "Edit NIC", +"label.edit.max.schedules": "Edit maximum backup schedules", "label.edit.project.details": "Edit project details", "label.edit.project.role": "Edit project role", "label.edit.role": "Edit Role", @@ -1685,6 +1687,7 @@ "label.maxpublicip": "Max. public IPs", "label.maxresolutionx": "Max. resolution X", "label.maxresolutiony": "Max. resolution Y", +"label.maxschedules": "Max backup schedules", "label.maxsecondarystorage": "Max. secondary storage (GiB)", "label.maxsize": "Maximum size", "label.maxsnaps": "Max. Snapshots", diff --git a/ui/public/locales/pt_BR.json b/ui/public/locales/pt_BR.json index b3eae6eb11ce..380a8775fcd5 100644 --- a/ui/public/locales/pt_BR.json +++ b/ui/public/locales/pt_BR.json @@ -441,6 +441,7 @@ "label.backedup": "Salvo", "label.backingup": "Salvando", "label.backup.attach.restore": "Restaurar e anexar volume de backup", +"label.backup.edit.schedule": "Editar pol\u00edtica de backup", "label.backuplimit": "Limite de backups", "label.backup.storage": "Armazenamento de backup", "label.backupstoragelimit": "Limite de armazenamento de backup (GiB)", @@ -936,6 +937,7 @@ "label.edit.acl.rule": "Editar regra ACL", "label.edit.autoscale.vmprofile": "Editar Perfil de Inst\u00e2ncia de AutoScale", "label.edit.nic": "Editar NIC", +"label.edit.max.schedules": "Editar n\u00famero m\u00e1ximo de pol\u00edticas de backup", "label.edit.project.details": "Editar detalhes do projeto", "label.edit.project.role": "Editar fun\u00e7\u00e3o do projeto", "label.edit.role": "Editar fun\u00e7\u00e3o", @@ -1444,6 +1446,7 @@ "label.managementservers": "N\u00famero de servidores de ger\u00eancia", "label.matchall": "Corresponder a todos", "label.max": "M\u00e1x.", +"label.maxschedules": "M\u00e1x. de política de Backup ", "label.max.migrations": "M\u00e1x. de migra\u00e7\u00f5es", "label.maxbackup": "M\u00e1x. de Backups", "label.maxbackupstorage": "M\u00e1x. de Armazenamento de Backup (GiB)", diff --git a/ui/src/config/section/offering.js b/ui/src/config/section/offering.js index afdf2605a6a8..10272f70c84d 100644 --- a/ui/src/config/section/offering.js +++ b/ui/src/config/section/offering.js @@ -406,8 +406,7 @@ export default { label: 'label.edit', dataView: true, popup: true, - groupMap: (selection) => { return selection.map(x => { return { id: x } }) }, - args: ['name', 'description', 'allowuserdrivenbackups'] + component: shallowRef(defineAsyncComponent(() => import('@/views/offering/UpdateBackupOffering.vue'))) }, { api: 'cloneBackupOffering', icon: 'copy-outlined', diff --git a/ui/src/utils/plugins.js b/ui/src/utils/plugins.js index b8856c7c9b00..54d2f1158566 100644 --- a/ui/src/utils/plugins.js +++ b/ui/src/utils/plugins.js @@ -617,7 +617,7 @@ export const backupUtilPlugin = { if (!provider && typeof provider !== 'string') { return false } - return ['nas'].includes(provider.toLowerCase()) + return ['nas', 'kboss'].includes(provider.toLowerCase()) } } } diff --git a/ui/src/views/compute/BackupScheduleWizard.vue b/ui/src/views/compute/BackupScheduleWizard.vue index 16edb9f5e448..3f70db71d01e 100644 --- a/ui/src/views/compute/BackupScheduleWizard.vue +++ b/ui/src/views/compute/BackupScheduleWizard.vue @@ -102,10 +102,10 @@ export default { diff --git a/ui/src/views/compute/backup/BackupSchedule.vue b/ui/src/views/compute/backup/BackupSchedule.vue index 526cd03b7ee4..07fa211a931f 100644 --- a/ui/src/views/compute/backup/BackupSchedule.vue +++ b/ui/src/views/compute/backup/BackupSchedule.vue @@ -71,6 +71,16 @@ + + From e3a7b32bb707f2e7d5ac8a380eb6888afaec6ed2 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 13:05:18 -0300 Subject: [PATCH 2/3] partial fix --- .../backup/KbossBackupProvider.java | 141 ++++++++++++++---- .../backup/KbossBackupProviderTest.java | 24 +-- .../LibvirtTakeKbossBackupCommandWrapper.java | 48 +++--- 3 files changed, 144 insertions(+), 69 deletions(-) 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 27f4d9b343e6..33dddc68fb4d 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 @@ -40,6 +40,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.TreeSet; import java.util.UUID; import java.util.concurrent.ExecutionException; import java.util.stream.Collectors; @@ -131,7 +132,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; @@ -463,7 +463,7 @@ public Pair orchestrateTakeBackup(Backup backup, boolean quiesceV KbossTO kbossTO = new KbossTO(volumeObjectTO, volumeIdToSnapshotDataStoreAndBackupPathList.getOrDefault(volumeObjectTO.getId(), new LinkedList<>())); kbossTOs.add(kbossTO); createDeltaReferences(fullBackup, runningVm, backup, parentBackupDeltasOnSecondary, - parentBackupDeltasOnPrimary, volumeUuidToDeltaPrimaryRef, volumeUuidToDeltaSecondaryRef, succeedingVmSnapshot, kbossTO); + parentBackupDeltasOnPrimary, volumeUuidToDeltaPrimaryRef, volumeUuidToDeltaSecondaryRef, succeedingVmSnapshot, succeedingBackup, kbossTO); } TakeKbossBackupCommand command = new TakeKbossBackupCommand(quiesceVm, runningVm, newBackupJoin.getEndOfChain(), userVm.getInstanceName(), imageStore.getUri(), @@ -1590,7 +1590,7 @@ protected boolean deleteFailedBackup(BackupVO backupVO) { protected void createDeltaReferences(boolean fullBackup, boolean runningVm, Backup backup, List parentBackupDeltasOnSecondary, List parentBackupDeltasOnPrimary, HashMap volumeUuidToDeltaPrimaryRef, HashMap volumeUuidToDeltaSecondaryRef, - VMSnapshotVO succeedingVmSnapshot, KbossTO kbossTO) { + VMSnapshotVO succeedingVmSnapshot, InternalBackupJoinVO succeedingBackup, KbossTO kbossTO) { VolumeObjectTO volumeObjectTO = kbossTO.getVolumeObjectTO(); logger.debug("Creating delta references for backup [{}] of volume [{}].", backup.getUuid(), volumeObjectTO.getUuid()); @@ -1603,7 +1603,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<>()); + succeedingBackup); findAndSetParentBackupPath(parentBackupDeltasOnSecondary, parentDeltaOnPrimary, kbossTO); } @@ -1639,7 +1639,7 @@ protected void mergeCurrentDeltasIntoVolume(Volume volume, VirtualMachine virtua 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<>()); + DeltaMergeTreeTO deltaMergeTreeTO = createDeltaMergeTree(true, isVmRunning, delta, (VolumeObjectTO)volumeObject.getTO(), null, null); MergeDiskOnlyVmSnapshotCommand cmd = new MergeDiskOnlyVmSnapshotCommand(List.of(deltaMergeTreeTO), isVmRunning, virtualMachine.getInstanceName()); Answer answer = sendBackupCommand(vmSnapshotHelper.pickRunningHost(virtualMachine.getId()), cmd); @@ -1760,36 +1760,83 @@ protected Map> mapVolumesToVmSnapshotAndBackupReference return volumeToSnapshotAndBackupRefs; } - List> volumeIdAndResourcePathAndCreatedDateList = new ArrayList<>(); - for (InternalBackupJoinVO internalBackupJoinVO : internalBackupJoinVOList) { - volumeIdAndResourcePathAndCreatedDateList.add(new Ternary<>(internalBackupJoinVO.getVolumeId(), internalBackupJoinVO.getStoragePoolDeltaPath(), internalBackupJoinVO.getDate())); - } + TreeSet volumeIdAndResourcePathAndCreatedDateList = new TreeSet<>(Comparator.comparing(DeltaInfo::getCreated)); for (VMSnapshotVO vmSnapshotVO : vmSnapshotVOList) { vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(vmSnapshotVO.getId()) - .forEach(snapshotDataStoreVO -> volumeIdAndResourcePathAndCreatedDateList.add(new Ternary<>(snapshotDataStoreVO.getVolumeId(), snapshotDataStoreVO.getInstallPath(), snapshotDataStoreVO.getCreated()))); + .forEach(snapshotDataStoreVO -> + volumeIdAndResourcePathAndCreatedDateList.add(new DeltaInfo(snapshotDataStoreVO.getCreated().getTime() + 1, snapshotDataStoreVO.getVolumeId(), + snapshotDataStoreVO.getInstallPath(), true))); } - volumeIdAndResourcePathAndCreatedDateList.sort(Comparator.comparing(Ternary::third)); + internalBackupJoinVOList.sort(Comparator.comparing(InternalBackupJoinVO::getDate)); + for (InternalBackupJoinVO internalBackupJoinVO : internalBackupJoinVOList) { + DeltaInfo delta = new DeltaInfo(internalBackupJoinVO.getDate().getTime() + 1, internalBackupJoinVO.getVolumeId(), internalBackupJoinVO.getStoragePoolDeltaPath(), + false); + volumeIdAndResourcePathAndCreatedDateList.add(delta); + addParentDeltaIfPreviousDeltaIsSnapshot(internalBackupJoinVO, volumeIdAndResourcePathAndCreatedDateList, delta); + } - for (Ternary volumeIdAndResourcePathAndCreatedDate : volumeIdAndResourcePathAndCreatedDateList) { - long volumeId = volumeIdAndResourcePathAndCreatedDate.first(); - String resourcePath = volumeIdAndResourcePathAndCreatedDate.second(); + for (DeltaInfo volumeIdAndResourcePathAndCreatedDate : volumeIdAndResourcePathAndCreatedDateList) { + long volumeId = volumeIdAndResourcePathAndCreatedDate.getVolumeId(); + String resourcePath = volumeIdAndResourcePathAndCreatedDate.getPath(); - volumeToSnapshotAndBackupRefs.computeIfAbsent(volumeId, k -> new LinkedList<>()).addLast(resourcePath); + LinkedList deltaPaths = volumeToSnapshotAndBackupRefs.computeIfAbsent(volumeId, k -> new LinkedList<>()); + if (deltaPaths.isEmpty() || !deltaPaths.getLast().equals(resourcePath)) { + deltaPaths.addLast(resourcePath); + } } logger.trace("Given volume objects [{}], VM snapshots [{}] and backups [{}], created the following map [{}].", volumeObjectTOs, vmSnapshotVOList, internalBackupJoinVOList, volumeToSnapshotAndBackupRefs); return volumeToSnapshotAndBackupRefs; } + /** + * If we have the following events:
+ * Create Backup chain B1 -> Create Snapshot S1 -> Create Snapshot S2 -> Create Backup chain B2 + *
+ * The VM chain will look like:
+ * Base volume <- Backup delta 1 <- Snapshot Delta 1 <- Snapshot Delta 2 <- Backup delta 2 + *
+ * In this case, both B1 and S1 point to Backup delta 1, S2 points to Snapshot Delta 1 and B2 points to Backup delta 2. Thus, when adding the reference to B2, we must also add + * its parent, which in this case is Snapshot Delta 2. Otherwise, we would be missing a delta on the chain. + * */ + private void addParentDeltaIfPreviousDeltaIsSnapshot(InternalBackupJoinVO internalBackupJoinVO, TreeSet volumeIdAndResourcePathAndCreatedDateList, DeltaInfo delta) { + DeltaInfo previousDelta = volumeIdAndResourcePathAndCreatedDateList.lower(delta); + if (previousDelta != null && previousDelta.isSnapshot()) { + DeltaInfo missingDelta = new DeltaInfo(internalBackupJoinVO.getDate().getTime(), internalBackupJoinVO.getVolumeId(), + internalBackupJoinVO.getStoragePoolParentPath(), false); + volumeIdAndResourcePathAndCreatedDateList.add(missingDelta); + } + } + + private class DeltaInfo { + private Long created; + private Long volumeId; + private String path; + private boolean isSnapshot; - protected void mapVolumesToSnapshotReferences(List volumeObjectTOs, List snapshotDataStoreVOS, Map> volumeToSnapshotRefs) { - for (VolumeObjectTO volumeObjectTO : volumeObjectTOs) { - List associatedSnapshots = snapshotDataStoreVOS.stream() - .filter(snapRef -> Objects.equals(snapRef.getVolumeId(), volumeObjectTO.getVolumeId())) - .collect(Collectors.toList()); - volumeToSnapshotRefs.put(volumeObjectTO.getId(), associatedSnapshots); + private DeltaInfo(Long created, Long volumeId, String path, boolean isSnapshot) { + this.created = created; + this.volumeId = volumeId; + this.path = path; + this.isSnapshot = isSnapshot; + } + + private Long getCreated() { + return created; + } + + private Long getVolumeId() { + return volumeId; + } + + private String getPath() { + return path; + } + + private boolean isSnapshot() { + return isSnapshot; } } @@ -1887,7 +1934,7 @@ protected void expungeOldDeltasAndUpdateVmSnapshotOrBackup(List deltasOnPrimary, VMSnapshotVO succeedingVmSnapshot, - KbossTO kbossTO, List succeedingBackupList) { + KbossTO kbossTO, InternalBackupJoinVO succeedingBackup) { VolumeObjectTO volumeObjectTO = kbossTO.getVolumeObjectTO(); InternalBackupStoragePoolVO deltaOnPrimary = deltasOnPrimary.stream() @@ -1901,12 +1948,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, succeedingBackup)); return deltaOnPrimary; } protected DeltaMergeTreeTO createDeltaMergeTree(boolean childIsVolume, boolean runningVm, InternalBackupStoragePoolVO deltaOnPrimary, - VolumeObjectTO volumeObjectTO, VMSnapshotVO succeedingVmSnapshot, List succeedingBackupsList) { + VolumeObjectTO volumeObjectTO, VMSnapshotVO succeedingVmSnapshot, InternalBackupJoinVO succeedingBackup) { DataStore store = dataStoreManager.getDataStore(deltaOnPrimary.getStoragePoolId(), DataStoreRole.Primary); DataTO deltaChild; if (childIsVolume) { @@ -1916,11 +1963,14 @@ 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<>(); + succeedingBackup = filterNonImmediateSucceedingBackup(deltaOnPrimary, succeedingBackup); + succeedingVmSnapshot = filterNonImmediateSucceedingSnapshot(deltaOnPrimary, succeedingVmSnapshot); + 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) + if (succeedingVmSnapshot != null || succeedingBackup != null) { + succeedingDeltaPaths = mapVolumesToVmSnapshotAndBackupReferences(List.of(volumeObjectTO), succeedingSnapshotList, + succeedingBackup == null ? new ArrayList<>() : Arrays.asList(succeedingBackup)) .getOrDefault(volumeObjectTO.getVolumeId(), new LinkedList<>()); if (!childIsVolume && !runningVm && succeedingDeltaPaths.isEmpty()) { @@ -1930,6 +1980,7 @@ protected DeltaMergeTreeTO createDeltaMergeTree(boolean childIsVolume, boolean r } } + succeedingDeltaPaths = new ArrayList<>(new HashSet<>(succeedingDeltaPaths)); List deltaGrandchildren = succeedingDeltaPaths.stream() .map(deltaPath -> new BackupDeltaTO(store.getTO(), Hypervisor.HypervisorType.KVM, deltaPath)) .collect(Collectors.toList()); @@ -1940,6 +1991,30 @@ protected DeltaMergeTreeTO createDeltaMergeTree(boolean childIsVolume, boolean r return deltaMergeTreeTO; } + /** + * Returns the succeedingBackup if it is the immediate child of the deltaOnPrimary. Otherwise, returns null. + * */ + private InternalBackupJoinVO filterNonImmediateSucceedingBackup(InternalBackupStoragePoolVO deltaOnPrimary, InternalBackupJoinVO succeedingBackup) { + if (succeedingBackup == null) { + return succeedingBackup; + } + return StringUtils.equals(succeedingBackup.getStoragePoolParentPath(), deltaOnPrimary.getBackupDeltaPath()) ? succeedingBackup : null; + } + + /** + * Returns the succeedingVmSnapshot if it is the immediate child of the deltaOnPrimary. Otherwise, returns null. + * */ + private VMSnapshotVO filterNonImmediateSucceedingSnapshot(InternalBackupStoragePoolVO deltaOnPrimary, VMSnapshotVO succeedingVmSnapshot) { + if (succeedingVmSnapshot == null) { + return succeedingVmSnapshot; + } + if (vmSnapshotHelper.getVolumeSnapshotsAssociatedWithKvmDiskOnlyVmSnapshot(succeedingVmSnapshot.getId()) + .stream().anyMatch(snapshotDelta -> snapshotDelta.getInstallPath().equals(deltaOnPrimary.getBackupDeltaPath()))) { + return succeedingVmSnapshot; + } + return null; + } + /** * Sets on the {@code kbossTO} the backupParentOnSecondary path based on the list of InternalBackupDataStoreVO. * @@ -2070,7 +2145,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, null)); continue; } @@ -2421,7 +2496,7 @@ protected boolean mergeCurrentBackupDeltas(InternalBackupJoinVO backupJoinVO) { 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, succeedingBackup); Long hostId = vmSnapshotHelper.pickRunningHost(backupJoinVO.getVmId()); Answer answer = sendBackupCommand(hostId, cmd); @@ -2460,7 +2535,7 @@ protected void createDeleteCommandsAndMergeTrees(List volumeObje 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, null)); } } } @@ -2495,7 +2570,7 @@ protected Pair, InternalBackupJoinVO> getParentsToBeE } private MergeDiskOnlyVmSnapshotCommand buildMergeDiskOnlyVmSnapshotCommandForCurrentBackup(InternalBackupJoinVO backupJoinVO, VirtualMachine userVm, VMSnapshotVO vmSnapshot, - List succeedingBackupList) { + InternalBackupJoinVO succeedingBackup) { List deltaMergeTreeTOs = new ArrayList<>(); List volumeTOs = vmSnapshotHelper.getVolumeTOList(backupJoinVO.getVmId()); @@ -2503,8 +2578,8 @@ private MergeDiskOnlyVmSnapshotCommand buildMergeDiskOnlyVmSnapshotCommandForCur 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); + boolean childIsVolume = vmSnapshot == null && succeedingBackup == null; + createDeltaMergeTreeForVolume(childIsVolume, userVm.getState() == VirtualMachine.State.Running, deltasOnPrimary, vmSnapshot, kbossTO, succeedingBackup); if (kbossTO.getDeltaMergeTreeTO() != null) { deltaMergeTreeTOs.add(kbossTO.getDeltaMergeTreeTO()); } else { 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 79276bb96e22..d6f7fd8d7fa9 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 @@ -669,7 +669,7 @@ public void mapVolumesToVmSnapshotAndBackupReferencesTestVmSnapshotAndBackupVOLi 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, + kbossBackupProviderSpy.createDeltaReferences(true, true, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, null, new KbossTO(volumeObjectToMock, new LinkedList<>())); verify(internalBackupDataStoreDaoMock, Mockito.times(1)).persist(any()); @@ -679,7 +679,7 @@ 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, + kbossBackupProviderSpy.createDeltaReferences(true, true, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, null, new KbossTO(volumeObjectToMock, new LinkedList<>())); verify(internalBackupDataStoreDaoMock, Mockito.times(1)).persist(any()); @@ -692,10 +692,10 @@ public void createDeltaReferencesTestIsolatedBackup() { 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()); + doReturn(null).when(kbossBackupProviderSpy).createDeltaMergeTreeForVolume(false, true, List.of(), null, kbossTO, null); 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, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, null, kbossTO); verify(internalBackupDataStoreDaoMock, Mockito.times(1)).persist(any()); verify(kbossBackupProviderSpy, Mockito.times(1)).findAndSetParentBackupPath(List.of(), null, kbossTO); @@ -705,7 +705,7 @@ 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, + kbossBackupProviderSpy.createDeltaReferences(true, true, backupVoMock, List.of(), List.of(), new HashMap<>(), new HashMap<>(), null, null, new KbossTO(volumeObjectToMock, new LinkedList<>())); verify(internalBackupDataStoreDaoMock, Mockito.times(1)).persist(any()); @@ -769,7 +769,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(), any(), any(), any(), any(), any(), any(), null, any()); verify(kbossBackupProviderSpy, Mockito.times(1)).processBackupFailure(any(), any(), Mockito.anyLong(), Mockito.anyBoolean(), any()); } @@ -800,7 +800,7 @@ 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(), any(), any(), any(), any(), any(), any(), null, any()); verify(kbossBackupProviderSpy, Mockito.times(1)).processBackupSuccess(anyBoolean(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), any(), anyLong(), anyBoolean(), anyBoolean(), any()); verify(kbossBackupProviderSpy, Mockito.times(1)).compressBackupAsync(internalBackupJoinVoMock, 0, 0); @@ -836,7 +836,7 @@ 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(), any(), any(), any(), any(), any(), any(), null, any()); verify(kbossBackupProviderSpy, Mockito.times(1)).processBackupSuccess(anyBoolean(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), any(), anyLong(), anyBoolean(), anyBoolean(), any()); verify(kbossBackupProviderSpy, Mockito.times(1)).validateBackupAsyncIfHasOfferingSupport(internalBackupJoinVoMock, 0, 0); @@ -2183,7 +2183,7 @@ public void createDeltaMergeTreeTestChildIsDeltaWithSucceedingSnapshotReferences doReturn("path").when(volumeObjectToMock).getPath(); DeltaMergeTreeTO result = kbossBackupProviderSpy.createDeltaMergeTree(false, false, internalBackupStoragePoolVoMock, - volumeObjectToMock, vmSnapshotVoMock, List.of()); + volumeObjectToMock, vmSnapshotVoMock, null); assertEquals("child-path", result.getChild().getPath()); assertEquals(1, result.getGrandChildren().size()); @@ -2198,7 +2198,7 @@ public void createDeltaMergeTreeTestChildIsDeltaWithSucceedingSnapshotButNoRefer doReturn("/volume/path").when(volumeObjectToMock).getPath(); DeltaMergeTreeTO result = kbossBackupProviderSpy.createDeltaMergeTree(false, false, internalBackupStoragePoolVoMock, - volumeObjectToMock, vmSnapshotVoMock, List.of()); + volumeObjectToMock, vmSnapshotVoMock, null); assertEquals(1, result.getGrandChildren().size()); assertEquals("/volume/path", result.getGrandChildren().get(0).getPath()); @@ -2276,7 +2276,7 @@ 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<>())); + eq(null)); List result = kbossBackupProviderSpy.populateDeltasToRemoveAndToMergeAndUpdateVolumePaths(List.of(internalBackupStoragePoolVoMock), deltasToRemove, List.of(volumeObjectToMock), List.of(volumeObjectToMock), "vm-uuid"); @@ -2284,7 +2284,7 @@ public void populateDeltasToRemoveAndToMergeAndUpdateVolumePathsTestVolumeIsPart 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<>())); + eq(null)); verify(dataStoreManagerMock, never()).getDataStore(anyLong(), eq(DataStoreRole.Primary)); } 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..d23699ac2b80 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 @@ -19,15 +19,18 @@ package com.cloud.hypervisor.kvm.resource.wrapper; -import com.cloud.agent.api.Answer; -import com.cloud.hypervisor.Hypervisor; -import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; -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.BackupException; +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.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.TimeoutException; +import java.util.stream.Collectors; + import org.apache.cloudstack.backup.TakeKbossBackupAnswer; import org.apache.cloudstack.backup.TakeKbossBackupCommand; import org.apache.cloudstack.storage.to.BackupDeltaTO; @@ -40,20 +43,17 @@ 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; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.UUID; -import java.util.concurrent.TimeoutException; -import java.util.stream.Collectors; +import com.cloud.agent.api.Answer; +import com.cloud.hypervisor.Hypervisor; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +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.BackupException; @ResourceWrapper(handles = TakeKbossBackupCommand.class) public class LibvirtTakeKbossBackupCommandWrapper extends CommandWrapper { @@ -144,8 +144,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 deltasSucceedingLastBackupInChain = kbossTO.getDeltaPaths(); + mergeBackupDelta(resource, deltaMergeTreeTO, volumeObjectTO, vmName, runningVM, volumeUuid, CollectionUtils.isEmpty(deltasSucceedingLastBackupInChain)); } if (command.isEndChain() || command.isIsolated()) { @@ -169,7 +169,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.getDeltaPaths()) ? new ArrayList<>() : new ArrayList<>(kbossTO.getDeltaPaths()); deltaPathsToCopy.add(delta.getPath()); KVMStoragePool parentImagePool = null; From 65a443c50e92a36acdbaa146e7033eabfa2003ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Wed, 16 Sep 2026 15:53:14 -0300 Subject: [PATCH 3/3] small fixes --- .../apache/cloudstack/backup/BackupManagerImpl.java | 11 ++++++++--- ui/src/views/compute/backup/FormSchedule.vue | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) 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 ef502d2a2b2d..0f3357cc540a 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -842,7 +842,7 @@ public BackupSchedule configureBackupSchedule(CreateBackupScheduleCmd cmd) { validateQuiesceAndIsolated(offering, quiesceVm, isolated); validateMaxScheduleForIntervalType(offering, intervalType, vm); - validateDifferentScheduleFromExistingSchedules(vm, intervalType, scheduleString); + validateDifferentScheduleFromExistingSchedules(vm, intervalType, scheduleString, null); final String timezoneId = timeZone.getID(); if (!timezoneId.equals(cmd.getTimezone())) { @@ -894,10 +894,12 @@ public BackupSchedule configureBackupSchedule(UpdateBackupScheduleCmd cmd) { boolean isolated = ObjectUtils.defaultIfNull(cmd.isIsolated(), schedule.isIsolated()); validateQuiesceAndIsolated(offering, quiesceVm, isolated); + String scheduleString = ObjectUtils.defaultIfNull(cmd.getSchedule(), schedule.getSchedule()); + validateDifferentScheduleFromExistingSchedules(vm, intervalType, scheduleString, schedule); + String timeZoneString = ObjectUtils.defaultIfNull(cmd.getTimezone(), schedule.getTimezone()); TimeZone timeZone = TimeZone.getTimeZone(timeZoneString); intervalType = ObjectUtils.defaultIfNull(intervalType, schedule.getScheduleType()); - String scheduleString = ObjectUtils.defaultIfNull(cmd.getSchedule(), schedule.getSchedule()); Date nextDateTime; try { @@ -2708,8 +2710,11 @@ protected void validateMaxScheduleForIntervalType(BackupOffering offering, DateU } } - private void validateDifferentScheduleFromExistingSchedules(VMInstanceVO vm, DateUtil.IntervalType intervalType, String schedule) { + private void validateDifferentScheduleFromExistingSchedules(VMInstanceVO vm, DateUtil.IntervalType intervalType, String schedule, BackupScheduleVO scheduleBeingValidated) { List existingBackupSchedules = backupScheduleDao.listByVMAndIntervalType(vm.getId(), intervalType); + if (scheduleBeingValidated != null) { + existingBackupSchedules.removeIf(sched -> sched.getId() == scheduleBeingValidated.getId()); + } if (existingBackupSchedules.stream().anyMatch(existingSchedule -> existingSchedule.getSchedule().equals(schedule))) { throw new CloudRuntimeException(String.format("VM [%s] already has a [%s] schedule at [%s]. Cannot have multiple schedules of the same type at the same time.", vm.getUuid(), intervalType.name(), schedule)); diff --git a/ui/src/views/compute/backup/FormSchedule.vue b/ui/src/views/compute/backup/FormSchedule.vue index 2254c6920770..f50a71b3fc76 100644 --- a/ui/src/views/compute/backup/FormSchedule.vue +++ b/ui/src/views/compute/backup/FormSchedule.vue @@ -288,7 +288,7 @@ export default { initForm () { this.formRef = ref() this.form = reactive({ - intervaltype: 'hourly', + intervaltype: this.scheduleToEdit?.intervaltype?.toLowerCase() ?? 'hourly', isolated: false }) this.rules = reactive({