Cisco nexus plugin fails to untrunk vlan if other hosts using vlan

Closes-Bug: #1246080

Without this fix, if two or more compute hosts have
instances which are
sharing a given VLAN on a Nexus switch, and then
all instances on one of the hosts which are using that
VLAN are terminated, while instances which are using
that VLAN on other hosts remain active, then
the VLAN is not being untrunked from the
corresponding interface on the Nexus switch as
expected.

This fix changes the VLAN removal logic from:
----If this the last instance using this VLAN on this switch:
--------untrunk the vlan from the switch interface
--------delete the VLAN from the switch
To:
----If this the last instance using this VLAN on this switch interface:
--------untrunk the vlan from the switch interface
--------If this the last instance using this VLAN on this switch:
------------delete the VLAN from the switch

Note that this bug also exists in the Cisco ML2
mechanism driver, but the code which implements
this is being redesigned, so it will be addressed for
the ML2 separately.

Change-Id: Icb1f95d1db4baa56c0f6fd68ce6342bbff27641d

diff --git a/neutron/plugins/cisco/nexus/cisco_nexus_plugin_v2.py b/neutron/plugins/cisco/nexus/cisco_nexus_plugin_v2.py
index 8d98e77c6211f867a65c1022a9f07c94ebc098b6..4d31644f5255b5024c31ad4541fb9dbc76aa7200 100644 (file)
--- a/neutron/plugins/cisco/nexus/cisco_nexus_plugin_v2.py
+++ b/neutron/plugins/cisco/nexus/cisco_nexus_plugin_v2.py
@@ -299,17 +299,28 @@ class NexusPlugin(L2DevicePluginBase):
             nxos_db.remove_nexusport_binding(row.port_id, row.vlan_id,
                                              row.switch_ip,
                                              row.instance_id)
-            # Check for any other bindings with the same vlan_id and switch_ip
+            # Check whether there are any remaining instances using this
+            # vlan on this Nexus port.
             try:
-                nxos_db.get_nexusvlan_binding(row.vlan_id, row.switch_ip)
+                nxos_db.get_port_vlan_switch_binding(row.port_id,
+                                                     row.vlan_id,
+                                                     row.switch_ip)
             except cisco_exc.NexusPortBindingNotFound:
                 try:
-                    # Delete this vlan from this switch
                     if nexus_port and auto_untrunk:
+                        # Untrunk the vlan from this Nexus interface
                         self._client.disable_vlan_on_trunk_int(
                             switch_ip, row.vlan_id, etype, nexus_port)
+
+                    # Check whether there are any remaining instances
+                    # using this vlan on the Nexus switch.
                     if auto_delete:
-                        self._client.delete_vlan(switch_ip, row.vlan_id)
+                        try:
+                            nxos_db.get_nexusvlan_binding(row.vlan_id,
+                                                          row.switch_ip)
+                        except cisco_exc.NexusPortBindingNotFound:
+                            # Delete this vlan from this switch
+                            self._client.delete_vlan(switch_ip, row.vlan_id)
                 except Exception:
                     # The delete vlan operation on the Nexus failed,
                     # so this delete_port request has failed. For

diff --git a/neutron/tests/unit/cisco/test_network_plugin.py b/neutron/tests/unit/cisco/test_network_plugin.py
index b330b5ec5b41182598de34eae4c6b89030b59398..c19f139bda28470d3ae49a28cf4878882a84e6dc 100644 (file)
--- a/neutron/tests/unit/cisco/test_network_plugin.py
+++ b/neutron/tests/unit/cisco/test_network_plugin.py
@@ -170,6 +170,7 @@ class CiscoNetworkPluginV2TestCase(test_db_plugin.NeutronDbPluginV2TestCase):
             configlet = call[2]['config']
             if all(word in configlet for word in words):
                 return True
+        return False
 
     def _is_in_last_nexus_cfg(self, words):
         """Check if last config sent to Nexus contains all words in a list."""
@@ -177,6 +178,20 @@ class CiscoNetworkPluginV2TestCase(test_db_plugin.NeutronDbPluginV2TestCase):
                     edit_config.mock_calls[-1][2]['config'])
         return all(word in last_cfg for word in words)
 
+    def _is_vlan_configured(self, vlan_creation_expected=True,
+                            add_keyword_expected=False):
+        vlan_created = self._is_in_nexus_cfg(['vlan', 'vlan-name'])
+        add_appears = self._is_in_last_nexus_cfg(['add'])
+        return (self._is_in_last_nexus_cfg(['allowed', 'vlan']) and
+                vlan_created == vlan_creation_expected and
+                add_appears == add_keyword_expected)
+
+    def _is_vlan_unconfigured(self, vlan_deletion_expected=True):
+        vlan_deleted = self._is_in_last_nexus_cfg(
+            ['no', 'vlan', 'vlan-id-create-delete'])
+        return (self._is_in_nexus_cfg(['allowed', 'vlan', 'remove']) and
+                vlan_deleted == vlan_deletion_expected)
+
 
 class TestCiscoBasicGet(CiscoNetworkPluginV2TestCase,
                         test_db_plugin.TestBasicGet):
@@ -307,14 +322,64 @@ class TestCiscoPortsV2(CiscoNetworkPluginV2TestCase,
 
     def test_nexus_enable_vlan_cmd(self):
         """Verify the syntax of the command to enable a vlan on an intf."""
+
         # First vlan should be configured without 'add' keyword
         with self._create_port_res(name='net1', cidr=CIDR_1):
-            self.assertTrue(self._is_in_last_nexus_cfg(['allowed', 'vlan']))
-            self.assertFalse(self._is_in_last_nexus_cfg(['add']))
+            self.assertTrue(self._is_vlan_configured(
+                vlan_creation_expected=True,
+                add_keyword_expected=False))
+            self.mock_ncclient.reset_mock()
+
             # Second vlan should be configured with 'add' keyword
             with self._create_port_res(name='net2', cidr=CIDR_2):
-                self.assertTrue(
-                    self._is_in_last_nexus_cfg(['allowed', 'vlan', 'add']))
+                self.assertTrue(self._is_vlan_configured(
+                    vlan_creation_expected=True,
+                    add_keyword_expected=True))
+
+    def test_nexus_vlan_config_two_hosts(self):
+        """Verify config/unconfig of vlan on two compute hosts."""
+
+        @contextlib.contextmanager
+        def _create_port_check_vlan(comp_host_name, device_id,
+                                    vlan_creation_expected=True):
+            arg_list = (portbindings.HOST_ID,)
+            port_dict = {portbindings.HOST_ID: comp_host_name,
+                         'device_id': device_id,
+                         'device_owner': DEVICE_OWNER}
+            with self.port(subnet=subnet, fmt=self.fmt,
+                           arg_list=arg_list, **port_dict):
+                self.assertTrue(self._is_vlan_configured(
+                    vlan_creation_expected=vlan_creation_expected,
+                    add_keyword_expected=False))
+                self.mock_ncclient.reset_mock()
+                yield
+
+        # Create network and subnet
+        with self.network(name=NETWORK_NAME) as network:
+            with self.subnet(network=network, cidr=CIDR_1) as subnet:
+
+                # Create an instance on first compute host
+                with _create_port_check_vlan(
+                    COMP_HOST_NAME, DEVICE_ID_1, vlan_creation_expected=True):
+
+                    # Create an instance on second compute host
+                    with _create_port_check_vlan(
+                        COMP_HOST_NAME_2, DEVICE_ID_2,
+                        vlan_creation_expected=False):
+                        pass
+
+                    # Instance on second host is now terminated.
+                    # Vlan should be untrunked from port, but vlan should
+                    # still exist on the switch.
+                    self.assertTrue(self._is_vlan_unconfigured(
+                        vlan_deletion_expected=False))
+                    self.mock_ncclient.reset_mock()
+
+                # Instance on first host is now terminated.
+                # Vlan should be untrunked from port and vlan should have
+                # been deleted from the switch.
+                self.assertTrue(self._is_vlan_unconfigured(
+                    vlan_deletion_expected=True))
 
     def test_nexus_connect_fail(self):
         """Test failure to connect to a Nexus switch.