This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.17
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.17 by this push:
new 66f351543a server: do not deploy or upgrade vm with inactive service
offering (#7063)
66f351543a is described below
commit 66f351543adfd9d6e948651e627695ac466dff5d
Author: Wei Zhou <[email protected]>
AuthorDate: Wed Feb 8 09:40:30 2023 +0100
server: do not deploy or upgrade vm with inactive service offering (#7063)
Co-authored-by: Stephan Krug <[email protected]>
---
.../com/cloud/vm/VirtualMachineManagerImpl.java | 4 ++
.../cloud/vm/VirtualMachineManagerImplTest.java | 8 ++++
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 4 ++
.../deployment/RouterDeploymentDefinition.java | 2 +-
.../java/com/cloud/vm/UserVmManagerImplTest.java | 46 ++++++++++++++++++++--
5 files changed, 59 insertions(+), 5 deletions(-)
diff --git
a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
index db600d28dd..fa1ad061a0 100755
---
a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
+++
b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -3741,6 +3741,10 @@ public class VirtualMachineManagerImpl extends
ManagerBase implements VirtualMac
throw new InvalidParameterValueException("Invalid parameter,
newServiceOffering can't be null");
}
+ if
(ServiceOffering.State.Inactive.equals(newServiceOffering.getState())) {
+ throw new InvalidParameterValueException(String.format("New
service offering is inactive: [%s].", newServiceOffering.getUuid()));
+ }
+
if (!(vmInstance.getState().equals(State.Stopped) ||
vmInstance.getState().equals(State.Running))) {
s_logger.warn("Unable to upgrade virtual machine " +
vmInstance.toString() + " in state " + vmInstance.getState());
throw new InvalidParameterValueException("Unable to upgrade
virtual machine " + vmInstance.toString() + " " + " in state " +
vmInstance.getState() +
diff --git
a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java
b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java
index 86b0ea4a96..cea863d1bd 100644
---
a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java
+++
b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java
@@ -57,6 +57,7 @@ import com.cloud.deploy.DeploymentPlanner.ExcludeList;
import com.cloud.host.HostVO;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.hypervisor.HypervisorGuru;
+import com.cloud.offering.ServiceOffering;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.storage.DiskOfferingVO;
@@ -234,6 +235,13 @@ public class VirtualMachineManagerImplTest {
virtualMachineManagerImpl.checkIfCanUpgrade(vmInstanceMock,
serviceOfferingMock);
}
+ @Test(expected = InvalidParameterValueException.class)
+ public void testCheckIfCanUpgradeFail() {
+
when(serviceOfferingMock.getState()).thenReturn(ServiceOffering.State.Inactive);
+
+ virtualMachineManagerImpl.checkIfCanUpgrade(vmInstanceMock,
serviceOfferingMock);
+ }
+
@Test
public void isStorageCrossClusterMigrationTestStorageTypeEqualsCluster() {
Mockito.doReturn(2L).when(storagePoolVoMock).getClusterId();
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index 6a2f37808b..89e319d3be 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -5658,6 +5658,10 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
throw new InvalidParameterValueException("Unable to find service
offering: " + serviceOfferingId);
}
+ if (ServiceOffering.State.Inactive.equals(serviceOffering.getState()))
{
+ throw new InvalidParameterValueException(String.format("Service
offering is inactive: [%s].", serviceOffering.getUuid()));
+ }
+
if (serviceOffering.getDiskOfferingStrictness() &&
overrideDiskOfferingId != null) {
throw new InvalidParameterValueException(String.format("Cannot
override disk offering id %d since provided service offering is strictly mapped
to its disk offering", overrideDiskOfferingId));
}
diff --git
a/server/src/main/java/org/apache/cloudstack/network/router/deployment/RouterDeploymentDefinition.java
b/server/src/main/java/org/apache/cloudstack/network/router/deployment/RouterDeploymentDefinition.java
index 088310dba4..2b8ea7ffe5 100644
---
a/server/src/main/java/org/apache/cloudstack/network/router/deployment/RouterDeploymentDefinition.java
+++
b/server/src/main/java/org/apache/cloudstack/network/router/deployment/RouterDeploymentDefinition.java
@@ -407,7 +407,7 @@ public class RouterDeploymentDefinition {
private void verifyServiceOfferingByUuid(String offeringUuid) {
logger.debug("Verifying router service offering with uuid : " +
offeringUuid);
ServiceOfferingVO serviceOffering =
serviceOfferingDao.findByUuid(offeringUuid);
- if (serviceOffering != null && serviceOffering.isSystemUse()) {
+ if (serviceOffering != null && serviceOffering.isSystemUse() &&
ServiceOffering.State.Active.equals(serviceOffering.getState())) {
DiskOfferingVO diskOffering =
diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
boolean isLocalStorage =
ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dest.getDataCenter().getId());
if (isLocalStorage == diskOffering.isUseLocalStorage()) {
diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
index 22b7f223e6..550532b318 100644
--- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
+++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
@@ -23,6 +23,7 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
@@ -34,6 +35,7 @@ import java.util.List;
import java.util.Map;
import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
+import org.apache.cloudstack.api.command.user.vm.DeployVMCmd;
import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd;
import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
import org.apache.cloudstack.context.CallContext;
@@ -49,13 +51,16 @@ import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.springframework.test.util.ReflectionTestUtils;
import com.cloud.configuration.Resource;
+import com.cloud.dc.DataCenter;
import com.cloud.dc.DataCenterVO;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.exception.InsufficientAddressCapacityException;
import com.cloud.exception.InsufficientCapacityException;
import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.hypervisor.Hypervisor;
import com.cloud.network.NetworkModel;
@@ -74,11 +79,13 @@ import com.cloud.storage.dao.GuestOSDao;
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
+import com.cloud.user.AccountService;
import com.cloud.user.AccountVO;
import com.cloud.user.ResourceLimitService;
import com.cloud.user.UserVO;
import com.cloud.user.dao.AccountDao;
import com.cloud.uservm.UserVm;
+import com.cloud.utils.db.EntityManager;
import com.cloud.vm.dao.NicDao;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.UserVmDetailsDao;
@@ -125,6 +132,12 @@ public class UserVmManagerImplTest {
@Mock
private AccountManager accountManager;
+ @Mock
+ private AccountService accountService;
+
+ @Mock
+ private EntityManager entityManager;
+
@Mock
private UserVmDetailsDao userVmDetailVO;
@@ -155,7 +168,16 @@ public class UserVmManagerImplTest {
@Mock
VolumeApiService volumeApiService;
- private long vmId = 1l;
+ @Mock
+ AccountVO account;
+
+ @Mock
+ private ServiceOfferingVO serviceOffering;
+
+ private static final long vmId = 1l;
+ private static final long zoneId = 2L;
+ private static final long accountId = 3L;
+ private static final long serviceOfferingId = 10L;
private static final long GiB_TO_BYTES = 1024 * 1024 * 1024;
@@ -171,7 +193,7 @@ public class UserVmManagerImplTest {
when(_dcDao.findById(anyLong())).thenReturn(_dcMock);
-
Mockito.when(userVmDao.findById(Mockito.eq(vmId))).thenReturn(userVmVoMock);
+ Mockito.when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
Mockito.when(callerAccount.getType()).thenReturn(Account.Type.ADMIN);
CallContext.register(callerUser, callerAccount);
@@ -202,14 +224,14 @@ public class UserVmManagerImplTest {
@Test
public void
validateGuestOsIdForUpdateVirtualMachineCommandTestOsTypeFound() {
Mockito.when(updateVmCommand.getOsTypeId()).thenReturn(1l);
-
Mockito.when(guestOSDao.findById(Mockito.eq(1l))).thenReturn(Mockito.mock(GuestOSVO.class));
+
Mockito.when(guestOSDao.findById(1l)).thenReturn(Mockito.mock(GuestOSVO.class));
userVmManagerImpl.validateGuestOsIdForUpdateVirtualMachineCommand(updateVmCommand);
}
@Test(expected = InvalidParameterValueException.class)
public void
validateInputsAndPermissionForUpdateVirtualMachineCommandTestVmNotFound() {
- Mockito.when(userVmDao.findById(Mockito.eq(vmId))).thenReturn(null);
+ Mockito.when(userVmDao.findById(vmId)).thenReturn(null);
userVmManagerImpl.validateInputsAndPermissionForUpdateVirtualMachineCommand(updateVmCommand);
}
@@ -573,4 +595,20 @@ public class UserVmManagerImplTest {
Mockito.when(newRootDiskOffering.getName()).thenReturn("OfferingName");
return newRootDiskOffering;
}
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void createVirtualMachineWithInactiveServiceOffering() throws
ResourceUnavailableException, InsufficientCapacityException,
ResourceAllocationException {
+ DeployVMCmd deployVMCmd = new DeployVMCmd();
+ ReflectionTestUtils.setField(deployVMCmd, "zoneId", zoneId);
+ ReflectionTestUtils.setField(deployVMCmd, "serviceOfferingId",
serviceOfferingId);
+ deployVMCmd._accountService = accountService;
+
+ when(accountService.finalyzeAccountId(nullable(String.class),
nullable(Long.class), nullable(Long.class), eq(true))).thenReturn(accountId);
+
when(accountService.getActiveAccountById(accountId)).thenReturn(account);
+ when(entityManager.findById(DataCenter.class,
zoneId)).thenReturn(_dcMock);
+ when(entityManager.findById(ServiceOffering.class,
serviceOfferingId)).thenReturn(serviceOffering);
+
when(serviceOffering.getState()).thenReturn(ServiceOffering.State.Inactive);
+
+ userVmManagerImpl.createVirtualMachine(deployVMCmd);
+ }
}
