From a0a462f0303f68d885ff344898437e310c64188c Mon Sep 17 00:00:00 2001 From: Dane LeBlanc Date: Wed, 30 Oct 2013 15:00:47 -0400 Subject: [PATCH] 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 --- .../cisco/nexus/cisco_nexus_plugin_v2.py | 19 ++++- .../tests/unit/cisco/test_network_plugin.py | 73 ++++++++++++++++++- 2 files changed, 84 insertions(+), 8 deletions(-) diff --git a/neutron/plugins/cisco/nexus/cisco_nexus_plugin_v2.py b/neutron/plugins/cisco/nexus/cisco_nexus_plugin_v2.py index 8d98e77c62..4d31644f52 100644 --- 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 b330b5ec5b..c19f139bda 100644 --- 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.