Skip to content

Commit 434df40

Browse files
Routed: get vm network statistics on Routed network (#13105)
* Routed: get vm network statistics on Routed network * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Create method isNetworkEligibleForNetworkStats and add unit tests --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent c467686 commit 434df40

2 files changed

Lines changed: 122 additions & 5 deletions

File tree

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

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
5858
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
5959
import org.apache.cloudstack.management.ManagementServerHost;
60+
import org.apache.cloudstack.network.RoutedIpv4Manager;
6061
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
6162
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
6263
import org.apache.cloudstack.utils.bytescale.ByteScaleUtils;
@@ -113,6 +114,8 @@
113114
import com.cloud.hypervisor.Hypervisor;
114115
import com.cloud.hypervisor.Hypervisor.HypervisorType;
115116
import com.cloud.network.as.AutoScaleManager;
117+
import com.cloud.network.dao.NetworkDao;
118+
import com.cloud.network.dao.NetworkVO;
116119
import com.cloud.org.Cluster;
117120
import com.cloud.resource.ResourceManager;
118121
import com.cloud.resource.ResourceState;
@@ -203,6 +206,11 @@
203206
@Component
204207
public class StatsCollector extends ManagerBase implements ComponentMethodInterceptable, Configurable, DbStatsCollection {
205208

209+
@Inject
210+
private NetworkDao networkDao;
211+
@Inject
212+
private RoutedIpv4Manager routedIpv4Manager;
213+
206214
public static enum ExternalStatsProtocol {
207215
NONE("none"), GRAPHITE("graphite"), INFLUXDB("influxdb");
208216
String _type;
@@ -268,9 +276,9 @@ public String toString() {
268276
private static final ConfigKey<Integer> vmDiskStatsIntervalMin = new ConfigKey<>("Advanced", Integer.class, "vm.disk.stats.interval.min", "300",
269277
"Minimal interval (in seconds) to report vm disk statistics. If vm.disk.stats.interval is smaller than this, use this to report vm disk statistics.", false);
270278
private static final ConfigKey<Integer> vmNetworkStatsInterval = new ConfigKey<>("Advanced", Integer.class, "vm.network.stats.interval", "0",
271-
"Interval (in seconds) to report vm network statistics (for Shared networks). Vm network statistics will be disabled if this is set to 0 or less than 0.", false);
279+
"Interval (in seconds) to report vm network statistics (for Shared and Routed networks). Vm network statistics will be disabled if this is set to 0 or less than 0.", false);
272280
private static final ConfigKey<Integer> vmNetworkStatsIntervalMin = new ConfigKey<>("Advanced", Integer.class, "vm.network.stats.interval.min", "300",
273-
"Minimal Interval (in seconds) to report vm network statistics (for Shared networks). If vm.network.stats.interval is smaller than this, use this to report vm network statistics.",
281+
"Minimal Interval (in seconds) to report vm network statistics (for Shared and Routed networks). If vm.network.stats.interval is smaller than this, use this to report vm network statistics.",
274282
false);
275283
private static final ConfigKey<Integer> StatsTimeout = new ConfigKey<>("Advanced", Integer.class, "stats.timeout", "60000",
276284
"The timeout for stats call in milli seconds.", true,
@@ -1601,9 +1609,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
16011609
SearchCriteria<NicVO> sc_nic = _nicDao.createSearchCriteria();
16021610
sc_nic.addAnd("macAddress", SearchCriteria.Op.EQ, vmNetworkStatEntry.getMacAddress());
16031611
NicVO nic = _nicDao.search(sc_nic, null).get(0);
1604-
List<VlanVO> vlan = _vlanDao.listVlansByNetworkId(nic.getNetworkId());
1605-
if (vlan == null || vlan.size() == 0 || vlan.get(0).getVlanType() != VlanType.DirectAttached)
1606-
continue; // only get network statistics for DirectAttached network (shared networks in Basic zone and Advanced zone with/without SG)
1612+
if (!isNetworkEligibleForNetworkStats(nic.getNetworkId())) {
1613+
continue; // only get network statistics for Shared or Routed network
1614+
}
16071615
UserStatisticsVO previousvmNetworkStats = _userStatsDao.findBy(userVm.getAccountId(), userVm.getDataCenterId(), nic.getNetworkId(),
16081616
nic.getIPv4Address(), vmId, "UserVm");
16091617
if (previousvmNetworkStats == null) {
@@ -2159,6 +2167,27 @@ protected boolean isCurrentVmDiskStatsDifferentFromPrevious(VmDiskStatisticsVO p
21592167
return true;
21602168
}
21612169

2170+
/**
2171+
* Returns {@code true} if the given network is eligible for VM network statistics collection.
2172+
* Only Shared (DirectAttached) networks and Routed networks qualify.
2173+
*
2174+
* @param networkId the network id to evaluate
2175+
* @return {@code true} when the network is routed or direct-attached, {@code false} otherwise
2176+
*/
2177+
protected boolean isNetworkEligibleForNetworkStats(Long networkId) {
2178+
if (networkId == null) {
2179+
return false;
2180+
}
2181+
List<VlanVO> vlans = _vlanDao.listVlansByNetworkId(networkId);
2182+
boolean isDirectAttachedNetwork = CollectionUtils.isNotEmpty(vlans)
2183+
&& vlans.get(0).getVlanType() == VlanType.DirectAttached;
2184+
if (isDirectAttachedNetwork) {
2185+
return true;
2186+
}
2187+
NetworkVO networkVO = networkDao.findById(networkId);
2188+
return networkVO != null && routedIpv4Manager.isRoutedNetwork(networkVO);
2189+
}
2190+
21622191
/**
21632192
* Returns true if all the VmDiskStatsEntry are Zeros (Bytes read, Bytes write, IO read, and IO write must be all equals to zero)
21642193
*/

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

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.text.SimpleDateFormat;
2727
import java.util.ArrayList;
2828
import java.util.Arrays;
29+
import java.util.Collections;
2930
import java.util.Date;
3031
import java.util.HashMap;
3132
import java.util.List;
@@ -38,6 +39,7 @@
3839
import com.cloud.utils.DateUtil;
3940
import com.google.gson.JsonSyntaxException;
4041
import org.apache.cloudstack.framework.config.ConfigKey;
42+
import org.apache.cloudstack.network.RoutedIpv4Manager;
4143
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
4244
import org.apache.commons.collections.CollectionUtils;
4345
import org.influxdb.InfluxDB;
@@ -64,7 +66,12 @@
6466
import com.cloud.agent.api.GetStorageStatsCommand;
6567
import com.cloud.agent.api.VmDiskStatsEntry;
6668
import com.cloud.agent.api.VmStatsEntry;
69+
import com.cloud.dc.Vlan.VlanType;
70+
import com.cloud.dc.VlanVO;
71+
import com.cloud.dc.dao.VlanDao;
6772
import com.cloud.hypervisor.Hypervisor;
73+
import com.cloud.network.dao.NetworkDao;
74+
import com.cloud.network.dao.NetworkVO;
6875
import com.cloud.server.StatsCollector.ExternalStatsProtocol;
6976
import com.cloud.storage.StorageStats;
7077
import com.cloud.storage.VolumeStatsVO;
@@ -118,6 +125,15 @@ public class StatsCollectorTest {
118125
@Mock
119126
private StoragePoolVO mockPool;
120127

128+
@Mock
129+
private RoutedIpv4Manager routedIpv4Manager;
130+
131+
@Mock
132+
private NetworkDao networkDao;
133+
134+
@Mock
135+
private VlanDao vlanDao;
136+
121137
private static Gson gson = new Gson();
122138

123139
private Gson msStatsGson;
@@ -744,6 +760,78 @@ public void testGsonDateFormatDeserializationWithDifferentDateFormat() throws Ex
744760
*/
745761
}
746762

763+
// -----------------------------------------------------------------------
764+
// Tests for isNetworkEligibleForNetworkStats
765+
// -----------------------------------------------------------------------
766+
767+
private VlanVO buildVlan(VlanType type) {
768+
VlanVO vlan = Mockito.mock(VlanVO.class);
769+
Mockito.when(vlan.getVlanType()).thenReturn(type);
770+
return vlan;
771+
}
772+
773+
@Test
774+
public void isNetworkEligibleForNetworkStats_RoutedNetwork_ReturnsTrue() {
775+
Long networkId = 1L;
776+
NetworkVO networkVO = Mockito.mock(NetworkVO.class);
777+
Mockito.when(networkDao.findById(networkId)).thenReturn(networkVO);
778+
Mockito.when(vlanDao.listVlansByNetworkId(networkId)).thenReturn(Collections.emptyList());
779+
Mockito.when(routedIpv4Manager.isRoutedNetwork(networkVO)).thenReturn(true);
780+
781+
Assert.assertTrue(statsCollector.isNetworkEligibleForNetworkStats(networkId));
782+
}
783+
784+
@Test
785+
public void isNetworkEligibleForNetworkStats_DirectAttachedNetwork_ReturnsTrue() {
786+
Long networkId = 1L;
787+
NetworkVO networkVO = Mockito.mock(NetworkVO.class);
788+
Mockito.when(networkDao.findById(networkId)).thenReturn(networkVO);
789+
Mockito.when(routedIpv4Manager.isRoutedNetwork(networkVO)).thenReturn(false);
790+
List<VlanVO> vlans = Collections.singletonList(buildVlan(VlanType.DirectAttached));
791+
Mockito.when(vlanDao.listVlansByNetworkId(networkId)).thenReturn(vlans);
792+
793+
Assert.assertTrue(statsCollector.isNetworkEligibleForNetworkStats(networkId));
794+
}
795+
796+
@Test
797+
public void isNetworkEligibleForNetworkStats_NeitherRoutedNorDirectAttached_ReturnsFalse() {
798+
Long networkId = 1L;
799+
NetworkVO networkVO = Mockito.mock(NetworkVO.class);
800+
Mockito.when(networkDao.findById(networkId)).thenReturn(networkVO);
801+
Mockito.when(routedIpv4Manager.isRoutedNetwork(networkVO)).thenReturn(false);
802+
List<VlanVO> vlans = Collections.singletonList(buildVlan(VlanType.VirtualNetwork));
803+
Mockito.when(vlanDao.listVlansByNetworkId(networkId)).thenReturn(vlans);
804+
805+
Assert.assertFalse(statsCollector.isNetworkEligibleForNetworkStats(networkId));
806+
}
807+
808+
@Test
809+
public void isNetworkEligibleForNetworkStats_NullNetworkAndEmptyVlans_ReturnsFalse() {
810+
Long networkId = 1L;
811+
Mockito.when(networkDao.findById(networkId)).thenReturn(null);
812+
Mockito.when(vlanDao.listVlansByNetworkId(networkId)).thenReturn(Collections.emptyList());
813+
814+
Assert.assertFalse(statsCollector.isNetworkEligibleForNetworkStats(networkId));
815+
}
816+
817+
@Test
818+
public void isNetworkEligibleForNetworkStats_NullNetworkButDirectAttached_ReturnsTrue() {
819+
Long networkId = 1L;
820+
Mockito.when(networkDao.findById(networkId)).thenReturn(null);
821+
List<VlanVO> vlans = Collections.singletonList(buildVlan(VlanType.DirectAttached));
822+
Mockito.when(vlanDao.listVlansByNetworkId(networkId)).thenReturn(vlans);
823+
824+
Assert.assertTrue(statsCollector.isNetworkEligibleForNetworkStats(networkId));
825+
Mockito.verify(routedIpv4Manager, Mockito.never()).isRoutedNetwork(Mockito.any());
826+
}
827+
828+
@Test
829+
public void isNetworkEligibleForNetworkStats_NullNetworkId_ReturnsFalse() {
830+
Assert.assertFalse(statsCollector.isNetworkEligibleForNetworkStats(null));
831+
Mockito.verify(networkDao, Mockito.never()).findById(Mockito.anyLong());
832+
Mockito.verify(vlanDao, Mockito.never()).listVlansByNetworkId(Mockito.anyLong());
833+
}
834+
747835
private static class TestClass {
748836
private String str;
749837
private int num;

0 commit comments

Comments
 (0)