diff --git a/quantum/plugins/cisco/common/cisco_exceptions.py b/quantum/plugins/cisco/common/cisco_exceptions.py index cf1d2020635..030de1669e4 100644 --- a/quantum/plugins/cisco/common/cisco_exceptions.py +++ b/quantum/plugins/cisco/common/cisco_exceptions.py @@ -88,6 +88,11 @@ class CredentialAlreadyExists(exceptions.QuantumException): "for tenant %(tenant_id)s") +class NexusConfigFailed(exceptions.QuantumException): + """Failed to configure Nexus switch.""" + message = _("Failed to configure Nexus: %(config)s. Reason: %(exc)s.") + + class NexusPortBindingNotFound(exceptions.QuantumException): """NexusPort Binding is not present""" message = _("Nexus Port Binding %(port_id)s is not present") diff --git a/quantum/plugins/cisco/nexus/cisco_nexus_network_driver_v2.py b/quantum/plugins/cisco/nexus/cisco_nexus_network_driver_v2.py index fd38d9fefff..ca11d6d16ec 100644 --- a/quantum/plugins/cisco/nexus/cisco_nexus_network_driver_v2.py +++ b/quantum/plugins/cisco/nexus/cisco_nexus_network_driver_v2.py @@ -26,6 +26,7 @@ from ncclient import manager +from quantum.plugins.cisco.common import cisco_exceptions from quantum.plugins.cisco.db import network_db_v2 as cdb from quantum.plugins.cisco.db import nexus_db_v2 from quantum.plugins.cisco.nexus import cisco_nexus_snippets as snipp @@ -41,6 +42,33 @@ class CiscoNEXUSDriver(): def __init__(self): pass + def _edit_config(self, mgr, target='running', config='', + allowed_exc_strs=None): + """Modify switch config for a target config type. + + :param mgr: NetConf client manager + :param target: Target config type + :param config: Configuration string in XML format + :param allowed_exc_strs: Exceptions which have any of these strings + as a subset of their exception message + (str(exception)) can be ignored + + :raises: NexusConfigFailed + + """ + if not allowed_exc_strs: + allowed_exc_strs = [] + try: + mgr.edit_config(target, config=config) + except Exception as e: + for exc_str in allowed_exc_strs: + if exc_str in str(e): + break + else: + # Raise a Quantum exception. Include a description of + # the original ncclient exception. + raise cisco_exceptions.NexusConfigFailed(config=config, exc=e) + def nxos_connect(self, nexus_host, nexus_ssh_port, nexus_user, nexus_password): """ @@ -58,12 +86,33 @@ def create_xml_snippet(self, cutomized_config): return conf_xml_snippet def enable_vlan(self, mgr, vlanid, vlanname): - """ - Creates a VLAN on Nexus Switch given the VLAN ID and Name - """ - confstr = snipp.CMD_VLAN_CONF_SNIPPET % (vlanid, vlanname) - confstr = self.create_xml_snippet(confstr) - mgr.edit_config(target='running', config=confstr) + """Create a VLAN on Nexus Switch given the VLAN ID and Name.""" + confstr = self.create_xml_snippet( + snipp.CMD_VLAN_CONF_SNIPPET % (vlanid, vlanname)) + self._edit_config(mgr, target='running', config=confstr) + + # Enable VLAN active and no-shutdown states. Some versions of + # Nexus switch do not allow state changes for the extended VLAN + # range (1006-4094), but these errors can be ignored (default + # values are appropriate). + state_config = [snipp.CMD_VLAN_ACTIVE_SNIPPET, + snipp.CMD_VLAN_NO_SHUTDOWN_SNIPPET] + for snippet in state_config: + try: + confstr = self.create_xml_snippet(snippet % vlanid) + self._edit_config( + mgr, + target='running', + config=confstr, + allowed_exc_strs=["Can't modify state for extended", + "Command is only allowed on VLAN"]) + except cisco_exceptions.NexusConfigFailed as e: + # Rollback VLAN creation + try: + self.disable_vlan(mgr, vlanid) + finally: + # Re-raise original exception + raise e def disable_vlan(self, mgr, vlanid): """ @@ -71,7 +120,7 @@ def disable_vlan(self, mgr, vlanid): """ confstr = snipp.CMD_NO_VLAN_CONF_SNIPPET % vlanid confstr = self.create_xml_snippet(confstr) - mgr.edit_config(target='running', config=confstr) + self._edit_config(mgr, target='running', config=confstr) def enable_port_trunk(self, mgr, interface): """ @@ -80,7 +129,7 @@ def enable_port_trunk(self, mgr, interface): confstr = snipp.CMD_PORT_TRUNK % (interface) confstr = self.create_xml_snippet(confstr) LOG.debug(_("NexusDriver: %s"), confstr) - mgr.edit_config(target='running', config=confstr) + self._edit_config(mgr, target='running', config=confstr) def disable_switch_port(self, mgr, interface): """ @@ -89,7 +138,7 @@ def disable_switch_port(self, mgr, interface): confstr = snipp.CMD_NO_SWITCHPORT % (interface) confstr = self.create_xml_snippet(confstr) LOG.debug(_("NexusDriver: %s"), confstr) - mgr.edit_config(target='running', config=confstr) + self._edit_config(mgr, target='running', config=confstr) def enable_vlan_on_trunk_int(self, mgr, nexus_switch, interface, vlanid): """Enable vlan in trunk interface. @@ -104,7 +153,7 @@ def enable_vlan_on_trunk_int(self, mgr, nexus_switch, interface, vlanid): snippet = snipp.CMD_INT_VLAN_SNIPPET confstr = self.create_xml_snippet(snippet % (interface, vlanid)) LOG.debug(_("NexusDriver: %s"), confstr) - mgr.edit_config(target='running', config=confstr) + self._edit_config(mgr, target='running', config=confstr) def disable_vlan_on_trunk_int(self, mgr, interface, vlanid): """ @@ -114,7 +163,7 @@ def disable_vlan_on_trunk_int(self, mgr, interface, vlanid): confstr = snipp.CMD_NO_VLAN_INT_SNIPPET % (interface, vlanid) confstr = self.create_xml_snippet(confstr) LOG.debug(_("NexusDriver: %s"), confstr) - mgr.edit_config(target='running', config=confstr) + self._edit_config(mgr, target='running', config=confstr) def create_vlan(self, vlan_name, vlan_id, nexus_host, nexus_user, nexus_password, nexus_ports, diff --git a/quantum/plugins/cisco/nexus/cisco_nexus_snippets.py b/quantum/plugins/cisco/nexus/cisco_nexus_snippets.py index bc1bef07e3d..a6a5018538c 100644 --- a/quantum/plugins/cisco/nexus/cisco_nexus_snippets.py +++ b/quantum/plugins/cisco/nexus/cisco_nexus_snippets.py @@ -45,9 +45,29 @@ %s + + + +""" + +CMD_VLAN_ACTIVE_SNIPPET = """ + + + <__XML__PARAM_value>%s + <__XML__MODE_vlan> active + + + +""" + +CMD_VLAN_NO_SHUTDOWN_SNIPPET = """ + + + <__XML__PARAM_value>%s + <__XML__MODE_vlan> diff --git a/quantum/tests/unit/cisco/test_network_plugin.py b/quantum/tests/unit/cisco/test_network_plugin.py index a26499c8eef..447b20f6ffe 100644 --- a/quantum/tests/unit/cisco/test_network_plugin.py +++ b/quantum/tests/unit/cisco/test_network_plugin.py @@ -17,12 +17,16 @@ import logging import mock -from quantum.common import config +import webob.exc as wexc + +from quantum.api.v2 import base +from quantum.common import exceptions as q_exc from quantum import context from quantum.db import api as db from quantum.manager import QuantumManager from quantum.plugins.cisco.common import cisco_constants as const from quantum.plugins.cisco.common import cisco_credentials_v2 +from quantum.plugins.cisco.common import cisco_exceptions from quantum.plugins.cisco.db import network_db_v2 from quantum.plugins.cisco.db import network_models_v2 from quantum.plugins.cisco import l2network_plugin_configuration @@ -137,6 +141,26 @@ def setUp(self): super(TestCiscoPortsV2, self).setUp() + @contextlib.contextmanager + def _patch_ncclient(self, attr, value): + """Configure an attribute on the mock ncclient module. + + This method can be used to inject errors by setting a side effect + or a return value for an ncclient method. + + :param attr: ncclient attribute (typically method) to be configured. + :param value: Value to be configured on the attribute. + + """ + # Configure attribute. + config = {attr: value} + self.mock_ncclient.configure_mock(**config) + # Continue testing + yield + # Unconfigure attribute + config = {attr: None} + self.mock_ncclient.configure_mock(**config) + @contextlib.contextmanager def _create_port_res(self, name='myname', cidr='1.0.0.0/24', device_id=DEVICE_ID_1, do_delete=True): @@ -152,8 +176,9 @@ def _create_port_res(self, name='myname', cidr='1.0.0.0/24', end of testing """ - with self.network(name=name) as network: - with self.subnet(network=network, cidr=cidr) as subnet: + with self.network(name=name, do_delete=do_delete) as network: + with self.subnet(network=network, cidr=cidr, + do_delete=do_delete) as subnet: net_id = subnet['subnet']['network_id'] res = self._create_port(self.fmt, net_id, device_id=device_id) port = self.deserialize(self.fmt, res) @@ -163,6 +188,23 @@ def _create_port_res(self, name='myname', cidr='1.0.0.0/24', if do_delete: self._delete('ports', port['port']['id']) + def _assertExpectedHTTP(self, status, exc): + """Confirm that an HTTP status corresponds to an expected exception. + + Confirm that an HTTP status which has been returned for an + quantum API request matches the HTTP status corresponding + to an expected exception. + + :param status: HTTP status + :param exc: Expected exception + + """ + if exc in base.FAULT_MAP: + expected_http = base.FAULT_MAP[exc].code + else: + expected_http = wexc.HTTPInternalServerError.code + self.assertEqual(status, expected_http) + def _is_in_last_nexus_cfg(self, words): last_cfg = (CiscoNetworkPluginV2TestCase.mock_ncclient.manager. connect.return_value.edit_config. @@ -195,8 +237,11 @@ def side_effect(*args, **kwargs): net['network']['id'], 'test', True) - # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'ports', 500) + # Expect an internal server error as we injected a fault + self._validate_behavior_on_bulk_failure( + res, + 'ports', + wexc.HTTPInternalServerError.code) def test_create_ports_bulk_native_plugin_failure(self): if self._skip_native_bulk: @@ -215,8 +260,11 @@ def side_effect(*args, **kwargs): patched_plugin.side_effect = side_effect res = self._create_port_bulk(self.fmt, 2, net['network']['id'], 'test', True, context=ctx) - # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'ports', 500) + # We expect an internal server error as we injected a fault + self._validate_behavior_on_bulk_failure( + res, + 'ports', + wexc.HTTPInternalServerError.code) def test_nexus_enable_vlan_cmd(self): """Verify the syntax of the command to enable a vlan on an intf.""" @@ -231,6 +279,59 @@ def test_nexus_enable_vlan_cmd(self): self.assertTrue( self._is_in_last_nexus_cfg(['allowed', 'vlan', 'add'])) + def test_nexus_extended_vlan_range_failure(self): + """Test that extended VLAN range config errors are ignored. + + Some versions of Nexus switch do not allow state changes for + the extended VLAN range (1006-4094), but these errors can be + ignored (default values are appropriate). Test that such errors + are ignored by the Nexus plugin. + + """ + def mock_edit_config_a(target, config): + if all(word in config for word in ['state', 'active']): + raise Exception("Can't modify state for extended") + + with self._patch_ncclient( + 'manager.connect.return_value.edit_config.side_effect', + mock_edit_config_a): + with self._create_port_res() as res: + self.assertEqual(res.status_int, wexc.HTTPCreated.code) + + def mock_edit_config_b(target, config): + if all(word in config for word in ['no', 'shutdown']): + raise Exception("Command is only allowed on VLAN") + + with self._patch_ncclient( + 'manager.connect.return_value.edit_config.side_effect', + mock_edit_config_b): + with self._create_port_res() as res: + self.assertEqual(res.status_int, wexc.HTTPCreated.code) + + def test_nexus_vlan_config_rollback(self): + """Test rollback following Nexus VLAN state config failure. + + Test that the Cisco Nexus plugin correctly deletes the VLAN + on the Nexus switch when the 'state active' command fails (for + a reason other than state configuration change is rejected + for the extended VLAN range). + + """ + def mock_edit_config(target, config): + if all(word in config for word in ['state', 'active']): + raise ValueError + with self._patch_ncclient( + 'manager.connect.return_value.edit_config.side_effect', + mock_edit_config): + with self._create_port_res(do_delete=False) as res: + # Confirm that the last configuration sent to the Nexus + # switch was deletion of the VLAN. + self.assertTrue( + self._is_in_last_nexus_cfg(['', '']) + ) + self._assertExpectedHTTP(res.status_int, + cisco_exceptions.NexusConfigFailed) + class TestCiscoNetworksV2(CiscoNetworkPluginV2TestCase, test_db_plugin.TestNetworksV2): @@ -256,8 +357,11 @@ def side_effect(*args, **kwargs): patched_plugin.side_effect = side_effect res = self._create_network_bulk(self.fmt, 2, 'test', True) LOG.debug("response is %s" % res) - # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'networks', 500) + # We expect an internal server error as we injected a fault + self._validate_behavior_on_bulk_failure( + res, + 'networks', + wexc.HTTPInternalServerError.code) def test_create_networks_bulk_native_plugin_failure(self): if self._skip_native_bulk: @@ -273,8 +377,11 @@ def side_effect(*args, **kwargs): patched_plugin.side_effect = side_effect res = self._create_network_bulk(self.fmt, 2, 'test', True) - # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'networks', 500) + # We expect an internal server error as we injected a fault + self._validate_behavior_on_bulk_failure( + res, + 'networks', + wexc.HTTPInternalServerError.code) class TestCiscoSubnetsV2(CiscoNetworkPluginV2TestCase, @@ -305,8 +412,11 @@ def side_effect(*args, **kwargs): res = self._create_subnet_bulk(self.fmt, 2, net['network']['id'], 'test') - # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'subnets', 500) + # We expect an internal server error as we injected a fault + self._validate_behavior_on_bulk_failure( + res, + 'subnets', + wexc.HTTPInternalServerError.code) def test_create_subnets_bulk_native_plugin_failure(self): if self._skip_native_bulk: @@ -325,8 +435,11 @@ def side_effect(*args, **kwargs): net['network']['id'], 'test') - # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'subnets', 500) + # We expect an internal server error as we injected a fault + self._validate_behavior_on_bulk_failure( + res, + 'subnets', + wexc.HTTPInternalServerError.code) class TestCiscoPortsV2XML(TestCiscoPortsV2):