From 2cd2b0ee5ff3b4d07ee3cd726e103d6c52518e65 Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Mon, 10 Feb 2014 15:24:54 +0900 Subject: [PATCH] Raise an error from ovs_lib list operations Previously list operations in ovs_lib returns an empty list if RuntimeError occurs and a caller cannot distinguish an error from normal results. This commit changes ovs_lib list operations (get_vif_port_set, get_vif_ports, get_bridges) to raise an exception when RuntimeError occurs. Note: callers of these commands are ovs/nec/ryu-agent and ovs_cleanup. - plugin agents: these commands are inside in try/except clause in daemon loop and there is no need to change. - ovs_cleanup: there is no error catch logic in main() at now and it calls commands other than ovs_lib, so it can be cleanup later if required. It also fixes the code to use excutils.save_and_reraise_exception when reraising an exception. Change-Id: I2aa3b51b8661c75846cb588c08c8f8ee00c37004 Closes-Bug: #1277029 --- neutron/agent/linux/ovs_lib.py | 51 +++++++++++-------- .../tests/unit/openvswitch/test_ovs_lib.py | 33 ++++++++---- 2 files changed, 54 insertions(+), 30 deletions(-) diff --git a/neutron/agent/linux/ovs_lib.py b/neutron/agent/linux/ovs_lib.py index 45e672096..3993cf824 100644 --- a/neutron/agent/linux/ovs_lib.py +++ b/neutron/agent/linux/ovs_lib.py @@ -24,6 +24,7 @@ from oslo.config import cfg from neutron.agent.linux import ip_lib from neutron.agent.linux import utils +from neutron.openstack.common import excutils from neutron.openstack.common import jsonutils from neutron.openstack.common import log as logging from neutron.plugins.common import constants as p_const @@ -68,10 +69,12 @@ class BaseOVS(object): try: return utils.execute(full_args, root_helper=self.root_helper) except Exception as e: - LOG.error(_("Unable to execute %(cmd)s. Exception: %(exception)s"), - {'cmd': full_args, 'exception': e}) - if check_error: - raise + with excutils.save_and_reraise_exception() as ctxt: + LOG.error(_("Unable to execute %(cmd)s. " + "Exception: %(exception)s"), + {'cmd': full_args, 'exception': e}) + if not check_error: + ctxt.reraise = False def add_bridge(self, bridge_name): self.run_vsctl(["--", "--may-exist", "add-br", bridge_name]) @@ -83,17 +86,19 @@ class BaseOVS(object): try: self.run_vsctl(['br-exists', bridge_name], check_error=True) except RuntimeError as e: - if 'Exit code: 2\n' in str(e): - return False - raise + with excutils.save_and_reraise_exception() as ctxt: + if 'Exit code: 2\n' in str(e): + ctxt.reraise = False + return False return True def get_bridge_name_for_port_name(self, port_name): try: return self.run_vsctl(['port-to-br', port_name], check_error=True) except RuntimeError as e: - if 'Exit code: 1\n' not in str(e): - raise + with excutils.save_and_reraise_exception() as ctxt: + if 'Exit code: 1\n' in str(e): + ctxt.reraise = False def port_exists(self, port_name): return bool(self.get_bridge_name_for_port_name(port_name)) @@ -281,15 +286,15 @@ class OVSBridge(BaseOVS): "type=patch", "options:peer=%s" % remote_name]) return self.get_port_ofport(local_name) - def db_get_map(self, table, record, column): - output = self.run_vsctl(["get", table, record, column]) + def db_get_map(self, table, record, column, check_error=False): + output = self.run_vsctl(["get", table, record, column], check_error) if output: output_str = output.rstrip("\n\r") return self.db_str_to_map(output_str) return {} - def db_get_val(self, table, record, column): - output = self.run_vsctl(["get", table, record, column]) + def db_get_val(self, table, record, column, check_error=False): + output = self.run_vsctl(["get", table, record, column], check_error) if output: return output.rstrip("\n\r") @@ -304,7 +309,7 @@ class OVSBridge(BaseOVS): return ret def get_port_name_list(self): - res = self.run_vsctl(["list-ports", self.br_name]) + res = self.run_vsctl(["list-ports", self.br_name], check_error=True) if res: return res.strip().split("\n") return [] @@ -318,16 +323,20 @@ class OVSBridge(BaseOVS): try: return utils.execute(args, root_helper=self.root_helper).strip() except Exception as e: - LOG.error(_("Unable to execute %(cmd)s. Exception: %(exception)s"), - {'cmd': args, 'exception': e}) + with excutils.save_and_reraise_exception(): + LOG.error(_("Unable to execute %(cmd)s. " + "Exception: %(exception)s"), + {'cmd': args, 'exception': e}) # returns a VIF object for each VIF port def get_vif_ports(self): edge_ports = [] port_names = self.get_port_name_list() for name in port_names: - external_ids = self.db_get_map("Interface", name, "external_ids") - ofport = self.db_get_val("Interface", name, "ofport") + external_ids = self.db_get_map("Interface", name, "external_ids", + check_error=True) + ofport = self.db_get_val("Interface", name, "ofport", + check_error=True) if "iface-id" in external_ids and "attached-mac" in external_ids: p = VifPort(name, ofport, external_ids["iface-id"], external_ids["attached-mac"], self) @@ -348,7 +357,7 @@ class OVSBridge(BaseOVS): edge_ports = set() args = ['--format=json', '--', '--columns=name,external_ids', 'list', 'Interface'] - result = self.run_vsctl(args) + result = self.run_vsctl(args, check_error=True) if not result: return edge_ports for row in jsonutils.loads(result)['data']: @@ -419,8 +428,8 @@ def get_bridges(root_helper): try: return utils.execute(args, root_helper=root_helper).strip().split("\n") except Exception as e: - LOG.exception(_("Unable to retrieve bridges. Exception: %s"), e) - return [] + with excutils.save_and_reraise_exception(): + LOG.exception(_("Unable to retrieve bridges. Exception: %s"), e) def get_installed_ovs_usr_version(root_helper): diff --git a/neutron/tests/unit/openvswitch/test_ovs_lib.py b/neutron/tests/unit/openvswitch/test_ovs_lib.py index 4950d3664..7f0e9f237 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_lib.py +++ b/neutron/tests/unit/openvswitch/test_ovs_lib.py @@ -454,10 +454,10 @@ class OVS_Lib_Test(base.BaseTestCase): get_xapi_iface_id.assert_called_once_with('tap99id') def test_get_vif_ports_nonxen(self): - self._test_get_vif_ports(False) + self._test_get_vif_ports(is_xen=False) def test_get_vif_ports_xen(self): - self._test_get_vif_ports(True) + self._test_get_vif_ports(is_xen=True) def test_get_vif_port_set_nonxen(self): self._test_get_vif_port_set(False) @@ -465,19 +465,24 @@ class OVS_Lib_Test(base.BaseTestCase): def test_get_vif_port_set_xen(self): self._test_get_vif_port_set(True) - def test_get_vif_port_set_list_ports_error(self): + def test_get_vif_ports_list_ports_error(self): expected_calls_and_values = [ (mock.call(["ovs-vsctl", self.TO, "list-ports", self.BR_NAME], root_helper=self.root_helper), RuntimeError()), - (mock.call(["ovs-vsctl", self.TO, "--format=json", - "--", "--columns=name,external_ids", - "list", "Interface"], + ] + tools.setup_mock_calls(self.execute, expected_calls_and_values) + self.assertRaises(RuntimeError, self.br.get_vif_ports) + tools.verify_mock_calls(self.execute, expected_calls_and_values) + + def test_get_vif_port_set_list_ports_error(self): + expected_calls_and_values = [ + (mock.call(["ovs-vsctl", self.TO, "list-ports", self.BR_NAME], root_helper=self.root_helper), - self._encode_ovs_json(['name', 'external_ids'], [])) + RuntimeError()), ] tools.setup_mock_calls(self.execute, expected_calls_and_values) - self.assertEqual(set(), self.br.get_vif_port_set()) + self.assertRaises(RuntimeError, self.br.get_vif_port_set) tools.verify_mock_calls(self.execute, expected_calls_and_values) def test_get_vif_port_set_list_interface_error(self): @@ -492,7 +497,7 @@ class OVS_Lib_Test(base.BaseTestCase): RuntimeError()), ] tools.setup_mock_calls(self.execute, expected_calls_and_values) - self.assertEqual(set(), self.br.get_vif_port_set()) + self.assertRaises(RuntimeError, self.br.get_vif_port_set) tools.verify_mock_calls(self.execute, expected_calls_and_values) def test_clear_db_attribute(self): @@ -569,6 +574,16 @@ class OVS_Lib_Test(base.BaseTestCase): mock.call('tap5678') ]) + def test_delete_neutron_ports_list_error(self): + expected_calls_and_values = [ + (mock.call(["ovs-vsctl", self.TO, "list-ports", self.BR_NAME], + root_helper=self.root_helper), + RuntimeError()), + ] + tools.setup_mock_calls(self.execute, expected_calls_and_values) + self.assertRaises(RuntimeError, self.br.delete_ports, all_ports=False) + tools.verify_mock_calls(self.execute, expected_calls_and_values) + def _test_get_bridges(self, exp_timeout=None): bridges = ['br-int', 'br-ex'] root_helper = 'sudo' -- 2.45.2