Open
Conversation
Three tests failed under cargo test (thread-per-test) but passed under cargo nextest (process-per-test) due to shared global state in the channel-based test network simulation: 1. unnumbered_peering_helper() leaked dispatcher threads by not returning dispatcher handles to callers. Leaked threads would periodically re-bind to addresses in the global NET endpoint map, stealing entries from concurrently running tests. 2. The simulated Network had no unbind mechanism, so endpoint entries persisted after a Listener was dropped, leaving stale state for subsequent tests. 3. Several tests used hardcoded scope_ids (2, 3) for link-local addresses, causing collisions in the shared NET endpoint map when tests ran as concurrent threads in the same process. Fix by: - Adding Network::unbind() and Listener::Drop to clean up NET endpoint entries when a listener goes out of scope - Returning dispatchers from unnumbered_peering_helper() so all callers can shut them down during test cleanup - Switching hardcoded scope_ids to next_scope_id() for unique per-test addresses, matching the newer test infrastructure
Fix PolicyAction::from_str() to convert "deny" into Self::Deny instead of Self::Allow.
When stdout is piped into a program that closes early (e.g. `mgadm bgp status neighbors 47 | head`), Rust's println! macro panics on the resulting EPIPE, causing a SIGABRT. Add println_nopipe!/print_nopipe!/eprintln_nopipe!/eprint_nopipe! macros to mg-common that silently exit on BrokenPipe instead of panicking. Use them throughout mgadm's print calls. Also catch EPIPE errors propagated via ? from TabWriter writeln/flush paths at both mgadm and ddmadm main() boundaries. Add clippy disallowed_macros entries for std::println, std::print, std::eprintln, and std::eprint in clippy.toml so future code is steered to the wrappers. Migrate the remaining workspace callsites (xtask, mg-admin-client, mg-ddm-verify, plus test scaffolding in bgp, rdb, mgd, bfd, and mg-tests) to the wrappers. Fixes #567 Signed-off-by: Trey Aspelund <trey@oxidecomputer.com>
We skipped both 127.0.0.1 and ::1 during install, but were not skipping ::1 during uninstall. Let's make that symmetric.
Move from using an installed bool to using a u32 to track the number of intra-process users of an IP, which prevents early uninstalls. Move loopback-skipping logic (127.0.0.1 / ::1) into a dedicated function and make a few methods private that don't need to be public.
If an installation fails for one of the requested IPs, rollback the IPs. Removes an unused uninstall() method.
taspelund
force-pushed
the
trey/light-cleanup
branch
from
9d709e0 to
49a9983
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second round of splits from #648
Minor changes:
cargo testin addition tocargo nextest)See each commit for specifics