diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/bucket/DeleteBucketCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/bucket/DeleteBucketCmd.java index 8cd2790e4ae..e7b940fc0ad 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/bucket/DeleteBucketCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/bucket/DeleteBucketCmd.java @@ -17,6 +17,7 @@ package org.apache.cloudstack.api.command.user.bucket; import com.cloud.exception.ConcurrentOperationException; +import com.cloud.exception.ResourceAllocationException; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.storage.object.Bucket; import com.cloud.user.Account; @@ -82,7 +83,7 @@ public class DeleteBucketCmd extends BaseCmd { } @Override - public void execute() throws ConcurrentOperationException { + public void execute() throws ConcurrentOperationException, ResourceAllocationException { CallContext.current().setEventDetails("Bucket Id: " + this._uuidMgr.getUuid(Bucket.class, getId())); boolean result = _bucketService.deleteBucket(id, CallContext.current().getCallingAccount()); SuccessResponse response = new SuccessResponse(getCommandName()); 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 29b97343281..07c32a7f8a5 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java +++ b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java @@ -226,7 +226,7 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer * @param forced Indicates if backup will be force removed or not * @return returns operation success */ - boolean deleteBackup(final Long backupId, final Boolean forced); + boolean deleteBackup(final Long backupId, final Boolean forced) throws ResourceAllocationException; void validateBackupForZone(Long zoneId); diff --git a/api/src/main/java/org/apache/cloudstack/storage/object/BucketApiService.java b/api/src/main/java/org/apache/cloudstack/storage/object/BucketApiService.java index e27ef308d7f..8c164133db8 100644 --- a/api/src/main/java/org/apache/cloudstack/storage/object/BucketApiService.java +++ b/api/src/main/java/org/apache/cloudstack/storage/object/BucketApiService.java @@ -95,7 +95,7 @@ public interface BucketApiService { */ Bucket createBucket(CreateBucketCmd cmd); - boolean deleteBucket(long bucketId, Account caller); + boolean deleteBucket(long bucketId, Account caller) throws ResourceAllocationException; boolean updateBucket(UpdateBucketCmd cmd, Account caller) throws ResourceAllocationException; 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 152d6286969..ec87980e03a 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -841,18 +841,12 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { resourceLimitMgr.incrementResourceCount(vm.getAccountId(), Resource.ResourceType.backup); resourceLimitMgr.incrementResourceCount(vm.getAccountId(), Resource.ResourceType.backup_storage, backup.getSize()); } - } catch (Exception e) { - if (e instanceof ResourceAllocationException) { - ResourceAllocationException rae = (ResourceAllocationException)e; - if (isScheduledBackup && (Resource.ResourceType.backup.equals(rae.getResourceType()) || - Resource.ResourceType.backup_storage.equals(rae.getResourceType()))) { - sendExceededBackupLimitAlert(owner.getUuid(), rae.getResourceType()); - } - throw rae; - } else if (e instanceof CloudRuntimeException) { - throw (CloudRuntimeException)e; + } catch (ResourceAllocationException e) { + if (isScheduledBackup && (Resource.ResourceType.backup.equals(e.getResourceType()) || + Resource.ResourceType.backup_storage.equals(e.getResourceType()))) { + sendExceededBackupLimitAlert(owner.getUuid(), e.getResourceType()); } - throw new CloudRuntimeException("Failed to create backup for VM with ID: " + vm.getUuid(), e); + throw e; } } @@ -905,7 +899,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { * @param vmId The ID of the VM associated with the backups * @param backupScheduleId Backup schedule ID of the backups */ - protected void deleteOldestBackupFromScheduleIfRequired(Long vmId, long backupScheduleId) { + protected void deleteOldestBackupFromScheduleIfRequired(Long vmId, long backupScheduleId) throws ResourceAllocationException { BackupScheduleVO backupScheduleVO = backupScheduleDao.findById(backupScheduleId); if (backupScheduleVO == null || backupScheduleVO.getMaxBackups() == 0) { logger.info("The schedule does not have a retention specified and, hence, not deleting any backups from it.", vmId); @@ -929,7 +923,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { * @param amountOfBackupsToDelete Number of backups to be deleted from the list of backups * @param backupScheduleId ID of the backup schedule associated with the backups */ - protected void deleteExcessBackups(List backups, int amountOfBackupsToDelete, long backupScheduleId) { + protected void deleteExcessBackups(List backups, int amountOfBackupsToDelete, long backupScheduleId) throws ResourceAllocationException { logger.debug("Deleting the [{}] oldest backups from the schedule [ID: {}].", amountOfBackupsToDelete, backupScheduleId); for (int i = 0; i < amountOfBackupsToDelete; i++) { @@ -1527,7 +1521,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { @Override @ActionEvent(eventType = EventTypes.EVENT_VM_BACKUP_DELETE, eventDescription = "deleting VM backup", async = true) - public boolean deleteBackup(final Long backupId, final Boolean forced) { + public boolean deleteBackup(final Long backupId, final Boolean forced) throws ResourceAllocationException { final BackupVO backup = backupDao.findByIdIncludingRemoved(backupId); if (backup == null) { throw new CloudRuntimeException("Backup " + backupId + " does not exist"); @@ -1552,7 +1546,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { return deleteCheckedBackup(forced, backupProvider, backup, vm); } - private boolean deleteCheckedBackup(Boolean forced, BackupProvider backupProvider, BackupVO backup, VMInstanceVO vm) { + private boolean deleteCheckedBackup(Boolean forced, BackupProvider backupProvider, BackupVO backup, VMInstanceVO vm) throws ResourceAllocationException { Account owner = accountManager.getAccount(backup.getAccountId()); long backupSize = backup.getSize() != null ? backup.getSize() : 0L; try (CheckedReservation backupReservation = new CheckedReservation(owner, Resource.ResourceType.backup, @@ -1572,11 +1566,6 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { } } throw new CloudRuntimeException("Failed to delete the backup"); - } catch (Exception e) { - if (e instanceof CloudRuntimeException) { - throw (CloudRuntimeException) e; - } - throw new CloudRuntimeException("Failed to delete the backup due to: " + e.getMessage(), e); } } diff --git a/server/src/main/java/org/apache/cloudstack/storage/object/BucketApiServiceImpl.java b/server/src/main/java/org/apache/cloudstack/storage/object/BucketApiServiceImpl.java index 59adb3a0e56..900cbdfac0d 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/object/BucketApiServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/storage/object/BucketApiServiceImpl.java @@ -163,13 +163,6 @@ public class BucketApiServiceImpl extends ManagerBase implements BucketApiServic (cmd.getQuota() * Resource.ResourceType.bytesToGiB)); } return bucket; - } catch (Exception e) { - if (e instanceof ResourceAllocationException) { - throw (ResourceAllocationException)e; - } else if (e instanceof CloudRuntimeException) { - throw (CloudRuntimeException)e; - } - throw new CloudRuntimeException(String.format("Failed to create bucket due to: %s", e.getMessage()), e); } } @@ -236,7 +229,7 @@ public class BucketApiServiceImpl extends ManagerBase implements BucketApiServic @Override @ActionEvent(eventType = EventTypes.EVENT_BUCKET_DELETE, eventDescription = "deleting bucket") - public boolean deleteBucket(long bucketId, Account caller) { + public boolean deleteBucket(long bucketId, Account caller) throws ResourceAllocationException { Bucket bucket = _bucketDao.findById(bucketId); if (bucket == null) { throw new InvalidParameterValueException("Unable to find bucket with ID: " + bucketId); @@ -247,7 +240,7 @@ public class BucketApiServiceImpl extends ManagerBase implements BucketApiServic return deleteCheckedBucket(objectStore, bucket, objectStoreVO); } - private boolean deleteCheckedBucket(ObjectStoreEntity objectStore, Bucket bucket, ObjectStoreVO objectStoreVO) { + private boolean deleteCheckedBucket(ObjectStoreEntity objectStore, Bucket bucket, ObjectStoreVO objectStoreVO) throws ResourceAllocationException { Account owner = _accountMgr.getAccount(bucket.getAccountId()); try (CheckedReservation bucketReservation = new CheckedReservation(owner, Resource.ResourceType.bucket, bucket.getId(), null, -1L, reservationDao, resourceLimitManager); @@ -265,11 +258,6 @@ public class BucketApiServiceImpl extends ManagerBase implements BucketApiServic return true; } return false; - } catch (Exception e) { - if (e instanceof CloudRuntimeException) { - throw (CloudRuntimeException) e; - } - throw new CloudRuntimeException(String.format("Failed to delete bucket due to: %s", e.getMessage()), e); } } @@ -284,16 +272,6 @@ public class BucketApiServiceImpl extends ManagerBase implements BucketApiServic _accountMgr.checkAccess(caller, null, true, bucket); ObjectStoreVO objectStoreVO = _objectStoreDao.findById(bucket.getObjectStoreId()); ObjectStoreEntity objectStore = (ObjectStoreEntity)_dataStoreMgr.getDataStore(objectStoreVO.getId(), DataStoreRole.Object); - Integer quota = cmd.getQuota(); - Integer quotaDelta = null; - - if (quota != null) { - quotaDelta = quota - bucket.getQuota(); - if (quotaDelta > 0) { - Account owner = _accountMgr.getActiveAccountById(bucket.getAccountId()); - resourceLimitManager.checkResourceLimit(owner, Resource.ResourceType.object_storage, (quotaDelta * Resource.ResourceType.bytesToGiB)); - } - } try { if (cmd.getEncryption() != null) { @@ -319,16 +297,8 @@ public class BucketApiServiceImpl extends ManagerBase implements BucketApiServic bucket.setPolicy(cmd.getPolicy()); } - if (cmd.getQuota() != null) { - objectStore.setQuota(bucketTO, cmd.getQuota()); - bucket.setQuota(cmd.getQuota()); - if (quotaDelta > 0) { - resourceLimitManager.incrementResourceCount(bucket.getAccountId(), Resource.ResourceType.object_storage, (quotaDelta * Resource.ResourceType.bytesToGiB)); - } else { - resourceLimitManager.decrementResourceCount(bucket.getAccountId(), Resource.ResourceType.object_storage, ((-quotaDelta) * Resource.ResourceType.bytesToGiB)); - } - _objectStoreDao.updateAllocatedSize(objectStoreVO, (quotaDelta * Resource.ResourceType.bytesToGiB)); - } + updateBucketQuota(cmd, bucket, objectStore, objectStoreVO, bucketTO); + _bucketDao.update(bucket.getId(), bucket); } catch (Exception e) { throw new CloudRuntimeException("Error while updating bucket: " +bucket.getName() +". "+e.getMessage()); @@ -337,6 +307,31 @@ public class BucketApiServiceImpl extends ManagerBase implements BucketApiServic return true; } + private void updateBucketQuota(UpdateBucketCmd cmd, BucketVO bucket, ObjectStoreEntity objectStore, ObjectStoreVO objectStoreVO, BucketTO bucketTO) throws ResourceAllocationException { + Integer quota = cmd.getQuota(); + if (quota == null) { + return; + } + + int quotaDelta = quota - bucket.getQuota(); + objectStore.setQuota(bucketTO, quota); + bucket.setQuota(quota); + + long diff = quotaDelta * Resource.ResourceType.bytesToGiB; + + if (quotaDelta < 0) { + resourceLimitManager.decrementResourceCount(bucket.getAccountId(), Resource.ResourceType.object_storage, Math.abs(diff)); + _objectStoreDao.updateAllocatedSize(objectStoreVO, diff); + return; + } + + Account owner = _accountMgr.getActiveAccountById(bucket.getAccountId()); + try (CheckedReservation objectStorageReservation = new CheckedReservation(owner, Resource.ResourceType.object_storage, diff, reservationDao, resourceLimitManager)) { + resourceLimitManager.incrementResourceCount(bucket.getAccountId(), Resource.ResourceType.object_storage, diff); + _objectStoreDao.updateAllocatedSize(objectStoreVO, diff); + } + } + public void getBucketUsage() { //ToDo track usage one last time when object store or bucket is removed List objectStores = _objectStoreDao.listObjectStores(); 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 1864eb81d5a..a65e2f2ed39 100644 --- a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java +++ b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java @@ -1531,7 +1531,7 @@ public class BackupManagerTest { } @Test - public void testDeleteBackupVmNotFound() { + public void testDeleteBackupVmNotFound() throws ResourceAllocationException { Long backupId = 1L; Long vmId = 2L; Long zoneId = 3L; @@ -1813,13 +1813,13 @@ public class BackupManagerTest { } @Test - public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenBackupScheduleIsNotFound() { + public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenBackupScheduleIsNotFound() throws ResourceAllocationException { backupManager.deleteOldestBackupFromScheduleIfRequired(1L, 1L); Mockito.verify(backupManager, Mockito.never()).deleteExcessBackups(Mockito.anyList(), Mockito.anyInt(), Mockito.anyLong()); } @Test - public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenRetentionIsEqualToZero() { + public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenRetentionIsEqualToZero() throws ResourceAllocationException { Mockito.when(backupScheduleDao.findById(1L)).thenReturn(backupScheduleVOMock); Mockito.when(backupScheduleVOMock.getMaxBackups()).thenReturn(0); backupManager.deleteOldestBackupFromScheduleIfRequired(1L, 1L); @@ -1827,7 +1827,7 @@ public class BackupManagerTest { } @Test - public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenAmountOfBackupsToBeDeletedIsLessThanOne() { + public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenAmountOfBackupsToBeDeletedIsLessThanOne() throws ResourceAllocationException { List backups = List.of(Mockito.mock(BackupVO.class), Mockito.mock(BackupVO.class)); Mockito.when(backupScheduleDao.findById(1L)).thenReturn(backupScheduleVOMock); Mockito.when(backupScheduleVOMock.getMaxBackups()).thenReturn(2); @@ -1837,7 +1837,7 @@ public class BackupManagerTest { } @Test - public void deleteOldestBackupFromScheduleIfRequiredTestDeleteBackupsWhenRequired() { + public void deleteOldestBackupFromScheduleIfRequiredTestDeleteBackupsWhenRequired() throws ResourceAllocationException { List backups = List.of(Mockito.mock(BackupVO.class), Mockito.mock(BackupVO.class)); Mockito.when(backupScheduleDao.findById(1L)).thenReturn(backupScheduleVOMock); Mockito.when(backupScheduleVOMock.getMaxBackups()).thenReturn(1); @@ -1848,7 +1848,7 @@ public class BackupManagerTest { } @Test - public void deleteExcessBackupsTestEnsureBackupsAreDeletedWhenMethodIsCalled() { + public void deleteExcessBackupsTestEnsureBackupsAreDeletedWhenMethodIsCalled() throws ResourceAllocationException { try (MockedStatic actionEventUtils = Mockito.mockStatic(ActionEventUtils.class)) { List backups = List.of(Mockito.mock(BackupVO.class), Mockito.mock(BackupVO.class), diff --git a/server/src/test/java/org/apache/cloudstack/storage/object/BucketApiServiceImplTest.java b/server/src/test/java/org/apache/cloudstack/storage/object/BucketApiServiceImplTest.java index 6dbdd45ce4c..a4429befc44 100644 --- a/server/src/test/java/org/apache/cloudstack/storage/object/BucketApiServiceImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/storage/object/BucketApiServiceImplTest.java @@ -199,7 +199,7 @@ public class BucketApiServiceImplTest { } @Test - public void testDeleteBucket() { + public void testDeleteBucket() throws ResourceAllocationException { Long bucketId = 1L; Long objectStoreId = 3L; String bucketName = "bucket1";