Refactor checks for device existence

The code calling driver.plug() shouldn't check for the device existence,
it's a duplicate and it's an expensive call.
Move check for device existence to base LinuxInterfaceDriver.plug()
to remove code duplication. Make plug_new() abstract instead.

Change-Id: Id118a64012ad10b197ba681ce5f1b2742eb135b4
Closes-Bug:1348703

diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py
index b959e5016bf685ada7abf2cab670714c2ecfda5b..050cb6331ae84369a95e7748b8518ae96fe45afb 100644 (file)
--- a/neutron/agent/l3/dvr_fip_ns.py
+++ b/neutron/agent/l3/dvr_fip_ns.py
@@ -94,14 +94,13 @@ class FipNamespace(namespaces.Namespace):
     def _gateway_added(self, ex_gw_port, interface_name):
         """Add Floating IP gateway port."""
         ns_name = self.get_name()
-        if not ip_lib.device_exists(interface_name, namespace=ns_name):
-            self.driver.plug(ex_gw_port['network_id'],
-                             ex_gw_port['id'],
-                             interface_name,
-                             ex_gw_port['mac_address'],
-                             bridge=self.agent_conf.external_network_bridge,
-                             namespace=ns_name,
-                             prefix=FIP_EXT_DEV_PREFIX)
+        self.driver.plug(ex_gw_port['network_id'],
+                         ex_gw_port['id'],
+                         interface_name,
+                         ex_gw_port['mac_address'],
+                         bridge=self.agent_conf.external_network_bridge,
+                         namespace=ns_name,
+                         prefix=FIP_EXT_DEV_PREFIX)
 
         ip_cidrs = common_utils.fixed_ip_cidrs(ex_gw_port['fixed_ips'])
         self.driver.init_l3(interface_name, ip_cidrs, namespace=ns_name)

diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py
index ebccf32b4ab37c4504c678a5a1fa0f791b9851c1..ece0f0a31ede3d1e4c0e374fcb805593ee1ffe8f 100644 (file)
--- a/neutron/agent/l3/ha_router.py
+++ b/neutron/agent/l3/ha_router.py
@@ -257,13 +257,12 @@ class HaRouter(router.RouterInfo):
         port_id = port['id']
         interface_name = self.get_internal_device_name(port_id)
 
-        if not ip_lib.device_exists(interface_name, namespace=self.ns_name):
-            self.driver.plug(port['network_id'],
-                             port_id,
-                             interface_name,
-                             port['mac_address'],
-                             namespace=self.ns_name,
-                             prefix=router.INTERNAL_DEV_PREFIX)
+        self.driver.plug(port['network_id'],
+                         port_id,
+                         interface_name,
+                         port['mac_address'],
+                         namespace=self.ns_name,
+                         prefix=router.INTERNAL_DEV_PREFIX)
 
         self._disable_ipv6_addressing_on_interface(interface_name)
         for ip_cidr in common_utils.fixed_ip_cidrs(port['fixed_ips']):

diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py
index b9b54b6107dd692f085207711ce665bd0181bb9b..3f7684b8462f9eb08f18a1aff061399cf6bde8e8 100644 (file)
--- a/neutron/agent/l3/router_info.py
+++ b/neutron/agent/l3/router_info.py
@@ -281,11 +281,9 @@ class RouterInfo(object):
     def _internal_network_added(self, ns_name, network_id, port_id,
                                 fixed_ips, mac_address,
                                 interface_name, prefix):
-        if not ip_lib.device_exists(interface_name,
-                                    namespace=ns_name):
-            self.driver.plug(network_id, port_id, interface_name, mac_address,
-                             namespace=ns_name,
-                             prefix=prefix)
+        self.driver.plug(network_id, port_id, interface_name, mac_address,
+                         namespace=ns_name,
+                         prefix=prefix)
 
         ip_cidrs = common_utils.fixed_ip_cidrs(fixed_ips)
         self.driver.init_l3(interface_name, ip_cidrs, namespace=ns_name)
@@ -418,14 +416,13 @@ class RouterInfo(object):
                 for ip in floating_ips]
 
     def _plug_external_gateway(self, ex_gw_port, interface_name, ns_name):
-        if not ip_lib.device_exists(interface_name, namespace=ns_name):
-            self.driver.plug(ex_gw_port['network_id'],
-                             ex_gw_port['id'],
-                             interface_name,
-                             ex_gw_port['mac_address'],
-                             bridge=self.agent_conf.external_network_bridge,
-                             namespace=ns_name,
-                             prefix=EXTERNAL_DEV_PREFIX)
+        self.driver.plug(ex_gw_port['network_id'],
+                         ex_gw_port['id'],
+                         interface_name,
+                         ex_gw_port['mac_address'],
+                         bridge=self.agent_conf.external_network_bridge,
+                         namespace=ns_name,
+                         prefix=EXTERNAL_DEV_PREFIX)
 
     def _external_gateway_added(self, ex_gw_port, interface_name,
                                 ns_name, preserve_ips):

diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py
index 99654cfc8f47d2b77f112f2268a3db5afbf1fd96..2e0af0d600f1658074baf75de1b639137381b9f5 100644 (file)
--- a/neutron/agent/linux/interface.py
+++ b/neutron/agent/linux/interface.py
@@ -179,9 +179,18 @@ class LinuxInterfaceDriver(object):
             ['sysctl', '-w', 'net.ipv6.conf.%s.accept_ra=2' % dev_name])
 
     @abc.abstractmethod
+    def plug_new(self, network_id, port_id, device_name, mac_address,
+                 bridge=None, namespace=None, prefix=None):
+        """Plug in the interface only for new devices that don't exist yet."""
+
     def plug(self, network_id, port_id, device_name, mac_address,
              bridge=None, namespace=None, prefix=None):
-        """Plug in the interface."""
+        if not ip_lib.device_exists(device_name,
+                                    namespace=namespace):
+            self.plug_new(network_id, port_id, device_name, mac_address,
+                          bridge, namespace, prefix)
+        else:
+            LOG.info(_LI("Device %s already exists"), device_name)
 
     @abc.abstractmethod
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
@@ -189,8 +198,8 @@ class LinuxInterfaceDriver(object):
 
 
 class NullDriver(LinuxInterfaceDriver):
-    def plug(self, network_id, port_id, device_name, mac_address,
-             bridge=None, namespace=None, prefix=None):
+    def plug_new(self, network_id, port_id, device_name, mac_address,
+                 bridge=None, namespace=None, prefix=None):
         pass
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
@@ -224,48 +233,44 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
         ovs = ovs_lib.OVSBridge(bridge)
         ovs.replace_port(device_name, *attrs)
 
-    def plug(self, network_id, port_id, device_name, mac_address,
-             bridge=None, namespace=None, prefix=None):
+    def plug_new(self, network_id, port_id, device_name, mac_address,
+                 bridge=None, namespace=None, prefix=None):
         """Plug in the interface."""
         if not bridge:
             bridge = self.conf.ovs_integration_bridge
 
-        if not ip_lib.device_exists(device_name, namespace=namespace):
+        self.check_bridge_exists(bridge)
 
-            self.check_bridge_exists(bridge)
+        ip = ip_lib.IPWrapper()
+        tap_name = self._get_tap_name(device_name, prefix)
 
-            ip = ip_lib.IPWrapper()
-            tap_name = self._get_tap_name(device_name, prefix)
+        if self.conf.ovs_use_veth:
+            # Create ns_dev in a namespace if one is configured.
+            root_dev, ns_dev = ip.add_veth(tap_name,
+                                           device_name,
+                                           namespace2=namespace)
+        else:
+            ns_dev = ip.device(device_name)
 
+        internal = not self.conf.ovs_use_veth
+        self._ovs_add_port(bridge, tap_name, port_id, mac_address,
+                           internal=internal)
+
+        ns_dev.link.set_address(mac_address)
+
+        if self.conf.network_device_mtu:
+            ns_dev.link.set_mtu(self.conf.network_device_mtu)
             if self.conf.ovs_use_veth:
-                # Create ns_dev in a namespace if one is configured.
-                root_dev, ns_dev = ip.add_veth(tap_name,
-                                               device_name,
-                                               namespace2=namespace)
-            else:
-                ns_dev = ip.device(device_name)
-
-            internal = not self.conf.ovs_use_veth
-            self._ovs_add_port(bridge, tap_name, port_id, mac_address,
-                               internal=internal)
-
-            ns_dev.link.set_address(mac_address)
-
-            if self.conf.network_device_mtu:
-                ns_dev.link.set_mtu(self.conf.network_device_mtu)
-                if self.conf.ovs_use_veth:
-                    root_dev.link.set_mtu(self.conf.network_device_mtu)
-
-            # Add an interface created by ovs to the namespace.
-            if not self.conf.ovs_use_veth and namespace:
-                namespace_obj = ip.ensure_namespace(namespace)
-                namespace_obj.add_device_to_namespace(ns_dev)
-
-            ns_dev.link.set_up()
-            if self.conf.ovs_use_veth:
-                root_dev.link.set_up()
-        else:
-            LOG.info(_LI("Device %s already exists"), device_name)
+                root_dev.link.set_mtu(self.conf.network_device_mtu)
+
+        # Add an interface created by ovs to the namespace.
+        if not self.conf.ovs_use_veth and namespace:
+            namespace_obj = ip.ensure_namespace(namespace)
+            namespace_obj.add_device_to_namespace(ns_dev)
+
+        ns_dev.link.set_up()
+        if self.conf.ovs_use_veth:
+            root_dev.link.set_up()
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         """Unplug the interface."""
@@ -289,34 +294,30 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
 
 class MidonetInterfaceDriver(LinuxInterfaceDriver):
 
-    def plug(self, network_id, port_id, device_name, mac_address,
-             bridge=None, namespace=None, prefix=None):
+    def plug_new(self, network_id, port_id, device_name, mac_address,
+                 bridge=None, namespace=None, prefix=None):
         """This method is called by the Dhcp agent or by the L3 agent
         when a new network is created
         """
-        if not ip_lib.device_exists(device_name, namespace=namespace):
-            ip = ip_lib.IPWrapper()
-            tap_name = device_name.replace(prefix or n_const.TAP_DEVICE_PREFIX,
-                                           n_const.TAP_DEVICE_PREFIX)
+        ip = ip_lib.IPWrapper()
+        tap_name = device_name.replace(prefix or n_const.TAP_DEVICE_PREFIX,
+                                       n_const.TAP_DEVICE_PREFIX)
 
-            # Create ns_dev in a namespace if one is configured.
-            root_dev, ns_dev = ip.add_veth(tap_name, device_name,
-                                           namespace2=namespace)
+        # Create ns_dev in a namespace if one is configured.
+        root_dev, ns_dev = ip.add_veth(tap_name, device_name,
+                                       namespace2=namespace)
 
-            ns_dev.link.set_address(mac_address)
-
-            # Add an interface created by ovs to the namespace.
-            namespace_obj = ip.ensure_namespace(namespace)
-            namespace_obj.add_device_to_namespace(ns_dev)
+        ns_dev.link.set_address(mac_address)
 
-            ns_dev.link.set_up()
-            root_dev.link.set_up()
+        # Add an interface created by ovs to the namespace.
+        namespace_obj = ip.ensure_namespace(namespace)
+        namespace_obj.add_device_to_namespace(ns_dev)
 
-            cmd = ['mm-ctl', '--bind-port', port_id, device_name]
-            utils.execute(cmd, run_as_root=True)
+        ns_dev.link.set_up()
+        root_dev.link.set_up()
 
-        else:
-            LOG.info(_LI("Device %s already exists"), device_name)
+        cmd = ['mm-ctl', '--bind-port', port_id, device_name]
+        utils.execute(cmd, run_as_root=True)
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         # the port will be deleted by the dhcp agent that will call the plugin
@@ -348,33 +349,29 @@ class IVSInterfaceDriver(LinuxInterfaceDriver):
         cmd = ['ivs-ctl', 'add-port', device_name]
         utils.execute(cmd, run_as_root=True)
 
-    def plug(self, network_id, port_id, device_name, mac_address,
-             bridge=None, namespace=None, prefix=None):
+    def plug_new(self, network_id, port_id, device_name, mac_address,
+                 bridge=None, namespace=None, prefix=None):
         """Plug in the interface."""
-        if not ip_lib.device_exists(device_name, namespace=namespace):
+        ip = ip_lib.IPWrapper()
+        tap_name = self._get_tap_name(device_name, prefix)
 
-            ip = ip_lib.IPWrapper()
-            tap_name = self._get_tap_name(device_name, prefix)
+        root_dev, ns_dev = ip.add_veth(tap_name, device_name)
 
-            root_dev, ns_dev = ip.add_veth(tap_name, device_name)
+        self._ivs_add_port(tap_name, port_id, mac_address)
 
-            self._ivs_add_port(tap_name, port_id, mac_address)
+        ns_dev = ip.device(device_name)
+        ns_dev.link.set_address(mac_address)
 
-            ns_dev = ip.device(device_name)
-            ns_dev.link.set_address(mac_address)
+        if self.conf.network_device_mtu:
+            ns_dev.link.set_mtu(self.conf.network_device_mtu)
+            root_dev.link.set_mtu(self.conf.network_device_mtu)
 
-            if self.conf.network_device_mtu:
-                ns_dev.link.set_mtu(self.conf.network_device_mtu)
-                root_dev.link.set_mtu(self.conf.network_device_mtu)
+        if namespace:
+            namespace_obj = ip.ensure_namespace(namespace)
+            namespace_obj.add_device_to_namespace(ns_dev)
 
-            if namespace:
-                namespace_obj = ip.ensure_namespace(namespace)
-                namespace_obj.add_device_to_namespace(ns_dev)
-
-            ns_dev.link.set_up()
-            root_dev.link.set_up()
-        else:
-            LOG.info(_LI("Device %s already exists"), device_name)
+        ns_dev.link.set_up()
+        root_dev.link.set_up()
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         """Unplug the interface."""
@@ -395,29 +392,25 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver):
 
     DEV_NAME_PREFIX = 'ns-'
 
-    def plug(self, network_id, port_id, device_name, mac_address,
-             bridge=None, namespace=None, prefix=None):
+    def plug_new(self, network_id, port_id, device_name, mac_address,
+                 bridge=None, namespace=None, prefix=None):
         """Plugin the interface."""
-        if not ip_lib.device_exists(device_name, namespace=namespace):
-            ip = ip_lib.IPWrapper()
+        ip = ip_lib.IPWrapper()
 
-            # Enable agent to define the prefix
-            tap_name = device_name.replace(prefix or self.DEV_NAME_PREFIX,
-                                        n_const.TAP_DEVICE_PREFIX)
-            # Create ns_veth in a namespace if one is configured.
-            root_veth, ns_veth = ip.add_veth(tap_name, device_name,
-                                             namespace2=namespace)
-            ns_veth.link.set_address(mac_address)
+        # Enable agent to define the prefix
+        tap_name = device_name.replace(prefix or self.DEV_NAME_PREFIX,
+                                       n_const.TAP_DEVICE_PREFIX)
+        # Create ns_veth in a namespace if one is configured.
+        root_veth, ns_veth = ip.add_veth(tap_name, device_name,
+                                         namespace2=namespace)
+        ns_veth.link.set_address(mac_address)
 
-            if self.conf.network_device_mtu:
-                root_veth.link.set_mtu(self.conf.network_device_mtu)
-                ns_veth.link.set_mtu(self.conf.network_device_mtu)
+        if self.conf.network_device_mtu:
+            root_veth.link.set_mtu(self.conf.network_device_mtu)
+            ns_veth.link.set_mtu(self.conf.network_device_mtu)
 
-            root_veth.link.set_up()
-            ns_veth.link.set_up()
-
-        else:
-            LOG.info(_LI("Device %s already exists"), device_name)
+        root_veth.link.set_up()
+        ns_veth.link.set_up()
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         """Unplug the interface."""
@@ -471,8 +464,8 @@ class MetaInterfaceDriver(LinuxInterfaceDriver):
         driver = self._get_driver_by_network_id(port.network_id)
         return driver.get_device_name(port)
 
-    def plug(self, network_id, port_id, device_name, mac_address,
-             bridge=None, namespace=None, prefix=None):
+    def plug_new(self, network_id, port_id, device_name, mac_address,
+                 bridge=None, namespace=None, prefix=None):
         driver = self._get_driver_by_network_id(network_id)
         ret = driver.plug(network_id, port_id, device_name, mac_address,
                           bridge=bridge, namespace=namespace, prefix=prefix)

diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py
index 058452ce9a8d49ea87a1297dc4e1331fd06564f6..87b6afffbaa215e6e34043ba983728d3495ce485 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_agent.py
+++ b/neutron/tests/unit/agent/l3/test_agent.py
@@ -764,7 +764,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
                                      'port_id': _uuid()}]}
         router[l3_constants.FLOATINGIP_KEY] = fake_fip['floatingips']
         ri.external_gateway_updated(ex_gw_port, interface_name)
-        self.assertEqual(self.mock_driver.plug.call_count, 0)
+        self.assertEqual(1, self.mock_driver.plug.call_count)
         self.assertEqual(self.mock_driver.init_l3.call_count, 1)
         exp_arp_calls = [mock.call(ri.ns_name, interface_name,
                                    '20.0.0.30', mock.ANY)]

diff --git a/neutron/tests/unit/agent/linux/test_interface.py b/neutron/tests/unit/agent/linux/test_interface.py
index df605eb57cb34e1707310bbdd31ce6c727be2a53..0cafc8add973523de2099b3cefdc7e2c18f12e2b 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_interface.py
+++ b/neutron/tests/unit/agent/linux/test_interface.py
@@ -27,7 +27,7 @@ from neutron.tests import base
 
 
 class BaseChild(interface.LinuxInterfaceDriver):
-    def plug(*args):
+    def plug_new(*args):
         pass
 
     def unplug(*args):