From b63809715a0b9b8ea0f354dfd6f1b3fd7a713352 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. Change-Id: Ic9b17a420068c0c76748e2c24d97be1ed7c460c7 Closes-Bug: #1837380 --- .../openvswitch/agent/ovs_neutron_agent.py | 20 ++++++++++++++++++- .../agent/test_ovs_neutron_agent.py | 18 +++++++++++++++-- 2 files changed, 35 insertions(+), 3 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 c6e9073fb67..f3ab474b32b 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/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: 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 5700c23c851..375559c8381 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 @@ -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")