diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java index 1285be3b7b3..43b70fa7665 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java +++ b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java @@ -51,6 +51,11 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer "300", "The backup and recovery background sync task polling interval in seconds.", true); + ConfigKey BackupEnableAttachDetachVolumes = new ConfigKey<>("Advanced", Boolean.class, + "backup.enable.attach.detach.of.volumes", + "false", + "Enable volume attach/detach operations for VMs that are assigned to Backup Offerings.", true); + /** * List backup provider offerings * @param zoneId zone id diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java index eb66bc01309..0b67074e937 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java @@ -106,6 +106,7 @@ import com.cloud.network.dao.PhysicalNetworkTrafficTypeVO; import com.cloud.network.dao.PhysicalNetworkVO; import com.cloud.secstorage.CommandExecLogDao; import com.cloud.secstorage.CommandExecLogVO; +import com.cloud.serializer.GsonHelper; import com.cloud.service.ServiceOfferingVO; import com.cloud.storage.DataStoreRole; import com.cloud.storage.DiskOfferingVO; @@ -139,6 +140,7 @@ import com.cloud.vm.VirtualMachine.Type; import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.VirtualMachineProfile; import com.cloud.vm.dao.UserVmDao; +import com.cloud.vm.dao.VMInstanceDao; import com.google.gson.Gson; import com.vmware.vim25.ManagedObjectReference; import com.vmware.vim25.VirtualDevice; @@ -153,6 +155,7 @@ import com.vmware.vim25.VirtualMachineRuntimeInfo; public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Configurable { private static final Logger s_logger = Logger.getLogger(VMwareGuru.class); + private static final Gson GSON = GsonHelper.getGson(); @Inject VmwareVmImplementer vmwareVmImplementer; @@ -161,6 +164,7 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co @Inject ClusterDetailsDao _clusterDetailsDao; @Inject CommandExecLogDao _cmdExecLogDao; @Inject VmwareManager _vmwareMgr; + @Inject VMInstanceDao vmDao; @Inject SecondaryStorageVmManager _secStorageMgr; @Inject PhysicalNetworkTrafficTypeDao _physicalNetworkTrafficTypeDao; @Inject VirtualMachineManager vmManager; @@ -697,7 +701,7 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co volumeVO.setTemplateId(templateId); volumeVO.setAttached(new Date()); volumeVO.setRemoved(null); - volumeVO.setChainInfo(new Gson().toJson(diskInfo)); + volumeVO.setChainInfo(GSON.toJson(diskInfo)); if (unitNumber != null) { volumeVO.setDeviceId(unitNumber.longValue()); } @@ -736,12 +740,14 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co volume.setPath(volumeName); volume.setPoolId(poolId); VirtualMachineDiskInfo diskInfo = getDiskInfo(vmToImport, poolId, volumeName); - volume.setChainInfo(new Gson().toJson(diskInfo)); + volume.setChainInfo(GSON.toJson(diskInfo)); volume.setInstanceId(vm.getId()); volume.setState(Volume.State.Ready); volume.setAttached(new Date()); _volumeDao.update(volume.getId(), volume); if (volume.getRemoved() != null) { + s_logger.debug(String.format("Marking volume [uuid: %s] of restored VM [%s] as non removed.", volume.getUuid(), + ReflectionToStringBuilderUtils.reflectOnlySelectedFields(vm, "uuid", "instanceName"))); _volumeDao.unremove(volume.getId()); if (vm.getType() == Type.User) { UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(), volume.getDiskOfferingId(), null, volume.getSize(), @@ -812,6 +818,23 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co return createVolumeRecord(type, volumeName, zoneId, domainId, accountId, diskOfferingId, provisioningType, size, instanceId, poolId, templateId, unitNumber, diskInfo); } + protected String createVolumeInfoFromVolumes(List vmVolumes) { + try { + List list = new ArrayList<>(); + for (VolumeVO vol : vmVolumes) { + list.add(new Backup.VolumeInfo(vol.getUuid(), vol.getPath(), vol.getVolumeType(), vol.getSize())); + } + return GSON.toJson(list.toArray(), Backup.VolumeInfo[].class); + } catch (Exception e) { + if (CollectionUtils.isEmpty(vmVolumes) || vmVolumes.get(0).getInstanceId() == null) { + s_logger.error(String.format("Failed to create VolumeInfo of VM [id: null] volumes due to: [%s].", e.getMessage()), e); + } else { + s_logger.error(String.format("Failed to create VolumeInfo of VM [id: %s] volumes due to: [%s].", vmVolumes.get(0).getInstanceId(), e.getMessage()), e); + } + throw e; + } + } + /** * Get physical network ID from zoneId and Vmware label */ @@ -919,11 +942,25 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co if (vm == null) { throw new CloudRuntimeException("Failed to find the volumes details from the VM backup"); } + List backedUpVolumes = vm.getBackupVolumeList(); Map usedVols = new HashMap<>(); Map map = new HashMap<>(); for (Backup.VolumeInfo backedUpVol : backedUpVolumes) { + VolumeVO volumeExtra = _volumeDao.findByUuid(backedUpVol.getUuid()); + if (volumeExtra != null) { + s_logger.debug(String.format("Marking volume [id: %s] of VM [%s] as removed for the backup process.", backedUpVol.getUuid(), ReflectionToStringBuilderUtils.reflectOnlySelectedFields(vm, "uuid", "instanceName"))); + _volumeDao.remove(volumeExtra.getId()); + + if (vm.getType() == Type.User) { + UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_DETACH, volumeExtra.getAccountId(), volumeExtra.getDataCenterId(), volumeExtra.getId(), + volumeExtra.getName(), volumeExtra.getDiskOfferingId(), null, volumeExtra.getSize(), Volume.class.getName(), + volumeExtra.getUuid(), volumeExtra.isDisplayVolume()); + _resourceLimitService.decrementResourceCount(vm.getAccountId(), Resource.ResourceType.volume, volumeExtra.isDisplayVolume()); + _resourceLimitService.decrementResourceCount(vm.getAccountId(), Resource.ResourceType.primary_storage, volumeExtra.isDisplayVolume(), volumeExtra.getSize()); + } + } for (VirtualDisk disk : virtualDisks) { if (!map.containsKey(disk) && backedUpVol.getSize().equals(disk.getCapacityInBytes()) && !usedVols.containsKey(backedUpVol.getUuid())) { String volId = backedUpVol.getUuid(); @@ -1022,7 +1059,8 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co return vm; } - @Override public boolean attachRestoredVolumeToVirtualMachine(long zoneId, String location, Backup.VolumeInfo volumeInfo, VirtualMachine vm, long poolId, Backup backup) + @Override + public boolean attachRestoredVolumeToVirtualMachine(long zoneId, String location, Backup.VolumeInfo volumeInfo, VirtualMachine vm, long poolId, Backup backup) throws Exception { DatacenterMO dcMo = getDatacenterMO(zoneId); VirtualMachineMO vmRestored = findVM(dcMo, location); @@ -1061,6 +1099,12 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co } createVolume(attachedDisk, vmMo, vm.getDomainId(), vm.getDataCenterId(), vm.getAccountId(), vm.getId(), poolId, vm.getTemplateId(), backup, false); + if (vm.getBackupOfferingId() == null) { + return true; + } + VMInstanceVO vmVO = (VMInstanceVO)vm; + vmVO.setBackupVolumes(createVolumeInfoFromVolumes(_volumeDao.findByInstance(vm.getId()))); + vmDao.update(vmVO.getId(), vmVO); return true; } diff --git a/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java index ca618ac1b0f..0db8271c8e5 100644 --- a/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java +++ b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java @@ -21,9 +21,11 @@ import com.cloud.agent.api.MigrateVmToPoolCommand; import com.cloud.dc.ClusterDetailsDao; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; +import com.cloud.storage.Storage.ProvisioningType; import com.cloud.storage.StoragePool; import com.cloud.storage.StoragePoolHostVO; import com.cloud.storage.Volume; +import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.utils.Pair; import com.cloud.vm.VirtualMachine; @@ -44,6 +46,8 @@ import org.powermock.modules.junit4.PowerMockRunner; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.support.AnnotationConfigContextLoader; +import static org.junit.Assert.assertEquals; + import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -116,4 +120,33 @@ public class VMwareGuruTest { Assert.assertEquals("HostSystem:host-a@x.x.x.x", migrateVmToPoolCommand.getHostGuidInTargetCluster()); } + @Test + public void createVolumeInfoFromVolumesTestEmptyVolumeListReturnEmptyArray() { + String volumeInfo = vMwareGuru.createVolumeInfoFromVolumes(new ArrayList<>()); + assertEquals("[]", volumeInfo); + } + + @Test(expected = NullPointerException.class) + public void createVolumeInfoFromVolumesTestNullVolume() { + vMwareGuru.createVolumeInfoFromVolumes(null); + } + + @Test + public void createVolumeInfoFromVolumesTestCorrectlyConvertOfVolumes() { + List volumesToTest = new ArrayList<>(); + + VolumeVO root = new VolumeVO("test", 1l, 1l, 1l, 1l, 1l, "test", "/root/dir", ProvisioningType.THIN, 555l, Volume.Type.ROOT); + String rootUuid = root.getUuid(); + + VolumeVO data = new VolumeVO("test", 1l, 1l, 1l, 1l, 1l, "test", "/root/dir/data", ProvisioningType.THIN, 1111000l, Volume.Type.DATADISK); + String dataUuid = data.getUuid(); + + volumesToTest.add(root); + volumesToTest.add(data); + + String result = vMwareGuru.createVolumeInfoFromVolumes(volumesToTest); + String expected = String.format("[{\"uuid\":\"%s\",\"type\":\"ROOT\",\"size\":555,\"path\":\"/root/dir\"},{\"uuid\":\"%s\",\"type\":\"DATADISK\",\"size\":1111000,\"path\":\"/root/dir/data\"}]", rootUuid, dataUuid); + + assertEquals(expected, result); + } } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 735a8b29306..7b32fdee977 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -45,6 +45,8 @@ import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd; import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd; import org.apache.cloudstack.api.command.user.volume.UploadVolumeCmd; import org.apache.cloudstack.api.response.GetUploadParamsResponse; +import org.apache.cloudstack.backup.Backup; +import org.apache.cloudstack.backup.BackupManager; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; @@ -97,9 +99,8 @@ import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.ObjectUtils; +import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.builder.ToStringBuilder; -import org.apache.commons.lang3.builder.ToStringStyle; import org.apache.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -2477,6 +2478,36 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic return volumeToAttach; } + protected void validateIfVmHasBackups(UserVmVO vm, boolean attach) { + if ((vm.getBackupOfferingId() == null || CollectionUtils.isEmpty(vm.getBackupVolumeList())) || BooleanUtils.isTrue(BackupManager.BackupEnableAttachDetachVolumes.value())) { + return; + } + String errorMsg = String.format("Unable to detach volume, cannot detach volume from a VM that has backups. First remove the VM from the backup offering or " + + "set the global configuration '%s' to true.", BackupManager.BackupEnableAttachDetachVolumes.key()); + if (attach) { + errorMsg = String.format("Unable to attach volume, please specify a VM that does not have any backups or set the global configuration " + + "'%s' to true.", BackupManager.BackupEnableAttachDetachVolumes.key()); + } + throw new InvalidParameterValueException(errorMsg); + } + + protected String createVolumeInfoFromVolumes(List vmVolumes) { + try { + List list = new ArrayList<>(); + for (VolumeVO vol : vmVolumes) { + list.add(new Backup.VolumeInfo(vol.getUuid(), vol.getPath(), vol.getVolumeType(), vol.getSize())); + } + return GsonHelper.getGson().toJson(list.toArray(), Backup.VolumeInfo[].class); + } catch (Exception e) { + if (CollectionUtils.isEmpty(vmVolumes) || vmVolumes.get(0).getInstanceId() == null) { + s_logger.error(String.format("Failed to create VolumeInfo of VM [id: null] volumes due to: [%s].", e.getMessage()), e); + } else { + s_logger.error(String.format("Failed to create VolumeInfo of VM [id: %s] volumes due to: [%s].", vmVolumes.get(0).getInstanceId(), e.getMessage()), e); + } + throw e; + } + } + @Override @ActionEvent(eventType = EventTypes.EVENT_VOLUME_UPDATE, eventDescription = "updating volume", async = true) public Volume updateVolume(long volumeId, String path, String state, Long storageId, Boolean displayVolume, @@ -2649,13 +2680,11 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic // Don't allow detach if target VM has associated VM snapshots List vmSnapshots = _vmSnapshotDao.findByVm(vmId); - if (vmSnapshots.size() > 0) { + if (CollectionUtils.isNotEmpty(vmSnapshots)) { throw new InvalidParameterValueException("Unable to detach volume, please specify a VM that does not have VM snapshots"); } - if (vm.getBackupOfferingId() != null || vm.getBackupVolumeList().size() > 0) { - throw new InvalidParameterValueException("Unable to detach volume, cannot detach volume from a VM that has backups. First remove the VM from the backup offering."); - } + validateIfVmHasBackups(vm, false); AsyncJobExecutionContext asyncExecutionContext = AsyncJobExecutionContext.getCurrentExecutionContext(); if (asyncExecutionContext != null) { @@ -2705,6 +2734,10 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic vol = _volsDao.findById((Long)jobResult); } } + if (vm.getBackupOfferingId() != null) { + vm.setBackupVolumes(createVolumeInfoFromVolumes(_volsDao.findByInstance(vm.getId()))); + _vmInstanceDao.update(vm.getId(), vm); + } return vol; } } @@ -2846,6 +2879,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic if (volumePool != null && hostId != null) { handleTargetsForVMware(hostId, volumePool.getHostAddress(), volumePool.getPort(), volume.get_iScsiName()); } + return _volsDao.findById(volumeId); } else { @@ -3025,7 +3059,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic snapshotHelper.checkKvmVolumeSnapshotsOnlyInPrimaryStorage(vol, _volsDao.getHypervisorType(vol.getId())); } catch (CloudRuntimeException ex) { throw new CloudRuntimeException(String.format("Unable to migrate %s to the destination storage pool [%s] due to [%s]", vol, - new ToStringBuilder(destPool, ToStringStyle.JSON_STYLE).append("uuid", destPool.getUuid()).append("name", destPool.getName()).toString(), ex.getMessage()), ex); + ReflectionToStringBuilderUtils.reflectOnlySelectedFields(destPool, "uuid", "name"), ex.getMessage()), ex); } DiskOfferingVO diskOffering = _diskOfferingDao.findById(vol.getDiskOfferingId()); diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index 1978cca6870..314d263f75d 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -587,6 +587,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { if (offering == null) { throw new CloudRuntimeException("Failed to find backup offering of the VM backup"); } + final BackupProvider backupProvider = getBackupProvider(offering.getProvider()); if (!backupProvider.restoreVMFromBackup(vm, backup)) { throw new CloudRuntimeException("Error restoring VM from backup ID " + backup.getId()); @@ -619,7 +620,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { final VMInstanceVO vm = findVmById(vmId); accountManager.checkAccess(CallContext.current().getCallingAccount(), null, true, vm); - if (vm.getBackupOfferingId() != null) { + if (vm.getBackupOfferingId() != null && !BackupEnableAttachDetachVolumes.value()) { throw new CloudRuntimeException("The selected VM has backups, cannot restore and attach volume to the VM."); } @@ -841,7 +842,8 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { return new ConfigKey[]{ BackupFrameworkEnabled, BackupProviderPlugin, - BackupSyncPollingInterval + BackupSyncPollingInterval, + BackupEnableAttachDetachVolumes }; } diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 4c4bbf789b0..45adc84d82a 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.storage; +import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyObject; @@ -90,6 +91,7 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.org.Grouping; import com.cloud.serializer.GsonHelper; import com.cloud.server.TaggedResourceService; +import com.cloud.storage.Storage.ProvisioningType; import com.cloud.storage.Volume.Type; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.StoragePoolTagsDao; @@ -1164,6 +1166,65 @@ public class VolumeApiServiceImplTest { Assert.assertTrue(result); } + @Test + public void validateIfVmHaveBackupsTestExceptionWhenTryToDetachVolumeFromVMWhichBackupOffering() { + try { + UserVmVO vm = Mockito.mock(UserVmVO.class); + when(vm.getBackupOfferingId()).thenReturn(1l); + volumeApiServiceImpl.validateIfVmHasBackups(vm, false); + } catch (Exception e) { + Assert.assertEquals("Unable to detach volume, cannot detach volume from a VM that has backups. First remove the VM from the backup offering or set the global configuration 'backup.enable.attach.detach.of.volumes' to true.", e.getMessage()); + } + } + + @Test + public void validateIfVmHaveBackupsTestExceptionWhenTryToAttachVolumeFromVMWhichBackupOffering() { + try { + UserVmVO vm = Mockito.mock(UserVmVO.class); + when(vm.getBackupOfferingId()).thenReturn(1l); + volumeApiServiceImpl.validateIfVmHasBackups(vm, true); + } catch (Exception e) { + Assert.assertEquals("Unable to attach volume, please specify a VM that does not have any backups or set the global configuration 'backup.enable.attach.detach.of.volumes' to true.", e.getMessage()); + } + } + + @Test + public void validateIfVmHaveBackupsTestSuccessWhenVMDontHaveBackupOffering() { + UserVmVO vm = Mockito.mock(UserVmVO.class); + when(vm.getBackupOfferingId()).thenReturn(null); + volumeApiServiceImpl.validateIfVmHasBackups(vm, true); + } + + @Test + public void createVolumeInfoFromVolumesTestEmptyVolumeListReturnEmptyArray() { + String volumeInfo = volumeApiServiceImpl.createVolumeInfoFromVolumes(new ArrayList<>()); + assertEquals("[]", volumeInfo); + } + + @Test(expected = NullPointerException.class) + public void createVolumeInfoFromVolumesTestNullVolume() { + volumeApiServiceImpl.createVolumeInfoFromVolumes(null); + } + + @Test + public void createVolumeInfoFromVolumesTestCorrectlyConvertOfVolumes() { + List volumesToTest = new ArrayList<>(); + + VolumeVO root = new VolumeVO("test", 1l, 1l, 1l, 1l, 1l, "test", "/root/dir", ProvisioningType.THIN, 555l, Type.ROOT); + String rootUuid = root.getUuid(); + + VolumeVO data = new VolumeVO("test", 1l, 1l, 1l, 1l, 1l, "test", "/root/dir/data", ProvisioningType.THIN, 1111000l, Type.DATADISK); + String dataUuid = data.getUuid(); + + volumesToTest.add(root); + volumesToTest.add(data); + + String result = volumeApiServiceImpl.createVolumeInfoFromVolumes(volumesToTest); + String expected = String.format("[{\"uuid\":\"%s\",\"type\":\"ROOT\",\"size\":555,\"path\":\"/root/dir\"},{\"uuid\":\"%s\",\"type\":\"DATADISK\",\"size\":1111000,\"path\":\"/root/dir/data\"}]", rootUuid, dataUuid); + + assertEquals(expected, result); + } + @Test public void isNotPossibleToResizeTestAllFormats() { Storage.ImageFormat[] imageFormat = Storage.ImageFormat.values(); @@ -1319,4 +1380,4 @@ public class VolumeApiServiceImplTest { offeringMockId, volumeVoMock.getTemplateId(), volumeVoMock.getSize(), Volume.class.getName(), volumeVoMock.getUuid(), volumeVoMock.isDisplay()); } -} +} \ No newline at end of file