Skip to content

Commit c681d0d

Browse files
Change vmsnapshot.max setting scope to the account level (#11616)
1 parent 9b4f16b commit c681d0d

File tree

3 files changed

+20
-4
lines changed

3 files changed

+20
-4
lines changed

engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSnapshotManager.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public interface VMSnapshotManager extends VMSnapshotService, Manager {
3131
static final ConfigKey<Integer> VMSnapshotExpireInterval = new ConfigKey<Integer>("Advanced", Integer.class, "vmsnapshot.expire.interval", "-1",
3232
"VM Snapshot expire interval in hours", true, ConfigKey.Scope.Account);
3333

34-
ConfigKey<Integer> VMSnapshotMax = new ConfigKey<Integer>("Advanced", Integer.class, "vmsnapshot.max", "10", "Maximum vm snapshots for a single vm", true, ConfigKey.Scope.Global);
34+
ConfigKey<Integer> VMSnapshotMax = new ConfigKey<Integer>("Advanced", Integer.class, "vmsnapshot.max", "10", "Maximum VM snapshots for a single VM", true, ConfigKey.Scope.Account);
3535

3636
/**
3737
* Delete all VM snapshots belonging to one VM

server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -400,10 +400,11 @@ public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDesc
400400
_accountMgr.checkAccess(caller, null, true, userVmVo);
401401

402402
// check max snapshot limit for per VM
403-
int vmSnapshotMax = VMSnapshotManager.VMSnapshotMax.value();
404-
403+
boolean vmBelongsToProject = _accountMgr.getAccount(userVmVo.getAccountId()).getType() == Account.Type.PROJECT;
404+
long accountIdToRetrieveConfigurationValueFrom = vmBelongsToProject ? caller.getId() : userVmVo.getAccountId();
405+
int vmSnapshotMax = VMSnapshotManager.VMSnapshotMax.valueIn(accountIdToRetrieveConfigurationValueFrom);
405406
if (_vmSnapshotDao.findByVm(vmId).size() >= vmSnapshotMax) {
406-
throw new CloudRuntimeException("Creating Instance Snapshot failed due to a Instance can just have : " + vmSnapshotMax + " Instance Snapshots. Please delete old ones");
407+
throw new CloudRuntimeException(String.format("Each VM can have at most [%s] VM snapshots.", vmSnapshotMax));
407408
}
408409

409410
// check if there are active volume snapshots tasks

server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.cloud.storage.dao.VolumeDao;
4242
import com.cloud.user.Account;
4343
import com.cloud.user.AccountManager;
44+
import com.cloud.user.AccountVO;
4445
import com.cloud.user.dao.AccountDao;
4546
import com.cloud.user.dao.UserDao;
4647
import com.cloud.uservm.UserVm;
@@ -136,6 +137,8 @@ public class VMSnapshotManagerTest {
136137
VMSnapshotDetailsDao _vmSnapshotDetailsDao;
137138
@Mock
138139
UserVmManager _userVmManager;
140+
@Mock
141+
private AccountVO accountVOMock;
139142

140143
private static final long TEST_VM_ID = 3L;
141144
private static final long SERVICE_OFFERING_ID = 1L;
@@ -285,8 +288,12 @@ public void testCreateVMSnapshotF3() throws AgentUnavailableException, Operation
285288
@SuppressWarnings("unchecked")
286289
@Test(expected = CloudRuntimeException.class)
287290
public void testAllocVMSnapshotF4() throws ResourceAllocationException {
291+
long accountId = 1L;
288292
List<VMSnapshotVO> mockList = mock(List.class);
289293
when(mockList.size()).thenReturn(10);
294+
when(_userVMDao.findById(TEST_VM_ID)).thenReturn(vmMock);
295+
when(userVm.getAccountId()).thenReturn(accountId);
296+
when(_accountMgr.getAccount(accountId)).thenReturn(accountVOMock);
290297
when(_vmSnapshotDao.findByVm(TEST_VM_ID)).thenReturn(mockList);
291298
_vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID, "", "", true);
292299
}
@@ -295,15 +302,23 @@ public void testAllocVMSnapshotF4() throws ResourceAllocationException {
295302
@SuppressWarnings("unchecked")
296303
@Test(expected = CloudRuntimeException.class)
297304
public void testAllocVMSnapshotF5() throws ResourceAllocationException {
305+
long accountId = 1L;
298306
List<SnapshotVO> mockList = mock(List.class);
299307
when(mockList.size()).thenReturn(1);
308+
when(_userVMDao.findById(TEST_VM_ID)).thenReturn(vmMock);
309+
when(userVm.getAccountId()).thenReturn(accountId);
310+
when(_accountMgr.getAccount(accountId)).thenReturn(accountVOMock);
300311
when(_snapshotDao.listByInstanceId(TEST_VM_ID, Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp)).thenReturn(mockList);
301312
_vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID, "", "", true);
302313
}
303314

304315
// successful creation case
305316
@Test
306317
public void testCreateVMSnapshot() throws AgentUnavailableException, OperationTimedoutException, ResourceAllocationException, NoTransitionException {
318+
long accountId = 1L;
319+
when(_userVMDao.findById(TEST_VM_ID)).thenReturn(vmMock);
320+
when(userVm.getAccountId()).thenReturn(accountId);
321+
when(_accountMgr.getAccount(accountId)).thenReturn(accountVOMock);
307322
when(vmMock.getState()).thenReturn(State.Running);
308323
_vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID, "", "", true);
309324
}

0 commit comments

Comments
 (0)