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

diff --git a/neutron/plugins/cisco/common/config.py b/neutron/plugins/cisco/common/config.py
index 86a55426fc29530e0a252fbf7cd7af061314a02c..a4a1196a8d8e9459c9c5935ebff44ef21bb2980b 100644 (file)
--- 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 4b9306240c88eb5c14f61acbf5605cad55f89b3b..8d98e77c6211f867a65c1022a9f07c94ebc098b6 100644 (file)
--- 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 8c32e88e9d03de29d2fa15962f05a464a4c7f7b1..b330b5ec5b41182598de34eae4c6b89030b59398 100644 (file)
--- 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 023bcc02938aa2a6b78dc439ea3d656aaa8af505..49b3dce50c28abc59fd15972958572b7e2bf4ee9 100644 (file)
--- 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 2908e0cd224aa4faba2760297ba01680556e837a..74309d92d080bbff59f2b8f59c8c5a3a8954c435 100644 (file)
--- 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."""