From 1eeee024bda937bba6ed7c819847615e81abc7cc Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sun, 2 Aug 2026 22:46:33 +0800 Subject: [PATCH] marker: rework pause/resume from project-wide to per-definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The project-wide mute (POST /markers/pause|resume + _markers_paused) paused every marker with one button. The actual need is per-rule control: pause one definition and toggle only its inherited global-{name} copies across all links. - Drop project-level: _markers_paused (init/asdict/load/start_all), the pause_all/resume_all_markers methods, and the /markers/pause|resume routes. - Add per-definition: a persisted `paused` flag on each definition; pause/resume_marker_definition fan out update_marker(enabled) to every global-{name} copy — instant, via the existing enable_packet_filter toggle (no NIO rebuild, pcap/emitted preserved). New links inherit a paused definition already off (inherit_marker passes enabled=not paused). - start_marker takes an enabled kwarg; update_marker's enabled-only short-circuit now also covers inherited copies so def pause/resume is instant. - Routes: POST /marker-definitions/{name}/pause|resume. - Docs + tests updated. --- docs/features/marker-traffic-insight.md | 42 ++++++------ gns3server/api/routes/controller/projects.py | 69 +++++++++++--------- gns3server/controller/link.py | 3 +- gns3server/controller/project.py | 61 +++++++---------- gns3server/controller/udp_link.py | 6 +- tests/controller/test_marker.py | 45 ++++++++----- tests/controller/test_project.py | 1 - 7 files changed, 113 insertions(+), 114 deletions(-) diff --git a/docs/features/marker-traffic-insight.md b/docs/features/marker-traffic-insight.md index 3287d4d20..3ca90e891 100644 --- a/docs/features/marker-traffic-insight.md +++ b/docs/features/marker-traffic-insight.md @@ -149,36 +149,28 @@ has no endpoints to choose from, so inherited markers always auto-pick per link. ## Pause & resume -Two independent ways to silence marker activity, both instant and without an -NIO rebuild or pcap flush: +Two levels of silencing, both instant (no NIO rebuild, no pcap flush): -- **Per-filter toggle** — `PUT /v3/projects/{pid}/links/{lid}/markers/{name}` - with `{"enabled": false}` flips the filter off in place (uBridge +- **Per-marker (private)** — `PUT /v3/projects/{pid}/links/{lid}/markers/{name}` + with `{"enabled": false}` flips that one filter off in place (uBridge `enable_packet_filter … off`): no signal, no pcap, but traffic still relays — a paused `mark` is a no-op tap, not a drop. `{"enabled": true}` flips it back. A change to `enabled` alone is a single command (the pcap identity and emitted counter are preserved); changing `bpf` or other fields still goes through a reset+reapply. -- **Project-wide mute** — `POST /v3/projects/{pid}/markers/pause` and `/resume` - issue uBridge `marker pause` / `marker resume` on every capture node. Pause - stops signal **and** pcap but keeps the sink open, so resume is instant. Use - for a global "mute all markers" button. - -The two levers compose and do not overlap: +- **Per-definition (inherited)** — `POST /v3/projects/{pid}/marker-definitions/{name}/pause` + and `/resume` toggle **every** inherited `global-{name}` copy across all links + at once (same `enable_packet_filter on|off`, fanned out per copy). Use to + pause or resume a whole rule independently of the others. The definition's + `paused` flag is persisted to the `.gns3` and echoed on the definition object, + so links created later inherit it already paused, and the Web UI renders the + per-rule button from server truth. | Action | signal | pcap | sink | |--------|--------|------|------| -| per-filter `enabled: false` | stop | stop | n/a | -| `marker pause` (project) | stop | stop | kept (resume instant) | -| `marker resume` (project) | resume | resume | kept | - -The project-wide pause state is **persisted** in the `.gns3` file as -`markers_paused` and echoed on the project object (`GET /v3/projects/{pid}`, -the `asdict()` body), so the Web UI renders the mute button from server truth -rather than a local optimistic flag. Because `marker pause` is a uBridge -runtime flag that resets when a node restarts, `start_all` re-applies the mute -to freshly started uBridges after a project reopen — so a paused project stays -paused across close/reopen. +| per-marker `enabled: false` | stop | stop | n/a | +| per-def `pause` (all `global-{name}` copies) | stop | stop | n/a | +| per-def `resume` | resume | resume | n/a | ## API Endpoints @@ -202,14 +194,14 @@ All endpoints require a JWT bearer token (`POST /v3/access/users/authenticate`). | POST | `/v3/projects/{pid}/marker-definitions` | Create definition (fans out to every link) | Project.Modify | | PUT | `/v3/projects/{pid}/marker-definitions/{name}` | Update definition (syncs all copies) | Project.Modify | | DELETE | `/v3/projects/{pid}/marker-definitions/{name}` | Delete definition (clears all copies) | Project.Modify | +| POST | `/v3/projects/{pid}/marker-definitions/{name}/pause` | Pause every inherited copy (instant, persisted) | Project.Modify | +| POST | `/v3/projects/{pid}/marker-definitions/{name}/resume` | Resume every inherited copy | Project.Modify | ### Aggregation | Method | Path | Description | Auth | |--------|------|-------------|------| | GET | `/v3/projects/{pid}/markers` | All markers across links, flat | Project.Audit | -| POST | `/v3/projects/{pid}/markers/pause` | Mute all markers project-wide (signal+pcap) | Project.Modify | -| POST | `/v3/projects/{pid}/markers/resume` | Resume all markers project-wide | Project.Modify | The link object returned by `GET /v3/projects/{pid}/links[/{lid}]` also carries a `markers` field (including inherited markers), so the Web UI can render a link's markers without an @@ -270,6 +262,8 @@ extra request. "tag": 5, "color": null, "highlight_duration": 1200, + "direction": null, + "paused": false, "link_ids": ["656ed826-...", "6bd9d156-..."] } } @@ -298,6 +292,8 @@ extra request. | `tag` | int \| null | Correlation id | | `color` | string \| null | Hex color render hint | | `highlight_duration` | int \| null | UI highlight duration in ms; `null` = UI default | +| `direction` | string \| null | `tx` / `rx` filter relative to the capture node; `null` = both | +| `paused` | bool | Per-definition mute flag — `true` mutes every inherited copy (persisted) | | `link_ids` | string[] | Links currently carrying an inherited copy (GET only) | ### Notifications diff --git a/gns3server/api/routes/controller/projects.py b/gns3server/api/routes/controller/projects.py index 107392bfa..c9c1c0555 100644 --- a/gns3server/api/routes/controller/projects.py +++ b/gns3server/api/routes/controller/projects.py @@ -219,37 +219,6 @@ def get_project_markers(project: Project = Depends(dep_project)) -> dict: return project.markers -@router.post( - "/{project_id}/markers/pause", - status_code=status.HTTP_204_NO_CONTENT, - dependencies=[Depends(has_privilege("Project.Modify"))] -) -async def pause_project_markers(project: Project = Depends(dep_project)) -> None: - """ - Pause marker signal+pcap emission project-wide (``marker pause`` on every - marker-hosting node's uBridge; resume is instant, sink retained). - - Required privilege: Project.Modify - """ - - await project.pause_all_markers() - - -@router.post( - "/{project_id}/markers/resume", - status_code=status.HTTP_204_NO_CONTENT, - dependencies=[Depends(has_privilege("Project.Modify"))] -) -async def resume_project_markers(project: Project = Depends(dep_project)) -> None: - """ - Resume marker signal+pcap emission project-wide. - - Required privilege: Project.Modify - """ - - await project.resume_all_markers() - - # --------------------------------------------------------------------------- # Project-level marker definitions (global rules inherited by every link) # --------------------------------------------------------------------------- @@ -332,6 +301,44 @@ async def update_marker_definition( return project.marker_definitions.get(def_name, {}) +@router.post( + "/{project_id}/marker-definitions/{def_name}/pause", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(has_privilege("Project.Modify"))] +) +async def pause_marker_definition( + def_name: str, + project: Project = Depends(dep_project) +) -> None: + """ + Pause a definition: toggle off every inherited ``global-{def_name}`` copy + on every link (uBridge ``enable_packet_filter off``, instant — no NIO + rebuild). The definition's ``paused`` flag is persisted, so links created + later inherit it already paused. + + Required privilege: Project.Modify + """ + + await project.pause_marker_definition(def_name) + + +@router.post( + "/{project_id}/marker-definitions/{def_name}/resume", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(has_privilege("Project.Modify"))] +) +async def resume_marker_definition( + def_name: str, + project: Project = Depends(dep_project) +) -> None: + """Resume a paused definition (toggle on every inherited copy). + + Required privilege: Project.Modify + """ + + await project.resume_marker_definition(def_name) + + @router.delete( "/{project_id}/marker-definitions/{def_name}", status_code=status.HTTP_204_NO_CONTENT, diff --git a/gns3server/controller/link.py b/gns3server/controller/link.py index 743abc676..607810d23 100644 --- a/gns3server/controller/link.py +++ b/gns3server/controller/link.py @@ -131,6 +131,7 @@ class Link: direction=marker_def.get("direction"), color=marker_def.get("color"), highlight_duration=marker_def.get("highlight_duration"), + enabled=not marker_def.get("paused", False), inherited_from=def_name, ) @@ -341,7 +342,7 @@ class Link: raise NotImplementedError - async def start_marker(self, name, bpf, tag=None, direction=None, capture_node_id=None): + async def start_marker(self, name, bpf, tag=None, direction=None, capture_node_id=None, enabled=True): """ Attach a traffic-insight marker to this link (base — UDPLink overrides). """ diff --git a/gns3server/controller/project.py b/gns3server/controller/project.py index 371167883..72b0bf9f6 100644 --- a/gns3server/controller/project.py +++ b/gns3server/controller/project.py @@ -214,7 +214,6 @@ class Project: self._nodes = {} self._links = {} self._marker_definitions = {} # name → {bpf, tag, color, highlight_duration} - self._markers_paused = False # project-wide marker mute (persisted, see asdict) self._drawings = {} self._snapshots = {} self._computes = [] @@ -925,44 +924,37 @@ class Project: } return result - async def pause_all_markers(self): + async def pause_marker_definition(self, name): """ - Pause marker signal+pcap emission on every node hosting a marker - (``marker pause`` per capture node's uBridge). Node-deduplicated and - best-effort: a node hosting markers on several links is paused once, - and a node that is down or running an old compute is skipped. + Pause every inherited copy of a definition (``global-{name}``) on every + link: toggle each filter off in place via ``update_marker(enabled=False)`` + — uBridge ``enable_packet_filter off``, no NIO rebuild, pcap/emitted + preserved. The definition's ``paused`` flag is persisted, so links + created later inherit the marker already paused. """ - self._markers_paused = True - seen = set() + if name not in self._marker_definitions: + raise ControllerError(f"Marker definition '{name}' not found") + self._marker_definitions[name]["paused"] = True + marker_name = f"global-{name}" for link in list(self._links.values()): - for info in link.markers.values(): - node_id = info.get("capture_node_id") - if not node_id or node_id in seen: - continue - seen.add(node_id) - try: - await self.get_node(node_id).post("/markers/pause") - except Exception: - pass + if marker_name in link.markers and link.markers[marker_name].get("inherited_from") == name: + await link.update_marker(marker_name, enabled=False, inherited=True) self.dump() + self.emit_notification("project.updated", self.asdict()) - async def resume_all_markers(self): - """Resume marker signal+pcap emission on every marker-hosting node.""" + async def resume_marker_definition(self, name): + """Resume every inherited copy of a definition (toggle on).""" - self._markers_paused = False - seen = set() + if name not in self._marker_definitions: + raise ControllerError(f"Marker definition '{name}' not found") + self._marker_definitions[name]["paused"] = False + marker_name = f"global-{name}" for link in list(self._links.values()): - for info in link.markers.values(): - node_id = info.get("capture_node_id") - if not node_id or node_id in seen: - continue - seen.add(node_id) - try: - await self.get_node(node_id).post("/markers/resume") - except Exception: - pass + if marker_name in link.markers and link.markers[marker_name].get("inherited_from") == name: + await link.update_marker(marker_name, enabled=True, inherited=True) self.dump() + self.emit_notification("project.updated", self.asdict()) @property def marker_definitions(self): @@ -983,7 +975,7 @@ class Project: f"Marker definition '{name}' already exists in this project" ) - self._marker_definitions[name] = {"bpf": bpf, "tag": tag, "direction": direction, "color": color, "highlight_duration": highlight_duration} + self._marker_definitions[name] = {"bpf": bpf, "tag": tag, "direction": direction, "color": color, "highlight_duration": highlight_duration, "paused": False} await self._apply_def_to_all_links(name) self.dump() self.emit_notification("project.updated", self.asdict()) @@ -1477,7 +1469,6 @@ class Project: defs = project_data.get("marker_definitions") if isinstance(defs, dict): self._marker_definitions = defs - self._markers_paused = bool(project_data.get("markers_paused", False)) topology = project_data["topology"] for compute in topology.get("computes", []): @@ -1816,11 +1807,6 @@ class Project: if not node.is_always_running(): pool.append(node.start) await pool.join() - # marker pause is a uBridge runtime flag that resets when a node - # restarts, so re-apply the project-wide mute to the freshly started - # uBridges (markers are installed during node start). - if self._markers_paused: - await self.pause_all_markers() @open_required async def stop_all(self): @@ -1936,7 +1922,6 @@ class Project: "variables": self._variables, "created_by": self._created_by, "marker_definitions": self._marker_definitions, - "markers_paused": self._markers_paused, } def __repr__(self): diff --git a/gns3server/controller/udp_link.py b/gns3server/controller/udp_link.py index b71bdf291..67a9dfe6b 100644 --- a/gns3server/controller/udp_link.py +++ b/gns3server/controller/udp_link.py @@ -350,7 +350,7 @@ class UDPLink(Link): # explicitly deletes a marker via the REST API, and a marker is torn # down automatically only when its link is deleted. - async def start_marker(self, name, bpf, tag=None, direction=None, capture_node_id=None, color=None, highlight_duration=None, inherited_from=None): + async def start_marker(self, name, bpf, tag=None, direction=None, capture_node_id=None, color=None, highlight_duration=None, enabled=True, inherited_from=None): """ Attach a traffic-insight marker to this link. @@ -390,7 +390,7 @@ class UDPLink(Link): marker_entry = { "bpf": bpf, "tag": tag, - "enabled": True, + "enabled": enabled, "color": color, "highlight_duration": highlight_duration, "capture_node_id": marker_side["node"].id, @@ -472,7 +472,7 @@ class UDPLink(Link): and color is None and highlight_duration is None ) - if only_enabled and self._created and not marker_info.get("inherited_from"): + if only_enabled and self._created: capture_node_id = marker_info.get("capture_node_id") side = next((s for s in self._nodes if str(s["node"].id) == str(capture_node_id)), None) if side is not None: diff --git a/tests/controller/test_marker.py b/tests/controller/test_marker.py index e78b3b60d..73dab47e8 100644 --- a/tests/controller/test_marker.py +++ b/tests/controller/test_marker.py @@ -512,29 +512,40 @@ async def test_update_marker_with_bpf_still_rebuilds_nio(project): # --------------------------------------------------------------------------- -# Part C: project-level pause/resume fan-out +# Per-definition pause/resume (toggle every inherited global-{name} copy) # --------------------------------------------------------------------------- @pytest.mark.asyncio -async def test_pause_all_markers_fans_out_to_capture_nodes(project): - # Part C: project-level pause hits each marker-hosting node once. +async def test_pause_marker_definition_toggles_copies_off(project): with _valid_bpf(): - link = await _make_link(project) - await link.start_marker("m", "icmp") - node = link._nodes[0]["node"] - node.post = AsyncioMagicMock() - project.get_node = MagicMock(return_value=node) - await project.pause_all_markers() - node.post.assert_any_call("/markers/pause") + link1 = await _make_link(project) + link2 = await _make_link(project) + await project.create_marker_definition("arp", "arp") + assert link1.markers["global-arp"]["enabled"] is True + assert link2.markers["global-arp"]["enabled"] is True + await project.pause_marker_definition("arp") + assert project.marker_definitions["arp"]["paused"] is True + assert link1.markers["global-arp"]["enabled"] is False + assert link2.markers["global-arp"]["enabled"] is False @pytest.mark.asyncio -async def test_resume_all_markers_fans_out(project): +async def test_resume_marker_definition_toggles_copies_on(project): with _valid_bpf(): link = await _make_link(project) - await link.start_marker("m", "icmp") - node = link._nodes[0]["node"] - node.post = AsyncioMagicMock() - project.get_node = MagicMock(return_value=node) - await project.resume_all_markers() - node.post.assert_any_call("/markers/resume") + await project.create_marker_definition("arp", "arp") + await project.pause_marker_definition("arp") + assert link.markers["global-arp"]["enabled"] is False + await project.resume_marker_definition("arp") + assert project.marker_definitions["arp"]["paused"] is False + assert link.markers["global-arp"]["enabled"] is True + + +@pytest.mark.asyncio +async def test_paused_definition_inherited_as_disabled(project): + # A link created after the definition was paused inherits it already off. + with _valid_bpf(): + await project.create_marker_definition("arp", "arp") + await project.pause_marker_definition("arp") + new_link = await _make_link(project) + assert new_link.markers["global-arp"]["enabled"] is False diff --git a/tests/controller/test_project.py b/tests/controller/test_project.py index 570fa7296..b0202c01f 100644 --- a/tests/controller/test_project.py +++ b/tests/controller/test_project.py @@ -83,7 +83,6 @@ async def test_json(): "supplier": None, "variables": None, "marker_definitions": {}, - "markers_paused": False, "created_by": None }