From 9bcd98876d6737a9139b2e33535f479e7300723f Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Tue, 7 Oct 2025 10:32:33 +0530 Subject: [PATCH] Make kvm domain persistent when unmanaged from CS (#11541) CS creates transient KVM domain.xml. When instance is unmanaged from CS, explicit dump of domain has to be taken to manage is outside of CS. With this PR domainXML gets backed up and becomes persistent for further management of Instance. Stopped instance also can be unmanaged, last host for instance is considered for defining domain hostid param is supported in unmanageVirtualMachine API for KVM hypervisor and for stopped Instances hostid field in response of unmanageVirtualMachine, representing host used for unmanage operation Disable unmanaging instance with config drive, can unmanage from API using forced=true param for KVM --- .../main/java/com/cloud/vm/UserVmService.java | 6 +- .../admin/vm/UnmanageVMInstanceCmd.java | 37 +- .../response/UnmanageVMInstanceResponse.java | 12 + .../cloudstack/vm/UnmanageVMService.java | 7 +- .../agent/api/UnmanageInstanceAnswer.java | 27 + .../agent/api/UnmanageInstanceCommand.java | 61 +++ .../com/cloud/vm/VirtualMachineManager.java | 2 +- .../cloud/vm/VirtualMachineManagerImpl.java | 121 ++++- .../vm/VirtualMachineManagerImplTest.java | 317 +++++++++++- ...LibvirtUnmanageInstanceCommandWrapper.java | 174 +++++++ ...irtUnmanageInstanceCommandWrapperTest.java | 357 +++++++++++++ .../java/com/cloud/vm/UserVmManagerImpl.java | 26 +- .../vm/UnmanagedVMsManagerImpl.java | 19 +- .../com/cloud/vm/UserVmManagerImplTest.java | 184 ++++++- .../vm/UnmanagedVMsManagerImplTest.java | 91 +++- .../test_vm_lifecycle_unmanage_kvm_import.py | 473 ++++++++++++++++++ tools/marvin/marvin/lib/base.py | 5 +- ui/public/locales/en.json | 2 +- 18 files changed, 1850 insertions(+), 71 deletions(-) create mode 100644 core/src/main/java/com/cloud/agent/api/UnmanageInstanceAnswer.java create mode 100644 core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java create mode 100644 plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnmanageInstanceCommandWrapper.java create mode 100644 plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnmanageInstanceCommandWrapperTest.java create mode 100644 test/integration/smoke/test_vm_lifecycle_unmanage_kvm_import.py diff --git a/api/src/main/java/com/cloud/vm/UserVmService.java b/api/src/main/java/com/cloud/vm/UserVmService.java index 0747e193ab8..01f11b73cd4 100644 --- a/api/src/main/java/com/cloud/vm/UserVmService.java +++ b/api/src/main/java/com/cloud/vm/UserVmService.java @@ -64,6 +64,7 @@ import com.cloud.storage.StoragePool; import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; import com.cloud.uservm.UserVm; +import com.cloud.utils.Pair; import com.cloud.utils.exception.ExecutionException; public interface UserVmService { @@ -538,9 +539,10 @@ public interface UserVmService { /** * Unmanage a guest VM from CloudStack - * @return true if the VM is successfully unmanaged, false if not. + * + * @return (true if successful, false if not, hostUuid) if the VM is successfully unmanaged. */ - boolean unmanageUserVM(Long vmId); + Pair unmanageUserVM(Long vmId, Long targetHostId); UserVm allocateVMFromBackup(CreateVMFromBackupCmd cmd) throws InsufficientCapacityException, ResourceAllocationException, ResourceUnavailableException; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/UnmanageVMInstanceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/UnmanageVMInstanceCmd.java index bbcb8840f66..2c60b574126 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/UnmanageVMInstanceCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/UnmanageVMInstanceCmd.java @@ -27,6 +27,7 @@ import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.user.Account; import com.cloud.uservm.UserVm; +import com.cloud.utils.Pair; import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; @@ -36,10 +37,12 @@ import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.BaseAsyncCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.response.HostResponse; import org.apache.cloudstack.api.response.UnmanageVMInstanceResponse; import org.apache.cloudstack.api.response.UserVmResponse; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.vm.UnmanagedVMsManager; +import org.apache.commons.lang3.BooleanUtils; import javax.inject.Inject; @@ -65,6 +68,20 @@ public class UnmanageVMInstanceCmd extends BaseAsyncCmd { description = "The ID of the virtual machine to unmanage") private Long vmId; + @Parameter(name = ApiConstants.HOST_ID, type = CommandType.UUID, + entityType = HostResponse.class, required = false, + description = "ID of the host which will be used for unmanaging the Instance. " + + "Applicable only for KVM hypervisor and stopped Instances. Domain XML will be stored on this host.", + since = "4.22.0") + private Long hostId; + + @Parameter(name = ApiConstants.FORCED, + type = CommandType.BOOLEAN, + required = false, + description = "Force unmanaging Instance with config drive. Applicable only for KVM Hypervisor.", + since = "4.22.0") + private Boolean forced; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -83,6 +100,18 @@ public class UnmanageVMInstanceCmd extends BaseAsyncCmd { return "unmanaging VM. VM ID = " + vmId; } + public Long getHostId() { + return hostId; + } + + public void setHostId(Long hostId) { + this.hostId = hostId; + } + + public Boolean isForced() { + return BooleanUtils.isTrue(forced); + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @@ -93,9 +122,10 @@ public class UnmanageVMInstanceCmd extends BaseAsyncCmd { UnmanageVMInstanceResponse response = new UnmanageVMInstanceResponse(); try { CallContext.current().setEventDetails("VM ID = " + vmId); - boolean result = unmanagedVMsManager.unmanageVMInstance(vmId); - response.setSuccess(result); - if (result) { + Pair result = unmanagedVMsManager.unmanageVMInstance(vmId, hostId, isForced()); + if (result.first()) { + response.setSuccess(true); + response.setHostId(result.second()); response.setDetails("VM unmanaged successfully"); } } catch (Exception e) { @@ -124,5 +154,4 @@ public class UnmanageVMInstanceCmd extends BaseAsyncCmd { public Long getApiResourceId() { return vmId; } - } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/UnmanageVMInstanceResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/UnmanageVMInstanceResponse.java index e9d45cb506a..bd95ecee5ae 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/UnmanageVMInstanceResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/UnmanageVMInstanceResponse.java @@ -32,6 +32,10 @@ public class UnmanageVMInstanceResponse extends BaseResponse { @Param(description = "details of the unmanage VM operation") private String details; + @SerializedName(ApiConstants.HOST_ID) + @Param(description = "The ID of the host used for unmanaged Instance") + private String hostId; + public UnmanageVMInstanceResponse() { } @@ -55,4 +59,12 @@ public class UnmanageVMInstanceResponse extends BaseResponse { public void setDetails(String details) { this.details = details; } + + public String getHostId() { + return hostId; + } + + public void setHostId(String hostId) { + this.hostId = hostId; + } } diff --git a/api/src/main/java/org/apache/cloudstack/vm/UnmanageVMService.java b/api/src/main/java/org/apache/cloudstack/vm/UnmanageVMService.java index 2315e5f2e95..70d8795f305 100644 --- a/api/src/main/java/org/apache/cloudstack/vm/UnmanageVMService.java +++ b/api/src/main/java/org/apache/cloudstack/vm/UnmanageVMService.java @@ -17,11 +17,14 @@ package org.apache.cloudstack.vm; +import com.cloud.utils.Pair; + public interface UnmanageVMService { /** * Unmanage a guest VM from CloudStack - * @return true if the VM is successfully unmanaged, false if not. + * + * @return (true if successful, false if not, hostUuid) if the VM is successfully unmanaged. */ - boolean unmanageVMInstance(long vmId); + Pair unmanageVMInstance(long vmId, Long paramHostId, boolean isForced); } diff --git a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceAnswer.java b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceAnswer.java new file mode 100644 index 00000000000..39a35d49990 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceAnswer.java @@ -0,0 +1,27 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.agent.api; + +public class UnmanageInstanceAnswer extends Answer { + + public UnmanageInstanceAnswer(UnmanageInstanceCommand cmd, boolean success, String details) { + super(cmd, success, details); + } +} diff --git a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java new file mode 100644 index 00000000000..dd504b9ea26 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java @@ -0,0 +1,61 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.agent.api; + +import com.cloud.agent.api.to.VirtualMachineTO; + +/** + */ +public class UnmanageInstanceCommand extends Command { + String instanceName; + boolean executeInSequence = false; + VirtualMachineTO vm; + boolean isConfigDriveAttached; + + @Override + public boolean executeInSequence() { + return executeInSequence; + } + + public UnmanageInstanceCommand(VirtualMachineTO vm) { + this.vm = vm; + this.instanceName = vm.getName(); + } + + public UnmanageInstanceCommand(String instanceName) { + this.instanceName = instanceName; + } + + public String getInstanceName() { + return instanceName; + } + + public VirtualMachineTO getVm() { + return vm; + } + + public boolean isConfigDriveAttached() { + return isConfigDriveAttached; + } + + public void setConfigDriveAttached(boolean configDriveAttached) { + isConfigDriveAttached = configDriveAttached; + } +} diff --git a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java index c05c29add55..cffba3d9a13 100644 --- a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java +++ b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java @@ -274,7 +274,7 @@ public interface VirtualMachineManager extends Manager { * - Remove the references of the VM and its volumes, nics, IPs from database * - Keep the VM as it is on the hypervisor */ - boolean unmanage(String vmUuid); + Pair unmanage(String vmUuid, Long paramHostId); UserVm restoreVirtualMachine(long vmId, Long newTemplateId, Long rootDiskOfferingId, boolean expunge, Map details) throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException; diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index b5597280364..7c5d43fea97 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -71,6 +71,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProviderManag import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver; import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.ca.Certificate; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; @@ -150,11 +151,13 @@ import com.cloud.agent.api.StopAnswer; import com.cloud.agent.api.StopCommand; import com.cloud.agent.api.UnPlugNicAnswer; import com.cloud.agent.api.UnPlugNicCommand; +import com.cloud.agent.api.UnmanageInstanceCommand; import com.cloud.agent.api.UnregisterVMCommand; import com.cloud.agent.api.VmDiskStatsEntry; import com.cloud.agent.api.VmNetworkStatsEntry; import com.cloud.agent.api.VmStatsEntry; import com.cloud.agent.api.routing.NetworkElementCommand; +import com.cloud.agent.api.to.DataTO; import com.cloud.agent.api.to.DiskTO; import com.cloud.agent.api.to.DpdkTO; import com.cloud.agent.api.to.GPUDeviceTO; @@ -2016,7 +2019,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } @Override - public boolean unmanage(String vmUuid) { + public Pair unmanage(String vmUuid, Long paramHostId) { VMInstanceVO vm = _vmDao.findByUuid(vmUuid); if (vm == null || vm.getRemoved() != null) { throw new CloudRuntimeException("Could not find VM with id = " + vmUuid); @@ -2029,6 +2032,10 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac throw new ConcurrentOperationException(msg); } + Long agentHostId = vm.getHostId(); + if (HypervisorType.KVM.equals(vm.getHypervisorType())) { + agentHostId = persistDomainForKVM(vm, paramHostId); + } Boolean result = Transaction.execute(new TransactionCallback() { @Override public Boolean doInTransaction(TransactionStatus status) { @@ -2052,21 +2059,66 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac return true; } }); + HostVO host = ApiDBUtils.findHostById(agentHostId); + if (host == null) { + return new Pair<>(result, null); + } + logger.debug("Selected host UUID: {} to unmanage Instance: {}.", host.getUuid(), vm.getName()); + ActionEventUtils.onActionEvent(User.UID_SYSTEM, Account.ACCOUNT_ID_SYSTEM, Domain.ROOT_DOMAIN, EventTypes.EVENT_VM_UNMANAGE, + String.format("Successfully unmanaged Instance: %s (ID: %s) on host ID: %s", vm.getName(), vm.getUuid(), host.getUuid()), + vm.getId(), ApiCommandResourceType.VirtualMachine.toString()); + return new Pair<>(result, host.getUuid()); + } - return BooleanUtils.isTrue(result); + Long persistDomainForKVM(VMInstanceVO vm, Long paramHostId) { + Long agentHostId = vm.getHostId(); + String vmName = vm.getName(); + UnmanageInstanceCommand unmanageInstanceCommand; + if (State.Stopped.equals(vm.getState())) { + if (paramHostId == null) { + Pair clusterAndHostId = findClusterAndHostIdForVm(vm, false); + agentHostId = clusterAndHostId.second(); + if (agentHostId == null) { + String errorMsg = "No available host to persist domain XML for Instance: " + vmName; + logger.debug(errorMsg); + throw new CloudRuntimeException(errorMsg); + } + } else { + agentHostId = paramHostId; + } + unmanageInstanceCommand = new UnmanageInstanceCommand(prepVmSpecForUnmanageCmd(vm.getId(), agentHostId)); // reconstruct vmSpec for stopped instance + } else { + unmanageInstanceCommand = new UnmanageInstanceCommand(vmName); + unmanageInstanceCommand.setConfigDriveAttached(vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.CONFIG_DRIVE_LOCATION) != null); + } + + logger.debug("Selected host ID: {} to persist domain XML for Instance: {}.", agentHostId, vmName); + try { + Answer answer = _agentMgr.send(agentHostId, unmanageInstanceCommand); + if (!answer.getResult()) { + String errorMsg = "Failed to persist domain XML for Instance: " + vmName + " on host ID: " + agentHostId; + logger.debug(errorMsg); + throw new CloudRuntimeException(errorMsg); + } + } catch (AgentUnavailableException | OperationTimedoutException e) { + String errorMsg = "Failed to send command to persist domain XML for Instance: " + vmName + " on host ID: " + agentHostId; + logger.error(errorMsg, e); + throw new CloudRuntimeException(errorMsg); + } + return agentHostId; } /** * Clean up VM snapshots (if any) from DB */ - private void unmanageVMSnapshots(VMInstanceVO vm) { + void unmanageVMSnapshots(VMInstanceVO vm) { _vmSnapshotMgr.deleteVMSnapshotsFromDB(vm.getId(), true); } /** * Clean up volumes for a VM to be unmanaged from CloudStack */ - private void unmanageVMVolumes(VMInstanceVO vm) { + void unmanageVMVolumes(VMInstanceVO vm) { final Long hostId = vm.getHostId() != null ? vm.getHostId() : vm.getLastHostId(); if (hostId != null) { volumeMgr.revokeAccess(vm.getId(), hostId); @@ -2084,7 +2136,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac * - If 'unmanage.vm.preserve.nics' = true: then the NICs are not removed but still Allocated, to preserve MAC addresses * - If 'unmanage.vm.preserve.nics' = false: then the NICs are removed while unmanaging */ - private void unmanageVMNics(VirtualMachineProfile profile, VMInstanceVO vm) { + void unmanageVMNics(VirtualMachineProfile profile, VMInstanceVO vm) { logger.debug("Cleaning up NICs of {}.", vm.toString()); Boolean preserveNics = UnmanagedVMsManager.UnmanageVMPreserveNic.valueIn(vm.getDataCenterId()); if (BooleanUtils.isTrue(preserveNics)) { @@ -4019,6 +4071,62 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac vmTo.setEnterHardwareSetup(enterSetup == null ? false : enterSetup); } + /** + * This method helps constructing vmSpec for Unmanage operation for Stopped Instance + * @param vmId + * @param hostId + * @return VirtualMachineTO + */ + protected VirtualMachineTO prepVmSpecForUnmanageCmd(Long vmId, Long hostId) { + final VMInstanceVO vm = _vmDao.findById(vmId); + final Account owner = _entityMgr.findById(Account.class, vm.getAccountId()); + final ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId()); + final VirtualMachineTemplate template = _entityMgr.findByIdIncludingRemoved(VirtualMachineTemplate.class, vm.getTemplateId()); + Host host = _hostDao.findById(hostId); + VirtualMachineProfileImpl vmProfile = new VirtualMachineProfileImpl(vm, template, offering, owner, null); + updateOverCommitRatioForVmProfile(vmProfile, host.getClusterId()); + final List nics = _nicsDao.listByVmId(vmProfile.getId()); + Collections.sort(nics, (nic1, nic2) -> { + Long nicId1 = Long.valueOf(nic1.getDeviceId()); + Long nicId2 = Long.valueOf(nic2.getDeviceId()); + return nicId1.compareTo(nicId2); + }); + + for (final NicVO nic : nics) { + final Network network = _networkModel.getNetwork(nic.getNetworkId()); + final NicProfile nicProfile = + new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(), null, _networkModel.isSecurityGroupSupportedInNetwork(network), + _networkModel.getNetworkTag(vmProfile.getHypervisorType(), network)); + vmProfile.addNic(nicProfile); + } + + List volumes = _volsDao.findUsableVolumesForInstance(vmId); + for (VolumeVO vol: volumes) { + VolumeInfo volumeInfo = volumeDataFactory.getVolume(vol.getId()); + DataTO dataTO = volumeInfo.getTO(); + DiskTO disk = storageMgr.getDiskWithThrottling(dataTO, vol.getVolumeType(), vol.getDeviceId(), vol.getPath(), vm.getServiceOfferingId(), vol.getDiskOfferingId()); + vmProfile.addDisk(disk); + } + + Map details = vmInstanceDetailsDao.listDetailsKeyPairs(vmId, + List.of(VirtualMachineProfile.Param.BootType.getName(), VirtualMachineProfile.Param.BootMode.getName(), + VirtualMachineProfile.Param.UefiFlag.getName())); + + if (details.containsKey(VirtualMachineProfile.Param.BootType.getName())) { + vmProfile.getParameters().put(VirtualMachineProfile.Param.BootType, details.get(VirtualMachineProfile.Param.BootType.getName())); + } + + if (details.containsKey(VirtualMachineProfile.Param.BootMode.getName())) { + vmProfile.getParameters().put(VirtualMachineProfile.Param.BootMode, details.get(VirtualMachineProfile.Param.BootMode.getName())); + } + + if (details.containsKey(VirtualMachineProfile.Param.UefiFlag.getName())) { + vmProfile.getParameters().put(VirtualMachineProfile.Param.UefiFlag, details.get(VirtualMachineProfile.Param.UefiFlag.getName())); + } + + return toVmTO(vmProfile); + } + protected VirtualMachineTO getVmTO(Long vmId) { final VMInstanceVO vm = _vmDao.findById(vmId); final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); @@ -6185,8 +6293,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac host = host == null ? _hostDao.findById(hostId) : host; if (host != null) { clusterId = host.getClusterId(); + return new Pair<>(clusterId, hostId); } - return new Pair<>(clusterId, hostId); + return findClusterAndHostIdForVmFromVolumes(vm.getId()); } private Pair findClusterAndHostIdForVm(VirtualMachine vm) { diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index 4f6329f81cb..a07870d09af 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -19,14 +19,18 @@ package com.cloud.vm; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -36,6 +40,8 @@ import static org.mockito.Mockito.when; import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -43,15 +49,29 @@ import java.util.Random; import java.util.UUID; import java.util.stream.Collectors; +import com.cloud.agent.api.UnmanageInstanceAnswer; +import com.cloud.agent.api.UnmanageInstanceCommand; +import com.cloud.agent.api.to.DataTO; +import com.cloud.agent.api.to.DiskTO; +import com.cloud.api.ApiDBUtils; +import com.cloud.event.ActionEventUtils; +import com.cloud.exception.ConcurrentOperationException; +import com.cloud.ha.HighAvailabilityManager; +import com.cloud.network.Network; +import com.cloud.network.NetworkModel; import com.cloud.resource.ResourceManager; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl; import org.apache.cloudstack.framework.extensions.dao.ExtensionDetailsDao; import org.apache.cloudstack.framework.extensions.manager.ExtensionsManager; import org.apache.cloudstack.framework.extensions.vo.ExtensionDetailsVO; +import org.apache.cloudstack.framework.jobs.dao.VmWorkJobDao; +import org.apache.cloudstack.framework.jobs.impl.VmWorkJobVO; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.to.VolumeObjectTO; @@ -61,6 +81,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -167,6 +188,9 @@ public class VirtualMachineManagerImplTest { private PrimaryDataStoreDao storagePoolDaoMock; @Mock private VMInstanceVO vmInstanceMock; + @Mock + private VmWorkJobDao _workJobDao; + private long vmInstanceVoMockId = 1L; @Mock @@ -181,6 +205,9 @@ public class VirtualMachineManagerImplTest { private long hostMockId = 1L; private long clusterMockId = 2L; private long zoneMockId = 3L; + private final String vmMockUuid = UUID.randomUUID().toString(); + private final String hostUuid = UUID.randomUUID().toString(); + @Mock private HostVO hostMock; @Mock @@ -192,6 +219,7 @@ public class VirtualMachineManagerImplTest { private StoragePoolVO storagePoolVoMock; private long storagePoolVoMockId = 11L; private long storagePoolVoMockClusterId = 234L; + private String vmName = "vm1"; @Mock private VolumeVO volumeVoMock; @@ -254,6 +282,16 @@ public class VirtualMachineManagerImplTest { NicDao _nicsDao; @Mock NetworkService networkService; + @Mock + NetworkModel networkModel; + @Mock + VolumeDataFactory volumeDataFactoryMock; + @Mock + StorageManager storageManager; + @Mock + private HighAvailabilityManager _haMgr; + @Mock + VirtualMachineGuru guru; private ConfigDepotImpl configDepotImpl; private boolean updatedConfigKeyDepot = false; @@ -263,6 +301,7 @@ public class VirtualMachineManagerImplTest { ReflectionTestUtils.getField(VirtualMachineManager.VmMetadataManufacturer, "s_depot"); virtualMachineManagerImpl.setHostAllocators(new ArrayList<>()); + when(vmInstanceMock.getName()).thenReturn(vmName); when(vmInstanceMock.getId()).thenReturn(vmInstanceVoMockId); when(vmInstanceMock.getServiceOfferingId()).thenReturn(2L); when(hostMock.getId()).thenReturn(hostMockId); @@ -1361,7 +1400,7 @@ public class VirtualMachineManagerImplTest { Mockito.doReturn(HypervisorType.KVM).when(vmInstanceMock).getHypervisorType(); Mockito.doReturn(List.of(new VolumeObjectTO())).when(virtualMachineManagerImpl).getVmVolumesWithCheckpointsToRecreate(Mockito.any()); - Mockito.doThrow(new AgentUnavailableException(0)).when(agentManagerMock).send(Mockito.anyLong(), (Command) any()); + doThrow(new AgentUnavailableException(0)).when(agentManagerMock).send(Mockito.anyLong(), (Command) any()); Mockito.doNothing().when(snapshotManagerMock).endSnapshotChainForVolume(Mockito.anyLong(), Mockito.any()); virtualMachineManagerImpl.recreateCheckpointsKvmOnVmAfterMigration(vmInstanceMock, 0); @@ -1374,7 +1413,7 @@ public class VirtualMachineManagerImplTest { Mockito.doReturn(HypervisorType.KVM).when(vmInstanceMock).getHypervisorType(); Mockito.doReturn(List.of(new VolumeObjectTO())).when(virtualMachineManagerImpl).getVmVolumesWithCheckpointsToRecreate(Mockito.any()); - Mockito.doThrow(new OperationTimedoutException(null, 0, 0, 0, false)).when(agentManagerMock).send(Mockito.anyLong(), (Command) any()); + doThrow(new OperationTimedoutException(null, 0, 0, 0, false)).when(agentManagerMock).send(Mockito.anyLong(), (Command) any()); Mockito.doNothing().when(snapshotManagerMock).endSnapshotChainForVolume(Mockito.anyLong(), Mockito.any()); virtualMachineManagerImpl.recreateCheckpointsKvmOnVmAfterMigration(vmInstanceMock, 0); @@ -1641,4 +1680,278 @@ public class VirtualMachineManagerImplTest { virtualMachineManagerImpl.processPrepareExternalProvisioning(true, host, vmProfile, mock(DataCenter.class)); verify(agentManagerMock).send(anyLong(), any(Command.class)); } + + @Test + public void testPrepVMSpecForUnmanageInstance() { + // Arrange + final Long accountId = 1L; + final Long offeringId = 1L; + final Long templateId = 1L; + + // Mock vm + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + when(vm.getId()).thenReturn(vmInstanceVoMockId); + when(vm.getAccountId()).thenReturn(accountId); + when(vm.getServiceOfferingId()).thenReturn(offeringId); + when(vm.getTemplateId()).thenReturn(templateId); + when(vm.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(vmInstanceDaoMock.findById(vmInstanceVoMockId)).thenReturn(vm); + + // Mock owner + AccountVO owner = Mockito.mock(AccountVO.class); + when(_entityMgr.findById(Account.class, accountId)).thenReturn(owner); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + when(serviceOfferingDaoMock.findById(vmInstanceVoMockId, offeringId)).thenReturn(offering); + + VMTemplateVO template = Mockito.mock(VMTemplateVO.class); + when(_entityMgr.findByIdIncludingRemoved(VirtualMachineTemplate.class, templateId)).thenReturn(template); + + when(hostMock.getClusterId()).thenReturn(clusterMockId); + + // Mock cpuOvercommitRatio and ramOvercommitRatio + ClusterDetailsVO cpuOvercommitRatio = Mockito.mock(ClusterDetailsVO.class); + when(cpuOvercommitRatio.getValue()).thenReturn("1.0"); + when(_clusterDetailsDao.findDetail(clusterMockId, VmDetailConstants.CPU_OVER_COMMIT_RATIO)).thenReturn(cpuOvercommitRatio); + ClusterDetailsVO ramOvercommitRatio = Mockito.mock(ClusterDetailsVO.class); + when(ramOvercommitRatio.getValue()).thenReturn("1.0"); + when(_clusterDetailsDao.findDetail(clusterMockId, VmDetailConstants.MEMORY_OVER_COMMIT_RATIO)).thenReturn(ramOvercommitRatio); + + // Mock NICs + List nics = new ArrayList<>(); + NicVO nic1 = Mockito.mock(NicVO.class); + when(nic1.getDeviceId()).thenReturn(1); + nics.add(nic1); + NicVO nic2 = Mockito.mock(NicVO.class); + when(nic2.getDeviceId()).thenReturn(0); + nics.add(nic2); + when(_nicsDao.listByVmId(vmInstanceVoMockId)).thenReturn(nics); + + Network networkMock = Mockito.mock(Network.class); + when(networkModel.getNetwork(anyLong())).thenReturn(networkMock); + + when(volumeVoMock.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(volumeVoMock.getDeviceId()).thenReturn(0L); + when(volumeVoMock.getPath()).thenReturn("/"); + when(volumeVoMock.getDiskOfferingId()).thenReturn(1L); + when(volumeDaoMock.findUsableVolumesForInstance(vmInstanceVoMockId)).thenReturn(List.of(volumeVoMock)); + + VolumeInfo volumeInfo = mock(VolumeInfo.class); + DataTO dataTO = mock(DataTO.class); + when(volumeInfo.getTO()).thenReturn(dataTO); + when(volumeDataFactoryMock.getVolume(anyLong())).thenReturn(volumeInfo); + when(storageManager.getDiskWithThrottling(any(), any(), anyLong(), anyString(), anyLong(), anyLong())).thenReturn(Mockito.mock(DiskTO.class)); + + Map details = new HashMap<>(); + details.put(VirtualMachineProfile.Param.BootType.getName(), "BIOS"); + details.put(VirtualMachineProfile.Param.BootMode.getName(), "LEGACY"); + details.put(VirtualMachineProfile.Param.UefiFlag.getName(), "Yes"); + when(vmInstanceDetailsDao.listDetailsKeyPairs(anyLong(), anyList())).thenReturn(details); + + com.cloud.hypervisor.HypervisorGuru guru = Mockito.mock(com.cloud.hypervisor.HypervisorGuru.class); + when(_hvGuruMgr.getGuru(HypervisorType.KVM)).thenReturn(guru); + VirtualMachineTO vmTO = new VirtualMachineTO() {}; + when(guru.implement(any(VirtualMachineProfile.class))).thenAnswer((Answer) invocation -> { + VirtualMachineProfile profile = invocation.getArgument(0); + assertEquals("BIOS", profile.getParameter(VirtualMachineProfile.Param.BootType)); + return vmTO; + }); + + // Act + VirtualMachineTO result = virtualMachineManagerImpl.prepVmSpecForUnmanageCmd(vmInstanceVoMockId, hostMockId); + + // Assert + assertNotNull(result); + assertEquals(vmTO, result); + verify(_clusterDetailsDao, times(2)).findDetail(eq(clusterMockId), anyString()); + verify(vmInstanceDetailsDao).listDetailsKeyPairs(anyLong(), anyList()); + } + + @Test + public void testPersistDomainForKvmForRunningVmSuccess() throws AgentUnavailableException, OperationTimedoutException { + when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Running); + when(vmInstanceMock.getHostId()).thenReturn(hostMockId); + UnmanageInstanceAnswer successAnswer = new UnmanageInstanceAnswer(null, true, "success"); + when(agentManagerMock.send(anyLong(), any(Command.class))).thenReturn(successAnswer); + virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock, null); + ArgumentCaptor hostIdCaptor = ArgumentCaptor.forClass(Long.class); + ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(UnmanageInstanceCommand.class); + verify(agentManagerMock).send(hostIdCaptor.capture(), commandCaptor.capture()); + assertEquals(hostMockId, hostIdCaptor.getValue().longValue()); + } + + @Test + public void testPersistDomainForKvmForStoppedVmSuccess() throws AgentUnavailableException, OperationTimedoutException { + when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Stopped); + VirtualMachineTO vmTO = new VirtualMachineTO() {}; + vmTO.setName(vmName); + doReturn(vmTO).when(virtualMachineManagerImpl).prepVmSpecForUnmanageCmd(vmInstanceVoMockId, 1L); + UnmanageInstanceAnswer successAnswer = new UnmanageInstanceAnswer(null, true, "success"); + when(agentManagerMock.send(anyLong(), any(UnmanageInstanceCommand.class))).thenReturn(successAnswer); + when(virtualMachineManagerImpl.findClusterAndHostIdForVm(vmInstanceMock, false)).thenReturn(new Pair<>(clusterMockId, hostMockId)); + virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock, null); + ArgumentCaptor hostIdCaptor = ArgumentCaptor.forClass(Long.class); + ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(UnmanageInstanceCommand.class); + verify(agentManagerMock).send(hostIdCaptor.capture(), commandCaptor.capture()); + assertEquals(1L, hostIdCaptor.getValue().longValue()); + UnmanageInstanceCommand sentCommand = commandCaptor.getValue(); + assertNotNull(sentCommand.getVm()); + assertEquals(vmTO, sentCommand.getVm()); + assertEquals(vmName, sentCommand.getInstanceName()); + verify(virtualMachineManagerImpl).prepVmSpecForUnmanageCmd(vmInstanceVoMockId, 1L); + } + + + @Test + public void testPersistDomainForKvmForStoppedVmNoHost() { + when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Stopped); + VirtualMachineTO vmTO = new VirtualMachineTO() {}; + vmTO.setName(vmName); + when(virtualMachineManagerImpl.findClusterAndHostIdForVm(vmInstanceMock, false)).thenReturn(new Pair<>(clusterMockId, null)); + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock, null)); + assertEquals("No available host to persist domain XML for Instance: " + vmName, exception.getMessage()); + } + + @Test + public void testPersistDomainForKvmForRunningVmAgentFailure() throws AgentUnavailableException, OperationTimedoutException { + when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Running); + when(vmInstanceMock.getHostId()).thenReturn(hostMockId); + UnmanageInstanceAnswer failureAnswer = new UnmanageInstanceAnswer(null, false, "failure"); + when(agentManagerMock.send(anyLong(), any(UnmanageInstanceCommand.class))).thenReturn(failureAnswer); + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock, null)); + assertEquals("Failed to persist domain XML for Instance: " + vmName + " on host ID: " + hostMockId, exception.getMessage()); + } + + @Test + public void testPersistDomainForKvmAgentUnavailable() throws AgentUnavailableException, OperationTimedoutException { + when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Running); + when(vmInstanceMock.getHostId()).thenReturn(hostMockId); + doThrow(new AgentUnavailableException("Agent down", hostMockId)).when(agentManagerMock).send(anyLong(), any(UnmanageInstanceCommand.class)); + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock, null)); + assertEquals("Failed to send command to persist domain XML for Instance: " + vmName + " on host ID: " + hostMockId, exception.getMessage()); + } + + @Test(expected = ConcurrentOperationException.class) + public void testUnmanagePendingHaWork() { + when(vmInstanceDaoMock.findByUuid(vmMockUuid)).thenReturn(vmInstanceMock); + when(_workJobDao.listPendingWorkJobs(VirtualMachine.Type.Instance, vmInstanceVoMockId)).thenReturn(Collections.emptyList()); + when(_haMgr.hasPendingHaWork(vmInstanceVoMockId)).thenReturn(true); + virtualMachineManagerImpl.unmanage(vmMockUuid, null); + } + + @Test + public void testPersistDomainForKvmOperationTimedOut() throws AgentUnavailableException, OperationTimedoutException { + when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Running); + when(vmInstanceMock.getHostId()).thenReturn(hostMockId); + doThrow(new OperationTimedoutException(null, hostMockId, 123L, 60, false)).when(agentManagerMock).send(anyLong(), any(UnmanageInstanceCommand.class)); + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock, null)); + assertEquals("Failed to send command to persist domain XML for Instance: " + vmName + " on host ID: " + hostMockId, exception.getMessage()); + } + + @Test(expected = CloudRuntimeException.class) + public void testUnmanageVmRemoved() { + when(vmInstanceMock.getRemoved()).thenReturn(new Date()); + when(vmInstanceDaoMock.findByUuid(vmMockUuid)).thenReturn(vmInstanceMock); + virtualMachineManagerImpl.unmanage(vmMockUuid, null); + } + + @Test(expected = ConcurrentOperationException.class) + public void testUnmanagePendingWorkJobs() { + when(vmInstanceDaoMock.findByUuid(vmMockUuid)).thenReturn(vmInstanceMock); + List pendingJobs = new ArrayList<>(); + VmWorkJobVO vmWorkJobVO = mock(VmWorkJobVO.class); + pendingJobs.add(vmWorkJobVO); + when(_workJobDao.listPendingWorkJobs(VirtualMachine.Type.Instance, vmInstanceVoMockId)).thenReturn(pendingJobs); + virtualMachineManagerImpl.unmanage(vmMockUuid, null); + } + + @Test + public void testUnmanageHostNotFoundAfterTransaction() { + when(vmInstanceMock.getHostId()).thenReturn(hostMockId); + when(vmInstanceDaoMock.findByUuid(vmMockUuid)).thenReturn(vmInstanceMock); + when(_workJobDao.listPendingWorkJobs(any(), anyLong())).thenReturn(Collections.emptyList()); + when(_haMgr.hasPendingHaWork(anyLong())).thenReturn(false); + doReturn(guru).when(virtualMachineManagerImpl).getVmGuru(vmInstanceMock); + doNothing().when(virtualMachineManagerImpl).unmanageVMSnapshots(vmInstanceMock); + doNothing().when(virtualMachineManagerImpl).unmanageVMNics(any(VirtualMachineProfile.class), any(VMInstanceVO.class)); + doNothing().when(virtualMachineManagerImpl).unmanageVMVolumes(vmInstanceMock); + doNothing().when(guru).finalizeUnmanage(vmInstanceMock); + try (MockedStatic ignored = Mockito.mockStatic(ApiDBUtils.class)) { + when(ApiDBUtils.findHostById(hostMockId)).thenReturn(null); + Pair result = virtualMachineManagerImpl.unmanage(vmMockUuid, null); + assertNull(result.second()); + } + } + + @Test + public void testUnmanageSuccessNonKvm() { + when(vmInstanceMock.getHostId()).thenReturn(hostMockId); + when(hostMock.getUuid()).thenReturn(hostUuid); + when(vmInstanceDaoMock.findByUuid(vmMockUuid)).thenReturn(vmInstanceMock); + when(_workJobDao.listPendingWorkJobs(any(), anyLong())).thenReturn(Collections.emptyList()); + when(_haMgr.hasPendingHaWork(anyLong())).thenReturn(false); + doReturn(guru).when(virtualMachineManagerImpl).getVmGuru(vmInstanceMock); + doNothing().when(virtualMachineManagerImpl).unmanageVMSnapshots(vmInstanceMock); + doNothing().when(virtualMachineManagerImpl).unmanageVMNics(any(VirtualMachineProfile.class), any(VMInstanceVO.class)); + doNothing().when(virtualMachineManagerImpl).unmanageVMVolumes(vmInstanceMock); + doNothing().when(guru).finalizeUnmanage(vmInstanceMock); + try (MockedStatic ignored = Mockito.mockStatic(ApiDBUtils.class)) { + when(ApiDBUtils.findHostById(hostMockId)).thenReturn(hostMock); + try (MockedStatic actionUtil = Mockito.mockStatic(ActionEventUtils.class)) { + actionUtil.when(() -> ActionEventUtils.onActionEvent( + anyLong(), anyLong(), anyLong(), + anyString(), anyString(), + anyLong(), anyString() + )).thenReturn(1L); + + Pair result = virtualMachineManagerImpl.unmanage(vmMockUuid, null); + assertNotNull(result); + assertTrue(result.first()); + assertEquals(hostUuid, result.second()); + + verify(virtualMachineManagerImpl, never()).persistDomainForKVM(any(VMInstanceVO.class), anyLong()); + verify(virtualMachineManagerImpl, times(1)).unmanageVMSnapshots(vmInstanceMock); + verify(virtualMachineManagerImpl, times(1)).unmanageVMNics(any(VirtualMachineProfile.class), any(VMInstanceVO.class)); + verify(virtualMachineManagerImpl, times(1)).unmanageVMVolumes(vmInstanceMock); + verify(guru, times(1)).finalizeUnmanage(vmInstanceMock); + } + } + } + + @Test + public void testUnmanageSuccessKvm() throws Exception { + when(vmInstanceMock.getHostId()).thenReturn(hostMockId); + when(hostMock.getUuid()).thenReturn(hostUuid); + when(vmInstanceMock.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(vmInstanceDaoMock.findByUuid(vmMockUuid)).thenReturn(vmInstanceMock); + when(_workJobDao.listPendingWorkJobs(any(), anyLong())).thenReturn(Collections.emptyList()); + when(_haMgr.hasPendingHaWork(anyLong())).thenReturn(false); + doReturn(guru).when(virtualMachineManagerImpl).getVmGuru(vmInstanceMock); + doNothing().when(virtualMachineManagerImpl).unmanageVMSnapshots(vmInstanceMock); + doNothing().when(virtualMachineManagerImpl).unmanageVMNics(any(VirtualMachineProfile.class), any(VMInstanceVO.class)); + doNothing().when(virtualMachineManagerImpl).unmanageVMVolumes(vmInstanceMock); + doNothing().when(guru).finalizeUnmanage(vmInstanceMock); + try (MockedStatic ignored = Mockito.mockStatic(ApiDBUtils.class)) { + when(ApiDBUtils.findHostById(hostMockId)).thenReturn(hostMock); + try (MockedStatic actionUtil = Mockito.mockStatic(ActionEventUtils.class)) { + actionUtil.when(() -> ActionEventUtils.onActionEvent( + anyLong(), anyLong(), anyLong(), + anyString(), anyString(), + anyLong(), anyString() + )).thenReturn(1L); + UnmanageInstanceAnswer successAnswer = new UnmanageInstanceAnswer(null, true, "success"); + when(agentManagerMock.send(anyLong(), any(UnmanageInstanceCommand.class))).thenReturn(successAnswer); + Pair result = virtualMachineManagerImpl.unmanage(vmMockUuid, null); + assertNotNull(result); + assertTrue(result.first()); + assertEquals(hostUuid, result.second()); + verify(virtualMachineManagerImpl, times(1)).persistDomainForKVM(vmInstanceMock, null); + verify(virtualMachineManagerImpl, times(1)).unmanageVMSnapshots(vmInstanceMock); + verify(virtualMachineManagerImpl, times(1)).unmanageVMNics(any(VirtualMachineProfile.class), any(VMInstanceVO.class)); + verify(virtualMachineManagerImpl, times(1)).unmanageVMVolumes(vmInstanceMock); + verify(guru, times(1)).finalizeUnmanage(vmInstanceMock); + } + } + } + } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnmanageInstanceCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnmanageInstanceCommandWrapper.java new file mode 100644 index 00000000000..33ad274b8d1 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnmanageInstanceCommandWrapper.java @@ -0,0 +1,174 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.hypervisor.kvm.resource.wrapper; + +import java.io.IOException; +import java.io.InputStream; +import java.io.StringWriter; +import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; + +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerException; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.dom.DOMSource; +import javax.xml.transform.stream.StreamResult; +import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathExpressionException; +import javax.xml.xpath.XPathFactory; + +import org.apache.cloudstack.utils.security.ParserUtils; +import org.apache.commons.io.IOUtils; +import org.libvirt.Connect; +import org.libvirt.Domain; +import org.libvirt.LibvirtException; +import org.w3c.dom.Document; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; +import org.xml.sax.SAXException; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.UnmanageInstanceAnswer; +import com.cloud.agent.api.UnmanageInstanceCommand; +import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.exception.InternalErrorException; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.resource.LibvirtKvmAgentHook; +import com.cloud.hypervisor.kvm.resource.LibvirtVMDef; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; + +@ResourceWrapper(handles = UnmanageInstanceCommand.class) +public final class LibvirtUnmanageInstanceCommandWrapper extends CommandWrapper { + + + @Override + public Answer execute(final UnmanageInstanceCommand command, final LibvirtComputingResource libvirtComputingResource) { + String instanceName = command.getInstanceName(); + VirtualMachineTO vmSpec = command.getVm(); + final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); + logger.debug("Attempting to unmanage KVM instance: {}", instanceName); + Domain dom = null; + Connect conn = null; + String vmFinalSpecification; + try { + if (vmSpec == null) { + conn = libvirtUtilitiesHelper.getConnectionByVmName(instanceName); + dom = conn.domainLookupByName(instanceName); + vmFinalSpecification = dom.getXMLDesc(1); + if (command.isConfigDriveAttached()) { + vmFinalSpecification = cleanupConfigDrive(vmFinalSpecification, instanceName); + } + } else { + // define domain using reconstructed vmSpec + logger.debug("Unmanaging Stopped KVM instance: {}", instanceName); + LibvirtVMDef vm = libvirtComputingResource.createVMFromSpec(vmSpec); + libvirtComputingResource.createVbd(conn, vmSpec, instanceName, vm); + conn = libvirtUtilitiesHelper.getConnectionByType(vm.getHvsType()); + String vmInitialSpecification = vm.toString(); + vmFinalSpecification = performXmlTransformHook(vmInitialSpecification, libvirtComputingResource); + } + conn.domainDefineXML(vmFinalSpecification).free(); + logger.debug("Successfully unmanaged KVM instance: {} with domain XML: {}", instanceName, vmFinalSpecification); + return new UnmanageInstanceAnswer(command, true, "Successfully unmanaged"); + } catch (final LibvirtException e) { + logger.error("LibvirtException occurred during unmanaging instance: {} ", instanceName, e); + return new UnmanageInstanceAnswer(command, false, e.getMessage()); + } catch (final IOException + | ParserConfigurationException + | SAXException + | TransformerException + | XPathExpressionException + | InternalErrorException + | URISyntaxException e) { + + logger.error("Failed to unmanage Instance: {}.", instanceName, e); + return new UnmanageInstanceAnswer(command, false, e.getMessage()); + } finally { + if (dom != null) { + try { + dom.free(); + } catch (LibvirtException e) { + logger.error("Ignore libvirt error on free.", e); + } + } + } + } + + String cleanupConfigDrive(String domainXML, String instanceName) throws ParserConfigurationException, IOException, SAXException, XPathExpressionException, TransformerException { + String isoName = "/" + instanceName + ".iso"; + DocumentBuilderFactory docFactory = ParserUtils.getSaferDocumentBuilderFactory(); + DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); + Document document; + try (InputStream inputStream = IOUtils.toInputStream(domainXML, StandardCharsets.UTF_8)) { + document = docBuilder.parse(inputStream); + } + XPathFactory xPathFactory = XPathFactory.newInstance(); + XPath xpath = xPathFactory.newXPath(); + + // Find all elements with source file containing instanceName.iso + String expression = String.format("//disk[@device='cdrom'][source/@file[contains(., '%s')]]", isoName); + NodeList cdromDisks = (NodeList) xpath.evaluate(expression, document, XPathConstants.NODESET); + + // If nothing matched, return original XML + if (cdromDisks == null || cdromDisks.getLength() == 0) { + logger.debug("No config drive found in domain XML for Instance: {}", instanceName); + return domainXML; + } + + // Remove all matched config drive disks + for (int i = 0; i < cdromDisks.getLength(); i++) { + Node diskNode = cdromDisks.item(i); + if (diskNode != null && diskNode.getParentNode() != null) { + diskNode.getParentNode().removeChild(diskNode); + } + } + logger.debug("Removed {} config drive ISO CD-ROM entries for instance: {}", cdromDisks.getLength(), instanceName); + + TransformerFactory transformerFactory = ParserUtils.getSaferTransformerFactory(); + Transformer transformer = transformerFactory.newTransformer(); + DOMSource source = new DOMSource(document); + StringWriter output = new StringWriter(); + StreamResult result = new StreamResult(output); + transformer.transform(source, result); + return output.toString(); + } + + private String performXmlTransformHook(String vmInitialSpecification, final LibvirtComputingResource libvirtComputingResource) { + String vmFinalSpecification; + try { + // if transformer fails, everything must go as it's just skipped. + LibvirtKvmAgentHook t = libvirtComputingResource.getTransformer(); + vmFinalSpecification = (String) t.handle(vmInitialSpecification); + if (null == vmFinalSpecification) { + logger.warn("Libvirt XML transformer returned NULL, will use XML specification unchanged."); + vmFinalSpecification = vmInitialSpecification; + } + } catch(Exception e) { + logger.warn("Exception occurred when handling LibVirt XML transformer hook: {}", e); + vmFinalSpecification = vmInitialSpecification; + } + return vmFinalSpecification; + } +} diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnmanageInstanceCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnmanageInstanceCommandWrapperTest.java new file mode 100644 index 00000000000..13ddc26610c --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnmanageInstanceCommandWrapperTest.java @@ -0,0 +1,357 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// +package com.cloud.hypervisor.kvm.resource.wrapper; + +import java.io.IOException; + +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.transform.TransformerException; +import javax.xml.xpath.XPathExpressionException; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; +import org.xml.sax.SAXException; + + +@RunWith(MockitoJUnitRunner.class) +public class LibvirtUnmanageInstanceCommandWrapperTest { + @Spy + LibvirtUnmanageInstanceCommandWrapper unmanageInstanceCommandWrapper = new LibvirtUnmanageInstanceCommandWrapper(); + + @Test + public void testCleanupConfigDriveFromDomain() throws XPathExpressionException, ParserConfigurationException, IOException, TransformerException, SAXException { + String domainXML = "\n" + + " i-2-6-VM\n" + + " 071628d0-84f1-421e-a9cf-d18bca2283bc\n" + + " CentOS 5.5 (64-bit)\n" + + " 524288\n" + + " 524288\n" + + " 1\n" + + " \n" + + " 250\n" + + " \n" + + " \n" + + " /machine\n" + + " \n" + + " \n" + + " \n" + + " Apache Software Foundation\n" + + " CloudStack KVM Hypervisor\n" + + " 071628d0-84f1-421e-a9cf-d18bca2283bc\n" + + " 071628d0-84f1-421e-a9cf-d18bca2283bc\n" + + " \n" + + " \n" + + " \n" + + " hvm\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " qemu64\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " destroy\n" + + " restart\n" + + " destroy\n" + + " \n" + + " /usr/libexec/qemu-kvm\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " 9329e4fa9db546c78b1a\n" + + " \n" + + "
\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "
\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "
\n" + + " \n" + + " \n" + + " \n" + + "
\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "
\n" + + " \n" + + " \n" + + " \n" + + "
\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "
\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "
\n" + + " \n" + + " \n" + + " \n" + + "
\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "