Liron Aravot has uploaded a new change for review.

Change subject: core: hosts from gluster clusters shouldn't be considered
......................................................................

core: hosts from gluster clusters shouldn't be considered

When moving a domain to maintenance or when handling problematic
domain, hosts from gluster clusters were considered although we don't
use any monitoring from those hosts which prevents hosts from moving to
maintenance or from properly handle a problematic domain.

This patch fixes it by loading only the hosts from virt service (which
we collect domain monitoring information from) instead of loading all
the hosts.

Change-Id: I2b89785529ec00114065e350138e05d6c966df44
Bug-Url: https://bugzilla.redhat.com/1186687
Signed-off-by: Liron Aravot <[email protected]>
---
M 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java
M 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java
M 
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java
M packaging/dbscripts/vds_sp.sql
6 files changed, 30 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/75/37375/1

diff --git 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java
 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java
index 12bf647..3a9055e 100644
--- 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java
+++ 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java
@@ -158,9 +158,9 @@
 
     /**
      * Retrieves all VDS instances by storage pool ID and status.
-     *
+     * NOTE- only hosts from VIRT service supporting cluster are returned
      * @param storagePool The storage pool's ID
-     * @param status The status of vds
+     * @param status The status of vds, null for all statuses
      * @return the list of VDS instances
      */
     List<VDS> getAllForStoragePoolAndStatus(Guid storagePool, VDSStatus 
status);
diff --git 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java
 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java
index 33bfe6a..90650b7 100644
--- 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java
+++ 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java
@@ -187,7 +187,7 @@
                 VdsRowMapper.instance,
                 getCustomMapSqlParameterSource()
                         .addValue("storage_pool_id", storagePool)
-                        .addValue("status", status.getValue()));
+                        .addValue("status", status != null ? status.getValue() 
: null));
     }
 
     @Override
diff --git 
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java
 
b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java
index b7e6530..3973fa9 100644
--- 
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java
+++ 
b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java
@@ -9,10 +9,12 @@
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
+import java.util.EnumSet;
 import java.util.List;
 
 import org.junit.Test;
 import org.ovirt.engine.core.common.businessentities.VDS;
+import org.ovirt.engine.core.common.businessentities.VDSStatus;
 import org.ovirt.engine.core.common.businessentities.VDSType;
 import org.ovirt.engine.core.compat.Guid;
 
@@ -329,10 +331,23 @@
 
     @Test
     public void testGetAllForStoragePoolAndStatus() {
+        assertNotNull(existingVds.getStatus());
         List<VDS> result = 
dao.getAllForStoragePoolAndStatus(existingVds.getStoragePoolId(), 
existingVds.getStatus());
         assertCorrectGetAllResult(result);
     }
 
+    @Test
+    public void testGetAllForStoragePoolAndStatusForAllStatuses() {
+        dbFacade.getVdsDynamicDao().updateStatus(existingVds.getId(), 
VDSStatus.Maintenance);
+        List<VDS> result = 
dao.getAllForStoragePoolAndStatus(existingVds.getStoragePoolId(), null);
+        EnumSet<VDSStatus> statuses = EnumSet.noneOf(VDSStatus.class);
+        for (VDS vds : result) {
+            statuses.add(vds.getStatus());
+        }
+        assertCorrectGetAllResult(result);
+        assertTrue("more then one different status expected", statuses.size() 
> 1);
+    }
+
     private void assertGetAllForVdsGroupCorrectResult(List<VDS> result) {
         assertNotNull(result);
         assertFalse(result.isEmpty());
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
index 286ca3b..ec1073e 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
@@ -54,6 +54,9 @@
      */
     public static void updateVdsDomainsData(VDS vds, Guid storagePoolId,
             ArrayList<VDSDomainsData> vdsDomainData) {
+        // NOTE - if this condition is ever updated, every place that acts 
upon the reporting
+        // should be updated as well, only hosts the we collect the report 
from should be affected
+        // from it.
         if (vds.getStatus() == reportingVdsStatus && 
vds.getVdsGroupSupportsVirtService()) {
             IrsProxyData proxy = _irsProxyData.get(storagePoolId);
             if (proxy != null) {
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java
index d9327c4..caa1a36 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java
@@ -199,7 +199,10 @@
     private static Set<Guid> getVdsConnectedToPool(Guid storagePoolId) {
         Set<Guid> vdsNotInMaintenance = new HashSet<>();
 
-        for (VDS vds : 
DbFacade.getInstance().getVdsDao().getAllForStoragePool(storagePoolId)) {
+        // Note - this method is used as it returns only hosts from VIRT 
supported clusters
+        // (we use the domain monitoring results only from those clusters 
hosts).
+        // every change to it should be inspected carefully.
+        for (VDS vds : 
DbFacade.getInstance().getVdsDao().getAllForStoragePoolAndStatus(storagePoolId, 
null)) {
             if (vds.getStatus() == VDSStatus.Up
                     || vds.getStatus() == VDSStatus.NonResponsive
                     || vds.getStatus() == VDSStatus.PreparingForMaintenance
@@ -1506,7 +1509,10 @@
         // build a list of all the hosts in status UP in
         // Pool.
         List<Guid> vdssInPool = new ArrayList<Guid>();
-        List<VDS> allVds = 
DbFacade.getInstance().getVdsDao().getAllForStoragePool(_storagePoolId);
+        // Note - this method is used as it returns only hosts from VIRT 
supported clusters
+        // (we use the domain monitoring results only from those clusters 
hosts).
+        // every change to it should be inspected carefully.
+        List<VDS> allVds = 
DbFacade.getInstance().getVdsDao().getAllForStoragePoolAndStatus(_storagePoolId,
 null);
         Map<Guid, VDS> vdsMap = new HashMap<Guid, VDS>();
         for (VDS tempVDS : allVds) {
             vdsMap.put(tempVDS.getId(), tempVDS);
diff --git a/packaging/dbscripts/vds_sp.sql b/packaging/dbscripts/vds_sp.sql
index 5e85a43..a4e4575 100644
--- a/packaging/dbscripts/vds_sp.sql
+++ b/packaging/dbscripts/vds_sp.sql
@@ -898,7 +898,7 @@
         RETURN QUERY SELECT vds.*
         FROM vds
         INNER JOIN vds_groups vdsgroup ON vds.vds_group_id = 
vdsgroup.vds_group_id
-        WHERE (vds.status = v_status) AND (vds.storage_pool_id = 
v_storage_pool_id)
+        WHERE (v_status IS NULL OR vds.status = v_status) AND 
(vds.storage_pool_id = v_storage_pool_id)
         AND vdsgroup.virt_service = true;
     END;
     RETURN;


-- 
To view, visit http://gerrit.ovirt.org/37375
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2b89785529ec00114065e350138e05d6c966df44
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.5
Gerrit-Owner: Liron Aravot <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to