Create method isNetworkEligibleForNetworkStats and add unit tests

Author: weizhouapache

server/src/main/java/com/cloud/server/StatsCollector.java

@@ -1589,12 +1589,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
                                     SearchCriteria<NicVO> sc_nic = _nicDao.createSearchCriteria();
                                     sc_nic.addAnd("macAddress", SearchCriteria.Op.EQ, vmNetworkStatEntry.getMacAddress());
                                     NicVO nic = _nicDao.search(sc_nic, null).get(0);
-                                    List<VlanVO> vlan = _vlanDao.listVlansByNetworkId(nic.getNetworkId());
-                                    NetworkVO networkVO = networkDao.findById(nic.getNetworkId());
-                                    boolean isRoutedNetwork = networkVO != null && routedIpv4Manager.isRoutedNetwork(networkVO);
-                                    boolean isDirectAttachedNetwork = CollectionUtils.isNotEmpty(vlan)
-                                            && vlan.get(0).getVlanType() == VlanType.DirectAttached;
-                                    if (!isRoutedNetwork && !isDirectAttachedNetwork) {
+                                    if (!isNetworkEligibleForNetworkStats(nic.getNetworkId())) {
                                         continue; // only get network statistics for Shared or Routed network
                                     }
                                     UserStatisticsVO previousvmNetworkStats = _userStatsDao.findBy(userVm.getAccountId(), userVm.getDataCenterId(), nic.getNetworkId(),
@@ -2152,6 +2147,27 @@ protected boolean isCurrentVmDiskStatsDifferentFromPrevious(VmDiskStatisticsVO p
         return true;
     }
 
+    /**
+     * Returns {@code true} if the given network is eligible for VM network statistics collection.
+     * Only Shared (DirectAttached) networks and Routed networks qualify.
+     *
+     * @param networkId the network id to evaluate
+     * @return {@code true} when the network is routed or direct-attached, {@code false} otherwise
+     */
+    protected boolean isNetworkEligibleForNetworkStats(Long networkId) {
+        if (networkId == null) {
+            return false;
+        }
+        List<VlanVO> vlans = _vlanDao.listVlansByNetworkId(networkId);
+        boolean isDirectAttachedNetwork = CollectionUtils.isNotEmpty(vlans)
+                && vlans.get(0).getVlanType() == VlanType.DirectAttached;
+        if (isDirectAttachedNetwork) {
+            return true;
+        }
+        NetworkVO networkVO = networkDao.findById(networkId);
+        return networkVO != null && routedIpv4Manager.isRoutedNetwork(networkVO);
+    }
+
     /**
      * Returns true if all the VmDiskStatsEntry are Zeros (Bytes read, Bytes write, IO read, and IO write must be all equals to zero)
      */

server/src/test/java/com/cloud/server/StatsCollectorTest.java

@@ -26,6 +26,7 @@
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
@@ -38,6 +39,7 @@
 import com.cloud.utils.DateUtil;
 import com.google.gson.JsonSyntaxException;
 import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.network.RoutedIpv4Manager;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.commons.collections.CollectionUtils;
 import org.influxdb.InfluxDB;
@@ -64,7 +66,12 @@
 import com.cloud.agent.api.GetStorageStatsCommand;
 import com.cloud.agent.api.VmDiskStatsEntry;
 import com.cloud.agent.api.VmStatsEntry;
+import com.cloud.dc.Vlan.VlanType;
+import com.cloud.dc.VlanVO;
+import com.cloud.dc.dao.VlanDao;
 import com.cloud.hypervisor.Hypervisor;
+import com.cloud.network.dao.NetworkDao;
+import com.cloud.network.dao.NetworkVO;
 import com.cloud.server.StatsCollector.ExternalStatsProtocol;
 import com.cloud.storage.StorageStats;
 import com.cloud.storage.VolumeStatsVO;
@@ -118,6 +125,15 @@ public class StatsCollectorTest {
     @Mock
     private StoragePoolVO mockPool;
 
+    @Mock
+    private RoutedIpv4Manager routedIpv4Manager;
+
+    @Mock
+    private NetworkDao networkDao;
+
+    @Mock
+    private VlanDao vlanDao;
+
     private static Gson gson = new Gson();
 
     private Gson msStatsGson;
@@ -701,6 +717,78 @@ public void testGsonDateFormatDeserializationWithDifferentDateFormat() throws Ex
          */
     }
 
+    // -----------------------------------------------------------------------
+    // Tests for isNetworkEligibleForNetworkStats
+    // -----------------------------------------------------------------------
+
+    private VlanVO buildVlan(VlanType type) {
+        VlanVO vlan = Mockito.mock(VlanVO.class);
+        Mockito.when(vlan.getVlanType()).thenReturn(type);
+        return vlan;
+    }
+
+    @Test
+    public void isNetworkEligibleForNetworkStats_RoutedNetwork_ReturnsTrue() {
+        Long networkId = 1L;
+        NetworkVO networkVO = Mockito.mock(NetworkVO.class);
+        Mockito.when(networkDao.findById(networkId)).thenReturn(networkVO);
+        Mockito.when(vlanDao.listVlansByNetworkId(networkId)).thenReturn(Collections.emptyList());
+        Mockito.when(routedIpv4Manager.isRoutedNetwork(networkVO)).thenReturn(true);
+
+        Assert.assertTrue(statsCollector.isNetworkEligibleForNetworkStats(networkId));
+    }
+
+    @Test
+    public void isNetworkEligibleForNetworkStats_DirectAttachedNetwork_ReturnsTrue() {
+        Long networkId = 1L;
+        NetworkVO networkVO = Mockito.mock(NetworkVO.class);
+        Mockito.when(networkDao.findById(networkId)).thenReturn(networkVO);
+        Mockito.when(routedIpv4Manager.isRoutedNetwork(networkVO)).thenReturn(false);
+        List<VlanVO> vlans = Collections.singletonList(buildVlan(VlanType.DirectAttached));
+        Mockito.when(vlanDao.listVlansByNetworkId(networkId)).thenReturn(vlans);
+
+        Assert.assertTrue(statsCollector.isNetworkEligibleForNetworkStats(networkId));
+    }
+
+    @Test
+    public void isNetworkEligibleForNetworkStats_NeitherRoutedNorDirectAttached_ReturnsFalse() {
+        Long networkId = 1L;
+        NetworkVO networkVO = Mockito.mock(NetworkVO.class);
+        Mockito.when(networkDao.findById(networkId)).thenReturn(networkVO);
+        Mockito.when(routedIpv4Manager.isRoutedNetwork(networkVO)).thenReturn(false);
+        List<VlanVO> vlans = Collections.singletonList(buildVlan(VlanType.VirtualNetwork));
+        Mockito.when(vlanDao.listVlansByNetworkId(networkId)).thenReturn(vlans);
+
+        Assert.assertFalse(statsCollector.isNetworkEligibleForNetworkStats(networkId));
+    }
+
+    @Test
+    public void isNetworkEligibleForNetworkStats_NullNetworkAndEmptyVlans_ReturnsFalse() {
+        Long networkId = 1L;
+        Mockito.when(networkDao.findById(networkId)).thenReturn(null);
+        Mockito.when(vlanDao.listVlansByNetworkId(networkId)).thenReturn(Collections.emptyList());
+
+        Assert.assertFalse(statsCollector.isNetworkEligibleForNetworkStats(networkId));
+    }
+
+    @Test
+    public void isNetworkEligibleForNetworkStats_NullNetworkButDirectAttached_ReturnsTrue() {
+        Long networkId = 1L;
+        Mockito.when(networkDao.findById(networkId)).thenReturn(null);
+        List<VlanVO> vlans = Collections.singletonList(buildVlan(VlanType.DirectAttached));
+        Mockito.when(vlanDao.listVlansByNetworkId(networkId)).thenReturn(vlans);
+
+        Assert.assertTrue(statsCollector.isNetworkEligibleForNetworkStats(networkId));
+        Mockito.verify(routedIpv4Manager, Mockito.never()).isRoutedNetwork(Mockito.any());
+    }
+
+    @Test
+    public void isNetworkEligibleForNetworkStats_NullNetworkId_ReturnsFalse() {
+        Assert.assertFalse(statsCollector.isNetworkEligibleForNetworkStats(null));
+        Mockito.verify(networkDao, Mockito.never()).findById(Mockito.anyLong());
+        Mockito.verify(vlanDao, Mockito.never()).listVlansByNetworkId(Mockito.anyLong());
+    }
+
     private static class TestClass {
         private String str;
         private int num;