change(tests): use OS-assigned unallocated ports for getting random known ports

[arya2]

Motivation

The random_known_port may return an allocated port, causing a test error.

This PR reduces the likelihood of those errors by having the OS find a port that is unallocated at the time that random_known_port is called, and by broadening the range of possible ports.

Solution

• Create a TcpListener with port 0 to find an unallocated port.
• Read the port number of new socket and drop the listener to close it.

Review

Reviewer Checklist

• Will the PR name make sense to users?
  • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
• Are the PR labels correct?
• Does the code do what the ticket and PR says?
• How do you know it works? Does it have tests?

[teor2345]

I'm happy to try this, and if it ends up not working on Windows, or in some tests, we can go back to using a numerically random port selection.

But every test it does work for is one less test that will randomly fail.

It's impossible to do this check in a reliable, cross-platform way.

Here's some background for that comment:

Windows sometimes waits a few minutes before allowing port reuse:
https://serverfault.com/questions/541070/linux-tcp-source-port-reuse-and-delay/609829#609829

On Linux and macOS, it would be easier if we set the SO_REUSEADDR option on Zebra's ports, but that's impossible for tokio and std Rust ports, because they don't expose that option. (Maybe they've improved the API since?)

https://programmer.group/so_reuseaddr-and-so_reuseport-socket-options-for-tcp-ip-programming.html

[arya2]

Windows sometimes waits a few minutes before allowing port reuse

It's a 2-minute default TcpTimedWaitDelay. That makes sense.

Tokio seems to support SO_REUSEADDR (https://docs.rs/tokio/latest/tokio/net/struct.TcpListener.html#implementations) but that's not cross-platform or supported by Rust std.

We can close this now or put it under #[cfg(any(target_os = "macos", target_os = "linux"))] if the tests pass on MacOS.

[codecov]

Codecov Report

Merging #5607 (6dd307e) into main (c447b03) will increase coverage by 0.11%.
The diff coverage is 81.74%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5607      +/-   ##
==========================================
+ Coverage   78.67%   78.79%   +0.11%     
==========================================
  Files         305      305              
  Lines       38211    38320     +109     
==========================================
+ Hits        30064    30195     +131     
+ Misses       8147     8125      -22     

[teor2345]

Tokio seems to support SO_REUSEADDR (https://docs.rs/tokio/latest/tokio/net/struct.TcpListener.html#implementations) but that's not cross-platform or supported by Rust std.

I think Rust std added it recently as well?

https://github.com/rust-lang/rust/blob/1b225414f325593f974c6b41e671a0a0dc5d7d5e/library/std/src/sys_common/net.rs#L402-L403

[teor2345]

We can close this now or put it under #[cfg(any(target_os = "macos", target_os = "linux"))] if the tests pass on MacOS.

It seems to work, so let's use the same #[cfg(not(windows))] logic as Rust std?

Then we can keep the old code and use it for Windows.

[arya2]

I think Rust std added it recently as well?

That explains why this worked.

[teor2345]

Looks good, but let's merge it separately from #5589, in case we need to revert?

[teor2345]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated