server: Fix network statistics for vpc (#3944)

This contains 3 main changes
(1) add NETWORK_STATS_ethX for all nics with public ips in VPC VRs (current: NETWORK_STATS_eth1)
(2) DO NOT create records in user_statistics for each VPC tier (only one record per public nic per VPC VR)
(3) send NetworkUsageCommand before unplugging a NIC with public IPs from VPC VR

api/src/main/java/com/cloud/network/VirtualNetworkApplianceService.java

@@ -26,6 +26,7 @@ import com.cloud.exception.InsufficientCapacityException;
 import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.network.router.VirtualRouter;
 import com.cloud.user.Account;
+import com.cloud.vm.Nic;
 
 public interface VirtualNetworkApplianceService {
     /**
@@ -74,4 +75,6 @@ public interface VirtualNetworkApplianceService {
      * @return
      */
     boolean performRouterHealthChecks(long routerId);
+
+    <T extends VirtualRouter> void collectNetworkStatistics(T router, Nic nic);
 }

engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java

@@ -383,12 +383,14 @@ public class DomainRouterDaoImpl extends GenericDaoBase<DomainRouterVO, Long> im
                 final RouterNetworkVO routerNtwkMap = new RouterNetworkVO(router.getId(), guestNetwork.getId(), guestNetwork.getGuestType());
                 _routerNetworkDao.persist(routerNtwkMap);
                 //2) create user stats entry for the network
-                UserStatisticsVO stats =
-                        _userStatsDao.findBy(router.getAccountId(), router.getDataCenterId(), guestNetwork.getId(), null, router.getId(), router.getType().toString());
-                if (stats == null) {
-                    stats =
-                            new UserStatisticsVO(router.getAccountId(), router.getDataCenterId(), null, router.getId(), router.getType().toString(), guestNetwork.getId());
-                    _userStatsDao.persist(stats);
+                if (router.getVpcId() == null) {
+                    UserStatisticsVO stats =
+                            _userStatsDao.findBy(router.getAccountId(), router.getDataCenterId(), guestNetwork.getId(), null, router.getId(), router.getType().toString());
+                    if (stats == null) {
+                        stats =
+                                new UserStatisticsVO(router.getAccountId(), router.getDataCenterId(), null, router.getId(), router.getType().toString(), guestNetwork.getId());
+                        _userStatsDao.persist(stats);
+                    }
                 }
                 txn.commit();
             }

server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java

@@ -732,100 +732,7 @@ Configurable, StateListener<VirtualMachine.State, VirtualMachine.Event, VirtualM
                 s_logger.debug("Found " + routers.size() + " running routers. ");
 
                 for (final DomainRouterVO router : routers) {
-                    final String privateIP = router.getPrivateIpAddress();
-
-                    if (privateIP != null) {
-                        final boolean forVpc = router.getVpcId() != null;
-                        final List<? extends Nic> routerNics = _nicDao.listByVmId(router.getId());
-                        for (final Nic routerNic : routerNics) {
-                            final Network network = _networkModel.getNetwork(routerNic.getNetworkId());
-                            // Send network usage command for public nic in VPC VR
-                            // Send network usage command for isolated guest nic of non) VPC VR
-
-                            //[TODO] Avoiding the NPE now, but I have to find out what is going on with the network. - Wilder Rodrigues
-                            if (network == null) {
-                                s_logger.error("Could not find a network with ID => " + routerNic.getNetworkId() + ". It might be a problem!");
-                                continue;
-                            }
-                            if (forVpc && network.getTrafficType() == TrafficType.Public || !forVpc && network.getTrafficType() == TrafficType.Guest
-                                    && network.getGuestType() == Network.GuestType.Isolated) {
-                                final NetworkUsageCommand usageCmd = new NetworkUsageCommand(privateIP, router.getHostName(), forVpc, routerNic.getIPv4Address());
-                                final String routerType = router.getType().toString();
-                                final UserStatisticsVO previousStats = _userStatsDao.findBy(router.getAccountId(), router.getDataCenterId(), network.getId(),
-                                        forVpc ? routerNic.getIPv4Address() : null, router.getId(), routerType);
-                                NetworkUsageAnswer answer = null;
-                                try {
-                                    answer = (NetworkUsageAnswer) _agentMgr.easySend(router.getHostId(), usageCmd);
-                                } catch (final Exception e) {
-                                    s_logger.warn("Error while collecting network stats from router: " + router.getInstanceName() + " from host: " + router.getHostId(), e);
-                                    continue;
-                                }
-
-                                if (answer != null) {
-                                    if (!answer.getResult()) {
-                                        s_logger.warn("Error while collecting network stats from router: " + router.getInstanceName() + " from host: " + router.getHostId()
-                                                + "; details: " + answer.getDetails());
-                                        continue;
-                                    }
-                                    try {
-                                        if (answer.getBytesReceived() == 0 && answer.getBytesSent() == 0) {
-                                            s_logger.debug("Recieved and Sent bytes are both 0. Not updating user_statistics");
-                                            continue;
-                                        }
-                                        final NetworkUsageAnswer answerFinal = answer;
-                                        Transaction.execute(new TransactionCallbackNoReturn() {
-                                            @Override
-                                            public void doInTransactionWithoutResult(final TransactionStatus status) {
-                                                final UserStatisticsVO stats = _userStatsDao.lock(router.getAccountId(), router.getDataCenterId(), network.getId(),
-                                                        forVpc ? routerNic.getIPv4Address() : null, router.getId(), routerType);
-                                                if (stats == null) {
-                                                    s_logger.warn("unable to find stats for account: " + router.getAccountId());
-                                                    return;
-                                                }
-
-                                                if (previousStats != null
-                                                        && (previousStats.getCurrentBytesReceived() != stats.getCurrentBytesReceived() || previousStats.getCurrentBytesSent() != stats
-                                                        .getCurrentBytesSent())) {
-                                                    s_logger.debug("Router stats changed from the time NetworkUsageCommand was sent. " + "Ignoring current answer. Router: "
-                                                            + answerFinal.getRouterName() + " Rcvd: " + answerFinal.getBytesReceived() + "Sent: " + answerFinal.getBytesSent());
-                                                    return;
-                                                }
-
-                                                if (stats.getCurrentBytesReceived() > answerFinal.getBytesReceived()) {
-                                                    if (s_logger.isDebugEnabled()) {
-                                                        s_logger.debug("Received # of bytes that's less than the last one.  "
-                                                                + "Assuming something went wrong and persisting it. Router: " + answerFinal.getRouterName() + " Reported: "
-                                                                + toHumanReadableSize(answerFinal.getBytesReceived()) + " Stored: " + toHumanReadableSize(stats.getCurrentBytesReceived()));
-                                                    }
-                                                    stats.setNetBytesReceived(stats.getNetBytesReceived() + stats.getCurrentBytesReceived());
-                                                }
-                                                stats.setCurrentBytesReceived(answerFinal.getBytesReceived());
-                                                if (stats.getCurrentBytesSent() > answerFinal.getBytesSent()) {
-                                                    if (s_logger.isDebugEnabled()) {
-                                                        s_logger.debug("Received # of bytes that's less than the last one.  "
-                                                                + "Assuming something went wrong and persisting it. Router: " + answerFinal.getRouterName() + " Reported: "
-                                                                + toHumanReadableSize(answerFinal.getBytesSent()) + " Stored: " + toHumanReadableSize(stats.getCurrentBytesSent()));
-                                                    }
-                                                    stats.setNetBytesSent(stats.getNetBytesSent() + stats.getCurrentBytesSent());
-                                                }
-                                                stats.setCurrentBytesSent(answerFinal.getBytesSent());
-                                                if (!_dailyOrHourly) {
-                                                    // update agg bytes
-                                                    stats.setAggBytesSent(stats.getNetBytesSent() + stats.getCurrentBytesSent());
-                                                    stats.setAggBytesReceived(stats.getNetBytesReceived() + stats.getCurrentBytesReceived());
-                                                }
-                                                _userStatsDao.update(stats.getId(), stats);
-                                            }
-                                        });
-
-                                    } catch (final Exception e) {
-                                        s_logger.warn("Unable to update user statistics for account: " + router.getAccountId() + " Rx: " + toHumanReadableSize(answer.getBytesReceived()) + "; Tx: "
-                                                + toHumanReadableSize(answer.getBytesSent()));
-                                    }
-                                }
-                            }
-                        }
-                    }
+                    collectNetworkStatistics(router, null);
                 }
             } catch (final Exception e) {
                 s_logger.warn("Error while collecting network stats", e);
@@ -3057,6 +2964,11 @@ Configurable, StateListener<VirtualMachine.State, VirtualMachine.Event, VirtualM
         // Collect network usage before stopping Vm
 
         final DomainRouterVO router = _routerDao.findById(profile.getVirtualMachine().getId());
+        collectNetworkStatistics(router, null);
+    }
+
+    @Override
+    public <T extends VirtualRouter> void collectNetworkStatistics(final T router, final Nic nic) {
         if (router == null) {
             return;
         }
@@ -3065,12 +2977,23 @@ Configurable, StateListener<VirtualMachine.State, VirtualMachine.Event, VirtualM
 
         if (privateIP != null) {
             final boolean forVpc = router.getVpcId() != null;
-            final List<? extends Nic> routerNics = _nicDao.listByVmId(router.getId());
+            List<Nic> routerNics = new ArrayList<Nic>();
+            if (nic != null) {
+                routerNics.add(nic);
+            } else {
+                routerNics.addAll(_nicDao.listByVmId(router.getId()));
+            }
             for (final Nic routerNic : routerNics) {
                 final Network network = _networkModel.getNetwork(routerNic.getNetworkId());
                 // Send network usage command for public nic in VPC VR
                 // Send network usage command for isolated guest nic of non VPC
                 // VR
+
+                //[TODO] Avoiding the NPE now, but I have to find out what is going on with the network. - Wilder Rodrigues
+                if (network == null) {
+                    s_logger.error("Could not find a network with ID => " + routerNic.getNetworkId() + ". It might be a problem!");
+                    continue;
+                }
                 if (forVpc && network.getTrafficType() == TrafficType.Public || !forVpc && network.getTrafficType() == TrafficType.Guest
                         && network.getGuestType() == Network.GuestType.Isolated) {
                     final NetworkUsageCommand usageCmd = new NetworkUsageCommand(privateIP, router.getHostName(), forVpc, routerNic.getIPv4Address());

server/src/main/java/com/cloud/network/rules/NicPlugInOutRules.java

@@ -40,6 +40,7 @@ import com.cloud.network.NetworkModel;
 import com.cloud.network.Networks.BroadcastDomainType;
 import com.cloud.network.Networks.IsolationType;
 import com.cloud.network.PublicIpAddress;
+import com.cloud.network.VpcVirtualNetworkApplianceService;
 import com.cloud.network.dao.FirewallRulesDao;
 import com.cloud.network.dao.IPAddressDao;
 import com.cloud.network.dao.IPAddressVO;
@@ -84,6 +85,7 @@ public class NicPlugInOutRules extends RuleApplier {
         NetworkModel networkModel = visitor.getVirtualNetworkApplianceFactory().getNetworkModel();
         VirtualMachineManager itMgr = visitor.getVirtualNetworkApplianceFactory().getItMgr();
         NicDao nicDao = visitor.getVirtualNetworkApplianceFactory().getNicDao();
+        VpcVirtualNetworkApplianceService routerService = visitor.getVirtualNetworkApplianceFactory().getRouterService();
 
         // de-associate IPs before unplugging nics
         if (!nicsToUnplug.isEmpty()) {
@@ -107,6 +109,12 @@ public class NicPlugInOutRules extends RuleApplier {
 
         // 1) Unplug the nics
         for (Entry<String, PublicIpAddress> entry : nicsToUnplug.entrySet()) {
+            PublicIpAddress ip = entry.getValue();
+            NicVO nic = nicDao.findByIp4AddressAndNetworkIdAndInstanceId(ip.getNetworkId(), _router.getId(), ip.getAddress().addr());
+            if (nic != null) {
+                s_logger.info("Collect network statistics for nic " + nic + " from router " + _router);
+                routerService.collectNetworkStatistics(_router, nic);
+            }
             Network publicNtwk = null;
             try {
                 publicNtwk = networkModel.getNetwork(entry.getValue().getNetworkId());

server/src/main/java/com/cloud/network/rules/VirtualNetworkApplianceFactory.java

@@ -26,6 +26,7 @@ import com.cloud.dc.dao.HostPodDao;
 import com.cloud.dc.dao.VlanDao;
 import com.cloud.network.IpAddressManager;
 import com.cloud.network.NetworkModel;
+import com.cloud.network.VpcVirtualNetworkApplianceService;
 import com.cloud.network.dao.FirewallRulesDao;
 import com.cloud.network.dao.IPAddressDao;
 import com.cloud.network.dao.LoadBalancerDao;
@@ -98,6 +99,8 @@ public class VirtualNetworkApplianceFactory {
 
     @Inject
     private NetworkTopologyContext _networkTopologyContext;
+    @Inject
+    private VpcVirtualNetworkApplianceService _routerService;
 
     public NetworkModel getNetworkModel() {
         return _networkModel;
@@ -190,4 +193,8 @@ public class VirtualNetworkApplianceFactory {
     public FirewallRulesDao getFirewallRulesDao() {
         return _rulesDao;
     }
+
+    public VpcVirtualNetworkApplianceService getRouterService() {
+        return _routerService;
+    }
 }

server/src/test/java/com/cloud/vpc/MockVpcVirtualNetworkApplianceManager.java

@@ -41,6 +41,7 @@ import com.cloud.user.Account;
 import com.cloud.user.User;
 import com.cloud.utils.component.ManagerBase;
 import com.cloud.vm.DomainRouterVO;
+import com.cloud.vm.Nic;
 import com.cloud.vm.VirtualMachineProfile;
 
 @Component
@@ -280,4 +281,10 @@ public class MockVpcVirtualNetworkApplianceManager extends ManagerBase implement
         // TODO Auto-generated method stub
         return false;
     }
-}
+
+    @Override
+    public <T extends VirtualRouter> void collectNetworkStatistics(T router, Nic nic) {
+        // TODO Auto-generated method stub
+        return;
+    }
+}

systemvm/debian/opt/cloud/bin/cs/CsAddress.py

@@ -476,11 +476,6 @@ class CsIP:
                             "-A PREROUTING -m state --state NEW -i %s -s %s ! -d %s/32 -j ACL_OUTBOUND_%s" %
                             (self.dev, guestNetworkCidr, self.address['gateway'], self.dev)])
 
-            self.fw.append(["", "front", "-A NETWORK_STATS_%s -i %s -d %s" %
-                            ("eth1", "eth1", guestNetworkCidr)])
-            self.fw.append(["", "front", "-A NETWORK_STATS_%s -o %s -s %s" %
-                            ("eth1", "eth1", guestNetworkCidr)])
-
         if self.is_private_gateway():
             self.fw.append(["filter", "", "-A FORWARD -d %s -o %s -j ACL_INBOUND_%s" %
                             (self.address['network'], self.dev, self.dev)])
@@ -518,6 +513,10 @@ class CsIP:
                 ["mangle", "", "-A VPN_STATS_%s -i %s -m mark --mark 0x524/0xffffffff" % (self.dev, self.dev)])
             self.fw.append(
                 ["", "front", "-A FORWARD -j NETWORK_STATS_%s" % self.dev])
+            self.fw.append(
+                ["", "front", "-A NETWORK_STATS_%s -s %s -o %s" % (self.dev, self.cl.get_vpccidr(), self.dev)])
+            self.fw.append(
+                ["", "front", "-A NETWORK_STATS_%s -d %s -i %s" % (self.dev, self.cl.get_vpccidr(), self.dev)])
 
         self.fw.append(["", "front", "-A FORWARD -j NETWORK_STATS"])
         self.fw.append(["", "front", "-A INPUT -j NETWORK_STATS"])