The rmtree error handler in BaseNode.delete() was chmod'ing the failed
path to S_IWRITE (0o200). On POSIX this strips the search permission
from directories, turning a transient deletion failure into a directory
that can no longer be traversed or deleted. The node deletion itself
silently "succeeds" (rmtree gives up once its error handler returns)
and a later project deletion then fails with EACCES. The handler also
never retried the failed operation, so it did not help on Windows
either (the platform it was written for).
The transient failure exists in practice: a concurrent MD5 checksum
computation caching its result in the node directory (e.g. a properties
request racing the deletion) can recreate a file after rmtree has
listed the directory, making the final rmdir fail with ENOTEMPTY.
- add the missing user permissions instead of replacing the whole mode,
and retry the failed unlink/rmdir
- retry the whole deletion a few times to absorb files recreated while
the directory is being deleted
- raise a ComputeError when the directory cannot be fully deleted
instead of failing silently
Only relying on the image name lets a moved tag (e.g. a newer :latest)
silently serve stale content from a compute that already has an image
under the same name. When creating a Docker node, the controller now
pins the image id (Id from the Docker daemon on the controller host)
into the create payload. A compute holding a different image under the
same tag reports the image as missing, which routes it through the
image sync added by the previous commit and re-aligns the tag.
No new template fields or database changes: the controller host daemon
remains the source of truth and the pin is resolved per creation. When
the image is not available on the controller host the pin is omitted
and behavior is unchanged (the compute pulls from the repository).
When a Docker node is created on a remote compute whose Docker daemon
does not have the image, the compute now raises ImageMissingError
instead of blindly pulling from the Docker repository. The controller
exports the image from the Docker daemon on its host (docker save
stream) and streams it to the compute which loads it, so locally built
or docker-loaded images work across computes. When the image is not
available on the controller host either, the compute is asked to pull
it from the Docker repository as a fallback.
- add a POST /docker/images/load compute endpoint that streams a
docker save tar into the Docker daemon
- let Docker.http_query pass raw (non-dict) request bodies through so
the tar can be streamed to the daemon
- drop the inline pull from DockerVM.create() and the now unused
DockerVM.pull_image wrapper
The one-shot reclaim container inherited the image's baked-in USER:
ghcr.io/nokia/srlinux runs as "user:user", so the "privileged" helper
was exactly as unprivileged as the server itself — chmod/chown on files
written by other uids (srlinux writes as a large internal uid) failed
with EPERM and node/project deletion still broke, just with a different
error. Pass --user 0:0 explicitly so the helper is root no matter what
the image declares, and fix the manual reclaim hint the same way.
Validated live on two stuck srlinux node directories (257/258
foreign-owned entries reclaimed to 0 in ~0.5 s each).
The stop-time permission pass necessarily runs before the container's
processes exit, so files written during the shutdown window (syslog
archives, trace flushes) and after any SIGKILL path stay owned by root
on the host. An unprivileged server can neither chown nor delete them,
which broke node deletion and project deletion.
Reclaim them through the only privilege door a non-root server has:
a one-shot throwaway container of the node's own image, entrypoint
overridden to the GNS3 busybox (nothing of the guest boots), chowning
the node directory back to the server user. It runs at the end of
close() — project deletion rmtrees the directory right after the nodes
close, so close must leave a clean tree — and as a retry fallback in
delete(). The helper resolves the image by its create-time ID with
--pull=never, so a stale or retagged image name cannot turn into a
registry pull attempt.
The replay tests marked sharkd_present skip gracefully without the
binary, but four manager-level tests (cap eviction, in-use protection,
single spawn under concurrency, filter-error mapping) run entirely
against fakes and died on the acquire-time PATH check instead - install
wireshark-common (which ships /usr/bin/sharkd on the Ubuntu runner) so
the real-engine tests actually run in CI, and patch the which() check in
the fake-session tests so the suite stays green on machines without
sharkd.
Review-driven session/transport fixes (each reproduced live against
sharkd 4.6.7 before fixing):
- raise the RPC stream limit to 16 MB: a full 1000-row frames page
measures ~190 KB against the 64 KB StreamReader default, which failed
the request with a 500 and desynchronized the resident session; a
line-over-limit ValueError is now treated as a transport failure
- verify JSON-RPC reply ids: a timed-out request's late reply was
served as the next request's answer; timeouts, dead pipes, malformed
and stale replies now kill the session for good instead
- make check-spawn atomic under one manager lock: concurrent requests
for the same pcap double-spawned sharkd and leaked the loser (process
plus /tmp scratch copy) forever
- refcount sessions and evict idle only (LRU, cap raised 8 -> 16): a
tag with more sources than the cap respawned every source on every
request, and concurrent requests could get their session killed
mid-RPC (spurious 502)
- map FilterError to sharkd's filter rejection (-13002) only; other
engine failures with a filter set are 502, not a client 400
- detail: accept an optional frame_number to disambiguate
same-microsecond frames (ts is not unique within a pcap); drop the
-8003 -> 404 mapping (the range is validated locally, engine errors
are real faults); a failed hex read is a 404 instead of "hex": null
- a pcap deleted mid-request is a 404, not a 500; the pcap-sized
scratch copy runs off the event loop; server shutdown kills every
resident session and drops its scratch directory
- pin the packet-list layout through scratch-HOME Wireshark
preferences: the column indexes are a contract the server owns
(protocol-level column negotiation is rejected by sharkd 4.6.x)
Range contract change (WebUI moved to an always-flat list): the merged
frame list is returned in full, deliberately uncapped - truncated and
per-second buckets are removed, frame_count always equals
len(frames), and rendering cost is the client's concern (the window
endpoint remains the incremental path).
Narrows the merged frame stream to one capture source BEFORE counting,
slicing and bucketing (frame_count / frames | buckets all recomputed on
the narrowed set), AND-composing with the display filter. A pure
identity filter applied before any engine work — only the selected
link's pcap gets a sharkd pass, so link+filter is cheaper than filter
alone.
Two boundaries by contract with the Web UI:
- sources[] stays the tag's stable inventory: every capture source
listed with engine-free TOTAL counts, unaffected by link/filter — a
source dropdown must not shrink when the view narrows (this also
settles sources[].count on total counts rather than post-filter
matches, which no spec ever required)
- an unknown link_id matches nothing: frame_count 0, start null, empty
frames/buckets — the same shape as a zero-match display filter,
deliberately not a 404; an empty link param is treated as absent
Real-data examples from a 9-link OSPF project (29 frames): range with
full columns and Wireshark coloring, filter verified in all four regimes
(match / zero-match / invalid 400 with sharkd's text / oversized), window
hit and miss, and a frame detail whose filter_expr carries OSPF's
multicast TTL (ip.ttl == 1) with byte ranges for hex highlighting.
Three precision fixes found by auditing the doc against the code: the
501 applies when the engine is consulted (an empty-capture tag answers
200 without sharkd), sources[].count is post-filter like every other
figure, and the unified {"message": …} error body is now stated.
sharkd (the Wireshark daemon) is now the single decode engine — the
tshark/PDML path is gone, and without sharkd every replay endpoint
returns 501 (no degraded mode: one engine, one rendering shape for the
Web UI).
- Frame entries gain packet-list columns from sharkd's frames RPC:
src/dst/proto/info plus the Wireshark coloring hints bg/fg
- range and frames accept ?filter=<display filter>, applied before
counting and slicing; invalid expressions are 400 carrying sharkd's
original text; filters travel as single argv-style elements, capped
at 2000 chars; filtered frames keep their original pcap frame numbers
- frame detail returns sharkd's protocol tree with keys renamed into
the REST contract (element/label/name/filter_expr/pos+size/expert/
generated/children): a census-verified closed key set, values
untouched, unknown keys passed through verbatim, Wireshark-internal
hf ids dropped. filter_expr gives the UI click-to-filter; pos/size
drives hex highlighting (hex still read straight from the pcap)
- one resident 'sharkd -' session per source pcap: lazy spawn, /tmp
scratch copy + scratch HOME (hardened profiles), per-request
(mtime,size) validation with respawn, LRU bound, per-session lock,
per-RPC timeout, bounded close
Timeline backbone (gate, record-header scan, merge ordering, canonical
ts strings, hex reads) stays plain Python — identity and ordering never
depend on the engine.
The bare 'os.kill = MagicMock()' was never restored, so every later test
in the same process ran with a no-op kill. Any test that kills a child
process and waits for it then hangs forever (resident sharkd sessions
waiting on an immortal process). Use monkeypatch so the patch is undone.
The project-open bulk path (_add_nio_binding / _get_existing_nio /
_update_nio_binding in routes/compute/projects.py) dropped port_number
for Docker nodes, unlike the per-node routes and the IOU branch. With
multi-port docker adapters (iol-runner nodes model 4 ports per adapter,
0ca9ccc63) every batched NIO landed on port 0 where Adapter.add_nio()
silently overwrites — the last entry per node won — so reopening a
project clobbered the port-0 links with the port-1 NIOs: links ended up
cross-wired between the wrong node pairs and the real links died
(IOL direct-link ping failures after close/reopen, EXCESSCOLL storms
from the phantom loops).
The waiting variant blocked on a fixed progress bar (up to 120s) even
when every start command had already failed (e.g. the uBridge >= 1.2.3
409), burying the error behind two minutes of silence. Drop it: the
single start_gns3_node now sends the commands and returns per-node
results immediately (GNS3StartNodeQuickTool kept as an alias). Boot
waiting moves to a dedicated wait_seconds tool the agent calls
explicitly between start and status checks, with a 600s ceiling and
liveness logging every 5s.
Upstream #2870 added TAP carrier control (set_nio_tap_carrier) and a
busybox interface-status monitor to the Docker base class. Both assume
TAP wiring: unix-socket NIO bridges carry no TAP NIO, so uBridge rejects
the carrier command ('bridge has no TAP NIO') and every link create,
update or delete on a running iol-runner node would fail; the monitor
polls eth{adapter} interfaces that do not exist in the container's
network namespace (or misreports the Docker default eth0 as adapter 0).
VendorDockerVM now short-circuits _set_adapter_carrier and the interface
monitor under GNS3_UNIX_SOCKET_NIO; TAP-wired vendor nodes keep the base
behavior.
Upstream #2870's carrier tests assert the two-argument
_set_adapter_carrier call; the 4-port-unit commit threads port_number
through every carrier call site, so single-port adapters now pass 0.
Design proposal for an image_type discriminator on Docker templates plus
a compute-side profile registry: vendor parameters graduate from
environment markers into schema-gated fields, generic-feature
applicability becomes declared capability data, existing markers stay as
a compatibility fallback. Explicitly not a new node type.
Four profiles already exist to shape it (iol-runner, XRd, SR Linux
prototype, FRR appliance); implementation follows the current PR series.
Follows the IOU pattern: the file lives in gns3server/configs/ and the
controller installs it into the user's configs directory on startup
(never overwriting a customized copy). Referenced by the documented
GNS3_IOL_STARTUP_CONFIG=iol-xe-base.txt template knob — without it a
fresh install had no file for the knob to resolve and nodes booted to
the initial configuration dialog.
Also fixes the configs directory named in the docs (~/GNS3/configs,
the configs_path server setting default — not ~/.config/GNS3/...).
