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>

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)
+        self.int_br.delete_port(cfg.CONF.OVS.int_peer_patch_port)
-        int_br.delete_port(cfg.CONF.OVS.int_peer_patch_port)
+        self.int_br.remove_all_flows()
-        int_br.remove_all_flows()
         # switch all traffic using L2 learning
-        int_br.add_flow(priority=1, actions="normal")
+        self.int_br.add_flow(priority=1, actions="normal")
-        return int_br
 
     def setup_ancillary_bridges(self, integ_br, tun_br):
         '''Setup ancillary bridges - for example br-ex.'''

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,
+        with mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.'
-                               'delete_flows') as delete_flows_func:
+                        'set_db_attribute',
-            self.agent.port_bound(port, net_uuid, 'local', None, None)
+                        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,
+        with mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.'
-                               'add_flow') as add_flow_func:
+                        'set_db_attribute',
-            self.agent.port_dead(mock.Mock())
+                        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(

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,