ConfigureStorageAccessCmd: Allow storage access configuration on empt… (#578) (#13551)

* ConfigureStorageAccessCmd: Allow storage access configuration on empty clusters/pods/zones

* update as per comments



* update as per review comments



* update as per comments



---------



(cherry picked from commit 32736b9e262522912cf8755acabfb7400006252f)

Co-authored-by: Sachin R <32716246+sachindoddaguni@users.noreply.github.com>
Co-authored-by: Sachin R Doddaguni <s_rudrappadoddagu@apple.com>
Co-authored-by: mprokopchuk <mprokopchuk@gmail.com>

server/src/main/java/com/cloud/resource/ResourceManagerImpl.java

@@ -2465,7 +2465,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
             List<HostVO> hostsInZone = _hostDao.findByDataCenterId(zoneId);
             Set<Long> hostIdsInUseSet = hostIdsUsingStorageAccessGroups.stream().collect(Collectors.toSet());
 
-            boolean allInUseZone = hostsInZone.stream()
+            // allMatch returns true on empty stream, need to check whether collection is not empty first
+            boolean allInUseZone = !hostsInZone.isEmpty() && hostsInZone.stream()
                     .map(HostVO::getId)
                     .allMatch(hostIdsInUseSet::contains);
 
@@ -2479,7 +2480,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
             List<HostVO> hostsInCluster = _hostDao.findByClusterId(clusterId, Type.Routing);
             Set<Long> hostIdsInUseSet = hostIdsUsingStorageAccessGroups.stream().collect(Collectors.toSet());
 
-            boolean allInUseCluster = hostsInCluster.stream()
+            // allMatch returns true on empty stream, need to check whether collection is not empty first
+            boolean allInUseCluster = !hostsInCluster.isEmpty() && hostsInCluster.stream()
                     .map(HostVO::getId)
                     .allMatch(hostIdsInUseSet::contains);
 
@@ -2493,7 +2495,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
             List<HostVO> hostsInPod = _hostDao.findByPodId(podId, Type.Routing);
             Set<Long> hostIdsInUseSet = hostIdsUsingStorageAccessGroups.stream().collect(Collectors.toSet());
 
-            boolean allInUsePod = hostsInPod.stream()
+            // allMatch returns true on empty stream, need to check whether collection is not empty first
+            boolean allInUsePod = !hostsInPod.isEmpty() && hostsInPod.stream()
                     .map(HostVO::getId)
                     .allMatch(hostIdsInUseSet::contains);
 

server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

@@ -1211,4 +1211,96 @@ public class ResourceManagerImplTest {
 
         Mockito.verify(resourceManager).doDeleteHost(hostId, false, false);
     }
+
+    @Test
+    public void testUpdateClusterStorageAccessGroupsWithEmptyHostsInCluster() {
+        Long clusterId = 1L;
+        List<String> newStorageAccessGroups = Arrays.asList("sag1", "sag2");
+
+        ClusterVO cluster = Mockito.mock(ClusterVO.class);
+        Mockito.when(cluster.getId()).thenReturn(clusterId);
+        Mockito.when(cluster.getStorageAccessGroups()).thenReturn("sag3,sag4"); // existing SAGs
+        Mockito.when(resourceManager.getCluster(clusterId)).thenReturn(cluster);
+        List<HostVO> emptyHostsList = new ArrayList<>();
+        Mockito.when(hostDao.findHypervisorHostInCluster(clusterId)).thenReturn(emptyHostsList);
+        Mockito.when(hostDao.findByClusterId(clusterId, Host.Type.Routing)).thenReturn(emptyHostsList);
+        List<Long> emptyHostIdsList = new ArrayList<>();
+        Mockito.doReturn(emptyHostIdsList).when(resourceManager)
+                .listOfHostIdsUsingTheStorageAccessGroups(Mockito.anyList(), eq(clusterId), eq(null), eq(null));
+        try {
+            resourceManager.updateClusterStorageAccessGroups(clusterId, newStorageAccessGroups);
+        } catch (CloudRuntimeException e) {
+            Assert.fail("updateClusterStorageAccessGroups should not throw CloudRuntimeException when cluster has no hosts. Error: " + e.getMessage());
+        }
+        Mockito.verify(resourceManager).checkIfAllHostsInUse(Mockito.anyList(), eq(clusterId), eq(null), eq(null));
+    }
+
+    @Test
+    public void testUpdateClusterStorageAccessGroupsWithEmptyHostsInZone() {
+        List<String> sagsToDelete = Arrays.asList("tag1", "tag2");
+        Long clusterId = null;
+        Long podId = null;
+        Long zoneId = 3L;
+
+        List<Long> emptyHostIdsList = new ArrayList<>();
+        Mockito.doReturn(emptyHostIdsList).when(resourceManager)
+                .listOfHostIdsUsingTheStorageAccessGroups(sagsToDelete, clusterId, podId, zoneId);
+        List<HostVO> emptyHostsInZone = new ArrayList<>();
+        Mockito.doReturn(emptyHostsInZone).when(hostDao).findByDataCenterId(zoneId);
+
+        try {
+            resourceManager.checkIfAllHostsInUse(sagsToDelete, clusterId, podId, zoneId);
+        } catch (CloudRuntimeException e) {
+            Assert.fail("checkIfAllHostsInUse should not throw CloudRuntimeException when zone has no hosts. Error: " + e.getMessage());
+        }
+        Mockito.verify(resourceManager).checkIfAllHostsInUse(Mockito.anyList(), eq(null), eq(null), eq(zoneId));
+    }
+
+    @Test
+    public void testUpdateClusterStorageAccessGroupsWithEmptyHostsInPod() {
+        List<String> sagsToDelete = Arrays.asList("tag1", "tag2");
+        Long clusterId = null;
+        Long podId = 2L;
+        Long zoneId = null;
+
+        List<Long> emptyHostIdsList = new ArrayList<>();
+        Mockito.doReturn(emptyHostIdsList).when(resourceManager)
+                .listOfHostIdsUsingTheStorageAccessGroups(sagsToDelete, clusterId, podId, zoneId);
+        List<HostVO> emptyHostsInPod = new ArrayList<>();
+        Mockito.doReturn(emptyHostsInPod).when(hostDao).findByPodId(podId, Host.Type.Routing);
+
+        try {
+            resourceManager.checkIfAllHostsInUse(sagsToDelete, clusterId, podId, zoneId);
+        } catch (CloudRuntimeException e) {
+            Assert.fail("checkIfAllHostsInUse should not throw CloudRuntimeException when pod has no hosts. Error: " + e.getMessage());
+        }
+        Mockito.verify(resourceManager).checkIfAllHostsInUse(Mockito.anyList(), eq(null), eq(podId), eq(null));
+    }
+
+    @Test
+    public void testCheckIfAllHostsInUseWithEmptyHostsInMultipleLevels() {
+        List<String> sagsToDelete = Arrays.asList("tag1", "tag2");
+        Long clusterId = 1L;
+        Long podId = 2L;
+        Long zoneId = 3L;
+
+        List<Long> emptyHostIdsList = new ArrayList<>();
+        Mockito.doReturn(emptyHostIdsList).when(resourceManager)
+                .listOfHostIdsUsingTheStorageAccessGroups(sagsToDelete, clusterId, podId, zoneId);
+        List<HostVO> emptyHostsInZone = new ArrayList<>();
+        List<HostVO> emptyHostsInCluster = new ArrayList<>();
+        List<HostVO> emptyHostsInPod = new ArrayList<>();
+        Mockito.doReturn(emptyHostsInZone).when(hostDao).findByDataCenterId(zoneId);
+        Mockito.doReturn(emptyHostsInCluster).when(hostDao).findByClusterId(clusterId, Host.Type.Routing);
+        Mockito.doReturn(emptyHostsInPod).when(hostDao).findByPodId(podId, Host.Type.Routing);
+
+        try {
+            resourceManager.checkIfAllHostsInUse(sagsToDelete, clusterId, podId, zoneId);
+        } catch (CloudRuntimeException e) {
+            Assert.fail("checkIfAllHostsInUse should not throw CloudRuntimeException when all levels have no hosts. Error: " + e.getMessage());
+        }
+        Mockito.verify(hostDao).findByDataCenterId(zoneId);
+        Mockito.verify(hostDao).findByClusterId(clusterId, Host.Type.Routing);
+        Mockito.verify(hostDao).findByPodId(podId, Host.Type.Routing);
+    }
 }