From fc5e92db5cb543785ed4f8d2d3952d252b912280 Mon Sep 17 00:00:00 2001 From: Michael Sherman Date: Mon, 10 Feb 2025 17:44:56 -0600 Subject: [PATCH 1/5] Fix binding l2-only ports by setting connectivity property As NGS inherits from neutron-lib's api.MechanismDriver, `connectivity` must be overridden, or it will return `portbindings.CONNECTIVITY_LEGACY`, rather than what we want, `CONNECTIVITY_L2`. Fixes-Bug: #2097803 Change-Id: I0c9ba7942d6e7c01f6ca77a49ae8fe447db30baf --- networking_generic_switch/generic_switch_mech.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/networking_generic_switch/generic_switch_mech.py b/networking_generic_switch/generic_switch_mech.py index 963a1c38..be08c0e2 100644 --- a/networking_generic_switch/generic_switch_mech.py +++ b/networking_generic_switch/generic_switch_mech.py @@ -32,6 +32,10 @@ class GenericSwitchDriver(api.MechanismDriver): + @property + def connectivity(self): + return portbindings.CONNECTIVITY_L2 + def initialize(self): """Perform driver initialization. From 2734a59f54af35332daa867c6b479276b2674be5 Mon Sep 17 00:00:00 2001 From: cid Date: Sun, 9 Mar 2025 09:08:40 +0100 Subject: [PATCH 2/5] Configuration error handling for non-Layer-2 ports Detect and raise port configuration failures when attempting to configure a port on a Dell Force10 switch that's not in Layer-2 mode, preventing incorrect port binding. Closes-Bug: #2100641 Change-Id: I9e86d1bb5004c9a2e8a984e54edc15e96bb52310 --- networking_generic_switch/devices/netmiko_devices/dell.py | 1 + 1 file changed, 1 insertion(+) diff --git a/networking_generic_switch/devices/netmiko_devices/dell.py b/networking_generic_switch/devices/netmiko_devices/dell.py index a5c824df..4f97bc43 100644 --- a/networking_generic_switch/devices/netmiko_devices/dell.py +++ b/networking_generic_switch/devices/netmiko_devices/dell.py @@ -198,4 +198,5 @@ def on_invalid_switchmode(): re.compile(r'VLAN was not created by user'), re.compile(r'Configuration Database locked by another application \- ' r'try later'), + re.compile(r'Port is not in Layer-2 mode'), ) From ff0085a03af2e18535d395aadcd0262a0731ac12 Mon Sep 17 00:00:00 2001 From: Vasyl Saienko Date: Sun, 2 Feb 2025 15:51:11 +0000 Subject: [PATCH 3/5] Add vlan aware VMs support With this patch ngs starts supporting attaching of trunk port to baremetal server. Only VLAN Neutron network is supported. Closes-Bug: #1653968 Co-Authored-By: Vasyl Saienko Co-Authored-By: Will Szumski Co-Authored-By: Mark Goddard Co-Authored-By: Seunghun Lee Depends-On: https://review.opendev.org/c/openstack/ironic/+/941023 Change-Id: I978cb6b1ea8c049b40aaf1b305d0d0f033282299 --- devstack/plugin.sh | 1 + networking_generic_switch/devices/__init__.py | 81 +++- .../devices/netmiko_devices/__init__.py | 143 ++++++- .../devices/netmiko_devices/arista.py | 23 + .../devices/netmiko_devices/cisco.py | 25 ++ .../devices/netmiko_devices/ovs.py | 40 ++ networking_generic_switch/exceptions.py | 5 + .../generic_switch_mech.py | 131 +++++- .../tests/unit/netmiko/test_arista_eos.py | 165 +++++++ .../tests/unit/netmiko/test_cisco_ios.py | 173 ++++++++ .../tests/unit/netmiko/test_netmiko_base.py | 9 +- .../tests/unit/netmiko/test_ovs_linux.py | 151 +++++++ .../tests/unit/test_generic_switch_mech.py | 404 +++++++++++++----- networking_generic_switch/trunk_driver.py | 109 +++++ networking_generic_switch/utils.py | 27 ++ .../vlan-aware-vms-3923cc17254829e9.yaml | 7 + 16 files changed, 1344 insertions(+), 150 deletions(-) create mode 100644 networking_generic_switch/trunk_driver.py create mode 100644 networking_generic_switch/utils.py create mode 100644 releasenotes/notes/vlan-aware-vms-3923cc17254829e9.yaml diff --git a/devstack/plugin.sh b/devstack/plugin.sh index f6db394c..d4c91b25 100644 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -220,6 +220,7 @@ function ngs_configure_tempest { if [ $GENERIC_SWITCH_USER_MAX_SESSIONS -gt 0 ]; then iniset $TEMPEST_CONFIG ngs port_dlm_concurrency $(($GENERIC_SWITCH_USER_MAX_SESSIONS * 2)) fi + iniset $TEMPEST_CONFIG baremetal_feature_enabled trunks_supported True } # check for service enabled diff --git a/networking_generic_switch/devices/__init__.py b/networking_generic_switch/devices/__init__.py index a3ae8b55..0b6f2cfb 100644 --- a/networking_generic_switch/devices/__init__.py +++ b/networking_generic_switch/devices/__init__.py @@ -112,6 +112,14 @@ def __init__(self, device_cfg, device_name=""): self._validate_network_name_format() + @property + def support_trunk_on_ports(self): + return False + + @property + def support_trunk_on_bond_ports(self): + return False + def _validate_network_name_format(self): """Validate the network name format configuration option.""" network_name_format = self.ngs_config['ngs_network_name_format'] @@ -223,17 +231,78 @@ 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): + """Plug port into network. + + :param port_id: Then name of the switch interface + :param segmentation_id: VLAN identifier of the network used as access + or native VLAN for port. + + :param trunk_details: trunk information if port is a part of trunk + """ pass @abc.abstractmethod - def delete_port(self, port_id, segmentation_id): + def delete_port(self, port_id, segmentation_id, trunk_details=None): + """Delete port from specific network. + + :param port_id: Then name of the switch interface + :param segmentation_id: VLAN identifier of the network used as access + or native VLAN for port. + + :param trunk_details: trunk information if port is a part of trunk + """ pass - def plug_bond_to_network(self, bond_id, segmentation_id): + def plug_bond_to_network(self, bond_id, segmentation_id, + trunk_details=None): + """Plug bond port into network. + + :param port_id: Then name of the switch interface + :param segmentation_id: VLAN identifier of the network used as access + or native VLAN for port. + + :param trunk_details: trunk information if port is a part of trunk + """ + kwargs = {} + if trunk_details: + kwargs["trunk_details"] = trunk_details # Fall back to interface method. - return self.plug_port_to_network(bond_id, segmentation_id) + return self.plug_port_to_network(bond_id, segmentation_id, **kwargs) - def unplug_bond_from_network(self, bond_id, segmentation_id): + def unplug_bond_from_network(self, bond_id, segmentation_id, + trunk_details=None): + """Unplug bond port from network. + + :param port_id: Then name of the switch interface + :param segmentation_id: VLAN identifier of the network used as access + or native VLAN for port. + + :param trunk_details: trunk information if port is a part of trunk + """ + kwargs = {} + if trunk_details: + kwargs["trunk_details"] = trunk_details # Fall back to interface method. - return self.delete_port(bond_id, segmentation_id) + return self.delete_port(bond_id, segmentation_id, **kwargs) + + def add_subports_on_trunk(self, binding_profile, port_id, subports): + """Allow subports on trunk + + :param binding_profile: Binding profile of parent port + :param port_id: The name of the switch port from + Local Link Information + :param subports: List with subports objects. + """ + pass + + def del_subports_on_trunk(self, binding_profile, port_id, subports): + """Allow subports on trunk + + :param binding_profile: Binding profile of parent port + :param port_id: The name of the switch port from + Local Link Information + :param subports: List with subports objects. + """ + pass diff --git a/networking_generic_switch/devices/netmiko_devices/__init__.py b/networking_generic_switch/devices/netmiko_devices/__init__.py index a028b293..211ed2ba 100644 --- a/networking_generic_switch/devices/netmiko_devices/__init__.py +++ b/networking_generic_switch/devices/netmiko_devices/__init__.py @@ -29,6 +29,7 @@ from networking_generic_switch.devices import utils as device_utils from networking_generic_switch import exceptions as exc from networking_generic_switch import locking as ngs_lock +from networking_generic_switch import utils as ngs_utils LOG = logging.getLogger(__name__) CONF = cfg.CONF @@ -90,6 +91,22 @@ class NetmikoSwitch(devices.GenericSwitchDevice): SAVE_CONFIGURATION = None + SET_NATIVE_VLAN = None + + DELETE_NATIVE_VLAN = None + + SET_NATIVE_VLAN_BOND = None + + DELETE_NATIVE_VLAN_BOND = None + + ADD_NETWORK_TO_TRUNK = None + + REMOVE_NETWORK_FROM_TRUNK = None + + ADD_NETWORK_TO_BOND_TRUNK = None + + DELETE_NETWORK_ON_BOND_TRUNK = None + ERROR_MSG_PATTERNS = () """Sequence of error message patterns. @@ -145,6 +162,14 @@ def __init__(self, device_cfg, *args, **kwargs): self.locker.start() atexit.register(self.locker.stop) + @property + def support_trunk_on_ports(self): + return bool(self.ADD_NETWORK_TO_TRUNK) + + @property + def support_trunk_on_bond_ports(self): + return bool(self.ADD_NETWORK_TO_BOND_TRUNK) + def _format_commands(self, commands, **kwargs): if not commands: return [] @@ -277,7 +302,7 @@ def del_network(self, segmentation_id, network_id): return self.send_commands_to_device(cmds) @check_output('plug port') - def plug_port_to_network(self, port, segmentation_id): + def plug_port_to_network(self, port, segmentation_id, trunk_details=None): cmds = [] if self._disable_inactive_ports() and self.ENABLE_PORT: cmds += self._format_commands(self.ENABLE_PORT, port=port) @@ -287,18 +312,38 @@ def plug_port_to_network(self, port, segmentation_id): self.DELETE_PORT, port=port, segmentation_id=ngs_port_default_vlan) - cmds += self._format_commands( - self.PLUG_PORT_TO_NETWORK, - port=port, - segmentation_id=segmentation_id) + + if trunk_details: + cmds += self._format_commands(self.SET_NATIVE_VLAN, + port=port, + segmentation_id=segmentation_id) + for sub_port in trunk_details.get('sub_ports', []): + cmds += self._format_commands( + self.ADD_NETWORK_TO_TRUNK, port=port, + segmentation_id=sub_port['segmentation_id']) + else: + cmds += self._format_commands( + self.PLUG_PORT_TO_NETWORK, + port=port, + segmentation_id=segmentation_id) + return self.send_commands_to_device(cmds) @check_output('unplug port') - def delete_port(self, port, segmentation_id): + def delete_port(self, port, segmentation_id, trunk_details=None): cmds = self._format_commands(self.DELETE_PORT, port=port, segmentation_id=segmentation_id) ngs_port_default_vlan = self._get_port_default_vlan() + if trunk_details: + cmds += self._format_commands(self.DELETE_NATIVE_VLAN, + port=port, + segmentation_id=segmentation_id) + for sub_port in trunk_details.get('sub_ports', []): + cmds += self._format_commands( + self.REMOVE_NETWORK_FROM_TRUNK, port=port, + segmentation_id=sub_port['segmentation_id']) + if ngs_port_default_vlan: # NOTE(mgoddard): Pass network_id and segmentation_id for drivers # not yet using network_name. @@ -315,14 +360,16 @@ def delete_port(self, port, segmentation_id): segmentation_id=ngs_port_default_vlan) if self._disable_inactive_ports() and self.DISABLE_PORT: cmds += self._format_commands(self.DISABLE_PORT, port=port) + return self.send_commands_to_device(cmds) @check_output('plug bond') - def plug_bond_to_network(self, bond, segmentation_id): + def plug_bond_to_network(self, bond, segmentation_id, trunk_details=None): # Fallback to regular plug port if no specialist PLUG_BOND_TO_NETWORK # commands set if not self.PLUG_BOND_TO_NETWORK: - return self.plug_port_to_network(bond, segmentation_id) + return self.plug_port_to_network(bond, segmentation_id, + trunk_details=trunk_details) cmds = [] if self._disable_inactive_ports() and self.ENABLE_BOND: cmds += self._format_commands(self.ENABLE_BOND, bond=bond) @@ -332,22 +379,44 @@ def plug_bond_to_network(self, bond, segmentation_id): self.UNPLUG_BOND_FROM_NETWORK, bond=bond, segmentation_id=ngs_port_default_vlan) - cmds += self._format_commands( - self.PLUG_BOND_TO_NETWORK, - bond=bond, - segmentation_id=segmentation_id) + + if trunk_details: + cmds += self._format_commands(self.SET_NATIVE_VLAN_BOND, + bond=bond, + segmentation_id=segmentation_id) + for sub_port in trunk_details.get('sub_ports', []): + cmds += self._format_commands( + self.ADD_NETWORK_TO_BOND_TRUNK, bond=bond, + segmentation_id=sub_port['segmentation_id']) + else: + cmds += self._format_commands( + self.PLUG_BOND_TO_NETWORK, + bond=bond, + segmentation_id=segmentation_id) + return self.send_commands_to_device(cmds) @check_output('unplug bond') - def unplug_bond_from_network(self, bond, segmentation_id): + def unplug_bond_from_network(self, bond, segmentation_id, + trunk_details=None): # Fallback to regular port delete if no specialist # UNPLUG_BOND_FROM_NETWORK commands set if not self.UNPLUG_BOND_FROM_NETWORK: - return self.delete_port(bond, segmentation_id) + return self.delete_port(bond, segmentation_id, + trunk_details=trunk_details) cmds = self._format_commands(self.UNPLUG_BOND_FROM_NETWORK, bond=bond, segmentation_id=segmentation_id) ngs_port_default_vlan = self._get_port_default_vlan() + if trunk_details: + cmds += self._format_commands(self.DELETE_NATIVE_VLAN_BOND, + bond=bond, + segmentation_id=segmentation_id) + for sub_port in trunk_details.get('sub_ports', []): + cmds += self._format_commands( + self.ADD_NETWORK_TO_BOND_TRUNK, bond=bond, + segmentation_id=sub_port['segmentation_id']) + if ngs_port_default_vlan: # NOTE(mgoddard): Pass network_id and segmentation_id for drivers # not yet using network_name. @@ -364,6 +433,7 @@ def unplug_bond_from_network(self, bond, segmentation_id): segmentation_id=ngs_port_default_vlan) if self._disable_inactive_ports() and self.DISABLE_BOND: cmds += self._format_commands(self.DISABLE_BOND, bond=bond) + return self.send_commands_to_device(cmds) def send_config_set(self, net_connect, cmd_set): @@ -417,3 +487,48 @@ def check_output(self, output, operation): raise exc.GenericSwitchNetmikoConfigError( config=device_utils.sanitise_config(self.config), error=msg) + + def add_subports_on_trunk(self, binding_profile, port_id, subports): + """Allow subports on trunk + + :param binding_profile: Binding profile of parent port + :param port_id: The name of the switch port from + Local Link Information + :param subports: List with subports objects. + """ + cmds = [] + is_802_3ad = ngs_utils.is_802_3ad(binding_profile) + + for sub_port in subports: + if is_802_3ad: + cmds += self._format_commands( + self.ADD_NETWORK_TO_BOND_TRUNK, bond=port_id, + segmentation_id=sub_port['segmentation_id']) + else: + cmds += self._format_commands( + self.ADD_NETWORK_TO_TRUNK, port=port_id, + segmentation_id=sub_port['segmentation_id']) + return self.send_commands_to_device(cmds) + + def del_subports_on_trunk(self, binding_profile, port_id, subports): + """Allow subports on trunk + + :param binding_profile: Binding profile of parent port + :param port_id: The name of the switch port from + Local Link Information + :param subports: List with subports objects. + """ + + cmds = [] + is_802_3ad = ngs_utils.is_802_3ad(binding_profile) + + for sub_port in subports: + if is_802_3ad: + cmds += self._format_commands( + self.DELETE_NETWORK_ON_BOND_TRUNK, bond=port_id, + segmentation_id=sub_port['segmentation_id']) + else: + cmds += self._format_commands( + self.REMOVE_NETWORK_FROM_TRUNK, port=port_id, + segmentation_id=sub_port['segmentation_id']) + return self.send_commands_to_device(cmds) diff --git a/networking_generic_switch/devices/netmiko_devices/arista.py b/networking_generic_switch/devices/netmiko_devices/arista.py index a9500ac9..42c1e110 100644 --- a/networking_generic_switch/devices/netmiko_devices/arista.py +++ b/networking_generic_switch/devices/netmiko_devices/arista.py @@ -37,3 +37,26 @@ 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}' + ) + + DELETE_NATIVE_VLAN = ( + 'interface {port}', + 'no switchport trunk native vlan {segmentation_id}', + 'switchport trunk allowed vlan remove {segmentation_id}', + ) + + ADD_NETWORK_TO_TRUNK = ( + 'interface {port}', + 'switchport trunk allowed vlan add {segmentation_id}' + ) + + REMOVE_NETWORK_FROM_TRUNK = ( + 'interface {port}', + 'switchport trunk allowed vlan remove {segmentation_id}' + ) diff --git a/networking_generic_switch/devices/netmiko_devices/cisco.py b/networking_generic_switch/devices/netmiko_devices/cisco.py index 4b919763..2d87700c 100644 --- a/networking_generic_switch/devices/netmiko_devices/cisco.py +++ b/networking_generic_switch/devices/netmiko_devices/cisco.py @@ -39,6 +39,31 @@ class CiscoIos(netmiko_devices.NetmikoSwitch): '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}', + ) + + DELETE_NATIVE_VLAN = ( + 'interface {port}', + 'no switchport mode trunk', + 'no switchport trunk native vlan {segmentation_id}', + 'switchport trunk allowed vlan remove {segmentation_id}', + ) + + ADD_NETWORK_TO_TRUNK = ( + 'interface {port}', + 'switchport mode trunk', + 'switchport trunk allowed vlan add {segmentation_id}', + ) + + REMOVE_NETWORK_FROM_TRUNK = ( + 'interface {port}', + 'switchport trunk allowed vlan remove {segmentation_id}', + ) + class CiscoNxOS(netmiko_devices.NetmikoSwitch): """Netmiko device driver for Cisco Nexus switches running NX-OS.""" diff --git a/networking_generic_switch/devices/netmiko_devices/ovs.py b/networking_generic_switch/devices/netmiko_devices/ovs.py index 7409b3a7..ae902855 100644 --- a/networking_generic_switch/devices/netmiko_devices/ovs.py +++ b/networking_generic_switch/devices/netmiko_devices/ovs.py @@ -27,3 +27,43 @@ class OvsLinux(netmiko_devices.NetmikoSwitch): 'ovs-vsctl clear port {port} trunks', 'ovs-vsctl clear port {port} vlan_mode' ) + + SET_NATIVE_VLAN = ( + 'ovs-vsctl set port {port} vlan_mode=native-untagged', + 'ovs-vsctl set port {port} tag={segmentation_id}', + 'ovs-vsctl add port {port} trunks {segmentation_id}', + ) + + DELETE_NATIVE_VLAN = ( + 'ovs-vsctl clear port {port} vlan_mode', + 'ovs-vsctl clear port {port} tag', + 'ovs-vsctl remove port {port} trunks {segmentation_id}', + ) + + SET_NATIVE_VLAN_BOND = ( + 'ovs-vsctl set port {bond} vlan_mode=native-untagged', + 'ovs-vsctl set port {bond} tag={segmentation_id}', + 'ovs-vsctl add port {bond} trunks {segmentation_id}', + ) + + DELETE_NATIVE_VLAN_BOND = ( + 'ovs-vsctl clear port {bond} vlan_mode', + 'ovs-vsctl clear port {bond} tag', + 'ovs-vsctl remove port {bond} trunks {segmentation_id}', + ) + + ADD_NETWORK_TO_TRUNK = ( + 'ovs-vsctl add port {port} trunks {segmentation_id}', + ) + + REMOVE_NETWORK_FROM_TRUNK = ( + 'ovs-vsctl remove port {port} trunks {segmentation_id}', + ) + + ADD_NETWORK_TO_BOND_TRUNK = ( + 'ovs-vsctl add port {bond} trunks {segmentation_id}', + ) + + DELETE_NETWORK_ON_BOND_TRUNK = ( + 'ovs-vsctl remove port {bond} trunks {segmentation_id}', + ) diff --git a/networking_generic_switch/exceptions.py b/networking_generic_switch/exceptions.py index 96b57e44..de68e621 100644 --- a/networking_generic_switch/exceptions.py +++ b/networking_generic_switch/exceptions.py @@ -53,3 +53,8 @@ class GenericSwitchNetmikoConfigError(GenericSwitchException): class GenericSwitchBatchError(GenericSwitchException): message = _("Batching error: %(device)s, error: %(error)s") + + +class GenericSwitchNotSupported(GenericSwitchException): + message = _("Requested feature %(feature)s is not supported by " + "networking-generic-switch on the %(switch)s. %(error)s") diff --git a/networking_generic_switch/generic_switch_mech.py b/networking_generic_switch/generic_switch_mech.py index 753ab70f..260bd191 100644 --- a/networking_generic_switch/generic_switch_mech.py +++ b/networking_generic_switch/generic_switch_mech.py @@ -18,12 +18,16 @@ from neutron_lib.api.definitions import portbindings from neutron_lib.callbacks import resources from neutron_lib import constants as const +from neutron_lib.plugins import directory from neutron_lib.plugins.ml2 import api 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 +from networking_generic_switch import trunk_driver +from networking_generic_switch import utils as ngs_utils LOG = logging.getLogger(__name__) @@ -53,6 +57,8 @@ def initialize(self): if not self.switches: LOG.error('No devices have been loaded') + self.trunk_driver = trunk_driver.GenericSwitchTrunkDriver.create(self) + def create_network_precommit(self, context): """Allocate resources for a new network. @@ -360,7 +366,7 @@ def update_port_postcommit(self, context): # at this point, but check just in case. if not self._is_link_valid(port, segment): return - is_802_3ad = self._is_802_3ad(port) + is_802_3ad = ngs_utils.is_802_3ad(binding_profile) for link in local_link_information: port_id = link.get('port_id') switch_info = link.get('switch_info') @@ -375,11 +381,27 @@ def update_port_postcommit(self, context): "%(switch_info)s in vlan %(segmentation_id)s", {'switch_port': port_id, 'switch_info': switch_info, 'segmentation_id': segmentation_id}) + trunk_details = port.get('trunk_details', {}) + plug_kwargs = {} + if trunk_details: + plug_kwargs["trunk_details"] = trunk_details # Move port to network if is_802_3ad: - switch.plug_bond_to_network(port_id, segmentation_id) + if (trunk_details and not + switch.support_trunk_on_bond_ports): + raise ngs_exc.GenericSwitchNotSupported( + "Trunks are not supported by " + "networking-generic-switch %s.", + switch.device_name) + switch.plug_bond_to_network(port_id, segmentation_id, + **plug_kwargs) else: - switch.plug_port_to_network(port_id, segmentation_id) + if trunk_details and not switch.support_trunk_on_ports: + raise ngs_exc.GenericSwitchNotSupported( + feature="trunks", + switch=switch.device_name) + switch.plug_port_to_network(port_id, segmentation_id, + **plug_kwargs) LOG.info("Successfully plugged port %(port_id)s in segment " "%(segment_id)s on device %(device)s", {'port_id': port['id'], 'device': switch_info, @@ -388,6 +410,13 @@ def update_port_postcommit(self, context): provisioning_blocks.provisioning_complete( context._plugin_context, port['id'], resources.PORT, GENERIC_SWITCH_ENTITY) + for subport in port.get('trunk_details', {}).get("sub_ports", []): + subport_obj = context._plugin.get_port(context.plugin_context, + subport['port_id']) + if subport_obj['status'] != const.PORT_STATUS_ACTIVE: + context._plugin.update_port_status( + context.plugin_context, subport["port_id"], + const.PORT_STATUS_ACTIVE) elif self._is_port_bound(context.original): # The port has been unbound. This will cause the local link # information to be lost, so remove the port from the segment on @@ -588,21 +617,6 @@ def _is_port_bound(port): vif_type = port[portbindings.VIF_TYPE] return vif_type == portbindings.VIF_TYPE_OTHER - @staticmethod - def _is_802_3ad(port): - """Return whether a port is using 802.3ad link aggregation. - - :param port: The port to check - :returns: Whether the port is a port group using 802.3ad link - aggregation. - """ - binding_profile = port['binding:profile'] - local_group_information = binding_profile.get( - 'local_group_information') - if not local_group_information: - return False - return local_group_information.get('bond_mode') in ['4', '802.3ad'] - def _unplug_port_from_segment(self, port, segment): """Unplug a port from a segment. @@ -618,7 +632,7 @@ def _unplug_port_from_segment(self, port, segment): if not local_link_information: return - is_802_3ad = self._is_802_3ad(port) + is_802_3ad = ngs_utils.is_802_3ad(binding_profile) for link in local_link_information: switch_info = link.get('switch_info') switch_id = link.get('switch_id') @@ -665,3 +679,82 @@ def _get_devices_by_physnet(self, physnet): # follow the old behaviour of mapping all networks to it. if not physnets or physnet in physnets: yield switch_name, switch + + def subports_added(self, context, port, subports): + """Tell the agent about new subports to add. + + :param context: Request context + :param port: Port dictionary + :subports: List with subports + """ + + # set the correct state on port in the case where it has subports. + # If the parent port has been deleted then that delete will handle + # removing the trunked vlans on the switch using the mac + if not port: + LOG.debug('Discarding attempt to ensure subports on a port' + 'that has been deleted') + return + + if not self._is_port_supported(port): + return + + binding_profile = port['binding:profile'] + local_link_information = binding_profile.get('local_link_information') + + if not local_link_information: + return + + for link in local_link_information: + port_id = link.get('port_id') + switch_info = link.get('switch_info') + switch_id = link.get('switch_id') + switch = device_utils.get_switch_device( + self.switches, switch_info=switch_info, + ngs_mac_address=switch_id) + + switch.add_subports_on_trunk( + binding_profile, port_id, subports) + + core_plugin = directory.get_plugin() + + for subport in subports: + subport_obj = core_plugin.get_port(context, + subport['port_id']) + if subport_obj['status'] != const.PORT_STATUS_ACTIVE: + core_plugin.update_port_status( + context, subport["port_id"], + const.PORT_STATUS_ACTIVE) + + def subports_deleted(self, context, port, subports): + """Tell the agent about subports to delete. + + :param context: Request context + :param port: Port dictionary + :subports: List with subports + """ + + if not port: + LOG.debug('Discarding attempt to ensure subports on a port' + 'that has been deleted') + return + + if not self._is_port_supported(port): + return + + binding_profile = port['binding:profile'] + local_link_information = binding_profile.get('local_link_information') + + if not local_link_information: + return + + for link in local_link_information: + port_id = link.get('port_id') + switch_info = link.get('switch_info') + switch_id = link.get('switch_id') + switch = device_utils.get_switch_device( + self.switches, switch_info=switch_info, + ngs_mac_address=switch_id) + + switch.del_subports_on_trunk( + binding_profile, port_id, subports) 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 c713f739..86699e92 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 oslo_utils import uuidutils + from networking_generic_switch.devices.netmiko_devices import arista from networking_generic_switch.tests.unit.netmiko import test_netmiko_base @@ -89,3 +91,166 @@ def test__format_commands(self): 'no switchport mode trunk', 'switchport trunk allowed vlan none'] self.assertEqual(del_exp, cmd_set) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_plug_port_to_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.plug_port_to_network(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', + 'switchport mode trunk', + 'switchport trunk native vlan 44', + 'switchport trunk allowed vlan add 44', + 'interface 4444', + 'switchport trunk allowed vlan add tag1', + 'interface 4444', + 'switchport trunk allowed vlan add tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_delete_port_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.delete_port(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', + 'no switchport access vlan 44', + 'no switchport mode trunk', + 'switchport trunk allowed vlan none', + 'interface 4444', + 'no switchport trunk native vlan 44', + 'switchport trunk allowed vlan remove 44', + 'interface 4444', + 'switchport trunk allowed vlan remove tag1', + 'interface 4444', + 'switchport trunk allowed vlan remove tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_plug_bond_to_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.plug_bond_to_network(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', + 'switchport mode trunk', + 'switchport trunk native vlan 44', + 'switchport trunk allowed vlan add 44', + 'interface 4444', + 'switchport trunk allowed vlan add tag1', + 'interface 4444', + 'switchport trunk allowed vlan add tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_unplug_bond_from_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.unplug_bond_from_network( + 4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', + 'no switchport access vlan 44', + 'no switchport mode trunk', + 'switchport trunk allowed vlan none', + 'interface 4444', + 'no switchport trunk native vlan 44', + 'switchport trunk allowed vlan remove 44', + 'interface 4444', + 'switchport trunk allowed vlan remove tag1', + 'interface 4444', + 'switchport trunk allowed vlan remove tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_add_subports_on_trunk_no_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [] + self.switch.add_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, []) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_add_subports_on_trunk_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + self.switch.add_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with( + self.switch, + ['interface 44', + 'switchport trunk allowed vlan add tag1', + 'interface 44', + 'switchport trunk allowed vlan add tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_del_subports_on_trunk_no_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [] + self.switch.del_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, []) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_del_subports_on_trunk_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + self.switch.del_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with( + self.switch, + ['interface 44', + 'switchport trunk allowed vlan remove tag1', + 'interface 44', + 'switchport trunk allowed vlan remove tag2']) 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 f52397e2..82a9aa21 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 oslo_utils import uuidutils + from networking_generic_switch.devices.netmiko_devices import cisco from networking_generic_switch.tests.unit.netmiko import test_netmiko_base @@ -89,3 +91,174 @@ def test__format_commands(self): 'no switchport mode trunk', 'switchport trunk allowed vlan none'] self.assertEqual(del_exp, cmd_set) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_plug_port_to_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.plug_port_to_network(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', + 'switchport mode trunk', + 'switchport trunk native vlan 44', + 'switchport trunk allowed vlan add 44', + 'interface 4444', + 'switchport mode trunk', + 'switchport trunk allowed vlan add tag1', + 'interface 4444', + 'switchport mode trunk', + 'switchport trunk allowed vlan add tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_delete_port_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.delete_port(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', + 'no switchport access vlan 44', + 'no switchport mode trunk', + 'switchport trunk allowed vlan none', + 'interface 4444', + 'no switchport mode trunk', + 'no switchport trunk native vlan 44', + 'switchport trunk allowed vlan remove 44', + 'interface 4444', + 'switchport trunk allowed vlan remove tag1', + 'interface 4444', + 'switchport trunk allowed vlan remove tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_plug_bond_to_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.plug_bond_to_network(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', + 'switchport mode trunk', + 'switchport trunk native vlan 44', + 'switchport trunk allowed vlan add 44', + 'interface 4444', + 'switchport mode trunk', + 'switchport trunk allowed vlan add tag1', + 'interface 4444', + 'switchport mode trunk', + 'switchport trunk allowed vlan add tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_unplug_bond_from_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.unplug_bond_from_network( + 4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', + 'no switchport access vlan 44', + 'no switchport mode trunk', + 'switchport trunk allowed vlan none', + 'interface 4444', + 'no switchport mode trunk', + 'no switchport trunk native vlan 44', + 'switchport trunk allowed vlan remove 44', + 'interface 4444', + 'switchport trunk allowed vlan remove tag1', + 'interface 4444', + 'switchport trunk allowed vlan remove tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_add_subports_on_trunk_no_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [] + self.switch.add_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, []) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_add_subports_on_trunk_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + self.switch.add_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with( + self.switch, + ['interface 44', + 'switchport mode trunk', + 'switchport trunk allowed vlan add tag1', + 'interface 44', + 'switchport mode trunk', + 'switchport trunk allowed vlan add tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_del_subports_on_trunk_no_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [] + self.switch.del_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, []) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_del_subports_on_trunk_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + self.switch.del_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with( + self.switch, + ['interface 44', + 'switchport trunk allowed vlan remove tag1', + 'interface 44', + 'switchport trunk allowed vlan remove tag2']) 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 aac29b2b..5223bdcd 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py +++ b/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py @@ -33,6 +33,7 @@ def setUp(self): super(NetmikoSwitchTestBase, self).setUp() self.cfg = self.useFixture(config_fixture.Config()) self.switch = self._make_switch_device() + self.ctxt = mock.MagicMock() def _make_switch_device(self, extra_cfg={}): patcher = mock.patch.object( @@ -163,7 +164,7 @@ def test_plug_port_to_network_disable_inactive(self, m_check, m_sctd): @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.check_output', autospec=True) def test_delete_port(self, m_check, m_sctd): - self.switch.delete_port(2222, 22) + self.switch.delete_port(2222, 22, trunk_details={}) m_sctd.assert_called_with(self.switch, []) m_check.assert_called_once_with(self.switch, 'fake output', 'unplug port') @@ -175,7 +176,7 @@ def test_delete_port(self, m_check, m_sctd): 'NetmikoSwitch.check_output', autospec=True) def test_delete_port_has_default_vlan(self, m_check, m_sctd): switch = self._make_switch_device({'ngs_port_default_vlan': '20'}) - switch.delete_port(2222, 22) + switch.delete_port(2222, 22, trunk_details={}) m_sctd.assert_called_with(switch, []) m_check.assert_called_once_with(switch, 'fake output', 'unplug port') @@ -196,14 +197,14 @@ def test_delete_port_disable_inactive(self, m_check, m_sctd): return_value='fake output', autospec=True) def test_plug_bond_to_network_fallback(self, m_plug): self.switch.plug_bond_to_network(2222, 22) - m_plug.assert_called_with(self.switch, 2222, 22) + m_plug.assert_called_with(self.switch, 2222, 22, trunk_details=None) @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.delete_port', return_value='fake output', autospec=True) def test_unplug_bond_from_network_fallback(self, m_delete): self.switch.unplug_bond_from_network(2222, 22) - m_delete.assert_called_with(self.switch, 2222, 22) + m_delete.assert_called_with(self.switch, 2222, 22, trunk_details=None) def test__format_commands(self): self.switch._format_commands( diff --git a/networking_generic_switch/tests/unit/netmiko/test_ovs_linux.py b/networking_generic_switch/tests/unit/netmiko/test_ovs_linux.py index 6964036a..ff69c34d 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_ovs_linux.py +++ b/networking_generic_switch/tests/unit/netmiko/test_ovs_linux.py @@ -14,6 +14,8 @@ from unittest import mock +from oslo_utils import uuidutils + from networking_generic_switch.devices.netmiko_devices import ovs from networking_generic_switch.tests.unit.netmiko import test_netmiko_base @@ -28,6 +30,10 @@ def _make_switch_device(self): def test_constants(self): self.assertIsNone(self.switch.SAVE_CONFIGURATION) + def test_features(self): + self.assertTrue(self.switch.support_trunk_on_ports) + self.assertTrue(self.switch.support_trunk_on_bond_ports) + @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.send_commands_to_device', autospec=True) def test_add_network(self, m_exec): @@ -49,6 +55,20 @@ def test_plug_port_to_network(self, mock_exec): ['ovs-vsctl set port 4444 vlan_mode=access', 'ovs-vsctl set port 4444 tag=44']) + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_plug_port_to_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.plug_port_to_network(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['ovs-vsctl set port 4444 vlan_mode=native-untagged', + 'ovs-vsctl set port 4444 tag=44', + 'ovs-vsctl add port 4444 trunks 44', + 'ovs-vsctl add port 4444 trunks tag1', + 'ovs-vsctl add port 4444 trunks tag2']) + @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.send_commands_to_device', autospec=True) def test_delete_port(self, mock_exec): @@ -59,6 +79,137 @@ def test_delete_port(self, mock_exec): 'ovs-vsctl clear port 4444 trunks', 'ovs-vsctl clear port 4444 vlan_mode']) + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_delete_port_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.delete_port(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['ovs-vsctl clear port 4444 tag', + 'ovs-vsctl clear port 4444 trunks', + 'ovs-vsctl clear port 4444 vlan_mode', + 'ovs-vsctl clear port 4444 vlan_mode', + 'ovs-vsctl clear port 4444 tag', + 'ovs-vsctl remove port 4444 trunks 44', + 'ovs-vsctl remove port 4444 trunks tag1', + 'ovs-vsctl remove port 4444 trunks tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_plug_bond_to_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.plug_bond_to_network(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['ovs-vsctl set port 4444 vlan_mode=native-untagged', + 'ovs-vsctl set port 4444 tag=44', + 'ovs-vsctl add port 4444 trunks 44', + 'ovs-vsctl add port 4444 trunks tag1', + 'ovs-vsctl add port 4444 trunks tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_unplug_bond_from_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.unplug_bond_from_network( + 4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['ovs-vsctl clear port 4444 tag', + 'ovs-vsctl clear port 4444 trunks', + 'ovs-vsctl clear port 4444 vlan_mode', + 'ovs-vsctl clear port 4444 vlan_mode', + 'ovs-vsctl clear port 4444 tag', + 'ovs-vsctl remove port 4444 trunks 44', + 'ovs-vsctl remove port 4444 trunks tag1', + 'ovs-vsctl remove port 4444 trunks tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_add_subports_on_trunk_no_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [] + self.switch.add_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, []) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_add_subports_on_trunk_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + self.switch.add_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, + ['ovs-vsctl add port 44 trunks tag1', + 'ovs-vsctl add port 44 trunks tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_del_subports_on_trunk_no_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [] + self.switch.del_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, []) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_del_subports_on_trunk_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + self.switch.del_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, + ['ovs-vsctl remove port 44 trunks tag1', + 'ovs-vsctl remove port 44 trunks tag2']) + def test__format_commands(self): cmd_set = self.switch._format_commands( ovs.OvsLinux.PLUG_PORT_TO_NETWORK, 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 cfb7f5f6..2f946b0c 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,9 @@ 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.devices import utils as device_utils from networking_generic_switch import exceptions from networking_generic_switch import generic_switch_mech as gsm @@ -34,6 +36,8 @@ def setUp(self): self.switch_mock.config = {'device_type': 'bar', 'spam': 'ham', 'ip': 'ip'} self.switch_mock._get_physical_networks.return_value = [] + self.ctxt = mock.MagicMock() + self.db = mock.MagicMock() patcher = mock.patch( 'networking_generic_switch.devices.device_manager', return_value=self.switch_mock, autospec=True) @@ -811,6 +815,92 @@ def test_update_port_postcommit_unbind(self, m_pc, m_list): self.switch_mock.delete_port.assert_called_once_with(2222, 123) m_pc.assert_not_called() + @mock.patch.object(provisioning_blocks, 'provisioning_complete', + autospec=True) + def test_update_port_postcommit_trunk_not_supported(self, m_pc, m_list): + driver = gsm.GenericSwitchDriver() + driver.initialize() + mock_context = mock.create_autospec(driver_context.PortContext) + mock_context._plugin_context = mock.MagicMock() + mock_context._plugin = mock.MagicMock() + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': 2222 + } + ] + }, + 'binding:vnic_type': 'baremetal', + 'id': '123', + 'binding:vif_type': 'other', + 'status': 'DOWN', + 'trunk_details': { + 'sub_ports': [ + {'segmentation_id': 1234, + 'port_id': 's1'} + ] + }} + mock_context.original = {'binding:profile': {}, + 'binding:vnic_type': 'baremetal', + 'id': '123', + 'binding:vif_type': 'unbound'} + + mock_context.top_bound_segment = {'physical_network': 'physnet1', + 'network_id': 'aaaa-bbbb-ccc', + 'segmentation_id': 123} + self.switch_mock._get_physical_networks.return_value = ['physnet1'] + self.switch_mock.support_trunk_on_bond_ports = False + self.switch_mock.support_trunk_on_ports = False + + with self.assertRaises(exceptions.GenericSwitchNotSupported): + driver.update_port_postcommit(mock_context) + self.switch_mock.plug_port_to_network.assert_not_called() + m_pc.assert_not_called() + + @mock.patch.object(provisioning_blocks, 'provisioning_complete', + autospec=True) + def test_update_port_postcommit_trunk(self, m_pc, m_list): + driver = gsm.GenericSwitchDriver() + driver.initialize() + mock_context = mock.create_autospec(driver_context.PortContext) + mock_context._plugin_context = mock.MagicMock() + mock_context._plugin = mock.MagicMock() + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': 2222 + } + ] + }, + 'binding:vnic_type': 'baremetal', + 'id': '123', + 'binding:vif_type': 'other', + 'status': 'DOWN', + 'trunk_details': { + 'sub_ports': [ + {'segmentation_id': 1234, + 'port_id': 's1'} + ] + }} + mock_context.original = {'binding:profile': {}, + 'binding:vnic_type': 'baremetal', + 'id': '123', + 'binding:vif_type': 'unbound'} + mock_context.top_bound_segment = {'physical_network': 'physnet1', + 'network_id': 'aaaa-bbbb-ccc', + 'segmentation_id': 123} + self.switch_mock._get_physical_networks.return_value = ['physnet1'] + self.switch_mock.support_trunk_on_bond_ports = True + self.switch_mock.support_trunk_on_ports = True + + driver.update_port_postcommit(mock_context) + self.switch_mock.plug_port_to_network.assert_called_once() + m_pc.assert_called_once() + @mock.patch.object(provisioning_blocks, 'provisioning_complete', autospec=True) def test_update_portgroup_postcommit_unbind(self, m_pc, m_list): @@ -1147,121 +1237,221 @@ def test_bind_portgroup_802_3ad_port_not_supported(self, m_apc, m_list): self.switch_mock.plug_port_to_network.assert_not_called() self.switch_mock.plug_bond_to_network.assert_not_called() - @mock.patch.object(provisioning_blocks, 'add_provisioning_component', - autospec=True) - def test_bind_port_unknown_switch(self, m_apc, m_list): - driver = gsm.GenericSwitchDriver() - driver.initialize() - mock_context = mock.create_autospec(driver_context.PortContext) - mock_context._plugin_context = mock.MagicMock() - mock_context.current = {'binding:profile': - {'local_link_information': - [ - { - 'switch_info': 'bar', - 'port_id': 2222 - } - ]}, - 'binding:vnic_type': 'baremetal', - 'id': '123'} - mock_context.network.current = { - 'provider:physical_network': 'physnet1' + @mock.patch.object(provisioning_blocks, 'add_provisioning_component', + autospec=True) + def test_bind_port_unknown_switch(self, m_apc, m_list): + driver = gsm.GenericSwitchDriver() + driver.initialize() + mock_context = mock.create_autospec(driver_context.PortContext) + mock_context._plugin_context = mock.MagicMock() + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': '123'} + mock_context.network.current = { + 'provider:physical_network': 'physnet1' + } + mock_context.segments_to_bind = [ + { + 'segmentation_id': None, + 'id': 123 } - mock_context.segments_to_bind = [ - { - 'segmentation_id': None, - 'id': 123 - } - ] - self.assertIsNone(driver.bind_port(mock_context)) - self.assertFalse(mock_context.set_binding.called) - self.switch_mock.plug_port_to_network.assert_not_called() - self.assertFalse(m_apc.called) - - @mock.patch.object(provisioning_blocks, 'add_provisioning_component', - autospec=True) - def test_bind_port_with_different_physnet(self, m_apc, m_list): - driver = gsm.GenericSwitchDriver() - driver.initialize() - mock_context = mock.create_autospec(driver_context.PortContext) - mock_context._plugin_context = mock.MagicMock() - mock_context.current = {'binding:profile': - {'local_link_information': - [ - { - 'switch_info': 'bar', - 'port_id': 2222 - } - ]}, - 'binding:vnic_type': 'baremetal', - 'id': '123'} - mock_context.network.current = { - 'provider:physical_network': 'physnet1' + ] + self.assertIsNone(driver.bind_port(mock_context)) + self.assertFalse(mock_context.set_binding.called) + self.switch_mock.plug_port_to_network.assert_not_called() + self.assertFalse(m_apc.called) + + @mock.patch.object(provisioning_blocks, 'add_provisioning_component', + autospec=True) + def test_bind_port_with_different_physnet(self, m_apc, m_list): + driver = gsm.GenericSwitchDriver() + driver.initialize() + mock_context = mock.create_autospec(driver_context.PortContext) + mock_context._plugin_context = mock.MagicMock() + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': '123'} + mock_context.network.current = { + 'provider:physical_network': 'physnet1' + } + mock_context.segments_to_bind = [ + { + 'segmentation_id': None, + 'id': 123 } - mock_context.segments_to_bind = [ - { - 'segmentation_id': None, - 'id': 123 - } - ] - self.switch_mock._get_physical_networks.return_value = ['physnet2'] - self.assertIsNone(driver.bind_port(mock_context)) - self.assertFalse(mock_context.set_binding.called) - self.switch_mock.plug_port_to_network.assert_not_called() - self.assertFalse(m_apc.called) - - def test_empty_methods(self, m_list): - driver = gsm.GenericSwitchDriver() - driver.initialize() - mock_context = mock.create_autospec(driver_context.NetworkContext) - mock_context.current = {'id': 22, - 'provider:network_type': 'vlan', - 'provider:segmentation_id': 22} - - driver.initialize() - - driver.create_network_precommit(mock_context) - driver.update_network_precommit(mock_context) - driver.update_network_postcommit(mock_context) - driver.delete_network_precommit(mock_context) - driver.create_subnet_precommit(mock_context) - driver.create_subnet_postcommit(mock_context) - driver.update_subnet_precommit(mock_context) - driver.update_subnet_postcommit(mock_context) - driver.delete_subnet_precommit(mock_context) - driver.delete_subnet_postcommit(mock_context) - driver.create_port_precommit(mock_context) - driver.create_port_postcommit(mock_context) - driver.update_port_precommit(mock_context) - driver.delete_port_precommit(mock_context) - - -class TestGenericSwitchDriverStaticMethods(unittest.TestCase): - - def test__is_802_3ad_no_lgi(self): + ] + self.switch_mock._get_physical_networks.return_value = ['physnet2'] + self.assertIsNone(driver.bind_port(mock_context)) + self.assertFalse(mock_context.set_binding.called) + self.switch_mock.plug_port_to_network.assert_not_called() + self.assertFalse(m_apc.called) + + @mock.patch.object(directory, "get_plugin", autospec=True) + @mock.patch.object(device_utils, "get_switch_device", autospec=True) + def test_subports_added_other_port(self, mock_get_switch, mock_plugin, + m_list): driver = gsm.GenericSwitchDriver() - port = {'binding:profile': {}} - self.assertFalse(driver._is_802_3ad(port)) + driver.initialize() - def test__is_802_3ad_no_bond_mode(self): + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': '123' + } + subports = [{"segmentation_id": "tag1", "port_id": "s1"}, + {"segmentation_id": "tag2", "port_id": "s2"}] + + mock_plugin.return_value = mock.MagicMock() + mock_plugin.return_value.get_port.return_value = {"status": "DOWN"} + + driver.subports_added(self.ctxt, parent_port, subports=subports) + mock_get_switch.return_value.add_subports_on_trunk.assart_not_called() + + @mock.patch.object(device_utils, "get_switch_device", autospec=True) + def test_subports_added_no_llc(self, mock_get_switch, m_list): + driver = gsm.GenericSwitchDriver() + driver.initialize() + + parent_port = { + 'binding:profile': {}, + 'binding:vnic_type': 'baremetal', + 'id': '123' + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + + driver.subports_added(self.ctxt, parent_port, subports=subports) + mock_get_switch.return_value.add_subports_on_trunk.assart_not_called() + + @mock.patch.object(directory, "get_plugin", autospec=True) + @mock.patch.object(device_utils, "get_switch_device", autospec=True) + def test_subports_added(self, mock_get_switch, mock_plugin, m_list): driver = gsm.GenericSwitchDriver() - port = {'binding:profile': {'local_group_information': {}}} - self.assertFalse(driver._is_802_3ad(port)) + driver.initialize() + + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': '123' + } + subports = [{"segmentation_id": "tag1", "port_id": "s1"}, + {"segmentation_id": "tag2", "port_id": "s2"}] + + mock_plugin.return_value = mock.MagicMock() + mock_plugin.return_value.get_port.return_value = {"status": "DOWN"} + + driver.subports_added(self.ctxt, parent_port, subports=subports) + mock_get_switch.return_value.add_subports_on_trunk.assert_has_calls( + [mock.call(parent_port['binding:profile'], 2222, subports)]) + + @mock.patch.object(device_utils, "get_switch_device", autospec=True) + def test_subports_deleted_other_port(self, mock_get_switch, m_list): + driver = gsm.GenericSwitchDriver() + driver.initialize() - def test__is_802_3ad_wrong_bond_mode(self): + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': '123' + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + + driver.subports_deleted(self.ctxt, parent_port, subports=subports) + mock_get_switch.return_value.del_subports_on_trunk.assart_not_called() + + @mock.patch.object(device_utils, "get_switch_device", autospec=True) + def test_subports_deleted_no_llc(self, mock_get_switch, m_list): driver = gsm.GenericSwitchDriver() - port = {'binding:profile': {'local_group_information': - {'bond_mode': 42}}} - self.assertFalse(driver._is_802_3ad(port)) + driver.initialize() - def test__is_802_3ad_numeric(self): + parent_port = { + 'binding:profile': {}, + 'binding:vnic_type': 'baremetal', + 'id': '123' + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + + driver.subports_deleted(self.ctxt, parent_port, subports=subports) + mock_get_switch.return_value.del_subports_on_trunk.assart_not_called() + + @mock.patch.object(device_utils, "get_switch_device", autospec=True) + def test_subports_deleted(self, mock_get_switch, m_list): driver = gsm.GenericSwitchDriver() - port = {'binding:profile': {'local_group_information': - {'bond_mode': '4'}}} - self.assertTrue(driver._is_802_3ad(port)) + driver.initialize() + + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': '123' + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + + driver.subports_deleted(self.ctxt, parent_port, subports=subports) + mock_get_switch.return_value.del_subports_on_trunk.assert_has_calls( + [mock.call(parent_port['binding:profile'], 2222, subports)]) - def test__is_802_3ad_string(self): + def test_empty_methods(self, m_list): driver = gsm.GenericSwitchDriver() - port = {'binding:profile': {'local_group_information': - {'bond_mode': '802.3ad'}}} - self.assertTrue(driver._is_802_3ad(port)) + driver.initialize() + mock_context = mock.create_autospec(driver_context.NetworkContext) + mock_context.current = {'id': 22, + 'provider:network_type': 'vlan', + 'provider:segmentation_id': 22} + + driver.initialize() + + driver.create_network_precommit(mock_context) + driver.update_network_precommit(mock_context) + driver.update_network_postcommit(mock_context) + driver.delete_network_precommit(mock_context) + driver.create_subnet_precommit(mock_context) + driver.create_subnet_postcommit(mock_context) + driver.update_subnet_precommit(mock_context) + driver.update_subnet_postcommit(mock_context) + driver.delete_subnet_precommit(mock_context) + driver.delete_subnet_postcommit(mock_context) + driver.create_port_precommit(mock_context) + driver.create_port_postcommit(mock_context) + driver.update_port_precommit(mock_context) + driver.delete_port_precommit(mock_context) diff --git a/networking_generic_switch/trunk_driver.py b/networking_generic_switch/trunk_driver.py new file mode 100644 index 00000000..3daf8ac6 --- /dev/null +++ b/networking_generic_switch/trunk_driver.py @@ -0,0 +1,109 @@ +# Copyright 2024 StackHPC Ltd +# +# 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 neutron.objects.ports import Port +from neutron.services.trunk.drivers import base as trunk_base +from neutron_lib.api.definitions import portbindings +from neutron_lib.callbacks import events +from neutron_lib.callbacks import registry +from neutron_lib.callbacks import resources +from neutron_lib import context as n_context +from neutron_lib.db import api as db_api +from neutron_lib.plugins import directory +from neutron_lib.services.trunk import constants as trunk_consts +from oslo_config import cfg +from oslo_log import log as logging + +LOG = logging.getLogger(__name__) + +MECH_DRIVER_NAME = 'genericswitch' + +SUPPORTED_INTERFACES = ( + portbindings.VIF_TYPE_OTHER, + portbindings.VIF_TYPE_VHOST_USER, +) + +SUPPORTED_SEGMENTATION_TYPES = ( + trunk_consts.SEGMENTATION_TYPE_VLAN, +) + + +class GenericSwitchTrunkDriver(trunk_base.DriverBase): + @property + def is_loaded(self): + try: + return (MECH_DRIVER_NAME in + cfg.CONF.ml2.mechanism_drivers) + except cfg.NoSuchOptError: + return False + + @registry.receives(resources.TRUNK_PLUGIN, [events.AFTER_INIT]) + def register(self, resource, event, trigger, payload=None): + super(GenericSwitchTrunkDriver, self).register( + resource, event, trigger, payload=payload) + self._handler = GenericSwitchTrunkHandler(self.plugin_driver) + registry.subscribe( + self._handler.subports_added, + resources.SUBPORTS, + events.AFTER_CREATE) + registry.subscribe( + self._handler.subports_deleted, + resources.SUBPORTS, + events.AFTER_DELETE) + + @classmethod + def create(cls, plugin_driver): + cls.plugin_driver = plugin_driver + return cls(MECH_DRIVER_NAME, + SUPPORTED_INTERFACES, + SUPPORTED_SEGMENTATION_TYPES, + None, + can_trunk_bound_port=True) + + +class GenericSwitchTrunkHandler(object): + def __init__(self, plugin_driver): + self.plugin_driver = plugin_driver + self.core_plugin = directory.get_plugin() + + def subports_added(self, resource, event, trunk_plugin, payload): + trunk = payload.states[0] + subports = payload.metadata['subports'] + LOG.debug("GenericSwitch: subports added %s to trunk %s", + subports, trunk) + context = n_context.get_admin_context() + with db_api.CONTEXT_READER.using(context): + parent_port = Port.get_object(context, id=trunk.port_id) + + parent_port_obj = self.core_plugin._make_port_dict(parent_port) + + self.plugin_driver.subports_added( + context, + parent_port_obj, + subports) + + def subports_deleted(self, resource, event, trunk_plugin, payload): + trunk = payload.states[0] + subports = payload.metadata['subports'] + LOG.debug("GenericSwitch: subports deleted %s from trunk %s", + subports, trunk) + context = n_context.get_admin_context() + with db_api.CONTEXT_READER.using(context): + parent_port = Port.get_object(context, id=trunk.port_id) + + parent_port_obj = self.core_plugin._make_port_dict(parent_port) + self.plugin_driver.subports_deleted( + context, + parent_port_obj, + subports) diff --git a/networking_generic_switch/utils.py b/networking_generic_switch/utils.py new file mode 100644 index 00000000..82464643 --- /dev/null +++ b/networking_generic_switch/utils.py @@ -0,0 +1,27 @@ +# Copyright 2025 Mirantis, Inc. +# +# 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. + +def is_802_3ad(binding_profile): + """Return whether a port binding profile is using 802.3ad link aggregation. + + :param binding_profile: The port binding_profile to check + :returns: Whether the port is a port group using 802.3ad link + aggregation. + """ + binding_profile = binding_profile or {} + local_group_information = binding_profile.get( + 'local_group_information') + if not local_group_information: + return False + return local_group_information.get('bond_mode') in ['4', '802.3ad'] diff --git a/releasenotes/notes/vlan-aware-vms-3923cc17254829e9.yaml b/releasenotes/notes/vlan-aware-vms-3923cc17254829e9.yaml new file mode 100644 index 00000000..c4b6269d --- /dev/null +++ b/releasenotes/notes/vlan-aware-vms-3923cc17254829e9.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Add support of VLAN aware instances for the following drivers + * OVS (as reference implementation) + * AristaEos + * CiscoIos From 1b53447e19663d6927b404d961e3c5996d24da85 Mon Sep 17 00:00:00 2001 From: Will Szumski Date: Fri, 7 Feb 2025 13:07:05 +0000 Subject: [PATCH 4/5] Adds vlan aware VMs support for Cumulus NVUE and DellOS10 Change-Id: Id8b90b1a4c2dca244d039bbdffd6030ed99cc1af --- .../devices/netmiko_devices/cumulus.py | 55 ++++++ .../devices/netmiko_devices/dell.py | 11 ++ .../tests/unit/netmiko/test_cumulus_nvue.py | 158 +++++++++++++++++- .../tests/unit/netmiko/test_dell.py | 143 ++++++++++++++++ .../vlan-aware-vms-3923cc17254829e9.yaml | 2 + 5 files changed, 365 insertions(+), 4 deletions(-) diff --git a/networking_generic_switch/devices/netmiko_devices/cumulus.py b/networking_generic_switch/devices/netmiko_devices/cumulus.py index 277c1ee9..ce965d2a 100644 --- a/networking_generic_switch/devices/netmiko_devices/cumulus.py +++ b/networking_generic_switch/devices/netmiko_devices/cumulus.py @@ -117,12 +117,67 @@ class CumulusNVUE(netmiko_devices.NetmikoSwitch): ] PLUG_PORT_TO_NETWORK = [ + 'nv unset interface {port} bridge domain br_default untagged', 'nv set interface {port} bridge domain br_default access ' '{segmentation_id}', ] + ADD_NETWORK_TO_TRUNK = [ + 'nv unset interface {port} bridge domain br_default access', + 'nv set interface {port} bridge domain br_default vlan ' + '{segmentation_id}', + ] + + ADD_NETWORK_TO_BOND_TRUNK = [ + 'nv unset interface {bond} bridge domain br_default access', + 'nv set interface {bond} bridge domain br_default vlan ' + '{segmentation_id}', + ] + + REMOVE_NETWORK_FROM_TRUNK = ( + 'nv unset interface {port} bridge domain br_default vlan ' + '{segmentation_id}', + ) + + DELETE_NETWORK_ON_BOND_TRUNK = ( + 'nv unset interface {bond} bridge domain br_default vlan ' + '{segmentation_id}', + ) + + SET_NATIVE_VLAN = [ + 'nv unset interface {port} bridge domain br_default access', + 'nv set interface {port} bridge domain br_default untagged ' + '{segmentation_id}', + 'nv set interface {port} bridge domain br_default vlan ' + '{segmentation_id}', + ] + + SET_NATIVE_VLAN_BOND = [ + 'nv unset interface {bond} bridge domain br_default access', + 'nv set interface {bond} bridge domain br_default untagged ' + '{segmentation_id}', + 'nv set interface {bond} bridge domain br_default vlan ' + '{segmentation_id}', + ] + + DELETE_NATIVE_VLAN = ( + 'nv unset interface {port} bridge domain br_default untagged ' + '{segmentation_id}', + 'nv unset interface {port} bridge domain br_default vlan ' + '{segmentation_id}', + ) + + DELETE_NATIVE_VLAN_BOND = ( + 'nv unset interface {bond} bridge domain br_default untagged ' + '{segmentation_id}', + 'nv unset interface {bond} bridge domain br_default vlan ' + '{segmentation_id}', + ) + DELETE_PORT = [ 'nv unset interface {port} bridge domain br_default access', + 'nv unset interface {port} bridge domain br_default untagged', + 'nv unset interface {port} bridge domain br_default vlan', ] ENABLE_PORT = [ diff --git a/networking_generic_switch/devices/netmiko_devices/dell.py b/networking_generic_switch/devices/netmiko_devices/dell.py index a5c824df..b3425d19 100644 --- a/networking_generic_switch/devices/netmiko_devices/dell.py +++ b/networking_generic_switch/devices/netmiko_devices/dell.py @@ -58,6 +58,17 @@ class DellOS10(netmiko_devices.NetmikoSwitch): "exit", ) + SET_NATIVE_VLAN = ( + 'interface {port}', + 'switchport mode trunk', + 'switchport access vlan {segmentation_id}', + ) + + DELETE_NATIVE_VLAN = ( + 'interface {port}', + 'no switchport access vlan', + ) + ENABLE_PORT = ( "interface {port}", "no shutdown", diff --git a/networking_generic_switch/tests/unit/netmiko/test_cumulus_nvue.py b/networking_generic_switch/tests/unit/netmiko/test_cumulus_nvue.py index d6479404..f262259f 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_cumulus_nvue.py +++ b/networking_generic_switch/tests/unit/netmiko/test_cumulus_nvue.py @@ -14,6 +14,8 @@ from unittest import mock +from oslo_utils import uuidutils + from networking_generic_switch.devices.netmiko_devices import cumulus from networking_generic_switch import exceptions as exc from networking_generic_switch.tests.unit.netmiko import test_netmiko_base @@ -57,6 +59,9 @@ def test_plug_port_to_network(self, mock_exec): self.switch, ['nv set interface 3333 link state up', 'nv unset interface 3333 bridge domain br_default access', + 'nv unset interface 3333 bridge domain br_default untagged', + 'nv unset interface 3333 bridge domain br_default vlan', + 'nv unset interface 3333 bridge domain br_default untagged', 'nv set interface 3333 bridge domain br_default access 33']) @mock.patch('networking_generic_switch.devices.netmiko_devices.' @@ -89,7 +94,8 @@ def test_plug_port_simple(self, mock_exec): switch.plug_port_to_network(3333, 33) mock_exec.assert_called_with( switch, - ['nv set interface 3333 bridge domain br_default access 33']) + ['nv unset interface 3333 bridge domain br_default untagged', + 'nv set interface 3333 bridge domain br_default access 33']) @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.send_commands_to_device', @@ -99,7 +105,10 @@ def test_delete_port(self, mock_exec): mock_exec.assert_called_with( self.switch, ['nv unset interface 3333 bridge domain br_default access', + 'nv unset interface 3333 bridge domain br_default untagged', + 'nv unset interface 3333 bridge domain br_default vlan', 'nv set bridge domain br_default vlan 123', + 'nv unset interface 3333 bridge domain br_default untagged', 'nv set interface 3333 bridge domain br_default access 123', 'nv set interface 3333 link state down']) @@ -114,7 +123,9 @@ def test_delete_port_simple(self, mock_exec): switch.delete_port(3333, 33) mock_exec.assert_called_with( switch, - ['nv unset interface 3333 bridge domain br_default access']) + ['nv unset interface 3333 bridge domain br_default access', + 'nv unset interface 3333 bridge domain br_default untagged', + 'nv unset interface 3333 bridge domain br_default vlan']) @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.send_commands_to_device', @@ -125,6 +136,9 @@ def test_plug_bond_to_network(self, mock_exec): self.switch, ['nv set interface 3333 link state up', 'nv unset interface 3333 bridge domain br_default access', + 'nv unset interface 3333 bridge domain br_default untagged', + 'nv unset interface 3333 bridge domain br_default vlan', + 'nv unset interface 3333 bridge domain br_default untagged', 'nv set interface 3333 bridge domain br_default access 33']) @mock.patch('networking_generic_switch.devices.netmiko_devices.' @@ -138,7 +152,8 @@ def test_plug_bond_simple(self, mock_exec): switch.plug_bond_to_network(3333, 33) mock_exec.assert_called_with( switch, - ['nv set interface 3333 bridge domain br_default access 33']) + ['nv unset interface 3333 bridge domain br_default untagged', + 'nv set interface 3333 bridge domain br_default access 33']) @mock.patch('networking_generic_switch.devices.netmiko_devices.' 'NetmikoSwitch.send_commands_to_device', @@ -148,7 +163,10 @@ def test_unplug_bond_from_network(self, mock_exec): mock_exec.assert_called_with( self.switch, ['nv unset interface 3333 bridge domain br_default access', + 'nv unset interface 3333 bridge domain br_default untagged', + 'nv unset interface 3333 bridge domain br_default vlan', 'nv set bridge domain br_default vlan 123', + 'nv unset interface 3333 bridge domain br_default untagged', 'nv set interface 3333 bridge domain br_default access 123', 'nv set interface 3333 link state down']) @@ -163,10 +181,142 @@ def test_unplug_bond_from_network_simple(self, mock_exec): switch.unplug_bond_from_network(3333, 33) mock_exec.assert_called_with( switch, - ['nv unset interface 3333 bridge domain br_default access']) + ['nv unset interface 3333 bridge domain br_default access', + 'nv unset interface 3333 bridge domain br_default untagged', + 'nv unset interface 3333 bridge domain br_default vlan']) def test_save(self): mock_connect = mock.MagicMock() mock_connect.save_config.side_effect = NotImplementedError self.switch.save_configuration(mock_connect) mock_connect.send_command.assert_called_with('nv config save') + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.check_output', autospec=True) + def test_plug_port_to_network_subports(self, _, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.plug_port_to_network(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['nv set interface 4444 link state up', + 'nv unset interface 4444 bridge domain br_default access', + 'nv unset interface 4444 bridge domain br_default untagged', + 'nv unset interface 4444 bridge domain br_default vlan', + 'nv unset interface 4444 bridge domain br_default access', + 'nv set interface 4444 bridge domain br_default untagged 44', + 'nv set interface 4444 bridge domain br_default vlan 44', + 'nv unset interface 4444 bridge domain br_default access', + 'nv set interface 4444 bridge domain br_default vlan tag1', + 'nv unset interface 4444 bridge domain br_default access', + 'nv set interface 4444 bridge domain br_default vlan tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.check_output', autospec=True) + def test_delete_port_subports(self, _, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.delete_port(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['nv unset interface 4444 bridge domain br_default access', + 'nv unset interface 4444 bridge domain br_default untagged', + 'nv unset interface 4444 bridge domain br_default vlan', + 'nv unset interface 4444 bridge domain br_default untagged 44', + 'nv unset interface 4444 bridge domain br_default vlan 44', + 'nv unset interface 4444 bridge domain br_default vlan tag1', + 'nv unset interface 4444 bridge domain br_default vlan tag2', + 'nv set bridge domain br_default vlan 123', + 'nv unset interface 4444 bridge domain br_default untagged', + 'nv set interface 4444 bridge domain br_default access 123', + 'nv set interface 4444 link state down']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_add_subports_on_trunk_no_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [] + self.switch.add_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, []) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_add_subports_on_trunk_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + self.switch.add_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with( + self.switch, + ['nv unset interface 44 bridge domain br_default access', + 'nv set interface 44 bridge domain br_default vlan tag1', + 'nv unset interface 44 bridge domain br_default access', + 'nv set interface 44 bridge domain br_default vlan tag2']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_del_subports_on_trunk_no_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [] + self.switch.del_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, []) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_del_subports_on_trunk_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + self.switch.del_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with( + self.switch, + ['nv unset interface 44 bridge domain br_default vlan tag1', + 'nv unset interface 44 bridge domain br_default vlan tag2']) diff --git a/networking_generic_switch/tests/unit/netmiko/test_dell.py b/networking_generic_switch/tests/unit/netmiko/test_dell.py index e7eb8284..d55c6357 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_dell.py +++ b/networking_generic_switch/tests/unit/netmiko/test_dell.py @@ -14,6 +14,8 @@ from unittest import mock +from oslo_utils import uuidutils + 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 @@ -271,6 +273,147 @@ def test__format_commands(self): 'no switchport trunk allowed vlan 33', 'exit']) + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_plug_port_to_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.plug_port_to_network(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', 'switchport mode trunk', + 'switchport access vlan 44', 'interface 4444', + 'switchport mode trunk', 'switchport trunk allowed vlan tag1', + 'exit', 'interface 4444', 'switchport mode trunk', + 'switchport trunk allowed vlan tag2', 'exit']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_delete_port_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.delete_port(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', 'no switchport access vlan', 'exit', + 'interface 4444', 'no switchport access vlan', 'interface 4444', + 'no switchport trunk allowed vlan tag1', 'exit', 'interface 4444', + 'no switchport trunk allowed vlan tag2', 'exit']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_plug_bond_to_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.plug_bond_to_network(4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', 'switchport mode trunk', + 'switchport access vlan 44', 'interface 4444', + 'switchport mode trunk', 'switchport trunk allowed vlan tag1', + 'exit', 'interface 4444', 'switchport mode trunk', + 'switchport trunk allowed vlan tag2', 'exit']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_unplug_bond_from_network_subports(self, mock_exec): + trunk_details = {"sub_ports": [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}]} + self.switch.unplug_bond_from_network( + 4444, 44, trunk_details=trunk_details) + mock_exec.assert_called_with( + self.switch, + ['interface 4444', 'no switchport access vlan', 'exit', + 'interface 4444', 'no switchport access vlan', 'interface 4444', + 'no switchport trunk allowed vlan tag1', 'exit', 'interface 4444', + 'no switchport trunk allowed vlan tag2', 'exit']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_add_subports_on_trunk_no_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [] + self.switch.add_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, []) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_add_subports_on_trunk_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + self.switch.add_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with( + self.switch, + ['interface 44', 'switchport mode trunk', + 'switchport trunk allowed vlan tag1', 'exit', 'interface 44', + 'switchport mode trunk', 'switchport trunk allowed vlan tag2', + 'exit']) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_del_subports_on_trunk_no_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [] + self.switch.del_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with(self.switch, []) + + @mock.patch('networking_generic_switch.devices.netmiko_devices.' + 'NetmikoSwitch.send_commands_to_device', autospec=True) + def test_del_subports_on_trunk_subports(self, mock_exec): + port_id = uuidutils.generate_uuid() + parent_port = { + 'binding:profile': { + 'local_link_information': [ + { + 'switch_info': 'bar', + 'port_id': 2222 + } + ]}, + 'binding:vnic_type': 'baremetal', + 'id': port_id + } + subports = [{"segmentation_id": "tag1"}, + {"segmentation_id": "tag2"}] + self.switch.del_subports_on_trunk(parent_port, 44, subports=subports) + mock_exec.assert_called_with( + self.switch, + ['interface 44', 'no switchport trunk allowed vlan tag1', 'exit', + 'interface 44', 'no switchport trunk allowed vlan tag2', 'exit']) + class TestNetmikoDellPowerConnect(test_netmiko_base.NetmikoSwitchTestBase): diff --git a/releasenotes/notes/vlan-aware-vms-3923cc17254829e9.yaml b/releasenotes/notes/vlan-aware-vms-3923cc17254829e9.yaml index c4b6269d..c102e178 100644 --- a/releasenotes/notes/vlan-aware-vms-3923cc17254829e9.yaml +++ b/releasenotes/notes/vlan-aware-vms-3923cc17254829e9.yaml @@ -5,3 +5,5 @@ features: * OVS (as reference implementation) * AristaEos * CiscoIos + * DellOS 10 + * Cumulus NVUE From 0d3cec2d217b94a61a79424cdbe1678cdf15eb54 Mon Sep 17 00:00:00 2001 From: cid Date: Fri, 28 Feb 2025 21:51:36 +0100 Subject: [PATCH 5/5] Fix info leakage in Netmiko connection errors Sanitize error messages generated by Netmiko on the event of a connection failure and redact sensitive information before it's propagated. Currenly, when a connection to a network device fails, sensitive information such as IP address, username, etc, are logged alongise the error message. Closes-Bug: #2100566 Change-Id: I124d44f161e3116ec2588617ed98d559f6fe3b1f --- .../devices/netmiko_devices/__init__.py | 52 ++++++++++++------- .../devices/netmiko_devices/juniper.py | 31 ++++++----- .../devices/netmiko_devices/nokia.py | 8 +-- networking_generic_switch/devices/utils.py | 3 +- networking_generic_switch/exceptions.py | 6 ++- .../tests/unit/devices/test_utils.py | 6 ++- .../tests/unit/netmiko/test_dell.py | 5 +- .../tests/unit/netmiko/test_juniper.py | 8 +-- .../tests/unit/netmiko/test_netmiko_base.py | 7 ++- ...netmiko-info-leakage-423c4c59b924c06f.yaml | 8 +++ 10 files changed, 85 insertions(+), 49 deletions(-) create mode 100644 releasenotes/notes/fix-netmiko-info-leakage-423c4c59b924c06f.yaml diff --git a/networking_generic_switch/devices/netmiko_devices/__init__.py b/networking_generic_switch/devices/netmiko_devices/__init__.py index 7c5c2d83..5c372ef8 100644 --- a/networking_generic_switch/devices/netmiko_devices/__init__.py +++ b/networking_generic_switch/devices/netmiko_devices/__init__.py @@ -26,6 +26,7 @@ import tenacity from tooz import coordination +from networking_generic_switch._i18n import _ from networking_generic_switch import batching from networking_generic_switch import devices from networking_generic_switch.devices import utils as device_utils @@ -153,10 +154,13 @@ def __init__(self, device_cfg, *args, **kwargs): 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") + error = ("ngs_batch_requests is true but [ngs_coordination] " + "backend_url is not provided") + LOG.error( + _("%(device)s, error: %(error)s"), + {'device': device_utils.sanitise_config(self.config), + 'error': error}) + raise exc.GenericSwitchNetmikoConfigError() # NOTE: we skip the lock if we are batching requests self.locker = None switch_name = self.lock_kwargs['locks_prefix'] @@ -221,13 +225,19 @@ def _create_connection(): try: net_connect = _create_connection() except tenacity.RetryError as e: - LOG.error("Reached maximum SSH connection attempts, not retrying") - raise exc.GenericSwitchNetmikoConnectError( - config=device_utils.sanitise_config(self.config), error=e) + LOG.error( + _("Reached maximum SSH connection attempts, not retrying " + "for device: %(device)s, error: %(error)s"), { + 'device': device_utils.sanitise_config(self.config), + 'error': e}) + raise exc.GenericSwitchNetmikoConnectError() except Exception as e: - LOG.error("Unexpected exception during SSH connection") - raise exc.GenericSwitchNetmikoConnectError( - config=device_utils.sanitise_config(self.config), error=e) + LOG.error( + _("Unexpected exception during SSH connection " + "to device: %(device)s, error: %(error)s"), { + 'device': device_utils.sanitise_config(self.config), + 'error': e}) + raise exc.GenericSwitchNetmikoConnectError() # Now yield the connection to the caller. with net_connect: @@ -257,8 +267,12 @@ def _send_commands_to_device(self, cmd_set): # module. raise except Exception as e: - raise exc.GenericSwitchNetmikoConnectError( - config=device_utils.sanitise_config(self.config), error=e) + LOG.error( + _("Error sending commands to device: %(device)s, " + "error: %(error)s"), { + 'device': device_utils.sanitise_config(self.config), + 'error': e}) + raise exc.GenericSwitchNetmikoConnectError() LOG.debug(output) return output @@ -488,12 +502,14 @@ def check_output(self, output, operation): for pattern in self.ERROR_MSG_PATTERNS: if pattern.search(output): - msg = ("Found invalid configuration in device response. " - "Operation: %(operation)s. Output: %(output)s" % - {'operation': operation, 'output': output}) - raise exc.GenericSwitchNetmikoConfigError( - config=device_utils.sanitise_config(self.config), - error=msg) + LOG.error( + _("Found invalid configuration in device response. " + "Operation: %(operation)s. Output: %(output)s. " + "Device: %(device)s"), { + 'operation': operation, 'output': output, + 'device': device_utils.sanitise_config(self.config) + }) + raise exc.GenericSwitchNetmikoConfigError() def add_subports_on_trunk(self, binding_profile, port_id, subports): """Allow subports on trunk diff --git a/networking_generic_switch/devices/netmiko_devices/juniper.py b/networking_generic_switch/devices/netmiko_devices/juniper.py index c4f0cbf8..93adff7e 100644 --- a/networking_generic_switch/devices/netmiko_devices/juniper.py +++ b/networking_generic_switch/devices/netmiko_devices/juniper.py @@ -15,6 +15,7 @@ from oslo_log import log as logging import tenacity +from networking_generic_switch._i18n import _ from networking_generic_switch.devices import netmiko_devices from networking_generic_switch.devices import utils as device_utils from networking_generic_switch import exceptions as exc @@ -172,20 +173,26 @@ def commit(): try: commit() except DBLocked as e: - msg = ("Reached timeout waiting for switch configuration DB lock. " - "Configuration might not be committed. Error: %s" % str(e)) + msg = _("Reached timeout waiting for switch configuration " + "DB lock. Configuration might not be committed. " + "Device: %(device)s, error: %(error)s") % { + 'device': device_utils.sanitise_config(self.config), + 'error': e} LOG.error(msg) - raise exc.GenericSwitchNetmikoConfigError( - config=device_utils.sanitise_config(self.config), error=msg) + raise exc.GenericSwitchNetmikoConfigError() except (WarningStmtNotExist, WarningStmtExists) as e: - msg = ("Reached timeout while attempting to apply configuration. " - "This is likely to be caused by multiple sessions " - "configuring the device concurrently. Error: %s" % str(e)) + msg = _("Reached timeout while attempting to apply " + "configuration. This is likely to be caused by multiple " + "sessions configuring the device concurrently. " + "Device: %(device)s, error: %(error)s") % { + 'device': device_utils.sanitise_config(self.config), + 'error': e} LOG.error(msg) - raise exc.GenericSwitchNetmikoConfigError( - config=device_utils.sanitise_config(self.config), error=msg) + raise exc.GenericSwitchNetmikoConfigError() except ValueError as e: - msg = "Failed to commit configuration: %s" % e + msg = _("Failed to commit configuration: Device: %(device)s, " + "error: %(error)s") % { + 'device': device_utils.sanitise_config(self.config), + 'error': e} LOG.error(msg) - raise exc.GenericSwitchNetmikoConfigError( - config=device_utils.sanitise_config(self.config), error=msg) + raise exc.GenericSwitchNetmikoConfigError() diff --git a/networking_generic_switch/devices/netmiko_devices/nokia.py b/networking_generic_switch/devices/netmiko_devices/nokia.py index 9b65d027..2ee56e08 100644 --- a/networking_generic_switch/devices/netmiko_devices/nokia.py +++ b/networking_generic_switch/devices/netmiko_devices/nokia.py @@ -14,6 +14,7 @@ from oslo_log import log as logging +from networking_generic_switch._i18n import _ from networking_generic_switch.devices import netmiko_devices from networking_generic_switch.devices import utils as device_utils from networking_generic_switch import exceptions as exc @@ -81,9 +82,10 @@ def send_commands_to_device(self, cmd_set): # module. raise except Exception as e: - raise exc.GenericSwitchNetmikoConnectError( - config=device_utils.sanitise_config(self.config), error=e - ) + LOG.error(_("Device: %(device)s, error: %(error)s"), { + 'device': device_utils.sanitise_config(self.config), + 'error': e}) + raise exc.GenericSwitchNetmikoConnectError() LOG.debug(output) return output diff --git a/networking_generic_switch/devices/utils.py b/networking_generic_switch/devices/utils.py index ecd34924..429eaf59 100644 --- a/networking_generic_switch/devices/utils.py +++ b/networking_generic_switch/devices/utils.py @@ -46,7 +46,8 @@ def sanitise_config(config): :param config: a configuration dict to sanitise. :returns: a copy of the configuration, with sensitive fields removed. """ - sanitised_fields = {"password"} + sanitised_fields = {"password", "ip", "device_type", "username", + "session_log"} return { key: "******" if key in sanitised_fields else value for key, value in config.items() diff --git a/networking_generic_switch/exceptions.py b/networking_generic_switch/exceptions.py index de68e621..79ffc74d 100644 --- a/networking_generic_switch/exceptions.py +++ b/networking_generic_switch/exceptions.py @@ -44,11 +44,13 @@ class GenericSwitchNetmikoNotSupported(GenericSwitchException): class GenericSwitchNetmikoConnectError(GenericSwitchException): - message = _("Netmiko connection error: %(config)s, error: %(error)s") + message = _("Failed to connect to Netmiko switch. " + "Please contact your administrator.") class GenericSwitchNetmikoConfigError(GenericSwitchException): - message = _("Netmiko configuration error: %(config)s, error: %(error)s") + message = _("Netmiko switch configuration operation failed. " + "Please contact your administrator.") class GenericSwitchBatchError(GenericSwitchException): diff --git a/networking_generic_switch/tests/unit/devices/test_utils.py b/networking_generic_switch/tests/unit/devices/test_utils.py index 73e3b358..d642f4f7 100644 --- a/networking_generic_switch/tests/unit/devices/test_utils.py +++ b/networking_generic_switch/tests/unit/devices/test_utils.py @@ -60,7 +60,9 @@ def test_get_switch_device_fallback_to_switch_info(self): ngs_mac_address='11:22:33:44:55:77')) def test_sanitise_config(self): - config = {'username': 'fake-user', 'password': 'fake-password'} + config = {'username': 'fake-user', 'password': 'fake-password', + 'ip': '123.456.789', 'session_log': '/some/path/here', + "device_type": "my_device"} result = device_utils.sanitise_config(config) - expected = {'username': 'fake-user', 'password': '******'} + expected = {k: '******' for k in config} self.assertEqual(expected, result) diff --git a/networking_generic_switch/tests/unit/netmiko/test_dell.py b/networking_generic_switch/tests/unit/netmiko/test_dell.py index d55c6357..0f0c79d0 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_dell.py +++ b/networking_generic_switch/tests/unit/netmiko/test_dell.py @@ -565,9 +565,8 @@ def test_check_output(self): self.switch.check_output('fake output', 'fake op') def _test_check_output_error(self, output): - msg = ("Found invalid configuration in device response. Operation: " - "fake op. Output: %s" % output) - self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, msg, + self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, + "switch configuration operation failed", self.switch.check_output, output, 'fake op') def test_check_output_incomplete_command(self): diff --git a/networking_generic_switch/tests/unit/netmiko/test_juniper.py b/networking_generic_switch/tests/unit/netmiko/test_juniper.py index fc1cf6e9..9e91e342 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_juniper.py +++ b/networking_generic_switch/tests/unit/netmiko/test_juniper.py @@ -177,7 +177,7 @@ def test_save_configuration_db_locked_timeout(self, m_stop, m_wait): "Commit failed with the following errors:\n\n{0}".format(output)) self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, - "Reached timeout waiting for", + "switch configuration operation failed", self.switch.save_configuration, mock_connection) self.assertGreater(mock_connection.commit.call_count, 1) @@ -227,7 +227,7 @@ def test_save_configuration_warn_already_exists_timeout( "Commit failed with the following errors:\n\n{0}".format(output)) self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, - "Reached timeout while attempting", + "switch configuration operation failed", self.switch.save_configuration, mock_connection) self.assertGreater(mock_connection.commit.call_count, 1) @@ -277,7 +277,7 @@ def test_save_configuration_warn_does_not_exist_timeout( "Commit failed with the following errors:\n\n{0}".format(output)) self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, - "Reached timeout while attempting", + "switch configuration operation failed", self.switch.save_configuration, mock_connection) self.assertGreater(mock_connection.commit.call_count, 1) @@ -299,7 +299,7 @@ def test_save_configuration_error(self): "Commit failed with the following errors:\n\n{0}".format(output)) self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, - "Failed to commit configuration", + "switch configuration operation failed", self.switch.save_configuration, mock_connection) mock_connection.commit.assert_called_once_with() 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 5223bdcd..0f78b17f 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py +++ b/networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py @@ -54,7 +54,7 @@ def test_batch(self): def test_batch_missing_backend_url(self): self.assertRaisesRegex( - Exception, "backend_url", + Exception, "switch configuration operation failed", self._make_switch_device, {'ngs_batch_requests': True}) @mock.patch('networking_generic_switch.devices.netmiko_devices.' @@ -445,7 +445,6 @@ def test_check_output_error(self): fake switch command fake error message """ - msg = ("Found invalid configuration in device response. Operation: " - "fake op. Output: %s" % output) - self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, msg, + self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, + "switch configuration operation failed", self.switch.check_output, output, 'fake op') diff --git a/releasenotes/notes/fix-netmiko-info-leakage-423c4c59b924c06f.yaml b/releasenotes/notes/fix-netmiko-info-leakage-423c4c59b924c06f.yaml new file mode 100644 index 00000000..3066013a --- /dev/null +++ b/releasenotes/notes/fix-netmiko-info-leakage-423c4c59b924c06f.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes a security vulnerability where sensitive information (IP addresses, + usernames, hostname and file paths) were leaked in error messages when a + connection to a network device fails. Error messages are now properly + sanitized to redact these sensitive details while preserving meaningful + information for troubleshooting.