router: Fixes #2719 program VR nics by device id order for VPC (#2888)

This fixes #2719 where private gateway IP might be incorrectly
programmed on a guest network nic. The VR would now check ipassoc
requests by mac addresses than provided nic/device id in case they are
wrong.

The root cause is that the device id information is lost when aggregated
commands are created upon starting of a new VPC VR, without the correct
device id in ip_associations json it mis-programs the VR.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

Author: yadvr

api/src/com/cloud/network/NetworkProfile.java

@@ -16,12 +16,12 @@
 // under the License.
 package com.cloud.network;
 
+import java.net.URI;
+
 import com.cloud.network.Networks.BroadcastDomainType;
 import com.cloud.network.Networks.Mode;
 import com.cloud.network.Networks.TrafficType;
 
-import java.net.URI;
-
 public class NetworkProfile implements Network {
     private final long id;
     private final String uuid;
@@ -33,6 +33,7 @@ public class NetworkProfile implements Network {
     private URI broadcastUri;
     private final State state;
     private boolean isRedundant;
+    private boolean isRollingRestart = false;
     private final String name;
     private final Mode mode;
     private final BroadcastDomainType broadcastDomainType;
@@ -92,6 +93,7 @@ public NetworkProfile(Network network) {
         guruName = network.getGuruName();
         strechedL2Subnet = network.isStrechedL2Network();
         isRedundant = network.isRedundant();
+        isRollingRestart = network.isRollingRestart();
         externalId = network.getExternalId();
     }
 
@@ -157,7 +159,7 @@ public boolean isRedundant() {
 
     @Override
     public boolean isRollingRestart() {
-        return false;
+        return isRollingRestart;
     }
 
     @Override

engine/schema/src/com/cloud/vm/dao/NicDao.java

@@ -26,6 +26,8 @@
 public interface NicDao extends GenericDao<NicVO, Long> {
     List<NicVO> listByVmId(long instanceId);
 
+    List<NicVO> listByVmIdOrderByDeviceId(long instanceId);
+
     List<String> listIpAddressInNetwork(long networkConfigId);
 
     List<NicVO> listByVmIdIncludingRemoved(long instanceId);

engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java

@@ -118,6 +118,13 @@ public List<NicVO> listByVmId(long instanceId) {
         return listBy(sc);
     }
 
+    @Override
+    public List<NicVO> listByVmIdOrderByDeviceId(long instanceId) {
+        SearchCriteria<NicVO> sc = AllFieldsSearch.create();
+        sc.setParameters("instance", instanceId);
+        return customSearch(sc, new Filter(NicVO.class, "deviceId", true, null, null));
+    }
+
     @Override
     public List<NicVO> listByVmIdIncludingRemoved(long instanceId) {
         SearchCriteria<NicVO> sc = AllFieldsSearch.create();

server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java

@@ -316,7 +316,7 @@ public boolean finalizeCommandsOnStart(final Commands cmds, final VirtualMachine
             final List<Pair<Nic, Network>> publicNics = new ArrayList<Pair<Nic, Network>>();
             final Map<String, String> vlanMacAddress = new HashMap<String, String>();
 
-            final List<? extends Nic> routerNics = _nicDao.listByVmId(profile.getId());
+            final List<? extends Nic> routerNics = _nicDao.listByVmIdOrderByDeviceId(profile.getId());
             for (final Nic routerNic : routerNics) {
                 final Network network = _networkModel.getNetwork(routerNic.getNetworkId());
                 if (network.getTrafficType() == TrafficType.Guest) {

systemvm/debian/opt/cloud/bin/cs_ip.py

@@ -16,9 +16,21 @@
 # specific language governing permissions and limitations
 # under the License.
 
+import os
 from netaddr import *
 
 
+def macdevice_map():
+    device_map = {}
+    for eth in os.listdir('/sys/class/net'):
+        if not eth.startswith('eth'):
+            continue
+        with open('/sys/class/net/%s/address' % eth) as f:
+            mac_address = f.read().replace('\n', '')
+            device_map[mac_address] = eth[3:]
+    return device_map
+
+
 def merge(dbag, ip):
     nic_dev_id = None
     for dev in dbag:
@@ -33,6 +45,11 @@ def merge(dbag, ip):
     ipo = IPNetwork(ip['public_ip'] + '/' + ip['netmask'])
     if 'nic_dev_id' in ip:
         nic_dev_id = ip['nic_dev_id']
+    if 'vif_mac_address' in ip:
+        mac_address = ip['vif_mac_address']
+        device_map = macdevice_map()
+        if mac_address in device_map:
+            nic_dev_id = device_map[mac_address]
     ip['device'] = 'eth' + str(nic_dev_id)
     ip['broadcast'] = str(ipo.broadcast)
     ip['cidr'] = str(ipo.ip) + '/' + str(ipo.prefixlen)

utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java

@@ -37,13 +37,13 @@
 import java.util.SortedSet;
 import java.util.TreeSet;
 
-import com.googlecode.ipv6.IPv6Network;
 import org.apache.log4j.Logger;
 import org.junit.Test;
 
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.net.NetUtils.SupersetOrSubset;
 import com.googlecode.ipv6.IPv6Address;
+import com.googlecode.ipv6.IPv6Network;
 
 public class NetUtilsTest {
 
@@ -682,6 +682,8 @@ public void testIsValidPort() {
     @Test
     public void testAllIpsOfDefaultNic() {
         final String defaultHostIp = NetUtils.getDefaultHostIp();
-        assertTrue(NetUtils.getAllDefaultNicIps().stream().anyMatch(defaultHostIp::contains));
+        if (defaultHostIp != null) {
+            assertTrue(NetUtils.getAllDefaultNicIps().stream().anyMatch(defaultHostIp::contains));
+        }
     }
 }