From 07dd62ea0a684b5af6efa4fe09fd67fcd0f08981 Mon Sep 17 00:00:00 2001 From: dhslove <44049806+dhslove@users.noreply.github.com> Date: Mon, 1 Jun 2026 07:50:27 +0900 Subject: [PATCH] Fix volume stats task null handling Skip volume stats collection for volumes whose format is unavailable instead of allowing List.of(...).contains(null) to throw. Also guard missing pool metadata in the volume stats path. --- .../java/com/cloud/server/StatsCollector.java | 31 ++++++-- .../java/com/cloud/vm/UserVmManagerImpl.java | 12 ++- .../com/cloud/server/StatsCollectorTest.java | 73 +++++++++++++++++++ 3 files changed, 109 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 344c8e4c9d36..861b85bd3d4a 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1929,13 +1929,29 @@ protected void runInContext() { continue; } List volumes = _volsDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); + if (CollectionUtils.isEmpty(volumes)) { + continue; + } + boolean unsupportedVolumeFormat = false; for (VolumeVO volume : volumes) { - if (!List.of(ImageFormat.QCOW2, ImageFormat.VHD, ImageFormat.OVA, ImageFormat.RAW).contains(volume.getFormat()) && - !List.of(Storage.StoragePoolType.PowerFlex, Storage.StoragePoolType.FiberChannel).contains(pool.getPoolType())) { - logger.warn("Volume stats not implemented for this format type " + volume.getFormat()); + ImageFormat volumeFormat = volume.getFormat(); + Storage.StoragePoolType poolType = pool.getPoolType(); + boolean poolTypeSupportsVolumeStats = poolType != null && List.of(Storage.StoragePoolType.PowerFlex, Storage.StoragePoolType.FiberChannel).contains(poolType); + boolean volumeFormatSupportsVolumeStats = volumeFormat != null && List.of(ImageFormat.QCOW2, ImageFormat.VHD, ImageFormat.OVA, ImageFormat.RAW).contains(volumeFormat); + if (volumeFormat == null && !poolTypeSupportsVolumeStats) { + logger.debug("Skipping volume stats for volume {} on storage pool {} as volume format is unavailable", volume.getUuid(), pool.getUuid()); + unsupportedVolumeFormat = true; + break; + } + if (!volumeFormatSupportsVolumeStats && !poolTypeSupportsVolumeStats) { + logger.warn("Volume stats not implemented for this format type " + volumeFormat); + unsupportedVolumeFormat = true; break; } } + if (unsupportedVolumeFormat) { + continue; + } try { Map volumeStatsByUuid; if (pool.getScope() == ScopeType.ZONE) { @@ -1947,7 +1963,12 @@ protected void runInContext() { } } } else { - volumeStatsByUuid = _userVmMgr.getVolumeStatistics(pool.getClusterId(), pool.getUuid(), pool.getPoolType(), StatsTimeout.value()); + Long clusterId = pool.getClusterId(); + if (clusterId == null) { + logger.debug("Skipping volume stats for storage pool {} as scope {} has no cluster id", pool.getUuid(), pool.getScope()); + continue; + } + volumeStatsByUuid = _userVmMgr.getVolumeStatistics(clusterId, pool.getUuid(), pool.getPoolType(), StatsTimeout.value()); } if (volumeStatsByUuid != null) { for (final Map.Entry entry : volumeStatsByUuid.entrySet()) { @@ -2486,4 +2507,4 @@ public ConfigKey[] getConfigKeys() { public double getImageStoreCapacityThreshold() { return CapacityManager.SecondaryStorageCapacityThreshold.value(); } -} \ No newline at end of file +} diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index ffc343d0e45f..b3c1b03d24d7 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2431,8 +2431,16 @@ private void changeDiskOfferingForRootVolume(Long vmId, DiskOfferingVO newDiskOf @Override public HashMap getVolumeStatistics(long clusterId, String poolUuid, StoragePoolType poolType, int timeout) { - List neighbors = _resourceMgr.listHostsInClusterByStatus(clusterId, Status.Up); StoragePoolVO storagePool = _storagePoolDao.findPoolByUUID(poolUuid); + if (storagePool == null) { + logger.debug("Unable to collect volume stats as storage pool {} was not found", poolUuid); + return null; + } + List neighbors = _resourceMgr.listHostsInClusterByStatus(clusterId, Status.Up); + if (CollectionUtils.isEmpty(neighbors)) { + logger.debug("Unable to collect volume stats for storage pool {} as cluster {} has no Up hosts", poolUuid, clusterId); + return null; + } HashMap volumeStatsByUuid = new HashMap<>(); for (HostVO neighbor : neighbors) { @@ -2451,7 +2459,7 @@ public HashMap getVolumeStatistics(long clusterId, Str DataStoreProvider storeProvider = _dataStoreProviderMgr .getDataStoreProvider(storagePool.getStorageProviderName()); - DataStoreDriver storeDriver = storeProvider.getDataStoreDriver(); + DataStoreDriver storeDriver = storeProvider != null ? storeProvider.getDataStoreDriver() : null; if (storeDriver instanceof PrimaryDataStoreDriver && ((PrimaryDataStoreDriver) storeDriver).canProvideVolumeStats()) { // Get volume stats from the pool directly instead of sending cmd to host diff --git a/server/src/test/java/com/cloud/server/StatsCollectorTest.java b/server/src/test/java/com/cloud/server/StatsCollectorTest.java index 3709f732f045..72d4f533000c 100644 --- a/server/src/test/java/com/cloud/server/StatsCollectorTest.java +++ b/server/src/test/java/com/cloud/server/StatsCollectorTest.java @@ -38,6 +38,7 @@ import com.cloud.utils.DateUtil; import com.google.gson.JsonSyntaxException; import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.commons.collections.CollectionUtils; import org.influxdb.InfluxDB; @@ -66,11 +67,16 @@ import com.cloud.agent.api.VmStatsEntry; import com.cloud.hypervisor.Hypervisor; import com.cloud.server.StatsCollector.ExternalStatsProtocol; +import com.cloud.storage.ScopeType; +import com.cloud.storage.Storage; import com.cloud.storage.StorageStats; import com.cloud.storage.VolumeStatsVO; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.dao.VolumeStatsDao; import com.cloud.user.VmDiskStatisticsVO; import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.UserVmManager; import com.cloud.vm.VmStats; import com.cloud.vm.VmStatsVO; import com.cloud.vm.dao.VmStatsDaoImpl; @@ -115,6 +121,15 @@ public class StatsCollectorTest { @Mock VolumeStatsDao volumeStatsDao = Mockito.mock(VolumeStatsDao.class); + @Mock + PrimaryDataStoreDao storagePoolDao = Mockito.mock(PrimaryDataStoreDao.class); + + @Mock + VolumeDao volumeDao = Mockito.mock(VolumeDao.class); + + @Mock + UserVmManager userVmManager = Mockito.mock(UserVmManager.class); + @Mock private StoragePoolVO mockPool; @@ -131,6 +146,9 @@ public void setUp() throws Exception { closeable = MockitoAnnotations.openMocks(this); statsCollector.vmStatsDao = vmStatsDaoMock; statsCollector.volumeStatsDao = volumeStatsDao; + ReflectionTestUtils.setField(statsCollector, "_storagePoolDao", storagePoolDao); + ReflectionTestUtils.setField(statsCollector, "_volsDao", volumeDao); + ReflectionTestUtils.setField(statsCollector, "_userVmMgr", userVmManager); Field msStatsGsonField = StatsCollector.class.getDeclaredField("msStatsGson"); msStatsGsonField.setAccessible(true); msStatsGson = (Gson) msStatsGsonField.get(null); @@ -622,6 +640,61 @@ public void testPoolNeedsIopsStatsUpdating_NullIops() { Mockito.verify(mockPool, Mockito.never()).setUsedIops(Mockito.anyLong()); } + @Test + public void volumeStatsTaskSkipsEmptyPools() { + configureStoragePool(1L, "pool-uuid", ScopeType.CLUSTER, 1L, Storage.StoragePoolType.NetworkFilesystem); + when(volumeDao.findNonDestroyedVolumesByPoolId(1L, null)).thenReturn(new ArrayList<>()); + + statsCollector.new VolumeStatsTask().runInContext(); + + Mockito.verify(userVmManager, Mockito.never()).getVolumeStatistics(Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyInt()); + } + + @Test + public void volumeStatsTaskSkipsNonZonePoolsWithoutClusterId() { + configureStoragePool(1L, "pool-uuid", ScopeType.CLUSTER, null, Storage.StoragePoolType.NetworkFilesystem); + VolumeVO volume = Mockito.mock(VolumeVO.class); + when(volume.getFormat()).thenReturn(Storage.ImageFormat.QCOW2); + when(volumeDao.findNonDestroyedVolumesByPoolId(1L, null)).thenReturn(Arrays.asList(volume)); + + statsCollector.new VolumeStatsTask().runInContext(); + + Mockito.verify(userVmManager, Mockito.never()).getVolumeStatistics(Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyInt()); + } + + @Test + public void volumeStatsTaskSkipsVolumesWithoutFormat() { + configureStoragePool(1L, "pool-uuid", ScopeType.CLUSTER, 1L, Storage.StoragePoolType.NetworkFilesystem); + VolumeVO volume = Mockito.mock(VolumeVO.class); + when(volume.getFormat()).thenReturn(null); + when(volumeDao.findNonDestroyedVolumesByPoolId(1L, null)).thenReturn(Arrays.asList(volume)); + + statsCollector.new VolumeStatsTask().runInContext(); + + Mockito.verify(userVmManager, Mockito.never()).getVolumeStatistics(Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyInt()); + } + + @Test + public void volumeStatsTaskSkipsVolumesWithoutPoolType() { + configureStoragePool(1L, "pool-uuid", ScopeType.CLUSTER, 1L, null); + VolumeVO volume = Mockito.mock(VolumeVO.class); + when(volume.getFormat()).thenReturn(null); + when(volumeDao.findNonDestroyedVolumesByPoolId(1L, null)).thenReturn(Arrays.asList(volume)); + + statsCollector.new VolumeStatsTask().runInContext(); + + Mockito.verify(userVmManager, Mockito.never()).getVolumeStatistics(Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyInt()); + } + + private void configureStoragePool(long poolId, String uuid, ScopeType scope, Long clusterId, Storage.StoragePoolType poolType) { + when(storagePoolDao.listAll()).thenReturn(Arrays.asList(mockPool)); + when(mockPool.getId()).thenReturn(poolId); + when(mockPool.getUuid()).thenReturn(uuid); + when(mockPool.getScope()).thenReturn(scope); + when(mockPool.getClusterId()).thenReturn(clusterId); + when(mockPool.getPoolType()).thenReturn(poolType); + } + @Test public void testGsonDateFormatSerialization() { Date now = new Date();