From 1c61066a473bf2ce470f92f7d29661c93ad291b9 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Wed, 21 May 2025 08:47:43 -0700 Subject: [PATCH 1/6] ci: workaround neutron's move to uwsgi only When neutron changed to using uwsgi only, it automatically excluded configuration parameters needed to supply configuration to launch ML2 plugins in the standard style of devstack, because the pattern only allowed for auto-configuration which only loads two files, or from a files in a single directory pattern, or a list of files supplied by environment variable. Ideally, we'd be able to restart neutron as well. We can't due to when things happen upstream; the post-config phase is not always late enough since neutron does some configuration there, the extra phase is too late as Ironic will be setting up nodes there. For now, we're relying on a race condition that we reliably win. Change-Id: I5cfd1fc247043ede9b71252ee681ce2ca413ede6 Signed-off-by: Jay Faulkner --- devstack/plugin.sh | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 24b25e97..85bd6bbf 100644 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -164,8 +164,13 @@ function configure_generic_switch { done fi fi + # NOTE(TheJulia): This is not respected presently with uwsgi launched + # neutron as it auto-identifies it's configuration files. neutron_server_config_add $GENERIC_SWITCH_INI_FILE + if [ -f /etc/neutron/neutron-api-uwsgi.ini ]; then + iniset -sudo /etc/neutron/neutron-api-uwsgi.ini uwsgi env OS_NEUTRON_CONFIG_FILES='/etc/neutron/neutron.conf;/etc/neutron/plugins/ml2/ml2_conf.ini;/etc/neutron/plugins/ml2/ml2_conf_genericswitch.ini' + fi } function add_generic_switch_to_ml2_config { @@ -241,6 +246,26 @@ function ngs_configure_tempest { fi } +function ngs_restart_neutron { + echo_summary "NGS doing required neutron restart. Stopping neutron." + # NOTE(JayF) In practice restarting OVN causes problems, I'm not sure why. + # This avoids the restart. + local existing_skip_stop_ovn + SKIP_STOP_OVN=True + # We are changing the base config, and need ot restart the neutron services + stop_neutron + # NOTE(JayF): Neutron services are initialized in a particular order, this appears to + # match that order as currently defined in stack.sh (2025-05-22). + # TODO(JayF): Introduce a function in upstream devstack that documents this order so + # ironic won't break anytime initialization steps are rearranged. + echo_summary "NGS starting neutron service" + start_neutron_service_and_check + echo_summary "NGS started neutron service, now launch neutron agents" + start_neutron + echo_summary "NGS required neutron restart completed." + SKIP_STOP_OVN=False +} + # check for service enabled if is_service_enabled generic_switch; then @@ -250,7 +275,7 @@ if is_service_enabled generic_switch; then install_generic_switch elif [[ "$1" == "stack" && "$2" == "post-config" ]]; then - # Configure after the other layer 1 and 2 services have been configured + # Configure after the other layer 1 and 2 services have been started echo_summary "Configuring Generic_switch ML2" # Source ml2 plugin, set default config @@ -262,7 +287,22 @@ if is_service_enabled generic_switch; then Q_PLUGIN_CLASS="ml2" fi + # TODO(JayF): This currently relies on winning a race, as many of the + # files modified by this method are created during this + # phase. In practice it works, but moving forward we likely + # need a supported-by-devstack/neutron-upstream method to + # ensure this is done at the right moment. configure_generic_switch + + if is_service_enabled neutron; then + # TODO(JayF): Similarly, we'd like to restart neutron to ensure + # our config changes have taken effect; we can't do + # that reliably here because it may not be fully + # configured, and extra phase is too late. + echo_summary "Skipping ngs_restart_neutron" + #ngs_restart_neutron + fi + elif [[ "$1" == "stack" && "$2" == "test-config" ]]; then if is_service_enabled tempest; then echo_summary "Configuring Tempest NGS" From c77674475b89f549210ab19e3b8a5499e2533629 Mon Sep 17 00:00:00 2001 From: Jay Faulkner Date: Tue, 27 May 2025 12:55:59 -0700 Subject: [PATCH 2/6] Make devstack runs slightly more deterministic This eliminates one of the two implied ordering things we're relying on. We should now be OK if networking_generic_switch is configured before neutron. We are still relying on implied behavior of neutron services restarting sometime after our work in the post-config phase. Change-Id: I5a763b74ea349a74c66feca38ded46cda91d82a4 --- devstack/plugin.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 85bd6bbf..226c48cd 100644 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -168,9 +168,10 @@ function configure_generic_switch { # neutron as it auto-identifies it's configuration files. neutron_server_config_add $GENERIC_SWITCH_INI_FILE - if [ -f /etc/neutron/neutron-api-uwsgi.ini ]; then - iniset -sudo /etc/neutron/neutron-api-uwsgi.ini uwsgi env OS_NEUTRON_CONFIG_FILES='/etc/neutron/neutron.conf;/etc/neutron/plugins/ml2/ml2_conf.ini;/etc/neutron/plugins/ml2/ml2_conf_genericswitch.ini' - fi + # NOTE(JayF): It's possible in some rare cases this config doesn't exist + # if so, no big deal, iniset is used in devstack and should + # not lose our changes. See `write_uwsgi_config` in lib/apache + iniset -sudo /etc/neutron/neutron-api-uwsgi.ini uwsgi env OS_NEUTRON_CONFIG_FILES='/etc/neutron/neutron.conf;/etc/neutron/plugins/ml2/ml2_conf.ini;/etc/neutron/plugins/ml2/ml2_conf_genericswitch.ini' } function add_generic_switch_to_ml2_config { From ae002a3839619159665557c1d71d992611b50446 Mon Sep 17 00:00:00 2001 From: Jay Faulkner Date: Tue, 27 May 2025 12:45:31 -0700 Subject: [PATCH 3/6] Remove explicit use of eventlet This removes explicit eventlet usages in NGS, enabling us to be compatible when neutron is done with their migration. Generated-By: claude-code Change-Id: I4ec319a19e8c27689babeef023eddb4f6e1dd918 --- networking_generic_switch/batching.py | 20 ++++++++++---------- requirements.txt | 2 +- tools/ngs-stress/ngs_stress.py | 3 --- tox.ini | 1 - 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/networking_generic_switch/batching.py b/networking_generic_switch/batching.py index 5ec3c8f2..f4d2dc10 100644 --- a/networking_generic_switch/batching.py +++ b/networking_generic_switch/batching.py @@ -14,14 +14,15 @@ import atexit import json +import threading import etcd3gw from etcd3gw import exceptions as etcd3gw_exc from etcd3gw.utils import _decode from etcd3gw.utils import _encode from etcd3gw.utils import _increment_last_byte -import eventlet from oslo_log import log as logging +from oslo_service import threadgroup from oslo_utils import netutils from oslo_utils import uuidutils import tenacity @@ -32,7 +33,7 @@ LOG = logging.getLogger(__name__) -THREAD_POOL = eventlet.greenpool.GreenPool() +THREAD_POOL = threadgroup.ThreadGroup() class ShutdownTimeout(Exception): @@ -48,12 +49,12 @@ def _wait_for_threads(): and performing switch configuration operations which should not be interrupted. """ + active_threads = len(THREAD_POOL.threads) LOG.info("Waiting %d seconds for %d threads to complete", - SHUTDOWN_TIMEOUT, THREAD_POOL.running()) + SHUTDOWN_TIMEOUT, active_threads) try: - with eventlet.Timeout(SHUTDOWN_TIMEOUT, ShutdownTimeout): - THREAD_POOL.waitall() - except ShutdownTimeout: + THREAD_POOL.stop(graceful=True, timeout=SHUTDOWN_TIMEOUT) + except Exception: LOG.error("Timed out waiting for threads to complete") else: LOG.info("Finished waiting for threads to complete") @@ -365,13 +366,12 @@ def do_work(): @staticmethod def _spawn(work_fn): - # TODO(johngarbutt) remove hard eventlet dependency - # in a similar way to etcd3gw # Sleep to let possible other work to batch together - eventlet.sleep(0) + # This works with both eventlet and native threading + threading.Event().wait(0.001) # Run all pending tasks, which might be a no op # if pending tasks already ran - THREAD_POOL.spawn_n(work_fn) + THREAD_POOL.add_thread(work_fn) def _execute_pending_batches(self, device, item): """Execute all batches currently registered. diff --git a/requirements.txt b/requirements.txt index 3d8dc11f..913556ec 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ # date but we do not test them so no guarantee of having them all correct. If # you find any incorrect lower bounds, let us know or propose a fix. etcd3gw>=2.1.0 # Apache-2.0 -eventlet>=0.18.2 # Apache-2.0 +oslo.service[threading]>=4.2.0 # Apache-2.0 stevedore>=1.20.0 # Apache-2.0 netmiko>=4.1.1 # MIT neutron>=13.0.0.0b1 # Apache-2.0 diff --git a/tools/ngs-stress/ngs_stress.py b/tools/ngs-stress/ngs_stress.py index 95af461e..85d4d933 100644 --- a/tools/ngs-stress/ngs_stress.py +++ b/tools/ngs-stress/ngs_stress.py @@ -18,15 +18,12 @@ import threading import uuid -import eventlet from neutron_lib.utils import net from oslo_config import cfg import oslo_log.log as logging import networking_generic_switch.generic_switch_mech as generic_switch -eventlet.monkey_patch() - CONF = cfg.CONF LOG = logging.getLogger(__name__) diff --git a/tox.ini b/tox.ini index 0b46a5af..24e913b9 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,6 @@ envlist = py3,pep8 ignore_basepython_conflict=true [testenv] -constrain_package_deps = true usedevelop = True setenv = VIRTUAL_ENV={envdir} PYTHONDONTWRITEBYTECODE = 1 From 1b58a37bc735893023002c4898caf8e8e1cbf9cc Mon Sep 17 00:00:00 2001 From: mumesan Date: Tue, 27 May 2025 14:20:28 +0100 Subject: [PATCH 4/6] Improve NGS documentation. Some minor nitpicks and changes Change-Id: I593a0c44b4ad3638875bc6959a3aea8feebdba0e --- doc/source/dev/dev-quickstart.rst | 121 ++++++++------- networking_generic_switch/devices/__init__.py | 8 +- .../devices/netmiko_devices/cumulus.py | 141 +++++++++--------- .../devices/netmiko_devices/smc.py | 4 +- .../devices/netmiko_devices/sonic.py | 54 +++---- 5 files changed, 173 insertions(+), 155 deletions(-) diff --git a/doc/source/dev/dev-quickstart.rst b/doc/source/dev/dev-quickstart.rst index 368a4f50..bdd4ef47 100644 --- a/doc/source/dev/dev-quickstart.rst +++ b/doc/source/dev/dev-quickstart.rst @@ -34,33 +34,36 @@ Switch to the stack user and clone DevStack:: git clone https://github.com/openstack-dev/devstack.git devstack Create devstack/local.conf with minimal settings required to enable -Networking-generic-switch. Here is and example of local.conf:: - - [[local|localrc]] - # Set credentials - ADMIN_PASSWORD=secrete - DATABASE_PASSWORD=secrete - RABBIT_PASSWORD=secrete - SERVICE_PASSWORD=secrete - SERVICE_TOKEN=secrete - - # Enable minimal required services - ENABLED_SERVICES="dstat,mysql,rabbit,key,q-svc,q-agt,q-dhcp" - - # Enable networking-generic-switch plugin - enable_plugin networking-generic-switch https://review.openstack.org/openstack/networking-generic-switch - - # Configure Neutron - OVS_PHYSICAL_BRIDGE=brbm - PHYSICAL_NETWORK=mynetwork - Q_PLUGIN=ml2 - ENABLE_TENANT_VLANS=True - Q_ML2_TENANT_NETWORK_TYPE=vlan - TENANT_VLAN_RANGE=100:150 - - # Configure logging - LOGFILE=$HOME/devstack.log - LOGDIR=$HOME/logs +Networking-generic-switch. Here is an example of local.conf: + + + .. code-block:: ini + + [[local|localrc]] + # Set credentials + ADMIN_PASSWORD=secrete + DATABASE_PASSWORD=secrete + RABBIT_PASSWORD=secrete + SERVICE_PASSWORD=secrete + SERVICE_TOKEN=secrete + + # Enable minimal required services + ENABLED_SERVICES="dstat,mysql,rabbit,key,q-svc,q-agt,q-dhcp" + + # Enable networking-generic-switch plugin + enable_plugin networking-generic-switch https://review.openstack.org/openstack/networking-generic-switch + + # Configure Neutron + OVS_PHYSICAL_BRIDGE=brbm + PHYSICAL_NETWORK=mynetwork + Q_PLUGIN=ml2 + ENABLE_TENANT_VLANS=True + Q_ML2_TENANT_NETWORK_TYPE=vlan + TENANT_VLAN_RANGE=100:150 + + # Configure logging + LOGFILE=$HOME/devstack.log + LOGDIR=$HOME/logs Run stack.sh:: @@ -86,45 +89,53 @@ Test with real hardware Add information about hardware switch to Networking-generic-switch config ``/etc/neutron/plugins/ml2/ml2_conf_genericswitch.ini`` and -restart Neutron server:: +restart Neutron server: + + .. code-block:: ini - [genericswitch:cisco_switch_1] - device_type = netmiko_cisco_ios - ip = 1.2.3.4 - username = cisco - password = cisco - secret = enable_password + [genericswitch:cisco_switch_1] + device_type = netmiko_cisco_ios + ip = 1.2.3.4 + username = cisco + password = cisco + secret = enable_password Get current configuration of the port on the switch, for example for -Cisco IOS device:: +Cisco IOS device: + + .. code-block:: ini - sh running-config int gig 0/12 - Building configuration... + sh running-config int gig 0/12 + Building configuration... - Current configuration : 283 bytes - ! - interface GigabitEthernet0/12 - switchport mode access - end + Current configuration : 283 bytes + ! + interface GigabitEthernet0/12 + switchport mode access + end Run exercise.py to create/update Neutron port. It will print VLAN id to be -assigned:: +assigned: - $ neutron net-create test - $ python ~/networking-generic-switch/devstack/exercise.py --switch_name cisco_switch_1 --port Gig0/12 --switch_id=06:58:1f:e7:b4:44 --network test - 126 + .. code-block:: ini + + $ neutron net-create test + $ python ~/networking-generic-switch/devstack/exercise.py --switch_name cisco_switch_1 --port Gig0/12 --switch_id=06:58:1f:e7:b4:44 --network test + 126 Verify that VLAN has been changed on the switch port, for example for -Cisco IOS device:: +Cisco IOS device: + + .. code-block:: ini - sh running-config int gig 0/12 - Building configuration... + sh running-config int gig 0/12 + Building configuration... - Current configuration : 311 bytes - ! - interface GigabitEthernet0/12 - switchport access vlan 126 - switchport mode access - end + Current configuration : 311 bytes + ! + interface GigabitEthernet0/12 + switchport access vlan 126 + switchport mode access + end diff --git a/networking_generic_switch/devices/__init__.py b/networking_generic_switch/devices/__init__.py index 0b6f2cfb..68209cdf 100644 --- a/networking_generic_switch/devices/__init__.py +++ b/networking_generic_switch/devices/__init__.py @@ -235,7 +235,7 @@ 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 port_id: The name of the switch interface :param segmentation_id: VLAN identifier of the network used as access or native VLAN for port. @@ -247,7 +247,7 @@ def plug_port_to_network(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 port_id: The name of the switch interface :param segmentation_id: VLAN identifier of the network used as access or native VLAN for port. @@ -259,7 +259,7 @@ 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 port_id: The name of the switch interface :param segmentation_id: VLAN identifier of the network used as access or native VLAN for port. @@ -275,7 +275,7 @@ 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 port_id: The name of the switch interface :param segmentation_id: VLAN identifier of the network used as access or native VLAN for port. diff --git a/networking_generic_switch/devices/netmiko_devices/cumulus.py b/networking_generic_switch/devices/netmiko_devices/cumulus.py index ce965d2a..895d02df 100644 --- a/networking_generic_switch/devices/netmiko_devices/cumulus.py +++ b/networking_generic_switch/devices/netmiko_devices/cumulus.py @@ -22,71 +22,73 @@ class Cumulus(netmiko_devices.NetmikoSwitch): Note for this switch you want config like this, where secret is the password needed for sudo su: - [genericswitch:] - device_type = netmiko_cumulus - ip = - username = - password = - secret = - ngs_physical_networks = physnet1 - ngs_max_connections = 1 - ngs_port_default_vlan = 123 - ngs_disable_inactive_ports = False + .. code-block:: ini + + [genericswitch:] + device_type = netmiko_cumulus + ip = + username = + password = + secret = + ngs_physical_networks = physnet1 + ngs_max_connections = 1 + ngs_port_default_vlan = 123 + ngs_disable_inactive_ports = False """ NETMIKO_DEVICE_TYPE = "linux" - ADD_NETWORK = [ + ADD_NETWORK = ( 'net add vlan {segmentation_id}', - ] + ) - DELETE_NETWORK = [ + DELETE_NETWORK = ( 'net del vlan {segmentation_id}', - ] + ) - PLUG_PORT_TO_NETWORK = [ + PLUG_PORT_TO_NETWORK = ( 'net add interface {port} bridge access {segmentation_id}', - ] + ) - DELETE_PORT = [ + DELETE_PORT = ( 'net del interface {port} bridge access {segmentation_id}', - ] + ) - PLUG_BOND_TO_NETWORK = [ + PLUG_BOND_TO_NETWORK = ( 'net add bond {bond} bridge access {segmentation_id}', - ] + ) - UNPLUG_BOND_FROM_NETWORK = [ + UNPLUG_BOND_FROM_NETWORK = ( 'net del bond {bond} bridge access {segmentation_id}', - ] + ) - ENABLE_PORT = [ + ENABLE_PORT = ( 'net del interface {port} link down', - ] + ) - DISABLE_PORT = [ + DISABLE_PORT = ( 'net add interface {port} link down', - ] + ) - ENABLE_BOND = [ + ENABLE_BOND = ( 'net del bond {bond} link down', - ] + ) - DISABLE_BOND = [ + DISABLE_BOND = ( 'net add bond {bond} link down', - ] + ) - SAVE_CONFIGURATION = [ + SAVE_CONFIGURATION = ( 'net commit', - ] + ) - ERROR_MSG_PATTERNS = [ + ERROR_MSG_PATTERNS = ( # Its tempting to add this error message, but as only one # bridge-access is allowed, we ignore that error for now: # re.compile(r'configuration does not have "bridge-access') re.compile(r'ERROR: Command not found.'), re.compile(r'command not found'), re.compile(r'is not a physical interface on this switch'), - ] + ) class CumulusNVUE(netmiko_devices.NetmikoSwitch): @@ -95,44 +97,47 @@ class CumulusNVUE(netmiko_devices.NetmikoSwitch): Note for this switch you want config like this, where secret is the password needed for sudo su: - [genericswitch:] - device_type = netmiko_cumulus_nvue - ip = - username = - password = - secret = - ngs_physical_networks = physnet1 - ngs_max_connections = 1 - ngs_port_default_vlan = 123 - ngs_disable_inactive_ports = False + .. code-block:: ini + + [genericswitch:] + device_type = netmiko_cumulus_nvue + ip = + username = + password = + secret = + ngs_physical_networks = physnet1 + ngs_max_connections = 1 + ngs_port_default_vlan = 123 + ngs_disable_inactive_ports = False + """ NETMIKO_DEVICE_TYPE = "linux" - ADD_NETWORK = [ + ADD_NETWORK = ( 'nv set bridge domain br_default vlan {segmentation_id}', - ] + ) - DELETE_NETWORK = [ + DELETE_NETWORK = ( 'nv unset bridge domain br_default vlan {segmentation_id}', - ] + ) - PLUG_PORT_TO_NETWORK = [ + 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 = [ + 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 = [ + 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 ' @@ -144,21 +149,21 @@ class CumulusNVUE(netmiko_devices.NetmikoSwitch): '{segmentation_id}', ) - SET_NATIVE_VLAN = [ + 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 = [ + 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 ' @@ -174,25 +179,25 @@ class CumulusNVUE(netmiko_devices.NetmikoSwitch): '{segmentation_id}', ) - DELETE_PORT = [ + 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 = [ + ENABLE_PORT = ( 'nv set interface {port} link state up', - ] + ) - DISABLE_PORT = [ + DISABLE_PORT = ( 'nv set interface {port} link state down', - ] + ) - SAVE_CONFIGURATION = [ + SAVE_CONFIGURATION = ( 'nv config save', - ] + ) - ERROR_MSG_PATTERNS = [ + ERROR_MSG_PATTERNS = ( # Its tempting to add this error message, but as only one # bridge-access is allowed, we ignore that error for now: # re.compile(r'configuration does not have "bridge-access') @@ -204,7 +209,7 @@ class CumulusNVUE(netmiko_devices.NetmikoSwitch): re.compile(r'Error: Invalid parameter'), re.compile(r'Unable to restart services'), re.compile(r'Failure during apply'), - ] + ) def send_config_set(self, net_connect, cmd_set): """Send a set of configuration lines to the device. diff --git a/networking_generic_switch/devices/netmiko_devices/smc.py b/networking_generic_switch/devices/netmiko_devices/smc.py index 12365082..e0174a7d 100644 --- a/networking_generic_switch/devices/netmiko_devices/smc.py +++ b/networking_generic_switch/devices/netmiko_devices/smc.py @@ -16,8 +16,8 @@ class SupermicroSmis(netmiko_devices.NetmikoSwitch): - """A class to represent a Supermicro SMIS switch""" - """ + """A class to represent a Supermicro SMIS switch + Inherits from: -------------- netmiko_devices.NetmikoSwitch diff --git a/networking_generic_switch/devices/netmiko_devices/sonic.py b/networking_generic_switch/devices/netmiko_devices/sonic.py index 7a78c80a..96f02565 100644 --- a/networking_generic_switch/devices/netmiko_devices/sonic.py +++ b/networking_generic_switch/devices/netmiko_devices/sonic.py @@ -22,53 +22,55 @@ class Sonic(netmiko_devices.NetmikoSwitch): Note for this switch you want config like this, where secret is the password needed for sudo su: - [genericswitch:] - device_type = netmiko_sonic - ip = - username = - password = - secret = - ngs_physical_networks = physnet1 - ngs_max_connections = 1 - ngs_port_default_vlan = 123 - ngs_disable_inactive_ports = False + .. code-block:: ini + + [genericswitch:] + device_type = netmiko_sonic + ip = + username = + password = + secret = + ngs_physical_networks = physnet1 + ngs_max_connections = 1 + ngs_port_default_vlan = 123 + ngs_disable_inactive_ports = False """ NETMIKO_DEVICE_TYPE = "linux" - ADD_NETWORK = [ + ADD_NETWORK = ( 'config vlan add {segmentation_id}', - ] + ) - DELETE_NETWORK = [ + DELETE_NETWORK = ( 'config vlan del {segmentation_id}', - ] + ) - PLUG_PORT_TO_NETWORK = [ + PLUG_PORT_TO_NETWORK = ( 'config vlan member add -u {segmentation_id} {port}', - ] + ) - DELETE_PORT = [ + DELETE_PORT = ( 'config vlan member del {segmentation_id} {port}', - ] + ) - ADD_NETWORK_TO_TRUNK = [ + ADD_NETWORK_TO_TRUNK = ( 'config vlan member add {segmentation_id} {port}', - ] + ) - REMOVE_NETWORK_FROM_TRUNK = [ + REMOVE_NETWORK_FROM_TRUNK = ( 'config vlan member del {segmentation_id} {port}', - ] + ) - SAVE_CONFIGURATION = [ + SAVE_CONFIGURATION = ( 'config save -y', - ] + ) - ERROR_MSG_PATTERNS = [ + ERROR_MSG_PATTERNS = ( re.compile(r'VLAN[0-9]+ doesn\'t exist'), re.compile(r'Invalid Vlan Id , Valid Range : 1 to 4094'), re.compile(r'Interface name is invalid!!'), re.compile(r'No such command'), - ] + ) def send_config_set(self, net_connect, cmd_set): """Send a set of configuration lines to the device. From ac24e71a8767156b2ff6cdd5c993eb2d63ee2082 Mon Sep 17 00:00:00 2001 From: mumesan Date: Tue, 3 Jun 2025 15:24:06 +0100 Subject: [PATCH 5/6] Improve Netmiko Device Commands Documentation Adds a Sphinx directive to parse each file in the netmiko devices folder and return documentation containing the switch, command modules capable of being executed by the switch, and the CLI commands sent to the switch when the command module is selected. Change-Id: Ie49b158be44d517c82b8f6647096fd8ec6cc903c Signed-off-by: mumesan --- .gitignore | 2 +- doc/source/_exts/netmiko_device_commands.py | 150 ++++++++++++++++++ doc/source/conf.py | 5 +- doc/source/index.rst | 1 + doc/source/netmiko-device-commands.rst | 9 ++ .../devices/netmiko_devices/smc.py | 28 +--- 6 files changed, 166 insertions(+), 29 deletions(-) create mode 100644 doc/source/_exts/netmiko_device_commands.py create mode 100644 doc/source/netmiko-device-commands.rst diff --git a/.gitignore b/.gitignore index cca74a74..8bac9558 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,7 @@ # Sphinx _build -doc/source/api/ +doc/source/contributor/api/ # release notes build releasenotes/build diff --git a/doc/source/_exts/netmiko_device_commands.py b/doc/source/_exts/netmiko_device_commands.py new file mode 100644 index 00000000..56c708fb --- /dev/null +++ b/doc/source/_exts/netmiko_device_commands.py @@ -0,0 +1,150 @@ +import ast +import inspect +import stevedore + +from docutils import nodes +from docutils.parsers import rst +from docutils.statemachine import ViewList +from sphinx.util.nodes import nested_parse_with_titles + +command_descriptions = { + 'ADD_NETWORK': 'A tuple of command strings used to add a VLAN', + 'DELETE_NETWORK': 'A tuple of command strings used to delete a VLAN', + 'PLUG_PORT_TO_NETWORK': 'A tuple of command strings used to configure a \ + port to connect to a specific VLAN', + 'DELETE_PORT': 'A tuple of command strings used to remove a port from the\ + VLAN', + 'NETMIKO_DEVICE_TYPE': 'Netmiko Supported device type', + 'ADD_NETWORK_TO_TRUNK': 'Adds a network to a trunk port.', + 'REMOVE_NETWORK_FROM_TRUNK': 'Removes a network from a trunk port.', + 'ENABLE_PORT': 'Enables the port', + 'DISABLE_PORT': 'Shuts down the port', + 'ERROR_MSG_PATTERNS': 'A tuple of regular expressions. These patterns are\ + used to match and handle error messages returned by the switch.', + 'SET_NATIVE_VLAN': 'Sets a specified native VLAN', + 'DELETE_NATIVE_VLAN': 'Removes the native VLAN', + 'SAVE_CONFIGURATION': 'Saves the configuration', + 'SET_NATIVE_VLAN_BOND': 'Sets the native VLAN for the bond interface', + 'DELETE_NATIVE_VLAN_BOND': 'Unsets the native VLAN for the bond \ + interface', + 'ADD_NETWORK_TO_BOND_TRUNK': 'Adds a VLAN to the bond interface for \ + trunking', + 'DELETE_NETWORK_ON_BOND_TRUNK': 'Removes a VLAN from the bond interface \ + for trunking', + 'PLUG_PORT_TO_NETWORK_GENERAL': 'Allows the VLAN and lets it carry \ + untagged frames', + 'DELETE_PORT_GENERAL': 'Removes VLAN from allowed list and stops allowing\ + it to carry untagged frames', + 'QUERY_PORT': 'Shows details about the switch for that port', + 'PLUG_BOND_TO_NETWORK': 'Adds bond to the bridge as a port for the VLAN', + 'UNPLUG_BOND_FROM_NETWORK': 'Removes bond\'s access VLAN assignment', + 'ENABLE_BOND': 'Enables bond interface by removing link down state', + 'DISABLE_BOND': 'Disables bond interface by setting its link state to \ + down', +} + + +class DeviceCommandsDirective(rst.Directive): + + def parse_tuples(value): + """Parses the value in the tuples and returns a list of its contents""" + tuple_values = [] + for elt in value.elts: + # Parsing if the item in the tuple is a function call + if isinstance(elt, ast.Call): + func_name = '' + if isinstance(elt.func, ast.Attribute): + func_name = f"{ast.unparse(elt.func.value)}.{elt.func.attr}" + elif isinstance(elt.func, ast.Name): + func_name = elt.func.id + args = [ast.literal_eval(arg) for arg in elt.args] + call_command = str(func_name) + '(\'' + str(args[0]) + '\')' + tuple_values.append(call_command) + + else: + tuple_values.append(ast.literal_eval(elt)) + return tuple_values + + def parse_file(file_content, filename): + """Uses ast to split document body into nodes and parse them""" + tree = ast.parse(file_content, filename=filename) + classes = {} + for node in tree.body: + if isinstance(node, ast.ClassDef): + device_name = node.name + cli_commands = {} + docstring = ast.get_docstring(node) + + if docstring: + cli_commands['__doc__'] = docstring + # Iterates through nodes, checks for type of node and extracts the value + for subnode in node.body: + if isinstance(subnode, ast.Assign): + for target in subnode.targets: + command_name = target.id + if isinstance(target, ast.Name): + ast_type = subnode.value + if isinstance(ast_type, ast.Tuple): + cli_commands[command_name] = DeviceCommandsDirective.parse_tuples(ast_type) + else: + cli_commands[command_name] = ast.literal_eval(ast_type) + if cli_commands: + classes[device_name] = cli_commands + return classes + + def format_output(switch_details): + """Formats output that is to be displayed""" + formatted_output = ViewList() + if '__doc__' in switch_details: + for line in switch_details['__doc__'].splitlines(): + formatted_output.append(f" {line}", "") + formatted_output.append("", "") + del switch_details['__doc__'] + for command_name, cli_commands in switch_details.items(): + desc = command_descriptions.get(command_name, 'No description provided') + formatted_output.append(f" - {command_name}: {desc}", "") + formatted_output.append(f" - CLI commands:", "") + if isinstance(cli_commands, list): + if cli_commands: + for command in cli_commands: + formatted_output.append(f" - {command}", "") + else: + formatted_output.append(f" - No cli commands for this switch command", "") + else: + formatted_output.append(f" - {cli_commands}", "") + return formatted_output + + def run(self): + """Loads the files, parses them and formats the output""" + manager = stevedore.ExtensionManager( + namespace='generic_switch.devices', + invoke_on_load=False, + ) + output_lines = ViewList() + output_lines.append("Switches", "") + output_lines.append("========", "") + + for file_loader in manager.extensions: + switch = file_loader.plugin + module = inspect.getmodule(switch) + file_content = inspect.getsource(module) + filename = module.__file__ + parsed_device_file = DeviceCommandsDirective.parse_file(file_content, filename) + switch_name = switch.__name__ + output_lines.append(f"{switch_name}:", "") + subheading_characters = "^" + subheading = subheading_characters * (len(switch_name) + 1) + output_lines.append(subheading, "") + + if switch_name in parsed_device_file: + switch_details = parsed_device_file[switch_name] + output_lines.extend(DeviceCommandsDirective.format_output(switch_details)) + output_lines.append("", "") + + node = nodes.section() + node.document = self.state.document + nested_parse_with_titles(self.state, output_lines, node) + return node.children + +def setup(app): + app.add_directive('netmiko-device-commands', DeviceCommandsDirective) diff --git a/doc/source/conf.py b/doc/source/conf.py index a311f66d..74d0c958 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -16,13 +16,15 @@ import sys sys.path.insert(0, os.path.abspath('../..')) +sys.path.insert(0, os.path.join(os.path.abspath('.'), '_exts')) # -- General configuration ---------------------------------------------------- # Add any Sphinx extension module names here, as strings. They can be # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom ones. extensions = [ 'sphinxcontrib.apidoc', - 'openstackdocstheme' + 'openstackdocstheme', + 'netmiko_device_commands' ] # openstackdocstheme options @@ -85,4 +87,5 @@ apidoc_output_dir = 'contributor/api' apidoc_excluded_paths = [ 'tests', + 'devices/netmiko_devices', ] diff --git a/doc/source/index.rst b/doc/source/index.rst index 0de4200d..50097295 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -9,6 +9,7 @@ Welcome to networking-generic-switch's documentation! configuration dev/dev-quickstart contributing + netmiko-device-commands contributor/api/modules Indices and tables diff --git a/doc/source/netmiko-device-commands.rst b/doc/source/netmiko-device-commands.rst new file mode 100644 index 00000000..9d597827 --- /dev/null +++ b/doc/source/netmiko-device-commands.rst @@ -0,0 +1,9 @@ +======================= +Netmiko Device Commands +======================= + +This section contains details of the commands capable of being executed +by the switches and the CLI commands sent to the switch for each command +module that is selected. + +.. netmiko-device-commands:: diff --git a/networking_generic_switch/devices/netmiko_devices/smc.py b/networking_generic_switch/devices/netmiko_devices/smc.py index e0174a7d..8c0b43d4 100644 --- a/networking_generic_switch/devices/netmiko_devices/smc.py +++ b/networking_generic_switch/devices/netmiko_devices/smc.py @@ -16,34 +16,8 @@ class SupermicroSmis(netmiko_devices.NetmikoSwitch): - """A class to represent a Supermicro SMIS switch + """A class to represent a Supermicro SMIS switch.""" - Inherits from: - -------------- - netmiko_devices.NetmikoSwitch - - Class Attributes: - ----------------- - ADD_NETWORK : tuple - A tuple of command strings used to add a VLAN - with a specific segmentation ID - and name the VLAN. - - DELETE_NETWORK : tuple - A tuple of command strings used to delete a VLAN - by its segmentation ID. - - PLUG_PORT_TO_NETWORK : tuple - A tuple of command strings used to configure a port - to connect to a specific VLAN. - This sets the port to access mode and assigns it - to the specified VLAN. - - DELETE_PORT : tuple - A tuple of command strings used to remove a port - from the VLAN. This removes - any trunking configuration and clears VLAN assignments. - """ ADD_NETWORK = ( 'vlan {segmentation_id}', 'name {network_name}', From d26adb822d2cd006e9fabe89a31359e416ee7bce Mon Sep 17 00:00:00 2001 From: Afonne-CID Date: Tue, 3 Jun 2025 23:14:10 +0100 Subject: [PATCH 6/6] Cast numeric Netmiko kwargs to native types. Convert port/timeout options from str to int/float, fixing type errors raised by ConnectHandler. Closes-Bug: #2110111 Change-Id: I9a4a1f857d2ace7c87cfbd6ed56356c1ed227caf Signed-off-by: Afonne-CID --- .../devices/netmiko_devices/__init__.py | 25 +++++++++++++++++++ .../tests/unit/test_devices.py | 21 ++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/networking_generic_switch/devices/netmiko_devices/__init__.py b/networking_generic_switch/devices/netmiko_devices/__init__.py index 5c372ef8..16a601d1 100644 --- a/networking_generic_switch/devices/netmiko_devices/__init__.py +++ b/networking_generic_switch/devices/netmiko_devices/__init__.py @@ -144,6 +144,31 @@ def __init__(self, device_cfg, *args, **kwargs): self.config['session_log_record_writes'] = True self.config['session_log_file_mode'] = 'append' + _NUMERIC_CAST = { + "port": int, + "global_delay_factor": float, + "conn_timeout": float, + "auth_timeout": float, + "banner_timeout": float, + "blocking_timeout": float, + "timeout": float, + "session_timeout": float, + "read_timeout_override": float, + "keepalive": int, + } + + for key, expected_type in _NUMERIC_CAST.items(): + value = self.config.get(key) + if isinstance(value, str): + try: + self.config[key] = expected_type(value) + except ValueError: + LOG.error( + "Invalid value %s for %s; expected %s", + value, key, expected_type.__name__, + ) + raise exc.GenericSwitchNetmikoConfigError() + self.lock_kwargs = { 'locks_pool_size': int(self.ngs_config['ngs_max_connections']), 'locks_prefix': self.config.get( diff --git a/networking_generic_switch/tests/unit/test_devices.py b/networking_generic_switch/tests/unit/test_devices.py index 4486c364..dc971007 100644 --- a/networking_generic_switch/tests/unit/test_devices.py +++ b/networking_generic_switch/tests/unit/test_devices.py @@ -267,3 +267,24 @@ def test__get_ssh_disabled_algorithms(self): "ciphers": ["blowfish-cbc", "3des-cbc"], } self.assertEqual(expected, algos) + + def test_float_params_cast(self): + config = { + "device_type": 'netmiko_ovs_linux', + "ip": "10.1.2.3", + "username": "u", + "password": "p", + "conn_timeout": "20.0", + "global_delay_factor": "2.5", + "port": "2222", + } + device = devices.device_manager(config) + + self.assertIsInstance(device.config["conn_timeout"], float) + self.assertEqual(device.config["conn_timeout"], 20.0) + + self.assertIsInstance(device.config["global_delay_factor"], float) + self.assertEqual(device.config["global_delay_factor"], 2.5) + + self.assertIsInstance(device.config["port"], int) + self.assertEqual(device.config["port"], 2222)