get_device_by_ip: don't fail if device was deleted

The function gets a list of all the devices that exists on the machine,
and then iterates on them one at a time in order to find the correct
device which holds the ip specified. However, if one of the devices was
in the mean time deleted, the code will raise an Exception. In the ovs
agent's case, this will cause it to not run at all (requiring a
restart).

Also, changes to a few tests of LinuxBridge were made because
linuxbridge doesn't check that cfg.CONF.VXLAN.local_ip is not empty
before using it (a bug, surely). Since it's out of scope of this patch
to fix this, workarounds were implemented to make sure the tests ignore
the option instead.

Closes-Bug: #1506503
Change-Id: Iad285d7c763b0e8e8f877c6892aadb0043e9a186

diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py
index 17600812de9eb121b062fd4f4c059d7869cb8e9e..cc7ff9fdc0a4d33e5f08b54dd97aec8c972c09ee 100644 (file)
--- a/neutron/agent/linux/ip_lib.py
+++ b/neutron/agent/linux/ip_lib.py
@@ -41,6 +41,7 @@ LOOPBACK_DEVNAME = 'lo'
 SYS_NET_PATH = '/sys/class/net'
 DEFAULT_GW_PATTERN = re.compile(r"via (\S+)")
 METRIC_PATTERN = re.compile(r"metric (\S+)")
+DEVICE_NAME_PATTERN = re.compile(r"(\d+?): (\S+?):.*")
 
 
 class AddressNotReady(exceptions.NeutronException):
@@ -129,10 +130,19 @@ class IPWrapper(SubProcessBase):
         return retval
 
     def get_device_by_ip(self, ip):
-        """Get the IPDevice from system which has ip configured."""
-        for device in self.get_devices():
-            if device.addr.list(to=ip):
-                return device
+        """Get the IPDevice from system which has ip configured.
+
+        @param ip: look for the device holding this ip. If this is None,
+                   None is returned.
+        @type ip: str.
+        """
+        if not ip:
+            return None
+
+        addr = IpAddrCommand(self)
+        devices = addr.get_devices_with_ip(to=ip)
+        if devices:
+            return IPDevice(devices[0]['name'], namespace=self.namespace)
 
     def add_tuntap(self, name, mode='tap'):
         self._as_root([], 'tuntap', ('add', name, 'mode', mode))
@@ -513,24 +523,40 @@ class IpAddrCommand(IpDeviceCommandBase):
     def flush(self, ip_version):
         self._as_root([ip_version], ('flush', self.name))
 
-    def list(self, scope=None, to=None, filters=None, ip_version=None):
+    def get_devices_with_ip(self, name=None, scope=None, to=None,
+                            filters=None, ip_version=None):
+        """Get a list of all the devices with an IP attached in the namespace.
+
+        @param name: if it's not None, only a device with that matching name
+                     will be returned.
+        """
         options = [ip_version] if ip_version else []
-        args = ['show', self.name]
+
+        args = ['show']
+        if name:
+            args += [name]
         if filters:
             args += filters
-
-        retval = []
-
         if scope:
             args += ['scope', scope]
         if to:
             args += ['to', to]
 
+        retval = []
+
         for line in self._run(options, tuple(args)).split('\n'):
             line = line.strip()
-            if not line.startswith('inet'):
+
+            match = DEVICE_NAME_PATTERN.search(line)
+            if match:
+                # Found a match for a device name, but its' addresses will
+                # only appear in following lines, so we may as well continue.
+                device_name = match.group(2)
                 continue
-            parts = line.split()
+            elif not line.startswith('inet'):
+                continue
+
+            parts = line.split(" ")
             if parts[0] == 'inet6':
                 scope = parts[3]
             else:
@@ -539,13 +565,19 @@ class IpAddrCommand(IpDeviceCommandBase):
                 else:
                     scope = parts[3]
 
-            retval.append(dict(cidr=parts[1],
+            retval.append(dict(name=device_name,
+                               cidr=parts[1],
                                scope=scope,
                                dynamic=('dynamic' == parts[-1]),
                                tentative=('tentative' in line),
                                dadfailed=('dadfailed' == parts[-1])))
         return retval
 
+    def list(self, scope=None, to=None, filters=None, ip_version=None):
+        """Get device details of a device named <self.name>."""
+        return self.get_devices_with_ip(
+            self.name, scope, to, filters, ip_version)
+
     def wait_until_address_ready(self, address, wait_time=30):
         """Wait until an address is no longer marked 'tentative'
 

diff --git a/neutron/tests/functional/agent/linux/test_ip_lib.py b/neutron/tests/functional/agent/linux/test_ip_lib.py
index 0aa39d113ae40acb9705b3653fb064793434b7bc..4be32098656e362ea207e45f01298aa31cdeaa23 100644 (file)
--- a/neutron/tests/functional/agent/linux/test_ip_lib.py
+++ b/neutron/tests/functional/agent/linux/test_ip_lib.py
@@ -121,6 +121,10 @@ class IpLibTestCase(IpLibTestFramework):
         device.link.delete()
         self.assertFalse(ip_lib.vxlan_in_use(9999, namespace=attr.namespace))
 
+    def test_ipwrapper_get_device_by_ip_None(self):
+        ip_wrapper = ip_lib.IPWrapper(namespace=None)
+        self.assertIsNone(ip_wrapper.get_device_by_ip(ip=None))
+
     def test_ipwrapper_get_device_by_ip(self):
         attr = self.generate_device_details()
         self.manage_device(attr)

diff --git a/neutron/tests/functional/agent/test_l2_lb_agent.py b/neutron/tests/functional/agent/test_l2_lb_agent.py
index 869be14b7bd3eb3641b6399a56d93d4c554efb5b..5dfdacd404be4754d02e15e93cc8e0354dd884e9 100644 (file)
--- a/neutron/tests/functional/agent/test_l2_lb_agent.py
+++ b/neutron/tests/functional/agent/test_l2_lb_agent.py
@@ -13,6 +13,7 @@
 #    under the License.
 
 import mock
+from oslo_config import cfg
 from oslo_log import log as logging
 import testtools
 
@@ -31,6 +32,7 @@ class LinuxBridgeAgentTests(test_ip_lib.IpLibTestFramework):
         agent_rpc = ('neutron.agent.rpc.PluginApi')
         mock.patch(agent_rpc).start()
         mock.patch('neutron.agent.rpc.PluginReportStateAPI').start()
+        cfg.CONF.set_override('enable_vxlan', False, 'VXLAN')
 
     def test_validate_interface_mappings(self):
         mappings = {'physnet1': 'int1', 'physnet2': 'int2'}

diff --git a/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py b/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py
index a3c13ddc1be59b22f7120927f8f26d058a3e1761..74af2bc21876704cf537eac0908125689824bdff 100644 (file)
--- a/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py
+++ b/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py
@@ -35,6 +35,7 @@ class LinuxbridgeCleanupTest(base.BaseSudoTestCase):
                     prefix=lb_agent.BRIDGE_NAME_PREFIX))).fixture
 
         config = callback(br_fixture)
+        config.update({'VXLAN': {'enable_vxlan': 'False'}})
 
         temp_dir = self.useFixture(fixtures.TempDir()).path
         conf = self.useFixture(config_fixtures.ConfigFileFixture(

diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py
index c49e44373cdba27d4ff18eaf1d07bb2024645563..addd714d5ea56d76d538d5610e6169da3587c7c7 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_ip_lib.py
+++ b/neutron/tests/unit/agent/linux/test_ip_lib.py
@@ -778,21 +778,21 @@ class TestIpAddrCommand(TestIPCmdBase):
 
     def test_list(self):
         expected = [
-            dict(scope='global', dadfailed=False, tentative=False,
+            dict(name='eth0', scope='global', dadfailed=False, tentative=False,
                  dynamic=False, cidr='172.16.77.240/24'),
-            dict(scope='global', dadfailed=False, tentative=False,
+            dict(name='eth0', scope='global', dadfailed=False, tentative=False,
                  dynamic=True, cidr='2001:470:9:1224:5595:dd51:6ba2:e788/64'),
-            dict(scope='link', dadfailed=False, tentative=True,
+            dict(name='eth0', scope='link', dadfailed=False, tentative=True,
                  dynamic=False, cidr='fe80::3023:39ff:febc:22ae/64'),
-            dict(scope='link', dadfailed=True, tentative=True,
+            dict(name='eth0', scope='link', dadfailed=True, tentative=True,
                  dynamic=False, cidr='fe80::3023:39ff:febc:22af/64'),
-            dict(scope='global', dadfailed=False, tentative=False,
+            dict(name='eth0', scope='global', dadfailed=False, tentative=False,
                  dynamic=True, cidr='2001:470:9:1224:fd91:272:581e:3a32/64'),
-            dict(scope='global', dadfailed=False, tentative=False,
+            dict(name='eth0', scope='global', dadfailed=False, tentative=False,
                  dynamic=True, cidr='2001:470:9:1224:4508:b885:5fb:740b/64'),
-            dict(scope='global', dadfailed=False, tentative=False,
+            dict(name='eth0', scope='global', dadfailed=False, tentative=False,
                  dynamic=True, cidr='2001:470:9:1224:dfcc:aaff:feb9:76ce/64'),
-            dict(scope='link', dadfailed=False, tentative=False,
+            dict(name='eth0', scope='link', dadfailed=False, tentative=False,
                  dynamic=False, cidr='fe80::dfcc:aaff:feb9:76ce/64')]
 
         test_cases = [ADDR_SAMPLE, ADDR_SAMPLE2]
@@ -824,7 +824,7 @@ class TestIpAddrCommand(TestIPCmdBase):
 
     def test_list_filtered(self):
         expected = [
-            dict(scope='global', tentative=False, dadfailed=False,
+            dict(name='eth0', scope='global', tentative=False, dadfailed=False,
                  dynamic=False, cidr='172.16.77.240/24')]
 
         test_cases = [ADDR_SAMPLE, ADDR_SAMPLE2]