Merge pull request #3003 from yueguobin/fix/mcp-device-config-remote-compute

Fix MCP device tools for nodes on remote computes

gns3server/agent/gns3_copilot/gns3_client/gns3_topology_reader.py

@@ -67,7 +67,7 @@ class GNS3TopologyTool(BaseTool):
     Output: Dictionary with:
     - `project_id`, `name`, `status`: Project metadata
     - `nodes`: Dict of node details (node_id, name, ports, console_port,
-               type, etc.)
+               console_host, type, etc.)
     - `links`: List of link connections
 
     Use this to understand network structure before making changes.

gns3server/agent/gns3_copilot/gns3_client/project_inventory.py

@@ -43,9 +43,9 @@ def build_nodes_inventory(nodes: list[dict[str, Any]], server_host: str | None)
     Build an inventory-style dict keyed by node name.
 
     Shape (per node name):
-        {server, name, node_id, console_port, console_type, type, ports,
-         status, x, y, tags, netmiko_device_type, default_username,
-         default_password}
+        {server, name, node_id, console_port, console_host, console_type,
+         type, ports, status, x, y, tags, netmiko_device_type,
+         default_username, default_password}
     """
     inventory: dict[str, Any] = {}
     for n in nodes:
@@ -54,6 +54,9 @@ def build_nodes_inventory(nodes: list[dict[str, Any]], server_host: str | None)
             "name": n.get("name"),
             "node_id": n.get("node_id"),
             "console_port": n.get("console"),
+            # Where the console port is bound: the compute's own host for
+            # remote computes, not the controller running this code.
+            "console_host": n.get("console_host"),
             "console_type": n.get("console_type"),
             "type": n.get("node_type"),
             "ports": n.get("ports"),

gns3server/agent/gns3_copilot/tools_v2/vpcs_tools_netmiko.py

@@ -451,7 +451,7 @@ class VPCSCommands(BaseTool):
                 continue
 
             # VPCS devices use gns3_vpcs_telnet device type
-            hosts_data[device_name] = {
+            host_entry = {
                 "port": port,
                 "platform": "vpcs",
                 "groups": ["vpcs_devices"],  # All VPCS devices share one group
@@ -465,6 +465,12 @@ class VPCSCommands(BaseTool):
                     }
                 },
             }
+            # Remote computes bind console ports on their own host; carry the
+            # per-node hostname from the topology so it overrides the group's
+            # controller-wide default.
+            if "hostname" in device_ports[device_name]:
+                host_entry["hostname"] = device_ports[device_name]["hostname"]
+            hosts_data[device_name] = host_entry
 
         return hosts_data
 

gns3server/agent/gns3_copilot/utils/get_gns3_device_port.py

@@ -68,7 +68,12 @@ def get_device_ports_from_topology(
                 }
             }
         }
-        Devices that don't exist or missing console_port will not be included
+        Devices that don't exist or missing console_port will not be included.
+        Nodes whose console is bound on a remote compute also carry a
+        host-level "hostname" (copied from the topology's console_host) so
+        netmiko dials the compute hosting the console port instead of the
+        controller; for everything else the field is omitted and the shared
+        group hostname (the controller host) applies.
     """
     # Log received parameters
     logger.info("Called with device_names=%s, project_id=%s", device_names, project_id)
@@ -167,6 +172,15 @@ def get_device_ports_from_topology(
                 "connection_options": {"netmiko": {"extras": {"device_type": device_type}}},
             }
 
+            # Per-node console hostname. Remote computes bind console ports
+            # on their own host, not on the controller running this code, so
+            # the group's controller-wide hostname cannot reach them. A
+            # host-level hostname (which wins over the group in nornir) fixes
+            # that; unset or wildcard values keep the group inheritance.
+            console_host = node_info.get("console_host")
+            if console_host and console_host not in ("0.0.0.0", "::"):
+                host_entry["hostname"] = console_host
+
             # Per-node default credentials (seeded from the template appliance
             # metadata) override the group's empty fallback. Only inject when
             # set, so credential-less devices keep inheriting the group values.

tests/agent/mcp/test_device_tools.py

@@ -130,6 +130,36 @@ class TestVPCSNodeTypeGuard:
         assert "output" not in result[0]
 
 
+class TestVPCSConsoleHostPropagation:
+    """
+    VPCS builds its own nornir host entries, so the per-node console
+    hostname resolved by get_device_ports_from_topology must be carried
+    over instead of silently falling back to the controller host.
+    """
+
+    def test_remote_compute_hostname_reaches_nornir_inventory(self):
+        from gns3server.agent.gns3_copilot.tools_v2.vpcs_tools_netmiko import VPCSCommands
+
+        with patch(
+            f"{VPCS_MOD}.get_device_ports_from_topology",
+            return_value={
+                "PC1": {"port": 5000, "node_type": "vpcs", "hostname": "192.168.122.10"},
+                "PC2": {"port": 5001, "node_type": "vpcs"},
+            },
+        ):
+            hosts_data = VPCSCommands()._prepare_device_hosts_data(
+                [
+                    {"device_name": "PC1", "commands": ["ip 10.0.0.1/24"]},
+                    {"device_name": "PC2", "commands": ["ip 10.0.0.2/24"]},
+                ],
+                PROJECT_ID,
+            )
+        # remote-compute console hostname lands at host level
+        assert hosts_data["PC1"]["hostname"] == "192.168.122.10"
+        # local-compute entries keep inheriting the group hostname
+        assert "hostname" not in hosts_data["PC2"]
+
+
 class TestDeviceToolErrorContract:
     """
     Every in-band error entry carries status "failed" plus an "error"

tests/agent/test_gns3_client.py

@@ -64,6 +64,35 @@ def test_nodes_inventory_emits_default_credentials():
     assert inventory["R1"]["tags"] == []
 
 
+def test_nodes_inventory_carries_console_host():
+    """
+    The inventory must keep the per-node console_host: remote computes bind
+    console ports on their own host, and the Nornir inventory needs it to
+    dial devices outside the controller machine.
+    """
+    pytest.importorskip("jwt", reason="ai-features extras not installed")
+    from gns3server.agent.gns3_copilot.gns3_client.project_inventory import (
+        build_nodes_inventory,
+    )
+
+    nodes = [
+        {
+            "name": "R1",
+            "node_id": "0d15c2e6-8f83-4b79-8875-9dbc3e5f2f1e",
+            "node_type": "dynamips",
+            "console": 5000,
+            "console_host": "192.168.122.10",
+            "console_type": "telnet",
+            "status": "started",
+        },
+    ]
+
+    inventory = build_nodes_inventory(nodes, "127.0.0.1")
+    assert inventory["R1"]["console_host"] == "192.168.122.10"
+    # the controller-wide "server" stays for backward compatibility
+    assert inventory["R1"]["server"] == "127.0.0.1"
+
+
 def test_links_summary_resolves_names_and_ports():
     """
     links_summary maps raw link endpoint lists to
@@ -234,3 +263,60 @@ def test_device_ports_inject_default_credentials(monkeypatch):
     assert "password" not in hosts["R2"]
     assert "username" not in hosts["R3"]
     assert "password" not in hosts["R3"]
+
+
+def test_device_ports_carry_per_node_console_host(monkeypatch):
+    """
+    Remote computes bind console ports on their own host: console_host from
+    the topology becomes a host-level nornir hostname (overriding the
+    group's controller-wide default). Missing or wildcard values keep the
+    group inheritance.
+    """
+    pytest.importorskip("jwt", reason="ai-features extras not installed")
+    from gns3server.agent.gns3_copilot import gns3_client
+    from gns3server.agent.gns3_copilot.utils import get_gns3_device_port
+
+    class _FakeTopology:
+        def _run(self, project_id=None, jwt_token=None, url=None):
+            return {
+                "nodes": {
+                    # node on a remote compute (another GNS3 VM)
+                    "R1": {
+                        "console_port": 5000,
+                        "console_host": "192.168.122.10",
+                        "tags": [],
+                        "netmiko_device_type": "cisco_ios_telnet",
+                    },
+                    # node on the local compute
+                    "R2": {
+                        "console_port": 5001,
+                        "console_host": "127.0.0.1",
+                        "tags": [],
+                        "netmiko_device_type": "cisco_ios_telnet",
+                    },
+                    # legacy topology payload without console_host
+                    "R3": {
+                        "console_port": 5002,
+                        "tags": [],
+                        "netmiko_device_type": "cisco_ios_telnet",
+                    },
+                    # wildcard host cannot be dialed; keep group inheritance
+                    "R4": {
+                        "console_port": 5003,
+                        "console_host": "0.0.0.0",
+                        "tags": [],
+                        "netmiko_device_type": "cisco_ios_telnet",
+                    },
+                }
+            }
+
+    # the function does a lazy from-import inside the body
+    monkeypatch.setattr(gns3_client, "GNS3TopologyTool", _FakeTopology)
+    hosts = get_gns3_device_port.get_device_ports_from_topology(["R1", "R2", "R3", "R4"])
+
+    # remote and local consoles dial their own host
+    assert hosts["R1"]["hostname"] == "192.168.122.10"
+    assert hosts["R2"]["hostname"] == "127.0.0.1"
+    # missing or wildcard console_host keeps inheriting the group hostname
+    assert "hostname" not in hosts["R3"]
+    assert "hostname" not in hosts["R4"]