Skip to content

Commit 4de2f38

Browse files
anniejiliAnnie LiDaanHooglandyadvr
authored
Adding vmId as part of error response when vm create fails. (#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 bb70da0 commit 4de2f38

File tree

2 files changed

+106
-20
lines changed

2 files changed

+106
-20
lines changed

‎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;
@@ -4474,7 +4475,9 @@ private UserVmVO commitUserVm(final boolean isImport, final DataCenter zone, fin
44744475

44754476
VMTemplateVO templateVO = _templateDao.findById(template.getId());
44764477
if (templateVO == null) {
4477-
throw new InvalidParameterValueException("Unable to look up template by id " + template.getId());
4478+
InvalidParameterValueException ipve = new InvalidParameterValueException("Unable to look up template by id " + template.getId());
4479+
ipve.add(VirtualMachine.class, vm.getUuid());
4480+
throw ipve;
44784481
}
44794482

44804483
validateRootDiskResize(hypervisorType, rootDiskSize, templateVO, vm, customParameters);
@@ -4545,6 +4548,43 @@ private UserVmVO commitUserVm(final boolean isImport, final DataCenter zone, fin
45454548
DiskOfferingVO rootDiskOfferingVO = _diskOfferingDao.findById(rootDiskOfferingId);
45464549
rootDiskTags.add(rootDiskOfferingVO.getTags());
45474550

4551+
orchestrateVirtualMachineCreate(vm, guestOSCategory, computeTags, rootDiskTags, plan, rootDiskSize, template, hostName, displayName, owner,
4552+
diskOfferingId, diskSize, offering, isIso,networkNicMap, hypervisorType, extraDhcpOptionMap, dataDiskTemplateToDiskOfferingMap,
4553+
rootDiskOfferingId);
4554+
4555+
}
4556+
CallContext.current().setEventDetails("Vm Id: " + vm.getUuid());
4557+
4558+
if (!isImport) {
4559+
if (!offering.isDynamic()) {
4560+
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(), template.getId(),
4561+
hypervisorType.toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
4562+
} else {
4563+
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(), template.getId(),
4564+
hypervisorType.toString(), VirtualMachine.class.getName(), vm.getUuid(), customParameters, vm.isDisplayVm());
4565+
}
4566+
4567+
try {
4568+
//Update Resource Count for the given account
4569+
resourceCountIncrement(accountId, isDisplayVm, new Long(offering.getCpu()), new Long(offering.getRamSize()));
4570+
} catch (CloudRuntimeException cre) {
4571+
ArrayList<ExceptionProxyObject> epoList = cre.getIdProxyList();
4572+
if (epoList == null || !epoList.stream().anyMatch( e -> e.getUuid().equals(vm.getUuid()))) {
4573+
cre.addProxyObject(vm.getUuid(), ApiConstants.VIRTUAL_MACHINE_ID);
4574+
}
4575+
throw cre;
4576+
}
4577+
}
4578+
return vm;
4579+
}
4580+
4581+
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,
4582+
Long diskOfferingId, Long diskSize,
4583+
ServiceOffering offering, boolean isIso, LinkedHashMap<String, List<NicProfile>> networkNicMap,
4584+
HypervisorType hypervisorType,
4585+
Map<String, Map<Integer, String>> extraDhcpOptionMap, Map<Long, DiskOffering> dataDiskTemplateToDiskOfferingMap,
4586+
Long rootDiskOfferingId) throws InsufficientCapacityException{
4587+
try {
45484588
if (isIso) {
45494589
_orchSrvc.createVirtualMachineFromScratch(vm.getUuid(), Long.toString(owner.getAccountId()), vm.getIsoId().toString(), hostName, displayName,
45504590
hypervisorType.name(), guestOSCategory.getName(), offering.getCpu(), offering.getSpeed(), offering.getRamSize(), diskSize, computeTags, rootDiskTags,
@@ -4558,22 +4598,20 @@ private UserVmVO commitUserVm(final boolean isImport, final DataCenter zone, fin
45584598
if (logger.isDebugEnabled()) {
45594599
logger.debug("Successfully allocated DB entry for " + vm);
45604600
}
4561-
}
4562-
CallContext.current().setEventDetails("Vm Id: " + vm.getUuid());
4601+
} catch (CloudRuntimeException cre) {
4602+
ArrayList<ExceptionProxyObject> epoList = cre.getIdProxyList();
4603+
if (epoList == null || !epoList.stream().anyMatch(e -> e.getUuid().equals(vm.getUuid()))) {
4604+
cre.addProxyObject(vm.getUuid(), ApiConstants.VIRTUAL_MACHINE_ID);
45634605

4564-
if (!isImport) {
4565-
if (!offering.isDynamic()) {
4566-
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(), template.getId(),
4567-
hypervisorType.toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
4568-
} else {
4569-
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(), template.getId(),
4570-
hypervisorType.toString(), VirtualMachine.class.getName(), vm.getUuid(), customParameters, vm.isDisplayVm());
45714606
}
4572-
4573-
//Update Resource Count for the given account
4574-
resourceCountIncrement(accountId, isDisplayVm, new Long(offering.getCpu()), new Long(offering.getRamSize()));
4607+
throw cre;
4608+
} catch (InsufficientCapacityException ice) {
4609+
ArrayList idList = ice.getIdProxyList();
4610+
if (idList == null || !idList.stream().anyMatch(i -> i.equals(vm.getUuid()))) {
4611+
ice.addProxyObject(vm.getUuid());
4612+
}
4613+
throw ice;
45754614
}
4576-
return vm;
45774615
}
45784616

45794617
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;
@@ -1043,11 +1047,15 @@ private List<VolumeVO> mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(int loca
10431047

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

10531061
private List<VolumeVO> mockVolumesForIsAllVmVolumesOnZoneWideStore(int nullPoolIdVolumes, int nullPoolVolumes, int zoneVolumes, int nonZoneVolumes) {
@@ -1106,7 +1114,7 @@ private Pair<VMInstanceVO, Host> mockObjectsForChooseVmMigrationDestinationUsing
11061114
Mockito.nullable(DeploymentPlanner.class)))
11071115
.thenReturn(destination);
11081116
} catch (InsufficientServerCapacityException e) {
1109-
Assert.fail("Failed to mock DeployDestination");
1117+
fail("Failed to mock DeployDestination");
11101118
}
11111119
}
11121120
return new Pair<>(vm, host);
@@ -1227,6 +1235,46 @@ public void testSetVmRequiredFieldsForImportNotImport() {
12271235
Mockito.verify(userVmVoMock, never()).setDataCenterId(anyLong());
12281236
}
12291237

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

0 commit comments

Comments
 (0)