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 <stephen.gran@guardian.co.uk>

diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
index 6cdca563c23b31422f0c9c034bc421aa95ebaa64..95531f0fbf35893a364a41e33571433fcc22ca8e 100644 (file)
--- 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 2764a9592e250d495d2fc39ced1a8d031c9c7b68..294f30ee27c14cd7aaa2df13b04421dce7eec8c6 100644 (file)
--- 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 c9f0f5791b2f58743bf53de4d0870db7cd7978e3..c0390841b2720579fc8ea84a49e7b03a2d287d39 100644 (file)
--- 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,