Skip to content

Commit e7891c8

Browse files
anniejiliAnnie LiDaanHooglandyadvr
authored and
dhslove
committed
Adding vmId as part of error response when vm create fails. (apache#8484)
* Adding vmId as part of error response when vm create fails. * Removed unneeded comments. * Fixed code review comments. * Update server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java Co-authored-by: dahn <daan.hoogland@gmail.com> * Fixed code review comments. * Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java * Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java * Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java * Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java --------- Co-authored-by: Annie Li <ji_li@apple.com> Co-authored-by: dahn <daan.hoogland@gmail.com> Co-authored-by: dahn <daan@onecht.net> Co-authored-by: Rohit Yadav <rohit.yadav@shapeblue.com> Co-authored-by: Rohit Yadav <rohityadav89@gmail.com>
1 parent aa1ab86 commit e7891c8

2 files changed

Lines changed: 106 additions & 20 deletions

File tree

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

Lines changed: 52 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import javax.xml.parsers.DocumentBuilder;
5252
import javax.xml.parsers.ParserConfigurationException;
5353

54+
import com.cloud.utils.exception.ExceptionProxyObject;
5455
import org.apache.cloudstack.acl.ControlledEntity;
5556
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
5657
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
@@ -4497,7 +4498,9 @@ private UserVmVO commitUserVm(final boolean isImport, final DataCenter zone, fin
44974498

44984499
VMTemplateVO templateVO = _templateDao.findById(template.getId());
44994500
if (templateVO == null) {
4500-
throw new InvalidParameterValueException("Unable to look up template by id " + template.getId());
4501+
InvalidParameterValueException ipve = new InvalidParameterValueException("Unable to look up template by id " + template.getId());
4502+
ipve.add(VirtualMachine.class, vm.getUuid());
4503+
throw ipve;
45014504
}
45024505

45034506
validateRootDiskResize(hypervisorType, rootDiskSize, templateVO, vm, customParameters);
@@ -4568,6 +4571,43 @@ private UserVmVO commitUserVm(final boolean isImport, final DataCenter zone, fin
45684571
DiskOfferingVO rootDiskOfferingVO = _diskOfferingDao.findById(rootDiskOfferingId);
45694572
rootDiskTags.add(rootDiskOfferingVO.getTags());
45704573

4574+
orchestrateVirtualMachineCreate(vm, guestOSCategory, computeTags, rootDiskTags, plan, rootDiskSize, template, hostName, displayName, owner,
4575+
diskOfferingId, diskSize, offering, isIso,networkNicMap, hypervisorType, extraDhcpOptionMap, dataDiskTemplateToDiskOfferingMap,
4576+
rootDiskOfferingId);
4577+
4578+
}
4579+
CallContext.current().setEventDetails("Vm Id: " + vm.getUuid());
4580+
4581+
if (!isImport) {
4582+
if (!offering.isDynamic()) {
4583+
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(), template.getId(),
4584+
hypervisorType.toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
4585+
} else {
4586+
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(), template.getId(),
4587+
hypervisorType.toString(), VirtualMachine.class.getName(), vm.getUuid(), customParameters, vm.isDisplayVm());
4588+
}
4589+
4590+
try {
4591+
//Update Resource Count for the given account
4592+
resourceCountIncrement(accountId, isDisplayVm, new Long(offering.getCpu()), new Long(offering.getRamSize()));
4593+
} catch (CloudRuntimeException cre) {
4594+
ArrayList<ExceptionProxyObject> epoList = cre.getIdProxyList();
4595+
if (epoList == null || !epoList.stream().anyMatch( e -> e.getUuid().equals(vm.getUuid()))) {
4596+
cre.addProxyObject(vm.getUuid(), ApiConstants.VIRTUAL_MACHINE_ID);
4597+
}
4598+
throw cre;
4599+
}
4600+
}
4601+
return vm;
4602+
}
4603+
4604+
private void orchestrateVirtualMachineCreate(UserVmVO vm, GuestOSCategoryVO guestOSCategory, List<String> computeTags, List<String> rootDiskTags, DataCenterDeployment plan, Long rootDiskSize, VirtualMachineTemplate template, String hostName, String displayName, Account owner,
4605+
Long diskOfferingId, Long diskSize,
4606+
ServiceOffering offering, boolean isIso, LinkedHashMap<String, List<NicProfile>> networkNicMap,
4607+
HypervisorType hypervisorType,
4608+
Map<String, Map<Integer, String>> extraDhcpOptionMap, Map<Long, DiskOffering> dataDiskTemplateToDiskOfferingMap,
4609+
Long rootDiskOfferingId) throws InsufficientCapacityException{
4610+
try {
45714611
if (isIso) {
45724612
_orchSrvc.createVirtualMachineFromScratch(vm.getUuid(), Long.toString(owner.getAccountId()), vm.getIsoId().toString(), hostName, displayName,
45734613
hypervisorType.name(), guestOSCategory.getName(), offering.getCpu(), offering.getSpeed(), offering.getRamSize(), diskSize, computeTags, rootDiskTags,
@@ -4581,22 +4621,20 @@ private UserVmVO commitUserVm(final boolean isImport, final DataCenter zone, fin
45814621
if (logger.isDebugEnabled()) {
45824622
logger.debug("Successfully allocated DB entry for " + vm);
45834623
}
4584-
}
4585-
CallContext.current().setEventDetails("Vm Id: " + vm.getUuid());
4624+
} catch (CloudRuntimeException cre) {
4625+
ArrayList<ExceptionProxyObject> epoList = cre.getIdProxyList();
4626+
if (epoList == null || !epoList.stream().anyMatch(e -> e.getUuid().equals(vm.getUuid()))) {
4627+
cre.addProxyObject(vm.getUuid(), ApiConstants.VIRTUAL_MACHINE_ID);
45864628

4587-
if (!isImport) {
4588-
if (!offering.isDynamic()) {
4589-
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(), template.getId(),
4590-
hypervisorType.toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
4591-
} else {
4592-
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(), template.getId(),
4593-
hypervisorType.toString(), VirtualMachine.class.getName(), vm.getUuid(), customParameters, vm.isDisplayVm());
45944629
}
4595-
4596-
//Update Resource Count for the given account
4597-
resourceCountIncrement(accountId, isDisplayVm, new Long(offering.getCpu()), new Long(offering.getRamSize()));
4630+
throw cre;
4631+
} catch (InsufficientCapacityException ice) {
4632+
ArrayList idList = ice.getIdProxyList();
4633+
if (idList == null || !idList.stream().anyMatch(i -> i.equals(vm.getUuid()))) {
4634+
ice.addProxyObject(vm.getUuid());
4635+
}
4636+
throw ice;
45984637
}
4599-
return vm;
46004638
}
46014639

46024640
protected void setVmRequiredFieldsForImport(boolean isImport, UserVmVO vm, DataCenter zone, HypervisorType hypervisorType,

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

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676
import com.cloud.utils.Pair;
7777
import com.cloud.utils.db.EntityManager;
7878
import com.cloud.utils.exception.CloudRuntimeException;
79+
import com.cloud.utils.exception.ExceptionProxyObject;
7980
import com.cloud.vm.dao.NicDao;
8081
import com.cloud.vm.dao.UserVmDao;
8182
import com.cloud.vm.dao.UserVmDetailsDao;
@@ -114,7 +115,10 @@
114115

115116
import static org.junit.Assert.assertEquals;
116117
import static org.junit.Assert.assertFalse;
118+
import static org.junit.Assert.assertNotNull;
119+
import static org.junit.Assert.assertThrows;
117120
import static org.junit.Assert.assertTrue;
121+
import static org.junit.Assert.fail;
118122
import static org.mockito.ArgumentMatchers.any;
119123
import static org.mockito.ArgumentMatchers.anyLong;
120124
import static org.mockito.ArgumentMatchers.anyMap;
@@ -1044,11 +1048,15 @@ private List<VolumeVO> mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(int loca
10441048

10451049
@Test
10461050
public void testIsAnyVmVolumeUsingLocalStorage() {
1047-
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(1, 0)));
1048-
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(2, 0)));
1049-
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(1, 1)));
1050-
Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(0, 2)));
1051-
Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(0, 0)));
1051+
try {
1052+
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(1, 0)));
1053+
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(2, 0)));
1054+
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(1, 1)));
1055+
Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(0, 2)));
1056+
Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(0, 0)));
1057+
}catch (NullPointerException npe) {
1058+
npe.printStackTrace();
1059+
}
10521060
}
10531061

10541062
private List<VolumeVO> mockVolumesForIsAllVmVolumesOnZoneWideStore(int nullPoolIdVolumes, int nullPoolVolumes, int zoneVolumes, int nonZoneVolumes) {
@@ -1107,7 +1115,7 @@ private Pair<VMInstanceVO, Host> mockObjectsForChooseVmMigrationDestinationUsing
11071115
Mockito.nullable(DeploymentPlanner.class)))
11081116
.thenReturn(destination);
11091117
} catch (InsufficientServerCapacityException e) {
1110-
Assert.fail("Failed to mock DeployDestination");
1118+
fail("Failed to mock DeployDestination");
11111119
}
11121120
}
11131121
return new Pair<>(vm, host);
@@ -1228,6 +1236,46 @@ public void testSetVmRequiredFieldsForImportNotImport() {
12281236
Mockito.verify(userVmVoMock, never()).setDataCenterId(anyLong());
12291237
}
12301238

1239+
1240+
@Test
1241+
public void createVirtualMachineWithCloudRuntimeException() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException {
1242+
DeployVMCmd deployVMCmd = new DeployVMCmd();
1243+
ReflectionTestUtils.setField(deployVMCmd, "zoneId", zoneId);
1244+
ReflectionTestUtils.setField(deployVMCmd, "templateId", templateId);
1245+
ReflectionTestUtils.setField(deployVMCmd, "serviceOfferingId", serviceOfferingId);
1246+
deployVMCmd._accountService = accountService;
1247+
1248+
when(accountService.finalyzeAccountId(nullable(String.class), nullable(Long.class), nullable(Long.class), eq(true))).thenReturn(accountId);
1249+
when(accountService.getActiveAccountById(accountId)).thenReturn(account);
1250+
when(entityManager.findById(DataCenter.class, zoneId)).thenReturn(_dcMock);
1251+
when(entityManager.findById(ServiceOffering.class, serviceOfferingId)).thenReturn(serviceOffering);
1252+
when(serviceOffering.getState()).thenReturn(ServiceOffering.State.Active);
1253+
1254+
when(entityManager.findById(VirtualMachineTemplate.class, templateId)).thenReturn(templateMock);
1255+
when(templateMock.getTemplateType()).thenReturn(Storage.TemplateType.VNF);
1256+
when(templateMock.isDeployAsIs()).thenReturn(false);
1257+
when(templateMock.getFormat()).thenReturn(Storage.ImageFormat.QCOW2);
1258+
when(templateMock.getUserDataId()).thenReturn(null);
1259+
Mockito.doNothing().when(vnfTemplateManager).validateVnfApplianceNics(any(), nullable(List.class));
1260+
1261+
ServiceOfferingJoinVO svcOfferingMock = Mockito.mock(ServiceOfferingJoinVO.class);
1262+
when(serviceOfferingJoinDao.findById(anyLong())).thenReturn(svcOfferingMock);
1263+
when(_dcMock.isLocalStorageEnabled()).thenReturn(true);
1264+
when(_dcMock.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic);
1265+
String vmId = "testId";
1266+
CloudRuntimeException cre = new CloudRuntimeException("Error and CloudRuntimeException is thrown");
1267+
cre.addProxyObject(vmId, "vmId");
1268+
1269+
Mockito.doThrow(cre).when(userVmManagerImpl).createBasicSecurityGroupVirtualMachine(any(), any(), any(), any(), any(), any(), any(),
1270+
any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), nullable(Boolean.class), any(), any(), any(),
1271+
any(), any(), any(), any(), eq(true), any());
1272+
1273+
CloudRuntimeException creThrown = assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.createVirtualMachine(deployVMCmd));
1274+
ArrayList<ExceptionProxyObject> proxyIdList = creThrown.getIdProxyList();
1275+
assertNotNull(proxyIdList != null );
1276+
assertTrue(proxyIdList.stream().anyMatch( p -> p.getUuid().equals(vmId)));
1277+
}
1278+
12311279
@Test
12321280
public void testSetVmRequiredFieldsForImportFromLastHost() {
12331281
HostVO lastHost = Mockito.mock(HostVO.class);

0 commit comments

Comments
 (0)