diff --git a/api/src/main/java/com/cloud/storage/VolumeApiService.java b/api/src/main/java/com/cloud/storage/VolumeApiService.java index ec20c33e17e..91b0bc0712f 100644 --- a/api/src/main/java/com/cloud/storage/VolumeApiService.java +++ b/api/src/main/java/com/cloud/storage/VolumeApiService.java @@ -79,6 +79,8 @@ public interface VolumeApiService { Volume attachVolumeToVM(AttachVolumeCmd command); + Volume detachVolumeViaDestroyVM(long vmId, long volumeId); + Volume detachVolumeFromVM(DetachVolumeCmd cmd); Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm, Snapshot.LocationType locationType, boolean asyncBackup) diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 5fa03a7d549..d519e4408e6 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -714,6 +714,7 @@ public class ApiConstants { public static final String STDERR = "stderr"; public static final String EXITCODE = "exitcode"; public static final String TARGET_ID = "targetid"; + public static final String VOLUME_IDS = "volumeids"; public enum HostDetails { all, capacity, events, stats, min; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java index 730c6776772..7b359b79047 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java @@ -31,6 +31,7 @@ import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ResponseObject.ResponseView; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.UserVmResponse; +import org.apache.cloudstack.api.response.VolumeResponse; import org.apache.cloudstack.context.CallContext; import com.cloud.event.EventTypes; @@ -63,6 +64,14 @@ public class DestroyVMCmd extends BaseAsyncCmd { since = "4.2.1") private Boolean expunge; + @Parameter( name = ApiConstants.VOLUME_IDS, + type = CommandType.LIST, + collectionType = CommandType.UUID, + entityType = VolumeResponse.class, + description = "Comma separated list of UUIDs for volumes that will be deleted", + since = "4.12.0") + private List volumeIds; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -78,6 +87,10 @@ public class DestroyVMCmd extends BaseAsyncCmd { return expunge; } + public List getVolumeIds() { + return volumeIds; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 0dfb990c8bd..21e35046143 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1857,8 +1857,12 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } } - private Volume orchestrateDetachVolumeFromVM(long vmId, long volumeId) { + @ActionEvent(eventType = EventTypes.EVENT_VOLUME_DETACH, eventDescription = "detaching volume") + public Volume detachVolumeViaDestroyVM(long vmId, long volumeId) { + return orchestrateDetachVolumeFromVM(vmId, volumeId); + } + private Volume orchestrateDetachVolumeFromVM(long vmId, long volumeId) { Volume volume = _volsDao.findById(volumeId); VMInstanceVO vm = _vmInstanceDao.findById(vmId); @@ -1869,7 +1873,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic if (hostId == null) { hostId = vm.getLastHostId(); - HostVO host = _hostDao.findById(hostId); if (host != null && host.getHypervisorType() == HypervisorType.VMware) { diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 96a40e1ee11..ebde735ef7c 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -39,12 +39,6 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; -import org.apache.commons.codec.binary.Base64; -import org.apache.commons.collections.MapUtils; -import org.apache.commons.lang3.StringUtils; -import org.apache.log4j.Logger; - -import com.cloud.user.AccountVO; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.affinity.AffinityGroupService; @@ -97,6 +91,10 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; +import org.apache.commons.codec.binary.Base64; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.log4j.Logger; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; @@ -256,6 +254,7 @@ import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountService; +import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; import com.cloud.user.SSHKeyPair; import com.cloud.user.SSHKeyPairVO; @@ -517,6 +516,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir private static final ConfigKey EnableAdditionalVmConfig = new ConfigKey<>("Advanced", Boolean.class, "enable.additional.vm.configuration", "false", "allow additional arbitrary configuration to vm", true, ConfigKey.Scope.Account); + private static final ConfigKey VmDestroyForcestop = new ConfigKey("Advanced", Boolean.class, "vm.destroy.forcestop", "false", + "On destroy, force-stop takes this value ", true); + @Override public UserVmVO getVirtualMachine(long vmId) { @@ -2757,10 +2759,15 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // check if VM exists UserVmVO vm = _vmDao.findById(vmId); - if (vm == null) { + if (vm == null || vm.getRemoved() != null) { throw new InvalidParameterValueException("unable to find a virtual machine with id " + vmId); } + if (vm.getState() == State.Destroyed || vm.getState() == State.Expunging) { + s_logger.debug("Vm id=" + vmId + " is already destroyed"); + return vm; + } + // check if there are active volume snapshots tasks s_logger.debug("Checking if there are any ongoing snapshots on the ROOT volumes associated with VM with ID " + vmId); if (checkStatusOfVolumeSnapshots(vmId, Volume.Type.ROOT)) { @@ -2768,6 +2775,15 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } s_logger.debug("Found no ongoing snapshots on volume of type ROOT, for the vm with id " + vmId); + List volumes = getVolumesFromIds(cmd); + + checkForUnattachedVolumes(vmId, volumes); + validateVolumes(volumes); + + stopVirtualMachine(vmId, VmDestroyForcestop.value()); + + detachVolumesFromVm(volumes); + UserVm destroyedVm = destroyVm(vmId, expunge); if (expunge) { if (!expunge(vm, ctx.getCallingUserId(), ctx.getCallingAccount())) { @@ -2775,9 +2791,26 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } } + deleteVolumesFromVm(volumes); + return destroyedVm; } + private List getVolumesFromIds(DestroyVMCmd cmd) { + List volumes = new ArrayList<>(); + if (cmd.getVolumeIds() != null) { + for (Long volId : cmd.getVolumeIds()) { + VolumeVO vol = _volsDao.findById(volId); + + if (vol == null) { + throw new InvalidParameterValueException("Unable to find volume with ID: " + volId); + } + volumes.add(vol); + } + } + return volumes; + } + @Override @DB public InstanceGroupVO createVmGroup(CreateVMGroupCmd cmd) { @@ -6518,4 +6551,52 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } return false; } + + private void checkForUnattachedVolumes(long vmId, List volumes) { + + StringBuilder sb = new StringBuilder(); + + for (VolumeVO volume : volumes) { + if (volume.getInstanceId() == null || vmId != volume.getInstanceId()) { + sb.append(volume.toString() + "; "); + } + } + + if (!StringUtils.isEmpty(sb.toString())) { + throw new InvalidParameterValueException("The following supplied volumes are not attached to the VM: " + sb.toString()); + } + } + + private void validateVolumes(List volumes) { + + for (VolumeVO volume : volumes) { + if (!(volume.getVolumeType() == Volume.Type.ROOT || volume.getVolumeType() == Volume.Type.DATADISK)) { + throw new InvalidParameterValueException("Please specify volume of type " + Volume.Type.DATADISK.toString() + " or " + Volume.Type.ROOT.toString()); + } + } + } + + private void detachVolumesFromVm(List volumes) { + + for (VolumeVO volume : volumes) { + + Volume detachResult = _volumeService.detachVolumeViaDestroyVM(volume.getInstanceId(), volume.getId()); + + if (detachResult == null) { + s_logger.error("DestroyVM remove volume - failed to detach and delete volume " + volume.getInstanceId() + " from instance " + volume.getId()); + } + } + } + + private void deleteVolumesFromVm(List volumes) { + + for (VolumeVO volume : volumes) { + + boolean deleteResult = _volumeService.deleteVolume(volume.getId(), CallContext.current().getCallingAccount()); + + if (!deleteResult) { + s_logger.error("DestroyVM remove volume - failed to delete volume " + volume.getInstanceId() + " from instance " + volume.getId()); + } + } + } } \ No newline at end of file diff --git a/test/integration/smoke/test_vm_life_cycle.py b/test/integration/smoke/test_vm_life_cycle.py index 5906a94869c..3ba43ac6245 100644 --- a/test/integration/smoke/test_vm_life_cycle.py +++ b/test/integration/smoke/test_vm_life_cycle.py @@ -32,7 +32,9 @@ from marvin.lib.base import (Account, Host, Iso, Router, - Configurations) + Configurations, + Volume, + DiskOffering) from marvin.lib.common import (get_domain, get_zone, get_template, @@ -786,6 +788,43 @@ class TestVMLifeCycle(cloudstackTestCase): ) return + @attr(tags = ["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false") + def test_11_destroy_vm_and_volumes(self): + """Test destroy Virtual Machine and it's volumes + """ + + # Validate the following + # 1. Deploys a VM and attaches disks to it + # 2. Destroys the VM with DataDisks option + + small_disk_offering = DiskOffering.list(self.apiclient, name='Small')[0] + + small_virtual_machine = VirtualMachine.create( + self.apiclient, + self.services["small"], + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.small_offering.id, + mode=self.services["mode"] + ) + vol1 = Volume.create( + self.apiclient, + self.services, + account=self.account.name, + diskofferingid=small_disk_offering.id, + domainid=self.account.domainid, + zoneid=self.zone.id + ) + + small_virtual_machine.attach_volume(self.apiclient, vol1) + + self.debug("Destroy VM - ID: %s" % small_virtual_machine.id) + small_virtual_machine.delete(self.apiclient, volumeIds=vol1.id) + + self.assertEqual(VirtualMachine.list(self.apiclient, id=small_virtual_machine.id), None, "List response contains records when it should not") + + self.assertEqual(Volume.list(self.apiclient, id=vol1.id), None, "List response contains records when it should not") + class TestSecuredVmMigration(cloudstackTestCase): @classmethod diff --git a/ui/css/cloudstack3.css b/ui/css/cloudstack3.css index 59e6235b7bb..ad1f624791b 100644 --- a/ui/css/cloudstack3.css +++ b/ui/css/cloudstack3.css @@ -4179,6 +4179,15 @@ textarea { float: left; } +.ui-dialog div.form-container div.value label { + display: block; + width: 119px; + text-align: left; + font-size: 13px; + margin-top: 2px; + margin-left: -10px; +} + .ui-dialog div.form-container div.value input.hasDatepicker { color: #2F5D86; cursor: pointer; diff --git a/ui/l10n/en.js b/ui/l10n/en.js index fb537f2f0eb..e20d6093b9a 100644 --- a/ui/l10n/en.js +++ b/ui/l10n/en.js @@ -640,6 +640,7 @@ var dictionary = { "label.delete.secondary.staging.store":"Delete Secondary Staging Store", "label.delete.sslcertificate":"Delete SSL Certificate", "label.delete.ucs.manager":"Delete UCS Manager", +"label.delete.volumes":"Volumes to be deleted", "label.delete.vpn.user":"Delete VPN user", "label.deleting.failed":"Deleting Failed", "label.deleting.processing":"Deleting....", @@ -1813,6 +1814,8 @@ var dictionary = { "label.volgroup":"Volume Group", "label.volume":"Volume", "label.volume.details":"Volume details", +"label.volume.empty":"No volumes attached to this VM", +"label.volume.ids":"Volume ID's", "label.volume.limits":"Volume Limits", "label.volume.migrated":"Volume migrated", "label.volume.name":"Volume Name", diff --git a/ui/scripts/instances.js b/ui/scripts/instances.js index 3b5816b039a..43e6bc9bb10 100644 --- a/ui/scripts/instances.js +++ b/ui/scripts/instances.js @@ -112,6 +112,34 @@ label: 'label.expunge', isBoolean: true, isChecked: false + }, + volumes: { + label: 'label.delete.volumes', + isBoolean: true, + isChecked: false + }, + volumeids: { + label: 'label.volume.ids', + dependsOn: 'volumes', + isBoolean: true, + isHidden: true, + emptyMessage: 'label.volume.empty', + multiDataArray: true, + multiData: function(args) { + $.ajax({ + url: createURL("listVolumes&virtualMachineId=" + args.context.instances[0].id) + "&type=DATADISK", + dataType: "json", + async: true, + success: function(json) { + var volumes = json.listvolumesresponse.volume; + args.response.success({ + descriptionField: 'name', + valueField: 'id', + data: volumes + }); + } + }); + } } } }, @@ -126,6 +154,26 @@ expunge: true }); } + if (args.data.volumes == 'on') { + + var regex = RegExp('[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}'); + + var selectedVolumes = []; + + for (var key in args.data) { + var matches = key.match(regex); + + if (matches != null) { + selectedVolumes.push(key); + } + } + + $.extend(data, { + volumeids: $(selectedVolumes).map(function(index, volume) { + return volume; + }).toArray().join(',') + }); + } $.ajax({ url: createURL('destroyVirtualMachine'), data: data, diff --git a/ui/scripts/ui/dialog.js b/ui/scripts/ui/dialog.js index 1f954d907d0..346e95ed42d 100644 --- a/ui/scripts/ui/dialog.js +++ b/ui/scripts/ui/dialog.js @@ -433,6 +433,68 @@ ); }); + } else if (field.multiDataArray) { + + $input = $('
'); + + multiArgs = { + context: args.context, + response: { + success: function(args) { + if (args.data == undefined || args.data.length == 0) { + + var label = field.emptyMessage != null ? field.emptyMessage : 'No data available'; + + $input + .addClass('value') + .appendTo($value) + .append( + $('