From: Terry Wilson Date: Wed, 6 Jan 2016 07:19:31 +0000 (-0600) Subject: Make sure datapath_type is updated on bridges changed X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=71fa1820008b998809890c52b366c8acf9cb72a4;p=openstack-build%2Fneutron-build.git Make sure datapath_type is updated on bridges changed 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 --- diff --git a/neutron/agent/ovsdb/native/commands.py b/neutron/agent/ovsdb/native/commands.py index dd977a7e4..23c2eae53 100644 --- a/neutron/agent/ovsdb/native/commands.py +++ b/neutron/agent/ovsdb/native/commands.py @@ -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 diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 08323bef6..3ac145ce4 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -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 diff --git a/neutron/tests/functional/agent/l2/base.py b/neutron/tests/functional/agent/l2/base.py index 66609b8f6..c2db452ed 100644 --- a/neutron/tests/functional/agent/l2/base.py +++ b/neutron/tests/functional/agent/l2/base.py @@ -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 diff --git a/neutron/tests/functional/agent/test_l2_ovs_agent.py b/neutron/tests/functional/agent/test_l2_ovs_agent.py index ffdc0ff04..763b26a42 100644 --- a/neutron/tests/functional/agent/test_l2_ovs_agent.py +++ b/neutron/tests/functional/agent/test_l2_ovs_agent.py @@ -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( diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 2b1873fa0..ecf9342a1 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -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'), diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py index 5bf9cd20f..a3bb97e21 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py @@ -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()),