From 41db09c434281977fb02196278613b1284990525 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Mon, 22 Jul 2019 13:29:28 +0200 Subject: [PATCH] Don't crash ovs agent during reconfigure of phys bridges In case when physical bridge was recreated on host, ovs agent is trying to reconfigure it. If there will be e.g. timeout while getting bridge's datapath_id, RuntimeError will be raised and that caused crash of whole agent. This patch changes that to not crash agent in such case but try to reconfigure everything in next rpc_loop iteration once again. Conflicts: neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py Change-Id: Ic9b17a420068c0c76748e2c24d97be1ed7c460c7 Closes-Bug: #1837380 (cherry picked from commit b63809715a0b9b8ea0f354dfd6f1b3fd7a713352) --- .../openvswitch/agent/ovs_neutron_agent.py | 21 ++++++++++++++++++- .../agent/test_ovs_neutron_agent.py | 21 +++++++++++++++---- 2 files changed, 37 insertions(+), 5 deletions(-) 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 986bce4ea97..4865804243f 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -185,6 +185,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.setup_physical_bridges(self.bridge_mappings) @@ -1124,6 +1126,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: @@ -2151,8 +2168,10 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, continue # Check if any physical bridge wasn't recreated recently if bridges_monitor: + added_bridges = ( + bridges_monitor.bridges_added + self.added_bridges) bridges_recreated = self._reconfigure_physical_bridges( - bridges_monitor.bridges_added) + added_bridges) sync |= bridges_recreated # Notify the plugin of tunnel IP if self.enable_tunneling and tunnel_sync: 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 13a46ff1df1..088e56e193d 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 @@ -1766,7 +1766,7 @@ 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'), @@ -1774,6 +1774,9 @@ class TestOvsNeutronAgent(object): phys_bridges = {'physnet0': ex_br_mocks[0], 'physnet1': ex_br_mocks[1]}, bm_mock = mock.Mock() + bridges_added = ['br-ex0'] + expected_added_bridges = ( + bridges_added if setup_bridges_side_effect else []) with mock.patch( 'neutron.agent.linux.ovsdb_monitor.get_bridges_monitor', return_value=bm_mock),\ @@ -1792,14 +1795,24 @@ class TestOvsNeutronAgent(object): mock.patch.object( self.agent, 'setup_physical_bridges') as setup_physical_bridges: - bm_mock.bridges_added = ['br-ex0'] + bm_mock.bridges_added = bridges_added + setup_physical_bridges.side_effect = setup_bridges_side_effect try: self.agent.rpc_loop(polling_manager=mock.Mock(), bridges_monitor=bm_mock) except TypeError: pass - setup_physical_bridges.assert_called_once_with( - {'physnet0': 'br-ex0'}) + # 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")