From 88333b33a9d3417e70f4a4686499a37e7d1a78ee Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Mon, 1 Sep 2025 11:33:00 +0530 Subject: [PATCH 01/17] persist kvm domain when unmanaged from CS --- .../agent/api/UnmanageInstanceAnswer.java | 32 +++++++ .../agent/api/UnmanageInstanceCommand.java | 50 +++++++++++ .../cloud/vm/VirtualMachineManagerImpl.java | 57 +++++++----- .../resource/LibvirtComputingResource.java | 8 ++ ...LibvirtUnmanageInstanceCommandWrapper.java | 87 +++++++++++++++++++ 5 files changed, 212 insertions(+), 22 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 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 000000000000..a20db18e73fe --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceAnswer.java @@ -0,0 +1,32 @@ +// +// 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); + } + + public UnmanageInstanceAnswer(UnmanageInstanceCommand cmd, Exception e) { + super(cmd, e); + } + +} 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 000000000000..19a006151499 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java @@ -0,0 +1,50 @@ +// +// 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 { + VirtualMachineTO vm; + boolean executeInSequence = false; + + public VirtualMachineTO getVirtualMachine() { + return vm; + } + + @Override + public boolean executeInSequence() { + //VR start doesn't go through queue + if (vm.getName() != null && vm.getName().startsWith("r-")) { + return false; + } + return executeInSequence; + } + + protected UnmanageInstanceCommand() { + } + + public UnmanageInstanceCommand(VirtualMachineTO vm, boolean executeInSequence) { + this.vm = vm; + this.executeInSequence = executeInSequence; + } +} 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 3a6e1b622774..215523d2244d 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -49,7 +49,6 @@ import javax.naming.ConfigurationException; import javax.persistence.EntityExistsException; - import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; @@ -150,6 +149,8 @@ import com.cloud.agent.api.StopCommand; import com.cloud.agent.api.UnPlugNicAnswer; import com.cloud.agent.api.UnPlugNicCommand; +import com.cloud.agent.api.UnmanageInstanceAnswer; +import com.cloud.agent.api.UnmanageInstanceCommand; import com.cloud.agent.api.UnregisterVMCommand; import com.cloud.agent.api.VmDiskStatsEntry; import com.cloud.agent.api.VmNetworkStatsEntry; @@ -297,8 +298,8 @@ import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.UserVmDao; -import com.cloud.vm.dao.VMInstanceDetailsDao; import com.cloud.vm.dao.VMInstanceDao; +import com.cloud.vm.dao.VMInstanceDetailsDao; import com.cloud.vm.snapshot.VMSnapshotManager; import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; @@ -2014,31 +2015,43 @@ public boolean unmanage(String vmUuid) { throw new ConcurrentOperationException(msg); } - Boolean result = Transaction.execute(new TransactionCallback() { - @Override - public Boolean doInTransaction(TransactionStatus status) { + boolean isCmdAnswerSuccess = true; + // define domain for kvm host + if (HypervisorType.KVM.equals(vm.getHypervisorType())) { + final UnmanageInstanceCommand unmanageInstanceCommand = new UnmanageInstanceCommand(getVmTO(vm.getId()), false); + try { + Answer answer = _agentMgr.send(vm.getHostId(), unmanageInstanceCommand); + isCmdAnswerSuccess = (answer instanceof UnmanageInstanceAnswer && answer.getResult()); + } catch (Exception ex) { + isCmdAnswerSuccess = false; + } + } - logger.debug("Unmanaging VM {}", vm); + if (isCmdAnswerSuccess) { + logger.debug("Unmanaging VM {}", vm); + Boolean result = Transaction.execute(new TransactionCallback() { + @Override + public Boolean doInTransaction(TransactionStatus status) { + final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); + final VirtualMachineGuru guru = getVmGuru(vm); - final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); - final VirtualMachineGuru guru = getVmGuru(vm); + try { + unmanageVMSnapshots(vm); + unmanageVMNics(profile, vm); + unmanageVMVolumes(vm); - try { - unmanageVMSnapshots(vm); - unmanageVMNics(profile, vm); - unmanageVMVolumes(vm); + guru.finalizeUnmanage(vm); + } catch (Exception e) { + logger.error("Error while unmanaging VM {}", vm, e); + return false; + } - guru.finalizeUnmanage(vm); - } catch (Exception e) { - logger.error("Error while unmanaging VM {}", vm, e); - return false; + return true; } - - return true; - } - }); - - return BooleanUtils.isTrue(result); + }); + return BooleanUtils.isTrue(result); + } + return false; } /** diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 9fd33c7284ce..a7ff01ddb9d9 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -2205,6 +2205,14 @@ public String startVM(final Connect conn, final String vmName, final String doma return null; } + public void defineVMDomain(final Connect conn, final String domainXML) throws LibvirtException { + try { + conn.domainDefineXML(domainXML); + } catch (final LibvirtException ex) { + throw ex; + } + } + @Override public boolean stop() { try { 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 000000000000..7db61fc5c4a7 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnmanageInstanceCommandWrapper.java @@ -0,0 +1,87 @@ +// +// 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 org.libvirt.Connect; +import org.libvirt.LibvirtException; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.UnmanageInstanceAnswer; +import com.cloud.agent.api.UnmanageInstanceCommand; +import com.cloud.agent.api.to.NicTO; +import com.cloud.agent.api.to.VirtualMachineTO; +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; +import com.cloud.vm.VirtualMachine; + +@ResourceWrapper(handles = UnmanageInstanceCommand.class) +public final class LibvirtUnmanageInstanceCommandWrapper extends CommandWrapper { + + + @Override + public Answer execute(final UnmanageInstanceCommand command, final LibvirtComputingResource libvirtComputingResource) { + final VirtualMachineTO vmSpec = command.getVirtualMachine(); + LibvirtVMDef vm = null; + final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); + Connect conn = null; + try { + + vm = libvirtComputingResource.createVMFromSpec(vmSpec); + conn = libvirtUtilitiesHelper.getConnectionByType(vm.getHvsType()); + + final NicTO[] nics = vmSpec.getNics(); + + for (final NicTO nic : nics) { + if (vmSpec.getType() != VirtualMachine.Type.User) { + nic.setPxeDisable(true); + } + } + + String vmInitialSpecification = vm.toString(); + String vmFinalSpecification = performXmlTransformHook(vmInitialSpecification, libvirtComputingResource); + libvirtComputingResource.defineVMDomain(conn, vmFinalSpecification); + + return new UnmanageInstanceAnswer(command, true, "Successfully unmanaged"); + } catch (final LibvirtException ex) { + logger.warn("LibvirtException ", ex); + return new UnmanageInstanceAnswer(command, false, ex.getMessage()); + } + } + + 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; + } +} From b8901a182768febd9f08aef8bd56fcf940c673df Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Tue, 2 Sep 2025 09:38:27 +0530 Subject: [PATCH 02/17] fetch domainxml from vm --- .../agent/api/UnmanageInstanceAnswer.java | 5 -- .../agent/api/UnmanageInstanceCommand.java | 19 +++---- .../cloud/vm/VirtualMachineManagerImpl.java | 27 +++++++--- ...LibvirtUnmanageInstanceCommandWrapper.java | 51 ++++--------------- 4 files changed, 37 insertions(+), 65 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceAnswer.java b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceAnswer.java index a20db18e73fe..39a35d499906 100644 --- a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceAnswer.java @@ -24,9 +24,4 @@ public class UnmanageInstanceAnswer extends Answer { public UnmanageInstanceAnswer(UnmanageInstanceCommand cmd, boolean success, String details) { super(cmd, success, details); } - - public UnmanageInstanceAnswer(UnmanageInstanceCommand cmd, Exception e) { - super(cmd, e); - } - } diff --git a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java index 19a006151499..2ea982b06a9a 100644 --- a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java +++ b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java @@ -19,32 +19,27 @@ package com.cloud.agent.api; -import com.cloud.agent.api.to.VirtualMachineTO; - /** */ public class UnmanageInstanceCommand extends Command { - VirtualMachineTO vm; + String instanceName; boolean executeInSequence = false; - public VirtualMachineTO getVirtualMachine() { - return vm; - } - @Override public boolean executeInSequence() { //VR start doesn't go through queue - if (vm.getName() != null && vm.getName().startsWith("r-")) { + if (instanceName != null && instanceName.startsWith("r-")) { return false; } return executeInSequence; } - protected UnmanageInstanceCommand() { + public UnmanageInstanceCommand(String instanceName, boolean executeInSequence) { + this.instanceName = instanceName; + this.executeInSequence = executeInSequence; } - public UnmanageInstanceCommand(VirtualMachineTO vm, boolean executeInSequence) { - this.vm = vm; - this.executeInSequence = executeInSequence; + public String getInstanceName() { + return instanceName; } } 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 215523d2244d..43c23b2b8269 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1171,6 +1171,8 @@ protected void checkAndAttemptMigrateVmAcrossCluster(final VMInstanceVO vm, fina markVolumesInPool(vm, answer); } + + protected void updateVmMetadataManufacturerAndProduct(VirtualMachineTO vmTO, VMInstanceVO vm) { String metadataManufacturer = VmMetadataManufacturer.valueIn(vm.getDataCenterId()); if (StringUtils.isBlank(metadataManufacturer)) { @@ -2015,19 +2017,19 @@ public boolean unmanage(String vmUuid) { throw new ConcurrentOperationException(msg); } - boolean isCmdAnswerSuccess = true; - // define domain for kvm host + boolean isDomainXMLPreserved = true; + // persist domain for kvm host if (HypervisorType.KVM.equals(vm.getHypervisorType())) { - final UnmanageInstanceCommand unmanageInstanceCommand = new UnmanageInstanceCommand(getVmTO(vm.getId()), false); + final UnmanageInstanceCommand unmanageInstanceCommand = new UnmanageInstanceCommand(vm.getName(), false); try { Answer answer = _agentMgr.send(vm.getHostId(), unmanageInstanceCommand); - isCmdAnswerSuccess = (answer instanceof UnmanageInstanceAnswer && answer.getResult()); + isDomainXMLPreserved = (answer instanceof UnmanageInstanceAnswer && answer.getResult()); } catch (Exception ex) { - isCmdAnswerSuccess = false; + isDomainXMLPreserved = false; } } - if (isCmdAnswerSuccess) { + if (isDomainXMLPreserved) { logger.debug("Unmanaging VM {}", vm); Boolean result = Transaction.execute(new TransactionCallback() { @Override @@ -2050,6 +2052,8 @@ public Boolean doInTransaction(TransactionStatus status) { } }); return BooleanUtils.isTrue(result); + } else { + logger.error("Error encountered persisting domainXML for vm: {}", vm.getName()); } return false; } @@ -4017,6 +4021,17 @@ private void checkAndSetEnterSetupMode(VirtualMachineTO vmTo, Map { @@ -40,48 +36,19 @@ public final class LibvirtUnmanageInstanceCommandWrapper extends CommandWrapper< @Override public Answer execute(final UnmanageInstanceCommand command, final LibvirtComputingResource libvirtComputingResource) { - final VirtualMachineTO vmSpec = command.getVirtualMachine(); - LibvirtVMDef vm = null; + String instanceName = command.getInstanceName(); final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); - Connect conn = null; + logger.debug("Unmanaging KVM instance: {}", instanceName); try { - - vm = libvirtComputingResource.createVMFromSpec(vmSpec); - conn = libvirtUtilitiesHelper.getConnectionByType(vm.getHvsType()); - - final NicTO[] nics = vmSpec.getNics(); - - for (final NicTO nic : nics) { - if (vmSpec.getType() != VirtualMachine.Type.User) { - nic.setPxeDisable(true); - } - } - - String vmInitialSpecification = vm.toString(); - String vmFinalSpecification = performXmlTransformHook(vmInitialSpecification, libvirtComputingResource); - libvirtComputingResource.defineVMDomain(conn, vmFinalSpecification); - + Connect conn = libvirtUtilitiesHelper.getConnectionByVmName(instanceName); + Domain dom = conn.domainLookupByName(instanceName); + String domainXML = dom.getXMLDesc(1); + conn.domainDefineXML(domainXML); + logger.debug("Successfully unmanaged KVM instance: {}", instanceName); return new UnmanageInstanceAnswer(command, true, "Successfully unmanaged"); } catch (final LibvirtException ex) { - logger.warn("LibvirtException ", ex); + logger.warn("LibvirtException occurred during unmanaging instance: {} ", instanceName, ex); return new UnmanageInstanceAnswer(command, false, ex.getMessage()); } } - - 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; - } } From c9bfec1d54b8e80cead452bbbaadece642923cd8 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Tue, 2 Sep 2025 15:41:51 +0530 Subject: [PATCH 03/17] cleanup code --- .../com/cloud/vm/VirtualMachineManagerImpl.java | 11 ----------- .../kvm/resource/LibvirtComputingResource.java | 13 ++----------- 2 files changed, 2 insertions(+), 22 deletions(-) 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 43c23b2b8269..fe51b1ed1bb6 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -4021,17 +4021,6 @@ private void checkAndSetEnterSetupMode(VirtualMachineTO vmTo, Map Date: Thu, 4 Sep 2025 16:14:41 +0530 Subject: [PATCH 04/17] rescontruct vmspec to unmanage Stopped VM --- .../agent/api/UnmanageInstanceCommand.java | 15 ++++- .../cloud/vm/VirtualMachineManagerImpl.java | 46 ++++++++++++- ...LibvirtUnmanageInstanceCommandWrapper.java | 64 ++++++++++++++++--- .../vm/UnmanagedVMsManagerImpl.java | 5 +- 4 files changed, 116 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java index 2ea982b06a9a..9323bce4c534 100644 --- a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java +++ b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java @@ -19,11 +19,14 @@ package com.cloud.agent.api; +import com.cloud.agent.api.to.VirtualMachineTO; + /** */ public class UnmanageInstanceCommand extends Command { String instanceName; boolean executeInSequence = false; + VirtualMachineTO vm; @Override public boolean executeInSequence() { @@ -34,12 +37,20 @@ public boolean executeInSequence() { return executeInSequence; } - public UnmanageInstanceCommand(String instanceName, boolean executeInSequence) { + public UnmanageInstanceCommand(VirtualMachineTO vm) { + this.vm = vm; + this.instanceName = vm.getName(); + } + + public UnmanageInstanceCommand(String instanceName) { this.instanceName = instanceName; - this.executeInSequence = executeInSequence; } public String getInstanceName() { return instanceName; } + + public VirtualMachineTO getVm() { + return vm; + } } 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 fe51b1ed1bb6..91f52a150f9e 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -70,6 +70,7 @@ 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; @@ -156,6 +157,7 @@ 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; @@ -2020,9 +2022,16 @@ public boolean unmanage(String vmUuid) { boolean isDomainXMLPreserved = true; // persist domain for kvm host if (HypervisorType.KVM.equals(vm.getHypervisorType())) { - final UnmanageInstanceCommand unmanageInstanceCommand = new UnmanageInstanceCommand(vm.getName(), false); + long hostId = vm.getHostId(); + UnmanageInstanceCommand unmanageInstanceCommand; + if (State.Stopped.equals(vm.getState())) { + hostId = vm.getLastHostId(); + unmanageInstanceCommand = new UnmanageInstanceCommand(prepareVmTO(vm.getId(), hostId)); // reconstruct vmSpec + } else { + unmanageInstanceCommand = new UnmanageInstanceCommand(vm.getName()); + } try { - Answer answer = _agentMgr.send(vm.getHostId(), unmanageInstanceCommand); + Answer answer = _agentMgr.send(hostId, unmanageInstanceCommand); isDomainXMLPreserved = (answer instanceof UnmanageInstanceAnswer && answer.getResult()); } catch (Exception ex) { isDomainXMLPreserved = false; @@ -4021,6 +4030,39 @@ private void checkAndSetEnterSetupMode(VirtualMachineTO vmTo, Map 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 volTO = volumeInfo.getTO(); + DiskTO disk = storageMgr.getDiskWithThrottling(volTO, vol.getVolumeType(), vol.getDeviceId(), vol.getPath(), vm.getServiceOfferingId(), vol.getDiskOfferingId()); + vmProfile.addDisk(disk); + } + return toVmTO(vmProfile); + } + protected VirtualMachineTO getVmTO(Long vmId) { final VMInstanceVO vm = _vmDao.findById(vmId); final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); 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 index 94a8b40cd5fd..0c525426fa53 100644 --- 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 @@ -19,6 +19,8 @@ package com.cloud.hypervisor.kvm.resource.wrapper; +import java.net.URISyntaxException; + import org.libvirt.Connect; import org.libvirt.Domain; import org.libvirt.LibvirtException; @@ -26,7 +28,11 @@ 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; @@ -37,18 +43,60 @@ 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("Unmanaging KVM instance: {}", instanceName); + logger.debug("Attempting to unmanage KVM instance: {}", instanceName); + Domain dom = null; + Connect conn = null; try { - Connect conn = libvirtUtilitiesHelper.getConnectionByVmName(instanceName); - Domain dom = conn.domainLookupByName(instanceName); - String domainXML = dom.getXMLDesc(1); - conn.domainDefineXML(domainXML); + if (vmSpec == null) { + conn = libvirtUtilitiesHelper.getConnectionByVmName(instanceName); + dom = conn.domainLookupByName(instanceName); + String domainXML = dom.getXMLDesc(1); + conn.domainDefineXML(domainXML).free(); + } 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(); + String vmFinalSpecification = performXmlTransformHook(vmInitialSpecification, libvirtComputingResource); + conn.domainDefineXML(vmFinalSpecification).free(); + } logger.debug("Successfully unmanaged KVM instance: {}", instanceName); return new UnmanageInstanceAnswer(command, true, "Successfully unmanaged"); - } catch (final LibvirtException ex) { - logger.warn("LibvirtException occurred during unmanaging instance: {} ", instanceName, ex); - return new UnmanageInstanceAnswer(command, false, ex.getMessage()); + } catch (final LibvirtException e) { + logger.warn("LibvirtException occurred during unmanaging instance: {} ", instanceName, e); + return new UnmanageInstanceAnswer(command, false, e.getMessage()); + } catch (final URISyntaxException | InternalErrorException e) { + logger.warn("URISyntaxException ", 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); + } + } + } + } + + 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/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index ad3d2a0f0ec2..52a436215ff4 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -2287,10 +2287,11 @@ public boolean unmanageVMInstance(long vmId) { performUnmanageVMInstancePrechecks(vmVO); - Long hostId = findSuitableHostId(vmVO); + boolean isVMStopped = VirtualMachine.State.Stopped.equals(vmVO.getState()); + Long hostId = isVMStopped ? vmVO.getLastHostId() : findSuitableHostId(vmVO); String instanceName = vmVO.getInstanceName(); - if (!existsVMToUnmanage(instanceName, hostId)) { + if (!isVMStopped && !existsVMToUnmanage(instanceName, hostId)) { throw new CloudRuntimeException(String.format("VM %s is not found in the hypervisor", vmVO)); } From 24c4280ece92a52c6168d3b84dcff6de65b8660b Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Thu, 4 Sep 2025 23:55:21 +0530 Subject: [PATCH 05/17] handle initial review comments --- .../agent/api/UnmanageInstanceCommand.java | 4 - .../cloud/vm/VirtualMachineManagerImpl.java | 94 +++++++++++-------- .../resource/LibvirtComputingResource.java | 5 +- 3 files changed, 56 insertions(+), 47 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java index 9323bce4c534..f728cb0ed745 100644 --- a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java +++ b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java @@ -30,10 +30,6 @@ public class UnmanageInstanceCommand extends Command { @Override public boolean executeInSequence() { - //VR start doesn't go through queue - if (instanceName != null && instanceName.startsWith("r-")) { - return false; - } return executeInSequence; } 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 91f52a150f9e..8a2f7a2ebe79 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -150,7 +150,6 @@ import com.cloud.agent.api.StopCommand; import com.cloud.agent.api.UnPlugNicAnswer; import com.cloud.agent.api.UnPlugNicCommand; -import com.cloud.agent.api.UnmanageInstanceAnswer; import com.cloud.agent.api.UnmanageInstanceCommand; import com.cloud.agent.api.UnregisterVMCommand; import com.cloud.agent.api.VmDiskStatsEntry; @@ -1173,8 +1172,6 @@ protected void checkAndAttemptMigrateVmAcrossCluster(final VMInstanceVO vm, fina markVolumesInPool(vm, answer); } - - protected void updateVmMetadataManufacturerAndProduct(VirtualMachineTO vmTO, VMInstanceVO vm) { String metadataManufacturer = VmMetadataManufacturer.valueIn(vm.getDataCenterId()); if (StringUtils.isBlank(metadataManufacturer)) { @@ -2019,52 +2016,61 @@ public boolean unmanage(String vmUuid) { throw new ConcurrentOperationException(msg); } - boolean isDomainXMLPreserved = true; - // persist domain for kvm host if (HypervisorType.KVM.equals(vm.getHypervisorType())) { - long hostId = vm.getHostId(); - UnmanageInstanceCommand unmanageInstanceCommand; - if (State.Stopped.equals(vm.getState())) { - hostId = vm.getLastHostId(); - unmanageInstanceCommand = new UnmanageInstanceCommand(prepareVmTO(vm.getId(), hostId)); // reconstruct vmSpec - } else { - unmanageInstanceCommand = new UnmanageInstanceCommand(vm.getName()); - } - try { - Answer answer = _agentMgr.send(hostId, unmanageInstanceCommand); - isDomainXMLPreserved = (answer instanceof UnmanageInstanceAnswer && answer.getResult()); - } catch (Exception ex) { - isDomainXMLPreserved = false; - } + persistDomainForKVM(vm); } + Boolean result = Transaction.execute(new TransactionCallback() { + @Override + public Boolean doInTransaction(TransactionStatus status) { - if (isDomainXMLPreserved) { - logger.debug("Unmanaging VM {}", vm); - Boolean result = Transaction.execute(new TransactionCallback() { - @Override - public Boolean doInTransaction(TransactionStatus status) { - final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); - final VirtualMachineGuru guru = getVmGuru(vm); + logger.debug("Unmanaging VM {}", vm); - try { - unmanageVMSnapshots(vm); - unmanageVMNics(profile, vm); - unmanageVMVolumes(vm); + final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); + final VirtualMachineGuru guru = getVmGuru(vm); - guru.finalizeUnmanage(vm); - } catch (Exception e) { - logger.error("Error while unmanaging VM {}", vm, e); - return false; - } + try { + unmanageVMSnapshots(vm); + unmanageVMNics(profile, vm); + unmanageVMVolumes(vm); - return true; + guru.finalizeUnmanage(vm); + } catch (Exception e) { + logger.error("Error while unmanaging VM {}", vm, e); + return false; } - }); - return BooleanUtils.isTrue(result); + + return true; + } + }); + + return BooleanUtils.isTrue(result); + } + + void persistDomainForKVM(VMInstanceVO vm) { + long hostId = vm.getHostId(); + UnmanageInstanceCommand unmanageInstanceCommand; + if (State.Stopped.equals(vm.getState())) { + hostId = vm.getLastHostId(); + unmanageInstanceCommand = new UnmanageInstanceCommand(prepVmSpecForUnmanageCmd(vm.getId(), hostId)); // reconstruct vmSpec for stopped instance } else { - logger.error("Error encountered persisting domainXML for vm: {}", vm.getName()); + unmanageInstanceCommand = new UnmanageInstanceCommand(vm.getName()); + } + try { + Answer answer = _agentMgr.send(hostId, unmanageInstanceCommand); + if (!answer.getResult()) { + String errorMsg = "Failed to persist domainXML for instance: " + vm.getName(); + logger.debug(errorMsg); + throw new CloudRuntimeException(errorMsg); + } + } catch (AgentUnavailableException e) { + String errorMsg = "Failed to send command, agent unavailable"; + logger.error(errorMsg, e); + throw new CloudRuntimeException(errorMsg); + } catch (OperationTimedoutException e) { + String errorMsg = "Failed to send command, operation timed out"; + logger.error(errorMsg, e); + throw new CloudRuntimeException(errorMsg); } - return false; } /** @@ -4030,7 +4036,13 @@ private void checkAndSetEnterSetupMode(VirtualMachineTO vmTo, Map Date: Mon, 8 Sep 2025 11:17:05 +0530 Subject: [PATCH 06/17] add boot type, mode and uefi flag --- .../com/cloud/vm/VirtualMachineManagerImpl.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 8a2f7a2ebe79..b94b3821cb85 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -4072,6 +4072,20 @@ protected VirtualMachineTO prepVmSpecForUnmanageCmd(Long vmId, Long hostId) { DiskTO disk = storageMgr.getDiskWithThrottling(volTO, vol.getVolumeType(), vol.getDeviceId(), vol.getPath(), vm.getServiceOfferingId(), vol.getDiskOfferingId()); vmProfile.addDisk(disk); } + + Map details = vmInstanceDetailsDao.listDetailsKeyPairs(vmId); + 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); } From da3c4556923c04954a8e6652c3b4a1691e33af79 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Mon, 8 Sep 2025 17:02:46 +0530 Subject: [PATCH 07/17] unit tests to improve coverage --- .../cloud/vm/VirtualMachineManagerImpl.java | 12 +- .../vm/VirtualMachineManagerImplTest.java | 173 +++++++++++++++++- .../vm/UnmanagedVMsManagerImpl.java | 2 +- .../vm/UnmanagedVMsManagerImplTest.java | 13 ++ 4 files changed, 193 insertions(+), 7 deletions(-) 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 b94b3821cb85..1c44f25b1a3c 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -49,6 +49,7 @@ import javax.naming.ConfigurationException; import javax.persistence.EntityExistsException; + import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; @@ -299,8 +300,8 @@ import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.UserVmDao; -import com.cloud.vm.dao.VMInstanceDao; import com.cloud.vm.dao.VMInstanceDetailsDao; +import com.cloud.vm.dao.VMInstanceDao; import com.cloud.vm.snapshot.VMSnapshotManager; import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; @@ -4068,12 +4069,15 @@ protected VirtualMachineTO prepVmSpecForUnmanageCmd(Long vmId, Long hostId) { List volumes = _volsDao.findUsableVolumesForInstance(vmId); for (VolumeVO vol: volumes) { VolumeInfo volumeInfo = volumeDataFactory.getVolume(vol.getId()); - DataTO volTO = volumeInfo.getTO(); - DiskTO disk = storageMgr.getDiskWithThrottling(volTO, vol.getVolumeType(), vol.getDeviceId(), vol.getPath(), vm.getServiceOfferingId(), vol.getDiskOfferingId()); + 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); + 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())); } 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 4f6329f81cb8..153311b15cd8 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 @@ 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; @@ -43,10 +47,18 @@ 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.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; @@ -61,6 +73,7 @@ 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; @@ -192,6 +205,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 +268,12 @@ public class VirtualMachineManagerImplTest { NicDao _nicsDao; @Mock NetworkService networkService; + @Mock + NetworkModel networkModel; + @Mock + VolumeDataFactory volumeDataFactoryMock; + @Mock + StorageManager storageManager; private ConfigDepotImpl configDepotImpl; private boolean updatedConfigKeyDepot = false; @@ -263,6 +283,7 @@ public void setup() { 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 +1382,7 @@ public void recreateCheckpointsKvmOnVmAfterMigrationTestAgentUnavailableThrowsCl 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 +1395,7 @@ public void recreateCheckpointsKvmOnVmAfterMigrationTestOperationTimedoutExcepti 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 +1662,152 @@ public void processPrepareExternalProvisioning_externalHypervisor_sendsCommand() 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); + 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); + when(vmInstanceMock.getLastHostId()).thenReturn(1L); + 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); + virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock); + 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 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)); + assertEquals("Failed to persist domainXML for instance: " + vmName, 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)); + assertEquals("Failed to send command, agent unavailable", exception.getMessage()); + } + + @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)); + assertEquals("Failed to send command, operation timed out", exception.getMessage()); + } } diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 52a436215ff4..7e4514909132 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -2205,7 +2205,7 @@ public List> getCommands() { * - VM must not have any associated volume snapshot * - VM must not have an attached ISO */ - private void performUnmanageVMInstancePrechecks(VMInstanceVO vmVO) { + void performUnmanageVMInstancePrechecks(VMInstanceVO vmVO) { if (hasVolumeSnapshotsPriorToUnmanageVM(vmVO)) { throw new UnsupportedServiceException("Cannot unmanage VM with id = " + vmVO.getUuid() + " as there are volume snapshots for its volume(s). Please remove snapshots before unmanaging."); diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index 4ea6af7c6f42..db623eba538e 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -70,6 +70,7 @@ import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; import com.cloud.agent.AgentManager; @@ -167,6 +168,7 @@ @RunWith(MockitoJUnitRunner.class) public class UnmanagedVMsManagerImplTest { + @Spy @InjectMocks private UnmanagedVMsManagerImpl unmanagedVMsManager = new UnmanagedVMsManagerImpl(); @@ -479,6 +481,17 @@ public void unmanageVMInstanceExistingISOAttachedTest() { unmanagedVMsManager.unmanageVMInstance(virtualMachineId); } + @Test + public void unmanageVMInstanceStoppedInstanceTest() { + when(virtualMachine.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + when(virtualMachine.getType()).thenReturn(VirtualMachine.Type.User); + when(virtualMachine.getState()).thenReturn(VirtualMachine.State.Stopped); + UserVmVO userVmVO = mock(UserVmVO.class); + when(userVmDao.findById(anyLong())).thenReturn(userVmVO); + Mockito.doNothing().when(unmanagedVMsManager).performUnmanageVMInstancePrechecks(any()); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId); + } + @Test public void testListRemoteInstancesTest() { ListVmsForImportCmd cmd = Mockito.mock(ListVmsForImportCmd.class); From 85242754e848a7f2c69cb8e52bf1ac3267934ee2 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Wed, 10 Sep 2025 10:28:41 +0530 Subject: [PATCH 08/17] add integration tests for unmanaging kvm instance --- .../cloud/vm/VirtualMachineManagerImpl.java | 2 +- .../test_vm_lifecycle_unmanage_kvm_import.py | 473 ++++++++++++++++++ tools/marvin/marvin/lib/base.py | 5 +- 3 files changed, 477 insertions(+), 3 deletions(-) create mode 100644 test/integration/smoke/test_vm_lifecycle_unmanage_kvm_import.py 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 1c44f25b1a3c..f14e850537a5 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2048,7 +2048,7 @@ public Boolean doInTransaction(TransactionStatus status) { } void persistDomainForKVM(VMInstanceVO vm) { - long hostId = vm.getHostId(); + Long hostId = vm.getHostId(); UnmanageInstanceCommand unmanageInstanceCommand; if (State.Stopped.equals(vm.getState())) { hostId = vm.getLastHostId(); diff --git a/test/integration/smoke/test_vm_lifecycle_unmanage_kvm_import.py b/test/integration/smoke/test_vm_lifecycle_unmanage_kvm_import.py new file mode 100644 index 000000000000..f39b15078426 --- /dev/null +++ b/test/integration/smoke/test_vm_lifecycle_unmanage_kvm_import.py @@ -0,0 +1,473 @@ +# 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. +""" BVT tests for Virtual Machine Life Cycle - Unmanage - Import +""" +# Import Local Modules +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.utils import * +from marvin.lib.base import (Account, + ServiceOffering, + VirtualMachine, + Host, + Network, + NetworkOffering, + VirtualMachine) +from marvin.lib.common import (get_domain, + get_zone, + get_suitable_test_template) +from marvin.codes import FAILED +from nose.plugins.attrib import attr +from marvin.lib.decoratorGenerators import skipTestIf +import unittest +from marvin.sshClient import SshClient + +_multiprocess_shared_ = True + +class TestUnmanageKvmVM(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + testClient = super(TestUnmanageKvmVM, cls).getClsTestClient() + cls.apiclient = testClient.getApiClient() + cls.services = testClient.getParsedTestDataConfig() + cls.hypervisor = testClient.getHypervisorInfo() + cls._cleanup = [] + + # Get Zone, Domain and templates + cls.domain = get_domain(cls.apiclient) + cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests()) + cls.services['mode'] = cls.zone.networktype + cls.template = get_suitable_test_template( + cls.apiclient, + cls.zone.id, + cls.services["ostype"], + cls.hypervisor + ) + if cls.template == FAILED: + assert False, "get_suitable_test_template() failed to return template with description %s" % cls.services["ostype"] + + cls.hypervisorNotSupported = cls.hypervisor.lower() != "kvm" + if cls.hypervisorNotSupported: + return + + cls.services["small"]["zoneid"] = cls.zone.id + cls.services["small"]["template"] = cls.template.id + + cls.account = Account.create( + cls.apiclient, + cls.services["account"], + domainid=cls.domain.id + ) + cls._cleanup.append(cls.account) + + cls.small_offering = ServiceOffering.create( + cls.apiclient, + cls.services["service_offerings"]["small"] + ) + cls._cleanup.append(cls.small_offering) + + cls.network_offering = NetworkOffering.create( + cls.apiclient, + cls.services["l2-network_offering"], + ) + cls._cleanup.append(cls.network_offering) + cls.network_offering.update(cls.apiclient, state='Enabled') + cls.isolated_network_offering = NetworkOffering.create( + cls.apiclient, + cls.services["nw_off_isolated_persistent"], + ) + cls._cleanup.append(cls.isolated_network_offering) + cls.isolated_network_offering.update(cls.apiclient, state='Enabled') + + @classmethod + def tearDownClass(cls): + super(TestUnmanageKvmVM, cls).tearDownClass() + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.dbclient = self.testClient.getDbConnection() + self.cleanup = [] + self.created_networks = [] + self.virtual_machine = None + self.unmanaged_instance = None + self.imported_vm = None + self.hostConfig = self.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][0].__dict__ + + if self.hypervisorNotSupported: + return + self.services["network"]["networkoffering"] = self.network_offering.id + network_data = self.services["l2-network"] + self.network = Network.create( + self.apiclient, + network_data, + zoneid=self.zone.id, + networkofferingid=self.network_offering.id + ) + self.cleanup.append(self.network) + self.created_networks.append(self.network) + network_data['name'] = "Test L2 Network1" + network_data['displaytext'] = "Test L2 Network1" + self.network1 = Network.create( + self.apiclient, + network_data, + zoneid=self.zone.id, + networkofferingid=self.network_offering.id + ) + self.cleanup.append(self.network1) + self.created_networks.append(self.network1) + self.network2 = Network.create( + self.apiclient, + self.services["isolated_network"], + zoneid=self.zone.id, + networkofferingid=self.isolated_network_offering.id + ) + self.cleanup.append(self.network2) + self.created_networks.append(self.network2) + + + def tearDown(self): + super(TestUnmanageKvmVM, self).tearDown() + + def check_vm_state(self, vm_id): + list_vm = VirtualMachine.list( + self.apiclient, + id=vm_id + ) + self.assertEqual( + isinstance(list_vm, list), + True, + "Check if virtual machine is present" + ) + vm_response = list_vm[0] + self.assertEqual( + vm_response.state, + "Running", + "VM state should be running after deployment" + ) + return vm_response + + + @attr(tags=["advanced", "advancedns", "smoke", "sg"], required_hardware="true") + @skipTestIf("hypervisorNotSupported") + def test_01_unmanage_vm_cycle_persistent_domain(self): + """ + Test the following: + 1. Deploy VM + 2. Unmanage VM + 3. Verify VM is not listed in CloudStack + 4. Verify VM is listed as part of the unmanaged instances + 5. Verify VM domain is persistent for KVM hypervisor + 6. Stop VM using virsh, confirm VM is stopped + 7. Start VM using virsh, confirm VM is running + 8. Import VM + 9. Verify details of imported VM + 10. Destroy VM + """ + + # 1 - Deploy VM + self.virtual_machine = VirtualMachine.create( + self.apiclient, + self.services["virtual_machine"], + templateid=self.template.id, + serviceofferingid=self.small_offering.id, + networkids=[self.network.id, self.network1.id, self.network2.id], + zoneid=self.zone.id + ) + + vm_id = self.virtual_machine.id + vm_instance_name = self.virtual_machine.instancename + + networks = [] + for network in self.created_networks: + n = Network.list( + self.apiclient, + id=network.id + )[0] + networks.append(n) + hostid = self.virtual_machine.hostid + hosts = Host.list( + self.apiclient, + id=hostid + ) + host = hosts[0] + clusterid = host.clusterid + self.check_vm_state(vm_id) + + # 2 - Unmanage VM from CloudStack + self.virtual_machine.unmanage(self.apiclient) + + # 3 - Verify VM is not listed in CloudStack + list_vm = VirtualMachine.list( + self.apiclient, + id=vm_id + ) + self.assertEqual( + list_vm, + None, + "VM should not be listed" + ) + # 4 - Verify VM is listed as part of the unmanaged instances + unmanaged_vms = VirtualMachine.listUnmanagedInstances( + self.apiclient, + clusterid=clusterid, + name=vm_instance_name + ) + + self.assertEqual( + len(unmanaged_vms), + 1, + "Unmanaged VMs matching instance name list size is 1" + ) + unmanaged_vm = unmanaged_vms[0] + self.assertEqual( + unmanaged_vm.powerstate, + "PowerOn", + "Unmanaged VM is still running" + ) + + # 5 - Verify VM domain is persistent for KVM + ssh_host = self.get_ssh_client(host.ipaddress, + self.hostConfig["username"], + self.hostConfig["password"], 10) + + cmd = f"virsh dominfo {vm_instance_name}" + result = ssh_host.execute(cmd) + if result == None or result == "": + raise Exception(f"Failed to fetch domain info for VM: {vm_instance_name} on host: {host.name}. Error: {result}") + persistent_line = next((line for line in result if "Persistent:" in line), None) + if not persistent_line: + raise Exception(f"'Persistent' info not found in dominfo output for VM: {vm_instance_name} on host: {host.name}") + if "yes" not in persistent_line.lower(): + raise Exception(f"VM: {vm_instance_name} is NOT persistent on host: {host.name}") + + + # 6 - Stop VM using virsh, confirm VM is stopped + host = Host.list( + self.apiclient, + id=unmanaged_vm.hostid + )[0] + cmd = "virsh destroy %s" % vm_instance_name + result = ssh_host.execute(cmd) + if result == None or result == "": + raise Exception("Failed to stop VM: %s on host: %s" % (vm_instance_name, host.name)) + + cmd = "virsh list --all | grep %s" % vm_instance_name + result = ssh_host.execute(cmd) + if result == None or result == "": + raise Exception("Failed to fetch VM: %s state on host: %s" % (vm_instance_name, host.name)) + state_line = next((line for line in result if vm_instance_name in line), None) + if not state_line: + raise Exception(f"VM: {vm_instance_name} not found in 'virsh list --all' output on host: {host.name}") + if 'shut off' not in state_line.lower(): + raise Exception(f"VM: {vm_instance_name} is NOT stopped on host: {host.name}, state: {state_line}") + + # 7 - Start VM using virsh, confirm VM is running + cmd = "virsh start %s" % vm_instance_name + result = ssh_host.execute(cmd) + if result == None or result == "": + raise Exception("Failed to start VM: %s on host: %s" % (vm_instance_name, host.name)) + time.sleep(30) + cmd = "virsh list | grep %s" % vm_instance_name + result = ssh_host.execute(cmd) + if result == None or result == "": + raise Exception("Failed to fetch VM: %s state on host: %s" % (vm_instance_name, host.name)) + if 'running' not in str(result).lower(): + raise Exception(f"VM: {vm_instance_name} is NOT running on host: {host.name}, state: {state_line}") + + # 8 - Import VM + + nicnetworklist = [] + nicipaddresslist = [] + for nic in unmanaged_vm.nic: + for network in networks: + if int(network.vlan) == int(nic.vlanid): + nicnetworklist.append({ + "nic": nic.id, + "network": network.id + }) + if network.type == "Isolated": + nicipaddresslist.append({ + "nic": nic.id, + "ip4Address": "auto" + }) + import_vm_service = { + "nicnetworklist": nicnetworklist, + "nicipaddresslist": nicipaddresslist + } + + self.imported_vm = VirtualMachine.importUnmanagedInstance( + self.apiclient, + clusterid=clusterid, + name=vm_instance_name, + serviceofferingid=self.small_offering.id, + services=import_vm_service, + templateid=self.template.id) + self.cleanup.append(self.imported_vm) + self.unmanaged_instance = None + # 9 - Verify details of the imported VM + self.assertEqual( + self.small_offering.id, + self.imported_vm.serviceofferingid, + "Imported VM service offering is different, expected: %s, actual: %s" % (self.small_offering.id, self.imported_vm.serviceofferingid) + ) + self.assertEqual( + self.template.id, + self.imported_vm.templateid, + "Imported VM template is different, expected: %s, actual: %s" % (self.template.id, self.imported_vm.templateid) + ) + self.check_vm_state(self.imported_vm.id) + # 10 - Destroy VM. This will be done during cleanup + + + + @attr(tags=["advanced", "advancedns", "smoke", "sg"], required_hardware="true") + @skipTestIf("hypervisorNotSupported") + def test_02_unmanage_stopped_vm_cycle_persistent_domain(self): + """ + Test the following: + 1. Deploy VM + 2. Stop VM + 3. Unmanage VM + 4. Verify VM is not listed in CloudStack + 5. Verify VM is listed as part of the unmanaged instances + 6. Start VM using virsh, confirm VM is running + 7. Import VM + 8. Verify details of imported VM + 9. Destroy VM + """ + # 1 - Deploy VM + self.virtual_machine = VirtualMachine.create( + self.apiclient, + self.services["virtual_machine"], + templateid=self.template.id, + serviceofferingid=self.small_offering.id, + networkids=[self.network.id, self.network1.id, self.network2.id], + zoneid=self.zone.id + ) + + vm_id = self.virtual_machine.id + vm_instance_name = self.virtual_machine.instancename + + networks = [] + for network in self.created_networks: + n = Network.list( + self.apiclient, + id=network.id + )[0] + networks.append(n) + hostid = self.virtual_machine.hostid + hosts = Host.list( + self.apiclient, + id=hostid + ) + host = hosts[0] + clusterid = host.clusterid + self.check_vm_state(vm_id) + + #2 - Stop VM + self.virtual_machine.stop(self.apiclient) + list_vm = VirtualMachine.list( + self.apiclient, + id=vm_id + ) + self.assertEqual( + isinstance(list_vm, list), + True, + "Check if virtual machine is present" + ) + vm_response = list_vm[0] + self.assertEqual( + vm_response.state, + "Stopped", + "VM state should be Stopped after stopping the VM" + ) + + # 3 - Unmanage VM from CloudStack + self.virtual_machine.unmanage(self.apiclient) + + # 4 - Verify VM is not listed in CloudStack + list_vm = VirtualMachine.list( + self.apiclient, + id=vm_id + ) + self.assertEqual( + list_vm, + None, + "VM should not be listed" + ) + + # 5 - Verify VM is listed as part of the unmanaged instances + ssh_host = self.get_ssh_client(host.ipaddress, + self.hostConfig["username"], + self.hostConfig["password"], 10) + cmd = "virsh list --all | grep %s" % vm_instance_name + result = ssh_host.execute(cmd) + if result == None or result == "": + raise Exception("Failed to fetch VM: %s state on host: %s" % (vm_instance_name, host.name)) + if 'shut off' not in str(result).lower(): + raise Exception(f"VM: {vm_instance_name} is NOT in stopped on host: {host.name}") + + # 6 - Start VM using virsh, confirm VM is running + cmd = "virsh start %s" % vm_instance_name + result = ssh_host.execute(cmd) + if result == None or result == "": + raise Exception("Failed to start VM: %s on host: %s" % (vm_instance_name, host.name)) + time.sleep(30) + cmd = "virsh list | grep %s" % vm_instance_name + result = ssh_host.execute(cmd) + if result == None or result == "": + raise Exception("Failed to fetch VM: %s state on host: %s" % (vm_instance_name, host.name)) + if 'running' not in str(result).lower(): + raise Exception(f"VM: {vm_instance_name} is NOT running on host: {host.name}") + + # 8 - Import VM + self.imported_vm = VirtualMachine.importUnmanagedInstance( + self.apiclient, + clusterid=clusterid, + name=vm_instance_name, + serviceofferingid=self.small_offering.id, + services={}, + templateid=self.template.id) + self.cleanup.append(self.imported_vm) + self.unmanaged_instance = None + + # 9 - Verify details of the imported VM + self.assertEqual( + self.small_offering.id, + self.imported_vm.serviceofferingid, + "Imported VM service offering is different, expected: %s, actual: %s" % (self.small_offering.id, self.imported_vm.serviceofferingid) + ) + self.assertEqual( + self.template.id, + self.imported_vm.templateid, + "Imported VM template is different, expected: %s, actual: %s" % (self.template.id, self.imported_vm.templateid) + ) + self.check_vm_state(self.imported_vm.id) + # 10 - Destroy VM. This will be done during cleanup + + + def get_ssh_client(self, ip, username, password, retries=10): + """ Setup ssh client connection and return connection """ + try: + ssh_client = SshClient(ip, 22, username, password, retries) + except Exception as e: + raise unittest.SkipTest("Unable to create ssh connection: " % e) + + self.assertIsNotNone( + ssh_client, "Failed to setup ssh connection to ip=%s" % ip) + + return ssh_client diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index 16b2467b63df..c3d0d37b3510 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -401,7 +401,8 @@ def __init__(self, items, services): self.ssh_port = 22 self.ssh_client = None # extract out the ipaddress - self.ipaddress = self.nic[0].ipaddress + if self.nic: + self.ipaddress = self.nic[0].ipaddress @classmethod def ssh_access_group(cls, apiclient, cmd): @@ -1083,7 +1084,7 @@ def scale(self, apiclient, serviceOfferingId, return apiclient.scaleVirtualMachine(cmd) def unmanage(self, apiclient): - """Unmanage a VM from CloudStack (currently VMware only)""" + """Unmanage a VM from CloudStack""" cmd = unmanageVirtualMachine.unmanageVirtualMachineCmd() cmd.id = self.id return apiclient.unmanageVirtualMachine(cmd) From 2c184968eb4ad1e5084c304193ba927fe0e74c52 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Thu, 11 Sep 2025 10:21:42 +0530 Subject: [PATCH 09/17] trim whitespace --- .../test_vm_lifecycle_unmanage_kvm_import.py | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/test/integration/smoke/test_vm_lifecycle_unmanage_kvm_import.py b/test/integration/smoke/test_vm_lifecycle_unmanage_kvm_import.py index f39b15078426..79c7513f2e2b 100644 --- a/test/integration/smoke/test_vm_lifecycle_unmanage_kvm_import.py +++ b/test/integration/smoke/test_vm_lifecycle_unmanage_kvm_import.py @@ -177,7 +177,7 @@ def test_01_unmanage_vm_cycle_persistent_domain(self): 9. Verify details of imported VM 10. Destroy VM """ - + # 1 - Deploy VM self.virtual_machine = VirtualMachine.create( self.apiclient, @@ -187,7 +187,7 @@ def test_01_unmanage_vm_cycle_persistent_domain(self): networkids=[self.network.id, self.network1.id, self.network2.id], zoneid=self.zone.id ) - + vm_id = self.virtual_machine.id vm_instance_name = self.virtual_machine.instancename @@ -240,10 +240,10 @@ def test_01_unmanage_vm_cycle_persistent_domain(self): ) # 5 - Verify VM domain is persistent for KVM - ssh_host = self.get_ssh_client(host.ipaddress, - self.hostConfig["username"], + ssh_host = self.get_ssh_client(host.ipaddress, + self.hostConfig["username"], self.hostConfig["password"], 10) - + cmd = f"virsh dominfo {vm_instance_name}" result = ssh_host.execute(cmd) if result == None or result == "": @@ -264,7 +264,7 @@ def test_01_unmanage_vm_cycle_persistent_domain(self): result = ssh_host.execute(cmd) if result == None or result == "": raise Exception("Failed to stop VM: %s on host: %s" % (vm_instance_name, host.name)) - + cmd = "virsh list --all | grep %s" % vm_instance_name result = ssh_host.execute(cmd) if result == None or result == "": @@ -286,8 +286,8 @@ def test_01_unmanage_vm_cycle_persistent_domain(self): if result == None or result == "": raise Exception("Failed to fetch VM: %s state on host: %s" % (vm_instance_name, host.name)) if 'running' not in str(result).lower(): - raise Exception(f"VM: {vm_instance_name} is NOT running on host: {host.name}, state: {state_line}") - + raise Exception(f"VM: {vm_instance_name} is NOT running on host: {host.name}, state: {state_line}") + # 8 - Import VM nicnetworklist = [] @@ -333,7 +333,7 @@ def test_01_unmanage_vm_cycle_persistent_domain(self): # 10 - Destroy VM. This will be done during cleanup - + @attr(tags=["advanced", "advancedns", "smoke", "sg"], required_hardware="true") @skipTestIf("hypervisorNotSupported") def test_02_unmanage_stopped_vm_cycle_persistent_domain(self): @@ -358,7 +358,7 @@ def test_02_unmanage_stopped_vm_cycle_persistent_domain(self): networkids=[self.network.id, self.network1.id, self.network2.id], zoneid=self.zone.id ) - + vm_id = self.virtual_machine.id vm_instance_name = self.virtual_machine.instancename @@ -394,7 +394,7 @@ def test_02_unmanage_stopped_vm_cycle_persistent_domain(self): vm_response.state, "Stopped", "VM state should be Stopped after stopping the VM" - ) + ) # 3 - Unmanage VM from CloudStack self.virtual_machine.unmanage(self.apiclient) @@ -411,16 +411,16 @@ def test_02_unmanage_stopped_vm_cycle_persistent_domain(self): ) # 5 - Verify VM is listed as part of the unmanaged instances - ssh_host = self.get_ssh_client(host.ipaddress, - self.hostConfig["username"], + ssh_host = self.get_ssh_client(host.ipaddress, + self.hostConfig["username"], self.hostConfig["password"], 10) cmd = "virsh list --all | grep %s" % vm_instance_name result = ssh_host.execute(cmd) if result == None or result == "": raise Exception("Failed to fetch VM: %s state on host: %s" % (vm_instance_name, host.name)) if 'shut off' not in str(result).lower(): - raise Exception(f"VM: {vm_instance_name} is NOT in stopped on host: {host.name}") - + raise Exception(f"VM: {vm_instance_name} is NOT in stopped on host: {host.name}") + # 6 - Start VM using virsh, confirm VM is running cmd = "virsh start %s" % vm_instance_name result = ssh_host.execute(cmd) @@ -432,8 +432,8 @@ def test_02_unmanage_stopped_vm_cycle_persistent_domain(self): if result == None or result == "": raise Exception("Failed to fetch VM: %s state on host: %s" % (vm_instance_name, host.name)) if 'running' not in str(result).lower(): - raise Exception(f"VM: {vm_instance_name} is NOT running on host: {host.name}") - + raise Exception(f"VM: {vm_instance_name} is NOT running on host: {host.name}") + # 8 - Import VM self.imported_vm = VirtualMachine.importUnmanagedInstance( self.apiclient, @@ -444,7 +444,7 @@ def test_02_unmanage_stopped_vm_cycle_persistent_domain(self): templateid=self.template.id) self.cleanup.append(self.imported_vm) self.unmanaged_instance = None - + # 9 - Verify details of the imported VM self.assertEqual( self.small_offering.id, From 7df8733c594a0ee067bc6c5b603e404a9a0f7075 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Mon, 15 Sep 2025 10:43:42 +0530 Subject: [PATCH 10/17] use any available host if last host is not there --- .../cloud/vm/VirtualMachineManagerImpl.java | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) 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 f14e850537a5..b669624bd7ff 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2049,17 +2049,32 @@ public Boolean doInTransaction(TransactionStatus status) { void persistDomainForKVM(VMInstanceVO vm) { Long hostId = vm.getHostId(); + String vmName = vm.getName(); UnmanageInstanceCommand unmanageInstanceCommand; if (State.Stopped.equals(vm.getState())) { - hostId = vm.getLastHostId(); + Pair clusterAndHostId = findClusterAndHostIdForVm(vm.getLastHostId()); + Long clusterId = clusterAndHostId.first(); + hostId = clusterAndHostId.second(); + if (hostId == null) { + logger.debug("No previous host found for Instance: {}. " + + "Searching for any available hosts in cluster with ID: {}.", vmName, clusterId); + List availableHosts = _resourceMgr.listAllUpAndEnabledHosts(Host.Type.Routing, clusterId, + null, vm.getDataCenterId()); + if (availableHosts.isEmpty()) { + String errorMsg = "No available host to persist domainXML for Instance: " + vmName; + logger.debug(errorMsg); + throw new CloudRuntimeException(errorMsg); + } + hostId = availableHosts.get(0).getId(); + } unmanageInstanceCommand = new UnmanageInstanceCommand(prepVmSpecForUnmanageCmd(vm.getId(), hostId)); // reconstruct vmSpec for stopped instance } else { - unmanageInstanceCommand = new UnmanageInstanceCommand(vm.getName()); + unmanageInstanceCommand = new UnmanageInstanceCommand(vmName); } try { Answer answer = _agentMgr.send(hostId, unmanageInstanceCommand); if (!answer.getResult()) { - String errorMsg = "Failed to persist domainXML for instance: " + vm.getName(); + String errorMsg = "Failed to persist domainXML for instance: " + vmName; logger.debug(errorMsg); throw new CloudRuntimeException(errorMsg); } From f118a9fee609b89f907a3b28558d30d0d4feffcc Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Mon, 15 Sep 2025 15:36:08 +0530 Subject: [PATCH 11/17] look for kvm hosts only, fix unit test --- .../cloud/vm/VirtualMachineManagerImpl.java | 7 ++-- .../vm/VirtualMachineManagerImplTest.java | 36 +++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) 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 b669624bd7ff..412fa378e9c3 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2053,13 +2053,11 @@ void persistDomainForKVM(VMInstanceVO vm) { UnmanageInstanceCommand unmanageInstanceCommand; if (State.Stopped.equals(vm.getState())) { Pair clusterAndHostId = findClusterAndHostIdForVm(vm.getLastHostId()); - Long clusterId = clusterAndHostId.first(); hostId = clusterAndHostId.second(); if (hostId == null) { logger.debug("No previous host found for Instance: {}. " + - "Searching for any available hosts in cluster with ID: {}.", vmName, clusterId); - List availableHosts = _resourceMgr.listAllUpAndEnabledHosts(Host.Type.Routing, clusterId, - null, vm.getDataCenterId()); + "Searching for any available hosts in Zone with ID: {}.", vmName, vm.getDataCenterId()); + List availableHosts = _hostDao.listByDataCenterIdAndHypervisorType(vm.getDataCenterId(), HypervisorType.KVM); if (availableHosts.isEmpty()) { String errorMsg = "No available host to persist domainXML for Instance: " + vmName; logger.debug(errorMsg); @@ -2071,6 +2069,7 @@ void persistDomainForKVM(VMInstanceVO vm) { } else { unmanageInstanceCommand = new UnmanageInstanceCommand(vmName); } + logger.debug("Selected host with ID: {} to persist domain XML for Instance: {}.", hostId, vmName); try { Answer answer = _agentMgr.send(hostId, unmanageInstanceCommand); if (!answer.getResult()) { 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 153311b15cd8..6f1330f8b108 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -1771,6 +1771,7 @@ public void testPersistDomainForKvmForStoppedVmSuccess() throws AgentUnavailable 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(vmInstanceVoMockId)).thenReturn(new Pair<>(1L, 1L)); virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock); ArgumentCaptor hostIdCaptor = ArgumentCaptor.forClass(Long.class); ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(UnmanageInstanceCommand.class); @@ -1783,6 +1784,41 @@ public void testPersistDomainForKvmForStoppedVmSuccess() throws AgentUnavailable verify(virtualMachineManagerImpl).prepVmSpecForUnmanageCmd(vmInstanceVoMockId, 1L); } + + @Test + public void testPersistDomainForKvmForStoppedVmHostRemoved() throws AgentUnavailableException, OperationTimedoutException { + when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Stopped); + when(vmInstanceMock.getLastHostId()).thenReturn(1L); + when(vmInstanceMock.getDataCenterId()).thenReturn(zoneMockId); + 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(vmInstanceVoMockId)).thenReturn(new Pair<>(clusterMockId, null)); + when(hostDaoMock.listByDataCenterIdAndHypervisorType(zoneMockId, HypervisorType.KVM)).thenReturn(List.of(hostMock)); + virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock); + 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() throws AgentUnavailableException, OperationTimedoutException { + when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Stopped); + when(vmInstanceMock.getLastHostId()).thenReturn(1L); + VirtualMachineTO vmTO = new VirtualMachineTO() {}; + vmTO.setName(vmName); + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock)); + assertEquals("No available host to persist domainXML for Instance: " + vmName, exception.getMessage()); + } + @Test public void testPersistDomainForKvmForRunningVmAgentFailure() throws AgentUnavailableException, OperationTimedoutException { when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Running); From 0a693a24a4425e0b879c78478523fc47ffa92017 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Mon, 15 Sep 2025 18:38:04 +0530 Subject: [PATCH 12/17] lookup in cluster for host --- .../cloud/vm/VirtualMachineManagerImpl.java | 19 ++++------ .../vm/VirtualMachineManagerImplTest.java | 35 +++---------------- 2 files changed, 12 insertions(+), 42 deletions(-) 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 412fa378e9c3..c8339d930134 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2052,18 +2052,12 @@ void persistDomainForKVM(VMInstanceVO vm) { String vmName = vm.getName(); UnmanageInstanceCommand unmanageInstanceCommand; if (State.Stopped.equals(vm.getState())) { - Pair clusterAndHostId = findClusterAndHostIdForVm(vm.getLastHostId()); + Pair clusterAndHostId = findClusterAndHostIdForVm(vm, false); hostId = clusterAndHostId.second(); if (hostId == null) { - logger.debug("No previous host found for Instance: {}. " + - "Searching for any available hosts in Zone with ID: {}.", vmName, vm.getDataCenterId()); - List availableHosts = _hostDao.listByDataCenterIdAndHypervisorType(vm.getDataCenterId(), HypervisorType.KVM); - if (availableHosts.isEmpty()) { - String errorMsg = "No available host to persist domainXML for Instance: " + vmName; - logger.debug(errorMsg); - throw new CloudRuntimeException(errorMsg); - } - hostId = availableHosts.get(0).getId(); + String errorMsg = "No available host to persist domain XML for Instance: " + vmName; + logger.debug(errorMsg); + throw new CloudRuntimeException(errorMsg); } unmanageInstanceCommand = new UnmanageInstanceCommand(prepVmSpecForUnmanageCmd(vm.getId(), hostId)); // reconstruct vmSpec for stopped instance } else { @@ -2073,7 +2067,7 @@ void persistDomainForKVM(VMInstanceVO vm) { try { Answer answer = _agentMgr.send(hostId, unmanageInstanceCommand); if (!answer.getResult()) { - String errorMsg = "Failed to persist domainXML for instance: " + vmName; + String errorMsg = "Failed to persist domain XML for instance: " + vmName; logger.debug(errorMsg); throw new CloudRuntimeException(errorMsg); } @@ -6273,8 +6267,9 @@ public Pair findClusterAndHostIdForVm(VirtualMachine vm, boolean ski 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 6f1330f8b108..99e04b0235be 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -1765,13 +1765,12 @@ public void testPersistDomainForKvmForRunningVmSuccess() throws AgentUnavailable @Test public void testPersistDomainForKvmForStoppedVmSuccess() throws AgentUnavailableException, OperationTimedoutException { when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Stopped); - when(vmInstanceMock.getLastHostId()).thenReturn(1L); 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(vmInstanceVoMockId)).thenReturn(new Pair<>(1L, 1L)); + when(virtualMachineManagerImpl.findClusterAndHostIdForVm(vmInstanceMock, false)).thenReturn(new Pair<>(clusterMockId, hostMockId)); virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock); ArgumentCaptor hostIdCaptor = ArgumentCaptor.forClass(Long.class); ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(UnmanageInstanceCommand.class); @@ -1786,37 +1785,13 @@ public void testPersistDomainForKvmForStoppedVmSuccess() throws AgentUnavailable @Test - public void testPersistDomainForKvmForStoppedVmHostRemoved() throws AgentUnavailableException, OperationTimedoutException { + public void testPersistDomainForKvmForStoppedVmNoHost() { when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Stopped); - when(vmInstanceMock.getLastHostId()).thenReturn(1L); - when(vmInstanceMock.getDataCenterId()).thenReturn(zoneMockId); - 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(vmInstanceVoMockId)).thenReturn(new Pair<>(clusterMockId, null)); - when(hostDaoMock.listByDataCenterIdAndHypervisorType(zoneMockId, HypervisorType.KVM)).thenReturn(List.of(hostMock)); - virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock); - 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() throws AgentUnavailableException, OperationTimedoutException { - when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Stopped); - when(vmInstanceMock.getLastHostId()).thenReturn(1L); 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)); - assertEquals("No available host to persist domainXML for Instance: " + vmName, exception.getMessage()); + assertEquals("No available host to persist domain XML for Instance: " + vmName, exception.getMessage()); } @Test @@ -1826,7 +1801,7 @@ public void testPersistDomainForKvmForRunningVmAgentFailure() throws AgentUnavai UnmanageInstanceAnswer failureAnswer = new UnmanageInstanceAnswer(null, false, "failure"); when(agentManagerMock.send(anyLong(), any(UnmanageInstanceCommand.class))).thenReturn(failureAnswer); CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock)); - assertEquals("Failed to persist domainXML for instance: " + vmName, exception.getMessage()); + assertEquals("Failed to persist domain XML for instance: " + vmName, exception.getMessage()); } @Test From a16fa51b29a430f3d53df7557d084d91ab29d8ae Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Wed, 17 Sep 2025 15:11:38 +0530 Subject: [PATCH 13/17] add hostid in unmanageVirtualMachine cmd and response --- .../main/java/com/cloud/vm/UserVmService.java | 6 ++- .../admin/vm/UnmanageVMInstanceCmd.java | 24 +++++++-- .../response/UnmanageVMInstanceResponse.java | 12 +++++ .../cloudstack/vm/UnmanageVMService.java | 7 ++- .../com/cloud/vm/VirtualMachineManager.java | 2 +- .../cloud/vm/VirtualMachineManagerImpl.java | 51 +++++++++++-------- .../vm/VirtualMachineManagerImplTest.java | 18 +++---- .../java/com/cloud/vm/UserVmManagerImpl.java | 16 +++--- .../vm/UnmanagedVMsManagerImpl.java | 13 +++-- .../vm/UnmanagedVMsManagerImplTest.java | 26 +++++++--- 10 files changed, 115 insertions(+), 60 deletions(-) diff --git a/api/src/main/java/com/cloud/vm/UserVmService.java b/api/src/main/java/com/cloud/vm/UserVmService.java index 6f1aba4613da..aa9e239d8061 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.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 { @@ -514,9 +515,10 @@ UserVm importVM(final DataCenter zone, final Host host, final VirtualMachineTemp /** * 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 bbcb8840f666..23919360e5eb 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.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,6 +37,7 @@ 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; @@ -65,6 +67,12 @@ 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 where domain XML is stored for stopped Instance", + since = "4.22.0") + private Long hostId; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -83,6 +91,14 @@ public String getEventDescription() { return "unmanaging VM. VM ID = " + vmId; } + public Long getHostId() { + return hostId; + } + + public void setHostId(Long hostId) { + this.hostId = hostId; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @@ -93,9 +109,10 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE 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); + if (result.first()) { + response.setSuccess(true); + response.setHostId(result.second()); response.setDetails("VM unmanaged successfully"); } } catch (Exception e) { @@ -124,5 +141,4 @@ public ApiCommandResourceType getApiResourceType() { 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 e9d45cb506ae..bd95ecee5ae3 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 String getDetails() { 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 2315e5f2e95f..44c4af655a97 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); } 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 c05c29add556..cffba3d9a132 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 @@ static String getHypervisorHostname(String name) { * - 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 c8339d930134..107da61bfe88 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2004,7 +2004,7 @@ public boolean getExecuteInSequence(final HypervisorType hypervisorType) { } @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); @@ -2017,8 +2017,9 @@ public boolean unmanage(String vmUuid) { throw new ConcurrentOperationException(msg); } + Long agentHostId = vm.getHostId(); if (HypervisorType.KVM.equals(vm.getHypervisorType())) { - persistDomainForKVM(vm); + agentHostId = persistDomainForKVM(vm, paramHostId); } Boolean result = Transaction.execute(new TransactionCallback() { @Override @@ -2043,43 +2044,49 @@ public Boolean doInTransaction(TransactionStatus status) { return true; } }); - - return BooleanUtils.isTrue(result); + HostVO host = ApiDBUtils.findHostById(agentHostId); + if (host == null) { + throw new CloudRuntimeException("Unable to retrieve host with ID: " + agentHostId); + } + logger.debug("Selected host UUID: {} to unmanage Instance: {}.", host.getUuid(), vm.getName()); + return new Pair<>(result, host.getUuid()); } - void persistDomainForKVM(VMInstanceVO vm) { - Long hostId = vm.getHostId(); + Long persistDomainForKVM(VMInstanceVO vm, Long paramHostId) { + Long agentHostId = vm.getHostId(); String vmName = vm.getName(); UnmanageInstanceCommand unmanageInstanceCommand; if (State.Stopped.equals(vm.getState())) { - Pair clusterAndHostId = findClusterAndHostIdForVm(vm, false); - hostId = clusterAndHostId.second(); - if (hostId == null) { - String errorMsg = "No available host to persist domain XML for Instance: " + vmName; - logger.debug(errorMsg); - throw new CloudRuntimeException(errorMsg); + 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(), hostId)); // reconstruct vmSpec for stopped instance + unmanageInstanceCommand = new UnmanageInstanceCommand(prepVmSpecForUnmanageCmd(vm.getId(), agentHostId)); // reconstruct vmSpec for stopped instance } else { unmanageInstanceCommand = new UnmanageInstanceCommand(vmName); } - logger.debug("Selected host with ID: {} to persist domain XML for Instance: {}.", hostId, vmName); + + logger.debug("Selected host ID: {} to persist domain XML for Instance: {}.", agentHostId, vmName); try { - Answer answer = _agentMgr.send(hostId, unmanageInstanceCommand); + Answer answer = _agentMgr.send(agentHostId, unmanageInstanceCommand); if (!answer.getResult()) { - String errorMsg = "Failed to persist domain XML for instance: " + vmName; + String errorMsg = "Failed to persist domain XML for Instance: " + vmName + " on host ID: " + agentHostId; logger.debug(errorMsg); throw new CloudRuntimeException(errorMsg); } - } catch (AgentUnavailableException e) { - String errorMsg = "Failed to send command, agent unavailable"; - logger.error(errorMsg, e); - throw new CloudRuntimeException(errorMsg); - } catch (OperationTimedoutException e) { - String errorMsg = "Failed to send command, operation timed out"; + } 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; } /** 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 99e04b0235be..4e655b81a644 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -1755,7 +1755,7 @@ public void testPersistDomainForKvmForRunningVmSuccess() throws AgentUnavailable when(vmInstanceMock.getHostId()).thenReturn(hostMockId); UnmanageInstanceAnswer successAnswer = new UnmanageInstanceAnswer(null, true, "success"); when(agentManagerMock.send(anyLong(), any(Command.class))).thenReturn(successAnswer); - virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock); + virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock, null); ArgumentCaptor hostIdCaptor = ArgumentCaptor.forClass(Long.class); ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(UnmanageInstanceCommand.class); verify(agentManagerMock).send(hostIdCaptor.capture(), commandCaptor.capture()); @@ -1771,7 +1771,7 @@ public void testPersistDomainForKvmForStoppedVmSuccess() throws AgentUnavailable 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); + virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock, null); ArgumentCaptor hostIdCaptor = ArgumentCaptor.forClass(Long.class); ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(UnmanageInstanceCommand.class); verify(agentManagerMock).send(hostIdCaptor.capture(), commandCaptor.capture()); @@ -1790,7 +1790,7 @@ public void testPersistDomainForKvmForStoppedVmNoHost() { 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)); + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> virtualMachineManagerImpl.persistDomainForKVM(vmInstanceMock, null)); assertEquals("No available host to persist domain XML for Instance: " + vmName, exception.getMessage()); } @@ -1800,8 +1800,8 @@ public void testPersistDomainForKvmForRunningVmAgentFailure() throws AgentUnavai 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)); - assertEquals("Failed to persist domain XML for instance: " + vmName, exception.getMessage()); + 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 @@ -1809,8 +1809,8 @@ public void testPersistDomainForKvmAgentUnavailable() throws AgentUnavailableExc 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)); - assertEquals("Failed to send command, agent unavailable", exception.getMessage()); + 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 @@ -1818,7 +1818,7 @@ public void testPersistDomainForKvmOperationTimedOut() throws AgentUnavailableEx 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)); - assertEquals("Failed to send command, operation timed out", exception.getMessage()); + 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()); } } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index cd7d198eb611..22d659d461a8 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -9399,18 +9399,19 @@ public UserVm importVM(final DataCenter zone, final Host host, final VirtualMach } @Override - public boolean unmanageUserVM(Long vmId) { + public Pair unmanageUserVM(Long vmId, Long paramHostId) { UserVmVO vm = _vmDao.findById(vmId); if (vm == null || vm.getRemoved() != null) { throw new InvalidParameterValueException("Unable to find a VM with ID = " + vmId); } vm = _vmDao.acquireInLockTable(vm.getId()); - boolean result; + try { if (vm.getState() != State.Running && vm.getState() != State.Stopped) { - logger.debug("VM {} is not running or stopped, cannot be unmanaged", vm); - return false; + String errorMsg = "Instance: " + vm.getName() + " is not running or stopped, cannot be unmanaged"; + logger.debug(errorMsg); + throw new CloudRuntimeException(errorMsg); } if (!UnmanagedVMsManager.isSupported(vm.getHypervisorType())) { @@ -9422,22 +9423,21 @@ public boolean unmanageUserVM(Long vmId) { checkUnmanagingVMOngoingVolumeSnapshots(vm); checkUnmanagingVMVolumes(vm, volumes); - result = _itMgr.unmanage(vm.getUuid()); - if (result) { + Pair result = _itMgr.unmanage(vm.getUuid(), paramHostId); + if (result.first()) { cleanupUnmanageVMResources(vm); unmanageVMFromDB(vm.getId()); publishUnmanageVMUsageEvents(vm, volumes); } else { throw new CloudRuntimeException("Error while unmanaging VM: " + vm.getUuid()); } + return result; } catch (Exception e) { logger.error("Could not unmanage VM {}", vm, e); throw new CloudRuntimeException(e); } finally { _vmDao.releaseFromLockTable(vm.getId()); } - - return true; } private void updateDetailsWithRootDiskAttributes(Map details, VmDiskInfo rootVmDiskInfo) { diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 7e4514909132..d13e05bb4e1c 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -2265,7 +2265,7 @@ private Long findSuitableHostId(VMInstanceVO vmVO) { @Override @ActionEvent(eventType = EventTypes.EVENT_VM_UNMANAGE, eventDescription = "unmanaging VM", async = true) - public boolean unmanageVMInstance(long vmId) { + public Pair unmanageVMInstance(long vmId, Long paramHostId) { VMInstanceVO vmVO = vmDao.findById(vmId); if (vmVO == null || vmVO.getRemoved() != null) { throw new InvalidParameterValueException("Could not find VM to unmanage, it is either removed or not existing VM"); @@ -2276,6 +2276,9 @@ public boolean unmanageVMInstance(long vmId) { vmVO.getHypervisorType().toString()); } else if (vmVO.getType() != VirtualMachine.Type.User) { throw new UnsupportedServiceException("Unmanage VM is currently allowed for guest VMs only"); + } else if (paramHostId != null && + (vmVO.getHypervisorType() != Hypervisor.HypervisorType.KVM || vmVO.getState() != VirtualMachine.State.Stopped)) { + throw new UnsupportedServiceException("Param hostid is only supported for KVM hypervisor for stopped Instances."); } if (vmVO.getType().equals(VirtualMachine.Type.User)) { @@ -2287,15 +2290,15 @@ public boolean unmanageVMInstance(long vmId) { performUnmanageVMInstancePrechecks(vmVO); - boolean isVMStopped = VirtualMachine.State.Stopped.equals(vmVO.getState()); - Long hostId = isVMStopped ? vmVO.getLastHostId() : findSuitableHostId(vmVO); + boolean isKvmVmStopped = VirtualMachine.State.Stopped.equals(vmVO.getState()) && vmVO.getHypervisorType() == Hypervisor.HypervisorType.KVM; + Long hostId = isKvmVmStopped ? vmVO.getLastHostId() : findSuitableHostId(vmVO); String instanceName = vmVO.getInstanceName(); - if (!isVMStopped && !existsVMToUnmanage(instanceName, hostId)) { + if (!isKvmVmStopped && !existsVMToUnmanage(instanceName, hostId)) { throw new CloudRuntimeException(String.format("VM %s is not found in the hypervisor", vmVO)); } - return userVmManager.unmanageUserVM(vmId); + return userVmManager.unmanageUserVM(vmId, paramHostId); } /** diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index db623eba538e..8921a467694a 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -448,37 +448,49 @@ public void importUnmanagedInstanceMissingInstanceTest() { @Test(expected = InvalidParameterValueException.class) public void unmanageVMInstanceMissingInstanceTest() { long notExistingId = 10L; - unmanagedVMsManager.unmanageVMInstance(notExistingId); + unmanagedVMsManager.unmanageVMInstance(notExistingId, null); } @Test(expected = InvalidParameterValueException.class) public void unmanageVMInstanceDestroyedInstanceTest() { when(virtualMachine.getState()).thenReturn(VirtualMachine.State.Destroyed); - unmanagedVMsManager.unmanageVMInstance(virtualMachineId); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId, null); } @Test(expected = InvalidParameterValueException.class) public void unmanageVMInstanceExpungedInstanceTest() { when(virtualMachine.getState()).thenReturn(VirtualMachine.State.Expunging); - unmanagedVMsManager.unmanageVMInstance(virtualMachineId); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId, null); } @Test(expected = UnsupportedServiceException.class) public void unmanageVMInstanceExistingVMSnapshotsTest() { when(virtualMachine.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.None); - unmanagedVMsManager.unmanageVMInstance(virtualMachineId); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId, null); } @Test(expected = UnsupportedServiceException.class) public void unmanageVMInstanceExistingVolumeSnapshotsTest() { when(virtualMachine.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.None); - unmanagedVMsManager.unmanageVMInstance(virtualMachineId); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId, null); } @Test(expected = UnsupportedServiceException.class) public void unmanageVMInstanceExistingISOAttachedTest() { when(virtualMachine.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.None); - unmanagedVMsManager.unmanageVMInstance(virtualMachineId); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId, null); + } + + @Test + public void unmanageVMInstanceVMwareHostId() { + when(virtualMachine.getType()).thenReturn(VirtualMachine.Type.User); + when(virtualMachine.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + when(virtualMachine.getState()).thenReturn(VirtualMachine.State.Stopped); + UserVmVO userVmVO = mock(UserVmVO.class); + when(userVmVO.getIsoId()).thenReturn(null); + when(userVmDao.findById(anyLong())).thenReturn(userVmVO); + when(vmDao.findById(virtualMachineId)).thenReturn(virtualMachine); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId, 1L); } @Test @@ -489,7 +501,7 @@ public void unmanageVMInstanceStoppedInstanceTest() { UserVmVO userVmVO = mock(UserVmVO.class); when(userVmDao.findById(anyLong())).thenReturn(userVmVO); Mockito.doNothing().when(unmanagedVMsManager).performUnmanageVMInstancePrechecks(any()); - unmanagedVMsManager.unmanageVMInstance(virtualMachineId); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId, null); } @Test From 237cda9c4552c207a0cf04e1462597d8f8f9126c Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Thu, 18 Sep 2025 15:07:08 +0530 Subject: [PATCH 14/17] generate unmanage event and improve code coverage --- .../admin/vm/UnmanageVMInstanceCmd.java | 3 +- .../cloud/vm/VirtualMachineManagerImpl.java | 9 +- .../vm/VirtualMachineManagerImplTest.java | 132 +++++++++++++ .../java/com/cloud/vm/UserVmManagerImpl.java | 10 +- .../com/cloud/vm/UserVmManagerImplTest.java | 184 +++++++++++++++--- .../vm/UnmanagedVMsManagerImplTest.java | 41 +++- 6 files changed, 343 insertions(+), 36 deletions(-) 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 23919360e5eb..ed106891ac8d 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 @@ -69,7 +69,8 @@ public class UnmanageVMInstanceCmd extends BaseAsyncCmd { @Parameter(name = ApiConstants.HOST_ID, type = CommandType.UUID, entityType = HostResponse.class, required = false, - description = "ID of the host where domain XML is stored for stopped Instance", + 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; 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 107da61bfe88..0deb6dec48ab 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2049,6 +2049,9 @@ public Boolean doInTransaction(TransactionStatus status) { throw new CloudRuntimeException("Unable to retrieve host with ID: " + agentHostId); } 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()); } @@ -2092,14 +2095,14 @@ Long persistDomainForKVM(VMInstanceVO vm, Long paramHostId) { /** * 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); @@ -2117,7 +2120,7 @@ private void unmanageVMVolumes(VMInstanceVO vm) { * - 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)) { 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 4e655b81a644..e352ba346e21 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -40,6 +40,8 @@ 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; @@ -51,6 +53,10 @@ 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; @@ -64,6 +70,8 @@ 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; @@ -180,6 +188,9 @@ public class VirtualMachineManagerImplTest { private PrimaryDataStoreDao storagePoolDaoMock; @Mock private VMInstanceVO vmInstanceMock; + @Mock + private VmWorkJobDao _workJobDao; + private long vmInstanceVoMockId = 1L; @Mock @@ -194,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 @@ -274,6 +288,10 @@ public class VirtualMachineManagerImplTest { VolumeDataFactory volumeDataFactoryMock; @Mock StorageManager storageManager; + @Mock + private HighAvailabilityManager _haMgr; + @Mock + VirtualMachineGuru guru; private ConfigDepotImpl configDepotImpl; private boolean updatedConfigKeyDepot = false; @@ -1813,6 +1831,14 @@ public void testPersistDomainForKvmAgentUnavailable() throws AgentUnavailableExc 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); @@ -1821,4 +1847,110 @@ public void testPersistDomainForKvmOperationTimedOut() throws AgentUnavailableEx 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(expected = CloudRuntimeException.class) + 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); + virtualMachineManagerImpl.unmanage(vmMockUuid, null); + } + } + + @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/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 22d659d461a8..d3d46e245034 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -9671,7 +9671,7 @@ public UserVm restoreVMFromBackup(CreateVMFromBackupCmd cmd) throws ResourceUnav /* Generate usage events related to unmanaging a VM */ - private void publishUnmanageVMUsageEvents(UserVmVO vm, List volumes) { + void publishUnmanageVMUsageEvents(UserVmVO vm, List volumes) { postProcessingUnmanageVMVolumes(volumes, vm); postProcessingUnmanageVM(vm); } @@ -9679,12 +9679,12 @@ private void publishUnmanageVMUsageEvents(UserVmVO vm, List volumes) { /* Cleanup the VM from resources and groups */ - private void cleanupUnmanageVMResources(UserVmVO vm) { + void cleanupUnmanageVMResources(UserVmVO vm) { cleanupVmResources(vm); removeVMFromAffinityGroups(vm.getId()); } - private void unmanageVMFromDB(long vmId) { + void unmanageVMFromDB(long vmId) { VMInstanceVO vm = _vmInstanceDao.findById(vmId); vmInstanceDetailsDao.removeDetails(vmId); vm.setState(State.Expunging); @@ -9746,7 +9746,7 @@ private void postProcessingUnmanageVMVolumes(List volumes, UserVmVO vm } } - private void checkUnmanagingVMOngoingVolumeSnapshots(UserVmVO vm) { + void checkUnmanagingVMOngoingVolumeSnapshots(UserVmVO vm) { logger.debug("Checking if there are any ongoing snapshots on the ROOT volumes associated with VM {}", vm); if (checkStatusOfVolumeSnapshots(vm, Volume.Type.ROOT)) { throw new CloudRuntimeException("There is/are unbacked up snapshot(s) on ROOT volume, vm unmanage is not permitted, please try again later."); @@ -9754,7 +9754,7 @@ private void checkUnmanagingVMOngoingVolumeSnapshots(UserVmVO vm) { logger.debug("Found no ongoing snapshots on volume of type ROOT, for the vm {}", vm); } - private void checkUnmanagingVMVolumes(UserVmVO vm, List volumes) { + void checkUnmanagingVMVolumes(UserVmVO vm, List volumes) { for (VolumeVO volume : volumes) { if (volume.getInstanceId() == null || !volume.getInstanceId().equals(vm.getId())) { throw new CloudRuntimeException(String.format("Invalid state for volume %s of VM %s: it is not attached to VM", volume, vm)); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index e1efd87dcd8b..29027599836f 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -33,8 +33,10 @@ import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -45,6 +47,7 @@ import java.time.ZoneOffset; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -86,6 +89,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.template.VnfTemplateManager; import org.apache.cloudstack.userdata.UserDataManager; +import org.apache.cloudstack.vm.UnmanagedVMsManager; import org.apache.cloudstack.vm.lease.VMLeaseManager; import org.junit.After; import org.junit.Assert; @@ -431,6 +435,8 @@ public class UserVmManagerImplTest { @Mock private UUIDManager uuidMgr; + MockedStatic unmanagedVMsManagerMockedStatic; + private static final long vmId = 1l; private static final long zoneId = 2L; private static final long accountId = 3L; @@ -438,6 +444,7 @@ public class UserVmManagerImplTest { private static final long templateId = 11L; private static final long volumeId = 1L; private static final long snashotId = 1L; + private static final String vmUuid = UUID.randomUUID().toString(); private static final long GiB_TO_BYTES = 1024 * 1024 * 1024; @@ -472,11 +479,13 @@ public void beforeTest() { lenient().doNothing().when(resourceLimitMgr).decrementResourceCount(anyLong(), any(Resource.ResourceType.class), anyLong()); Mockito.when(virtualMachineProfile.getId()).thenReturn(vmId); + unmanagedVMsManagerMockedStatic = mockStatic(UnmanagedVMsManager.class); } @After public void afterTest() { CallContext.unregister(); + unmanagedVMsManagerMockedStatic.close(); } @Test @@ -1409,7 +1418,7 @@ public void createVirtualMachineWithCloudRuntimeException() throws ResourceUnava CloudRuntimeException cre = new CloudRuntimeException("Error and CloudRuntimeException is thrown"); cre.addProxyObject(vmId, "vmId"); - Mockito.doThrow(cre).when(userVmManagerImpl).createBasicSecurityGroupVirtualMachine(any(), any(), any(), any(), any(), any(), any(), + doThrow(cre).when(userVmManagerImpl).createBasicSecurityGroupVirtualMachine(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), nullable(Boolean.class), any(), any(), any(), any(), any(), any(), any(), eq(true), any(), any(), any()); @@ -1784,7 +1793,7 @@ public void checkExpungeVMPermissionTestAccountIsNotAdminConfigFalseThrowsPermis public void checkExpungeVmPermissionTestAccountIsNotAdminConfigTrueNoApiAccessThrowsPermissionDeniedException () { Mockito.doReturn(false).when(accountManager).isAdmin(Mockito.anyLong()); Mockito.doReturn(true).when(userVmManagerImpl).getConfigAllowUserExpungeRecoverVm(Mockito.anyLong()); - Mockito.doThrow(PermissionDeniedException.class).when(accountManager).checkApiAccess(accountMock, "expungeVirtualMachine"); + doThrow(PermissionDeniedException.class).when(accountManager).checkApiAccess(accountMock, "expungeVirtualMachine"); Assert.assertThrows(PermissionDeniedException.class, () -> userVmManagerImpl.checkExpungeVmPermission(accountMock)); } @@ -1798,7 +1807,7 @@ public void checkExpungeVmPermissionTestAccountIsNotAdminConfigTrueHasApiAccessR @Test public void checkExpungeVmPermissionTestAccountIsAdminNoApiAccessThrowsPermissionDeniedException () { Mockito.doReturn(true).when(accountManager).isAdmin(Mockito.anyLong()); - Mockito.doThrow(PermissionDeniedException.class).when(accountManager).checkApiAccess(accountMock, "expungeVirtualMachine"); + doThrow(PermissionDeniedException.class).when(accountManager).checkApiAccess(accountMock, "expungeVirtualMachine"); Assert.assertThrows(PermissionDeniedException.class, () -> userVmManagerImpl.checkExpungeVmPermission(accountMock)); } @@ -2604,7 +2613,7 @@ public void canAccountUseNetworkTestPermissionDeniedExceptionThrownReturnFalse() Mockito.doReturn(ControlledEntity.ACLType.Domain).when(networkMock).getAclType(); Mockito.doReturn(Network.GuestType.L2).when(networkMock).getGuestType(); - Mockito.doThrow(PermissionDeniedException.class).when(networkModel).checkNetworkPermissions(accountMock, networkMock); + doThrow(PermissionDeniedException.class).when(networkModel).checkNetworkPermissions(accountMock, networkMock); boolean canAccountUseNetwork = userVmManagerImpl.canAccountUseNetwork(accountMock, networkMock); @@ -2616,7 +2625,7 @@ public void implementNetworkTestImplementedNetworkIsNullReturnCurrentNewNetwork( CallContext callContextMock = Mockito.mock(CallContext.class); NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); - try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { + try (MockedStatic ignored = mockStatic(CallContext.class)) { Mockito.when(CallContext.current()).thenReturn(callContextMock); Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); @@ -2635,7 +2644,7 @@ public void implementNetworkTestImplementedNetworkFirstIsNullReturnCurrentNewNet CallContext callContextMock = Mockito.mock(CallContext.class); NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); - try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { + try (MockedStatic ignored = mockStatic(CallContext.class)) { Mockito.when(CallContext.current()).thenReturn(callContextMock); Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); @@ -2657,7 +2666,7 @@ public void implementNetworkTestImplementedNetworkSecondIsNullReturnCurrentNewNe CallContext callContextMock = Mockito.mock(CallContext.class); NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); - try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { + try (MockedStatic ignored = mockStatic(CallContext.class)) { Mockito.when(CallContext.current()).thenReturn(callContextMock); Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); @@ -2680,7 +2689,7 @@ public void implementNetworkTestImplementedNetworkSecondIsNotNullReturnImplement CallContext callContextMock = Mockito.mock(CallContext.class); NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); - try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { + try (MockedStatic ignored = mockStatic(CallContext.class)) { Mockito.when(CallContext.current()).thenReturn(callContextMock); Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); @@ -2704,7 +2713,7 @@ public void implementNetworkTestImplementedNetworkCatchException() throws Resour CallContext callContextMock = Mockito.mock(CallContext.class); - try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { + try (MockedStatic ignored = mockStatic(CallContext.class)) { Mockito.when(CallContext.current()).thenReturn(callContextMock); Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); @@ -2712,7 +2721,7 @@ public void implementNetworkTestImplementedNetworkCatchException() throws Resour Pair implementedNetwork = Mockito.mock(Pair.class); Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); - Mockito.doThrow(InvalidParameterValueException.class).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); + doThrow(InvalidParameterValueException.class).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); CloudRuntimeException assertThrows = Assert.assertThrows(expectedCloudRuntimeException, () -> { userVmManagerImpl.implementNetwork(accountMock, _dcMock, networkMock); @@ -2968,7 +2977,7 @@ public void moveVmToUserTestCallerIsNotRootAdminAndDomainAdminThrowsInvalidParam public void moveVmToUserTestValidateVmExistsAndIsNotRunningThrowsInvalidParameterValueException() { Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); - Mockito.doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfVmSupportsMigration(Mockito.any(), Mockito.anyLong()); + doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfVmSupportsMigration(Mockito.any(), Mockito.anyLong()); Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); } @@ -3011,7 +3020,7 @@ public void moveVmToUserTestValidateIfVmHasNoRulesThrowsInvalidParameterValueExc configureDoNothingForMethodsThatWeDoNotWantToTest(); - Mockito.doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfVmHasNoRules(Mockito.any(), Mockito.anyLong()); + doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfVmHasNoRules(Mockito.any(), Mockito.anyLong()); Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); } @@ -3030,7 +3039,7 @@ public void moveVmToUserTestSnapshotsForVolumeExistThrowsInvalidParameterValueEx configureDoNothingForMethodsThatWeDoNotWantToTest(); - Mockito.doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfVolumesHaveNoSnapshots(Mockito.any()); + doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfVolumesHaveNoSnapshots(Mockito.any()); Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); } @@ -3048,7 +3057,7 @@ public void moveVmToUserTestVerifyResourceLimitsForAccountAndStorageThrowsResour configureDoNothingForMethodsThatWeDoNotWantToTest(); - Mockito.doThrow(ResourceAllocationException.class).when(userVmManagerImpl).verifyResourceLimitsForAccountAndStorage(Mockito.any(), Mockito.any(), + doThrow(ResourceAllocationException.class).when(userVmManagerImpl).verifyResourceLimitsForAccountAndStorage(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Assert.assertThrows(ResourceAllocationException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); @@ -3067,7 +3076,7 @@ public void moveVmToUserTestVerifyValidateIfNewOwnerHasAccessToTemplateThrowsInv configureDoNothingForMethodsThatWeDoNotWantToTest(); - Mockito.doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfNewOwnerHasAccessToTemplate(Mockito.any(), Mockito.any(), Mockito.any()); + doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfNewOwnerHasAccessToTemplate(Mockito.any(), Mockito.any(), Mockito.any()); Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); } @@ -3087,7 +3096,7 @@ public void moveVmToUserTestAccountManagerCheckAccessThrowsPermissionDeniedExcep configureDoNothingForMethodsThatWeDoNotWantToTest(); - Mockito.doThrow(PermissionDeniedException.class).when(accountManager).checkAccess(Mockito.any(Account.class), Mockito.any()); + doThrow(PermissionDeniedException.class).when(accountManager).checkAccess(Mockito.any(Account.class), Mockito.any()); Assert.assertThrows(PermissionDeniedException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); } @@ -3098,12 +3107,12 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficie LinkedList volumes = new LinkedList(); - try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + try (MockedStatic ignored = mockStatic(UsageEventUtils.class)) { Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(userVmVoMock).getHypervisorType(); configureDoNothingForMethodsThatWeDoNotWantToTest(); - Mockito.doThrow(InsufficientAddressCapacityException.class).when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + doThrow(InsufficientAddressCapacityException.class).when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, @@ -3121,12 +3130,12 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsResourceAl LinkedList volumes = new LinkedList(); - try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + try (MockedStatic ignored = mockStatic(UsageEventUtils.class)) { Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(userVmVoMock).getHypervisorType(); configureDoNothingForMethodsThatWeDoNotWantToTest(); - Mockito.doThrow(ResourceAllocationException.class).when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + doThrow(ResourceAllocationException.class).when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, @@ -3145,7 +3154,7 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab LinkedList volumes = new LinkedList(); - try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + try (MockedStatic ignored = mockStatic(UsageEventUtils.class)) { Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(userVmVoMock).getHypervisorType(); Mockito.doReturn(false).when(userVmManagerImpl).isResourceCountRunningVmsOnlyEnabled(); @@ -3168,7 +3177,7 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab LinkedList volumes = new LinkedList(); - try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + try (MockedStatic ignored = mockStatic(UsageEventUtils.class)) { Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(userVmVoMock).getHypervisorType(); Mockito.doReturn(true).when(userVmManagerImpl).isResourceCountRunningVmsOnlyEnabled(); @@ -3618,7 +3627,7 @@ public void testDestroyVm() throws ResourceUnavailableException { when(callContext.getCallingAccount()).thenReturn(callingAccount); when(accountManager.isAdmin(callingAccount.getId())).thenReturn(true); doNothing().when(accountManager).checkApiAccess(callingAccount, BaseCmd.getCommandNameByClass(ExpungeVMCmd.class)); - try (MockedStatic mockedCallContext = Mockito.mockStatic(CallContext.class)) { + try (MockedStatic mockedCallContext = mockStatic(CallContext.class)) { mockedCallContext.when(CallContext::current).thenReturn(callContext); mockedCallContext.when(() -> CallContext.register(callContext, ApiCommandResourceType.Volume)).thenReturn(callContext); @@ -3650,7 +3659,7 @@ public void testDestroyVm() throws ResourceUnavailableException { doReturn(vm).when(userVmManagerImpl).destroyVm(vmId, expunge); doReturn(true).when(userVmManagerImpl).expunge(vm); - try (MockedStatic mockedUsageEventUtils = Mockito.mockStatic(UsageEventUtils.class)) { + try (MockedStatic mockedUsageEventUtils = mockStatic(UsageEventUtils.class)) { UserVm result = userVmManagerImpl.destroyVm(cmd); @@ -3795,7 +3804,7 @@ public void testApplyLeaseOnUpdateInstanceToRemoveLease() { UserVmVO userVm = Mockito.mock(UserVmVO.class); when(userVm.getId()).thenReturn(vmId);; when(vmInstanceDetailsDao.listDetailsKeyPairs(anyLong(), anyList())).thenReturn(getLeaseDetails(2, VMLeaseManager.LeaseActionExecution.PENDING.name())); - try (MockedStatic ignored = Mockito.mockStatic(ActionEventUtils.class)) { + try (MockedStatic ignored = mockStatic(ActionEventUtils.class)) { Mockito.when(ActionEventUtils.onActionEvent(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), @@ -3908,4 +3917,129 @@ public void createVirtualMachineWithExistingSnapshot() throws ResourceUnavailabl userVmManagerImpl.createVirtualMachine(deployVMCmd); } + + @Test + public void testUnmanageUserVMVmNotFound() { + when(userVmDao.findById(vmId)).thenReturn(null); + InvalidParameterValueException exception = assertThrows(InvalidParameterValueException.class, () -> { + userVmManagerImpl.unmanageUserVM(vmId, null); + }); + assertEquals("Unable to find a VM with ID = " + vmId, exception.getMessage()); + verify(userVmDao, never()).acquireInLockTable(anyLong()); + verify(userVmDao, never()).releaseFromLockTable(anyLong()); + } + + @Test + public void testUnmanageUserVMAlreadyRemoved() { + when(userVmVoMock.getRemoved()).thenReturn(new Date()); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + assertThrows(InvalidParameterValueException.class, () -> { + userVmManagerImpl.unmanageUserVM(vmId, null); + }); + } + + @Test + public void testUnmanageUserVMInvalidState() { + when(userVmVoMock.getId()).thenReturn(vmId); + when(userVmVoMock.getName()).thenReturn("test-vm"); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + when(userVmDao.acquireInLockTable(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Starting); + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> { + userVmManagerImpl.unmanageUserVM(vmId, null); + }); + assertEquals("Instance: test-vm is not running or stopped, cannot be unmanaged", exception.getMessage()); + verify(userVmDao, times(1)).releaseFromLockTable(vmId); + } + + @Test + public void testUnmanageUserVMUnsupportedHypervisor() { + when(userVmVoMock.getId()).thenReturn(vmId); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + when(userVmDao.acquireInLockTable(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Stopped); + when(userVmVoMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.Hyperv); + unmanagedVMsManagerMockedStatic.when(() -> UnmanagedVMsManager.isSupported(Hypervisor.HypervisorType.Hyperv)).thenReturn(false); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> { + userVmManagerImpl.unmanageUserVM(vmId, null); + }); + + assertEquals("Unmanaging a VM is currently not supported on hypervisor Hyperv", exception.getMessage()); + verify(userVmDao, times(1)).releaseFromLockTable(vmId); + } + + @Test + public void testUnmanageUserVMItManagerReturnsFalse() { + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getId()).thenReturn(vmId); + when(userVmVoMock.getUuid()).thenReturn(vmUuid); + when(userVmDao.acquireInLockTable(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running); + when(userVmVoMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + unmanagedVMsManagerMockedStatic.when(() -> UnmanagedVMsManager.isSupported(Hypervisor.HypervisorType.KVM)).thenReturn(true); + when(volumeDaoMock.findByInstance(vmId)).thenReturn(Collections.emptyList()); + when(virtualMachineManager.unmanage(vmUuid, null)).thenReturn(new Pair<>(false, "Backend failure")); + + doNothing().when(userVmManagerImpl).checkUnmanagingVMOngoingVolumeSnapshots(any(UserVmVO.class)); + doNothing().when(userVmManagerImpl).checkUnmanagingVMVolumes(any(UserVmVO.class), any(List.class)); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> { + userVmManagerImpl.unmanageUserVM(vmId, null); + }); + + assertEquals("Error while unmanaging VM: " + vmUuid, exception.getMessage()); + verify(userVmManagerImpl, never()).cleanupUnmanageVMResources(any(UserVmVO.class)); + verify(userVmManagerImpl, never()).unmanageVMFromDB(anyLong()); + verify(userVmDao, times(1)).releaseFromLockTable(vmId); + } + + @Test + public void testUnmanageUserVMGenericException() { + RuntimeException testException = new RuntimeException("Something went wrong"); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getId()).thenReturn(vmId); + when(userVmDao.acquireInLockTable(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running); + when(userVmVoMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + unmanagedVMsManagerMockedStatic.when(() -> UnmanagedVMsManager.isSupported(Hypervisor.HypervisorType.KVM)).thenReturn(true); + doThrow(testException).when(userVmManagerImpl).checkUnmanagingVMOngoingVolumeSnapshots(any(UserVmVO.class)); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> { + userVmManagerImpl.unmanageUserVM(vmId, null); + }); + + assertNotNull(exception.getCause()); + assertEquals(testException, exception.getCause()); + verify(userVmDao, times(1)).releaseFromLockTable(vmId); + } + + @Test + public void testUnmanageUserVMSuccess() { + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getId()).thenReturn(vmId); + when(userVmVoMock.getUuid()).thenReturn(vmUuid); + when(userVmDao.acquireInLockTable(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running); + when(userVmVoMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + unmanagedVMsManagerMockedStatic.when(() -> UnmanagedVMsManager.isSupported(Hypervisor.HypervisorType.KVM)).thenReturn(true); + when(volumeDaoMock.findByInstance(vmId)).thenReturn(Collections.emptyList()); + when(virtualMachineManager.unmanage(vmUuid, null)).thenReturn(new Pair<>(true, "Unmanaged successfully")); + doNothing().when(userVmManagerImpl).checkUnmanagingVMOngoingVolumeSnapshots(any(UserVmVO.class)); + doNothing().when(userVmManagerImpl).checkUnmanagingVMVolumes(any(UserVmVO.class), any(List.class)); + doNothing().when(userVmManagerImpl).cleanupUnmanageVMResources(any(UserVmVO.class)); + doNothing().when(userVmManagerImpl).unmanageVMFromDB(anyLong()); + doNothing().when(userVmManagerImpl).publishUnmanageVMUsageEvents(any(UserVmVO.class), any(List.class)); + Pair result = userVmManagerImpl.unmanageUserVM(vmId, null); + assertTrue(result.first()); + assertEquals("Unmanaged successfully", result.second()); + verify(userVmDao, times(1)).acquireInLockTable(vmId); + verify(userVmManagerImpl, times(1)).checkUnmanagingVMOngoingVolumeSnapshots(userVmVoMock); + verify(userVmManagerImpl, times(1)).checkUnmanagingVMVolumes(userVmVoMock, Collections.emptyList()); + verify(userVmManagerImpl, times(1)).cleanupUnmanageVMResources(userVmVoMock); + verify(userVmManagerImpl, times(1)).unmanageVMFromDB(vmId); + verify(userVmManagerImpl, times(1)).publishUnmanageVMUsageEvents(userVmVoMock, Collections.emptyList()); + verify(userVmDao, times(1)).releaseFromLockTable(vmId); + } + } diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index 8921a467694a..3eb71815a517 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -31,6 +31,7 @@ import java.net.URI; import java.util.ArrayList; +import java.util.Date; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -447,8 +448,16 @@ public void importUnmanagedInstanceMissingInstanceTest() { @Test(expected = InvalidParameterValueException.class) public void unmanageVMInstanceMissingInstanceTest() { - long notExistingId = 10L; - unmanagedVMsManager.unmanageVMInstance(notExistingId, null); + when(vmDao.findById(anyLong())).thenReturn(null); + unmanagedVMsManager.unmanageVMInstance(1L, null); + } + + @Test(expected = InvalidParameterValueException.class) + public void unmanageVMInstanceRemovedInstanceTest() { + VMInstanceVO userVmVO = mock(VMInstanceVO.class); + when(vmDao.findById(anyLong())).thenReturn(userVmVO); + when(userVmVO.getRemoved()).thenReturn(new Date()); + unmanagedVMsManager.unmanageVMInstance(1L, null); } @Test(expected = InvalidParameterValueException.class) @@ -469,6 +478,19 @@ public void unmanageVMInstanceExistingVMSnapshotsTest() { unmanagedVMsManager.unmanageVMInstance(virtualMachineId, null); } + @Test(expected = UnsupportedServiceException.class) + public void unmanageVMInstanceInvalidHyperVisor() { + when(virtualMachine.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.None); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId, null); + } + + @Test(expected = UnsupportedServiceException.class) + public void unmanageVMInstanceInvalidVmType() { + when(virtualMachine.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.VMware); + when(virtualMachine.getType()).thenReturn(VirtualMachine.Type.ConsoleProxy); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId, null); + } + @Test(expected = UnsupportedServiceException.class) public void unmanageVMInstanceExistingVolumeSnapshotsTest() { when(virtualMachine.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.None); @@ -481,6 +503,21 @@ public void unmanageVMInstanceExistingISOAttachedTest() { unmanagedVMsManager.unmanageVMInstance(virtualMachineId, null); } + @Test(expected = UnsupportedServiceException.class) + public void unmanageVMInstanceVmwareHostIdParamTest() { + when(virtualMachine.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.VMware); + when(virtualMachine.getType()).thenReturn(VirtualMachine.Type.User); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId, 1L); + } + + @Test(expected = UnsupportedServiceException.class) + public void unmanageVMInstanceRunningHostIdParamTest() { + when(virtualMachine.getType()).thenReturn(VirtualMachine.Type.User); + when(virtualMachine.getState()).thenReturn(VirtualMachine.State.Running); + when(virtualMachine.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + unmanagedVMsManager.unmanageVMInstance(virtualMachineId, 1L); + } + @Test public void unmanageVMInstanceVMwareHostId() { when(virtualMachine.getType()).thenReturn(VirtualMachine.Type.User); From d5e42599c6861ded5c043bcf3687d20ff060945d Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Thu, 25 Sep 2025 20:53:05 +0530 Subject: [PATCH 15/17] fix unmanage stopped instance in vmware --- .../main/java/com/cloud/vm/VirtualMachineManagerImpl.java | 2 +- .../java/com/cloud/vm/VirtualMachineManagerImplTest.java | 5 +++-- ui/public/locales/en.json | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) 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 0deb6dec48ab..b1666fc3f7f3 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2046,7 +2046,7 @@ public Boolean doInTransaction(TransactionStatus status) { }); HostVO host = ApiDBUtils.findHostById(agentHostId); if (host == null) { - throw new CloudRuntimeException("Unable to retrieve host with ID: " + agentHostId); + 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, 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 e352ba346e21..a07870d09af2 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -1865,7 +1865,7 @@ public void testUnmanagePendingWorkJobs() { virtualMachineManagerImpl.unmanage(vmMockUuid, null); } - @Test(expected = CloudRuntimeException.class) + @Test public void testUnmanageHostNotFoundAfterTransaction() { when(vmInstanceMock.getHostId()).thenReturn(hostMockId); when(vmInstanceDaoMock.findByUuid(vmMockUuid)).thenReturn(vmInstanceMock); @@ -1878,7 +1878,8 @@ public void testUnmanageHostNotFoundAfterTransaction() { doNothing().when(guru).finalizeUnmanage(vmInstanceMock); try (MockedStatic ignored = Mockito.mockStatic(ApiDBUtils.class)) { when(ApiDBUtils.findHostById(hostMockId)).thenReturn(null); - virtualMachineManagerImpl.unmanage(vmMockUuid, null); + Pair result = virtualMachineManagerImpl.unmanage(vmMockUuid, null); + assertNull(result.second()); } } diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 394de6ca6d26..d9ec55069d8a 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -3998,7 +3998,7 @@ "message.vr.alert.upon.network.offering.creation.others": "As none of the obligatory services for creating a virtual router (VPN, DHCP, DNS, Firewall, LB, UserData, SourceNat, StaticNat, PortForwarding) are enabled, the virtual router will not be created and the compute offering will not be used.", "message.warn.change.primary.storage.scope": "This feature is tested and supported for the following configurations:
KVM - NFS/Ceph - DefaultPrimary
VMware - NFS - DefaultPrimary
*There might be extra steps involved to make it work for other configurations.", "message.warn.filetype": "jpg, jpeg, png, bmp and svg are the only supported image formats.", -"message.warn.importing.instance.without.nic": "WARNING: This Instance is being imported without NICs and many Network resources will not be available. Consider creating a NIC via vCenter before importing or as soon as the Instance is imported.", +"message.warn.importing.instance.without.nic": "WARNING: This Instance is being imported without NICs and many Network resources will not be available. Consider creating a NIC via vCenter before importing or as soon as the Instance is imported. For KVM host, allocate a NIC to Instance after import.", "message.warn.select.template": "Please select a Template for Registration.", "message.warn.zone.mtu.update": "Please note that this limit won't affect pre-existing Network's MTU settings", "message.webhook.deliveries.time.filter": "Webhook deliveries list can be filtered based on date-time. Select 'Custom' for specifying start and end date range.", From c5d6b2f876461f171eaf8ed37ce13be27a8a7a40 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Mon, 29 Sep 2025 12:02:27 +0530 Subject: [PATCH 16/17] Use forced true to unmanage instance with config drive --- .../admin/vm/UnmanageVMInstanceCmd.java | 14 +- .../cloudstack/vm/UnmanageVMService.java | 2 +- .../agent/api/UnmanageInstanceCommand.java | 9 + .../cloud/vm/VirtualMachineManagerImpl.java | 1 + ...LibvirtUnmanageInstanceCommandWrapper.java | 88 ++++- ...irtUnmanageInstanceCommandWrapperTest.java | 357 ++++++++++++++++++ .../vm/UnmanagedVMsManagerImpl.java | 5 +- .../vm/UnmanagedVMsManagerImplTest.java | 41 +- 8 files changed, 493 insertions(+), 24 deletions(-) create mode 100644 plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnmanageInstanceCommandWrapperTest.java 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 ed106891ac8d..2c60b5741261 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 @@ -42,6 +42,7 @@ 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; @@ -74,6 +75,13 @@ public class UnmanageVMInstanceCmd extends BaseAsyncCmd { 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 /////////////////////// ///////////////////////////////////////////////////// @@ -100,6 +108,10 @@ public void setHostId(Long hostId) { this.hostId = hostId; } + public Boolean isForced() { + return BooleanUtils.isTrue(forced); + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @@ -110,7 +122,7 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE UnmanageVMInstanceResponse response = new UnmanageVMInstanceResponse(); try { CallContext.current().setEventDetails("VM ID = " + vmId); - Pair result = unmanagedVMsManager.unmanageVMInstance(vmId, hostId); + Pair result = unmanagedVMsManager.unmanageVMInstance(vmId, hostId, isForced()); if (result.first()) { response.setSuccess(true); response.setHostId(result.second()); 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 44c4af655a97..70d8795f3058 100644 --- a/api/src/main/java/org/apache/cloudstack/vm/UnmanageVMService.java +++ b/api/src/main/java/org/apache/cloudstack/vm/UnmanageVMService.java @@ -26,5 +26,5 @@ public interface UnmanageVMService { * * @return (true if successful, false if not, hostUuid) if the VM is successfully unmanaged. */ - Pair unmanageVMInstance(long vmId, Long paramHostId); + Pair unmanageVMInstance(long vmId, Long paramHostId, boolean isForced); } diff --git a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java index f728cb0ed745..dd504b9ea265 100644 --- a/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java +++ b/core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java @@ -27,6 +27,7 @@ public class UnmanageInstanceCommand extends Command { String instanceName; boolean executeInSequence = false; VirtualMachineTO vm; + boolean isConfigDriveAttached; @Override public boolean executeInSequence() { @@ -49,4 +50,12 @@ public String getInstanceName() { public VirtualMachineTO getVm() { return vm; } + + public boolean isConfigDriveAttached() { + return isConfigDriveAttached; + } + + public void setConfigDriveAttached(boolean configDriveAttached) { + isConfigDriveAttached = configDriveAttached; + } } 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 880cad35dd0b..7c5d43fea97c 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2089,6 +2089,7 @@ Long persistDomainForKVM(VMInstanceVO vm, Long 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); 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 index 0c525426fa53..33ad274b8d1a 100644 --- 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 @@ -19,11 +19,34 @@ 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; @@ -48,12 +71,15 @@ public Answer execute(final UnmanageInstanceCommand command, final LibvirtComput 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); - String domainXML = dom.getXMLDesc(1); - conn.domainDefineXML(domainXML).free(); + vmFinalSpecification = dom.getXMLDesc(1); + if (command.isConfigDriveAttached()) { + vmFinalSpecification = cleanupConfigDrive(vmFinalSpecification, instanceName); + } } else { // define domain using reconstructed vmSpec logger.debug("Unmanaging Stopped KVM instance: {}", instanceName); @@ -61,16 +87,23 @@ public Answer execute(final UnmanageInstanceCommand command, final LibvirtComput libvirtComputingResource.createVbd(conn, vmSpec, instanceName, vm); conn = libvirtUtilitiesHelper.getConnectionByType(vm.getHvsType()); String vmInitialSpecification = vm.toString(); - String vmFinalSpecification = performXmlTransformHook(vmInitialSpecification, libvirtComputingResource); - conn.domainDefineXML(vmFinalSpecification).free(); + vmFinalSpecification = performXmlTransformHook(vmInitialSpecification, libvirtComputingResource); } - logger.debug("Successfully unmanaged KVM instance: {}", instanceName); + 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.warn("LibvirtException occurred during unmanaging instance: {} ", instanceName, e); + logger.error("LibvirtException occurred during unmanaging instance: {} ", instanceName, e); return new UnmanageInstanceAnswer(command, false, e.getMessage()); - } catch (final URISyntaxException | InternalErrorException e) { - logger.warn("URISyntaxException ", e); + } 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) { @@ -83,6 +116,45 @@ public Answer execute(final UnmanageInstanceCommand command, final LibvirtComput } } + 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 { 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 000000000000..13ddc26610ce --- /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" + + "