refactor(cli): centralize managed target preparation

Both client applets duplicated the step from parsed arguments to a runnable
managed setup: target specs, managed target preparation, a ClientConfig
built from an arbitrary primary target, pacing conversion, and an identical
ManagedClientConfig skeleton with its own copy of the event-capacity
constant. The primary-target step was the worst of it, since the managed
driver replaces server_addr per target, so the address the applets went out
of their way to find was immediately discarded.

shared::client::prepare now performs that step once: it validates the target
set first, then returns the validated targets, one target-free ClientConfig
template, and the pacing, with managed_config()/managed_targets() for the
driver. CommonClientArgs::to_client_config no longer takes a server address.

Applet-specific semantics are unchanged: each command keeps its own duration
default and help text, the client still accepts no target for
--list-columns, and the TUI still rejects an empty target list at parse
time.
This commit is contained in:
Nils Andreas Svee committed 2026-08-15 10:04:08 +07:00
1 parent c2666d776e
commit 50e4633c7a
8 files changed
+347 -128

No files matched your search

+30 -38
View File
@@ -1,12 +1,10 @@
use std::time::Duration;
use clap::{Parser, ValueEnum};
use irtt_client::ClientConfig;
use crate::shared::client::{
parse_labelled_target, parse_test_duration, prepare_managed_targets, target_specs,
CommonClientArgs, GroupPacingArg, LabelledTargetArg, PreparedTarget, TargetSpec, TimestampArg,
parse_labelled_target, parse_test_duration, prepare_managed_run, CommonClientArgs,
GroupPacingArg, LabelledTargetArg, ManagedRunSetup, TargetSelection, TimestampArg,
};
use clap::{Parser, ValueEnum};
pub const DEFAULT_CLIENT_DURATION: Duration = Duration::from_secs(10);
@@ -91,11 +89,19 @@ pub struct ClientArgs {
}
impl ClientArgs {
pub fn to_client_config(&self) -> ClientConfig {
self.common.to_client_config(
self.primary_target_addr()
.expect("target is required unless --list-columns is set"),
/// Validate the selected targets and prepare the managed run.
///
/// Targets are only required here. `--list-columns` returns before this is
/// called, so listing columns still needs no target.
pub fn prepare(&self) -> Result<ManagedRunSetup, String> {
prepare_managed_run(
&self.common,
self.duration,
TargetSelection {
positional: &self.targets,
labelled: &self.labelled_targets,
pacing: self.pacing,
},
)
}
@@ -106,22 +112,6 @@ impl ClientArgs {
pub fn timestamp_mode(&self) -> TimestampArg {
self.common.tstamp
}
pub fn target_specs(&self) -> Result<Vec<CliTargetSpec>, String> {
target_specs(&self.targets, &self.labelled_targets)
}
pub fn managed_targets(&self) -> Result<Vec<CliManagedTarget>, String> {
prepare_managed_targets(self.target_specs()?)
}
fn primary_target_addr(&self) -> Option<&str> {
self.targets.first().map(String::as_str).or_else(|| {
self.labelled_targets
.first()
.map(|target| target.addr.as_str())
})
}
}
impl std::ops::Deref for ClientArgs {
@@ -144,9 +134,6 @@ pub enum OutputFormat {
Jsonl,
}
pub type CliTargetSpec = TargetSpec;
pub type CliManagedTarget = PreparedTarget;
impl OutputFormat {
pub fn prints_summary(self) -> bool {
matches!(self, Self::Table)
@@ -191,7 +178,7 @@ mod tests {
assert!(!args.is_continuous());
assert_eq!(args.timestamp_mode(), TimestampArg::Both);
let config = args.to_client_config();
let config = args.prepare().unwrap().client;
assert_eq!(config.duration, Some(DEFAULT_CLIENT_DURATION));
assert_eq!(config.received_stats, ReceivedStats::Both);
assert_eq!(config.stamp_at, StampAt::Both);
@@ -212,17 +199,22 @@ mod tests {
let args = parse(&["--list-columns"]).unwrap();
assert!(args.targets.is_empty());
assert!(args.list_columns);
// The parser accepts no target; preparation is where a target becomes
// mandatory, and the stream command returns before preparing when
// listing columns.
let err = args.prepare().unwrap_err();
assert!(err.contains("--list-columns"));
}
#[test]
fn multiple_positional_targets_parse_as_targets() {
let args = parse(&["host-a:2112", "host-b:2112"]).unwrap();
let specs = args.target_specs().unwrap();
let specs = args.prepare().unwrap().targets;
assert_eq!(specs[0].label, "host-a:2112");
assert_eq!(specs[0].addr, "host-a:2112");
assert_eq!(specs[0].managed.server_addr, "host-a:2112");
assert_eq!(specs[1].label, "host-b:2112");
assert_eq!(specs[1].addr, "host-b:2112");
assert_eq!(specs[1].managed.server_addr, "host-b:2112");
}
#[test]
@@ -234,18 +226,18 @@ mod tests {
"sg=sg.example.com:2112",
])
.unwrap();
let specs = args.target_specs().unwrap();
let specs = args.prepare().unwrap().targets;
assert_eq!(specs[0].label, "ams");
assert_eq!(specs[0].addr, "ams.example.com:2112");
assert_eq!(specs[0].managed.server_addr, "ams.example.com:2112");
assert_eq!(specs[1].label, "sg");
assert_eq!(specs[1].addr, "sg.example.com:2112");
assert_eq!(specs[1].managed.server_addr, "sg.example.com:2112");
}
#[test]
fn duplicate_labels_are_rejected() {
let args = parse(&["host-a:2112", "--target", "host-a:2112=host-b:2112"]).unwrap();
let err = args.target_specs().unwrap_err();
let err = args.prepare().unwrap_err();
assert!(err.contains("duplicate target label"));
}
@@ -253,7 +245,7 @@ mod tests {
#[test]
fn duplicate_positional_target_strings_get_stable_suffixes() {
let args = parse(&["host-a:2112", "host-a:2112"]).unwrap();
let specs = args.target_specs().unwrap();
let specs = args.prepare().unwrap().targets;
assert_eq!(specs[0].label, "host-a:2112");
assert_eq!(specs[1].label, "host-a:2112#2");
@@ -262,7 +254,7 @@ mod tests {
#[test]
fn duplicate_target_endpoints_are_allowed() {
let args = parse(&["127.0.0.1:2112", "127.0.0.1"]).unwrap();
assert_eq!(args.managed_targets().unwrap().len(), 2);
assert_eq!(args.prepare().unwrap().target_count(), 2);
}
#[test]
+10 -23
View File
@@ -9,9 +9,8 @@ use std::{
use irtt_client::{
managed::{
BlockingManagedClient, ManagedClientConfig, ManagedCompletionPolicy, ManagedEndReason,
ManagedEvent, ManagedEventSubscription, ManagedEventTryRecvError, ManagedTargetEndReason,
ManagedTargetOutcome, TargetInstance,
BlockingManagedClient, ManagedEndReason, ManagedEvent, ManagedEventSubscription,
ManagedEventTryRecvError, ManagedTargetEndReason, ManagedTargetOutcome, TargetInstance,
},
ClientEvent,
};
@@ -44,7 +43,6 @@ const FINITE_STATS_MEMORY_STRONG_WARNING_BYTES: u64 = 512 * MIB;
const FINITE_STATS_MEMORY_VERY_STRONG_WARNING_BYTES: u64 = GIB;
const MANAGED_EVENT_WAIT_SLICE: Duration = Duration::from_millis(20);
const MANAGED_EVENT_CAPACITY: usize = 16_384;
pub fn run_stream(
args: ClientArgs,
@@ -54,10 +52,10 @@ pub fn run_stream(
print!("{}", OutputConfig::list_columns());
return Ok(());
}
let targets = args
.managed_targets()
let setup = args
.prepare()
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?;
let multi_target = targets.len() > 1;
let multi_target = setup.is_multi_target();
let output_config = OutputConfig::new(
args.format,
args.columns.as_deref(),
@@ -67,7 +65,7 @@ pub fn run_stream(
)
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?;
let continuous = args.is_continuous();
let target_count = targets.len();
let target_count = setup.target_count();
if let Some(warning) = finite_stats_memory_warning(&args, target_count) {
eprintln!("{warning}");
@@ -75,21 +73,9 @@ pub fn run_stream(
if is_shutdown_requested(shutdown_requested) {
return Ok(());
}
let config = ManagedClientConfig {
client: args.to_client_config(),
pacing: args.pacing.into(),
completion: ManagedCompletionPolicy::FinishWhenQuiescent,
event_capacity: MANAGED_EVENT_CAPACITY,
outcome_history_limit: target_count,
max_live_target_generations: target_count,
..ManagedClientConfig::default()
};
let (owner, mut events) = BlockingManagedClient::start_with_subscription(
config,
targets
.iter()
.map(|target| target.managed.clone())
.collect(),
setup.managed_config(),
setup.managed_targets(),
)?;
let handle = owner.handle();
let mut stdout = io::LineWriter::new(io::stdout().lock());
@@ -101,7 +87,8 @@ pub fn run_stream(
out: &mut stdout,
};
let mut stats = targets
let mut stats = setup
.targets
.iter()
.map(|target| {
(
+30 -40
View File
@@ -1,13 +1,12 @@
use std::time::Duration;
use clap::Parser;
use irtt_client::ClientConfig;
#[cfg(test)]
use crate::shared::client::TimestampArg;
use crate::shared::client::{
parse_labelled_target, parse_test_duration, prepare_managed_targets, target_specs,
CommonClientArgs, GroupPacingArg, LabelledTargetArg, PreparedTarget, TargetSpec,
parse_labelled_target, parse_test_duration, prepare_managed_run, CommonClientArgs,
GroupPacingArg, LabelledTargetArg, ManagedRunSetup, TargetSelection,
};
pub const DEFAULT_TUI_DURATION: Duration = Duration::ZERO;
@@ -56,11 +55,19 @@ pub struct TuiArgs {
}
impl TuiArgs {
pub fn to_client_config(&self) -> ClientConfig {
self.common.to_client_config(
self.primary_target_addr()
.expect("at least one target is required"),
/// Validate the selected targets and prepare the managed run.
///
/// The TUI always requires a target: the parser rejects an empty target
/// list, and this rejects a target set that cannot be labelled uniquely.
pub fn prepare(&self) -> Result<ManagedRunSetup, String> {
prepare_managed_run(
&self.common,
self.duration,
TargetSelection {
positional: &self.targets,
labelled: &self.labelled_targets,
pacing: self.pacing,
},
)
}
@@ -68,31 +75,12 @@ impl TuiArgs {
self.duration == Duration::ZERO
}
pub fn target_specs(&self) -> Result<Vec<TuiTargetSpec>, String> {
target_specs(&self.targets, &self.labelled_targets)
}
pub fn managed_targets(&self) -> Result<Vec<TuiManagedTarget>, String> {
prepare_managed_targets(self.target_specs()?)
}
#[cfg(test)]
pub fn timestamp_mode(&self) -> TimestampArg {
self.common.tstamp
}
fn primary_target_addr(&self) -> Option<&str> {
self.targets.first().map(String::as_str).or_else(|| {
self.labelled_targets
.first()
.map(|target| target.addr.as_str())
})
}
}
pub type TuiTargetSpec = TargetSpec;
pub type TuiManagedTarget = PreparedTarget;
impl std::ops::Deref for TuiArgs {
type Target = CommonClientArgs;
@@ -132,12 +120,12 @@ mod tests {
assert_eq!(args.pacing, GroupPacingArg::Staggered);
assert_eq!(args.duration, DEFAULT_TUI_DURATION);
assert!(args.is_continuous());
assert_eq!(args.to_client_config().duration, None);
assert_eq!(args.prepare().unwrap().client.duration, None);
let finite = parse(&["--duration", "30s", "127.0.0.1:2112"]).unwrap();
assert_eq!(finite.duration, Duration::from_secs(30));
assert_eq!(
finite.to_client_config().duration,
finite.prepare().unwrap().client.duration,
Some(Duration::from_secs(30))
);
@@ -149,12 +137,12 @@ mod tests {
#[test]
fn multiple_positional_targets_parse() {
let args = parse(&["host-a:2112", "host-b:2112"]).unwrap();
let specs = args.target_specs().unwrap();
let specs = args.prepare().unwrap().targets;
assert_eq!(specs[0].label, "host-a:2112");
assert_eq!(specs[0].addr, "host-a:2112");
assert_eq!(specs[0].managed.server_addr, "host-a:2112");
assert_eq!(specs[1].label, "host-b:2112");
assert_eq!(specs[1].addr, "host-b:2112");
assert_eq!(specs[1].managed.server_addr, "host-b:2112");
}
#[test]
@@ -166,23 +154,26 @@ mod tests {
"sg=sg.example.com:2112",
])
.unwrap();
let specs = args.target_specs().unwrap();
let specs = args.prepare().unwrap().targets;
assert_eq!(specs[0].label, "ams");
assert_eq!(specs[0].addr, "ams.example.com:2112");
assert_eq!(specs[0].managed.server_addr, "ams.example.com:2112");
assert_eq!(specs[1].label, "sg");
assert_eq!(specs[1].addr, "sg.example.com:2112");
assert_eq!(specs[1].managed.server_addr, "sg.example.com:2112");
}
#[test]
fn at_least_one_target_is_required() {
// The TUI has no target-free mode, so this is rejected at parse time
// rather than during preparation.
assert!(parse(&[]).is_err());
assert!(parse(&["--target", "ams=ams.example.com:2112"]).is_ok());
}
#[test]
fn duplicate_labels_are_rejected() {
let args = parse(&["host-a:2112", "--target", "host-a:2112=host-b:2112"]).unwrap();
let err = args.target_specs().unwrap_err();
let err = args.prepare().unwrap_err();
assert!(err.contains("duplicate target label"));
}
@@ -190,7 +181,7 @@ mod tests {
#[test]
fn duplicate_positional_target_strings_get_stable_suffixes() {
let args = parse(&["host-a:2112", "host-a:2112"]).unwrap();
let specs = args.target_specs().unwrap();
let specs = args.prepare().unwrap().targets;
assert_eq!(specs[0].label, "host-a:2112");
assert_eq!(specs[1].label, "host-a:2112#2");
@@ -199,7 +190,7 @@ mod tests {
#[test]
fn duplicate_target_endpoints_are_allowed() {
let args = parse(&["127.0.0.1:2112", "127.0.0.1"]).unwrap();
assert_eq!(args.managed_targets().unwrap().len(), 2);
assert_eq!(args.prepare().unwrap().target_count(), 2);
}
#[test]
@@ -272,11 +263,10 @@ mod tests {
"--loose",
"127.0.0.1:2112",
];
let tui = parse(&tui_args).unwrap().to_client_config();
let tui = parse(&tui_args).unwrap().prepare().unwrap().client;
let common = parse_shared(&shared).unwrap().common;
let shared = common.to_client_config("127.0.0.1:2112", Duration::from_secs(30));
let shared = common.to_client_config(Duration::from_secs(30));
assert_eq!(shared.server_addr, tui.server_addr);
assert_eq!(shared.duration, tui.duration);
assert_eq!(shared.interval, tui.interval);
assert_eq!(shared.length, tui.length);
+8 -22
View File
@@ -19,27 +19,26 @@ use crate::{
};
use crossterm::event::{self, Event, KeyCode, KeyEventKind, KeyModifiers};
use irtt_client::managed::{
BlockingManagedClient, ManagedClientConfig, ManagedCompletionPolicy, ManagedEndReason,
ManagedEvent, ManagedEventSubscription, ManagedEventTryRecvError, TargetInstance,
BlockingManagedClient, ManagedEndReason, ManagedEvent, ManagedEventSubscription,
ManagedEventTryRecvError, TargetInstance,
};
const RENDER_INTERVAL: Duration = Duration::from_millis(250);
const TUI_WAIT_SLICE: Duration = Duration::from_millis(20);
const MANAGED_EVENT_CAPACITY: usize = 16_384;
const INPUT_EVENT_WORK_BUDGET: usize = 128;
pub fn run_tui(
args: TuiArgs,
shutdown_requested: &AtomicBool,
) -> Result<(), Box<dyn std::error::Error>> {
let targets = args
.managed_targets()
let setup = args
.prepare()
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?;
let continuous = args.is_continuous();
let mut terminal = TuiTerminal::enter()?;
let mut state = TuiState::with_target_labels(
TuiConfig::from_args(&args),
targets.iter().map(|target| target.label.clone()),
TuiConfig::from_args(&args, &setup),
setup.targets.iter().map(|target| target.label.clone()),
);
let mut next_render = Instant::now();
if is_shutdown_requested(shutdown_requested) {
@@ -47,22 +46,9 @@ pub fn run_tui(
}
state.set_status(TuiStatus::Opening);
render_if_due(&mut terminal, &state, &mut next_render, true)?;
let target_count = targets.len();
let config = ManagedClientConfig {
client: args.to_client_config(),
pacing: args.pacing.into(),
completion: ManagedCompletionPolicy::FinishWhenQuiescent,
event_capacity: MANAGED_EVENT_CAPACITY,
outcome_history_limit: target_count,
max_live_target_generations: target_count,
..ManagedClientConfig::default()
};
let (owner, mut events) = match BlockingManagedClient::start_with_subscription(
config,
targets
.iter()
.map(|target| target.managed.clone())
.collect(),
setup.managed_config(),
setup.managed_targets(),
) {
Ok(value) => value,
Err(error) => {
+3 -3
View File
@@ -30,7 +30,7 @@ use crate::{
},
tui::args::TuiArgs,
},
shared::client::{expected_probe_count, GroupPacingArg},
shared::client::{expected_probe_count, GroupPacingArg, ManagedRunSetup},
};
const HISTORY_LIMIT: usize = 100_000;
@@ -656,11 +656,11 @@ pub(super) struct TuiConfig {
}
impl TuiConfig {
pub(super) fn from_args(args: &TuiArgs) -> Self {
pub(super) fn from_args(args: &TuiArgs, setup: &ManagedRunSetup) -> Self {
Self {
interval: args.interval,
duration: (!args.is_continuous()).then_some(args.duration),
timeout: args.to_client_config().probe_timeout,
timeout: setup.client.probe_timeout,
target_probes: (!args.is_continuous())
.then(|| expected_probe_count(args.duration, args.interval)),
pacing: args.pacing,