fix(tracker): replace address-keyed bootstrap containers with ordered vec - #2044
Conversation
…d evidence protocol
… vec Replaces HashMap<SocketAddr, Container> with Vec<(ConfigurationInstanceId, Container)> so that duplicate port-zero configuration entries each receive their own container. Lookup methods now take an index instead of a SocketAddr. Restructures integration tests to follow the binary-per-configuration convention: one runner test per binary that starts the tracker once, then calls scenario functions sequentially. Adds aggregate_stats_fixed_ports.rs and aggregate_stats_port_zero.rs with both HTTP and UDP scenarios using the tracker-client library. Splits tests/common/ into focused submodules. Closes torrust#2035
There was a problem hiding this comment.
Pull request overview
This PR addresses #2035 by changing bootstrap container storage from an address-keyed map to an order-preserving list keyed by configuration instance identity, ensuring repeated 0.0.0.0:0 blocks do not overwrite each other. It also restructures integration tests into per-configuration binaries and introduces shared HTTP/UDP announce helpers in test-helpers.
Changes:
- Replace
HashMap<SocketAddr, Container>withVec<(ConfigurationInstanceId, Container)>for HTTP/UDP instance containers and switch lookups to configuration indices. - Restructure integration tests into
aggregate_stats_port_zero.rsandaggregate_stats_fixed_ports.rs, with shared helpers undertests/common/. - Add HTTP/UDP announce helpers to
packages/test-helpersand adjust dependency policy (deny.toml).
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/stats.rs | Removes legacy stats integration-test scaffold. |
| tests/servers/mod.rs | Removes legacy servers test module root. |
| tests/servers/api/mod.rs | Removes legacy API test module root. |
| tests/servers/api/contract/mod.rs | Removes legacy contract test module root. |
| tests/servers/api/contract/stats/mod.rs | Removes legacy stats contract scenarios (superseded by new binaries). |
| tests/common/workspace.rs | New workspace/startup + registar-based endpoint discovery helpers. |
| tests/common/statistics.rs | New REST API aggregate-statistics query helper. |
| tests/common/mod.rs | Splits common helpers into submodules and re-exports them. |
| tests/common/announce.rs | New wrappers delegating announces to test-helpers. |
| tests/aggregate_stats_port_zero.rs | New port-zero multi-instance aggregate-statistics integration test binary. |
| tests/aggregate_stats_fixed_ports.rs | New fixed-port multi-instance aggregate-statistics integration test binary. |
| tests/AGENTS.md | Updates documentation to match the new integration-test structure. |
| src/container.rs | Switches per-instance container storage to ordered vec keyed by configuration instance identity. |
| src/app.rs | Updates instance startup to resolve containers by configuration index (not bind address). |
| packages/test-helpers/src/udp.rs | Adds UDP announce helper performing connect→announce handshake. |
| packages/test-helpers/src/http.rs | Adds HTTP announce helper using tracker-client + protocol types. |
| packages/test-helpers/src/lib.rs | Exposes new http and udp helper modules. |
| packages/test-helpers/Cargo.toml | Adds client/protocol deps required by new helpers. |
| docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md | Updates spec metadata/links (but currently includes a stale removed test path). |
| docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md | Restructures evidence doc (but currently contains a duplicated heading artifact). |
| docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md | Updates spec metadata/plan (but currently includes a stale removed test path). |
| deny.toml | Allows torrust-tracker-test-helpers as an approved wrapper for protocol crates. |
| Cargo.lock | Locks added dependencies for torrust-tracker-test-helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| map.keys() | ||
| .find(|b| b.protocol() == torrust_net_primitives::service_binding::Protocol::HTTP && b.bind_address().ip().is_loopback()) | ||
| .map(|b| loopback_url(b.bind_address())) |
| let (container, jobs) = app::run().await; | ||
| // Allow spawned tasks (registar insertions, listener binds) to | ||
| // complete before we attempt to read the registar or make requests. | ||
| tokio::time::sleep(std::time::Duration::from_millis(500)).await; | ||
|
|
||
| (container, jobs) |
| let addr: std::net::SocketAddr = url.as_str()[6..].parse().expect("UDP URL must parse to SocketAddr"); | ||
| common::udp_announce(addr, &info_hash, &peer_id, 17548).await; |
| let addr: std::net::SocketAddr = url.as_str()[6..].parse().expect("UDP URL must parse to SocketAddr"); | ||
| common::udp_announce(addr, &info_hash, &peer_id, 17548).await; |
| - docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md | ||
| - docs/issues/open/2036-add-runtime-service-registry-metadata/ISSUE.md | ||
| - evidence.md | ||
| - tests/servers/api/contract/stats/mod.rs |
| - docs/issues/open/2041-migrate-runtime-service-registry-metadata/ISSUE.md | ||
| - docs/events-architecture.md | ||
| - evidence.md | ||
| - tests/servers/api/contract/stats/mod.rs |
| Add the exact tracker configuration, commands, observed REST statistics, and | ||
| post-change comparison for each probe here. Do not overwrite baseline evidence.# Event Metrics Normalization Evidence | ||
|
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2044 +/- ##
===========================================
+ Coverage 81.63% 82.02% +0.39%
===========================================
Files 346 346
Lines 24735 24733 -2
Branches 24735 24733 -2
===========================================
+ Hits 20192 20288 +96
+ Misses 4238 4135 -103
- Partials 305 310 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
Replaces
HashMap<SocketAddr, Container>withVec<(ConfigurationInstanceId, Container)>so that duplicate port-zero configuration entries each receive their own container.Bootstrap fix
AppContainernow stores HTTP and UDP instance containers in order-preservingVecs keyed by(ConfigurationInstanceId, Container).SocketAddr.0.0.0.0:0entries no longer overwrite each other.Test restructuring
tests/stats.rs+tests/servers/tree withtests/aggregate_stats_port_zero.rsandtests/aggregate_stats_fixed_ports.rs, following the binary-per-configuration convention.tests/common/into focused submodules:workspace.rs,announce.rs,statistics.rs.Supporting changes
http_announceandudp_announcehelpers topackages/test-helpers.torrust-tracker-client-lib,torrust-tracker-udp-protocol) totest-helpers.deny.tomlto allowtest-helpersas a wrapper for protocol crates.tests/AGENTS.mdto reflect the new structure.Validation
cargo machete,cargo deny check bans,linter all, documentation tests.Scope
This PR deliberately does not implement duplicate-port-zero aggregate-statistics filtering (#2039) or registry metadata migration (#2041).
Closes #2035