From d72848c9afba40d21235a4e95cf8e69549290dca Mon Sep 17 00:00:00 2001 From: Stephen Gran Date: Thu, 29 Aug 2013 07:11:44 +0100 Subject: [PATCH] Create RPC connection before modifying OVS bridges On startup, the agent removes and readds flows to the OVS bridges. If an RPC setup error exits the process prematurely, this can leave the bridges in an unsafe state. It is better to set the RPC communication up before making changes to the host system. Closes-Bug: 1217980 Change-Id: Ib9bbb864b9129bb7b1376a150a37a0c07908d74b Signed-off-by: Stephen Gran --- .../openvswitch/agent/ovs_neutron_agent.py | 34 +++++++++---------- .../openvswitch/test_ovs_neutron_agent.py | 24 +++++++++---- .../tests/unit/openvswitch/test_ovs_tunnel.py | 2 +- 3 files changed, 36 insertions(+), 24 deletions(-) diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index 6cdca563c..95531f0fb 100644 --- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py @@ -169,7 +169,19 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): self.root_helper = root_helper self.available_local_vlans = set(xrange(q_const.MIN_VLAN_TAG, q_const.MAX_VLAN_TAG)) - self.int_br = self.setup_integration_br(integ_br) + self.tunnel_types = tunnel_types or [] + self.agent_state = { + 'binary': 'neutron-openvswitch-agent', + 'host': cfg.CONF.host, + 'topic': q_const.L2_AGENT_TOPIC, + 'configurations': bridge_mappings, + 'agent_type': q_const.AGENT_TYPE_OVS, + 'tunnel_types': self.tunnel_types, + 'start_flag': True} + + self.int_br = ovs_lib.OVSBridge(integ_br, self.root_helper) + self.setup_rpc() + self.setup_integration_br() self.setup_physical_bridges(bridge_mappings) self.local_vlan_map = {} self.tun_br_ofports = {constants.TYPE_GRE: set(), @@ -183,22 +195,12 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): self.enable_tunneling = False self.local_ip = local_ip self.tunnel_count = 0 - self.tunnel_types = tunnel_types or [] self.vxlan_udp_port = cfg.CONF.AGENT.vxlan_udp_port self._check_ovs_version() if self.enable_tunneling: self.setup_tunnel_br(tun_br) # Collect additional bridges to monitor self.ancillary_brs = self.setup_ancillary_bridges(integ_br, tun_br) - self.agent_state = { - 'binary': 'neutron-openvswitch-agent', - 'host': cfg.CONF.host, - 'topic': q_const.L2_AGENT_TOPIC, - 'configurations': bridge_mappings, - 'agent_type': q_const.AGENT_TYPE_OVS, - 'tunnel_types': self.tunnel_types, - 'start_flag': True} - self.setup_rpc() # Keep track of int_br's device count for use by _report_state() self.int_br_device_count = 0 @@ -518,7 +520,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): DEAD_VLAN_TAG) self.int_br.add_flow(priority=2, in_port=port.ofport, actions="drop") - def setup_integration_br(self, bridge_name): + def setup_integration_br(self): '''Setup the integration bridge. Create patch ports and remove all existing flows. @@ -526,12 +528,10 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): :param bridge_name: the name of the integration bridge. :returns: the integration bridge ''' - int_br = ovs_lib.OVSBridge(bridge_name, self.root_helper) - int_br.delete_port(cfg.CONF.OVS.int_peer_patch_port) - int_br.remove_all_flows() + self.int_br.delete_port(cfg.CONF.OVS.int_peer_patch_port) + self.int_br.remove_all_flows() # switch all traffic using L2 learning - int_br.add_flow(priority=1, actions="normal") - return int_br + self.int_br.add_flow(priority=1, actions="normal") def setup_ancillary_bridges(self, integ_br, tun_br): '''Setup ancillary bridges - for example br-ex.''' diff --git a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py index 2764a9592..294f30ee2 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py @@ -106,6 +106,9 @@ class TestOvsNeutronAgent(base.BaseTestCase): mock.patch('neutron.plugins.openvswitch.agent.ovs_neutron_agent.' 'OVSNeutronAgent.setup_ancillary_bridges', return_value=[]), + mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' + 'get_local_port_mac', + return_value='00:00:00:00:00:01'), mock.patch('neutron.agent.linux.utils.get_interface_mac', return_value='00:00:00:00:00:01')): self.agent = ovs_neutron_agent.OVSNeutronAgent(**kwargs) @@ -116,9 +119,12 @@ class TestOvsNeutronAgent(base.BaseTestCase): port = mock.Mock() port.ofport = ofport net_uuid = 'my-net-uuid' - with mock.patch.object(self.agent.int_br, - 'delete_flows') as delete_flows_func: - self.agent.port_bound(port, net_uuid, 'local', None, None) + with mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' + 'set_db_attribute', + return_value=True): + with mock.patch.object(self.agent.int_br, + 'delete_flows') as delete_flows_func: + self.agent.port_bound(port, net_uuid, 'local', None, None) self.assertEqual(delete_flows_func.called, ofport != -1) def test_port_bound_deletes_flows_for_valid_ofport(self): @@ -128,9 +134,12 @@ class TestOvsNeutronAgent(base.BaseTestCase): self._mock_port_bound(ofport=-1) def test_port_dead(self): - with mock.patch.object(self.agent.int_br, - 'add_flow') as add_flow_func: - self.agent.port_dead(mock.Mock()) + with mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' + 'set_db_attribute', + return_value=True): + with mock.patch.object(self.agent.int_br, + 'add_flow') as add_flow_func: + self.agent.port_dead(mock.Mock()) self.assertTrue(add_flow_func.called) def mock_update_ports(self, vif_port_set=None, registered_ports=None): @@ -423,6 +432,9 @@ class AncillaryBridgesTest(base.BaseTestCase): return_value=mock.Mock()), mock.patch('neutron.agent.linux.utils.get_interface_mac', return_value='00:00:00:00:00:01'), + mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' + 'get_local_port_mac', + return_value='00:00:00:00:00:01'), mock.patch('neutron.agent.linux.ovs_lib.get_bridges', return_value=bridges), mock.patch( diff --git a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py index c9f0f5791..c0390841b 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py +++ b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py @@ -89,6 +89,7 @@ class TunnelTest(base.BaseTestCase): self.mox.StubOutClassWithMocks(ovs_lib, 'OVSBridge') self.mock_int_bridge = ovs_lib.OVSBridge(self.INT_BRIDGE, 'sudo') + self.mock_int_bridge.get_local_port_mac().AndReturn('000000000001') self.mock_int_bridge.delete_port('patch-tun') self.mock_int_bridge.remove_all_flows() self.mock_int_bridge.add_flow(priority=1, actions='normal') @@ -168,7 +169,6 @@ class TunnelTest(base.BaseTestCase): 'int-tunnel_bridge_mapping', 'phy-tunnel_bridge_mapping').AndReturn([self.inta, self.intb]) - self.mock_int_bridge.get_local_port_mac().AndReturn('000000000001') self.mox.StubOutWithMock(ovs_lib, 'get_bridges') ovs_lib.get_bridges('sudo').AndReturn([self.INT_BRIDGE, self.TUN_BRIDGE, -- 2.45.2