[CLOUDSTACK-10128] Simplify the search for host to execute command

Author: rafaelweingartner

engine/schema/src/com/cloud/host/dao/HostDao.java

@@ -19,13 +19,12 @@
 import java.util.Date;
 import java.util.List;
 
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-
 import com.cloud.host.Host;
 import com.cloud.host.Host.Type;
 import com.cloud.host.HostVO;
 import com.cloud.host.Status;
 import com.cloud.hypervisor.Hypervisor;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.info.RunningHostCountInfo;
 import com.cloud.resource.ResourceState;
 import com.cloud.utils.db.GenericDao;
@@ -99,14 +98,13 @@ public interface HostDao extends GenericDao<HostVO, Long>, StateDao<Status, Stat
     HostVO findByIp(String ip);
 
     /**
-     * This method will look for a host that is of the same hypervisor and same zone as the storage pool where the volume of the Snapshot is stored.
+     * This method will look for a host that is of the same hypervisor and zone as indicated in its parameters.
      * <ul>
-     * <li>(this is applicable only for XenServer) If the storage pool is managed, then we will look for a host that has the property 'supportsResign' in cloud.cluster_details
      * <li>We give priority to 'Enabled' hosts, but if no 'Enabled' hosts are found, we use 'Disabled' hosts
      * <li>If no host is found, we throw a runtime exception
      * </ul>
      *
      * Side note: this method is currently only used in XenServerGuru; therefore, it was designed to meet XenServer deployment scenarios requirements.
      */
-    HostVO findHostToOperateOnSnapshotBasedOnStoragePool(StoragePoolVO storagePoolVO);
+    HostVO findHostInZoneToExecuteCommand(long zoneId, HypervisorType hypervisorType);
 }

engine/schema/src/com/cloud/host/dao/HostDaoImpl.java

@@ -24,14 +24,13 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.TimeZone;
 
 import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.persistence.TableGenerator;
 
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
@@ -50,6 +49,7 @@
 import com.cloud.host.Status;
 import com.cloud.host.Status.Event;
 import com.cloud.hypervisor.Hypervisor;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.info.RunningHostCountInfo;
 import com.cloud.org.Grouping;
 import com.cloud.org.Managed;
@@ -1168,47 +1168,42 @@ public List<HostVO> listByType(Host.Type type) {
         return listBy(sc);
     }
 
-    String sqlFindHostConnectedToStoragePoolToExecuteCommand = "select h.id from storage_pool pool "
-            + " join cluster c on pool.cluster_id = c.id "
-            + " %s "
-            + " join host h on h.data_center_id = c.data_center_id and h.hypervisor_type = c.hypervisor_type"
-            + " where pool.id = ? and h.status = 'Up' and h.type = 'Routing' and resource_state = '%s' "
-            + " ORDER by rand() limit 1 ";
+    String sqlFindHostInZoneToExecuteCommand = "Select  id from host "
+            + " where type = 'Routing' and hypervisor_type = ? and data_center_id = ? and status = 'Up' "
+            + " and resource_state = '%s' "
+            + " ORDER by rand() limit 1";
 
     @Override
-    public HostVO findHostToOperateOnSnapshotBasedOnStoragePool(StoragePoolVO storagePoolVO) {
+    public HostVO findHostInZoneToExecuteCommand(long zoneId, HypervisorType hypervisorType) {
         try (TransactionLegacy tx = TransactionLegacy.currentTxn()) {
-            String sql = createSqlFindHostConnectedToStoragePoolToExecuteCommand(storagePoolVO, false);
-            ResultSet rs = executeSqlGetResultSetForMethodFindHostToOperateBasedOnStoragePool(storagePoolVO, tx, sql);
+            String sql = createSqlFindHostToExecuteCommand(false);
+            ResultSet rs = executeSqlGetResultsetForMethodFindHostInZoneToExecuteCommand(hypervisorType, zoneId, tx, sql);
             if (rs.next()) {
                 return findById(rs.getLong("id"));
             }
-            sql = createSqlFindHostConnectedToStoragePoolToExecuteCommand(storagePoolVO, true);
-            rs = executeSqlGetResultSetForMethodFindHostToOperateBasedOnStoragePool(storagePoolVO, tx, sql);
+            sql = createSqlFindHostToExecuteCommand(true);
+            rs = executeSqlGetResultsetForMethodFindHostInZoneToExecuteCommand(hypervisorType, zoneId, tx, sql);
             if (!rs.next()) {
-                throw new CloudRuntimeException(String.format("Could not find a host connected to the storage pool [storagepool=%d]. ", storagePoolVO.getId()));
+                throw new CloudRuntimeException(String.format("Could not find a host in zone [zoneId=%d] to operate on. ", zoneId));
             }
             return findById(rs.getLong("id"));
         } catch (SQLException e) {
             throw new CloudRuntimeException(e);
         }
     }
 
-    private ResultSet executeSqlGetResultSetForMethodFindHostToOperateBasedOnStoragePool(StoragePoolVO storagePoolVO, TransactionLegacy tx, String sql) throws SQLException {
+    private ResultSet executeSqlGetResultsetForMethodFindHostInZoneToExecuteCommand(HypervisorType hypervisorType, long zoneId, TransactionLegacy tx, String sql) throws SQLException {
         PreparedStatement pstmt = tx.prepareAutoCloseStatement(sql);
-        pstmt.setLong(1, storagePoolVO.getId());
+        pstmt.setString(1, Objects.toString(hypervisorType));
+        pstmt.setLong(2, zoneId);
         return pstmt.executeQuery();
     }
 
-    private String createSqlFindHostConnectedToStoragePoolToExecuteCommand(StoragePoolVO storagePoolVO, boolean useDisabledHosts) {
+    private String createSqlFindHostToExecuteCommand(boolean useDisabledHosts) {
         String hostResourceStatus = "Enabled";
         if (useDisabledHosts) {
             hostResourceStatus = "Disabled";
         }
-        String joinForManagedStorage = StringUtils.EMPTY;
-        if (storagePoolVO.isManaged()) {
-            joinForManagedStorage = " join cluster_details cd on cd.cluster_id = c.id and cd.name = 'supportsResign' and cd.value = 'true' ";
-        }
-        return String.format(sqlFindHostConnectedToStoragePoolToExecuteCommand, joinForManagedStorage, hostResourceStatus);
+        return String.format(sqlFindHostInZoneToExecuteCommand, hostResourceStatus);
     }
 }

engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java

@@ -123,11 +123,4 @@ public interface PrimaryDataStoreDao extends GenericDao<StoragePoolVO, Long> {
     List<StoragePoolVO> listLocalStoragePoolByPath(long datacenterId, String path);
 
     void deletePoolTags(long poolId);
-
-    /**
-     *  Looks for a storage pool where the original volume of the snapshot was taken.
-     *  Even if the volume has already been deleted, we will return the last storage pool where it was stored.
-     */
-    StoragePoolVO findStoragePoolForSnapshot(long snapshotId);
-
 }

engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java

@@ -554,25 +554,4 @@ public List<StoragePoolVO> findZoneWideStoragePoolsByHypervisor(long dataCenterI
     public void deletePoolTags(long poolId) {
         _tagsDao.deleteTags(poolId);
     }
-
-    private String sqlIsSnapshotStoragePoolManaged = "select pool.id from snapshots s "
-            + " join volumes v on v.id = s.volume_id "
-            + " join storage_pool pool on pool.id = v.pool_id  "
-            + " where s.id = ?";
-
-    @Override
-    public StoragePoolVO findStoragePoolForSnapshot(long snapshotId) {
-        try (TransactionLegacy tx = TransactionLegacy.currentTxn();
-                PreparedStatement pstmt = tx.prepareAutoCloseStatement(sqlIsSnapshotStoragePoolManaged);) {
-            pstmt.setLong(1, snapshotId);
-            ResultSet rs = pstmt.executeQuery();
-            if (!rs.next()) {
-                throw new CloudRuntimeException(String.format("Could not find a storage pool for snapshot [snapshotId=%d]. ", snapshotId));
-            }
-            long storagePoolId = rs.getLong("id");
-            return findById(storagePoolId);
-        } catch (SQLException e) {
-            throw new CloudRuntimeException(e);
-        }
-    }
 }

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/XenServerGuru.java

@@ -202,9 +202,9 @@ public Pair<Boolean, Long> getCommandHostDelegation(long hostId, Command cmd) {
             logger.debug("We are returning the default host to execute commands because the source and destination objects are not snapshot and template respectively.");
             return defaultHostToExecuteCommands;
         }
-        long snapshotId = srcData.getId();
-        StoragePoolVO storagePoolVO = storagePoolDao.findStoragePoolForSnapshot(snapshotId);
-        HostVO hostCandidateToExecutedCommand = hostDao.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolVO);
+        HostVO defaultHostToExecuteCommand = hostDao.findById(hostId);
+
+        HostVO hostCandidateToExecutedCommand = hostDao.findHostInZoneToExecuteCommand(defaultHostToExecuteCommand.getDataCenterId(), srcData.getHypervisorType());
         hostDao.loadDetails(hostCandidateToExecutedCommand);
         String hypervisorVersion = hostCandidateToExecutedCommand.getHypervisorVersion();
         if (StringUtils.isBlank(hypervisorVersion)) {
@@ -219,9 +219,7 @@ public Pair<Boolean, Long> getCommandHostDelegation(long hostId, Command cmd) {
         String snapshotHotFixVersion = hostCandidateToExecutedCommand.getDetail(XenserverConfigs.XS620HotFix);
         boolean isXenServer620 = StringUtils.equals(hypervisorVersion, "6.2.0");
         if (isXenServer620 && !StringUtils.equalsIgnoreCase(XenserverConfigs.XSHotFix62ESP1004, snapshotHotFixVersion)) {
-            logger.debug(String.format(
-                    "We are returning the default host to execute commands because the hypervisor version is not 6.2.0 with hotfix ESP1004 [hypervisorVersion=%s, hotfixVersion=%s]",
-                    hypervisorVersion, snapshotHotFixVersion));
+            logger.debug(String.format("We are returning the default host to execute commands because the hypervisor version is not 6.2.0 with hotfix ESP1004 [hypervisorVersion=%s, hotfixVersion=%s]", hypervisorVersion, snapshotHotFixVersion));
             return defaultHostToExecuteCommands;
         }
         logger.debug(String.format("We are changing the hostId to executed command from %d to %d.", hostId, hostCandidateToExecutedCommand.getId()));

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/XenServerGuruTest.java

@@ -21,7 +21,6 @@
 import org.apache.cloudstack.storage.command.CopyCommand;
 import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.commons.lang.StringUtils;
 import org.junit.Assert;
 import org.junit.Before;
@@ -54,8 +53,6 @@ public class XenServerGuruTest {
     @Mock
     private PrimaryDataStoreDao storagePoolDao;
 
-    private Long defaultHostId = 1l;
-
     @Mock
     private CopyCommand copyCommandMock;
 
@@ -65,23 +62,30 @@ public class XenServerGuruTest {
     @Mock
     private DataTO destinationDataMock;
 
+    private Long defaultHostId = 1l;
+
     @Mock
-    private HostVO hostMock;
+    private HostVO defaultHost;
 
     @Mock
-    private StoragePoolVO storagePoolMock;
+    private HostVO changedHost;
 
     private Long changedHostId = 12l;
 
-    private long snapshotId = 5l;
+    private long zoneId = 100l;
 
     @Before
     public void beforeTest() {
+        Mockito.when(sourceDataMock.getHypervisorType()).thenReturn(HypervisorType.XenServer);
+
         Mockito.when(copyCommandMock.getSrcTO()).thenReturn(sourceDataMock);
         Mockito.when(copyCommandMock.getDestTO()).thenReturn(destinationDataMock);
-        Mockito.when(hostMock.getId()).thenReturn(changedHostId);
-        Mockito.when(sourceDataMock.getId()).thenReturn(snapshotId);
-        Mockito.when(storagePoolDao.findStoragePoolForSnapshot(snapshotId)).thenReturn(storagePoolMock);
+
+        Mockito.when(changedHost.getId()).thenReturn(changedHostId);
+        Mockito.when(defaultHost.getId()).thenReturn(defaultHostId);
+        Mockito.when(defaultHost.getDataCenterId()).thenReturn(zoneId);
+
+        Mockito.when(hostDaoMock.findById(defaultHostId)).thenReturn(changedHost);
     }
 
     @Test
@@ -162,8 +166,8 @@ public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervi
         configureSourceAndDestinationDataMockDataStoreAsNfsToType();
         configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
 
-        Mockito.when(hostMock.getHypervisorVersion()).thenReturn(StringUtils.EMPTY);
-        Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+        Mockito.when(changedHost.getHypervisorVersion()).thenReturn(StringUtils.EMPTY);
+        Mockito.when(hostDaoMock.findHostInZoneToExecuteCommand(zoneId, HypervisorType.XenServer)).thenReturn(changedHost);
 
         Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
 
@@ -181,8 +185,8 @@ public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervi
         configureSourceAndDestinationDataMockDataStoreAsNfsToType();
         configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
 
-        Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.1.0");
-        Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+        Mockito.when(changedHost.getHypervisorVersion()).thenReturn("6.1.0");
+        Mockito.when(hostDaoMock.findHostInZoneToExecuteCommand(zoneId, HypervisorType.XenServer)).thenReturn(changedHost);
 
         Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
 
@@ -194,8 +198,8 @@ public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervi
         configureSourceAndDestinationDataMockDataStoreAsNfsToType();
         configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
 
-        Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.2.0");
-        Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+        Mockito.when(changedHost.getHypervisorVersion()).thenReturn("6.2.0");
+        Mockito.when(hostDaoMock.findHostInZoneToExecuteCommand(zoneId, HypervisorType.XenServer)).thenReturn(changedHost);
 
         Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
 
@@ -207,10 +211,10 @@ public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervi
         configureSourceAndDestinationDataMockDataStoreAsNfsToType();
         configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
 
-        Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.2.0");
-        Mockito.when(hostMock.getDetail(XenserverConfigs.XS620HotFix)).thenReturn(XenserverConfigs.XSHotFix62ESP1004);
+        Mockito.when(changedHost.getHypervisorVersion()).thenReturn("6.2.0");
+        Mockito.when(changedHost.getDetail(XenserverConfigs.XS620HotFix)).thenReturn(XenserverConfigs.XSHotFix62ESP1004);
 
-        Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+        Mockito.when(hostDaoMock.findHostInZoneToExecuteCommand(zoneId, HypervisorType.XenServer)).thenReturn(changedHost);
 
         Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
 
@@ -223,9 +227,9 @@ public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervi
         configureSourceAndDestinationDataMockDataStoreAsNfsToType();
         configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
 
-        Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.5.0");
+        Mockito.when(changedHost.getHypervisorVersion()).thenReturn("6.5.0");
 
-        Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+        Mockito.when(hostDaoMock.findHostInZoneToExecuteCommand(zoneId, HypervisorType.XenServer)).thenReturn(changedHost);
 
         Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);