From 6b21cd29bec6b4c069eeda5bfd4b40e8d1f2a89e Mon Sep 17 00:00:00 2001 From: Dane LeBlanc Date: Wed, 13 Nov 2013 23:10:21 -0500 Subject: [PATCH] Round-robin SVI switch selection fails on Cisco Nexus plugin Fixes bug 1250969 This fix addresses improper behavior with the Cisco Nexus plugin's selection of Nexus switch on which to place a Switch Virtual Interface (SVI) when round-robin switch placement is enabled. The expected behavior when round-robin SVI switch selection is configured via the cisco_plugins.ini file, i.e.: [cisco] nexus_l3_enable = True svi_round_robin = True is that when a virtual router interface is created, the Nexus plugin should select the Nexus switch with the least number of SVI interfaces configured for creating the new SVI. The current selection is based on the first entry in a dictionary, and is therefore indeterminate. Similarly, this fix also addresses incorrect behavior when round-robin selection is disabled. In this case, the desired behavior is that the plugin should select the first switch which appears in the Nexus switch configuration. Instead, the current selection is also based on the first entry in a dictionary, and is likewise indeterminate. Change-Id: I548c1efaa8b54695246251b6823ab59e077836fe --- neutron/plugins/cisco/common/config.py | 12 +++++ .../cisco/nexus/cisco_nexus_plugin_v2.py | 7 ++- .../tests/unit/cisco/test_network_plugin.py | 6 +++ neutron/tests/unit/cisco/test_nexus_db.py | 49 +++++++++++++++++++ neutron/tests/unit/cisco/test_nexus_plugin.py | 8 ++- 5 files changed, 77 insertions(+), 5 deletions(-) diff --git a/neutron/plugins/cisco/common/config.py b/neutron/plugins/cisco/common/config.py index 86a55426f..a4a1196a8 100644 --- a/neutron/plugins/cisco/common/config.py +++ b/neutron/plugins/cisco/common/config.py @@ -106,6 +106,13 @@ CISCO_PLUGINS = cfg.CONF.CISCO_PLUGINS # device_dictionary = {} +# +# first_device_ip - IP address of first switch discovered in config +# +# Used for SVI placement when round-robin placement is disabled +# +first_device_ip = None + class CiscoConfigOptions(): """Cisco Configuration Options Class.""" @@ -119,17 +126,22 @@ class CiscoConfigOptions(): device supported sections. Ex. NEXUS_SWITCH, N1KV. """ + global first_device_ip + multi_parser = cfg.MultiConfigParser() read_ok = multi_parser.read(CONF.config_file) if len(read_ok) != len(CONF.config_file): raise cfg.Error(_("Some config files were not parsed properly")) + first_device_ip = None for parsed_file in multi_parser.parsed: for parsed_item in parsed_file.keys(): dev_id, sep, dev_ip = parsed_item.partition(':') if dev_id.lower() in ['nexus_switch', 'n1kv']: for dev_key, value in parsed_file[parsed_item].items(): + if dev_ip and not first_device_ip: + first_device_ip = dev_ip device_dictionary[dev_id, dev_ip, dev_key] = value[0] diff --git a/neutron/plugins/cisco/nexus/cisco_nexus_plugin_v2.py b/neutron/plugins/cisco/nexus/cisco_nexus_plugin_v2.py index 4b9306240..8d98e77c6 100644 --- a/neutron/plugins/cisco/nexus/cisco_nexus_plugin_v2.py +++ b/neutron/plugins/cisco/nexus/cisco_nexus_plugin_v2.py @@ -228,15 +228,14 @@ class NexusPlugin(L2DevicePluginBase): switch_dict[switch_ip] += 1 # Search for the lowest value in the dict if switch_dict: - switch_ip = min(switch_dict.items(), key=switch_dict.get) - return switch_ip[0] + switch_ip = min(switch_dict, key=switch_dict.get) + return switch_ip except cisco_exc.NexusPortBindingNotFound: pass LOG.debug(_("No round robin or zero weights, using first switch")) # Return the first switch in the config - for switch_ip, attr in nexus_switches: - return switch_ip + return conf.first_device_ip def delete_network(self, tenant_id, net_id, **kwargs): """Delete network. diff --git a/neutron/tests/unit/cisco/test_network_plugin.py b/neutron/tests/unit/cisco/test_network_plugin.py index 8c32e88e9..b330b5ec5 100644 --- a/neutron/tests/unit/cisco/test_network_plugin.py +++ b/neutron/tests/unit/cisco/test_network_plugin.py @@ -127,6 +127,12 @@ class CiscoNetworkPluginV2TestCase(test_db_plugin.NeutronDbPluginV2TestCase): super(CiscoNetworkPluginV2TestCase, self).setUp(CORE_PLUGIN) self.port_create_status = 'DOWN' + # Set Cisco config module's first configured Nexus IP address. + # Used for SVI placement when round-robin placement is disabled. + mock.patch.object(cisco_config, 'first_device_ip', + new=NEXUS_IP_ADDR).start() + self.addCleanup(mock.patch.stopall) + def _get_plugin_ref(self): plugin_obj = NeutronManager.get_plugin() if getattr(plugin_obj, "_master"): diff --git a/neutron/tests/unit/cisco/test_nexus_db.py b/neutron/tests/unit/cisco/test_nexus_db.py index 023bcc029..49b3dce50 100644 --- a/neutron/tests/unit/cisco/test_nexus_db.py +++ b/neutron/tests/unit/cisco/test_nexus_db.py @@ -14,11 +14,14 @@ # under the License. import collections +import mock import testtools from neutron.db import api as db from neutron.plugins.cisco.common import cisco_exceptions as c_exc +from neutron.plugins.cisco.common import config from neutron.plugins.cisco.db import nexus_db_v2 as nxdb +from neutron.plugins.cisco.nexus import cisco_nexus_plugin_v2 from neutron.tests import base @@ -166,6 +169,52 @@ class CiscoNexusDbTest(base.BaseTestCase): npb_svi = nxdb.get_nexussvi_bindings() self.assertEqual(len(npb_svi), 2) + def test_nexussviswitch_find(self): + """Test Nexus switch selection for SVI placement.""" + # Configure 2 Nexus switches + nexus_switches = { + ('1.1.1.1', 'username'): 'admin', + ('1.1.1.1', 'password'): 'password1', + ('1.1.1.1', 'host1'): '1/1', + ('2.2.2.2', 'username'): 'admin', + ('2.2.2.2', 'password'): 'password2', + ('2.2.2.2', 'host2'): '1/1', + } + nexus_plugin = cisco_nexus_plugin_v2.NexusPlugin() + nexus_plugin._client = mock.Mock() + nexus_plugin._client.nexus_switches = nexus_switches + + # Set the Cisco config module's first configured device IP address + # according to the preceding switch config + with mock.patch.object(config, 'first_device_ip', new='1.1.1.1'): + + # Enable round-robin mode with no SVIs configured on any of the + # Nexus switches (i.e. no entries in the SVI database). The + # plugin should select the first switch in the configuration. + config.CONF.set_override('svi_round_robin', True, 'CISCO') + switch_ip = nexus_plugin._find_switch_for_svi() + self.assertEqual(switch_ip, '1.1.1.1') + + # Keep round-robin mode enabled, and add entries to the SVI + # database. The plugin should select the switch with the least + # number of entries in the SVI database. + vlan = 100 + npbr11 = self._npb_test_obj('router', vlan, switch='1.1.1.1', + instance='instance11') + npbr12 = self._npb_test_obj('router', vlan, switch='1.1.1.1', + instance='instance12') + npbr21 = self._npb_test_obj('router', vlan, switch='2.2.2.2', + instance='instance21') + self._add_to_db([npbr11, npbr12, npbr21]) + switch_ip = nexus_plugin._find_switch_for_svi() + self.assertEqual(switch_ip, '2.2.2.2') + + # Disable round-robin mode. The plugin should select the + # first switch in the configuration. + config.CONF.clear_override('svi_round_robin', 'CISCO') + switch_ip = nexus_plugin._find_switch_for_svi() + self.assertEqual(switch_ip, '1.1.1.1') + def test_nexusbinding_update(self): npb11 = self._npb_test_obj(10, 100, switch='1.1.1.1', instance='test') npb21 = self._npb_test_obj(20, 100, switch='1.1.1.1', instance='test') diff --git a/neutron/tests/unit/cisco/test_nexus_plugin.py b/neutron/tests/unit/cisco/test_nexus_plugin.py index 2908e0cd2..74309d92d 100644 --- a/neutron/tests/unit/cisco/test_nexus_plugin.py +++ b/neutron/tests/unit/cisco/test_nexus_plugin.py @@ -22,6 +22,7 @@ from neutron.extensions import providernet as provider from neutron.openstack.common import importutils from neutron.plugins.cisco.common import cisco_constants as const from neutron.plugins.cisco.common import cisco_exceptions as cisco_exc +from neutron.plugins.cisco.common import config as cisco_config from neutron.plugins.cisco.db import network_db_v2 as cdb from neutron.plugins.cisco.nexus import cisco_nexus_plugin_v2 from neutron.tests import base @@ -146,12 +147,17 @@ class TestCiscoNexusPlugin(base.BaseTestCase): self.patch_obj = mock.patch.dict('sys.modules', {'ncclient': self.mock_ncclient}) self.patch_obj.start() + self.addCleanup(self.patch_obj.stop) with mock.patch.object(cisco_nexus_plugin_v2.NexusPlugin, '__init__', new=new_nexus_init): self._cisco_nexus_plugin = cisco_nexus_plugin_v2.NexusPlugin() - self.addCleanup(self.patch_obj.stop) + # Set the Cisco config module's first configured device IP address + # according to the preceding switch config. + mock.patch.object(cisco_config, 'first_device_ip', + new=NEXUS_IP_ADDRESS).start() + self.addCleanup(mock.patch.stopall) def test_create_delete_networks(self): """Tests creation of two new Virtual Networks."""