From 364ff937834f9ca9bc617f37f0cb72d2e266b9f4 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 13 Mar 2026 09:32:18 +0800 Subject: [PATCH] efactor(copilot): adopt Nornir best practice for multi-vendor device support Refactor Nornir configuration to use host-level connection_options instead of dynamic groups, following Nornir's configuration priority model (host > group > defaults). **Problem:** Previous implementation used first device's configuration for all devices, causing Cisco devices to use Huawei driver and vice versa. **Solution:** - Each host now has device-specific connection_options at host level - Single generic "network_devices" group for shared settings - Host-level config automatically overrides group-level config **Changes:** - Remove: _get_nornir_groups_config() helper function - Remove: _get_nornir_group() helper function - Simplify: _initialize_nornir() to use single generic group - Update: get_gns3_device_port.py() to return host-level config - Reserve: platform field for future NAPALM/scrapli plugin support **Benefits:** - Cleaner code structure (no dynamic group creation) - Follows Nornir best practice ("configuration proximity") - Easy to extend with new device types - Properly handles mixed-vendor topologies --- .../multi-vendor-device-support.md | 282 ++++++++++++++++-- .../tools_v2/config_tools_nornir.py | 114 ++----- .../tools_v2/display_tools_nornir.py | 114 ++----- .../utils/get_gns3_device_port.py | 29 +- 4 files changed, 320 insertions(+), 219 deletions(-) diff --git a/docs/gns3-copilot/implemented/multi-vendor-device-support.md b/docs/gns3-copilot/implemented/multi-vendor-device-support.md index c862a148b..77418955d 100644 --- a/docs/gns3-copilot/implemented/multi-vendor-device-support.md +++ b/docs/gns3-copilot/implemented/multi-vendor-device-support.md @@ -231,29 +231,191 @@ platform:huawei → Nornir platform (high-level) | Cisco IOS | `device_type:cisco_ios_telnet` | `platform:cisco_ios` | | Huawei CE | `device_type:huawei_telnet_ce` | `platform:huawei` | -### Automatic Group Generation +### Nornir Best Practice: Host-Level Connection Configuration -Nornir groups are dynamically generated based on device type: +The system uses **Nornir's configuration priority** (host > group > defaults) to handle multi-vendor environments efficiently: ```python # From get_gns3_device_port.py -if device_type and "_telnet" in device_type: - group_name = f"{platform}_telnet" # e.g., "huawei_telnet" -else: - group_name = platform # e.g., "cisco_ios" - hosts_data[device_name] = { "port": console_port, - "groups": [group_name], - "device_type": device_type, - "platform": platform, + "platform": platform, # Reserved for future use (NAPALM, scrapli) + "groups": ["network_devices"], # All devices share one group + "connection_options": { + "netmiko": { + "extras": {"device_type": device_type} # Device-specific driver + } + }, } ``` -**Why Dynamic Groups?** -- Single Nornir instance can handle multiple vendors -- Each device gets correct connection parameters -- Vendor-specific commands work automatically +**Why Host-Level `connection_options`?** +- ✅ Each device has its own `device_type` (host-level config) +- ✅ All devices share common settings via group inheritance (`hostname`, `timeout`) +- ✅ No need to dynamically create multiple groups for each device type +- ✅ Cleaner code structure - single generic group for all devices +- ✅ Follows Nornir best practice: "configuration proximity" + +**Configuration Priority:** +``` +Host Level (connection_options.device_type) + ↓ OVERRIDES +Group Level (hostname, timeout, username, password) + ↓ OVERRIDES +Defaults Level (data.location) +``` + +**Before (Old Approach - Dynamic Groups):** +```python +# Had to create multiple groups dynamically +groups = { + "cisco_ios_telnet": {"device_type": "cisco_ios_telnet", ...}, + "huawei_telnet": {"device_type": "huawei_telnet_ce", ...}, + "juniper_junos": {"device_type": "juniper_junos_telnet", ...}, +} +# Each host assigned to its vendor-specific group +``` + +**After (Current Approach - Host-Level Config):** +```python +# Single group for shared settings +groups = { + "network_devices": { + "hostname": "127.0.0.1", + "timeout": 120, + "username": "", + "password": "", + } +} +# Each host has device-specific connection_options +# Host config overrides group config automatically +``` + +## Architecture Evolution + +### Problem: Multi-Vendor Device Support + +**Initial Challenge:** +``` +Topology: Cisco R1 + Huawei SW1 + Juniper SRX + ↓ +Need: Different Netmiko drivers for each device + ↓ +Question: How to configure Nornir for multiple device types? +``` + +### Solution Evolution + +#### ❌ Approach 1: Single Group with First Device's Type (Initial Implementation) + +```python +# PROBLEM: Only uses first device's configuration +def _initialize_nornir(hosts_data): + first_device = next(iter(hosts_data.values())) + device_type = first_device["device_type"] # Only one type! + + return InitNornir( + inventory={ + "options": { + "hosts": hosts_data, # Has multiple device types + "groups": { + "network_devices": { + "connection_options": { + "netmiko": {"extras": {"device_type": device_type}} + } + } + } + } + } + ) +``` + +**Issue:** All devices use the first device's driver! +- Cisco R1 → Uses Huawei driver (if Huawei is first) ❌ +- Huawei SW1 → Uses Cisco driver (if Cisco is first) ❌ + +#### ❌ Approach 2: Dynamic Groups (Intermediate Solution) + +```python +# COMPLEX: Create multiple groups dynamically +groups = {} +for host_data in hosts_data.values(): + device_type = host_data["device_type"] + platform = host_data["platform"] + group_name = f"{platform}_telnet" # e.g., "huawei_telnet" + + if group_name not in groups: + groups[group_name] = { + "platform": platform, + "connection_options": { + "netmiko": {"extras": {"device_type": device_type}} + } + } + + host_data["groups"] = [group_name] +``` + +**Issues:** +- Complex logic to detect and create groups +- Code duplication in multiple files +- Had to delete helper functions (`_get_nornir_groups_config`, `_get_nornir_group`) +- Not following Nornir best practices + +#### ✅ Approach 3: Host-Level Configuration (Current - Best Practice) + +```python +# SIMPLE: Single group + host-level device_type +hosts_data[device_name] = { + "port": console_port, + "platform": platform, # Reserved for future use + "groups": ["network_devices"], # All devices in one group + "connection_options": { # Device-specific config + "netmiko": { + "extras": {"device_type": device_type} + } + } +} + +# Single generic group for shared settings +groups = { + "network_devices": { + "hostname": "127.0.0.1", + "timeout": 120, + "username": "", + "password": "", + } +} +``` + +**Advantages:** +- ✅ Clean, simple code +- ✅ Follows Nornir best practice (host > group > defaults) +- ✅ No dynamic group creation logic +- ✅ Each device's `connection_options` overrides group settings automatically +- ✅ Easy to extend with new device types + +### Configuration Priority Demonstration + +```python +# Host level (highest priority) +host["connection_options"]["netmiko"]["extras"]["device_type"] = "huawei_telnet_ce" + + ↓ OVERRIDES + +# Group level (middle priority) +group["hostname"] = "127.0.0.1" +group["timeout"] = 120 + + ↓ OVERRIDES + +# Defaults level (lowest priority) +defaults["data"]["location"] = "gns3" +``` + +**Result:** +- Each device uses its own `device_type` from host level +- All devices share `hostname`, `timeout` from group level +- All devices share `data.location` from defaults level ## Usage Examples @@ -314,33 +476,44 @@ from gns3server.agent.gns3_copilot.utils.custom_netmiko import huawei_ce huawei_ce.register_custom_device_type() # Initialize Nornir with mixed-vendor inventory +# Using host-level connection_options (best practice) inventory = { + "plugin": "DictInventory", "options": { "hosts": { "huawei-sw1": { "hostname": "127.0.0.1", "port": 5001, - "platform": "huawei", - "device_type": "huawei_telnet_ce", - "groups": ["huawei_telnet"] + "platform": "huawei", # Reserved for future use + "groups": ["network_devices"], + "connection_options": { + "netmiko": { + "extras": {"device_type": "huawei_telnet_ce"} + } + } }, "cisco-r1": { "hostname": "127.0.0.1", "port": 5002, "platform": "cisco_ios", - "device_type": "cisco_ios_telnet", - "groups": ["cisco_ios_telnet"] + "groups": ["network_devices"], + "connection_options": { + "netmiko": { + "extras": {"device_type": "cisco_ios_telnet"} + } + } } }, "groups": { - "huawei_telnet": { - "platform": "huawei", - "device_type": "huawei_telnet_ce" - }, - "cisco_ios_telnet": { - "platform": "cisco_ios", - "device_type": "cisco_ios_telnet" + "network_devices": { + "hostname": "127.0.0.1", # Shared by all devices + "timeout": 120, + "username": "", + "password": "", } + }, + "defaults": { + "data": {"location": "gns3"} } } } @@ -351,6 +524,8 @@ nr = InitNornir(inventory=inventory) result = nr.run(task=send_commands, commands=["display version"]) # Each device gets vendor-specific command handling +# huawei-sw1 uses huawei_telnet_ce driver +# cisco-r1 uses cisco_ios_telnet driver ``` ### GNS3 Copilot Tool Usage @@ -390,12 +565,22 @@ gns3server/agent/gns3_copilot/ │ │ └── tests/ # Unit tests │ │ ├── __init__.py │ │ └── test_huawei_ce.py # Huawei CE driver tests -│ └── get_gns3_device_port.py # Device port extraction +│ └── get_gns3_device_port.py # Device port extraction with host-level config ├── tools_v2/ │ ├── display_tools_nornir.py # Multi-vendor display commands +│ │ ├── _get_nornir_defaults() # Returns default Nornir config +│ │ └── _initialize_nornir() # Single generic group + host-level device_type │ └── config_tools_nornir.py # Multi-vendor config commands +│ ├── _get_nornir_defaults() # Returns default Nornir config +│ └── _initialize_nornir() # Single generic group + host-level device_type ``` +**Key Architectural Changes (2026-03-13):** +- ❌ Removed: `_get_nornir_groups_config()` - No longer needed +- ❌ Removed: `_get_nornir_group()` - No longer needed +- ✅ Simplified: `_initialize_nornir()` - Uses single generic group +- ✅ Updated: `get_gns3_device_port.py()` - Returns host-level `connection_options` + ## Unit Testing ### Test Coverage @@ -435,20 +620,39 @@ python gns3server/agent/gns3_copilot/utils/custom_netmiko/tests/test_huawei_ce.p **Platform (Nornir):** - High-level vendor identifier -- Used for inventory grouping +- **Reserved for future use** with plugins like NAPALM, scrapli +- Used for metadata and logging - Examples: `huawei`, `cisco_ios` +- ⚠️ **Not used by nornir_netmiko** (only `device_type` matters) **Device Type (Netmiko):** -- Precise driver type +- Precise driver type for Netmiko connection - Includes protocol information +- **Actively used** to determine which Netmiko driver class to load - Examples: `huawei_telnet_ce`, `cisco_ios_telnet` +### Why Keep `platform` Field? + +| Purpose | Plugin | Uses `platform`? | +|---------|--------|------------------| +| Connection driver | nornir_netmiko | ❌ No (uses `device_type`) | +| Driver selection | NAPALM | ✅ Yes | +| Driver selection | scrapli | ✅ Yes | +| Metadata/Logging | General | ✅ Yes (future) | + +**Conclusion:** The `platform` field is kept for: +1. **Future plugin support** (NAPALM, scrapli) +2. **Debugging and logging** (vendor identification) +3. **Data completeness** (industry standard practice) + ### Mapping -| Platform | Device Type | Notes | -|----------|-------------|-------| -| `huawei` | `huawei_telnet_ce` | Custom driver for GNS3 | -| `cisco_ios` | `cisco_ios_telnet` | Standard Netmiko driver | +| Platform | Device Type | Netmiko Usage | Notes | +|----------|-------------|---------------|-------| +| `huawei` | `huawei_telnet_ce` | ✅ Active | Custom driver for GNS3 | +| `cisco_ios` | `cisco_ios_telnet` | ✅ Active | Standard Netmiko driver | + +**Important:** For nornir_netmiko, only `device_type` in `connection_options` matters. The `platform` field is informational only. ## Related Documentation @@ -467,6 +671,18 @@ python gns3server/agent/gns3_copilot/utils/custom_netmiko/tests/test_huawei_ce.p _Implementation Date: 2026-03-12_ +_Last Updated: 2026-03-13 (Architecture refactored to host-level configuration)_ + _Status: ✅ Implemented - Custom Huawei driver for GNS3 emulation, multi-vendor support with Cisco IOS and Huawei tested_ +_Architecture: Nornir best practice - host-level connection_options with single generic group_ + _Unit Tests: ✅ 9/9 passing_ + +_Changelog:_ +- **2026-03-13**: Refactored to use host-level `connection_options` instead of dynamic groups + - Removed `_get_nornir_groups_config()` and `_get_nornir_group()` helper functions + - Simplified `_initialize_nornir()` to use single generic group + - Updated `get_gns3_device_port.py()` to return host-level configuration + - Reserved `platform` field for future NAPALM/scrapli plugin support +- **2026-03-12**: Initial implementation with custom Huawei driver diff --git a/gns3server/agent/gns3_copilot/tools_v2/config_tools_nornir.py b/gns3server/agent/gns3_copilot/tools_v2/config_tools_nornir.py index 0289aab29..ed1ad2984 100644 --- a/gns3server/agent/gns3_copilot/tools_v2/config_tools_nornir.py +++ b/gns3server/agent/gns3_copilot/tools_v2/config_tools_nornir.py @@ -104,53 +104,6 @@ def _get_nornir_defaults() -> dict[str, Any]: return {"data": {"location": "gns3"}} -def _get_nornir_groups_config( - device_type: str = "cisco_ios_telnet", platform: str = "cisco_ios" -) -> dict[str, Any]: - """ - Get Nornir group configuration for network devices. - - Args: - device_type: Device type for Netmiko (e.g., 'cisco_ios_telnet', 'huawei_telnet') - platform: Platform type for Nornir (e.g., 'cisco_ios', 'huawei') - - Returns: - Dictionary containing Nornir group configuration - """ - return { - "platform": platform, - "hostname": get_gns3_server_host(), - "timeout": 120, - "username": "", - "password": "", - "connection_options": { - "netmiko": {"extras": {"device_type": device_type}} - }, - } - - -def _get_nornir_group( - group_name: str = "network_devices_telnet", - device_type: str = "cisco_ios_telnet", - platform: str = "cisco_ios", -) -> dict[str, Any]: - """ - Get Nornir group configuration for a specific group. - - Args: - group_name: Name of the group - device_type: Device type for Netmiko (e.g., 'cisco_ios_telnet', 'huawei_telnet') - platform: Platform type for Nornir (e.g., 'cisco_ios', 'huawei') - - Returns: - Dictionary containing group configuration - """ - # _get_nornir_groups_config now returns the group config directly - return _get_nornir_groups_config( - device_type=device_type, platform=platform - ) - - class ExecuteMultipleDeviceConfigCommands(BaseTool): """ A tool to execute configuration commands on multiple devices in a GNS3 @@ -540,51 +493,38 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool): def _initialize_nornir( self, hosts_data: dict[str, dict[str, Any]] ) -> Nornir: - """Initialize Nornir with the provided hosts data.""" + """ + Initialize Nornir with the provided hosts data. + + Each host now has its own connection_options (device_type), so we only + need a single generic group for shared configuration (hostname, timeout, etc.). + This is the Nornir best practice for multi-vendor environments. + """ try: - # Extract device_type and platform from hosts_data - # Use the first device's configuration as default - device_type = None - platform = None - - if hosts_data: - first_device_data = next(iter(hosts_data.values()), {}) - device_type = first_device_data.get( - "device_type", "cisco_ios_telnet" - ) - platform = first_device_data.get("platform", "cisco_ios") - - logger.info( - "Extracted from tags: device_type=%s, platform=%s", - device_type, - platform, - ) - - # Get environment config with dynamic device_type and platform - groups_data = _get_nornir_groups_config( - device_type=device_type or "cisco_ios_telnet", - platform=platform or "cisco_ios", - ) defaults = _get_nornir_defaults() - - # Dynamically generate group name based on platform and device type - # e.g., "huawei_telnet", "cisco_ios_telnet", "juniper_junos" - actual_platform = platform or "cisco_ios" - if device_type and "_telnet" in device_type: - group_name = f"{actual_platform}_telnet" - else: - group_name = actual_platform - - # Log nornir account information gns3_host = get_gns3_server_host() + # Create a single generic group for shared configuration + # Individual device types are handled at host level via connection_options + groups_data = { + "network_devices": { + "hostname": gns3_host, + "timeout": 120, + "username": "", + "password": "", + } + } + + # Log device types being configured + device_types = [ + host["connection_options"]["netmiko"]["extras"]["device_type"] + for host in hosts_data.values() + ] logger.info( - "Initializing Nornir: host=%s, platform=%s, device_type=%s, group=%s, timeout=%d", + "Initializing Nornir: host=%s, device_types=%s, hosts=%d", gns3_host, - groups_data.get("platform"), - device_type, - group_name, - groups_data.get("timeout"), + list(set(device_types)), + len(hosts_data), ) return InitNornir( @@ -592,7 +532,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool): "plugin": "DictInventory", "options": { "hosts": hosts_data, - "groups": {group_name: groups_data}, + "groups": groups_data, "defaults": defaults, }, }, diff --git a/gns3server/agent/gns3_copilot/tools_v2/display_tools_nornir.py b/gns3server/agent/gns3_copilot/tools_v2/display_tools_nornir.py index 9fd0eea78..d8f799bf5 100644 --- a/gns3server/agent/gns3_copilot/tools_v2/display_tools_nornir.py +++ b/gns3server/agent/gns3_copilot/tools_v2/display_tools_nornir.py @@ -104,53 +104,6 @@ def _get_nornir_defaults() -> dict[str, Any]: return {"data": {"location": "gns3"}} -def _get_nornir_groups_config( - device_type: str = "cisco_ios_telnet", platform: str = "cisco_ios" -) -> dict[str, Any]: - """ - Get Nornir group configuration for network devices. - - Args: - device_type: Device type for Netmiko (e.g., 'cisco_ios_telnet', 'huawei_telnet') - platform: Platform type for Nornir (e.g., 'cisco_ios', 'huawei') - - Returns: - Dictionary containing Nornir group configuration - """ - return { - "platform": platform, - "hostname": get_gns3_server_host(), - "timeout": 120, - "username": "", - "password": "", - "connection_options": { - "netmiko": {"extras": {"device_type": device_type}} - }, - } - - -def _get_nornir_group( - group_name: str = "network_devices_telnet", - device_type: str = "cisco_ios_telnet", - platform: str = "cisco_ios", -) -> dict[str, Any]: - """ - Get Nornir group configuration for a specific group. - - Args: - group_name: Name of the group - device_type: Device type for Netmiko (e.g., 'cisco_ios_telnet', 'huawei_telnet') - platform: Platform type for Nornir (e.g., 'cisco_ios', 'huawei') - - Returns: - Dictionary containing group configuration - """ - # _get_nornir_groups_config now returns the group config directly - return _get_nornir_groups_config( - device_type=device_type, platform=platform - ) - - class ExecuteMultipleDeviceCommands(BaseTool): """ A READ-ONLY diagnostic tool for viewing network device configurations. @@ -544,51 +497,38 @@ class ExecuteMultipleDeviceCommands(BaseTool): def _initialize_nornir( self, hosts_data: dict[str, dict[str, Any]] ) -> Nornir: - """Initialize Nornir with the provided hosts data.""" + """ + Initialize Nornir with the provided hosts data. + + Each host now has its own connection_options (device_type), so we only + need a single generic group for shared configuration (hostname, timeout, etc.). + This is the Nornir best practice for multi-vendor environments. + """ try: - # Extract device_type and platform from hosts_data - # Use the first device's configuration as default - device_type = None - platform = None - - if hosts_data: - first_device_data = next(iter(hosts_data.values()), {}) - device_type = first_device_data.get( - "device_type", "cisco_ios_telnet" - ) - platform = first_device_data.get("platform", "cisco_ios") - - logger.info( - "Extracted from tags: device_type=%s, platform=%s", - device_type, - platform, - ) - - # Get environment config with dynamic device_type and platform - groups_data = _get_nornir_groups_config( - device_type=device_type or "cisco_ios_telnet", - platform=platform or "cisco_ios", - ) defaults = _get_nornir_defaults() - - # Dynamically generate group name based on platform and device type - # e.g., "huawei_telnet", "cisco_ios_telnet", "juniper_junos" - actual_platform = platform or "cisco_ios" - if device_type and "_telnet" in device_type: - group_name = f"{actual_platform}_telnet" - else: - group_name = actual_platform - - # Log nornir account information gns3_host = get_gns3_server_host() + # Create a single generic group for shared configuration + # Individual device types are handled at host level via connection_options + groups_data = { + "network_devices": { + "hostname": gns3_host, + "timeout": 120, + "username": "", + "password": "", + } + } + + # Log device types being configured + device_types = [ + host["connection_options"]["netmiko"]["extras"]["device_type"] + for host in hosts_data.values() + ] logger.info( - "Initializing Nornir: host=%s, platform=%s, device_type=%s, group=%s, timeout=%d", + "Initializing Nornir: host=%s, device_types=%s, hosts=%d", gns3_host, - groups_data.get("platform"), - device_type, - group_name, - groups_data.get("timeout"), + list(set(device_types)), + len(hosts_data), ) return InitNornir( @@ -596,7 +536,7 @@ class ExecuteMultipleDeviceCommands(BaseTool): "plugin": "DictInventory", "options": { "hosts": hosts_data, - "groups": {group_name: groups_data}, + "groups": groups_data, "defaults": defaults, }, }, diff --git a/gns3server/agent/gns3_copilot/utils/get_gns3_device_port.py b/gns3server/agent/gns3_copilot/utils/get_gns3_device_port.py index 1f345489c..f38ca70d3 100644 --- a/gns3server/agent/gns3_copilot/utils/get_gns3_device_port.py +++ b/gns3server/agent/gns3_copilot/utils/get_gns3_device_port.py @@ -49,9 +49,13 @@ def get_device_ports_from_topology( { "device_name": { "port": console_port, - "groups": ["platform_telnet"], # Dynamically generated from platform/device_type - "device_type": "huawei_telnet", # Extracted from tags - "platform": "huawei" # Extracted from tags + "platform": "huawei", # Extracted from tags + "groups": ["network_devices"], # For inheriting shared settings + "connection_options": { + "netmiko": { + "extras": {"device_type": "huawei_telnet"} # Extracted from tags + } + } } } Devices that don't exist or missing console_port will not be included @@ -128,18 +132,19 @@ def get_device_ports_from_topology( platform, ) - # Add device to hosts_data - # Dynamically generate group name based on platform and device_type - if device_type and "_telnet" in device_type: - group_name = f"{platform}_telnet" - else: - group_name = platform - + # Add device to hosts_data with connection_options at host level + # This is the Nornir best practice - each host has its own + # connection configuration (device_type), while sharing common + # settings (hostname, timeout) via group inheritance. hosts_data[device_name] = { "port": node_info["console_port"], - "groups": [group_name], - "device_type": device_type, "platform": platform, + "groups": ["network_devices"], # For inheriting hostname, timeout, etc. + "connection_options": { + "netmiko": { + "extras": {"device_type": device_type} + } + }, } logger.info("Returning %d device port mappings", len(hosts_data))