]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Make sure datapath_type is updated on bridges changed
authorTerry Wilson <twilson@redhat.com>
Wed, 6 Jan 2016 07:19:31 +0000 (01:19 -0600)
committerTerry Wilson <twilson@redhat.com>
Tue, 12 Jan 2016 16:10:49 +0000 (16:10 +0000)
When changing datapath_type in the config, physical and tunnel bridges
do not have their datapath_type updated. Calling create() on already
created bridges should be safe as it passes '--may-exist' when adding
the bridge, which will do nothing if the bridge already exists, but
the second part of the transaction will still update things like
datapath_type.

It should be noted that ancillary bridges (like br-ex) are not
modified by this patch as datapath_type was never applied to them to
begin with.

Incidentally, the native and vsctl versions behaved slightly
differently when handling datapath_type: vsctl builds the multi-cmd
transaction with add-br ... -- set ..., so that the second cmd would
actually complete. The native just bailed if may_exist and the bridge
existed. This is fixed as part of this patch.

Change-Id: Ib8bc817c7bc724d80193d0ca7af480a7ea103f77
Closes-Bug: 1532273

neutron/agent/ovsdb/native/commands.py
neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
neutron/tests/functional/agent/l2/base.py
neutron/tests/functional/agent/test_l2_ovs_agent.py
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py

index dd977a7e4c399c00d412c0999902b859558cd7c1..23c2eae5327557ac6eee4666d20f1550e861f282 100644 (file)
@@ -61,6 +61,8 @@ class AddBridgeCommand(BaseCommand):
             br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name',
                                        self.name, None)
             if br:
+                if self.datapath_type:
+                    br.datapath_type = self.datapath_type
                 return
         row = txn.insert(self.api._tables['Bridge'])
         row.name = self.name
index 08323bef69a388f97b711237909df0d9cb837a9f..3ac145ce46080ebe456e1b022c21f87632451b7d 100644 (file)
@@ -998,8 +998,9 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
             self.tun_br = self.br_tun_cls(tun_br_name)
         self.tun_br.set_agent_uuid_stamp(self.agent_uuid_stamp)
 
-        if not self.tun_br.bridge_exists(self.tun_br.br_name):
-            self.tun_br.create(secure_mode=True)
+        # tun_br.create() won't recreate bridge if it exists, but will handle
+        # cases where something like datapath_type has changed
+        self.tun_br.create(secure_mode=True)
         self.tun_br.setup_controllers(self.conf)
         if (not self.int_br.port_exists(self.conf.OVS.int_peer_patch_port) or
                 self.patch_tun_ofport == ovs_lib.INVALID_OFPORT):
@@ -1057,6 +1058,9 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                            'bridge': bridge})
                 sys.exit(1)
             br = self.br_phys_cls(bridge)
+            # The bridge already exists, so create won't recreate it, but will
+            # handle things like changing the datapath_type
+            br.create()
             br.setup_controllers(self.conf)
             br.setup_default_table()
             self.phys_brs[physical_network] = br
index 66609b8f6abcdefbc553cd382629e4e9b5ee2640..c2db452edbea3c32855503505bb2506498929615 100644 (file)
@@ -58,6 +58,8 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
                                          prefix='br-int')
         self.br_tun = base.get_rand_name(n_const.DEVICE_NAME_MAX_LEN,
                                          prefix='br-tun')
+        self.br_phys = base.get_rand_name(n_const.DEVICE_NAME_MAX_LEN,
+                                          prefix='br-phys')
         patch_name_len = n_const.DEVICE_NAME_MAX_LEN - len("-patch-tun")
         self.patch_tun = "%s-patch-tun" % self.br_int[patch_name_len:]
         self.patch_int = "%s-patch-int" % self.br_tun[patch_name_len:]
@@ -102,17 +104,20 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
             tunnel_types = [p_const.TYPE_VXLAN]
         else:
             tunnel_types = None
-        bridge_mappings = ['physnet:%s' % self.br_int]
+        bridge_mappings = ['physnet:%s' % self.br_phys]
         self.config.set_override('tunnel_types', tunnel_types, "AGENT")
         self.config.set_override('polling_interval', 1, "AGENT")
         self.config.set_override('prevent_arp_spoofing', False, "AGENT")
         self.config.set_override('local_ip', '192.168.10.1', "OVS")
         self.config.set_override('bridge_mappings', bridge_mappings, "OVS")
+        # Physical bridges should be created prior to running
+        self._bridge_classes()['br_phys'](self.br_phys).create()
         agent = ovs_agent.OVSNeutronAgent(self._bridge_classes(),
                                           self.config)
         self.addCleanup(self.ovs.delete_bridge, self.br_int)
         if tunnel_types:
             self.addCleanup(self.ovs.delete_bridge, self.br_tun)
+        self.addCleanup(self.ovs.delete_bridge, self.br_phys)
         agent.sg_agent = mock.Mock()
         agent.ancillary_brs = []
         return agent
index ffdc0ff04e99fcbb41bec944c8bc4e83f9000880..763b26a4242dd3c33c94712ef016707cd519225c 100644 (file)
@@ -41,14 +41,14 @@ class TestOVSAgent(base.OVSAgentTestFramework):
                                      "OVS")
         agent = self.create_agent()
         self.start_agent(agent)
-        actual = self.ovs.db_get_val('Bridge',
-                                     agent.int_br.br_name,
-                                     'datapath_type')
-        self.assertEqual(expected, actual)
-        actual = self.ovs.db_get_val('Bridge',
-                                     agent.tun_br.br_name,
-                                     'datapath_type')
-        self.assertEqual(expected, actual)
+        for br_name in (getattr(self, br) for br in
+                        ('br_int', 'br_tun', 'br_phys')):
+            actual = self.ovs.db_get_val('Bridge', br_name, 'datapath_type')
+            self.assertEqual(expected, actual)
+
+    def test_datapath_type_change(self):
+        self._check_datapath_type_netdev('system')
+        self._check_datapath_type_netdev('netdev')
 
     def test_datapath_type_netdev(self):
         self._check_datapath_type_netdev(
index 2b1873fa0535c36115ea3941e8cf8d89499e8307..ecf9342a1f447944839b3e8a8fc4a2eb7481bdd9 100644 (file)
@@ -980,6 +980,7 @@ class TestOvsNeutronAgent(object):
             self.agent.setup_physical_bridges({"physnet1": "br-eth"})
             expected_calls = [
                 mock.call.phys_br_cls('br-eth'),
+                mock.call.phys_br.create(),
                 mock.call.phys_br.setup_controllers(mock.ANY),
                 mock.call.phys_br.setup_default_table(),
                 mock.call.int_br.db_get_val('Interface', 'int-br-eth',
@@ -1060,6 +1061,7 @@ class TestOvsNeutronAgent(object):
             self.agent.setup_physical_bridges({"physnet1": "br-eth"})
             expected_calls = [
                 mock.call.phys_br_cls('br-eth'),
+                mock.call.phys_br.create(),
                 mock.call.phys_br.setup_controllers(mock.ANY),
                 mock.call.phys_br.setup_default_table(),
                 mock.call.int_br.delete_port('int-br-eth'),
index 5bf9cd20fc207c6327d28e500266afc4208fad4c..a3bb97e21eb10824ce8a572b8ff6f4d4fe1477a5 100644 (file)
@@ -188,6 +188,7 @@ class TunnelTest(object):
         ]
 
         self.mock_map_tun_bridge_expected = [
+            mock.call.create(),
             mock.call.setup_controllers(mock.ANY),
             mock.call.setup_default_table(),
             mock.call.add_patch_port('phy-%s' % self.MAP_TUN_BRIDGE,
@@ -214,8 +215,7 @@ class TunnelTest(object):
 
         self.mock_tun_bridge_expected = [
             mock.call.set_agent_uuid_stamp(mock.ANY),
-            mock.call.bridge_exists(mock.ANY),
-            nonzero(mock.call.bridge_exists()),
+            mock.call.create(secure_mode=True),
             mock.call.setup_controllers(mock.ANY),
             mock.call.port_exists('patch-int'),
             nonzero(mock.call.port_exists()),
@@ -627,6 +627,7 @@ class TunnelTestUseVethInterco(TunnelTest):
         ]
 
         self.mock_map_tun_bridge_expected = [
+            mock.call.create(),
             mock.call.setup_controllers(mock.ANY),
             mock.call.setup_default_table(),
             mock.call.add_port(self.intb),
@@ -646,8 +647,7 @@ class TunnelTestUseVethInterco(TunnelTest):
 
         self.mock_tun_bridge_expected = [
             mock.call.set_agent_uuid_stamp(mock.ANY),
-            mock.call.bridge_exists(mock.ANY),
-            nonzero(mock.call.bridge_exists()),
+            mock.call.create(secure_mode=True),
             mock.call.setup_controllers(mock.ANY),
             mock.call.port_exists('patch-int'),
             nonzero(mock.call.port_exists()),