Merge "Don't crash ovs agent during reconfigure of phys bridges"

neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py

@@ -193,6 +193,8 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
         # keeps association between ports and ofports to detect ofport change
         self.vifname_to_ofport_map = {}
         self.setup_rpc()
+        # Stores newly created bridges
+        self.added_bridges = list()
         self.bridge_mappings = self._parse_bridge_mappings(
             ovs_conf.bridge_mappings)
         self.rp_bandwidths = place_utils.parse_rp_bandwidths(
@@ -1308,6 +1310,21 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
                                         self.arp_responder_enabled)
 
     def _reconfigure_physical_bridges(self, bridges):
+        try:
+            sync = self._do_reconfigure_physical_bridges(bridges)
+            self.added_bridges = []
+        except RuntimeError:
+            # If there was error and bridges aren't properly reconfigured,
+            # there is no need to do full sync once again. It will be done when
+            # reconfiguration of physical bridges will be finished without
+            # errors
+            sync = False
+            self.added_bridges = bridges
+            LOG.warning("RuntimeError during setup of physical bridges: %s",
+                        bridges)
+        return sync
+
+    def _do_reconfigure_physical_bridges(self, bridges):
         sync = False
         bridge_mappings = {}
         for bridge in bridges:
@@ -2341,8 +2358,9 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
                 self.loop_count_and_wait(start, port_stats)
                 continue
             # Check if any physical bridge wasn't recreated recently
+            added_bridges = idl_monitor.bridges_added + self.added_bridges
             bridges_recreated = self._reconfigure_physical_bridges(
-                idl_monitor.bridges_added)
+                added_bridges)
             sync |= bridges_recreated
             # Notify the plugin of tunnel IP
             if self.enable_tunneling and tunnel_sync:

neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py

@@ -1834,13 +1834,16 @@ class TestOvsNeutronAgent(object):
             self.agent.reclaim_local_vlan('net2')
             tun_br.delete_port.assert_called_once_with('gre-02020202')
 
-    def test_ext_br_recreated(self):
+    def _test_ext_br_recreated(self, setup_bridges_side_effect):
         bridge_mappings = {'physnet0': 'br-ex0',
                            'physnet1': 'br-ex1'}
         ex_br_mocks = [mock.Mock(br_name='br-ex0'),
                        mock.Mock(br_name='br-ex1')]
         phys_bridges = {'physnet0': ex_br_mocks[0],
                         'physnet1': ex_br_mocks[1]},
+        bridges_added = ['br-ex0']
+        expected_added_bridges = (
+            bridges_added if setup_bridges_side_effect else [])
         with mock.patch.object(self.agent, 'check_ovs_status',
                                return_value=constants.OVS_NORMAL), \
                 mock.patch.object(self.agent, '_agent_has_updates',
@@ -1853,12 +1856,23 @@ class TestOvsNeutronAgent(object):
                 setup_physical_bridges, \
                 mock.patch.object(self.agent.ovs.ovsdb, 'idl_monitor') as \
                 mock_idl_monitor:
-            mock_idl_monitor.bridges_added = ['br-ex0']
+            mock_idl_monitor.bridges_added = bridges_added
+            setup_physical_bridges.side_effect = setup_bridges_side_effect
             try:
                 self.agent.rpc_loop(polling_manager=mock.Mock())
             except TypeError:
                 pass
+        # Setup bridges should be called once even if it will raise Runtime
+        # Error because there is raised TypeError in _agent_has_updates to stop
+        # agent after first loop iteration
         setup_physical_bridges.assert_called_once_with({'physnet0': 'br-ex0'})
+        self.assertEqual(expected_added_bridges, self.agent.added_bridges)
+
+    def test_ext_br_recreated(self):
+        self._test_ext_br_recreated(setup_bridges_side_effect=None)
+
+    def test_ext_br_recreated_fail_setup_physical_bridge(self):
+        self._test_ext_br_recreated(setup_bridges_side_effect=RuntimeError)
 
     def test_daemon_loop_uses_polling_manager(self):
         ex_br_mock = mock.Mock(br_name="br-ex0")