From fc914f66502ee066f41780e10b0e659334ffd060 Mon Sep 17 00:00:00 2001 From: Dane LeBlanc Date: Mon, 29 Apr 2013 23:43:14 -0400 Subject: [PATCH] Fix Cisco Nexus plugin failures for vlan IDs 1006-4094 Fixes bug 1174593 VLAN IDs in the range 1006-4094 are considered to be in the extended range by the Cisco Nexus 3K switch. As such, the 3K rejects any state change configuration commands for VLANs in this range, including "state active" and "no shutdown". The errors returned by the 3K for these commands can be ignored, since the default states for these commands are acceptable for the 3K. For the 5K and 7K versions of the Nexus switch, on the other hand, the "state active" and "no shutdown" commands are required for proper VLAN configuration, regardless of VLAN ID. This fix splits the configuration commands which are used to create a VLAN on the Nexus switch into three separate configurations: - VLAN creation - state active - no shutdown For the "state active" and "no shutdown" configurations, the Cisco Nexus plugin will tolerate (ignore) errors involving invalid setting of state for VLANs in the extended VLAN range. These specific errors can be identified by looking for the appearance of certain signature strings in the associated exception's message string, i.e. "Can't modify state for extended" or "Command is allowed on VLAN". This approach will yield a very small hit in performance, but the solution is much less error prone than requiring customers to configure the Cisco plugin for 3K vs. 5K vs. 7K, or perhaps even specific software versions of the 3K, in order to inform the Cisco plugin whether VLAN state configuration commands should be used or not. Change-Id: I1be4966ddc6f462bca38428c4f904f8c982f318f --- .../nexus/cisco_nexus_network_driver_v2.py | 47 ++++++-- .../cisco/nexus/cisco_nexus_snippets.py | 20 ++++ .../tests/unit/cisco/test_network_plugin.py | 112 +++++++++++++++--- 3 files changed, 155 insertions(+), 24 deletions(-) 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 b0748f1ef7..f7a162e38a 100644 --- a/quantum/plugins/cisco/nexus/cisco_nexus_network_driver_v2.py +++ b/quantum/plugins/cisco/nexus/cisco_nexus_network_driver_v2.py @@ -37,22 +37,32 @@ class CiscoNEXUSDriver(): def __init__(self): pass - def _edit_config(self, mgr, target='running', config=''): + 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: - # Raise a Quantum exception. Include a description of - # the original ncclient exception. - raise cexc.NexusConfigFailed(config=config, exc=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 cexc.NexusConfigFailed(config=config, exc=e) def nxos_connect(self, nexus_host, nexus_ssh_port, nexus_user, nexus_password): @@ -77,11 +87,34 @@ class CiscoNEXUSDriver(): 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) + """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 cexc.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): """Delete a VLAN on Nexus Switch given the VLAN ID.""" confstr = snipp.CMD_NO_VLAN_CONF_SNIPPET % vlanid diff --git a/quantum/plugins/cisco/nexus/cisco_nexus_snippets.py b/quantum/plugins/cisco/nexus/cisco_nexus_snippets.py index a2f79269d9..aeb7d43c90 100644 --- a/quantum/plugins/cisco/nexus/cisco_nexus_snippets.py +++ b/quantum/plugins/cisco/nexus/cisco_nexus_snippets.py @@ -45,9 +45,29 @@ CMD_VLAN_CONF_SNIPPET = """ %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 bab52177f5..a2a239066f 100644 --- a/quantum/tests/unit/cisco/test_network_plugin.py +++ b/quantum/tests/unit/cisco/test_network_plugin.py @@ -18,6 +18,8 @@ import inspect import logging import mock +import webob.exc as wexc + from quantum.api.v2 import base from quantum.common import exceptions as q_exc from quantum import context @@ -190,9 +192,14 @@ class TestCiscoPortsV2(CiscoNetworkPluginV2TestCase, if exc in base.FAULT_MAP: expected_http = base.FAULT_MAP[exc].code else: - expected_http = 500 + expected_http = wexc.HTTPInternalServerError.code self.assertEqual(status, expected_http) + def _is_in_last_nexus_cfg(self, words): + last_cfg = (self.mock_ncclient.manager.connect(). + edit_config.mock_calls[-1][2]['config']) + return all(word in last_cfg for word in words) + def test_create_ports_bulk_emulated_plugin_failure(self): real_has_attr = hasattr @@ -219,8 +226,11 @@ class TestCiscoPortsV2(CiscoNetworkPluginV2TestCase, 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: @@ -239,8 +249,11 @@ class TestCiscoPortsV2(CiscoNetworkPluginV2TestCase, 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_connect_fail(self): """Test failure to connect to a Nexus switch. @@ -273,6 +286,60 @@ class TestCiscoPortsV2(CiscoNetworkPluginV2TestCase, self._assertExpectedHTTP(res.status_int, c_exc.NexusConfigFailed) + 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(self.fmt, name='myname') 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(self.fmt, name='myname') 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(self.fmt, no_delete=True, + name='myname') 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, + c_exc.NexusConfigFailed) + def test_get_seg_id_fail(self): """Test handling of a NetworkSegmentIDNotFound exception. @@ -329,10 +396,9 @@ class TestCiscoPortsV2(CiscoNetworkPluginV2TestCase, name='myname') as res: # Confirm that the last configuration sent to the Nexus # switch was a removal of vlan from the test interface. - last_nexus_cfg = (self.mock_ncclient.manager.connect(). - edit_config.mock_calls[-1][2]['config']) - self.assertTrue('' in last_nexus_cfg) - self.assertTrue('' in last_nexus_cfg) + self.assertTrue( + self._is_in_last_nexus_cfg(['', '']) + ) self._assertExpectedHTTP(res.status_int, KeyError) def test_model_delete_port_rollback(self): @@ -424,8 +490,11 @@ class TestCiscoNetworksV2(CiscoNetworkPluginV2TestCase, 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: @@ -441,8 +510,11 @@ class TestCiscoNetworksV2(CiscoNetworkPluginV2TestCase, 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, @@ -473,8 +545,11 @@ class TestCiscoSubnetsV2(CiscoNetworkPluginV2TestCase, 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: @@ -493,8 +568,11 @@ class TestCiscoSubnetsV2(CiscoNetworkPluginV2TestCase, 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):