Skip to content

Commit 3322abf

Browse files
shwstpprshwstppr
committed
refactor, test
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
1 parent 56b2ea1 commit 3322abf

File tree

2 files changed

+156
-67
lines changed

2 files changed

+156
-67
lines changed

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 39 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
import javax.xml.parsers.DocumentBuilder;
5555
import javax.xml.parsers.ParserConfigurationException;
5656

57-
import com.cloud.network.NetworkService;
5857
import org.apache.cloudstack.acl.ControlledEntity;
5958
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
6059
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
@@ -134,8 +133,8 @@
134133
import org.apache.cloudstack.userdata.UserDataManager;
135134
import org.apache.cloudstack.utils.bytescale.ByteScaleUtils;
136135
import org.apache.cloudstack.utils.security.ParserUtils;
137-
import org.apache.cloudstack.vm.schedule.VMScheduleManager;
138136
import org.apache.cloudstack.vm.UnmanagedVMsManager;
137+
import org.apache.cloudstack.vm.schedule.VMScheduleManager;
139138
import org.apache.commons.collections.CollectionUtils;
140139
import org.apache.commons.collections.MapUtils;
141140
import org.apache.commons.lang.math.NumberUtils;
@@ -254,6 +253,7 @@
254253
import com.cloud.network.Network.Provider;
255254
import com.cloud.network.Network.Service;
256255
import com.cloud.network.NetworkModel;
256+
import com.cloud.network.NetworkService;
257257
import com.cloud.network.Networks.TrafficType;
258258
import com.cloud.network.PhysicalNetwork;
259259
import com.cloud.network.as.AutoScaleManager;
@@ -1283,47 +1283,45 @@ private void validateOfferingMaxResource(ServiceOfferingVO offering) {
12831283

12841284
@Override
12851285
public void validateCustomParameters(ServiceOfferingVO serviceOffering, Map<String, String> customParameters) {
1286-
//TODO need to validate custom cpu, and memory against min/max CPU/Memory ranges from service_offering_details table
1287-
if (customParameters.size() != 0) {
1288-
Map<String, String> offeringDetails = serviceOfferingDetailsDao.listDetailsKeyPairs(serviceOffering.getId());
1289-
if (serviceOffering.getCpu() == null) {
1290-
int minCPU = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MIN_CPU_NUMBER), 1);
1291-
int maxCPU = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MAX_CPU_NUMBER), Integer.MAX_VALUE);
1292-
int cpuNumber = NumbersUtil.parseInt(customParameters.get(UsageEventVO.DynamicParameters.cpuNumber.name()), -1);
1293-
Integer maxCPUCores = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value() == 0 ? Integer.MAX_VALUE: ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value();
1294-
if (cpuNumber < minCPU || cpuNumber > maxCPU || cpuNumber > maxCPUCores) {
1295-
throw new InvalidParameterValueException(String.format("Invalid CPU cores value, specify a value between %d and %d", minCPU, Math.min(maxCPUCores, maxCPU)));
1296-
}
1297-
} else if (customParameters.containsKey(UsageEventVO.DynamicParameters.cpuNumber.name())) {
1298-
throw new InvalidParameterValueException("The CPU cores of this offering id:" + serviceOffering.getUuid()
1299-
+ " is not customizable. This is predefined in the Template.");
1300-
}
1301-
1302-
if (serviceOffering.getSpeed() == null) {
1303-
String cpuSpeed = customParameters.get(UsageEventVO.DynamicParameters.cpuSpeed.name());
1304-
if ((cpuSpeed == null) || (NumbersUtil.parseInt(cpuSpeed, -1) <= 0)) {
1305-
throw new InvalidParameterValueException("Invalid CPU speed value, specify a value between 1 and " + Integer.MAX_VALUE);
1306-
}
1307-
} else if (!serviceOffering.isCustomCpuSpeedSupported() && customParameters.containsKey(UsageEventVO.DynamicParameters.cpuSpeed.name())) {
1308-
throw new InvalidParameterValueException(String.format("The CPU speed of this offering id:%s"
1309-
+ " is not customizable. This is predefined as %d MHz.",
1310-
serviceOffering.getUuid(), serviceOffering.getSpeed()));
1311-
}
1312-
1313-
if (serviceOffering.getRamSize() == null) {
1314-
int minMemory = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MIN_MEMORY), 32);
1315-
int maxMemory = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MAX_MEMORY), Integer.MAX_VALUE);
1316-
int memory = NumbersUtil.parseInt(customParameters.get(UsageEventVO.DynamicParameters.memory.name()), -1);
1317-
Integer maxRAMSize = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_RAM_SIZE.value() == 0 ? Integer.MAX_VALUE: ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_RAM_SIZE.value();
1318-
if (memory < minMemory || memory > maxMemory || memory > maxRAMSize) {
1319-
throw new InvalidParameterValueException(String.format("Invalid memory value, specify a value between %d and %d", minMemory, Math.min(maxRAMSize, maxMemory)));
1320-
}
1321-
} else if (customParameters.containsKey(UsageEventVO.DynamicParameters.memory.name())) {
1322-
throw new InvalidParameterValueException("The memory of this offering id:" + serviceOffering.getUuid() + " is not customizable. This is predefined in the Template.");
1323-
}
1324-
} else {
1286+
if (MapUtils.isEmpty(customParameters) && serviceOffering.isDynamic()) {
13251287
throw new InvalidParameterValueException("Need to specify custom parameter values cpu, cpu speed and memory when using custom offering");
13261288
}
1289+
Map<String, String> offeringDetails = serviceOfferingDetailsDao.listDetailsKeyPairs(serviceOffering.getId());
1290+
if (serviceOffering.getCpu() == null) {
1291+
int minCPU = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MIN_CPU_NUMBER), 1);
1292+
int maxCPU = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MAX_CPU_NUMBER), Integer.MAX_VALUE);
1293+
int cpuNumber = NumbersUtil.parseInt(customParameters.get(UsageEventVO.DynamicParameters.cpuNumber.name()), -1);
1294+
int maxCPUCores = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value() == 0 ? Integer.MAX_VALUE: ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value();
1295+
if (cpuNumber < minCPU || cpuNumber > maxCPU || cpuNumber > maxCPUCores) {
1296+
throw new InvalidParameterValueException(String.format("Invalid CPU cores value, specify a value between %d and %d", minCPU, Math.min(maxCPUCores, maxCPU)));
1297+
}
1298+
} else if (customParameters.containsKey(UsageEventVO.DynamicParameters.cpuNumber.name())) {
1299+
throw new InvalidParameterValueException("The CPU cores of this offering id:" + serviceOffering.getUuid()
1300+
+ " is not customizable. This is predefined in the Template.");
1301+
}
1302+
1303+
if (serviceOffering.getSpeed() == null) {
1304+
String cpuSpeed = customParameters.get(UsageEventVO.DynamicParameters.cpuSpeed.name());
1305+
if ((cpuSpeed == null) || (NumbersUtil.parseInt(cpuSpeed, -1) <= 0)) {
1306+
throw new InvalidParameterValueException("Invalid CPU speed value, specify a value between 1 and " + Integer.MAX_VALUE);
1307+
}
1308+
} else if (!serviceOffering.isCustomCpuSpeedSupported() && customParameters.containsKey(UsageEventVO.DynamicParameters.cpuSpeed.name())) {
1309+
throw new InvalidParameterValueException(String.format("The CPU speed of this offering id:%s"
1310+
+ " is not customizable. This is predefined as %d MHz.",
1311+
serviceOffering.getUuid(), serviceOffering.getSpeed()));
1312+
}
1313+
1314+
if (serviceOffering.getRamSize() == null) {
1315+
int minMemory = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MIN_MEMORY), 32);
1316+
int maxMemory = NumbersUtil.parseInt(offeringDetails.get(ApiConstants.MAX_MEMORY), Integer.MAX_VALUE);
1317+
int memory = NumbersUtil.parseInt(customParameters.get(UsageEventVO.DynamicParameters.memory.name()), -1);
1318+
int maxRAMSize = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_RAM_SIZE.value() == 0 ? Integer.MAX_VALUE: ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_RAM_SIZE.value();
1319+
if (memory < minMemory || memory > maxMemory || memory > maxRAMSize) {
1320+
throw new InvalidParameterValueException(String.format("Invalid memory value, specify a value between %d and %d", minMemory, Math.min(maxRAMSize, maxMemory)));
1321+
}
1322+
} else if (customParameters.containsKey(UsageEventVO.DynamicParameters.memory.name())) {
1323+
throw new InvalidParameterValueException("The memory of this offering id:" + serviceOffering.getUuid() + " is not customizable. This is predefined in the Template.");
1324+
}
13271325
}
13281326

13291327
private UserVm upgradeStoppedVirtualMachine(Long vmId, Long svcOffId, Map<String, String> customParameters) throws ResourceAllocationException {

‎server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java‎

Lines changed: 117 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,13 @@
3838
import java.util.ArrayList;
3939
import java.util.Collections;
4040
import java.util.HashMap;
41+
import java.util.HashSet;
42+
import java.util.LinkedHashMap;
43+
import java.util.LinkedHashSet;
44+
import java.util.LinkedList;
4145
import java.util.List;
4246
import java.util.Map;
4347

44-
import com.cloud.network.NetworkService;
4548
import org.apache.cloudstack.acl.ControlledEntity;
4649
import org.apache.cloudstack.acl.SecurityChecker;
4750
import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
@@ -65,6 +68,7 @@
6568
import org.junit.runner.RunWith;
6669
import org.mockito.InjectMocks;
6770
import org.mockito.Mock;
71+
import org.mockito.MockedStatic;
6872
import org.mockito.Mockito;
6973
import org.mockito.Spy;
7074
import org.mockito.junit.MockitoJUnitRunner;
@@ -80,6 +84,10 @@
8084
import com.cloud.deploy.DeployDestination;
8185
import com.cloud.deploy.DeploymentPlanner;
8286
import com.cloud.deploy.DeploymentPlanningManager;
87+
import com.cloud.domain.DomainVO;
88+
import com.cloud.domain.dao.DomainDao;
89+
import com.cloud.event.UsageEventUtils;
90+
import com.cloud.event.UsageEventVO;
8391
import com.cloud.exception.InsufficientAddressCapacityException;
8492
import com.cloud.exception.InsufficientCapacityException;
8593
import com.cloud.exception.InsufficientServerCapacityException;
@@ -91,15 +99,33 @@
9199
import com.cloud.host.HostVO;
92100
import com.cloud.host.dao.HostDao;
93101
import com.cloud.hypervisor.Hypervisor;
102+
import com.cloud.network.Network;
94103
import com.cloud.network.NetworkModel;
104+
import com.cloud.network.NetworkService;
105+
import com.cloud.network.dao.FirewallRulesDao;
106+
import com.cloud.network.dao.IPAddressDao;
107+
import com.cloud.network.dao.IPAddressVO;
108+
import com.cloud.network.dao.LoadBalancerVMMapDao;
109+
import com.cloud.network.dao.LoadBalancerVMMapVO;
95110
import com.cloud.network.dao.NetworkDao;
96111
import com.cloud.network.dao.NetworkVO;
112+
import com.cloud.network.dao.PhysicalNetworkDao;
113+
import com.cloud.network.dao.PhysicalNetworkVO;
114+
import com.cloud.network.guru.NetworkGuru;
115+
import com.cloud.network.rules.FirewallRuleVO;
116+
import com.cloud.network.rules.PortForwardingRule;
117+
import com.cloud.network.rules.dao.PortForwardingRulesDao;
118+
import com.cloud.network.security.SecurityGroupManager;
97119
import com.cloud.network.security.SecurityGroupVO;
98120
import com.cloud.offering.DiskOffering;
121+
import com.cloud.offering.NetworkOffering;
99122
import com.cloud.offering.ServiceOffering;
123+
import com.cloud.offerings.NetworkOfferingVO;
124+
import com.cloud.offerings.dao.NetworkOfferingDao;
100125
import com.cloud.server.ManagementService;
101126
import com.cloud.service.ServiceOfferingVO;
102127
import com.cloud.service.dao.ServiceOfferingDao;
128+
import com.cloud.service.dao.ServiceOfferingDetailsDao;
103129
import com.cloud.storage.DiskOfferingVO;
104130
import com.cloud.storage.GuestOSVO;
105131
import com.cloud.storage.ScopeType;
@@ -136,31 +162,6 @@
136162
import com.cloud.vm.dao.UserVmDetailsDao;
137163
import com.cloud.vm.snapshot.VMSnapshotVO;
138164
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
139-
import org.mockito.MockedStatic;
140-
141-
import java.util.HashSet;
142-
import java.util.LinkedHashMap;
143-
import java.util.LinkedHashSet;
144-
import java.util.LinkedList;
145-
import com.cloud.domain.DomainVO;
146-
import com.cloud.domain.dao.DomainDao;
147-
import com.cloud.event.UsageEventUtils;
148-
import com.cloud.network.Network;
149-
import com.cloud.network.dao.FirewallRulesDao;
150-
import com.cloud.network.dao.IPAddressDao;
151-
import com.cloud.network.dao.IPAddressVO;
152-
import com.cloud.network.dao.LoadBalancerVMMapDao;
153-
import com.cloud.network.dao.LoadBalancerVMMapVO;
154-
import com.cloud.network.dao.PhysicalNetworkDao;
155-
import com.cloud.network.dao.PhysicalNetworkVO;
156-
import com.cloud.network.guru.NetworkGuru;
157-
import com.cloud.network.rules.FirewallRuleVO;
158-
import com.cloud.network.rules.PortForwardingRule;
159-
import com.cloud.network.rules.dao.PortForwardingRulesDao;
160-
import com.cloud.network.security.SecurityGroupManager;
161-
import com.cloud.offering.NetworkOffering;
162-
import com.cloud.offerings.NetworkOfferingVO;
163-
import com.cloud.offerings.dao.NetworkOfferingDao;
164165

165166
@RunWith(MockitoJUnitRunner.class)
166167
public class UserVmManagerImplTest {
@@ -370,6 +371,9 @@ public class UserVmManagerImplTest {
370371
@Mock
371372
NetworkService networkServiceMock;
372373

374+
@Mock
375+
ServiceOfferingDetailsDao serviceOfferingDetailsDao;
376+
373377
private static final long vmId = 1l;
374378
private static final long zoneId = 2L;
375379
private static final long accountId = 3L;
@@ -3121,4 +3125,91 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab
31213125
Mockito.verify(userVmManagerImpl, Mockito.never()).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
31223126
}
31233127
}
3128+
3129+
private ServiceOfferingVO getMockedServiceOffering(boolean custom, boolean customSpeed) {
3130+
ServiceOfferingVO serviceOffering = mock(ServiceOfferingVO.class);
3131+
when(serviceOffering.getUuid()).thenReturn("offering-uuid");
3132+
when(serviceOffering.isDynamic()).thenReturn(custom);
3133+
when(serviceOffering.isCustomCpuSpeedSupported()).thenReturn(customSpeed);
3134+
if (custom) {
3135+
when(serviceOffering.getCpu()).thenReturn(null);
3136+
when(serviceOffering.getRamSize()).thenReturn(null);
3137+
} else {
3138+
when(serviceOffering.getCpu()).thenReturn(2);
3139+
when(serviceOffering.getRamSize()).thenReturn(2048);
3140+
}
3141+
if (customSpeed) {
3142+
when(serviceOffering.getSpeed()).thenReturn(null);
3143+
} else {
3144+
when(serviceOffering.isCustomCpuSpeedSupported()).thenReturn(false);
3145+
when(serviceOffering.getSpeed()).thenReturn(2500);
3146+
}
3147+
System.out.println("Service Offering customized: " + serviceOffering.isCustomized() + ", speed: " + serviceOffering.getSpeed());
3148+
return serviceOffering;
3149+
}
3150+
3151+
@Test
3152+
public void customOfferingNeedsCustomizationThrowsException() {
3153+
ServiceOfferingVO serviceOffering = getMockedServiceOffering(true, true);
3154+
InvalidParameterValueException ex = Assert.assertThrows(InvalidParameterValueException.class, () ->
3155+
userVmManagerImpl.validateCustomParameters(serviceOffering, Collections.emptyMap()));
3156+
assertEquals("Need to specify custom parameter values cpu, cpu speed and memory when using custom offering", ex.getMessage());
3157+
}
3158+
3159+
@Test
3160+
public void cpuSpeedCustomizationNotAllowedThrowsException() {
3161+
ServiceOfferingVO serviceOffering = getMockedServiceOffering(true, false);
3162+
3163+
Map<String, String> customParameters = new HashMap<>();
3164+
customParameters.put(UsageEventVO.DynamicParameters.cpuSpeed.name(), "2500");
3165+
3166+
InvalidParameterValueException ex = Assert.assertThrows(InvalidParameterValueException.class, () ->
3167+
userVmManagerImpl.validateCustomParameters(serviceOffering, customParameters));
3168+
Assert.assertTrue(ex.getMessage().startsWith("The CPU speed of this offering"));
3169+
}
3170+
3171+
@Test
3172+
public void cpuSpeedCustomizationAllowedDoesNotThrowException() {
3173+
ServiceOfferingVO serviceOffering = getMockedServiceOffering(true, true);
3174+
3175+
Map<String, String> customParameters = new HashMap<>();
3176+
customParameters.put(UsageEventVO.DynamicParameters.cpuSpeed.name(), "2500");
3177+
3178+
userVmManagerImpl.validateCustomParameters(serviceOffering, customParameters);
3179+
}
3180+
3181+
@Test
3182+
public void verifyVmLimits_fixedOffering_throwsException() {
3183+
when(userVmVoMock.getId()).thenReturn(1L);
3184+
when(userVmVoMock.getServiceOfferingId()).thenReturn(1L);
3185+
when(accountDao.findById(anyLong())).thenReturn(callerAccount);
3186+
ServiceOfferingVO serviceOffering = getMockedServiceOffering(false, false);
3187+
when(_serviceOfferingDao.findById(anyLong())).thenReturn(serviceOffering);
3188+
when(_serviceOfferingDao.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(serviceOffering);
3189+
3190+
Map<String, String> customParameters = new HashMap<>();
3191+
customParameters.put(VmDetailConstants.CPU_SPEED, "2500");
3192+
3193+
InvalidParameterValueException ex = Assert.assertThrows(InvalidParameterValueException.class, () ->
3194+
userVmManagerImpl.verifyVmLimits(userVmVoMock, customParameters));
3195+
assertEquals("CPU number, Memory and CPU speed cannot be updated for a non-dynamic offering", ex.getMessage());
3196+
}
3197+
3198+
@Test
3199+
public void verifyVmLimits_constrainedOffering_throwsException() {
3200+
when(userVmVoMock.getId()).thenReturn(1L);
3201+
when(userVmVoMock.getServiceOfferingId()).thenReturn(1L);
3202+
when(accountDao.findById(anyLong())).thenReturn(callerAccount);
3203+
ServiceOfferingVO serviceOffering = getMockedServiceOffering(true, false);
3204+
when(_serviceOfferingDao.findById(anyLong())).thenReturn(serviceOffering);
3205+
when(_serviceOfferingDao.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(serviceOffering);
3206+
3207+
Map<String, String> customParameters = new HashMap<>();
3208+
customParameters.put(VmDetailConstants.CPU_NUMBER, "1");
3209+
customParameters.put(VmDetailConstants.CPU_SPEED, "2500");
3210+
3211+
InvalidParameterValueException ex = Assert.assertThrows(InvalidParameterValueException.class, () ->
3212+
userVmManagerImpl.verifyVmLimits(userVmVoMock, customParameters));
3213+
Assert.assertTrue(ex.getMessage().startsWith("The CPU speed of this offering"));
3214+
}
31243215
}

0 commit comments

Comments
 (0)