add hostid in unmanageVirtualMachine cmd and response

Author: sudo87

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<Boolean, String> unmanageUserVM(Long vmId, Long targetHostId);
 
     UserVm allocateVMFromBackup(CreateVMFromBackupCmd cmd) throws InsufficientCapacityException, ResourceAllocationException, ResourceUnavailableException;
 

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<Boolean, String> 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;
     }
-
 }

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;
+    }
 }

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<Boolean, String> unmanageVMInstance(long vmId, Long paramHostId);
 }

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<Boolean, String> unmanage(String vmUuid, Long paramHostId);
 
     UserVm restoreVirtualMachine(long vmId, Long newTemplateId, Long rootDiskOfferingId, boolean expunge, Map<String, String> details) throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException;
 

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<Boolean, String> 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<Boolean>() {
             @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<Long, Long> 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<Long, Long> 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;
     }
 
     /**

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<Long> hostIdCaptor = ArgumentCaptor.forClass(Long.class);
         ArgumentCaptor<UnmanageInstanceCommand> 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<Long> hostIdCaptor = ArgumentCaptor.forClass(Long.class);
         ArgumentCaptor<UnmanageInstanceCommand> 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,25 +1800,25 @@ 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
     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());
+        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
     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());
+        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());
     }
 }

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<Boolean, String> 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<Boolean, String> 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<String, String> details, VmDiskInfo rootVmDiskInfo) {

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<Boolean, String> 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);
     }
 
     /**