Skip to content

Commit 354190e

Browse files
authored
daemon-cli: move multicast transport verbs into daemon-cli crate (#4037)
## Summary of Changes * Move the multicast transport verbs (`multicast subscribe`/`unsubscribe`/`publish`/`unpublish`) from the `doublezero` binary into the `doublezero-daemon-cli` crate per [RFC-20](https://github.com/malbeclabs/doublezero/blob/main/rfcs/rfc20-cli-standardization.md) — PR 7 of the daemon-cli extraction stack (parent: malbeclabs/infra#1458). * The verbs use the RFC-20 execute signature (`CliContext` + generic `DaemonClient` + `LedgerClient` + writer) but are **not** `DaemonCommand` variants — the binary's `MulticastCommands` enum nests them so `doublezero multicast <verb>` invocations are unchanged. Onchain `multicast group` CRUD still routes to the serviceability crate. * Delete the binary's `command/helpers.rs` and the `servicecontroller.rs` remnant (these verbs were their last users; the crate's `resolve_client_ip`/`DaemonClient` survive), and drop the now-orphaned deps (`hyper` stack, `chrono`, `indicatif`) from the binary. Dedupe `init_spinner` into the crate's `helpers.rs` (was copied verbatim in connect/disconnect/multicast). * Behavior parity: message text, skip semantics, role-carry flags, warning conditions, per-group failure aggregation, and exit codes are unchanged. Informational lines now route through the stdout writer instead of the stderr spinner, matching the connect/disconnect migrations. * Fixes malbeclabs/infra#1493 ## Testing Verification * All 21 binary unit tests ported to the crate and strengthened: they now exercise the full `execute` path through `MockDaemonClient` (the old tests bypassed IP resolution via `execute_inner`) and assert rendered writer output. Added `unsubscribe_errors_when_daemon_has_no_client_ip` and a publish partial-failure aggregation test (one group fails onchain → remaining groups still attempted, aggregated non-zero exit). * CLI surface verified with the built binary: `doublezero multicast --help` shows the same five subcommands, `multicast subscribe --help` unchanged, and no transport verb leaks to the top level. * Checked e2e usages (`e2e/main_test.go`, `multicast_register_test.go`): they assert exit codes only, so the stderr→stdout message move is safe. * Four independent reviews (architecture, security, and two style/domain reviewers) found no critical/high issues; the `init_spinner` dedup and publish failure test came out of them. ## Follow-ups (pre-existing behavior deliberately preserved for parity) * The four verbs skip `check_requirements()`/`check_daemon` preflights that `connect`/`disconnect` run — a missing keypair or env mismatch surfaces mid-loop instead of upfront. * `finish_update` prints the "✅ Updated." line even when every per-group call failed (exit code is still non-zero). * Duplicate group args (`subscribe g g`) issue the onchain update twice; multiple multicast users on one IP are picked in HashMap order.
1 parent 66f7811 commit 354190e

14 files changed

Lines changed: 537 additions & 721 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ All notable changes to this project will be documented in this file.
3232
- Onchain programs
3333
- Restrict granting the `FOUNDATION` permission flag: a plain `PERMISSION_ADMIN` holder can no longer grant `FOUNDATION` (a privilege escalation). Only a `foundation_allowlist` member or an existing `FOUNDATION` holder may grant it, enforced independently of `RequirePermissionAccounts` so foundation members are never locked out.
3434
- CLI
35+
- Move the multicast transport verbs (`multicast subscribe`/`unsubscribe`/`publish`/`unpublish`) into the `doublezero-daemon-cli` crate per RFC-20. Each now takes `&CliContext` + generic `&D: DaemonClient` + `&L: LedgerClient` + `&mut W` writer; informational/result lines route through the shared writer (stdout, previously the stderr spinner). They stay nested under `doublezero multicast` (not hoisted as top-level daemon verbs); onchain `multicast group` CRUD is unchanged. The binary's `command/helpers.rs` (`resolve_client_ip`) and the `servicecontroller.rs` remnant are deleted — the crate copies survive. Flags, output content, and exit codes are unchanged. (#4037)
3536
- Move the `connect` verb into the `doublezero-daemon-cli` crate per RFC-20. It now takes `&CliContext` + generic `&D: DaemonClient` + `&L: LedgerClient` + `&mut W` writer; informational/result lines route through the shared writer (stdout, previously the stderr spinner), spinners stay on stderr, pre-flight diagnostics route through `tracing`, and device selection uses the crate's latency utilities (from #3995). The binary's `dzd_latency.rs` and the orphaned `check_doublezero` pre-flight are deleted. Flags, output content, `--verbose`, exit codes, and version-check semantics are unchanged. (#4010)
3637
- Move the `disconnect` verb into the `doublezero-daemon-cli` crate per RFC-20. It now takes `&CliContext` + generic `&D: DaemonClient` + `&L: LedgerClient` + `&mut W` writer; informational/result lines route through the shared writer, spinners stay on stderr, and diagnostics route through `tracing`. Behavior (flags, output, `--verbose`, `--no-wait`, version-check) is unchanged. (#4008)
3738
- Move the `latency` and `routes` verbs, plus the device-selection/latency-polling utilities and `resolve_client_ip`, into the `doublezero-daemon-cli` crate per RFC-20. Both verbs now take `&CliContext` + generic `&D: DaemonClient` + `&L: LedgerClient` + `&mut W` writer; diagnostics route through `tracing` and output through the shared writer helper. Behavior (flags, output, `--json`, version-check) is unchanged. (#3995)

Cargo.lock

Lines changed: 0 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

client/doublezero/Cargo.toml

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,13 @@ repository.workspace = true
1414
[dependencies]
1515
anyhow.workspace = true
1616
backon.workspace = true
17-
chrono.workspace = true
1817
clap.workspace = true
1918
clap_complete.workspace = true
2019
console.workspace = true
2120
eyre.workspace = true
2221
futures.workspace = true
2322
futures-util.workspace = true
2423
libc.workspace = true
25-
http.workspace = true
26-
http-body-util.workspace = true
27-
hyper.workspace = true
28-
hyper-util.workspace = true
29-
hyperlocal.workspace = true
30-
indicatif.workspace = true
3124
mockall.workspace = true
3225
serde.workspace = true
3326
serde_json.workspace = true

client/doublezero/src/cli/command.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ use crate::cli::{multicast::MulticastCliCommand, sentinel::SentinelCliCommand};
1515
/// `doublezero_daemon_cli`. The binary retains the
1616
/// `doublezero-geolocation-cli` module crate's geolocation subtree (via
1717
/// `GeolocationArgs`), the binary-only `Completion` generator, and `Multicast`
18-
/// (whose `Subscribe`/`Unsubscribe`/`Publish`/`Unpublish` arms depend on
19-
/// binary-local daemon-control infrastructure).
18+
/// (whose `Subscribe`/`Unsubscribe`/`Publish`/`Unpublish` arms route to
19+
/// `doublezero-daemon-cli` but stay nested to preserve the
20+
/// `doublezero multicast <verb>` invocation).
2021
#[derive(Subcommand, Debug)]
2122
pub enum Command {
2223
/// Daemon-control verbs migrated to `doublezero-daemon-cli` (RFC-20).
Lines changed: 10 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use clap::{Args, Subcommand};
22

3+
use doublezero_daemon_cli::multicast::{Publish, Subscribe, Unpublish, Unsubscribe};
34
use doublezero_serviceability_cli::cli::multicastgroup::MulticastGroupCliCommand;
45

56
#[derive(Args, Debug)]
@@ -8,49 +9,26 @@ pub struct MulticastCliCommand {
89
pub command: MulticastCommands,
910
}
1011

12+
/// Multicast subtree: `Group` CRUD dispatches to the serviceability module
13+
/// crate; the transport verbs (`Subscribe`/`Unsubscribe`/`Publish`/`Unpublish`)
14+
/// dispatch to `doublezero-daemon-cli`. They are nested here — not hoisted as
15+
/// `DaemonCommand` variants — to preserve the `doublezero multicast <verb>`
16+
/// invocation.
1117
#[derive(Debug, Subcommand)]
1218
pub enum MulticastCommands {
1319
/// Manage multicast groups
1420
#[clap()]
1521
Group(MulticastGroupCliCommand),
1622
/// Subscribe to one or more multicast groups (user must already be connected)
1723
#[clap()]
18-
Subscribe(MulticastSubscribeCliCommand),
24+
Subscribe(Subscribe),
1925
/// Unsubscribe from one or more multicast groups
2026
#[clap()]
21-
Unsubscribe(MulticastUnsubscribeCliCommand),
27+
Unsubscribe(Unsubscribe),
2228
/// Publish to one or more multicast groups (user must already be connected)
2329
#[clap()]
24-
Publish(MulticastPublishCliCommand),
30+
Publish(Publish),
2531
/// Stop publishing to one or more multicast groups
2632
#[clap()]
27-
Unpublish(MulticastUnpublishCliCommand),
28-
}
29-
30-
#[derive(Args, Debug)]
31-
pub struct MulticastSubscribeCliCommand {
32-
/// Multicast group code(s) to subscribe to
33-
#[arg(num_args = 1..)]
34-
pub groups: Vec<String>,
35-
}
36-
37-
#[derive(Args, Debug)]
38-
pub struct MulticastUnsubscribeCliCommand {
39-
/// Multicast group code(s) to unsubscribe from
40-
#[arg(num_args = 1..)]
41-
pub groups: Vec<String>,
42-
}
43-
44-
#[derive(Args, Debug)]
45-
pub struct MulticastPublishCliCommand {
46-
/// Multicast group code(s) to publish to
47-
#[arg(num_args = 1..)]
48-
pub groups: Vec<String>,
49-
}
50-
51-
#[derive(Args, Debug)]
52-
pub struct MulticastUnpublishCliCommand {
53-
/// Multicast group code(s) to stop publishing to
54-
#[arg(num_args = 1..)]
55-
pub groups: Vec<String>,
33+
Unpublish(Unpublish),
5634
}

client/doublezero/src/command/helpers.rs

Lines changed: 0 additions & 19 deletions
This file was deleted.

client/doublezero/src/command/mod.rs

Lines changed: 0 additions & 2 deletions
This file was deleted.

client/doublezero/src/main.rs

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@ use clap::{CommandFactory, Parser};
22
use clap_complete::generate;
33
use std::path::PathBuf;
44
mod cli;
5-
mod command;
6-
use doublezero_config::Environment;
7-
mod servicecontroller;
85
use crate::cli::{command::Command, multicast::MulticastCommands, sentinel::SentinelCommands};
96
use doublezero_cli_core::LogLevel;
7+
use doublezero_config::Environment;
108
use doublezero_daemon_cli::{DaemonClientImpl, DaemonCommand};
119
use doublezero_geolocation_cli::GeoCliCommandImpl;
1210
use doublezero_sdk::{
@@ -20,7 +18,6 @@ use doublezero_serviceability_cli::{
2018
doublezerocommand::{CliCommand, CliCommandImpl},
2119
requirements::{check_requirements, CHECK_BALANCE, CHECK_ID_JSON},
2220
};
23-
use servicecontroller::ServiceControllerImpl;
2421

2522
/// Adapter bridging the binary's `CliCommand` to the daemon-cli crate's
2623
/// `LedgerClient` trait. Holds the client so ledger-backed reads/writes (e.g.
@@ -266,7 +263,6 @@ async fn main() -> eyre::Result<()> {
266263
doublezero_cli_core::init_logging(app.log_level);
267264

268265
if let Some(sock_file) = &app.sock_file {
269-
ServiceControllerImpl::set_global_socket_path(sock_file.to_string_lossy());
270266
DaemonClientImpl::set_global_socket_path(sock_file.to_string_lossy());
271267
}
272268

@@ -447,18 +443,38 @@ async fn main() -> eyre::Result<()> {
447443
}
448444

449445
// Multicast: the `Group` subtree is module-crate business and dispatched by
450-
// `MulticastGroupCommands::execute`; the daemon-coupled async verbs
451-
// (Subscribe/Unsubscribe/Publish/Unpublish) stay binary-local because
452-
// they depend on `ServiceControllerImpl` and `resolve_client_ip`.
453-
Command::Multicast(args) => match args.command {
454-
MulticastCommands::Group(args) => {
455-
args.command.execute(&ctx, &client, &mut handle).await
446+
// `MulticastGroupCommands::execute`; the daemon-coupled transport verbs
447+
// (Subscribe/Unsubscribe/Publish/Unpublish) route to
448+
// `doublezero-daemon-cli`, nested here to keep the
449+
// `doublezero multicast <verb>` invocation.
450+
Command::Multicast(args) => {
451+
let daemon = DaemonClientImpl::new(
452+
ctx.daemon_socket_path
453+
.as_ref()
454+
.map(|p| p.to_string_lossy().into_owned()),
455+
);
456+
let ledger = LedgerAdapter {
457+
env: client.get_environment(),
458+
client: &client,
459+
};
460+
match args.command {
461+
MulticastCommands::Group(args) => {
462+
args.command.execute(&ctx, &client, &mut handle).await
463+
}
464+
MulticastCommands::Subscribe(cmd) => {
465+
cmd.execute(&ctx, &daemon, &ledger, &mut handle).await
466+
}
467+
MulticastCommands::Unsubscribe(cmd) => {
468+
cmd.execute(&ctx, &daemon, &ledger, &mut handle).await
469+
}
470+
MulticastCommands::Publish(cmd) => {
471+
cmd.execute(&ctx, &daemon, &ledger, &mut handle).await
472+
}
473+
MulticastCommands::Unpublish(cmd) => {
474+
cmd.execute(&ctx, &daemon, &ledger, &mut handle).await
475+
}
456476
}
457-
MulticastCommands::Subscribe(args) => args.execute(&client).await,
458-
MulticastCommands::Unsubscribe(args) => args.execute(&client).await,
459-
MulticastCommands::Publish(args) => args.execute(&client).await,
460-
MulticastCommands::Unpublish(args) => args.execute(&client).await,
461-
},
477+
}
462478

463479
// Clap shell-completion generator (binary-local)
464480
Command::Completion(args) => {

0 commit comments

Comments
 (0)