vm-import: fix stopped managed vms listing in unmanaged instances (#7606)

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Co-authored-by: Rohit Yadav <rohit.yadav@shapeblue.com>

engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java

@@ -164,5 +164,5 @@ public interface VMInstanceDao extends GenericDao<VMInstanceVO, Long>, StateDao<
 
     void updateSystemVmTemplateId(long templateId, Hypervisor.HypervisorType hypervisorType);
 
-    List<VMInstanceVO> listByHostOrLastHostOrHostPod(long hostId, long podId);
+    List<VMInstanceVO> listByHostOrLastHostOrHostPod(List<Long> hostIds, long podId);
 }

engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java

@@ -989,19 +989,19 @@ public class VMInstanceDaoImpl extends GenericDaoBase<VMInstanceVO, Long> implem
     }
 
     @Override
-    public List<VMInstanceVO> listByHostOrLastHostOrHostPod(long hostId, long podId) {
+    public List<VMInstanceVO> listByHostOrLastHostOrHostPod(List<Long> hostIds, long podId) {
         SearchBuilder<VMInstanceVO> sb = createSearchBuilder();
-        sb.or().op("hostId", sb.entity().getHostId(), Op.EQ);
+        sb.and().op("hostId", sb.entity().getHostId(), Op.IN);
-        sb.or("lastHostId", sb.entity().getLastHostId(), Op.EQ);
+        sb.or("lastHostId", sb.entity().getLastHostId(), Op.IN);
-        sb.and().op("hostIdNull", sb.entity().getHostId(), SearchCriteria.Op.NULL);
+        sb.or().op("hostIdNull", sb.entity().getHostId(), SearchCriteria.Op.NULL);
         sb.and("lastHostIdNull", sb.entity().getHostId(), SearchCriteria.Op.NULL);
         sb.and("podId", sb.entity().getPodIdToDeployIn(), Op.EQ);
         sb.cp();
         sb.cp();
         sb.done();
         SearchCriteria<VMInstanceVO> sc = sb.create();
-        sc.setParameters("hostId", String.valueOf(hostId));
+        sc.setParameters("hostId", hostIds.toArray());
-        sc.setParameters("lastHostId", String.valueOf(hostId));
+        sc.setParameters("lastHostId", hostIds.toArray());
         sc.setParameters("podId", String.valueOf(podId));
         return listBy(sc);
     }

server/src/main/java/com/cloud/api/query/QueryManagerImpl.java

@@ -195,6 +195,7 @@ import com.cloud.exception.CloudAuthenticationException;
 import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.exception.PermissionDeniedException;
 import com.cloud.ha.HighAvailabilityManager;
+import com.cloud.host.Host;
 import com.cloud.hypervisor.Hypervisor;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.network.RouterHealthCheckResult;
@@ -1076,7 +1077,12 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
         sb.and("stateNIN", sb.entity().getState(), SearchCriteria.Op.NIN);
         sb.and("dataCenterId", sb.entity().getDataCenterId(), SearchCriteria.Op.EQ);
         sb.and("podId", sb.entity().getPodId(), SearchCriteria.Op.EQ);
-        sb.and("clusterId", sb.entity().getClusterId(), SearchCriteria.Op.EQ);
+        if (clusterId != null) {
+            sb.and().op("clusterId", sb.entity().getClusterId(), SearchCriteria.Op.EQ);
+            sb.or("clusterHostId", sb.entity().getHostId(), Op.IN);
+            sb.or("clusterLastHostId", sb.entity().getLastHostId(), Op.IN);
+            sb.cp();
+        }
         sb.and("hypervisorType", sb.entity().getHypervisorType(), SearchCriteria.Op.EQ);
         sb.and("hostIdEQ", sb.entity().getHostId(), SearchCriteria.Op.EQ);
         sb.and("templateId", sb.entity().getTemplateId(), SearchCriteria.Op.EQ);
@@ -1272,6 +1278,10 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
 
             if (clusterId != null) {
                 sc.setParameters("clusterId", clusterId);
+                List<HostJoinVO> hosts = _hostJoinDao.findByClusterId((Long)clusterId, Host.Type.Routing);
+                List<Long> hostIds = hosts.stream().map(HostJoinVO::getId).collect(Collectors.toList());
+                sc.setParameters("clusterHostId", hostIds.toArray());
+                sc.setParameters("clusterLastHostId", hostIds.toArray());
             }
 
             if (hostId != null) {

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java

@@ -363,8 +363,11 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
         return additionalNameFilter;
     }
 
-    private List<String> getHostManagedVms(Host host) {
+    private List<String> getHostsManagedVms(List<HostVO> hosts) {
-        List<VMInstanceVO> instances = vmDao.listByHostOrLastHostOrHostPod(host.getId(), host.getPodId());
+        if (CollectionUtils.isEmpty(hosts)) {
+            return new ArrayList<>();
+        }
+        List<VMInstanceVO> instances = vmDao.listByHostOrLastHostOrHostPod(hosts.stream().map(HostVO::getId).collect(Collectors.toList()), hosts.get(0).getPodId());
         List<String> managedVms = instances.stream().map(VMInstanceVO::getInstanceName).collect(Collectors.toList());
         return managedVms;
     }
@@ -1035,6 +1038,24 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
         return userVm;
     }
 
+    private HashMap<String, UnmanagedInstanceTO> getUnmanagedInstancesForHost(HostVO host, String instanceName, List<String> managedVms) {
+        HashMap<String, UnmanagedInstanceTO> unmanagedInstances = new HashMap<>();
+        if (host.isInMaintenanceStates()) {
+            return unmanagedInstances;
+        }
+
+        GetUnmanagedInstancesCommand command = new GetUnmanagedInstancesCommand();
+        command.setInstanceName(instanceName);
+        command.setManagedInstancesNames(managedVms);
+        Answer answer = agentManager.easySend(host.getId(), command);
+        if (!(answer instanceof GetUnmanagedInstancesAnswer)) {
+            return unmanagedInstances;
+        }
+        GetUnmanagedInstancesAnswer unmanagedInstancesAnswer = (GetUnmanagedInstancesAnswer) answer;
+        unmanagedInstances = unmanagedInstancesAnswer.getUnmanagedInstances();
+        return unmanagedInstances;
+    }
+
     @Override
     public ListResponse<UnmanagedInstanceResponse> listUnmanagedInstances(ListUnmanagedInstancesCmd cmd) {
         final Account caller = CallContext.current().getCallingAccount();
@@ -1058,24 +1079,11 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
         }
         List<HostVO> hosts = resourceManager.listHostsInClusterByStatus(clusterId, Status.Up);
         List<String> additionalNameFilters = getAdditionalNameFilters(cluster);
+        List<String> managedVms = new ArrayList<>(additionalNameFilters);
+        managedVms.addAll(getHostsManagedVms(hosts));
         List<UnmanagedInstanceResponse> responses = new ArrayList<>();
         for (HostVO host : hosts) {
-            if (host.isInMaintenanceStates()) {
+            HashMap<String, UnmanagedInstanceTO> unmanagedInstances = getUnmanagedInstancesForHost(host, cmd.getName(), managedVms);
-                continue;
-            }
-            List<String> managedVms = new ArrayList<>();
-            managedVms.addAll(additionalNameFilters);
-            managedVms.addAll(getHostManagedVms(host));
-
-            GetUnmanagedInstancesCommand command = new GetUnmanagedInstancesCommand();
-            command.setInstanceName(cmd.getName());
-            command.setManagedInstancesNames(managedVms);
-            Answer answer = agentManager.easySend(host.getId(), command);
-            if (!(answer instanceof GetUnmanagedInstancesAnswer)) {
-                continue;
-            }
-            GetUnmanagedInstancesAnswer unmanagedInstancesAnswer = (GetUnmanagedInstancesAnswer) answer;
-            HashMap<String, UnmanagedInstanceTO> unmanagedInstances = new HashMap<>(unmanagedInstancesAnswer.getUnmanagedInstances());
             Set<String> keys = unmanagedInstances.keySet();
             for (String key : keys) {
                 UnmanagedInstanceTO instance = unmanagedInstances.get(key);
@@ -1098,9 +1106,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
             throw new PermissionDeniedException(String.format("Cannot perform this operation, Calling account is not root admin: %s", caller.getUuid()));
         }
         final Long clusterId = cmd.getClusterId();
-        if (clusterId == null) {
-            throw new InvalidParameterValueException(String.format("Cluster ID cannot be null"));
-        }
         final Cluster cluster = clusterDao.findById(clusterId);
         if (cluster == null) {
             throw new InvalidParameterValueException(String.format("Cluster ID: %d cannot be found", clusterId));
@@ -1111,10 +1116,10 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
         final DataCenter zone = dataCenterDao.findById(cluster.getDataCenterId());
         final String instanceName = cmd.getName();
         if (StringUtils.isEmpty(instanceName)) {
-            throw new InvalidParameterValueException(String.format("Instance name cannot be empty"));
+            throw new InvalidParameterValueException("Instance name cannot be empty");
         }
         if (cmd.getDomainId() != null && StringUtils.isEmpty(cmd.getAccountName())) {
-            throw new InvalidParameterValueException("domainid parameter must be specified with account parameter");
+            throw new InvalidParameterValueException(String.format("%s parameter must be specified with %s parameter", ApiConstants.DOMAIN_ID, ApiConstants.ACCOUNT));
         }
         final Account owner = accountService.getActiveAccountById(cmd.getEntityOwnerId());
         long userId = CallContext.current().getCallingUserId();
@@ -1122,7 +1127,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
         if (CollectionUtils.isNotEmpty(userVOs)) {
             userId = userVOs.get(0).getId();
         }
-        VMTemplateVO template = null;
+        VMTemplateVO template;
         final Long templateId = cmd.getTemplateId();
         if (templateId == null) {
             template = templateDao.findByName(VM_IMPORT_DEFAULT_TEMPLATE_NAME);
@@ -1139,9 +1144,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
             throw new InvalidParameterValueException(String.format("Template ID: %d cannot be found", templateId));
         }
         final Long serviceOfferingId = cmd.getServiceOfferingId();
-        if (serviceOfferingId == null) {
-            throw new InvalidParameterValueException(String.format("Service offering ID cannot be null"));
-        }
         final ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(serviceOfferingId);
         if (serviceOffering == null) {
             throw new InvalidParameterValueException(String.format("Service offering ID: %d cannot be found", serviceOfferingId));
@@ -1160,7 +1162,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
         String hostName = cmd.getHostName();
         if (StringUtils.isEmpty(hostName)) {
             if (!NetUtils.verifyDomainNameLabel(instanceName, true)) {
-                throw new InvalidParameterValueException(String.format("Please provide hostname for the VM. VM name contains unsupported characters for it to be used as hostname"));
+                throw new InvalidParameterValueException("Please provide hostname for the VM. VM name contains unsupported characters for it to be used as hostname");
             }
             hostName = instanceName;
         }
@@ -1185,59 +1187,49 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
         List<HostVO> hosts = resourceManager.listHostsInClusterByStatus(clusterId, Status.Up);
         UserVm userVm = null;
         List<String> additionalNameFilters = getAdditionalNameFilters(cluster);
+        List<String> managedVms = new ArrayList<>(additionalNameFilters);
+        managedVms.addAll(getHostsManagedVms(hosts));
         for (HostVO host : hosts) {
-            if (host.isInMaintenanceStates()) {
+            HashMap<String, UnmanagedInstanceTO> unmanagedInstances = getUnmanagedInstancesForHost(host, cmd.getName(), managedVms);
-                continue;
-            }
-            List<String> managedVms = new ArrayList<>();
-            managedVms.addAll(additionalNameFilters);
-            managedVms.addAll(getHostManagedVms(host));
-            GetUnmanagedInstancesCommand command = new GetUnmanagedInstancesCommand(instanceName);
-            command.setManagedInstancesNames(managedVms);
-            Answer answer = agentManager.easySend(host.getId(), command);
-            if (!(answer instanceof GetUnmanagedInstancesAnswer)) {
-                continue;
-            }
-            GetUnmanagedInstancesAnswer unmanagedInstancesAnswer = (GetUnmanagedInstancesAnswer) answer;
-            HashMap<String, UnmanagedInstanceTO> unmanagedInstances = unmanagedInstancesAnswer.getUnmanagedInstances();
             if (MapUtils.isEmpty(unmanagedInstances)) {
                 continue;
             }
             Set<String> names = unmanagedInstances.keySet();
             for (String name : names) {
-                if (instanceName.equals(name)) {
+                if (!instanceName.equals(name)) {
-                    UnmanagedInstanceTO unmanagedInstance = unmanagedInstances.get(name);
+                    continue;
-                    if (unmanagedInstance == null) {
-                        throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Unable to retrieve details for unmanaged VM: %s", name));
-                    }
-                    if (template.getName().equals(VM_IMPORT_DEFAULT_TEMPLATE_NAME)) {
-                        String osName = unmanagedInstance.getOperatingSystem();
-                        GuestOS guestOS = null;
-                        if (StringUtils.isNotEmpty(osName)) {
-                            guestOS = guestOSDao.findOneByDisplayName(osName);
-                        }
-                        GuestOSHypervisor guestOSHypervisor = null;
-                        if (guestOS != null) {
-                            guestOSHypervisor = guestOSHypervisorDao.findByOsIdAndHypervisor(guestOS.getId(), host.getHypervisorType().toString(), host.getHypervisorVersion());
-                        }
-                        if (guestOSHypervisor == null && StringUtils.isNotEmpty(unmanagedInstance.getOperatingSystemId())) {
-                            guestOSHypervisor = guestOSHypervisorDao.findByOsNameAndHypervisor(unmanagedInstance.getOperatingSystemId(), host.getHypervisorType().toString(), host.getHypervisorVersion());
-                        }
-                        if (guestOSHypervisor == null) {
-                            if (guestOS != null) {
-                                throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Unable to find hypervisor guest OS ID: %s details for unmanaged VM: %s for hypervisor: %s version: %s. templateid parameter can be used to assign template for VM", guestOS.getUuid(), name, host.getHypervisorType().toString(), host.getHypervisorVersion()));
-                            }
-                            throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Unable to retrieve guest OS details for unmanaged VM: %s with OS name: %s, OS ID: %s for hypervisor: %s version: %s. templateid parameter can be used to assign template for VM", name, osName, unmanagedInstance.getOperatingSystemId(), host.getHypervisorType().toString(), host.getHypervisorVersion()));
-                        }
-                        template.setGuestOSId(guestOSHypervisor.getGuestOsId());
-                    }
-                    userVm = importVirtualMachineInternal(unmanagedInstance, instanceName, zone, cluster, host,
-                            template, displayName, hostName, caller, owner, userId,
-                            serviceOffering, dataDiskOfferingMap,
-                            nicNetworkMap, nicIpAddressMap,
-                            details, cmd.getMigrateAllowed(), forced);
-                    break;
                 }
+                UnmanagedInstanceTO unmanagedInstance = unmanagedInstances.get(name);
+                if (unmanagedInstance == null) {
+                    throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Unable to retrieve details for unmanaged VM: %s", name));
+                }
+                if (template.getName().equals(VM_IMPORT_DEFAULT_TEMPLATE_NAME)) {
+                    String osName = unmanagedInstance.getOperatingSystem();
+                    GuestOS guestOS = null;
+                    if (StringUtils.isNotEmpty(osName)) {
+                        guestOS = guestOSDao.findOneByDisplayName(osName);
+                    }
+                    GuestOSHypervisor guestOSHypervisor = null;
+                    if (guestOS != null) {
+                        guestOSHypervisor = guestOSHypervisorDao.findByOsIdAndHypervisor(guestOS.getId(), host.getHypervisorType().toString(), host.getHypervisorVersion());
+                    }
+                    if (guestOSHypervisor == null && StringUtils.isNotEmpty(unmanagedInstance.getOperatingSystemId())) {
+                        guestOSHypervisor = guestOSHypervisorDao.findByOsNameAndHypervisor(unmanagedInstance.getOperatingSystemId(), host.getHypervisorType().toString(), host.getHypervisorVersion());
+                    }
+                    if (guestOSHypervisor == null) {
+                        if (guestOS != null) {
+                            throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Unable to find hypervisor guest OS ID: %s details for unmanaged VM: %s for hypervisor: %s version: %s. templateid parameter can be used to assign template for VM", guestOS.getUuid(), name, host.getHypervisorType().toString(), host.getHypervisorVersion()));
+                        }
+                        throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Unable to retrieve guest OS details for unmanaged VM: %s with OS name: %s, OS ID: %s for hypervisor: %s version: %s. templateid parameter can be used to assign template for VM", name, osName, unmanagedInstance.getOperatingSystemId(), host.getHypervisorType().toString(), host.getHypervisorVersion()));
+                    }
+                    template.setGuestOSId(guestOSHypervisor.getGuestOsId());
+                }
+                userVm = importVirtualMachineInternal(unmanagedInstance, instanceName, zone, cluster, host,
+                        template, displayName, hostName, caller, owner, userId,
+                        serviceOffering, dataDiskOfferingMap,
+                        nicNetworkMap, nicIpAddressMap,
+                        details, cmd.getMigrateAllowed(), forced);
+                break;
             }
             if (userVm != null) {
                 break;