You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2018/10/10 09:50:43 UTC

[cloudstack] branch 4.11 updated: router: Fixes #2719 program VR nics by device id order for VPC (#2888)

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch 4.11
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/4.11 by this push:
     new ea771cf  router: Fixes #2719 program VR nics by device id order for VPC (#2888)
ea771cf is described below

commit ea771cfda4839e934cffbf647906f5c0af4993a4
Author: Rohit Yadav <ro...@apache.org>
AuthorDate: Wed Oct 10 15:20:36 2018 +0530

    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 <ro...@shapeblue.com>
---
 api/src/com/cloud/network/NetworkProfile.java           |  8 +++++---
 engine/schema/src/com/cloud/vm/dao/NicDao.java          |  2 ++
 engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java      |  7 +++++++
 .../router/VpcVirtualNetworkApplianceManagerImpl.java   |  2 +-
 systemvm/debian/opt/cloud/bin/cs_ip.py                  | 17 +++++++++++++++++
 .../src/test/java/com/cloud/utils/net/NetUtilsTest.java |  6 ++++--
 6 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/api/src/com/cloud/network/NetworkProfile.java b/api/src/com/cloud/network/NetworkProfile.java
index d8733ca..bf21c93 100644
--- a/api/src/com/cloud/network/NetworkProfile.java
+++ b/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 class NetworkProfile implements Network {
         guruName = network.getGuruName();
         strechedL2Subnet = network.isStrechedL2Network();
         isRedundant = network.isRedundant();
+        isRollingRestart = network.isRollingRestart();
         externalId = network.getExternalId();
     }
 
@@ -157,7 +159,7 @@ public class NetworkProfile implements Network {
 
     @Override
     public boolean isRollingRestart() {
-        return false;
+        return isRollingRestart;
     }
 
     @Override
diff --git a/engine/schema/src/com/cloud/vm/dao/NicDao.java b/engine/schema/src/com/cloud/vm/dao/NicDao.java
index 0d86964..df4fb06 100644
--- a/engine/schema/src/com/cloud/vm/dao/NicDao.java
+++ b/engine/schema/src/com/cloud/vm/dao/NicDao.java
@@ -26,6 +26,8 @@ import com.cloud.vm.VirtualMachine;
 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);
diff --git a/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java b/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java
index f953a4c..c125d80 100644
--- a/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java
+++ b/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java
@@ -119,6 +119,13 @@ public class NicDaoImpl extends GenericDaoBase<NicVO, Long> implements NicDao {
     }
 
     @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();
         sc.setParameters("instance", instanceId);
diff --git a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
index d22dcba..fefd972 100644
--- a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
+++ b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
@@ -316,7 +316,7 @@ public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplian
             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) {
diff --git a/systemvm/debian/opt/cloud/bin/cs_ip.py b/systemvm/debian/opt/cloud/bin/cs_ip.py
index 1e7b326..a4e0c33 100755
--- a/systemvm/debian/opt/cloud/bin/cs_ip.py
+++ b/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)
diff --git a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java b/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java
index 80d25e8..262e928 100644
--- a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java
+++ b/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java
@@ -37,13 +37,13 @@ import java.net.UnknownHostException;
 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 class NetUtilsTest {
     @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));
+        }
     }
 }