This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 4de2f38cd13 Adding vmId as part of error response when vm create
fails. (#8484)
4de2f38cd13 is described below
commit 4de2f38cd13fbc9266e81c9da4360f32478fe209
Author: anniejili <47866640+anniej...@users.noreply.github.com>
AuthorDate: Thu Feb 8 10:30:29 2024 -0800
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.hoogl...@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...@apple.com>
Co-authored-by: dahn <daan.hoogl...@gmail.com>
Co-authored-by: dahn <d...@onecht.net>
Co-authored-by: Rohit Yadav <rohit.ya...@shapeblue.com>
Co-authored-by: Rohit Yadav <rohityada...@gmail.com>
---
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 66 +++++++++++++++++-----
.../java/com/cloud/vm/UserVmManagerImplTest.java | 60 ++++++++++++++++++--
2 files changed, 106 insertions(+), 20 deletions(-)
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index c68f51cbda4..063ed09c839 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -51,6 +51,7 @@ import javax.naming.ConfigurationException;
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;
@@ -4474,7 +4475,9 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
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;
}
validateRootDiskResize(hypervisorType, rootDiskSize, templateVO,
vm, customParameters);
@@ -4545,6 +4548,43 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
DiskOfferingVO rootDiskOfferingVO =
_diskOfferingDao.findById(rootDiskOfferingId);
rootDiskTags.add(rootDiskOfferingVO.getTags());
+ orchestrateVirtualMachineCreate(vm, guestOSCategory, computeTags,
rootDiskTags, plan, rootDiskSize, template, hostName, displayName, owner,
+ diskOfferingId, diskSize, offering, isIso,networkNicMap,
hypervisorType, extraDhcpOptionMap, dataDiskTemplateToDiskOfferingMap,
+ rootDiskOfferingId);
+
+ }
+ CallContext.current().setEventDetails("Vm Id: " + vm.getUuid());
+
+ if (!isImport) {
+ if (!offering.isDynamic()) {
+ UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE,
accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(),
template.getId(),
+ hypervisorType.toString(),
VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
+ } else {
+ UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE,
accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(),
template.getId(),
+ hypervisorType.toString(),
VirtualMachine.class.getName(), vm.getUuid(), customParameters,
vm.isDisplayVm());
+ }
+
+ 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(),
ApiConstants.VIRTUAL_MACHINE_ID);
+ }
+ throw cre;
+ }
+ }
+ return vm;
+ }
+
+ 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,
+ Long diskOfferingId, Long diskSize,
+ ServiceOffering offering, boolean
isIso, LinkedHashMap<String, List<NicProfile>> networkNicMap,
+ HypervisorType hypervisorType,
+ Map<String, Map<Integer, String>>
extraDhcpOptionMap, Map<Long, DiskOffering> dataDiskTemplateToDiskOfferingMap,
+ Long rootDiskOfferingId) throws
InsufficientCapacityException{
+ 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,
@@ -4558,22 +4598,20 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
if (logger.isDebugEnabled()) {
logger.debug("Successfully allocated DB entry for " + vm);
}
- }
- CallContext.current().setEventDetails("Vm Id: " + vm.getUuid());
+ } catch (CloudRuntimeException cre) {
+ ArrayList<ExceptionProxyObject> epoList = cre.getIdProxyList();
+ if (epoList == null || !epoList.stream().anyMatch(e ->
e.getUuid().equals(vm.getUuid()))) {
+ cre.addProxyObject(vm.getUuid(),
ApiConstants.VIRTUAL_MACHINE_ID);
- if (!isImport) {
- if (!offering.isDynamic()) {
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE,
accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(),
template.getId(),
- hypervisorType.toString(),
VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- } else {
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE,
accountId, zone.getId(), vm.getId(), vm.getHostName(), offering.getId(),
template.getId(),
- 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()));
+ 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;
}
- return vm;
}
protected void setVmRequiredFieldsForImport(boolean isImport, UserVmVO vm,
DataCenter zone, HypervisorType hypervisorType,
diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
index 1b353c8626a..13b937789f2 100644
--- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
+++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
@@ -76,6 +76,7 @@ import com.cloud.uservm.UserVm;
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;
@@ -114,7 +115,10 @@ import java.util.Map;
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;
@@ -1043,11 +1047,15 @@ public class UserVmManagerImplTest {
@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) {
@@ -1106,7 +1114,7 @@ public class UserVmManagerImplTest {
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);
@@ -1227,6 +1235,46 @@ public class UserVmManagerImplTest {
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);
