Skip to content

Commit c298f8f

Browse files
author
Daan Hoogland
committed
Merge release branch 4.22.0.1 to 4.22
* tag '4.22.0.1': Implement limit validations on updateBucket Address reviews
2 parents 160876c + 2511fdf commit c298f8f

File tree

7 files changed

+49
-64
lines changed

7 files changed

+49
-64
lines changed

‎api/src/main/java/org/apache/cloudstack/api/command/user/bucket/DeleteBucketCmd.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.apache.cloudstack.api.command.user.bucket;
1818

1919
import com.cloud.exception.ConcurrentOperationException;
20+
import com.cloud.exception.ResourceAllocationException;
2021
import org.apache.cloudstack.acl.RoleType;
2122
import org.apache.cloudstack.storage.object.Bucket;
2223
import com.cloud.user.Account;
@@ -82,7 +83,7 @@ public ApiCommandResourceType getApiResourceType() {
8283
}
8384

8485
@Override
85-
public void execute() throws ConcurrentOperationException {
86+
public void execute() throws ConcurrentOperationException, ResourceAllocationException {
8687
CallContext.current().setEventDetails("Bucket Id: " + this._uuidMgr.getUuid(Bucket.class, getId()));
8788
boolean result = _bucketService.deleteBucket(id, CallContext.current().getCallingAccount());
8889
SuccessResponse response = new SuccessResponse(getCommandName());

‎api/src/main/java/org/apache/cloudstack/backup/BackupManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer
226226
* @param forced Indicates if backup will be force removed or not
227227
* @return returns operation success
228228
*/
229-
boolean deleteBackup(final Long backupId, final Boolean forced);
229+
boolean deleteBackup(final Long backupId, final Boolean forced) throws ResourceAllocationException;
230230

231231
void validateBackupForZone(Long zoneId);
232232

‎api/src/main/java/org/apache/cloudstack/storage/object/BucketApiService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public interface BucketApiService {
9595
*/
9696
Bucket createBucket(CreateBucketCmd cmd);
9797

98-
boolean deleteBucket(long bucketId, Account caller);
98+
boolean deleteBucket(long bucketId, Account caller) throws ResourceAllocationException;
9999

100100
boolean updateBucket(UpdateBucketCmd cmd, Account caller) throws ResourceAllocationException;
101101

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

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -841,18 +841,12 @@ private void createCheckedBackup(CreateBackupCmd cmd, Account owner, boolean isS
841841
resourceLimitMgr.incrementResourceCount(vm.getAccountId(), Resource.ResourceType.backup);
842842
resourceLimitMgr.incrementResourceCount(vm.getAccountId(), Resource.ResourceType.backup_storage, backup.getSize());
843843
}
844-
} catch (Exception e) {
845-
if (e instanceof ResourceAllocationException) {
846-
ResourceAllocationException rae = (ResourceAllocationException)e;
847-
if (isScheduledBackup && (Resource.ResourceType.backup.equals(rae.getResourceType()) ||
848-
Resource.ResourceType.backup_storage.equals(rae.getResourceType()))) {
849-
sendExceededBackupLimitAlert(owner.getUuid(), rae.getResourceType());
850-
}
851-
throw rae;
852-
} else if (e instanceof CloudRuntimeException) {
853-
throw (CloudRuntimeException)e;
844+
} catch (ResourceAllocationException e) {
845+
if (isScheduledBackup && (Resource.ResourceType.backup.equals(e.getResourceType()) ||
846+
Resource.ResourceType.backup_storage.equals(e.getResourceType()))) {
847+
sendExceededBackupLimitAlert(owner.getUuid(), e.getResourceType());
854848
}
855-
throw new CloudRuntimeException("Failed to create backup for VM with ID: " + vm.getUuid(), e);
849+
throw e;
856850
}
857851
}
858852

@@ -905,7 +899,7 @@ protected Long getBackupScheduleId(Object job) {
905899
* @param vmId The ID of the VM associated with the backups
906900
* @param backupScheduleId Backup schedule ID of the backups
907901
*/
908-
protected void deleteOldestBackupFromScheduleIfRequired(Long vmId, long backupScheduleId) {
902+
protected void deleteOldestBackupFromScheduleIfRequired(Long vmId, long backupScheduleId) throws ResourceAllocationException {
909903
BackupScheduleVO backupScheduleVO = backupScheduleDao.findById(backupScheduleId);
910904
if (backupScheduleVO == null || backupScheduleVO.getMaxBackups() == 0) {
911905
logger.info("The schedule does not have a retention specified and, hence, not deleting any backups from it.", vmId);
@@ -929,7 +923,7 @@ protected void deleteOldestBackupFromScheduleIfRequired(Long vmId, long backupSc
929923
* @param amountOfBackupsToDelete Number of backups to be deleted from the list of backups
930924
* @param backupScheduleId ID of the backup schedule associated with the backups
931925
*/
932-
protected void deleteExcessBackups(List<BackupVO> backups, int amountOfBackupsToDelete, long backupScheduleId) {
926+
protected void deleteExcessBackups(List<BackupVO> backups, int amountOfBackupsToDelete, long backupScheduleId) throws ResourceAllocationException {
933927
logger.debug("Deleting the [{}] oldest backups from the schedule [ID: {}].", amountOfBackupsToDelete, backupScheduleId);
934928

935929
for (int i = 0; i < amountOfBackupsToDelete; i++) {
@@ -1527,7 +1521,7 @@ protected Pair<Boolean, String> restoreBackedUpVolume(final Backup.VolumeInfo ba
15271521

15281522
@Override
15291523
@ActionEvent(eventType = EventTypes.EVENT_VM_BACKUP_DELETE, eventDescription = "deleting VM backup", async = true)
1530-
public boolean deleteBackup(final Long backupId, final Boolean forced) {
1524+
public boolean deleteBackup(final Long backupId, final Boolean forced) throws ResourceAllocationException {
15311525
final BackupVO backup = backupDao.findByIdIncludingRemoved(backupId);
15321526
if (backup == null) {
15331527
throw new CloudRuntimeException("Backup " + backupId + " does not exist");
@@ -1552,7 +1546,7 @@ public boolean deleteBackup(final Long backupId, final Boolean forced) {
15521546
return deleteCheckedBackup(forced, backupProvider, backup, vm);
15531547
}
15541548

1555-
private boolean deleteCheckedBackup(Boolean forced, BackupProvider backupProvider, BackupVO backup, VMInstanceVO vm) {
1549+
private boolean deleteCheckedBackup(Boolean forced, BackupProvider backupProvider, BackupVO backup, VMInstanceVO vm) throws ResourceAllocationException {
15561550
Account owner = accountManager.getAccount(backup.getAccountId());
15571551
long backupSize = backup.getSize() != null ? backup.getSize() : 0L;
15581552
try (CheckedReservation backupReservation = new CheckedReservation(owner, Resource.ResourceType.backup,
@@ -1572,11 +1566,6 @@ private boolean deleteCheckedBackup(Boolean forced, BackupProvider backupProvide
15721566
}
15731567
}
15741568
throw new CloudRuntimeException("Failed to delete the backup");
1575-
} catch (Exception e) {
1576-
if (e instanceof CloudRuntimeException) {
1577-
throw (CloudRuntimeException) e;
1578-
}
1579-
throw new CloudRuntimeException("Failed to delete the backup due to: " + e.getMessage(), e);
15801569
}
15811570
}
15821571

‎server/src/main/java/org/apache/cloudstack/storage/object/BucketApiServiceImpl.java‎

Lines changed: 29 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,6 @@ private BucketVO createCheckedBucket(CreateBucketCmd cmd, Account owner, long si
163163
(cmd.getQuota() * Resource.ResourceType.bytesToGiB));
164164
}
165165
return bucket;
166-
} catch (Exception e) {
167-
if (e instanceof ResourceAllocationException) {
168-
throw (ResourceAllocationException)e;
169-
} else if (e instanceof CloudRuntimeException) {
170-
throw (CloudRuntimeException)e;
171-
}
172-
throw new CloudRuntimeException(String.format("Failed to create bucket due to: %s", e.getMessage()), e);
173166
}
174167
}
175168

@@ -236,7 +229,7 @@ public Bucket createBucket(CreateBucketCmd cmd) {
236229

237230
@Override
238231
@ActionEvent(eventType = EventTypes.EVENT_BUCKET_DELETE, eventDescription = "deleting bucket")
239-
public boolean deleteBucket(long bucketId, Account caller) {
232+
public boolean deleteBucket(long bucketId, Account caller) throws ResourceAllocationException {
240233
Bucket bucket = _bucketDao.findById(bucketId);
241234
if (bucket == null) {
242235
throw new InvalidParameterValueException("Unable to find bucket with ID: " + bucketId);
@@ -247,7 +240,7 @@ public boolean deleteBucket(long bucketId, Account caller) {
247240
return deleteCheckedBucket(objectStore, bucket, objectStoreVO);
248241
}
249242

250-
private boolean deleteCheckedBucket(ObjectStoreEntity objectStore, Bucket bucket, ObjectStoreVO objectStoreVO) {
243+
private boolean deleteCheckedBucket(ObjectStoreEntity objectStore, Bucket bucket, ObjectStoreVO objectStoreVO) throws ResourceAllocationException {
251244
Account owner = _accountMgr.getAccount(bucket.getAccountId());
252245
try (CheckedReservation bucketReservation = new CheckedReservation(owner, Resource.ResourceType.bucket,
253246
bucket.getId(), null, -1L, reservationDao, resourceLimitManager);
@@ -265,11 +258,6 @@ private boolean deleteCheckedBucket(ObjectStoreEntity objectStore, Bucket bucket
265258
return true;
266259
}
267260
return false;
268-
} catch (Exception e) {
269-
if (e instanceof CloudRuntimeException) {
270-
throw (CloudRuntimeException) e;
271-
}
272-
throw new CloudRuntimeException(String.format("Failed to delete bucket due to: %s", e.getMessage()), e);
273261
}
274262
}
275263

@@ -284,16 +272,6 @@ public boolean updateBucket(UpdateBucketCmd cmd, Account caller) throws Resource
284272
_accountMgr.checkAccess(caller, null, true, bucket);
285273
ObjectStoreVO objectStoreVO = _objectStoreDao.findById(bucket.getObjectStoreId());
286274
ObjectStoreEntity objectStore = (ObjectStoreEntity)_dataStoreMgr.getDataStore(objectStoreVO.getId(), DataStoreRole.Object);
287-
Integer quota = cmd.getQuota();
288-
Integer quotaDelta = null;
289-
290-
if (quota != null) {
291-
quotaDelta = quota - bucket.getQuota();
292-
if (quotaDelta > 0) {
293-
Account owner = _accountMgr.getActiveAccountById(bucket.getAccountId());
294-
resourceLimitManager.checkResourceLimit(owner, Resource.ResourceType.object_storage, (quotaDelta * Resource.ResourceType.bytesToGiB));
295-
}
296-
}
297275

298276
try {
299277
if (cmd.getEncryption() != null) {
@@ -319,16 +297,8 @@ public boolean updateBucket(UpdateBucketCmd cmd, Account caller) throws Resource
319297
bucket.setPolicy(cmd.getPolicy());
320298
}
321299

322-
if (cmd.getQuota() != null) {
323-
objectStore.setQuota(bucketTO, cmd.getQuota());
324-
bucket.setQuota(cmd.getQuota());
325-
if (quotaDelta > 0) {
326-
resourceLimitManager.incrementResourceCount(bucket.getAccountId(), Resource.ResourceType.object_storage, (quotaDelta * Resource.ResourceType.bytesToGiB));
327-
} else {
328-
resourceLimitManager.decrementResourceCount(bucket.getAccountId(), Resource.ResourceType.object_storage, ((-quotaDelta) * Resource.ResourceType.bytesToGiB));
329-
}
330-
_objectStoreDao.updateAllocatedSize(objectStoreVO, (quotaDelta * Resource.ResourceType.bytesToGiB));
331-
}
300+
updateBucketQuota(cmd, bucket, objectStore, objectStoreVO, bucketTO);
301+
332302
_bucketDao.update(bucket.getId(), bucket);
333303
} catch (Exception e) {
334304
throw new CloudRuntimeException("Error while updating bucket: " +bucket.getName() +". "+e.getMessage());
@@ -337,6 +307,31 @@ public boolean updateBucket(UpdateBucketCmd cmd, Account caller) throws Resource
337307
return true;
338308
}
339309

310+
private void updateBucketQuota(UpdateBucketCmd cmd, BucketVO bucket, ObjectStoreEntity objectStore, ObjectStoreVO objectStoreVO, BucketTO bucketTO) throws ResourceAllocationException {
311+
Integer quota = cmd.getQuota();
312+
if (quota == null) {
313+
return;
314+
}
315+
316+
int quotaDelta = quota - bucket.getQuota();
317+
objectStore.setQuota(bucketTO, quota);
318+
bucket.setQuota(quota);
319+
320+
long diff = quotaDelta * Resource.ResourceType.bytesToGiB;
321+
322+
if (quotaDelta < 0) {
323+
resourceLimitManager.decrementResourceCount(bucket.getAccountId(), Resource.ResourceType.object_storage, Math.abs(diff));
324+
_objectStoreDao.updateAllocatedSize(objectStoreVO, diff);
325+
return;
326+
}
327+
328+
Account owner = _accountMgr.getActiveAccountById(bucket.getAccountId());
329+
try (CheckedReservation objectStorageReservation = new CheckedReservation(owner, Resource.ResourceType.object_storage, diff, reservationDao, resourceLimitManager)) {
330+
resourceLimitManager.incrementResourceCount(bucket.getAccountId(), Resource.ResourceType.object_storage, diff);
331+
_objectStoreDao.updateAllocatedSize(objectStoreVO, diff);
332+
}
333+
}
334+
340335
public void getBucketUsage() {
341336
//ToDo track usage one last time when object store or bucket is removed
342337
List<ObjectStoreVO> objectStores = _objectStoreDao.listObjectStores();

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1531,7 +1531,7 @@ public void testGetIpToNetworkMapFromBackup() {
15311531
}
15321532

15331533
@Test
1534-
public void testDeleteBackupVmNotFound() {
1534+
public void testDeleteBackupVmNotFound() throws ResourceAllocationException {
15351535
Long backupId = 1L;
15361536
Long vmId = 2L;
15371537
Long zoneId = 3L;
@@ -1813,21 +1813,21 @@ public void getBackupScheduleTestReturnNullWhenSpecifiedBackupScheduleIdIsNotALo
18131813
}
18141814

18151815
@Test
1816-
public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenBackupScheduleIsNotFound() {
1816+
public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenBackupScheduleIsNotFound() throws ResourceAllocationException {
18171817
backupManager.deleteOldestBackupFromScheduleIfRequired(1L, 1L);
18181818
Mockito.verify(backupManager, Mockito.never()).deleteExcessBackups(Mockito.anyList(), Mockito.anyInt(), Mockito.anyLong());
18191819
}
18201820

18211821
@Test
1822-
public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenRetentionIsEqualToZero() {
1822+
public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenRetentionIsEqualToZero() throws ResourceAllocationException {
18231823
Mockito.when(backupScheduleDao.findById(1L)).thenReturn(backupScheduleVOMock);
18241824
Mockito.when(backupScheduleVOMock.getMaxBackups()).thenReturn(0);
18251825
backupManager.deleteOldestBackupFromScheduleIfRequired(1L, 1L);
18261826
Mockito.verify(backupManager, Mockito.never()).deleteExcessBackups(Mockito.anyList(), Mockito.anyInt(), Mockito.anyLong());
18271827
}
18281828

18291829
@Test
1830-
public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenAmountOfBackupsToBeDeletedIsLessThanOne() {
1830+
public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenAmountOfBackupsToBeDeletedIsLessThanOne() throws ResourceAllocationException {
18311831
List<BackupVO> backups = List.of(Mockito.mock(BackupVO.class), Mockito.mock(BackupVO.class));
18321832
Mockito.when(backupScheduleDao.findById(1L)).thenReturn(backupScheduleVOMock);
18331833
Mockito.when(backupScheduleVOMock.getMaxBackups()).thenReturn(2);
@@ -1837,7 +1837,7 @@ public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenAmountOf
18371837
}
18381838

18391839
@Test
1840-
public void deleteOldestBackupFromScheduleIfRequiredTestDeleteBackupsWhenRequired() {
1840+
public void deleteOldestBackupFromScheduleIfRequiredTestDeleteBackupsWhenRequired() throws ResourceAllocationException {
18411841
List<BackupVO> backups = List.of(Mockito.mock(BackupVO.class), Mockito.mock(BackupVO.class));
18421842
Mockito.when(backupScheduleDao.findById(1L)).thenReturn(backupScheduleVOMock);
18431843
Mockito.when(backupScheduleVOMock.getMaxBackups()).thenReturn(1);
@@ -1848,7 +1848,7 @@ public void deleteOldestBackupFromScheduleIfRequiredTestDeleteBackupsWhenRequire
18481848
}
18491849

18501850
@Test
1851-
public void deleteExcessBackupsTestEnsureBackupsAreDeletedWhenMethodIsCalled() {
1851+
public void deleteExcessBackupsTestEnsureBackupsAreDeletedWhenMethodIsCalled() throws ResourceAllocationException {
18521852
try (MockedStatic<ActionEventUtils> actionEventUtils = Mockito.mockStatic(ActionEventUtils.class)) {
18531853
List<BackupVO> backups = List.of(Mockito.mock(BackupVO.class),
18541854
Mockito.mock(BackupVO.class),

‎server/src/test/java/org/apache/cloudstack/storage/object/BucketApiServiceImplTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ public void testCreateBucket() {
199199
}
200200

201201
@Test
202-
public void testDeleteBucket() {
202+
public void testDeleteBucket() throws ResourceAllocationException {
203203
Long bucketId = 1L;
204204
Long objectStoreId = 3L;
205205
String bucketName = "bucket1";

0 commit comments

Comments
 (0)