Skip to content

Commit

Permalink
Improvements and fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@nvazquez
nvazquez committed Jun 8, 2020
1 parent 7cb5e3b commit a7582d6
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 20 deletions.
30 changes: 23 additions & 7 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/vm/UnmanageVMInstanceCmd.java
View file
Expand Up @@ -27,12 +27,14 @@
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.user.Account;
import com.cloud.uservm.UserVm;
import com.cloud.vm.VirtualMachine;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiCommandJobType;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.ApiErrorCode;
import org.apache.cloudstack.api.BaseAsyncCmd;
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.ResponseObject;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.response.UnmanageVMInstanceResponse;
import org.apache.cloudstack.api.response.UserVmResponse;
Expand All @@ -44,8 +46,8 @@

@APICommand(name = UnmanageVMInstanceCmd.API_NAME,
description = "Unmanage a guest virtual machine.",
entityType = {VirtualMachine.class},
responseObject = UnmanageVMInstanceResponse.class,
responseView = ResponseObject.ResponseView.Full,
requestHasSensitiveInfo = false,
authorized = {RoleType.Admin},
since = "4.15.0")
Expand Down Expand Up @@ -94,14 +96,17 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE
UnmanageVMInstanceResponse response = new UnmanageVMInstanceResponse();
try {
CallContext.current().setEventDetails("VM ID = " + vmId);
unmanageVMManager.unmanageVMInstance(vmId);
response.setSuccess(true);
boolean result = unmanageVMManager.unmanageVMInstance(vmId);
response.setSuccess(result);
if (result) {
response.setDetails("VM unmanaged successfully");
}
} catch (Exception e) {
response.setSuccess(false);
response.setDetails(e.getMessage());
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage());
}
response.setResponseName(getCommandName());
setResponseObject(response);
response.setObjectName(getCommandName());
this.setResponseObject(response);
}

@Override
Expand All @@ -117,4 +122,15 @@ public long getEntityOwnerId() {
}
return Account.ACCOUNT_ID_SYSTEM;
}

@Override
public ApiCommandJobType getInstanceType() {
return ApiCommandJobType.VirtualMachine;
}

@Override
public Long getInstanceId() {
return vmId;
}

}
2 changes: 1 addition & 1 deletion ...n/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java
View file
Expand Up @@ -101,7 +101,7 @@ DiskProfile allocateRawVolume(Type type, String name, DiskOffering offering, Lon

void release(VirtualMachineProfile profile);

void cleanupVolumes(long vmId, boolean expunge) throws ConcurrentOperationException;
void cleanupVolumes(long vmId) throws ConcurrentOperationException;

void revokeAccess(DataObject dataObject, Host host, DataStore dataStore);

Expand Down
2 changes: 1 addition & 1 deletion engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
View file
Expand Up @@ -577,7 +577,7 @@ protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableExcepti
}

// Clean up volumes based on the vm's instance id
volumeMgr.cleanupVolumes(vm.getId(), true);
volumeMgr.cleanupVolumes(vm.getId());

if (hostId != null && CollectionUtils.isNotEmpty(targets)) {
removeDynamicTargets(hostId, targets);
Expand Down
13 changes: 5 additions & 8 deletions ...stration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
View file
Expand Up @@ -885,7 +885,7 @@ public void release(VirtualMachineProfile profile) {

@Override
@DB
public void cleanupVolumes(long vmId, boolean expunge) throws ConcurrentOperationException {
public void cleanupVolumes(long vmId) throws ConcurrentOperationException {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Cleaning storage for vm: " + vmId);
}
Expand All @@ -904,9 +904,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
} else {
s_logger.debug("Skipping destroy for the volume " + vol + " as its in state " + vol.getState().toString());
}
if (expunge) {
toBeExpunged.add(vol);
}
} else {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Detaching " + vol);
Expand All @@ -918,14 +915,14 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
});

AsyncCallFuture<VolumeApiResult> future = null;
for (VolumeVO volExpunge : toBeExpunged) {
future = volService.expungeVolumeAsync(volFactory.getVolume(volExpunge.getId()));
for (VolumeVO expunge : toBeExpunged) {
future = volService.expungeVolumeAsync(volFactory.getVolume(expunge.getId()));
try {
future.get();
} catch (InterruptedException e) {
s_logger.debug("failed expunge volume" + volExpunge.getId(), e);
s_logger.debug("failed expunge volume" + expunge.getId(), e);
} catch (ExecutionException e) {
s_logger.debug("failed expunge volume" + volExpunge.getId(), e);
s_logger.debug("failed expunge volume" + expunge.getId(), e);
}
}
}
Expand Down
23 changes: 20 additions & 3 deletions server/src/main/java/org/apache/cloudstack/vm/UnmanageVMManagerImpl.java
View file
Expand Up @@ -29,8 +29,10 @@
import com.cloud.agent.api.PrepareUnmanageVMInstanceCommand;
import com.cloud.event.ActionEvent;
import com.cloud.exception.UnsupportedServiceException;
import com.cloud.storage.Snapshot;
import com.cloud.storage.SnapshotVO;
import com.cloud.storage.dao.SnapshotDao;
import com.cloud.vm.NicVO;
import com.cloud.vm.UserVmVO;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.snapshot.VMSnapshotVO;
Expand Down Expand Up @@ -868,10 +870,10 @@ private void publishVMUsageUpdateResourceCount(final UserVm userVm, ServiceOffer
}
try {
if (!serviceOfferingVO.isDynamic()) {
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_IMPORT, userVm.getAccountId(), userVm.getDataCenterId(), userVm.getId(), userVm.getHostName(), serviceOfferingVO.getId(), userVm.getTemplateId(),
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, userVm.getAccountId(), userVm.getDataCenterId(), userVm.getId(), userVm.getHostName(), serviceOfferingVO.getId(), userVm.getTemplateId(),
userVm.getHypervisorType().toString(), VirtualMachine.class.getName(), userVm.getUuid(), userVm.isDisplayVm());
} else {
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_IMPORT, userVm.getAccountId(), userVm.getAccountId(), userVm.getDataCenterId(), userVm.getHostName(), serviceOfferingVO.getId(), userVm.getTemplateId(),
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, userVm.getAccountId(), userVm.getAccountId(), userVm.getDataCenterId(), userVm.getHostName(), serviceOfferingVO.getId(), userVm.getTemplateId(),
userVm.getHypervisorType().toString(), VirtualMachine.class.getName(), userVm.getUuid(), userVm.getDetails(), userVm.isDisplayVm());
}
} catch (Exception e) {
Expand All @@ -894,6 +896,17 @@ private void publishVMUsageUpdateResourceCount(final UserVm userVm, ServiceOffer
resourceLimitService.incrementResourceCount(userVm.getAccountId(), Resource.ResourceType.volume, volume.isDisplayVolume());
resourceLimitService.incrementResourceCount(userVm.getAccountId(), Resource.ResourceType.primary_storage, volume.isDisplayVolume(), volume.getSize());
}

List<NicVO> nics = nicDao.listByVmId(userVm.getId());
for (NicVO nic : nics) {
try {
NetworkVO network = networkDao.findById(nic.getNetworkId());
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NETWORK_OFFERING_ASSIGN, userVm.getAccountId(), userVm.getDataCenterId(), userVm.getId(),
Long.toString(nic.getId()), network.getNetworkOfferingId(), null, 1L, VirtualMachine.class.getName(), userVm.getUuid(), userVm.isDisplay());
} catch (Exception e) {
LOGGER.error(String.format("Failed to publish network usage records during VM import. %s", Strings.nullToEmpty(e.getMessage())));
}
}
}

private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedInstance, final String instanceName, final DataCenter zone, final Cluster cluster, final HostVO host,
Expand Down Expand Up @@ -1267,7 +1280,11 @@ private boolean hasVolumeSnapshotsPriorToUnmanageVM(VMInstanceVO vmVO) {
for (VolumeVO volume : volumes) {
List<SnapshotVO> snaps = snapshotDao.listByVolumeId(volume.getId());
if (CollectionUtils.isNotEmpty(snaps)) {
return true;
for (SnapshotVO snap : snaps) {
if (snap.getState() != Snapshot.State.Destroyed && snap.getRemoved() == null) {
return true;
}
}
}
}
return false;
Expand Down
9 changes: 9 additions & 0 deletions server/src/test/java/org/apache/cloudstack/vm/UnmanageVMManagerImplTest.java
View file
Expand Up @@ -37,6 +37,8 @@
import com.cloud.exception.UnsupportedServiceException;
import com.cloud.storage.SnapshotVO;
import com.cloud.storage.dao.SnapshotDao;
import com.cloud.vm.NicVO;
import com.cloud.vm.dao.NicDao;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.snapshot.VMSnapshotVO;
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
Expand Down Expand Up @@ -176,9 +178,13 @@ public class UnmanageVMManagerImplTest {
private SnapshotDao snapshotDao;
@Mock
private UserVmDao userVmDao;
@Mock
private NicDao nicDao;

@Mock
private VMInstanceVO virtualMachine;
@Mock
private NicVO nicVO;

private static final long virtualMachineId = 1L;

Expand Down Expand Up @@ -321,6 +327,9 @@ public void setUp() throws Exception {
when(volumeDao.findByInstance(virtualMachineId)).thenReturn(Collections.singletonList(volumeVO));
when(volumeVO.getInstanceId()).thenReturn(virtualMachineId);
when(volumeVO.getId()).thenReturn(virtualMachineId);
when(nicDao.listByVmId(virtualMachineId)).thenReturn(Collections.singletonList(nicVO));
when(nicVO.getNetworkId()).thenReturn(1L);
when(networkDao.findById(1L)).thenReturn(networkVO);
}

@After
Expand Down

0 comments on commit a7582d6

Please sign in to comment.