…ect+detect) (#615) * test(monitoring): retry the TCP connect+detect cycle to fix a flaky detection test test_suspicious_tcp_port_detected intermittently failed with 'Expected network threat for TCP:4444, got none' under CI load. The suspicious connection is fire-and-forget, so a transiently failed connect() (server race / slow python3 startup) leaves no ESTABLISHED socket to detect, and the host-side /proc-net poller can also miss a single detection window. Re-issue the connection and poll again up to 3 times (fresh connect each attempt); any detection passes. No change to what the test asserts. Prior commits widened the poll window (40s->75s) and added a wait-for-listen; this addresses the residual fire-and-forget/poll-window race directly. * test(monitoring): verify ESTABLISHED before polling (review follow-up) Applies the /code-review findings on the retry hardening: - Verify the in-container connect() actually reached ESTABLISHED (ss ESTAB on :4444) before polling for detection. This distinguishes the two failure modes the blind retry conflated: a failed connect is retried; an ESTABLISHED-but- undetected connection now fails as a real detector miss with a precise message (findings #1, #5). - Re-ensure the port-4444 server is listening each attempt, restarting it if it died between attempts (finding #4). - Track the fire-and-forget connect processes and terminate them in `finally` so up to three `incus exec ... sleep(120)` sessions don't linger (finding #3). - Per-attempt poll trimmed to 30s (3x40s worst case unchanged from the retry version) so runtime doesn't regress (finding #2). Extracted small helpers (_start_http_server_4444 / _wait_port4444_listening / _port4444_listening / _port4444_established) shared by the initial setup and the retry loop. Test-only; no product code touched. * test(monitoring): fix defects found reviewing the ESTABLISHED-verify upgrade Applies the max-effort /code-review findings on the prior commit: - Anchored port match (#1/#7): replace `grep ':4444'` / `grep 'ESTAB.*:4444'` substring checks with exact `ss` state+port filters (`ss -Htln 'sport = :PORT'`, `ss -Htn state established '( sport = :PORT or dport = :PORT )'` + `grep -q .`). The old substring matched ephemeral ports 44440-44449 and IPv6 4444 hextets, which could flip established=True on a connect flake and misfire the "detector miss" assertion. One idiom now (grep -q + returncode) for both predicates. - TimeoutExpired safety (#2): `_incus_bash` catches subprocess.TimeoutExpired and returns None ("not ready"); predicates guard for it. The establish gate no longer adds an uncaught-timeout hard-ERROR surface to the detection path. - Establish/poll split (#3/#4): Phase 1 retries only the CONNECT (bounded) and confirms ESTABLISHED; Phase 2 polls ONCE with the full 75s budget. Drops the sticky `established` misdiagnosis and stops connect-retries from shrinking the detection-latency window below the old 75s. - Skip-vs-fail consistency (#5): an in-loop server restart that fails now pytest.skips (environmental), matching the initial start. - Parameterize + reuse (#6): helpers take (container, port); the metadata sibling drops its condemned `time.sleep(2)` for the same wait-for-listening. - Efficiency (#8/#9/#10): server-start unified into the loop (no redundant back-to-back probe); establish-wait polls at 2s (monitor's interval); break on first establish (no redundant extra connects). Pre-existing, out of scope: _poll_network_threats returning on the first network event, and the `"4444" in json.dumps` substring assert.

Generalize the API so we can be code-on-incus #5

Description

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions