Skip to content

Commit caf70f3

Browse files
committed
Keep maxbackups as null for Veeam where retention is enforced by Veeam.
1 parent 900d939 commit caf70f3

File tree

7 files changed

+25
-17
lines changed

7 files changed

+25
-17
lines changed

api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupScheduleCmd.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@
3636
import com.cloud.utils.DateUtil;
3737
import com.cloud.utils.exception.CloudRuntimeException;
3838

39-
import java.util.Objects;
40-
4139
@APICommand(name = "createBackupSchedule",
4240
description = "Creates a user-defined VM backup schedule",
4341
responseObject = BackupResponse.class, since = "4.14.0",
@@ -79,7 +77,6 @@ public class CreateBackupScheduleCmd extends BaseCmd {
7977

8078
@Parameter(name = ApiConstants.MAX_BACKUPS,
8179
type = CommandType.INTEGER,
82-
required = true,
8380
description = "maximum number of backups to retain",
8481
since = "4.21.0")
8582
private Integer maxBackups;
@@ -104,7 +101,9 @@ public String getTimezone() {
104101
return timezone;
105102
}
106103

107-
public Integer getMaxBackups() { return Objects.nonNull(maxBackups) ? maxBackups : 0; }
104+
public Integer getMaxBackups() {
105+
return maxBackups;
106+
}
108107

109108
/////////////////////////////////////////////////////
110109
/////////////// API Implementation///////////////////

api/src/main/java/org/apache/cloudstack/backup/BackupSchedule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,5 @@ public interface BackupSchedule extends InternalIdentity {
3030
String getTimezone();
3131
Date getScheduledTimestamp();
3232
Long getAsyncJobId();
33-
int getMaxBackups();
33+
Integer getMaxBackups();
3434
}

engine/schema/src/main/java/org/apache/cloudstack/backup/BackupScheduleVO.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public class BackupScheduleVO implements BackupSchedule {
5858
Long asyncJobId;
5959

6060
@Column(name = "max_backups")
61-
int maxBackups = 0;
61+
Integer maxBackups = 0;
6262

6363
public BackupScheduleVO() {
6464
}
@@ -125,11 +125,11 @@ public void setAsyncJobId(Long asyncJobId) {
125125
this.asyncJobId = asyncJobId;
126126
}
127127

128-
public int getMaxBackups() {
128+
public Integer getMaxBackups() {
129129
return maxBackups;
130130
}
131131

132-
public void setMaxBackups(int maxBackups) {
132+
public void setMaxBackups(Integer maxBackups) {
133133
this.maxBackups = maxBackups;
134134
}
135135
}

engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ CREATE TABLE `cloud`.`backup_repository` (
412412
ALTER TABLE `cloud`.`backup_schedule` DROP FOREIGN KEY fk_backup_schedule__vm_id;
413413
ALTER TABLE `cloud`.`backup_schedule` DROP INDEX vm_id;
414414
ALTER TABLE `cloud`.`backup_schedule` ADD CONSTRAINT fk_backup_schedule__vm_id FOREIGN KEY (vm_id) REFERENCES vm_instance(id) ON DELETE CASCADE;
415-
ALTER TABLE `cloud`.`backup_schedule` ADD COLUMN `max_backups` int(8) NOT NULL default 0 COMMENT 'maximum number of backups to maintain';
415+
ALTER TABLE `cloud`.`backup_schedule` ADD COLUMN `max_backups` int(8) default NULL COMMENT 'maximum number of backups to maintain';
416416
ALTER TABLE `cloud`.`backups` ADD COLUMN `backup_interval_type` int(5) COMMENT 'type of backup, e.g. manual, recurring - hourly, daily, weekly or monthly';
417417

418418
-- Add volume details to the backups table to keep track of the volumes being backed up

server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -443,13 +443,13 @@ public BackupSchedule configureBackupSchedule(CreateBackupScheduleCmd cmd) {
443443
if (vm.getBackupOfferingId() == null) {
444444
throw new CloudRuntimeException("Cannot configure backup schedule for the VM without having any backup offering");
445445
}
446-
if (maxBackups <= 0) {
446+
if (maxBackups != null && maxBackups <= 0) {
447447
throw new InvalidParameterValueException(String.format("maxBackups [%s] for instance %s should be greater than 0.", maxBackups, vm.getName()));
448448
}
449449

450450
Backup.Type backupType = Backup.Type.valueOf(intervalType.name());
451451
int intervalMaxBackups = backupType.getMax();
452-
if (maxBackups > intervalMaxBackups) {
452+
if (maxBackups != null && maxBackups > intervalMaxBackups) {
453453
throw new InvalidParameterValueException(String.format("maxBackups [%s] for instance %s exceeds limit [%s] for interval type [%s].", maxBackups, vm.getName(),
454454
intervalMaxBackups, intervalType));
455455
}
@@ -458,7 +458,7 @@ public BackupSchedule configureBackupSchedule(CreateBackupScheduleCmd cmd) {
458458

459459
long accountLimit = resourceLimitMgr.findCorrectResourceLimitForAccount(owner, Resource.ResourceType.backup, null);
460460
long domainLimit = resourceLimitMgr.findCorrectResourceLimitForDomain(domainManager.getDomain(owner.getDomainId()), Resource.ResourceType.backup, null);
461-
if (!accountManager.isRootAdmin(owner.getId()) && ((accountLimit != -1 && maxBackups > accountLimit) || (domainLimit != -1 && maxBackups > domainLimit))) {
461+
if (maxBackups != null && !accountManager.isRootAdmin(owner.getId()) && ((accountLimit != -1 && maxBackups > accountLimit) || (domainLimit != -1 && maxBackups > domainLimit))) {
462462
String message = "domain/account";
463463
if (owner.getType() == Account.Type.PROJECT) {
464464
message = "domain/project";
@@ -471,6 +471,13 @@ public BackupSchedule configureBackupSchedule(CreateBackupScheduleCmd cmd) {
471471
throw new CloudRuntimeException("The selected backup offering does not allow user-defined backup schedule");
472472
}
473473

474+
if (maxBackups == null && !"veeam".equals(offering.getProvider())) {
475+
throw new CloudRuntimeException("Please specify the maximum number of buckets to retain.");
476+
}
477+
if (maxBackups != null && "veeam".equals(offering.getProvider())) {
478+
throw new CloudRuntimeException("The maximum backups to retain cannot be configured through CloudStack for Veeam. Retention is managed directly in Veeam based on the settings specified when creating the backup job.");
479+
}
480+
474481
final String timezoneId = timeZone.getID();
475482
if (!timezoneId.equals(cmd.getTimezone())) {
476483
logger.warn("Using timezone: " + timezoneId + " for running this snapshot policy as an equivalent of " + cmd.getTimezone());
@@ -535,7 +542,10 @@ private void postCreateScheduledBackup(Backup.Type backupType, Long vmId) {
535542
if (schedule == null) {
536543
return;
537544
}
538-
int maxBackups = schedule.getMaxBackups();
545+
Integer maxBackups = schedule.getMaxBackups();
546+
if (maxBackups == null) {
547+
return;
548+
}
539549
List<BackupVO> backups = backupDao.listBackupsByVMandIntervalType(vmId, backupType);
540550
while (backups.size() > maxBackups) {
541551
BackupVO oldestBackup = backups.get(0);

server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -418,8 +418,8 @@ public void testCreateScheduledBackup() throws ResourceAllocationException {
418418
Long accountId = 5L;
419419
Long backupId = 6L;
420420
Long oldestBackupId = 7L;
421-
Long newBackupSize = 1000000L;
422-
Long oldBackupSize = 9999999L;
421+
Long newBackupSize = 1000000000L;
422+
Long oldBackupSize = 400000000L;
423423

424424
VMInstanceVO vm = Mockito.mock(VMInstanceVO.class);
425425
when(vmInstanceDao.findById(vmId)).thenReturn(vm);
@@ -448,6 +448,7 @@ public void testCreateScheduledBackup() throws ResourceAllocationException {
448448
BackupProvider backupProvider = mock(BackupProvider.class);
449449
Backup backup = mock(Backup.class);
450450
when(backup.getId()).thenReturn(backupId);
451+
when(backup.getProtectedSize()).thenReturn(newBackupSize);
451452
when(backupProvider.getName()).thenReturn("test");
452453
when(backupProvider.takeBackup(vm)).thenReturn(new Pair<>(true, backup));
453454
Map<String, BackupProvider> backupProvidersMap = new HashMap<>();
@@ -456,7 +457,6 @@ public void testCreateScheduledBackup() throws ResourceAllocationException {
456457

457458
BackupVO backupVO = mock(BackupVO.class);
458459
when(backupVO.getId()).thenReturn(backupId);
459-
when(backupVO.getProtectedSize()).thenReturn(newBackupSize);
460460
BackupVO oldestBackupVO = mock(BackupVO.class);
461461
when(oldestBackupVO.getProtectedSize()).thenReturn(oldBackupSize);
462462
when(oldestBackupVO.getId()).thenReturn(oldestBackupId);

ui/src/views/compute/backup/FormSchedule.vue

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,6 @@ export default {
205205
timeSelect: [{ type: 'object', required: true, message: this.$t('message.error.time') }],
206206
'day-of-week': [{ type: 'number', required: true, message: `${this.$t('message.error.select')}` }],
207207
'day-of-month': [{ required: true, message: `${this.$t('message.error.select')}` }],
208-
maxbackups: [{ required: true, message: this.$t('message.error.required.input') }],
209208
timezone: [{ required: true, message: `${this.$t('message.error.select')}` }]
210209
})
211210
},

0 commit comments

Comments
 (0)