E2e deflake - #315
Conversation
The relay test asked whether node 1 could see exactly one peer besides the router, by taking `(connected_peers | length) - 1` and comparing it with -eq. That is the wrong question twice over: it fails on an overcount as readily as an undercount, and a relayed connection is free to leave other entries in the set. Ask instead whether node 2's peer ID is present, which is what the test name claims to check, and which every other multi-node test here already does. Polling also began before the node could answer. get_mesh_info goes through MCP, so the first attempts returned nothing, jq mapped that to 0, and the budget drained on a node that had not finished starting rather than on discovery. Gate on MCP responding first. The remaining budget goes from 30 to 60 because this path has to cross two isolated networks over a relay circuit rather than dial directly. This should reduce the flake seen in CI, where node 1 reported dht_size 1 and never learned node 2 existed. It is not a proven fix: that failure has not been reproduced locally, so confidence has to come from watching CI. If it recurs, the answer is to split the discovery claim from the relay datapath claim, as datapath.bats already does with an explicit connect_peer.
The suite reused the kind cluster whenever one existed. That kept the database PVC, the control plane signing keys and the bootstrap tokens from every earlier run, a state CI cannot reach because it builds a cluster per run. Left alone long enough it wedges: the router holds a biscuit no current key verifies and a bootstrap token past the control plane's 24h default, so lease renewal 401s and re-enrolment fails. Every node then dies on "control plane returned no router addresses", days after the run that caused it. Build from scratch by default so local matches CI. E2E_REUSE_CLUSTER=1 keeps the fast loop for iterating, and restarts the router on that path so reuse cannot wedge the same way. Separately, wait for the router's lease to reach the control plane before any test starts a node. mesh_wait_for_rollout returns when the pod is Ready, which precedes the lease, and /register serves router addresses from that lease, so a node starting in the gap enrols against an empty list and exits. Polling /info uses the same GetActiveRouters source /register reads, so it waits on the real precondition rather than a proxy for it, and reports at setup instead of surfacing three layers down as an unexplained log-wait timeout.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the reliability and speed of E2E tests by introducing support for reusing local kind clusters, waiting for the router lease to reach the control plane before starting nodes, and asserting on specific peer connections rather than node counts. The review feedback suggests two improvements: replacing the heavy python:3.12 image with a lightweight curlimages/curl image for curl polling, and using a robust regex instead of awk to extract PeerIDs from logs to prevent fragility against log format changes.
| until docker run --rm --network "${MESH_NETWORK:-kind}" python:3.12 \ | ||
| curl -sf --max-time 5 "http://${router_node_ip}:8080/info" 2>/dev/null | grep -qaF "${router_peer_id}"; do |
There was a problem hiding this comment.
Using the heavy python:3.12 image (~1GB) to run a simple curl command in a loop introduces significant overhead and can slow down test execution, potentially leading to flakes on resource-constrained CI runners. Since curlimages/curl:8.6.0 is already used in the Helm chart and is extremely lightweight (~10MB), we should use it instead.
until docker run --rm --network "${MESH_NETWORK:-kind}" curlimages/curl:8.6.0 \\
-sf --max-time 5 "http://${router_node_ip}:8080/info" 2>/dev/null | grep -qF "${router_peer_id}"; do
| node1_peer_id=$(docker logs "${MESH_PREFIX}-node-1" 2>&1 | grep "PeerID:" | head -n 1 | awk '{print $2}' | tr -d '\r') | ||
| node2_peer_id=$(docker logs "${MESH_PREFIX}-node-2" 2>&1 | grep "PeerID:" | head -n 1 | awk '{print $2}' | tr -d '\r') |
There was a problem hiding this comment.
Using awk '{print $2}' to extract the PeerID from logs is fragile because any changes to the log format (such as adding timestamps, log levels, or changing the logger output format) will break the parsing. Using a robust regular expression with grep -oE to match the standard libp2p PeerID format (12D3Koo...) is much safer and eliminates the need for tr -d '\\r'.
node1_peer_id=$(docker logs "${MESH_PREFIX}-node-1" 2>&1 | grep -oE '12D3Koo[a-zA-Z0-9]+' | head -n 1)
node2_peer_id=$(docker logs "${MESH_PREFIX}-node-2" 2>&1 | grep -oE '12D3Koo[a-zA-Z0-9]+' | head -n 1)
1 participant
Deflake e2e test and improve e2e local UX