]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Fix Cisco Nexus plugin failures for vlan IDs 1006-4094
authorDane LeBlanc <leblancd@cisco.com>
Tue, 30 Apr 2013 03:43:14 +0000 (23:43 -0400)
committerDane LeBlanc <leblancd@cisco.com>
Thu, 16 May 2013 01:39:37 +0000 (21:39 -0400)
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

quantum/plugins/cisco/nexus/cisco_nexus_network_driver_v2.py
quantum/plugins/cisco/nexus/cisco_nexus_snippets.py
quantum/tests/unit/cisco/test_network_plugin.py

index b0748f1ef7ba6390c6f39b4dc441be378417f7be..f7a162e38afc03b45c43ec8ba75b88aa5ecdd147 100644 (file)
@@ -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
index a2f79269d925d0bec274de96904d131284ea119d..aeb7d43c903bfa09570be0ff442a43c912cf92fc 100644 (file)
@@ -45,9 +45,29 @@ CMD_VLAN_CONF_SNIPPET = """
                   <name>
                     <vlan-name>%s</vlan-name>
                   </name>
+                </__XML__MODE_vlan>
+              </vlan-id-create-delete>
+            </vlan>
+"""
+
+CMD_VLAN_ACTIVE_SNIPPET = """
+            <vlan>
+              <vlan-id-create-delete>
+                <__XML__PARAM_value>%s</__XML__PARAM_value>
+                <__XML__MODE_vlan>
                   <state>
                     <vstate>active</vstate>
                   </state>
+                </__XML__MODE_vlan>
+              </vlan-id-create-delete>
+            </vlan>
+"""
+
+CMD_VLAN_NO_SHUTDOWN_SNIPPET = """
+            <vlan>
+              <vlan-id-create-delete>
+                <__XML__PARAM_value>%s</__XML__PARAM_value>
+                <__XML__MODE_vlan>
                   <no>
                     <shutdown/>
                   </no>
index bab52177f5c524e249e11ff8a2ecdf1f961ca8a4..a2a239066fc2da121aa156cb8158855fc4390fa5 100644 (file)
@@ -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(['<no>', '<vlan>'])
+                )
+                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('<vlan>' in last_nexus_cfg)
-                self.assertTrue('<remove>' in last_nexus_cfg)
+                self.assertTrue(
+                    self._is_in_last_nexus_cfg(['<vlan>', '<remove>'])
+                )
                 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):