Skip to content

Commit

Permalink
refactor(iroh-net): Delete some unused testing infrastructure (#2028)
Browse files Browse the repository at this point in the history
## Description

This came from the tailscale code but we never seem to have used it
and our routers haven't exploded yet.

## Notes & open questions

Admittedly it would be nice not to keep hitting so many external
things on the network.  But our entire testsuite would need a big
review for that.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
  • Loading branch information
flub committed Feb 16, 2024
1 parent c62950e commit e7af74d
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 32 deletions.
6 changes: 0 additions & 6 deletions iroh-net/src/netcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,10 +383,6 @@ struct Actor {
reports: Reports,

// Actor configuration.
/// Whether the client should try to reach things other than localhost.
///
/// This is set to true in tests to avoid probing the local LAN's router, etc.
skip_external_network: bool,
/// The port mapper client, if those are requested.
///
/// The port mapper is responsible for talking to routers via UPnP and the like to try
Expand Down Expand Up @@ -414,7 +410,6 @@ impl Actor {
receiver,
sender,
reports: Default::default(),
skip_external_network: false,
port_mapper,
in_flight_stun_requests: Default::default(),
current_report_run: None,
Expand Down Expand Up @@ -516,7 +511,6 @@ impl Actor {
self.addr(),
self.reports.last.clone(),
self.port_mapper.clone(),
self.skip_external_network,
derp_map,
stun_sock_v4,
stun_sock_v6,
Expand Down
40 changes: 14 additions & 26 deletions iroh-net/src/netcheck/reportgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ impl Client {
netcheck: netcheck::Addr,
last_report: Option<Arc<Report>>,
port_mapper: Option<portmapper::Client>,
skip_external_network: bool,
derp_map: DerpMap,
stun_sock4: Option<Arc<UdpSocket>>,
stun_sock6: Option<Arc<UdpSocket>>,
Expand All @@ -101,7 +100,6 @@ impl Client {
netcheck: netcheck.clone(),
last_report,
port_mapper,
skip_external_network,
incremental,
derp_map,
stun_sock4,
Expand Down Expand Up @@ -172,13 +170,6 @@ struct Actor {
last_report: Option<Arc<Report>>,
/// The portmapper client, if there is one.
port_mapper: Option<portmapper::Client>,
/// Whether the actor should try to reach things other than localhost.
///
/// This is set to true in tests to avoid probing the local LAN's router etc.
///
/// TODO: currently this only skips the portmapper, but still does STUN, HTTP and ICMP
/// probes. Furthermore none of our tests actually use this feature.
skip_external_network: bool,
/// The DERP configuration.
derp_map: DerpMap,
/// Socket to send IPv4 STUN requests from.
Expand Down Expand Up @@ -234,7 +225,6 @@ impl Actor {
async fn run_inner(&mut self) -> Result<()> {
debug!(
port_mapper = %self.port_mapper.is_some(),
skip_external_network=%self.skip_external_network,
"reportstate actor starting",
);

Expand Down Expand Up @@ -500,23 +490,21 @@ impl Actor {
&mut self,
) -> MaybeFuture<Pin<Box<impl Future<Output = Option<portmapper::ProbeOutput>>>>> {
let mut port_mapping = MaybeFuture::default();
if !self.skip_external_network {
if let Some(port_mapper) = self.port_mapper.clone() {
port_mapping.inner = Some(Box::pin(async move {
match port_mapper.probe().await {
Ok(Ok(res)) => Some(res),
Ok(Err(err)) => {
warn!("skipping port mapping: {err:?}");
None
}
Err(recv_err) => {
warn!("skipping port mapping: {recv_err:?}");
None
}
if let Some(port_mapper) = self.port_mapper.clone() {
port_mapping.inner = Some(Box::pin(async move {
match port_mapper.probe().await {
Ok(Ok(res)) => Some(res),
Ok(Err(err)) => {
warn!("skipping port mapping: {err:?}");
None
}
}));
self.outstanding_tasks.port_mapper = true;
}
Err(recv_err) => {
warn!("skipping port mapping: {recv_err:?}");
None
}
}
}));
self.outstanding_tasks.port_mapper = true;
}
port_mapping
}
Expand Down

0 comments on commit e7af74d

Please sign in to comment.