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 db600d28dd4..fa1ad061a0b 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 86b0ea4a96d..cea863d1bde 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 6a2f37808b7..89e319d3be3 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 088310dba40..2b8ea7ffe5f 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 22b7f223e6e..550532b3184 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); + } }