Remove ovs-vsctl call from OVSInterfaceDriver

ovs_lib module is responsible for the interaction with OVS but
OVSInterfaceDriver._ovs_add_port() method calls directly
ovs-vsctl which implies inconsistencies: ovs_lib calls ovs-vsctl
with a timeout but not OVSInterfaceDriver._ovs_add_port().

This change moves ovs-vsctl call in OVSInterfaceDriver._ovs_add_port
to ovs_lib.

Closes-Bug: #1396489
Change-Id: I4d03f8beef2c5f2366ad2d1cf5f0b18b4f2857cd

diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py
index fdc1d8e3f684be6088f4ce9cde91501118cd9313..0d7ed71c32e588e9e12393542ec43e6382896966 100644 (file)
--- a/neutron/agent/linux/interface.py
+++ b/neutron/agent/linux/interface.py
@@ -207,17 +207,14 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
 
     def _ovs_add_port(self, bridge, device_name, port_id, mac_address,
                       internal=True):
-        cmd = ['ovs-vsctl', '--', '--if-exists', 'del-port', device_name, '--',
-               'add-port', bridge, device_name]
+        attrs = [('external-ids:iface-id', port_id),
+                 ('external-ids:iface-status', 'active'),
+                 ('external-ids:attached-mac', mac_address)]
         if internal:
-            cmd += ['--', 'set', 'Interface', device_name, 'type=internal']
-        cmd += ['--', 'set', 'Interface', device_name,
-                'external-ids:iface-id=%s' % port_id,
-                '--', 'set', 'Interface', device_name,
-                'external-ids:iface-status=active',
-                '--', 'set', 'Interface', device_name,
-                'external-ids:attached-mac=%s' % mac_address]
-        utils.execute(cmd, self.root_helper)
+            attrs.insert(0, ('type', 'internal'))
+
+        ovs = ovs_lib.OVSBridge(bridge, self.root_helper)
+        ovs.replace_port(device_name, *attrs)
 
     def plug(self, network_id, port_id, device_name, mac_address,
              bridge=None, namespace=None, prefix=None):

diff --git a/neutron/agent/linux/ovs_lib.py b/neutron/agent/linux/ovs_lib.py
index a2a8d4b3a42c1cb79c70000ad14863db41178759..2a61187b9613ff4636991c325f1ca7920d4fedb0 100644 (file)
--- a/neutron/agent/linux/ovs_lib.py
+++ b/neutron/agent/linux/ovs_lib.py
@@ -150,6 +150,15 @@ class OVSBridge(BaseOVS):
                         port_name])
         return self.get_port_ofport(port_name)
 
+    def replace_port(self, port_name, *interface_attr_tuples):
+        """Replace existing port or create it, and configure port interface."""
+        cmd = ['--', '--if-exists', 'del-port', port_name,
+               '--', 'add-port', self.br_name, port_name]
+        if interface_attr_tuples:
+            cmd += ['--', 'set', 'Interface', port_name]
+            cmd += ['%s=%s' % kv for kv in interface_attr_tuples]
+        self.run_vsctl(cmd)
+
     def delete_port(self, port_name):
         self.run_vsctl(["--", "--if-exists", "del-port", self.br_name,
                         port_name])

diff --git a/neutron/tests/unit/agent/linux/test_ovs_lib.py b/neutron/tests/unit/agent/linux/test_ovs_lib.py
index 3d2611e2e894bbf382b65f1bb92d8b0dcefc7ce0..dfe39b52a3ffd211664386c126e56f39c489a177 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_ovs_lib.py
+++ b/neutron/tests/unit/agent/linux/test_ovs_lib.py
@@ -218,6 +218,27 @@ class OVS_Lib_Test(base.BaseTestCase):
     def _build_timeout_opt(self, exp_timeout):
         return "--timeout=%d" % exp_timeout if exp_timeout else self.TO
 
+    def test_replace_port(self):
+        pname = "tap5"
+        self.br.replace_port(pname)
+        self.execute.assert_called_once_with(
+            ["ovs-vsctl", self.TO,
+             "--", "--if-exists", "del-port", pname,
+             "--", "add-port", self.BR_NAME, pname],
+            root_helper=self.root_helper)
+
+    def test_replace_port_with_attrs(self):
+        pname = "tap5"
+        self.br.replace_port(pname, ('type', 'internal'),
+                             ('external-ids:iface-status', 'active'))
+        self.execute.assert_called_once_with(
+            ["ovs-vsctl", self.TO,
+             "--", "--if-exists", "del-port", pname,
+             "--", "add-port", self.BR_NAME, pname,
+             "--", "set", "Interface", pname,
+             "type=internal", "external-ids:iface-status=active"],
+            root_helper=self.root_helper)
+
     def _test_delete_port(self, exp_timeout=None):
         exp_timeout_str = self._build_timeout_opt(exp_timeout)
         pname = "tap5"

diff --git a/neutron/tests/unit/test_linux_interface.py b/neutron/tests/unit/test_linux_interface.py
index 431687c9e6c09018ce7db03b73bdb36954b240bb..0136bd2408c33b8ca5f4d6a36966d21e3eb21d22 100644 (file)
--- a/neutron/tests/unit/test_linux_interface.py
+++ b/neutron/tests/unit/test_linux_interface.py
@@ -19,6 +19,7 @@ from neutron.agent.common import config
 from neutron.agent.linux import dhcp
 from neutron.agent.linux import interface
 from neutron.agent.linux import ip_lib
+from neutron.agent.linux import ovs_lib
 from neutron.agent.linux import utils
 from neutron.extensions import flavor
 from neutron.openstack.common import uuidutils
@@ -212,17 +213,7 @@ class TestOVSInterfaceDriver(TestBase):
         def device_exists(dev, root_helper=None, namespace=None):
             return dev == bridge
 
-        vsctl_cmd = ['ovs-vsctl', '--', '--if-exists', 'del-port',
-                     'tap0', '--', 'add-port',
-                     bridge, 'tap0', '--', 'set', 'Interface', 'tap0',
-                     'type=internal', '--', 'set', 'Interface', 'tap0',
-                     'external-ids:iface-id=port-1234', '--', 'set',
-                     'Interface', 'tap0',
-                     'external-ids:iface-status=active', '--', 'set',
-                     'Interface', 'tap0',
-                     'external-ids:attached-mac=aa:bb:cc:dd:ee:ff']
-
-        with mock.patch.object(utils, 'execute') as execute:
+        with mock.patch.object(ovs_lib.OVSBridge, 'replace_port') as replace:
             ovs = interface.OVSInterfaceDriver(self.conf)
             self.device_exists.side_effect = device_exists
             ovs.plug('01234567-1234-1234-99',
@@ -231,7 +222,12 @@ class TestOVSInterfaceDriver(TestBase):
                      'aa:bb:cc:dd:ee:ff',
                      bridge=bridge,
                      namespace=namespace)
-            execute.assert_called_once_with(vsctl_cmd, 'sudo')
+            replace.assert_called_once_with(
+                'tap0',
+                ('type', 'internal'),
+                ('external-ids:iface-id', 'port-1234'),
+                ('external-ids:iface-status', 'active'),
+                ('external-ids:attached-mac', 'aa:bb:cc:dd:ee:ff'))
 
         expected = [mock.call('sudo'),
                     mock.call().device('tap0'),
@@ -300,15 +296,7 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver):
                     mock.call().add_veth('tap0', devname,
                                          namespace2=namespace)]
 
-        vsctl_cmd = ['ovs-vsctl', '--', '--if-exists', 'del-port',
-                     'tap0', '--', 'add-port',
-                     bridge, 'tap0', '--', 'set', 'Interface', 'tap0',
-                     'external-ids:iface-id=port-1234', '--', 'set',
-                     'Interface', 'tap0',
-                     'external-ids:iface-status=active', '--', 'set',
-                     'Interface', 'tap0',
-                     'external-ids:attached-mac=aa:bb:cc:dd:ee:ff']
-        with mock.patch.object(utils, 'execute') as execute:
+        with mock.patch.object(ovs_lib.OVSBridge, 'replace_port') as replace:
             ovs.plug('01234567-1234-1234-99',
                      'port-1234',
                      devname,
@@ -316,7 +304,11 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver):
                      bridge=bridge,
                      namespace=namespace,
                      prefix=prefix)
-            execute.assert_called_once_with(vsctl_cmd, 'sudo')
+            replace.assert_called_once_with(
+                'tap0',
+                ('external-ids:iface-id', 'port-1234'),
+                ('external-ids:iface-status', 'active'),
+                ('external-ids:attached-mac', 'aa:bb:cc:dd:ee:ff'))
 
         ns_dev.assert_has_calls(
             [mock.call.link.set_address('aa:bb:cc:dd:ee:ff')])