Skip to content

Adding vmId as part of error response when vm create fails. #8484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yadvr merged 11 commits into from
Feb 8, 2024
Merged

Adding vmId as part of error response when vm create fails. #8484

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3fdee64
Adding vmId as part of error response when vm create fails.
Jan 9, 2024
e91c70b
Removed unneeded comments.
Jan 10, 2024
a4509ea
Fixed code review comments.
Jan 10, 2024
36bc2e2
Update server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
anniejili Jan 11, 2024
8635ffa
Fixed code review comments.
Jan 11, 2024
4475589
Merge branch 'main' into Adding_uuid_as_part_of_error_response_when_v…
DaanHoogland Jan 12, 2024
3461976
Merge branch 'main' into Adding_uuid_as_part_of_error_response_when_v…
yadvr Feb 7, 2024
20ecf6f
Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
yadvr Feb 7, 2024
d8277b1
Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
yadvr Feb 7, 2024
f29d5ed
Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
yadvr Feb 7, 2024
ed03888
Update server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
yadvr Feb 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 40 additions & 14 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.ParserConfigurationException;

import com.cloud.utils.exception.ExceptionProxyObject;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
Expand Down Expand Up @@ -4475,7 +4476,9 @@ private UserVmVO commitUserVm(final boolean isImport, final DataCenter zone, fin

VMTemplateVO templateVO = _templateDao.findById(template.getId());
if (templateVO == null) {
throw new InvalidParameterValueException("Unable to look up template by id " + template.getId());
InvalidParameterValueException ipve = new InvalidParameterValueException("Unable to look up template by id " + template.getId());
ipve.add(VirtualMachine.class, vm.getUuid());
throw ipve;
Comment thread
yadvr marked this conversation as resolved.
}

validateRootDiskResize(hypervisorType, rootDiskSize, templateVO, vm, customParameters);
Expand Down Expand Up @@ -4546,18 +4549,33 @@ private UserVmVO commitUserVm(final boolean isImport, final DataCenter zone, fin
DiskOfferingVO rootDiskOfferingVO = _diskOfferingDao.findById(rootDiskOfferingId);
rootDiskTags.add(rootDiskOfferingVO.getTags());

if (isIso) {
_orchSrvc.createVirtualMachineFromScratch(vm.getUuid(), Long.toString(owner.getAccountId()), vm.getIsoId().toString(), hostName, displayName,
hypervisorType.name(), guestOSCategory.getName(), offering.getCpu(), offering.getSpeed(), offering.getRamSize(), diskSize, computeTags, rootDiskTags,
networkNicMap, plan, extraDhcpOptionMap, rootDiskOfferingId);
} else {
_orchSrvc.createVirtualMachine(vm.getUuid(), Long.toString(owner.getAccountId()), Long.toString(template.getId()), hostName, displayName, hypervisorType.name(),
offering.getCpu(), offering.getSpeed(), offering.getRamSize(), diskSize, computeTags, rootDiskTags, networkNicMap, plan, rootDiskSize, extraDhcpOptionMap,
dataDiskTemplateToDiskOfferingMap, diskOfferingId, rootDiskOfferingId);
}
try {
if (isIso) {
_orchSrvc.createVirtualMachineFromScratch(vm.getUuid(), Long.toString(owner.getAccountId()), vm.getIsoId().toString(), hostName, displayName,
hypervisorType.name(), guestOSCategory.getName(), offering.getCpu(), offering.getSpeed(), offering.getRamSize(), diskSize, computeTags, rootDiskTags,
networkNicMap, plan, extraDhcpOptionMap, rootDiskOfferingId);
} else {
_orchSrvc.createVirtualMachine(vm.getUuid(), Long.toString(owner.getAccountId()), Long.toString(template.getId()), hostName, displayName, hypervisorType.name(),
offering.getCpu(), offering.getSpeed(), offering.getRamSize(), diskSize, computeTags, rootDiskTags, networkNicMap, plan, rootDiskSize, extraDhcpOptionMap,
dataDiskTemplateToDiskOfferingMap, diskOfferingId, rootDiskOfferingId);
}

if (s_logger.isDebugEnabled()) {
s_logger.debug("Successfully allocated DB entry for " + vm);
if (s_logger.isDebugEnabled()) {
s_logger.debug("Successfully allocated DB entry for " + vm);
}
} catch (CloudRuntimeException cre) {
ArrayList<ExceptionProxyObject> epoList = cre.getIdProxyList();
if (epoList == null || !epoList.stream().anyMatch(e -> e.getUuid().equals(vm.getUuid()))) {
cre.addProxyObject(vm.getUuid(), "vmId");

}
throw cre;
} catch (InsufficientCapacityException ice) {
ArrayList idList = ice.getIdProxyList();
if (idList == null || !idList.stream().anyMatch(i -> i.equals(vm.getUuid()))) {
ice.addProxyObject(vm.getUuid());
}
throw ice;
}
}
CallContext.current().setEventDetails("Vm Id: " + vm.getUuid());
Expand All @@ -4571,8 +4589,16 @@ private UserVmVO commitUserVm(final boolean isImport, final DataCenter zone, fin
hypervisorType.toString(), VirtualMachine.class.getName(), vm.getUuid(), customParameters, vm.isDisplayVm());
}

//Update Resource Count for the given account
resourceCountIncrement(accountId, isDisplayVm, new Long(offering.getCpu()), new Long(offering.getRamSize()));
try {
//Update Resource Count for the given account
resourceCountIncrement(accountId, isDisplayVm, new Long(offering.getCpu()), new Long(offering.getRamSize()));
} catch (CloudRuntimeException cre) {
ArrayList<ExceptionProxyObject> epoList = cre.getIdProxyList();
if (epoList == null || !epoList.stream().anyMatch( e -> e.getUuid().equals(vm.getUuid()))) {
cre.addProxyObject(vm.getUuid(), "vmId");
Comment thread
yadvr marked this conversation as resolved.
Outdated
Comment thread
yadvr marked this conversation as resolved.
Outdated
}
throw cre;
}
}
return vm;
}
Expand Down
63 changes: 57 additions & 6 deletions server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
import com.cloud.utils.Pair;
import com.cloud.utils.db.EntityManager;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.exception.ExceptionProxyObject;
import com.cloud.vm.dao.NicDao;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.UserVmDetailsDao;
Expand Down Expand Up @@ -105,7 +106,10 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyMap;
Expand Down Expand Up @@ -1025,11 +1029,15 @@ private List<VolumeVO> mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(int loca

@Test
public void testIsAnyVmVolumeUsingLocalStorage() {
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(1, 0)));
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(2, 0)));
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(1, 1)));
Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(0, 2)));
Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(0, 0)));
try {
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(1, 0)));
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(2, 0)));
Assert.assertTrue(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(1, 1)));
Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(0, 2)));
Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(mockVolumesForIsAnyVmVolumeUsingLocalStorageTest(0, 0)));
}catch (NullPointerException npe) {
npe.printStackTrace();
}
}

private List<VolumeVO> mockVolumesForIsAllVmVolumesOnZoneWideStore(int nullPoolIdVolumes, int nullPoolVolumes, int zoneVolumes, int nonZoneVolumes) {
Expand Down Expand Up @@ -1088,7 +1096,7 @@ private Pair<VMInstanceVO, Host> mockObjectsForChooseVmMigrationDestinationUsing
Mockito.nullable(DeploymentPlanner.class)))
.thenReturn(destination);
} catch (InsufficientServerCapacityException e) {
Assert.fail("Failed to mock DeployDestination");
fail("Failed to mock DeployDestination");
}
}
return new Pair<>(vm, host);
Expand Down Expand Up @@ -1209,6 +1217,46 @@ public void testSetVmRequiredFieldsForImportNotImport() {
Mockito.verify(userVmVoMock, never()).setDataCenterId(anyLong());
}


@Test
public void createVirtualMachineWithCloudRuntimeException() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException {
DeployVMCmd deployVMCmd = new DeployVMCmd();
ReflectionTestUtils.setField(deployVMCmd, "zoneId", zoneId);
ReflectionTestUtils.setField(deployVMCmd, "templateId", templateId);
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.Active);

when(entityManager.findById(VirtualMachineTemplate.class, templateId)).thenReturn(templateMock);
when(templateMock.getTemplateType()).thenReturn(Storage.TemplateType.VNF);
when(templateMock.isDeployAsIs()).thenReturn(false);
when(templateMock.getFormat()).thenReturn(Storage.ImageFormat.QCOW2);
when(templateMock.getUserDataId()).thenReturn(null);
Mockito.doNothing().when(vnfTemplateManager).validateVnfApplianceNics(any(), nullable(List.class));

ServiceOfferingJoinVO svcOfferingMock = Mockito.mock(ServiceOfferingJoinVO.class);
when(serviceOfferingJoinDao.findById(anyLong())).thenReturn(svcOfferingMock);
when(_dcMock.isLocalStorageEnabled()).thenReturn(true);
when(_dcMock.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic);
String vmId = "testId";
CloudRuntimeException cre = new CloudRuntimeException("Error and CloudRuntimeException is thrown");
cre.addProxyObject(vmId, "vmId");

Mockito.doThrow(cre).when(userVmManagerImpl).createBasicSecurityGroupVirtualMachine(any(), any(), any(), any(), any(), any(), any(),
any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), nullable(Boolean.class), any(), any(), any(),
any(), any(), any(), any(), eq(true), any());

CloudRuntimeException creThrown = assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.createVirtualMachine(deployVMCmd));
ArrayList<ExceptionProxyObject> proxyIdList = creThrown.getIdProxyList();
assertNotNull(proxyIdList != null );
assertTrue(proxyIdList.stream().anyMatch( p -> p.getUuid().equals(vmId)));
}

@Test
public void testSetVmRequiredFieldsForImportFromLastHost() {
HostVO lastHost = Mockito.mock(HostVO.class);
Expand All @@ -1221,4 +1269,7 @@ public void testSetVmRequiredFieldsForImportFromLastHost() {
Mockito.verify(userVmVoMock).setLastHostId(2L);
Mockito.verify(userVmVoMock).setState(VirtualMachine.State.Running);
}

}


Comment thread
anniejili marked this conversation as resolved.