From 701e93f85487b372d6a4dd732a242ef4bb0a4d93 Mon Sep 17 00:00:00 2001 From: Vasyl Saienko Date: Fri, 9 Dec 2016 14:54:15 +0200 Subject: [PATCH 01/10] Add vlan aware VMs support With this patch ngs starts supporting attaching of trunk port to baremetal server. Only VLAN Neutron network is supported. There are two ways to configure trunk port: * C1: When segmentation details for trunk ports are inherited from Neutron network. VLAN translation support is not required. Added implementation for cisco, arista, OVS on Linux. * C2: When user set segmentation details for trunk port explicitly. Switch should support VLAN translation for this case. Implement only for OVS on Linux, experimental. NetmikoSwitch.plug_port_to_network() is deprecated. New bind_port() should be used instead. New switch config option: vlan_translation_supported was introduced. This option defines if switch support vlan translation which affect the way how trunk is configured. Change-Id: If084382f4c17438e5142f51f88d6e436f8b85082 (cherry picked from commit 1163a1f062dbf53d9eb5891effb8182fee6a0c21) (cherry picked from commit 7dfe3e2f16cc17f10f07ea389298aa95548a1127) (cherry picked from commit c54e9ba5d4a65a5b2b3ad8a7ae1a58e5ac017e1a) --- networking_generic_switch/devices/__init__.py | 3 +- .../devices/netmiko_devices/__init__.py | 41 +++++++++ .../devices/netmiko_devices/arista.py | 12 +++ .../devices/netmiko_devices/cisco.py | 12 +++ .../devices/netmiko_devices/dell.py | 70 ++++++++++++++++ networking_generic_switch/exceptions.py | 5 ++ .../generic_switch_mech.py | 51 ++++++++++-- .../tests/unit/netmiko/test_arista_eos.py | 39 +++++++++ .../tests/unit/netmiko/test_cisco_ios.py | 39 +++++++++ .../tests/unit/netmiko/test_dell.py | 47 +++++++++++ .../tests/unit/netmiko/test_netmiko_base.py | 83 +++++++++++++++++++ .../tests/unit/test_devices.py | 8 ++ .../tests/unit/test_generic_switch_mech.py | 6 +- 13 files changed, 409 insertions(+), 7 deletions(-) diff --git a/networking_generic_switch/devices/__init__.py b/networking_generic_switch/devices/__init__.py index 42a30382..14548963 100644 --- a/networking_generic_switch/devices/__init__.py +++ b/networking_generic_switch/devices/__init__.py @@ -42,6 +42,7 @@ {'name': 'ngs_network_name_format', 'default': '{network_id}'}, # If false, ngs will not add and delete VLANs from switches {'name': 'ngs_manage_vlans', 'default': True}, + {'name': 'vlan_translation_supported', 'default': False} ] @@ -145,7 +146,7 @@ def del_network(self, segmentation_id, network_id): pass @abc.abstractmethod - def plug_port_to_network(self, port_id, segmentation_id): + def plug_port_to_network(self, port_id, segmentation_id, trunk_details=None, vtr=False): pass @abc.abstractmethod diff --git a/networking_generic_switch/devices/netmiko_devices/__init__.py b/networking_generic_switch/devices/netmiko_devices/__init__.py index 14bd989d..185e6ffb 100644 --- a/networking_generic_switch/devices/netmiko_devices/__init__.py +++ b/networking_generic_switch/devices/netmiko_devices/__init__.py @@ -89,6 +89,10 @@ class NetmikoSwitch(devices.GenericSwitchDevice): SAVE_CONFIGURATION = None + SET_NATIVE_VLAN = None + + ALLOW_NETWORK_ON_TRUNK = None + ERROR_MSG_PATTERNS = () """Sequence of error message patterns. @@ -251,6 +255,27 @@ def del_network(self, segmentation_id, network_id): network_name=network_name) return self.send_commands_to_device(cmds) + @check_output('plug port trunk') + def plug_port_to_network_trunk(self, port, segmentation_id, trunk_details=None, vtr=False): + cmd_set = [] + vts = self.ngs_config.get('vlan_translation_supported', False) + # NOTE(vsaienko) Always use vlan translation if it is supported. + if vts: + cmd_set.extend(self.get_trunk_port_cmds_vlan_translation( + port, segmentation_id, trunk_details)) + else: + if vtr: + msg = _("Cannot bind_port VLAN aware port as switch %s " + "doesn't support VLAN translation. " + "But it is required.") % self.config['ip'] + raise exc.GenericSwitchNotSupported(error=msg) + else: + cmd_set.extend( + self.get_trunk_port_cmds_no_vlan_translation( + port, segmentation_id, trunk_details)) + + self.send_commands_to_device(cmd_set) + @check_output('plug port') def plug_port_to_network(self, port, segmentation_id): cmds = [] @@ -384,3 +409,19 @@ def check_output(self, output, operation): raise exc.GenericSwitchNetmikoConfigError( config=device_utils.sanitise_config(self.config), error=msg) + + def get_trunk_port_cmds_no_vlan_translation(self, port_id, segmentation_id, trunk_details): + cmd_set = [] + cmd_set.extend( + self._format_commands(self.SET_NATIVE_VLAN, + port=port_id, + segmentation_id=segmentation_id)) + for sub_port in trunk_details.get('sub_ports'): + cmd_set.extend( + self._format_commands( + self.ALLOW_NETWORK_ON_TRUNK, port=port_id, + segmentation_id=sub_port['segmentation_id'])) + return cmd_set + + def get_trunk_port_cmds_vlan_translation(self, port_id, segmentation_id, trunk_details): + pass diff --git a/networking_generic_switch/devices/netmiko_devices/arista.py b/networking_generic_switch/devices/netmiko_devices/arista.py index a9500ac9..c32e338b 100644 --- a/networking_generic_switch/devices/netmiko_devices/arista.py +++ b/networking_generic_switch/devices/netmiko_devices/arista.py @@ -37,3 +37,15 @@ class AristaEos(netmiko_devices.NetmikoSwitch): 'no switchport mode trunk', 'switchport trunk allowed vlan none' ) + + SET_NATIVE_VLAN = ( + 'interface {port}', + 'switchport mode trunk', + 'switchport trunk native vlan {segmentation_id}', + 'switchport trunk allowed vlan add {segmentation_id}' + ) + + ALLOW_NETWORK_ON_TRUNK = ( + 'interface {port}', + 'switchport trunk allowed vlan add {segmentation_id}' + ) diff --git a/networking_generic_switch/devices/netmiko_devices/cisco.py b/networking_generic_switch/devices/netmiko_devices/cisco.py index d0a19768..7be992e0 100644 --- a/networking_generic_switch/devices/netmiko_devices/cisco.py +++ b/networking_generic_switch/devices/netmiko_devices/cisco.py @@ -37,3 +37,15 @@ class CiscoIos(netmiko_devices.NetmikoSwitch): 'no switchport mode trunk', 'switchport trunk allowed vlan none' ) + + SET_NATIVE_VLAN = ( + 'interface {port}', + 'switchport mode trunk', + 'switchport trunk native vlan {segmentation_id}', + 'switchport trunk allowed vlan add {segmentation_id}' + ) + + ALLOW_NETWORK_ON_TRUNK = ( + 'interface {port}', + 'switchport trunk allowed vlan add {segmentation_id}' + ) diff --git a/networking_generic_switch/devices/netmiko_devices/dell.py b/networking_generic_switch/devices/netmiko_devices/dell.py index 70a126b6..6ad51687 100644 --- a/networking_generic_switch/devices/netmiko_devices/dell.py +++ b/networking_generic_switch/devices/netmiko_devices/dell.py @@ -18,6 +18,76 @@ from networking_generic_switch import exceptions as exc +class DellOS10(netmiko_devices.NetmikoSwitch): + """Netmiko device driver for Dell PowerSwitch switches.""" + + ADD_NETWORK = ( + "interface vlan {segmentation_id}", + "exit", + ) + + DELETE_NETWORK = ( + "no interface vlan {segmentation_id}", + "exit", + ) + + PLUG_PORT_TO_NETWORK = ( + "interface {port}", + "switchport mode access", + "switchport access vlan {segmentation_id}", + "exit", + ) + + DELETE_PORT = ( + "interface {port}", + "no switchport access vlan", + "exit", + ) + + ADD_NETWORK_TO_TRUNK = ( + "interface {port}", + "switchport mode trunk", + "switchport trunk allowed vlan {segmentation_id}", + "exit", + ) + + REMOVE_NETWORK_FROM_TRUNK = ( + "interface {port}", + "no switchport trunk allowed vlan {segmentation_id}", + "exit", + ) + + ENABLE_PORT = ( + "interface {port}", + "no shutdown", + "exit", + ) + + DISABLE_PORT = ( + "interface {port}", + "shutdown", + "exit", + ) + + SET_NATIVE_VLAN = ( + 'interface {port}', + 'switchport mode trunk', + 'switchport access vlan {segmentation_id}', + ) + + ALLOW_NETWORK_ON_TRUNK = ( + 'interface {port}', + 'switchport trunk allowed vlan {segmentation_id}' + ) + + ERROR_MSG_PATTERNS = () + """Sequence of error message patterns. + + Sequence of re.RegexObject objects representing patterns to check for in + device output that indicate a failure to apply configuration. + """ + + class DellNos(netmiko_devices.NetmikoSwitch): """Netmiko device driver for Dell Force10 (OS9) switches.""" diff --git a/networking_generic_switch/exceptions.py b/networking_generic_switch/exceptions.py index 441d4cd7..edadfb5d 100644 --- a/networking_generic_switch/exceptions.py +++ b/networking_generic_switch/exceptions.py @@ -49,3 +49,8 @@ class GenericSwitchNetmikoConnectError(GenericSwitchException): class GenericSwitchNetmikoConfigError(GenericSwitchException): message = _("Netmiko configuration error: %(config)s, error: %(error)s") + + +class GenericSwitchNotSupported(GenericSwitchException): + message = _("Requested feature is not supported by " + "networking-generic-switch. %(error)s") diff --git a/networking_generic_switch/generic_switch_mech.py b/networking_generic_switch/generic_switch_mech.py index 237ac99c..0e223a20 100644 --- a/networking_generic_switch/generic_switch_mech.py +++ b/networking_generic_switch/generic_switch_mech.py @@ -18,11 +18,13 @@ from neutron_lib.api.definitions import portbindings from neutron_lib.callbacks import resources from neutron_lib.plugins.ml2 import api +from neutron_lib.plugins import directory from oslo_log import log as logging from networking_generic_switch import config as gsw_conf from networking_generic_switch import devices from networking_generic_switch.devices import utils as device_utils +from networking_generic_switch import exceptions as ngs_exc LOG = logging.getLogger(__name__) @@ -393,6 +395,23 @@ def delete_port_postcommit(self, context): if self._is_port_bound(port): self._unplug_port_from_network(port, context.network.current) + def _is_vlan_translation_required(self, trunk_details): + """Check if vlan translation is required to configure specific trunk. + + :returns: True if vlan translation is required, False otherwise. + """ + core_plugin = directory.get_plugin() + for sub_port in trunk_details.get('sub_ports', {}): + sp_id = sub_port['port_id'] + sp_sid = sub_port['segmentation_id'] + sp_port = core_plugin.get_port(context._plugin_context, sp_id) + sp_net = core_plugin.get_network(context._plugin_context, + sp_port['network_id']) + if sp_sid != sp_net['provider:segmentation_id']: + return True + + return False + def bind_port(self, context): """Attempt to bind a port. @@ -445,7 +464,6 @@ def bind_port(self, context): # of the links should be processed. if not self._is_link_valid(port, network): return - is_802_3ad = self._is_802_3ad(port) for link in local_link_information: port_id = link.get('port_id') @@ -458,15 +476,38 @@ def bind_port(self, context): segments = context.segments_to_bind # If segmentation ID is None, set vlan 1 segmentation_id = segments[0].get('segmentation_id') or 1 + trunk_details = port.get('trunk_details', {}) LOG.debug("Putting port %(port_id)s on %(switch_info)s " "to vlan: %(segmentation_id)s", {'port_id': port_id, 'switch_info': switch_info, 'segmentation_id': segmentation_id}) # Move port to network - if is_802_3ad and hasattr(switch, 'plug_bond_to_network'): - switch.plug_bond_to_network(port_id, segmentation_id) - else: - switch.plug_port_to_network(port_id, segmentation_id) + # START + try: + if trunk_details: + vtr = self._is_vlan_translation_required(trunk_details) + switch.plug_port_to_network_trunk( + port_id, segmentation_id, trunk_details, vtr) + elif is_802_3ad and hasattr(switch, 'plug_bond_to_network'): + switch.plug_bond_to_network(port_id, segmentation_id) + else: + switch.plug_port_to_network( + port_id, segmentation_id) + except ngs_exc.GenericSwitchNotSupported as e: + LOG.warning("Operation is not supported by " + "networking-generic-switch. %(err)s)", + {'err': e}) + raise e + except Exception as e: + LOG.error("Failed to bind port %(port_id)s in " + "segment %(segment_id)s on device " + "%(device)s due to error %(err)s", + {'port_id': port['id'], + 'device': switch_info, + 'segment_id': segmentation_id, + 'err': e}) + raise e + # END LOG.info("Successfully bound port %(port_id)s in segment " "%(segment_id)s on device %(device)s", {'port_id': port['id'], 'device': switch_info, diff --git a/networking_generic_switch/tests/unit/netmiko/test_arista_eos.py b/networking_generic_switch/tests/unit/netmiko/test_arista_eos.py index 69984376..5e17a360 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_arista_eos.py +++ b/networking_generic_switch/tests/unit/netmiko/test_arista_eos.py @@ -14,6 +14,8 @@ from unittest import mock +from neutron.plugins.ml2 import driver_context + from networking_generic_switch.devices.netmiko_devices import arista from networking_generic_switch.tests.unit.netmiko import test_netmiko_base @@ -57,6 +59,43 @@ def test_delete_port(self, mock_exec): 'no switchport mode trunk', 'switchport trunk allowed vlan none']) + def test_get_trunk_port_cmds_no_vlan_translation(self): + mock_context = mock.create_autospec(driver_context.PortContext) + self.switch.ngs_config['vlan_translation_supported'] = False + trunk_details = {'trunk_id': 'aaa-bbb-ccc-ddd', + 'sub_ports': [{'segmentation_id': 130, + 'port_id': 'aaa-bbb-ccc-ddd', + 'segmentation_type': 'vlan', + 'mac_address': u'fa:16:3e:1c:c2:7e'}]} + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': '2222' + } + ] + }, + 'binding:vnic_type': 'baremetal', + 'id': 'aaaa-bbbb-cccc', + 'trunk_details': trunk_details} + mock_context.network = mock.Mock() + mock_context.network.current = {'provider:segmentation_id': 123} + mock_context.segments_to_bind = [ + { + 'segmentation_id': 777, + 'id': 123 + } + ] + res = self.switch.get_trunk_port_cmds_no_vlan_translation( + '2222', 777, trunk_details) + self.assertEqual(['interface 2222', 'switchport mode trunk', + 'switchport trunk native vlan 777', + 'switchport trunk allowed vlan add 777', + 'interface 2222', + 'switchport trunk allowed vlan add 130'], + res) + def test__format_commands(self): cmd_set = self.switch._format_commands( arista.AristaEos.ADD_NETWORK, diff --git a/networking_generic_switch/tests/unit/netmiko/test_cisco_ios.py b/networking_generic_switch/tests/unit/netmiko/test_cisco_ios.py index 22705b66..0ffdb42f 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_cisco_ios.py +++ b/networking_generic_switch/tests/unit/netmiko/test_cisco_ios.py @@ -14,6 +14,8 @@ from unittest import mock +from neutron.plugins.ml2 import driver_context + from networking_generic_switch.devices.netmiko_devices import cisco from networking_generic_switch.tests.unit.netmiko import test_netmiko_base @@ -56,6 +58,43 @@ def test_delete_port(self, mock_exec): ['interface 3333', 'no switchport access vlan 33', 'no switchport mode trunk', 'switchport trunk allowed vlan none']) + def test_get_trunk_port_cmds_no_vlan_translation(self): + mock_context = mock.create_autospec(driver_context.PortContext) + self.switch.ngs_config['vlan_translation_supported'] = True + trunk_details = {'trunk_id': 'aaa-bbb-ccc-ddd', + 'sub_ports': [{'segmentation_id': 130, + 'port_id': 'aaa-bbb-ccc-ddd', + 'segmentation_type': 'vlan', + 'mac_address': u'fa:16:3e:1c:c2:7e'}]} + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': '2222' + } + ] + }, + 'binding:vnic_type': 'baremetal', + 'id': 'aaaa-bbbb-cccc', + 'trunk_details': trunk_details} + mock_context.network = mock.Mock() + mock_context.network.current = {'provider:segmentation_id': 123} + mock_context.segments_to_bind = [ + { + 'segmentation_id': 777, + 'id': 123 + } + ] + res = self.switch.get_trunk_port_cmds_no_vlan_translation( + '2222', 777, trunk_details) + self.assertEqual(['interface 2222', 'switchport mode trunk', + 'switchport trunk native vlan 777', + 'switchport trunk allowed vlan add 777', + 'interface 2222', + 'switchport trunk allowed vlan add 130'], + res) + def test__format_commands(self): cmd_set = self.switch._format_commands( cisco.CiscoIos.ADD_NETWORK, diff --git a/networking_generic_switch/tests/unit/netmiko/test_dell.py b/networking_generic_switch/tests/unit/netmiko/test_dell.py index cfe29d24..8203863c 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_dell.py +++ b/networking_generic_switch/tests/unit/netmiko/test_dell.py @@ -14,11 +14,58 @@ from unittest import mock +from neutron.plugins.ml2 import driver_context + from networking_generic_switch.devices.netmiko_devices import dell from networking_generic_switch import exceptions as exc from networking_generic_switch.tests.unit.netmiko import test_netmiko_base +class TestNetmikoDellOS10(test_netmiko_base.NetmikoSwitchTestBase): + + def _make_switch_device(self, extra_cfg={}): + device_cfg = {'device_type': 'netmiko_dell_os10'} + device_cfg.update(extra_cfg) + return dell.DellOS10(device_cfg) + + + def test_get_trunk_port_cmds_no_vlan_translation(self): + mock_context = mock.create_autospec(driver_context.PortContext) + self.switch.ngs_config['vlan_translation_supported'] = True + trunk_details = {'trunk_id': 'aaa-bbb-ccc-ddd', + 'sub_ports': [{'segmentation_id': 130, + 'port_id': 'aaa-bbb-ccc-ddd', + 'segmentation_type': 'vlan', + 'mac_address': u'fa:16:3e:1c:c2:7e'}]} + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': '2222' + } + ] + }, + 'binding:vnic_type': 'baremetal', + 'id': 'aaaa-bbbb-cccc', + 'trunk_details': trunk_details} + mock_context.network = mock.Mock() + mock_context.network.current = {'provider:segmentation_id': 123} + mock_context.segments_to_bind = [ + { + 'segmentation_id': 777, + 'id': 123 + } + ] + res = self.switch.get_trunk_port_cmds_no_vlan_translation( + '2222', 777, trunk_details) + self.assertEqual(['interface 2222', 'switchport mode trunk', + 'switchport access vlan 777', + 'interface 2222', + 'switchport trunk allowed vlan 130'], + res) + + class TestNetmikoDellNos(test_netmiko_base.NetmikoSwitchTestBase): def _make_switch_device(self, extra_cfg={}): diff --git a/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py b/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py index ac99713b..ac2a2b83 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py +++ b/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py @@ -15,6 +15,8 @@ import re from unittest import mock +from neutron.plugins.ml2 import driver_context + import fixtures import netmiko import netmiko.base_connection @@ -387,3 +389,84 @@ def test_check_output_error(self): "fake op. Output: %s" % output) self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, msg, self.switch.check_output, output, 'fake op') + + @mock.patch.object(netmiko_devices.netmiko, 'ConnectHandler') + @mock.patch.object(netmiko_devices.NetmikoSwitch, + 'send_commands_to_device') + @mock.patch.object(netmiko_devices.NetmikoSwitch, + 'get_trunk_port_cmds_no_vlan_translation') + @mock.patch.object(netmiko_devices.NetmikoSwitch, + 'get_trunk_port_cmds_vlan_translation') + def test_bind_port_trunk_no_vts(self, t_mock, nt_mock, sctd_mock, + nm_mock): + mock_context = mock.create_autospec(driver_context.PortContext) + connect_mock = mock.Mock() + nm_mock.return_value = connect_mock + self.switch.ngs_config['vlan_translation_supported'] = False + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': '2222' + } + ] + }, + 'binding:vnic_type': 'baremetal', + 'id': 'aaaa-bbbb-cccc', + 'trunk_details': {'sub_ports': + [{'segmentation_id': 123}]}} + trunk_details = {'sub_ports': [{'segmentation_id': 123}]} + self.switch.plug_port_to_network_trunk('2222', None, trunk_details, vtr=False) + nt_mock.assert_called_once_with('2222', None, {'sub_ports': [{'segmentation_id': 123}]}) + self.assertFalse(t_mock.called) + + @mock.patch.object(netmiko_devices.netmiko, 'ConnectHandler') + @mock.patch.object(netmiko_devices.NetmikoSwitch, + 'send_commands_to_device') + @mock.patch.object(netmiko_devices.NetmikoSwitch, + 'get_trunk_port_cmds_no_vlan_translation') + @mock.patch.object(netmiko_devices.NetmikoSwitch, + 'get_trunk_port_cmds_vlan_translation') + def test_bind_port_trunk_vts(self, t_mock, nt_mock, sctd_mock, + nm_mock): + mock_context = mock.create_autospec(driver_context.PortContext) + connect_mock = mock.Mock() + nm_mock.return_value = connect_mock + self.switch.ngs_config['vlan_translation_supported'] = True + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': '2222' + } + ] + }, + 'binding:vnic_type': 'baremetal', + 'id': 'aaaa-bbbb-cccc', + 'trunk_details': {'sub_ports': + [{'segmentation_id': 123}]}} + trunk_details = {'sub_ports': [{'segmentation_id': 123}]} + self.switch.plug_port_to_network_trunk('2222', None, trunk_details, vtr=False) + t_mock.assert_called_once_with('2222', None, {'sub_ports': [{'segmentation_id': 123}]}) + self.assertFalse(nt_mock.called) + + @mock.patch.object(netmiko_devices.netmiko, 'ConnectHandler') + @mock.patch.object(netmiko_devices.NetmikoSwitch, + 'send_commands_to_device') + @mock.patch.object(netmiko_devices.NetmikoSwitch, + 'get_trunk_port_cmds_no_vlan_translation') + @mock.patch.object(netmiko_devices.NetmikoSwitch, + 'get_trunk_port_cmds_vlan_translation') + def test_bind_port_trunk_no_vts_raise(self, t_mock, nt_mock, sctd_mock, + nm_mock): + connect_mock = mock.Mock() + mock_context = mock.create_autospec(driver_context.PortContext) + nm_mock.return_value = connect_mock + self.switch.ngs_config['vlan_translation_supported'] = False + trunk_details = {'sub_ports': [{'segmentation_id': 123}]} + self.assertRaises(exc.GenericSwitchNotSupported, + self.switch.plug_port_to_network_trunk, '2222', None, trunk_details, vtr=True) + self.assertFalse(t_mock.called) + self.assertFalse(nt_mock.called) diff --git a/networking_generic_switch/tests/unit/test_devices.py b/networking_generic_switch/tests/unit/test_devices.py index d293c634..2289c922 100644 --- a/networking_generic_switch/tests/unit/test_devices.py +++ b/networking_generic_switch/tests/unit/test_devices.py @@ -202,3 +202,11 @@ def test__get_network_name_both(self): device = FakeDevice(device_cfg) name = device._get_network_name('fake-id', 22) self.assertEqual('fake-id_net_22', name) + + def test_driver_load_config_override(self): + device_cfg = {"device_type": 'netmiko_ovs_linux', + "vlan_translation_supported": True} + device = devices.device_manager(device_cfg) + self.assertIsInstance(device, devices.GenericSwitchDevice) + self.assertNotIn('vlan_translation_support', device.config) + self.assertTrue(device.ngs_config['vlan_translation_supported']) diff --git a/networking_generic_switch/tests/unit/test_generic_switch_mech.py b/networking_generic_switch/tests/unit/test_generic_switch_mech.py index 6c3356c9..6b1ae9f5 100644 --- a/networking_generic_switch/tests/unit/test_generic_switch_mech.py +++ b/networking_generic_switch/tests/unit/test_generic_switch_mech.py @@ -19,6 +19,7 @@ from neutron.db import provisioning_blocks from neutron.plugins.ml2 import driver_context from neutron_lib.callbacks import resources +from neutron_lib.plugins import directory from networking_generic_switch import exceptions from networking_generic_switch import generic_switch_mech as gsm @@ -512,10 +513,13 @@ def test_update_port_postcommit_complete_provisioning(self, m_pc, m_list): resources.PORT, 'GENERICSWITCH') + @mock.patch.object(gsm.GenericSwitchDriver, + '_is_vlan_translation_required', return_value=False) @mock.patch.object(provisioning_blocks, 'provisioning_complete') def test_update_portgroup_postcommit_complete_provisioning(self, m_pc, - m_list): + m_list, + m_ivtr): driver = gsm.GenericSwitchDriver() driver.initialize() mock_context = mock.create_autospec(driver_context.PortContext) From 4ba8c346b16efa3732f32ad428602e6e39d8b364 Mon Sep 17 00:00:00 2001 From: Will Szumski Date: Fri, 25 Feb 2022 18:41:10 +0000 Subject: [PATCH 02/10] fixup! Add vlan aware VMs support (cherry picked from commit 10b1fbb9f8d5ecfdea496d5c7b399605f75f9ff1) (cherry picked from commit 81e4b13f2969605e74b6ff0567445134d7653bcc) --- networking_generic_switch/devices/__init__.py | 6 ++- .../generic_switch_mech.py | 11 +--- .../tests/unit/test_generic_switch_mech.py | 50 +++++++++++++++++++ 3 files changed, 56 insertions(+), 11 deletions(-) diff --git a/networking_generic_switch/devices/__init__.py b/networking_generic_switch/devices/__init__.py index 14548963..8b824721 100644 --- a/networking_generic_switch/devices/__init__.py +++ b/networking_generic_switch/devices/__init__.py @@ -146,7 +146,11 @@ def del_network(self, segmentation_id, network_id): pass @abc.abstractmethod - def plug_port_to_network(self, port_id, segmentation_id, trunk_details=None, vtr=False): + def plug_port_to_network_trunk(self, port_id, segmentation_id, trunk_details=None, vtr=False): + pass + + @abc.abstractmethod + def plug_port_to_network(self, port_id, segmentation_id): pass @abc.abstractmethod diff --git a/networking_generic_switch/generic_switch_mech.py b/networking_generic_switch/generic_switch_mech.py index 0e223a20..3c76554b 100644 --- a/networking_generic_switch/generic_switch_mech.py +++ b/networking_generic_switch/generic_switch_mech.py @@ -400,16 +400,7 @@ def _is_vlan_translation_required(self, trunk_details): :returns: True if vlan translation is required, False otherwise. """ - core_plugin = directory.get_plugin() - for sub_port in trunk_details.get('sub_ports', {}): - sp_id = sub_port['port_id'] - sp_sid = sub_port['segmentation_id'] - sp_port = core_plugin.get_port(context._plugin_context, sp_id) - sp_net = core_plugin.get_network(context._plugin_context, - sp_port['network_id']) - if sp_sid != sp_net['provider:segmentation_id']: - return True - + # FIXME: removed for simplicity return False def bind_port(self, context): diff --git a/networking_generic_switch/tests/unit/test_generic_switch_mech.py b/networking_generic_switch/tests/unit/test_generic_switch_mech.py index 6b1ae9f5..51798569 100644 --- a/networking_generic_switch/tests/unit/test_generic_switch_mech.py +++ b/networking_generic_switch/tests/unit/test_generic_switch_mech.py @@ -787,6 +787,56 @@ def test_bind_port(self, m_apc, m_list): resources.PORT, 'GENERICSWITCH') + @mock.patch.object(gsm.GenericSwitchDriver, + '_is_vlan_translation_required', return_value=False) + @mock.patch.object(provisioning_blocks, 'add_provisioning_component') + def test_bind_port_trunk(self, m_apc, m_list, m_vlan): + driver = gsm.GenericSwitchDriver() + driver.initialize() + mock_context = mock.create_autospec(driver_context.PortContext) + mock_context._plugin_context = mock.MagicMock() + trunk_details = {'trunk_id': 'aaa-bbb-ccc-ddd', + 'sub_ports': [{'segmentation_id': 130, + 'port_id': 'aaa-bbb-ccc-ddd', + 'segmentation_type': 'vlan', + 'mac_address': u'fa:16:3e:1c:c2:7e'}]} + mock_context.network.current = { + 'provider:physical_network': 'physnet1' + } + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': '2222' + } + ] + }, + 'binding:vnic_type': 'baremetal', + 'id': 'aaaa-bbbb-cccc', + 'trunk_details': trunk_details} + mock_context.network = mock.Mock() + mock_context.network.current = { + 'provider:segmentation_id': 123, + 'provider:physical_network': 'physnet1' + } + mock_context.segments_to_bind = [ + { + 'segmentation_id': 777, + 'id': 123 + } + ] + + driver.bind_port(mock_context) + self.switch_mock.plug_port_to_network_trunk.assert_called_once_with( + '2222', 777, trunk_details, False) + mock_context.set_binding.assert_called_with(123, 'other', {}) + m_apc.assert_called_once_with(mock_context._plugin_context, + mock_context.current['id'], + resources.PORT, + 'GENERICSWITCH') + + @mock.patch.object(provisioning_blocks, 'add_provisioning_component') def test_bind_portgroup(self, m_apc, m_list): driver = gsm.GenericSwitchDriver() From 0ae429654a3cf91cccf866c49ef0843333dce6bb Mon Sep 17 00:00:00 2001 From: Will Szumski Date: Fri, 25 Feb 2022 19:35:49 +0000 Subject: [PATCH 03/10] fixup! Add vlan aware VMs support (cherry picked from commit d9409564012f932bd9ee91f11c2b8f3a51213b33) (cherry picked from commit e57a5c5e2aa23255fba828b69da8e8534f610c0f) --- networking_generic_switch/devices/__init__.py | 1 - networking_generic_switch/devices/netmiko_devices/dell.py | 2 ++ networking_generic_switch/tests/unit/netmiko/test_dell.py | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/networking_generic_switch/devices/__init__.py b/networking_generic_switch/devices/__init__.py index 8b824721..23a6bd80 100644 --- a/networking_generic_switch/devices/__init__.py +++ b/networking_generic_switch/devices/__init__.py @@ -145,7 +145,6 @@ def add_network(self, segmentation_id, network_id): def del_network(self, segmentation_id, network_id): pass - @abc.abstractmethod def plug_port_to_network_trunk(self, port_id, segmentation_id, trunk_details=None, vtr=False): pass diff --git a/networking_generic_switch/devices/netmiko_devices/dell.py b/networking_generic_switch/devices/netmiko_devices/dell.py index 6ad51687..4ed7e0d0 100644 --- a/networking_generic_switch/devices/netmiko_devices/dell.py +++ b/networking_generic_switch/devices/netmiko_devices/dell.py @@ -71,6 +71,8 @@ class DellOS10(netmiko_devices.NetmikoSwitch): SET_NATIVE_VLAN = ( 'interface {port}', + # Clean all the old trunked vlans by switching to access mode first + 'switchport mode access', 'switchport mode trunk', 'switchport access vlan {segmentation_id}', ) diff --git a/networking_generic_switch/tests/unit/netmiko/test_dell.py b/networking_generic_switch/tests/unit/netmiko/test_dell.py index 8203863c..c85df012 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_dell.py +++ b/networking_generic_switch/tests/unit/netmiko/test_dell.py @@ -59,7 +59,8 @@ def test_get_trunk_port_cmds_no_vlan_translation(self): ] res = self.switch.get_trunk_port_cmds_no_vlan_translation( '2222', 777, trunk_details) - self.assertEqual(['interface 2222', 'switchport mode trunk', + self.assertEqual(['interface 2222', 'switchport mode access', + 'switchport mode trunk', 'switchport access vlan 777', 'interface 2222', 'switchport trunk allowed vlan 130'], From 8fa4b7b875bbc8a889650e9cde55705b6d3dc071 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Wed, 8 Feb 2023 11:11:46 +0000 Subject: [PATCH 04/10] VLAN aware VMs: pep8 fix ups Change-Id: Ia8b2e2b8ea90830fbdbc60d8a3b64afe693224db (cherry picked from commit 9ad464f79835da339035c044b4fbdb72eb309e9a) --- networking_generic_switch/devices/__init__.py | 3 ++- .../devices/netmiko_devices/__init__.py | 16 +++++++++----- .../generic_switch_mech.py | 22 +++++++++---------- .../tests/unit/netmiko/test_dell.py | 1 - .../tests/unit/netmiko/test_netmiko_base.py | 19 +++++++++------- .../tests/unit/test_generic_switch_mech.py | 2 -- 6 files changed, 34 insertions(+), 29 deletions(-) diff --git a/networking_generic_switch/devices/__init__.py b/networking_generic_switch/devices/__init__.py index 23a6bd80..b79fe717 100644 --- a/networking_generic_switch/devices/__init__.py +++ b/networking_generic_switch/devices/__init__.py @@ -145,7 +145,8 @@ def add_network(self, segmentation_id, network_id): def del_network(self, segmentation_id, network_id): pass - def plug_port_to_network_trunk(self, port_id, segmentation_id, trunk_details=None, vtr=False): + def plug_port_to_network_trunk(self, port_id, segmentation_id, + trunk_details=None, vtr=False): pass @abc.abstractmethod diff --git a/networking_generic_switch/devices/netmiko_devices/__init__.py b/networking_generic_switch/devices/netmiko_devices/__init__.py index 185e6ffb..9bf70606 100644 --- a/networking_generic_switch/devices/netmiko_devices/__init__.py +++ b/networking_generic_switch/devices/netmiko_devices/__init__.py @@ -256,7 +256,8 @@ def del_network(self, segmentation_id, network_id): return self.send_commands_to_device(cmds) @check_output('plug port trunk') - def plug_port_to_network_trunk(self, port, segmentation_id, trunk_details=None, vtr=False): + def plug_port_to_network_trunk(self, port, segmentation_id, + trunk_details=None, vtr=False): cmd_set = [] vts = self.ngs_config.get('vlan_translation_supported', False) # NOTE(vsaienko) Always use vlan translation if it is supported. @@ -265,9 +266,9 @@ def plug_port_to_network_trunk(self, port, segmentation_id, trunk_details=None, port, segmentation_id, trunk_details)) else: if vtr: - msg = _("Cannot bind_port VLAN aware port as switch %s " - "doesn't support VLAN translation. " - "But it is required.") % self.config['ip'] + msg = ("Cannot bind_port VLAN aware port as switch %s " + "doesn't support VLAN translation. " + "But it is required.") % self.config['ip'] raise exc.GenericSwitchNotSupported(error=msg) else: cmd_set.extend( @@ -410,7 +411,9 @@ def check_output(self, output, operation): config=device_utils.sanitise_config(self.config), error=msg) - def get_trunk_port_cmds_no_vlan_translation(self, port_id, segmentation_id, trunk_details): + def get_trunk_port_cmds_no_vlan_translation(self, port_id, + segmentation_id, + trunk_details): cmd_set = [] cmd_set.extend( self._format_commands(self.SET_NATIVE_VLAN, @@ -423,5 +426,6 @@ def get_trunk_port_cmds_no_vlan_translation(self, port_id, segmentation_id, trun segmentation_id=sub_port['segmentation_id'])) return cmd_set - def get_trunk_port_cmds_vlan_translation(self, port_id, segmentation_id, trunk_details): + def get_trunk_port_cmds_vlan_translation(self, port_id, segmentation_id, + trunk_details): pass diff --git a/networking_generic_switch/generic_switch_mech.py b/networking_generic_switch/generic_switch_mech.py index 3c76554b..46945f9a 100644 --- a/networking_generic_switch/generic_switch_mech.py +++ b/networking_generic_switch/generic_switch_mech.py @@ -18,7 +18,6 @@ from neutron_lib.api.definitions import portbindings from neutron_lib.callbacks import resources from neutron_lib.plugins.ml2 import api -from neutron_lib.plugins import directory from oslo_log import log as logging from networking_generic_switch import config as gsw_conf @@ -479,25 +478,26 @@ def bind_port(self, context): vtr = self._is_vlan_translation_required(trunk_details) switch.plug_port_to_network_trunk( port_id, segmentation_id, trunk_details, vtr) - elif is_802_3ad and hasattr(switch, 'plug_bond_to_network'): + elif (is_802_3ad + and hasattr(switch, 'plug_bond_to_network')): switch.plug_bond_to_network(port_id, segmentation_id) else: switch.plug_port_to_network( port_id, segmentation_id) except ngs_exc.GenericSwitchNotSupported as e: LOG.warning("Operation is not supported by " - "networking-generic-switch. %(err)s)", + "networking-generic-switch. %(err)s)", {'err': e}) raise e except Exception as e: - LOG.error("Failed to bind port %(port_id)s in " - "segment %(segment_id)s on device " - "%(device)s due to error %(err)s", - {'port_id': port['id'], - 'device': switch_info, - 'segment_id': segmentation_id, - 'err': e}) - raise e + LOG.error("Failed to bind port %(port_id)s in " + "segment %(segment_id)s on device " + "%(device)s due to error %(err)s", + {'port_id': port['id'], + 'device': switch_info, + 'segment_id': segmentation_id, + 'err': e}) + raise e # END LOG.info("Successfully bound port %(port_id)s in segment " "%(segment_id)s on device %(device)s", diff --git a/networking_generic_switch/tests/unit/netmiko/test_dell.py b/networking_generic_switch/tests/unit/netmiko/test_dell.py index c85df012..7bb86980 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_dell.py +++ b/networking_generic_switch/tests/unit/netmiko/test_dell.py @@ -28,7 +28,6 @@ def _make_switch_device(self, extra_cfg={}): device_cfg.update(extra_cfg) return dell.DellOS10(device_cfg) - def test_get_trunk_port_cmds_no_vlan_translation(self): mock_context = mock.create_autospec(driver_context.PortContext) self.switch.ngs_config['vlan_translation_supported'] = True diff --git a/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py b/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py index ac2a2b83..a9270f65 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py +++ b/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py @@ -15,11 +15,10 @@ import re from unittest import mock -from neutron.plugins.ml2 import driver_context - import fixtures import netmiko import netmiko.base_connection +from neutron.plugins.ml2 import driver_context from oslo_config import fixture as config_fixture import paramiko import tenacity @@ -417,8 +416,10 @@ def test_bind_port_trunk_no_vts(self, t_mock, nt_mock, sctd_mock, 'trunk_details': {'sub_ports': [{'segmentation_id': 123}]}} trunk_details = {'sub_ports': [{'segmentation_id': 123}]} - self.switch.plug_port_to_network_trunk('2222', None, trunk_details, vtr=False) - nt_mock.assert_called_once_with('2222', None, {'sub_ports': [{'segmentation_id': 123}]}) + self.switch.plug_port_to_network_trunk('2222', None, trunk_details, + vtr=False) + nt_mock.assert_called_once_with( + '2222', None, {'sub_ports': [{'segmentation_id': 123}]}) self.assertFalse(t_mock.called) @mock.patch.object(netmiko_devices.netmiko, 'ConnectHandler') @@ -448,8 +449,10 @@ def test_bind_port_trunk_vts(self, t_mock, nt_mock, sctd_mock, 'trunk_details': {'sub_ports': [{'segmentation_id': 123}]}} trunk_details = {'sub_ports': [{'segmentation_id': 123}]} - self.switch.plug_port_to_network_trunk('2222', None, trunk_details, vtr=False) - t_mock.assert_called_once_with('2222', None, {'sub_ports': [{'segmentation_id': 123}]}) + self.switch.plug_port_to_network_trunk('2222', None, + trunk_details, vtr=False) + t_mock.assert_called_once_with( + '2222', None, {'sub_ports': [{'segmentation_id': 123}]}) self.assertFalse(nt_mock.called) @mock.patch.object(netmiko_devices.netmiko, 'ConnectHandler') @@ -462,11 +465,11 @@ def test_bind_port_trunk_vts(self, t_mock, nt_mock, sctd_mock, def test_bind_port_trunk_no_vts_raise(self, t_mock, nt_mock, sctd_mock, nm_mock): connect_mock = mock.Mock() - mock_context = mock.create_autospec(driver_context.PortContext) nm_mock.return_value = connect_mock self.switch.ngs_config['vlan_translation_supported'] = False trunk_details = {'sub_ports': [{'segmentation_id': 123}]} self.assertRaises(exc.GenericSwitchNotSupported, - self.switch.plug_port_to_network_trunk, '2222', None, trunk_details, vtr=True) + self.switch.plug_port_to_network_trunk, '2222', None, + trunk_details, vtr=True) self.assertFalse(t_mock.called) self.assertFalse(nt_mock.called) diff --git a/networking_generic_switch/tests/unit/test_generic_switch_mech.py b/networking_generic_switch/tests/unit/test_generic_switch_mech.py index 51798569..ebbdebd9 100644 --- a/networking_generic_switch/tests/unit/test_generic_switch_mech.py +++ b/networking_generic_switch/tests/unit/test_generic_switch_mech.py @@ -19,7 +19,6 @@ from neutron.db import provisioning_blocks from neutron.plugins.ml2 import driver_context from neutron_lib.callbacks import resources -from neutron_lib.plugins import directory from networking_generic_switch import exceptions from networking_generic_switch import generic_switch_mech as gsm @@ -836,7 +835,6 @@ def test_bind_port_trunk(self, m_apc, m_list, m_vlan): resources.PORT, 'GENERICSWITCH') - @mock.patch.object(provisioning_blocks, 'add_provisioning_component') def test_bind_portgroup(self, m_apc, m_list): driver = gsm.GenericSwitchDriver() From c397a0f377fde7553fb84d6f9fcae85dc65fa632 Mon Sep 17 00:00:00 2001 From: Will Szumski Date: Tue, 15 Feb 2022 12:16:12 +0000 Subject: [PATCH 05/10] Add support for Dell OS10 This adds a new driver for Dell OS10 based switches. Original change by msherman64[1]. [1] https://github.com/ChameleonCloud/networking-generic-switch/pull/14 Change-Id: Ib5bba3067352e6c7e12120982fcf5b206c9dd365 (cherry picked from commit 0ecd02aaa036311c67671717562db956bbc810aa) --- doc/source/configuration.rst | 10 ++ doc/source/supported-devices.rst | 1 + .../devices/netmiko_devices/dell.py | 1 + .../tests/unit/netmiko/test_dell.py | 118 ++++++++++++++++++ ...add-dellos10-support-c6426372f960ded4.yaml | 5 + setup.cfg | 1 + 6 files changed, 136 insertions(+) create mode 100644 releasenotes/notes/add-dellos10-support-c6426372f960ded4.yaml diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index 28c35a54..5e1afff3 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -92,6 +92,16 @@ for the Dell Force10 device:: password = password secret = secret +for the Dell OS10 device:: + + [genericswitch:dell-hostname] + device_type = netmiko_dell_os10 + ngs_mac_address = + ip = + username = admin + password = password + secret = secret + for the Dell PowerConnect device:: [genericswitch:dell-hostname] diff --git a/doc/source/supported-devices.rst b/doc/source/supported-devices.rst index 45632c97..3e207fac 100644 --- a/doc/source/supported-devices.rst +++ b/doc/source/supported-devices.rst @@ -10,6 +10,7 @@ The following devices are supported by this plugin: * Cisco IOS switches * Cumulus Linux (via NCLU) * Dell Force10 +* Dell OS10 * Dell PowerConnect * HPE 5900 Series switches * Huawei switches diff --git a/networking_generic_switch/devices/netmiko_devices/dell.py b/networking_generic_switch/devices/netmiko_devices/dell.py index 4ed7e0d0..2ce89323 100644 --- a/networking_generic_switch/devices/netmiko_devices/dell.py +++ b/networking_generic_switch/devices/netmiko_devices/dell.py @@ -23,6 +23,7 @@ class DellOS10(netmiko_devices.NetmikoSwitch): ADD_NETWORK = ( "interface vlan {segmentation_id}", + "description {network_name}", "exit", ) diff --git a/networking_generic_switch/tests/unit/netmiko/test_dell.py b/networking_generic_switch/tests/unit/netmiko/test_dell.py index 7bb86980..f489ed4e 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_dell.py +++ b/networking_generic_switch/tests/unit/netmiko/test_dell.py @@ -168,6 +168,124 @@ def test__format_commands(self): ['interface vlan 33', 'no tagged 3333', 'exit']) +class TestNetmikoDellOS10(test_netmiko_base.NetmikoSwitchTestBase): + + def _make_switch_device(self, extra_cfg={}): + device_cfg = {'device_type': 'netmiko_dell_os10'} + device_cfg.update(extra_cfg) + return dell.DellOS10(device_cfg) + + def test_constants(self): + self.assertIsNone(self.switch.SAVE_CONFIGURATION) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device') + def test_add_network(self, m_exec): + self.switch.add_network(33, '0ae071f5-5be9-43e4-80ea-e41fefe85b21') + m_exec.assert_called_with( + ['interface vlan 33', + 'description 0ae071f55be943e480eae41fefe85b21', + 'exit'] + ) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device') + def test_add_network_with_trunk_ports(self, mock_exec): + switch = self._make_switch_device({'ngs_trunk_ports': 'port1, port2'}) + switch.add_network(33, '0ae071f5-5be9-43e4-80ea-e41fefe85b21') + mock_exec.assert_called_with( + ['interface vlan 33', + 'description 0ae071f55be943e480eae41fefe85b21', + 'exit', + 'interface port1', 'switchport mode trunk', + 'switchport trunk allowed vlan 33', 'exit', + 'interface port2', 'switchport mode trunk', + 'switchport trunk allowed vlan 33', 'exit'] + ) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device') + def test_del_network(self, mock_exec): + self.switch.del_network(33, '0ae071f5-5be9-43e4-80ea-e41fefe85b21') + mock_exec.assert_called_with(['no interface vlan 33', 'exit']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device') + def test_del_network_with_trunk_ports(self, mock_exec): + switch = self._make_switch_device({'ngs_trunk_ports': 'port1, port2'}) + switch.del_network(33, '0ae071f55be943e480eae41fefe85b21') + mock_exec.assert_called_with( + ['interface port1', 'no switchport trunk allowed vlan 33', 'exit', + 'interface port2', 'no switchport trunk allowed vlan 33', 'exit', + 'no interface vlan 33', 'exit']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device') + def test_plug_port_to_network(self, mock_exec): + self.switch.plug_port_to_network(3333, 33) + mock_exec.assert_called_with( + ['interface 3333', 'switchport mode access', + 'switchport access vlan 33', + 'exit'] + ) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device') + def test_delete_port(self, mock_exec): + self.switch.delete_port(3333, 33) + mock_exec.assert_called_with( + ['interface 3333', 'no switchport access vlan', 'exit'] + ) + + def test__format_commands(self): + cmd_set = self.switch._format_commands( + dell.DellOS10.ADD_NETWORK, + segmentation_id=22, + network_id=22, + network_name='vlan-22') + self.assertEqual(cmd_set, + ['interface vlan 22', + 'description vlan-22', + 'exit'] + ) + + cmd_set = self.switch._format_commands( + dell.DellOS10.DELETE_NETWORK, + segmentation_id=22) + self.assertEqual(cmd_set, ['no interface vlan 22', 'exit']) + + cmd_set = self.switch._format_commands( + dell.DellOS10.PLUG_PORT_TO_NETWORK, + port=3333, + segmentation_id=33) + self.assertEqual(cmd_set, ['interface 3333', 'switchport mode access', + 'switchport access vlan 33', 'exit']) + cmd_set = self.switch._format_commands( + dell.DellOS10.DELETE_PORT, + port=3333, + segmentation_id=33) + self.assertEqual(cmd_set, + ['interface 3333', 'no switchport access vlan', + 'exit']) + + cmd_set = self.switch._format_commands( + dell.DellOS10.ADD_NETWORK_TO_TRUNK, + port=3333, + segmentation_id=33) + self.assertEqual(cmd_set, + ['interface 3333', 'switchport mode trunk', + 'switchport trunk allowed vlan 33', + 'exit']) + cmd_set = self.switch._format_commands( + dell.DellOS10.REMOVE_NETWORK_FROM_TRUNK, + port=3333, + segmentation_id=33) + self.assertEqual(cmd_set, + ['interface 3333', + 'no switchport trunk allowed vlan 33', + 'exit']) + + class TestNetmikoDellPowerConnect(test_netmiko_base.NetmikoSwitchTestBase): def _make_switch_device(self, extra_cfg={}): diff --git a/releasenotes/notes/add-dellos10-support-c6426372f960ded4.yaml b/releasenotes/notes/add-dellos10-support-c6426372f960ded4.yaml new file mode 100644 index 00000000..6e9d70bc --- /dev/null +++ b/releasenotes/notes/add-dellos10-support-c6426372f960ded4.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Adds a new device driver, ``netmiko_dell_os10``, for managing Dell OS10 + based switch devices. diff --git a/setup.cfg b/setup.cfg index bd83edbd..68ebe4e7 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,6 +36,7 @@ generic_switch.devices = netmiko_huawei = networking_generic_switch.devices.netmiko_devices.huawei:Huawei netmiko_huawei_vrpv8 = networking_generic_switch.devices.netmiko_devices.huawei_vrpv8:Huawei netmiko_arista_eos = networking_generic_switch.devices.netmiko_devices.arista:AristaEos + netmiko_dell_os10 = networking_generic_switch.devices.netmiko_devices.dell:DellOS10 netmiko_dell_force10 = networking_generic_switch.devices.netmiko_devices.dell:DellNos netmiko_dell_powerconnect = networking_generic_switch.devices.netmiko_devices.dell:DellPowerConnect netmiko_brocade_fastiron = networking_generic_switch.devices.netmiko_devices.brocade:BrocadeFastIron From e16b831a9b0972e510e99f8d224eb71d3b9158d5 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Wed, 8 Feb 2023 11:59:19 +0000 Subject: [PATCH 06/10] fixup! Dell OS10 backport without trunk port strip patch Change-Id: I5d2f098d7633a23d60ae85d8b1e4cffb43182b4e (cherry picked from commit 56735ab351936f669eabfb27d0160190ac8a5350) --- networking_generic_switch/tests/unit/netmiko/test_dell.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/networking_generic_switch/tests/unit/netmiko/test_dell.py b/networking_generic_switch/tests/unit/netmiko/test_dell.py index f489ed4e..8b16a140 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_dell.py +++ b/networking_generic_switch/tests/unit/netmiko/test_dell.py @@ -191,7 +191,7 @@ def test_add_network(self, m_exec): @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.send_commands_to_device') def test_add_network_with_trunk_ports(self, mock_exec): - switch = self._make_switch_device({'ngs_trunk_ports': 'port1, port2'}) + switch = self._make_switch_device({'ngs_trunk_ports': 'port1,port2'}) switch.add_network(33, '0ae071f5-5be9-43e4-80ea-e41fefe85b21') mock_exec.assert_called_with( ['interface vlan 33', @@ -212,7 +212,7 @@ def test_del_network(self, mock_exec): @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.send_commands_to_device') def test_del_network_with_trunk_ports(self, mock_exec): - switch = self._make_switch_device({'ngs_trunk_ports': 'port1, port2'}) + switch = self._make_switch_device({'ngs_trunk_ports': 'port1,port2'}) switch.del_network(33, '0ae071f55be943e480eae41fefe85b21') mock_exec.assert_called_with( ['interface port1', 'no switchport trunk allowed vlan 33', 'exit', From b8331f9a86cd43ae0e9a51f147a568498fe2f679 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Mon, 20 Feb 2023 20:09:50 +0000 Subject: [PATCH 07/10] fixup! Dell OS10 trunk port unit tests Change-Id: I46684ae1cdd8207833172e5ee04aa865eb965931 (cherry picked from commit 8aafe00013cde9231622bde81014377fc9f33bf4) --- .../tests/unit/netmiko/test_dell.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/networking_generic_switch/tests/unit/netmiko/test_dell.py b/networking_generic_switch/tests/unit/netmiko/test_dell.py index 8b16a140..2cbbe791 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_dell.py +++ b/networking_generic_switch/tests/unit/netmiko/test_dell.py @@ -285,6 +285,43 @@ def test__format_commands(self): 'no switchport trunk allowed vlan 33', 'exit']) + def test_get_trunk_port_cmds_no_vlan_translation(self): + mock_context = mock.create_autospec(driver_context.PortContext) + self.switch.ngs_config['vlan_translation_supported'] = True + trunk_details = {'trunk_id': 'aaa-bbb-ccc-ddd', + 'sub_ports': [{'segmentation_id': 130, + 'port_id': 'aaa-bbb-ccc-ddd', + 'segmentation_type': 'vlan', + 'mac_address': u'fa:16:3e:1c:c2:7e'}]} + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': '2222' + } + ] + }, + 'binding:vnic_type': 'baremetal', + 'id': 'aaaa-bbbb-cccc', + 'trunk_details': trunk_details} + mock_context.network = mock.Mock() + mock_context.network.current = {'provider:segmentation_id': 123} + mock_context.segments_to_bind = [ + { + 'segmentation_id': 777, + 'id': 123 + } + ] + res = self.switch.get_trunk_port_cmds_no_vlan_translation( + '2222', 777, trunk_details) + self.assertEqual(['interface 2222', 'switchport mode access', + 'switchport mode trunk', + 'switchport access vlan 777', + 'interface 2222', + 'switchport trunk allowed vlan 130'], + res) + class TestNetmikoDellPowerConnect(test_netmiko_base.NetmikoSwitchTestBase): From bdf12060821defeebf7de70315159db7f050b273 Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Mon, 27 Jul 2020 17:59:10 +0100 Subject: [PATCH 08/10] Support batching up commands When you have around 60 baremetal nodes attached to a single switch, it takes a long time to execute all those commands. This gets worse when you limit the number of concurrent ssh connections. Here we look to batch up commands to send to the switch together using a single connection. The results of each port's commands are returned when available. This is implemented using etcd as a queueing system. Commands are added to an input key, then a worker thread processes the available commands for a particular switch device. We pull off the queue using the version at which the keys were added, giving a FIFO style queue. The result of each command set are added to an output key, which the original request thread is watching. Distributed locks are used to serialise the processing of commands for each switch device. Various neat etcd features are used here to alleviate some of the issues of distributed task coordination, including transactions, leases, watches, historical key/value tracking, etc. Co-Authored-By: Mark Goddard Change-Id: I8c458bbc94df5630cfede5434bcdbe527988059c (cherry picked from commit 45b237b754cd2d9c6f9c588e759fa74da5b9eacc) --- doc/source/configuration.rst | 46 +- networking_generic_switch/batching.py | 451 ++++++++++++++++++ networking_generic_switch/devices/__init__.py | 9 + .../devices/netmiko_devices/__init__.py | 33 +- networking_generic_switch/exceptions.py | 4 + .../tests/unit/netmiko/test_netmiko_base.py | 9 + .../tests/unit/test_batching.py | 442 +++++++++++++++++ .../notes/batching-12d9005924fd9d74.yaml | 4 + requirements.txt | 2 + 9 files changed, 989 insertions(+), 11 deletions(-) create mode 100644 networking_generic_switch/batching.py create mode 100644 networking_generic_switch/tests/unit/test_batching.py create mode 100644 releasenotes/notes/batching-12d9005924fd9d74.yaml diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index 5e1afff3..76ee1c10 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -118,8 +118,9 @@ for the Dell PowerConnect device:: ngs_switchport_mode = access Dell PowerConnect devices have been seen to have issues with multiple -concurrent configuration sessions. See :ref:`synchronization` for details on -how to limit the number of concurrent active connections to each device. +concurrent configuration sessions. See :ref:`synchronization` and +:ref:`batching` for details on how to limit the number of concurrent active +connections to each device. for the Brocade FastIron (ICX) device:: @@ -217,8 +218,16 @@ connection URL for the backend should be configured as follows:: [ngs_coordination] backend_url = -The default is to limit the number of concurrent active connections to each -device to one, but the number may be configured per-device as follows:: +The backend URL format includes the Tooz driver as the scheme, with driver +options passed using query string parameters. For example, to use the +``etcd3gw`` driver with an API version of ``v3`` and a path to a CA +certificate:: + + [ngs_coordination] + backend_url = etcd3+https://etcd.example.com?api_version=v3,ca_cert=/path/to/ca/cert.crt + +The default behaviour is to limit the number of concurrent active connections +to each device to one, but the number may be configured per-device as follows:: [genericswitch:device-hostname] ngs_max_connections = @@ -232,6 +241,35 @@ timeout of 60 seconds before failing. This timeout can be configured as follows ... acquire_timeout = +.. _batching: + +Batching +======== + +For many network devices there is a significant SSH connection overhead which +is incurred for each network or port configuration change. In a large scale +system with many concurrent changes, this overhead adds up quickly. Since the +Antelope release, the Generic Switch driver includes support to batch up switch +configuration changes and apply them together using a single SSH connection. + +This is implemented using etcd as a queueing system. Commands are added +to an input key, then a worker thread processes the available commands +for a particular switch device. We pull off the queue using the version +at which the keys were added, giving a FIFO style queue. The result of +each command set are added to an output key, which the original request +thread is watching. Distributed locks are used to serialise the +processing of commands for each switch device. + +The etcd endpoint is configured using the same ``[ngs_coordination] +backend_url`` option used in :ref:`synchronization`, with the limitation that +only ``etcd3gw`` is supported. + +Additionally, each device that will use batched configuration should include +the following option:: + + [genericswitch:device-hostname] + ngs_batch_requests = True + Disabling Inactive Ports ======================== diff --git a/networking_generic_switch/batching.py b/networking_generic_switch/batching.py new file mode 100644 index 00000000..50599cbe --- /dev/null +++ b/networking_generic_switch/batching.py @@ -0,0 +1,451 @@ +# Copyright 2023 StackHPC +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import atexit +import json + +import etcd3gw +from etcd3gw import exceptions as etcd3gw_exc +from etcd3gw.utils import _decode +from etcd3gw.utils import _encode +from etcd3gw.utils import _increment_last_byte +import eventlet +from oslo_log import log as logging +from oslo_utils import netutils +from oslo_utils import uuidutils +import tenacity + +from networking_generic_switch import exceptions as exc + +SHUTDOWN_TIMEOUT = 60 + +LOG = logging.getLogger(__name__) + +THREAD_POOL = eventlet.greenpool.GreenPool() + + +class ShutdownTimeout(Exception): + """Exception raised when shutdown timeout is exceeded.""" + + +@atexit.register +def _wait_for_threads(): + """Wait for all threads in the pool to complete. + + This function is registered to execute at exit, to ensure that all worker + threads have completed. These threads may be holding switch execution locks + and performing switch configuration operations which should not be + interrupted. + """ + LOG.info("Waiting %d seconds for %d threads to complete", + SHUTDOWN_TIMEOUT, THREAD_POOL.running()) + try: + with eventlet.Timeout(SHUTDOWN_TIMEOUT, ShutdownTimeout): + THREAD_POOL.waitall() + except ShutdownTimeout: + LOG.error("Timed out waiting for threads to complete") + else: + LOG.info("Finished waiting for threads to complete") + + +class SwitchQueueItem(object): + """An item in the queue.""" + + def __init__(self, uuid, create_revision): + self.uuid = uuid + self.create_revision = create_revision + + +class SwitchQueue(object): + INPUT_PREFIX = "/ngs/batch/%s/input/" + INPUT_ITEM_KEY = "/ngs/batch/%s/input/%s" + RESULT_ITEM_KEY = "/ngs/batch/%s/output/%s" + EXEC_LOCK = "/ngs/batch/%s/execute_lock" + + def __init__(self, switch_name, etcd_client): + self.switch_name = switch_name + self.client = etcd_client + self.lease_ttl = 600 + + def add_batch(self, cmds): + """Clients add batch, given key events. + + Each batch is given an uuid that is used to generate both + and input and result key in etcd. + + First we watch for any results, second we write the input + in a location that the caller of get_batches will be looking. + + No locks are required when calling this function to send work + to the workers, and start waiting for results. + + :param cmds: an iterable of commands + :return: a SwitchQueueItem object + """ + + uuid = uuidutils.generate_uuid() + result_key = self.RESULT_ITEM_KEY % (self.switch_name, uuid) + input_key = self.INPUT_ITEM_KEY % (self.switch_name, uuid) + + batch = { + "uuid": uuid, + "input_key": input_key, + "result_key": result_key, + "cmds": cmds, + } + value = json.dumps(batch, sort_keys=True).encode("utf-8") + lease = self.client.lease(ttl=self.lease_ttl) + # Use a transaction rather than create() in order to extract the + # create revision. + base64_key = _encode(input_key) + base64_value = _encode(value) + txn = { + 'compare': [{ + 'key': base64_key, + 'result': 'EQUAL', + 'target': 'CREATE', + 'create_revision': 0 + }], + 'success': [{ + 'request_put': { + 'key': base64_key, + 'value': base64_value, + } + }], + 'failure': [] + } + txn['success'][0]['request_put']['lease'] = lease.id + result = self.client.transaction(txn) + + success = result.get('succeeded', False) + # Be sure to free watcher resources on error + if not success: + raise exc.GenericSwitchBatchError( + device=self.switch_name, + error="Failed to add batch to input key: %s" % input_key) + + put_response = result['responses'][0]['response_put'] + create_revision = put_response['header']['revision'] + LOG.debug("written input key %s revision %s", + input_key, create_revision) + + return SwitchQueueItem(uuid, create_revision) + + def wait_for_result(self, item, timeout): + """Wait for the result of a command batch. + + :param item: SwitchQueueItem object returned by add_batch + :param timeout: wait timeout in seconds + :return: output string generated by this command set + :raises: Exception if waiting times out or the command batch was + unsuccessful + """ + result_key = self.RESULT_ITEM_KEY % (self.switch_name, item.uuid) + try: + event = self.client.watch_once(result_key, timeout=timeout, + start_revision=item.create_revision) + except etcd3gw_exc.WatchTimedOut: + raise exc.GenericSwitchBatchError( + device=self.switch_name, + error="Timed out waiting for result key: %s" % result_key) + + LOG.debug("got event: %s", event) + if event["kv"]["version"] == 0: + raise exc.GenericSwitchBatchError( + device=self.switch_name, + error="Output key was deleted, perhaps lease expired") + # TODO(johngarbutt) check we have the create event and result? + result_dict = self._get_and_delete_result(result_key) + LOG.debug("got result: %s", result_dict) + if "result" in result_dict: + return result_dict["result"] + else: + raise exc.GenericSwitchBatchError( + device=self.switch_name, + error=result_dict["error"]) + + def _get_and_delete_result(self, result_key): + # called when watch event says the result key should exist + txn = { + 'compare': [], + 'success': [{ + 'request_delete_range': { + 'key': _encode(result_key), + 'prev_kv': True, + } + }], + 'failure': [] + } + result = self.client.transaction(txn) + success = result.get('succeeded', False) + if not success: + raise exc.GenericSwitchBatchError( + device=self.switch_name, + error="Unable to find result: %s" % result_key) + delete_response = result['responses'][0]['response_delete_range'] + raw_value = delete_response['prev_kvs'][0]['value'] + result_dict = json.loads(_decode(raw_value)) + LOG.debug("fetched and deleted result for: %s", result_key) + return result_dict + + def _get_raw_batches(self, max_create_revision=None): + input_prefix = self.INPUT_PREFIX % self.switch_name + # Sort order ensures FIFO style queue + # Use get rather than get_prefix since get accepts max_create_revision. + range_end = _encode(_increment_last_byte(input_prefix)) + raw_batches = self.client.get(input_prefix, + metadata=True, + range_end=range_end, + sort_order="ascend", + sort_target="create", + max_create_revision=max_create_revision) + return raw_batches + + def get_batches(self, item=None): + """Return a list of the event dicts written in wait for result. + + This is called both with or without getting a lock to get the + latest list of work that has send to the per switch queue in + etcd. + + :param item: Optional SwitchQueueItem object. If provided, only batches + added up to and including this item are returned. + """ + max_create_revision = item.create_revision if item else None + raw_batches = self._get_raw_batches(max_create_revision) + LOG.debug("found %s batches", len(raw_batches)) + + batches = [] + for raw_value, metadata in raw_batches: + batch = json.loads(raw_value.decode('utf-8')) + batches.append(batch) + return batches + + def record_result(self, batch): + """Record the result from executing given command set. + + We assume that a lock is held before getting a fresh list + of batches, executing them, and then calling this record + results function, before finally dropping the lock. + """ + # Write results and delete input keys so the next worker to hold the + # lock knows not to execute these batches + lease = self.client.lease(ttl=self.lease_ttl) + result_value = json.dumps(batch, sort_keys=True).encode('utf-8') + txn = { + 'compare': [], + 'success': [ + { + 'request_put': { + 'key': _encode(batch['result_key']), + 'value': _encode(result_value), + 'lease': lease.id, + } + }, + { + 'request_delete_range': { + 'key': _encode(batch['input_key']), + } + } + ], + 'failure': [] + } + result = self.client.transaction(txn) + success = result.get('succeeded', False) + if not success: + LOG.error("failed to report batch result for: %s", + batch) + else: + LOG.debug("written result key: %s", batch['result_key']) + + def acquire_worker_lock(self, item, acquire_timeout=300, lock_ttl=120, + wait=None): + """Wait for lock needed to call record_result. + + This blocks until the work queue is empty of the switch lock is + acquired. If we timeout waiting for the lock we raise an exception. + """ + lock_name = self.EXEC_LOCK % self.switch_name + lock = self.client.lock(lock_name, lock_ttl) + + if wait is None: + wait = tenacity.wait_random(min=1, max=3) + + @tenacity.retry( + # Log a message after each failed attempt. + after=tenacity.after_log(LOG, logging.DEBUG), + # Retry if we haven't got the lock yet + retry=tenacity.retry_if_result(lambda x: x is False), + # Stop after timeout. + stop=tenacity.stop_after_delay(acquire_timeout), + # Wait between lock retries + wait=wait, + ) + def _acquire_lock_with_retry(): + lock_acquired = lock.acquire() + if lock_acquired: + return lock + + # Stop waiting for the lock if there is nothing to do + work = self._get_raw_batches(item.create_revision) + if not work: + return None + + # Trigger a retry + return False + + return _acquire_lock_with_retry() + + +class SwitchBatch(object): + def __init__(self, switch_name, etcd_url=None, switch_queue=None): + if switch_queue is None: + parsed_url = netutils.urlsplit(etcd_url) + host = parsed_url.hostname + port = parsed_url.port + protocol = 'https' if parsed_url.scheme.endswith( + 'https') else 'http' + # Use the same parameter format as tooz etcd3gw driver. + params = parsed_url.params() + ca_cert = params.get('ca_cert') + cert_key = params.get('cert_key') + cert_cert = params.get('cert_cert') + api_version = params.get('api_version', 'v3alpha') + etcd_client = etcd3gw.client( + host=host, port=port, protocol=protocol, + ca_cert=ca_cert, cert_key=cert_key, cert_cert=cert_cert, + api_path='/' + api_version + '/', + timeout=30) + self.queue = SwitchQueue(switch_name, etcd_client) + else: + self.queue = switch_queue + self.switch_name = switch_name + + def do_batch(self, device, cmd_set, timeout=300): + """Batch up switch configuration commands to reduce overheads. + + We collect together the iterables in the cmd_set, and + execute them toegether in a single larger batch. + + :param device: a NetmikoSwitch device object + :param cmd_set: an iterable of commands + :return: output string generated by this command set + """ + + # request that the cmd_set by executed + cmd_list = list(cmd_set) + item = self.queue.add_batch(cmd_list) + + def do_work(): + try: + self._execute_pending_batches(device, item) + except Exception as e: + LOG.error("failed to run execute batch: %s", e, + exec_info=True) + raise + + self._spawn(do_work) + + # Wait for our result key + # as the result might be done before the above task starts + output = self.queue.wait_for_result(item, timeout) + LOG.debug("Got batch result: %s", output) + return output + + @staticmethod + def _spawn(work_fn): + # TODO(johngarbutt) remove hard eventlet dependency + # in a similar way to etcd3gw + # Sleep to let possible other work to batch together + eventlet.sleep(0) + # Run all pending tasks, which might be a no op + # if pending tasks already ran + THREAD_POOL.spawn_n(work_fn) + + def _execute_pending_batches(self, device, item): + """Execute all batches currently registered. + + Typically called by every caller of add_batch. + Could be a noop if all batches are already executed. + + :param device: a NetmikoSwitch device object + :param item: a SwitchQueueItem object + """ + batches = self.queue.get_batches(item) + if not batches: + LOG.debug("Skipped execution for %s", self.switch_name) + return + LOG.debug("Found %d batches - trying to acquire lock for %s", + len(batches), self.switch_name) + + # Many workers can end up piling up here trying to acquire the + # lock. Only consider batches at least as old as the one that triggered + # this worker, to ensure they don't wait forever. + lock = self.queue.acquire_worker_lock(item) + if lock is None: + # This means we stopped waiting as the work queue was empty + LOG.debug("Work list empty for %s", self.switch_name) + return + + # Check we got the lock + if not lock.is_acquired(): + raise exc.GenericSwitchBatchError( + device=self.switch_name, + error="unable to get lock for: %s" % self.switch_name) + + # be sure to drop the lock when we are done + try: + LOG.debug("got lock for %s", self.switch_name) + + # Fetch fresh list now we have the lock + # and order the list so we execute in order added + batches = self.queue.get_batches() + if not batches: + LOG.debug("No batches to execute %s", self.switch_name) + return + + LOG.debug("Starting to execute %d batches", len(batches)) + self._send_commands(device, batches, lock) + finally: + lock.release() + + LOG.debug("end of lock for %s", self.switch_name) + + def _send_commands(self, device, batches, lock): + with device._get_connection() as net_connect: + for batch in batches: + try: + output = device.send_config_set(net_connect, batch['cmds']) + batch["result"] = output + except Exception as e: + batch["error"] = str(e) + + # The switch configuration can take a long time, and may exceed + # the lock TTL. Periodically refresh our lease, and verify that + # we still own the lock before recording the results. + lock.refresh() + if not lock.is_acquired(): + raise exc.GenericSwitchBatchError( + device=self.switch_name, + error="Worker aborting - lock timed out") + + # Tell request watchers the result and + # tell workers which batches have now been executed + self.queue.record_result(batch) + + try: + device.save_configuration(net_connect) + except Exception: + LOG.exception("Failed to save configuration") + # Probably not worth failing all batches for this. diff --git a/networking_generic_switch/devices/__init__.py b/networking_generic_switch/devices/__init__.py index b79fe717..4ee6da14 100644 --- a/networking_generic_switch/devices/__init__.py +++ b/networking_generic_switch/devices/__init__.py @@ -43,6 +43,10 @@ # If false, ngs will not add and delete VLANs from switches {'name': 'ngs_manage_vlans', 'default': True}, {'name': 'vlan_translation_supported', 'default': False} + # If False, ngs will skip saving configuration on devices + {'name': 'ngs_save_configuration', 'default': True}, + # When true try to batch up in flight switch requests + {'name': 'ngs_batch_requests', 'default': False}, ] @@ -137,6 +141,11 @@ def _do_vlan_management(self): """Check if drivers should add and remove VLANs from switches.""" return strutils.bool_from_string(self.ngs_config['ngs_manage_vlans']) + def _batch_requests(self): + """Return whether to batch up requests to the switch.""" + return strutils.bool_from_string( + self.ngs_config['ngs_batch_requests']) + @abc.abstractmethod def add_network(self, segmentation_id, network_id): pass diff --git a/networking_generic_switch/devices/netmiko_devices/__init__.py b/networking_generic_switch/devices/netmiko_devices/__init__.py index 9bf70606..c4d16f60 100644 --- a/networking_generic_switch/devices/netmiko_devices/__init__.py +++ b/networking_generic_switch/devices/netmiko_devices/__init__.py @@ -24,6 +24,7 @@ import tenacity from tooz import coordination +from networking_generic_switch import batching from networking_generic_switch import devices from networking_generic_switch.devices import utils as device_utils from networking_generic_switch import exceptions as exc @@ -117,20 +118,32 @@ def __init__(self, device_cfg): self.config['session_log_record_writes'] = True self.config['session_log_file_mode'] = 'append' + self.lock_kwargs = { + 'locks_pool_size': int(self.ngs_config['ngs_max_connections']), + 'locks_prefix': self.config.get( + 'host', '') or self.config.get('ip', ''), + 'timeout': CONF.ngs_coordination.acquire_timeout} + self.locker = None - if CONF.ngs_coordination.backend_url: + self.batch_cmds = None + if self._batch_requests(): + if not CONF.ngs_coordination.backend_url: + raise exc.GenericSwitchNetmikoConfigError( + config=device_utils.sanitise_config(self.config), + error="ngs_batch_requests is true but [ngs_coordination] " + "backend_url is not provided") + # NOTE: we skip the lock if we are batching requests + self.locker = None + switch_name = self.lock_kwargs['locks_prefix'] + self.batch_cmds = batching.SwitchBatch( + switch_name, CONF.ngs_coordination.backend_url) + elif CONF.ngs_coordination.backend_url: self.locker = coordination.get_coordinator( CONF.ngs_coordination.backend_url, ('ngs-' + device_utils.get_hostname()).encode('ascii')) self.locker.start() atexit.register(self.locker.stop) - self.lock_kwargs = { - 'locks_pool_size': int(self.ngs_config['ngs_max_connections']), - 'locks_prefix': self.config.get( - 'host', '') or self.config.get('ip', ''), - 'timeout': CONF.ngs_coordination.acquire_timeout} - def _format_commands(self, commands, **kwargs): if not commands: return [] @@ -192,6 +205,12 @@ def send_commands_to_device(self, cmd_set): LOG.debug("Nothing to execute") return + # If configured, batch up requests to the switch + if self.batch_cmds is not None: + return self.batch_cmds.do_batch(self, cmd_set) + return self._send_commands_to_device(cmd_set) + + def _send_commands_to_device(self, cmd_set): try: with ngs_lock.PoolLock(self.locker, **self.lock_kwargs): with self._get_connection() as net_connect: diff --git a/networking_generic_switch/exceptions.py b/networking_generic_switch/exceptions.py index edadfb5d..27f4de8e 100644 --- a/networking_generic_switch/exceptions.py +++ b/networking_generic_switch/exceptions.py @@ -54,3 +54,7 @@ class GenericSwitchNetmikoConfigError(GenericSwitchException): class GenericSwitchNotSupported(GenericSwitchException): message = _("Requested feature is not supported by " "networking-generic-switch. %(error)s") + + +class GenericSwitchBatchError(GenericSwitchException): + message = _("Batching error: %(device)s, error: %(error)s") diff --git a/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py b/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py index a9270f65..b4a0826e 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py +++ b/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py @@ -48,6 +48,15 @@ def _make_switch_device(self, extra_cfg={}): class TestNetmikoSwitch(NetmikoSwitchTestBase): + def test_batch(self): + self.cfg.config(backend_url='url', group='ngs_coordination') + self._make_switch_device({'ngs_batch_requests': True}) + + def test_batch_missing_backend_url(self): + self.assertRaisesRegex( + Exception, "backend_url", + self._make_switch_device, {'ngs_batch_requests': True}) + @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.send_commands_to_device', return_value='fake output') diff --git a/networking_generic_switch/tests/unit/test_batching.py b/networking_generic_switch/tests/unit/test_batching.py new file mode 100644 index 00000000..868186a8 --- /dev/null +++ b/networking_generic_switch/tests/unit/test_batching.py @@ -0,0 +1,442 @@ +# Copyright 2023 StackHPC +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from unittest import mock + +from etcd3gw.exceptions import Etcd3Exception +from etcd3gw.utils import _encode +from etcd3gw.utils import _increment_last_byte +import fixtures +from oslo_config import fixture as config_fixture +from oslo_utils import uuidutils +import tenacity + +from networking_generic_switch import batching +from networking_generic_switch import exceptions as exc + + +class SwitchQueueTest(fixtures.TestWithFixtures): + def setUp(self): + super(SwitchQueueTest, self).setUp() + self.cfg = self.useFixture(config_fixture.Config()) + + self.client = mock.Mock() + self.switch_name = "switch1" + self.queue = batching.SwitchQueue(self.switch_name, self.client) + + @mock.patch.object(uuidutils, "generate_uuid") + def test_add_batch(self, mock_uuid): + mock_uuid.return_value = "uuid" + self.client.transaction.return_value = { + "succeeded": True, + "responses": [{ + "response_put": { + "header": { + "revision": 42 + } + } + }] + } + + item = self.queue.add_batch(["cmd1", "cmd2"]) + + self.assertEqual("uuid", item.uuid) + self.assertEqual(42, item.create_revision) + input_key = '/ngs/batch/switch1/input/uuid' + expected_value = ( + b'{"cmds": ["cmd1", "cmd2"], ' + b'"input_key": "/ngs/batch/switch1/input/uuid", ' + b'"result_key": "/ngs/batch/switch1/output/uuid", ' + b'"uuid": "uuid"}') + expected_txn = { + 'compare': [{ + 'key': _encode(input_key), + 'result': 'EQUAL', + 'target': 'CREATE', + 'create_revision': 0 + }], + 'success': [{ + 'request_put': { + 'key': _encode(input_key), + 'value': _encode(expected_value), + 'lease': mock.ANY, + } + }], + 'failure': [] + } + self.client.transaction.assert_called_once_with(expected_txn) + + @mock.patch.object(uuidutils, "generate_uuid") + def test_add_batch_failure(self, mock_uuid): + mock_uuid.return_value = "uuid" + self.client.transaction.side_effect = Etcd3Exception + + self.assertRaises(Etcd3Exception, + self.queue.add_batch, ["cmd1", "cmd2"]) + + @mock.patch.object(uuidutils, "generate_uuid") + def test_add_batch_failure2(self, mock_uuid): + mock_uuid.return_value = "uuid" + self.client.transaction.return_value = {"succeeded": False} + + self.assertRaises(exc.GenericSwitchBatchError, + self.queue.add_batch, ["cmd1", "cmd2"]) + + @mock.patch.object(batching.SwitchQueue, "_get_and_delete_result") + def test_wait_for_result(self, mock_get): + event = { + "kv": { + "version": 1 + } + } + self.client.watch_once.return_value = event + mock_get.return_value = {"result": "result1"} + item = batching.SwitchQueueItem("uuid", 42) + + result = self.queue.wait_for_result(item, 43) + + self.assertEqual("result1", result) + result_key = '/ngs/batch/switch1/output/uuid' + self.client.watch_once.assert_called_once_with( + result_key, timeout=43, start_revision=42) + mock_get.assert_called_once_with(result_key) + + def test_get_and_delete_result(self): + self.client.transaction.return_value = { + "succeeded": True, + "responses": [{ + "response_delete_range": { + "prev_kvs": [{ + "value": _encode(b'{"foo": "bar"}') + }] + } + }] + } + + result = self.queue._get_and_delete_result(b"result_key") + + self.assertEqual({"foo": "bar"}, result) + + expected_txn = { + 'compare': [], + 'success': [{ + 'request_delete_range': { + 'key': _encode(b"result_key"), + 'prev_kv': True, + } + }], + 'failure': [] + } + self.client.transaction.assert_called_once_with(expected_txn) + + def test_get_and_delete_result_failure(self): + self.client.transaction.return_value = {"succeeded": False} + + self.assertRaises(exc.GenericSwitchBatchError, + self.queue._get_and_delete_result, b"result_key") + + def test_get_batches(self): + self.client.get.return_value = [ + (b'{"foo": "bar"}', {}), + (b'{"foo1": "bar1"}', {}) + ] + + batches = self.queue.get_batches() + + self.assertEqual([ + {"foo": "bar"}, + {"foo1": "bar1"} + ], batches) + input_prefix = '/ngs/batch/switch1/input/' + self.client.get.assert_called_once_with( + input_prefix, + metadata=True, + range_end=_encode(_increment_last_byte(input_prefix)), + sort_order='ascend', sort_target='create', + max_create_revision=None) + + def test_get_batches_with_item(self): + self.client.get.return_value = [ + (b'{"foo": "bar"}', {}), + (b'{"foo1": "bar1"}', {}) + ] + item = batching.SwitchQueueItem("uuid", 42) + + batches = self.queue.get_batches(item) + + self.assertEqual([ + {"foo": "bar"}, + {"foo1": "bar1"} + ], batches) + input_prefix = '/ngs/batch/switch1/input/' + self.client.get.assert_called_once_with( + input_prefix, + metadata=True, + range_end=_encode(_increment_last_byte(input_prefix)), + sort_order='ascend', sort_target='create', + max_create_revision=42) + + def test_record_result(self): + self.client.transaction.return_value = {"succeeded": True} + batch = {"result_key": "result1", "input_key": "input1", + "result": "asdf"} + + self.queue.record_result(batch) + + self.client.lease.assert_called_once_with(ttl=600) + expected_value = ( + b'{"input_key": "input1", ' + b'"result": "asdf", "result_key": "result1"}') + expected_txn = { + 'compare': [], + 'success': [ + { + 'request_put': { + 'key': _encode("result1"), + 'value': _encode(expected_value), + 'lease': mock.ANY, + } + }, + { + 'request_delete_range': { + 'key': _encode("input1"), + } + } + ], + 'failure': [] + } + self.client.transaction.assert_called_once_with(expected_txn) + + def test_record_result_failure(self): + self.client.transaction.return_value = {"succeeded": False} + batch = {"result_key": "result1", "input_key": "input1", + "result": "asdf"} + + # Should not raise an exception. + self.queue.record_result(batch) + + @mock.patch.object(batching.SwitchQueue, "_get_raw_batches") + def test_acquire_worker_lock_timeout(self, mock_get): + mock_get.return_value = ["work"] + lock = mock.MagicMock() + lock.acquire.return_value = False + self.client.lock.return_value = lock + item = batching.SwitchQueueItem("uuid", 42) + + wait = tenacity.wait_none() + self.assertRaises( + tenacity.RetryError, + self.queue.acquire_worker_lock, + item, wait=wait, acquire_timeout=0.05) + + @mock.patch.object(batching.SwitchQueue, "_get_raw_batches") + def test_acquire_worker_lock_no_work(self, mock_get): + mock_get.side_effect = [["work"], None] + lock = mock.MagicMock() + lock.acquire.return_value = False + self.client.lock.return_value = lock + item = batching.SwitchQueueItem("uuid", 42) + + wait = tenacity.wait_none() + result = self.queue.acquire_worker_lock( + item, wait=wait, acquire_timeout=0.05) + + self.assertIsNone(result) + self.assertEqual(2, mock_get.call_count) + self.assertEqual(2, lock.acquire.call_count) + + @mock.patch.object(batching.SwitchQueue, "_get_raw_batches") + def test_acquire_worker_lock_success(self, mock_get): + mock_get.return_value = ["work"] + lock = mock.MagicMock() + lock.acquire.side_effect = [False, False, True] + self.client.lock.return_value = lock + item = batching.SwitchQueueItem("uuid", 42) + + wait = tenacity.wait_none() + result = self.queue.acquire_worker_lock( + item, wait=wait, acquire_timeout=0.05) + + self.assertEqual(lock, result) + self.assertEqual(2, mock_get.call_count) + self.assertEqual(3, lock.acquire.call_count) + + +class SwitchBatchTest(fixtures.TestWithFixtures): + def setUp(self): + super(SwitchBatchTest, self).setUp() + self.cfg = self.useFixture(config_fixture.Config()) + + self.queue = mock.Mock() + self.switch_name = "switch1" + self.batch = batching.SwitchBatch( + self.switch_name, switch_queue=self.queue) + + @mock.patch.object(batching.SwitchBatch, "_spawn") + def test_do_batch(self, mock_spawn): + self.queue.add_batch.return_value = "item" + self.queue.wait_for_result.return_value = "output" + + result = self.batch.do_batch("device", ["cmd1"]) + + self.assertEqual("output", result) + self.assertEqual(1, mock_spawn.call_count) + self.queue.add_batch.assert_called_once_with(["cmd1"]) + self.queue.wait_for_result.assert_called_once_with("item", 300) + + def test_execute_pending_batches_skip(self): + self.queue.get_batches.return_value = [] + + result = self.batch._execute_pending_batches("device", "item") + + self.assertIsNone(result) + + def test_execute_pending_batches_skip2(self): + self.queue.get_batches.return_value = ["work"] + # Work was consumed by another worker before we could get the lock. + self.queue.acquire_worker_lock.return_value = None + + result = self.batch._execute_pending_batches("device", "item") + + self.assertIsNone(result) + + @mock.patch.object(batching.SwitchBatch, "_send_commands") + def test_execute_pending_batches_skip3(self, mock_send): + self.queue.get_batches.side_effect = [["work"], None] + # Work was consumed by another worker before we got the lock. + lock = mock.MagicMock() + self.queue.acquire_worker_lock.return_value = lock + + result = self.batch._execute_pending_batches("device", "item") + + self.assertIsNone(result) + self.assertEqual(0, mock_send.call_count) + + @mock.patch.object(batching.SwitchBatch, "_send_commands") + def test_execute_pending_batches_success(self, mock_send): + batches = [ + {"cmds": ["cmd1", "cmd2"]}, + {"cmds": ["cmd3", "cmd4"]}, + ] + self.queue.get_batches.return_value = batches + device = mock.MagicMock() + lock = mock.MagicMock() + self.queue.acquire_worker_lock.return_value = lock + + self.batch._execute_pending_batches(device, "item") + + mock_send.assert_called_once_with(device, batches, lock) + self.queue.acquire_worker_lock.assert_called_once_with("item") + lock.release.assert_called_once_with() + + @mock.patch.object(batching.SwitchBatch, "_send_commands") + def test_execute_pending_batches_failure(self, mock_send): + batches = [ + {"cmds": ["cmd1", "cmd2"]}, + {"cmds": ["cmd3", "cmd4"]}, + ] + self.queue.get_batches.return_value = batches + device = mock.MagicMock() + lock = mock.MagicMock() + self.queue.acquire_worker_lock.return_value = lock + mock_send.side_effect = exc.GenericSwitchBatchError + + self.assertRaises(exc.GenericSwitchBatchError, + self.batch._execute_pending_batches, + device, "item") + + lock.release.assert_called_once_with() + + def test_send_commands_one_batch(self): + device = mock.MagicMock() + device.send_config_set.return_value = "output" + batches = [ + {"cmds": ["cmd1", "cmd2"]}, + ] + lock = mock.MagicMock() + + self.batch._send_commands(device, batches, lock) + + connection = device._get_connection.return_value.__enter__.return_value + device.send_config_set.assert_called_once_with( + connection, ["cmd1", "cmd2"]) + lock.refresh.assert_called_once_with() + lock.is_acquired.assert_called_once_with() + self.queue.record_result.assert_called_once_with( + {"cmds": ["cmd1", "cmd2"], "result": "output"}) + device.save_configuration.assert_called_once_with(connection) + + def test_send_commands_two_batches(self): + device = mock.MagicMock() + device.send_config_set.side_effect = ["output1", "output2"] + batches = [ + {"cmds": ["cmd1", "cmd2"]}, + {"cmds": ["cmd3", "cmd4"]}, + ] + lock = mock.MagicMock() + + self.batch._send_commands(device, batches, lock) + + connection = device._get_connection.return_value.__enter__.return_value + self.assertEqual(2, device.send_config_set.call_count) + device.send_config_set.assert_has_calls([ + mock.call(connection, ["cmd1", "cmd2"]), + mock.call(connection, ["cmd3", "cmd4"]) + ]) + self.assertEqual(2, lock.refresh.call_count) + self.assertEqual(2, lock.is_acquired.call_count) + self.assertEqual(2, self.queue.record_result.call_count) + self.queue.record_result.assert_has_calls([ + mock.call({"cmds": ["cmd1", "cmd2"], "result": "output1"}), + mock.call({"cmds": ["cmd3", "cmd4"], "result": "output2"}) + ]) + device.save_configuration.assert_called_once_with(connection) + self.assertEqual(1, device.save_configuration.call_count) + + def test_send_commands_failure(self): + device = mock.MagicMock() + device.send_config_set.side_effect = Exception("Bang") + batches = [ + {"cmds": ["cmd1", "cmd2"]}, + ] + lock = mock.MagicMock() + + self.batch._send_commands(device, batches, lock) + + connection = device._get_connection.return_value.__enter__.return_value + device.send_config_set.assert_called_once_with( + connection, ["cmd1", "cmd2"]) + lock.refresh.assert_called_once_with() + lock.is_acquired.assert_called_once_with() + self.queue.record_result.assert_called_once_with( + {"cmds": ["cmd1", "cmd2"], "error": "Bang"}) + device.save_configuration.assert_called_once_with(connection) + + def test_send_commands_lock_timeout(self): + device = mock.MagicMock() + device.send_config_set.side_effect = Exception("Bang") + batches = [ + {"cmds": ["cmd1", "cmd2"]}, + ] + lock = mock.MagicMock() + lock.is_acquired.return_value = False + + self.assertRaises(exc.GenericSwitchBatchError, + self.batch._send_commands, device, batches, lock) + + connection = device._get_connection.return_value.__enter__.return_value + device.send_config_set.assert_called_once_with( + connection, ["cmd1", "cmd2"]) + self.assertEqual(0, self.queue.record_result.call_count) + self.assertEqual(0, device.save_configuration.call_count) diff --git a/releasenotes/notes/batching-12d9005924fd9d74.yaml b/releasenotes/notes/batching-12d9005924fd9d74.yaml new file mode 100644 index 00000000..2174fc47 --- /dev/null +++ b/releasenotes/notes/batching-12d9005924fd9d74.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Adds support for batching of requests using etcd as a task queue. diff --git a/requirements.txt b/requirements.txt index a0371ae8..c263fd2a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,8 @@ # The order of packages is significant, because pip processes them in the order # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. +etcd3gw>=0.2.4 # Apache-2.0 +eventlet>=0.18.2 # Apache-2.0 stevedore>=1.20.0 # Apache-2.0 netmiko>=4.1.1 # MIT neutron>=13.0.0.0b1 # Apache-2.0 From edc50ca0ef696b49f11752ca6c66f7d2402f59de Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Thu, 2 Mar 2023 16:23:13 +0000 Subject: [PATCH 09/10] fixup: batching adapt etcd3gw client creation for Yoga The api_path was only added to the client helper function in etcd3gw 2.1.0. This is available in upper constraints for Zed. Change-Id: Ide34499f64f8e8a92be80a132ece6090701733a9 (cherry picked from commit 871e8bb32790c7a8523df0877d3d68b372686114) --- networking_generic_switch/batching.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/networking_generic_switch/batching.py b/networking_generic_switch/batching.py index 50599cbe..df765a14 100644 --- a/networking_generic_switch/batching.py +++ b/networking_generic_switch/batching.py @@ -322,7 +322,7 @@ def __init__(self, switch_name, etcd_url=None, switch_queue=None): cert_key = params.get('cert_key') cert_cert = params.get('cert_cert') api_version = params.get('api_version', 'v3alpha') - etcd_client = etcd3gw.client( + etcd_client = etcd3gw.Etcd3Client( host=host, port=port, protocol=protocol, ca_cert=ca_cert, cert_key=cert_key, cert_cert=cert_cert, api_path='/' + api_version + '/', From deb3bc7c6f439302fd73ceeee8fb780934fa2319 Mon Sep 17 00:00:00 2001 From: Alex-Welsh Date: Mon, 17 Jul 2023 14:16:45 +0000 Subject: [PATCH 10/10] Fix backports --- networking_generic_switch/devices/__init__.py | 2 +- .../tests/unit/netmiko/test_dell.py | 45 ------------------- 2 files changed, 1 insertion(+), 46 deletions(-) diff --git a/networking_generic_switch/devices/__init__.py b/networking_generic_switch/devices/__init__.py index 4ee6da14..4425f56c 100644 --- a/networking_generic_switch/devices/__init__.py +++ b/networking_generic_switch/devices/__init__.py @@ -42,7 +42,7 @@ {'name': 'ngs_network_name_format', 'default': '{network_id}'}, # If false, ngs will not add and delete VLANs from switches {'name': 'ngs_manage_vlans', 'default': True}, - {'name': 'vlan_translation_supported', 'default': False} + {'name': 'vlan_translation_supported', 'default': False}, # If False, ngs will skip saving configuration on devices {'name': 'ngs_save_configuration', 'default': True}, # When true try to batch up in flight switch requests diff --git a/networking_generic_switch/tests/unit/netmiko/test_dell.py b/networking_generic_switch/tests/unit/netmiko/test_dell.py index 2cbbe791..efb5b6b5 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_dell.py +++ b/networking_generic_switch/tests/unit/netmiko/test_dell.py @@ -21,51 +21,6 @@ from networking_generic_switch.tests.unit.netmiko import test_netmiko_base -class TestNetmikoDellOS10(test_netmiko_base.NetmikoSwitchTestBase): - - def _make_switch_device(self, extra_cfg={}): - device_cfg = {'device_type': 'netmiko_dell_os10'} - device_cfg.update(extra_cfg) - return dell.DellOS10(device_cfg) - - def test_get_trunk_port_cmds_no_vlan_translation(self): - mock_context = mock.create_autospec(driver_context.PortContext) - self.switch.ngs_config['vlan_translation_supported'] = True - trunk_details = {'trunk_id': 'aaa-bbb-ccc-ddd', - 'sub_ports': [{'segmentation_id': 130, - 'port_id': 'aaa-bbb-ccc-ddd', - 'segmentation_type': 'vlan', - 'mac_address': u'fa:16:3e:1c:c2:7e'}]} - mock_context.current = {'binding:profile': - {'local_link_information': - [ - { - 'switch_info': 'foo', - 'port_id': '2222' - } - ] - }, - 'binding:vnic_type': 'baremetal', - 'id': 'aaaa-bbbb-cccc', - 'trunk_details': trunk_details} - mock_context.network = mock.Mock() - mock_context.network.current = {'provider:segmentation_id': 123} - mock_context.segments_to_bind = [ - { - 'segmentation_id': 777, - 'id': 123 - } - ] - res = self.switch.get_trunk_port_cmds_no_vlan_translation( - '2222', 777, trunk_details) - self.assertEqual(['interface 2222', 'switchport mode access', - 'switchport mode trunk', - 'switchport access vlan 777', - 'interface 2222', - 'switchport trunk allowed vlan 130'], - res) - - class TestNetmikoDellNos(test_netmiko_base.NetmikoSwitchTestBase): def _make_switch_device(self, extra_cfg={}):