From f733c9e94c4245a439ae6387683f412397746951 Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Mon, 23 Nov 2020 15:10:10 +0000 Subject: [PATCH] Add ngs_manage_vlans configuration This allows operators to opt out of NGS adding or removing VLANs from switches. This is useful when not all switch configuration is being managed by Networking Generic Switch. A similar concenpt is already supported by ansible-networking: https://opendev.org/x/networking-ansible/src/commit/6f56fd45425141e5435884d2d0cf88ee96a887c6/etc/neutron/plugins/ml2/ml2_conf_ansible.ini.sample Co-Authored-By: Michal Nasiadka Change-Id: I271263a349d6cccd0fe9980e10b1b8d5fd86716e --- doc/source/configuration.rst | 22 +++++++++++++++++++ networking_generic_switch/devices/__init__.py | 6 +++++ .../devices/netmiko_devices/__init__.py | 8 +++++++ .../tests/unit/netmiko/test_netmiko_base.py | 22 +++++++++++++++++++ .../notes/manage-vlans-c75e4c2e9b9b3403.yaml | 6 +++++ 5 files changed, 64 insertions(+) create mode 100644 releasenotes/notes/manage-vlans-c75e4c2e9b9b3403.yaml diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index 31c5ca4f..a452ec10 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -244,3 +244,25 @@ For example:: Some switches have issues assigning VLANs a name that starts with a number, and this configuration option can be used to avoid this. + +Manage VLANs +============ + +By default, on network creation VLANs are added to all switches. In a similar +way, VLANs are removed when it seems they are no longer required. +However, in some cases only a subset of the ports are managed by Neutron. +In a similar way, when multiple switches are used, it is very common that +the network administrator restricts the VLANs allowed. In these cases, there +is little utility in adding and removing vlans on the switches. This process +takes time, so not doing this can speed up a number of common operations. +A particular case where this can cause problems is when a VLAN used for +the switch management interface, or any other port not managed by Neutron, +is removed by this Neutron driver. + +To stop networking generic switch trying to add or remove VLANs on the switch +administrator are expected to pre-add all enabled VLANs. Once those VLANs are +preconfigured on the switch, you can use the following configuration to stop +networking generic switch adding or removing any VLANs:: + + [genericswitch:device-hostname] + ngs_manage_vlans = False diff --git a/networking_generic_switch/devices/__init__.py b/networking_generic_switch/devices/__init__.py index 66a7ab4f..1feb79cb 100644 --- a/networking_generic_switch/devices/__init__.py +++ b/networking_generic_switch/devices/__init__.py @@ -40,6 +40,8 @@ # String format for network name to configure on switches. # Accepts {network_id} and {segmentation_id} formatting options. {'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}, ] @@ -130,6 +132,10 @@ def _get_network_name(self, network_id, segmentation_id): return network_name_format.format(network_id=network_id, segmentation_id=segmentation_id) + 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']) + @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 b80cda10..00843730 100644 --- a/networking_generic_switch/devices/netmiko_devices/__init__.py +++ b/networking_generic_switch/devices/netmiko_devices/__init__.py @@ -200,6 +200,10 @@ def send_commands_to_device(self, cmd_set): @check_output('add network') def add_network(self, segmentation_id, network_id): + if not self._do_vlan_management(): + LOG.info(f"Skipping add network for {segmentation_id}") + return "" + # NOTE(zhenguo): Remove dashes from uuid as on most devices 32 chars # is the max length of vlan name. network_id = uuid.UUID(network_id).hex @@ -218,6 +222,10 @@ def add_network(self, segmentation_id, network_id): @check_output('delete network') def del_network(self, segmentation_id, network_id): + if not self._do_vlan_management(): + LOG.info(f"Skipping delete network for {segmentation_id}") + return "" + # NOTE(zhenguo): Remove dashes from uuid as on most devices 32 chars # is the max length of vlan name. network_id = uuid.UUID(network_id).hex 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 27d62921..33aa281c 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py +++ b/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py @@ -67,6 +67,17 @@ def test_add_network_with_trunk_ports(self, m_check, m_sctd): m_sctd.assert_called_with([]) m_check.assert_called_once_with('fake output', 'add network') + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', + return_value='fake output') + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.check_output') + def test_add_network_with_no_manage_vlans(self, m_check, m_sctd): + switch = self._make_switch_device({'ngs_manage_vlans': False}) + switch.add_network(22, '0ae071f5-5be9-43e4-80ea-e41fefe85b21') + self.assertFalse(m_sctd.called) + m_check.assert_called_once_with('', 'add network') + @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.send_commands_to_device', return_value='fake output') @@ -88,6 +99,17 @@ def test_del_network_with_trunk_ports(self, m_check, m_sctd): m_sctd.assert_called_with([]) m_check.assert_called_once_with('fake output', 'delete network') + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', + return_value='fake output') + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.check_output') + def test_del_network_with_no_manage_vlans(self, m_check, m_sctd): + switch = self._make_switch_device({'ngs_manage_vlans': False}) + switch.del_network(22, '0ae071f5-5be9-43e4-80ea-e41fefe85b21') + self.assertFalse(m_sctd.called) + m_check.assert_called_once_with('', 'delete network') + @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.send_commands_to_device', return_value='fake output') diff --git a/releasenotes/notes/manage-vlans-c75e4c2e9b9b3403.yaml b/releasenotes/notes/manage-vlans-c75e4c2e9b9b3403.yaml new file mode 100644 index 00000000..51cc4b2c --- /dev/null +++ b/releasenotes/notes/manage-vlans-c75e4c2e9b9b3403.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + New per switch `ngs_manage_vlans` option. It defaults to True, but can + be set to False so that no VLANs are added or removed on that switch. + This is useful when not all ports on the switch are managed by Neutron.