You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2026/04/15 06:55:36 UTC
(cloudstack) 01/02: Address reviews
This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to tag 4.22.0.1
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 13842a626d7e3bf1debcb62daae35ddab87c14ac
Author: Fabricio Duarte <fa...@gmail.com>
AuthorDate: Tue Mar 17 16:16:54 2026 -0300
Address reviews
---
.../api/command/user/bucket/DeleteBucketCmd.java | 3 ++-
.../apache/cloudstack/backup/BackupManager.java | 2 +-
.../storage/object/BucketApiService.java | 2 +-
.../cloudstack/backup/BackupManagerImpl.java | 29 +++++++---------------
.../storage/object/BucketApiServiceImpl.java | 16 ++----------
.../cloudstack/backup/BackupManagerTest.java | 12 ++++-----
.../storage/object/BucketApiServiceImplTest.java | 2 +-
7 files changed, 22 insertions(+), 44 deletions(-)
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 db051313d96..2f9bfdd6b2a 100644
--- a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java
+++ b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java
@@ -224,7 +224,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 088517e705b..a38d4f65db1 100644
--- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
+++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
@@ -843,18 +843,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;
}
}
@@ -907,7 +901,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);
@@ -931,7 +925,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<BackupVO> backups, int amountOfBackupsToDelete, long backupScheduleId) {
+ protected void deleteExcessBackups(List<BackupVO> 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++) {
@@ -1529,7 +1523,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..21f01991407 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);
}
}
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 7395f222a86..6abacc36333 100644
--- a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java
+++ b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java
@@ -1526,7 +1526,7 @@ public class BackupManagerTest {
}
@Test
- public void testDeleteBackupVmNotFound() {
+ public void testDeleteBackupVmNotFound() throws ResourceAllocationException {
Long backupId = 1L;
Long vmId = 2L;
Long zoneId = 3L;
@@ -1782,13 +1782,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);
@@ -1796,7 +1796,7 @@ public class BackupManagerTest {
}
@Test
- public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenAmountOfBackupsToBeDeletedIsLessThanOne() {
+ public void deleteOldestBackupFromScheduleIfRequiredTestSkipDeletionWhenAmountOfBackupsToBeDeletedIsLessThanOne() throws ResourceAllocationException {
List<BackupVO> backups = List.of(Mockito.mock(BackupVO.class), Mockito.mock(BackupVO.class));
Mockito.when(backupScheduleDao.findById(1L)).thenReturn(backupScheduleVOMock);
Mockito.when(backupScheduleVOMock.getMaxBackups()).thenReturn(2);
@@ -1806,7 +1806,7 @@ public class BackupManagerTest {
}
@Test
- public void deleteOldestBackupFromScheduleIfRequiredTestDeleteBackupsWhenRequired() {
+ public void deleteOldestBackupFromScheduleIfRequiredTestDeleteBackupsWhenRequired() throws ResourceAllocationException {
List<BackupVO> backups = List.of(Mockito.mock(BackupVO.class), Mockito.mock(BackupVO.class));
Mockito.when(backupScheduleDao.findById(1L)).thenReturn(backupScheduleVOMock);
Mockito.when(backupScheduleVOMock.getMaxBackups()).thenReturn(1);
@@ -1817,7 +1817,7 @@ public class BackupManagerTest {
}
@Test
- public void deleteExcessBackupsTestEnsureBackupsAreDeletedWhenMethodIsCalled() {
+ public void deleteExcessBackupsTestEnsureBackupsAreDeletedWhenMethodIsCalled() throws ResourceAllocationException {
try (MockedStatic<ActionEventUtils> actionEventUtils = Mockito.mockStatic(ActionEventUtils.class)) {
List<BackupVO> 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";