diff --git a/Cargo.lock b/Cargo.lock index b1f90d863..de3ca6f4b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2392,7 +2392,7 @@ dependencies = [ [[package]] name = "iii" -version = "0.11.0-next.1" +version = "0.11.0-next.5" dependencies = [ "anyhow", "async-trait", @@ -2467,7 +2467,7 @@ dependencies = [ [[package]] name = "iii-console" -version = "0.11.0-next.1" +version = "0.11.0-next.5" dependencies = [ "anyhow", "axum", @@ -2536,7 +2536,7 @@ dependencies = [ [[package]] name = "iii-sdk" -version = "0.11.0-next.1" +version = "0.11.0-next.5" dependencies = [ "async-trait", "ctor", diff --git a/crates/iii-worker/src/cli/app.rs b/crates/iii-worker/src/cli/app.rs index 48f69b0b6..d9fff54fc 100644 --- a/crates/iii-worker/src/cli/app.rs +++ b/crates/iii-worker/src/cli/app.rs @@ -18,38 +18,18 @@ pub struct Cli { #[derive(Subcommand, Debug)] pub enum Commands { - /// Add a worker from the registry or by OCI image reference + /// Add one or more workers from the registry or by OCI image reference Add { - /// Worker name or OCI image reference (e.g., "pdfkit", "pdfkit@1.0.0", "ghcr.io/org/worker:tag") - #[arg(value_name = "WORKER[@VERSION]")] - worker_name: String, - - /// Container runtime - #[arg(long, default_value = "libkrun")] - runtime: String, - - /// Engine host address - #[arg(long, default_value = "localhost")] - address: String, - - /// Engine WebSocket port - #[arg(long, default_value_t = DEFAULT_PORT)] - port: u16, + /// Worker names or OCI image references (e.g., "pdfkit", "pdfkit@1.0.0", "ghcr.io/org/worker:tag") + #[arg(value_name = "WORKER[@VERSION]", required = true, num_args = 1..)] + worker_names: Vec, }, - /// Remove a worker (stops and removes the container) + /// Remove one or more workers (stops and removes containers) Remove { - /// Worker name to remove (e.g., "pdfkit") - #[arg(value_name = "WORKER")] - worker_name: String, - - /// Engine host address - #[arg(long, default_value = "localhost")] - address: String, - - /// Engine WebSocket port - #[arg(long, default_value_t = DEFAULT_PORT)] - port: u16, + /// Worker names to remove (e.g., "pdfkit") + #[arg(value_name = "WORKER", required = true, num_args = 1..)] + worker_names: Vec, }, /// Start a previously stopped managed worker container diff --git a/crates/iii-worker/src/cli/managed.rs b/crates/iii-worker/src/cli/managed.rs index cb598c737..34074eee8 100644 --- a/crates/iii-worker/src/cli/managed.rs +++ b/crates/iii-worker/src/cli/managed.rs @@ -12,22 +12,34 @@ use super::binary_download; use super::builtin_defaults::get_builtin_default; use super::lifecycle::build_container_spec; use super::registry::{ - MANIFEST_PATH, WorkerType, fetch_registry, parse_worker_input, resolve_image, + MANIFEST_PATH, RegistryV2, WorkerType, fetch_registry, parse_worker_input, resolve_image, }; use super::worker_manager::state::WorkerDef; pub use super::dev::handle_worker_dev; -pub async fn handle_binary_add(input: &str, _runtime: &str, _address: &str, _port: u16) -> i32 { +pub async fn handle_binary_add( + input: &str, + brief: bool, + cached_registry: Option<&RegistryV2>, +) -> i32 { let (worker_name, version_override) = parse_worker_input(input); - eprintln!(" Resolving {}...", worker_name.bold()); - let registry = match fetch_registry().await { - Ok(r) => r, - Err(e) => { - eprintln!("{} {}", "error:".red(), e); - return 1; - } + if !brief { + eprintln!(" Resolving {}...", worker_name.bold()); + } + let fetched; + let registry = if let Some(r) = cached_registry { + r + } else { + fetched = match fetch_registry().await { + Ok(r) => r, + Err(e) => { + eprintln!("{} {}", "error:".red(), e); + return 1; + } + }; + &fetched }; let entry = match registry.workers.get(&worker_name) { @@ -67,14 +79,15 @@ pub async fn handle_binary_add(input: &str, _runtime: &str, _address: &str, _por let has_checksum = entry.has_checksum.unwrap_or(false); let target = binary_download::current_target(); - eprintln!( - " {} Resolved to {} (binary v{})", - "✓".green(), - repo.to_string().dimmed(), - version - ); - - eprintln!(" Downloading {}...", worker_name.bold()); + if !brief { + eprintln!( + " {} Resolved to {} (binary v{})", + "✓".green(), + repo.to_string().dimmed(), + version + ); + eprintln!(" Downloading {}...", worker_name.bold()); + } let install_path = match binary_download::download_and_install_binary( &worker_name, &repo, @@ -92,21 +105,23 @@ pub async fn handle_binary_add(input: &str, _runtime: &str, _address: &str, _por } }; - eprintln!(" {} Downloaded successfully", "✓".green()); + if !brief { + eprintln!(" {} Downloaded successfully", "✓".green()); - // Show metadata matching OCI worker style - eprintln!(" {}: {}", "Name".bold(), worker_name); - eprintln!(" {}: {}", "Version".bold(), version); - if !entry.description.is_empty() { - eprintln!(" {}: {}", "Description".bold(), entry.description); - } - eprintln!(" {}: {}", "Platform".bold(), target); - if let Ok(metadata) = std::fs::metadata(&install_path) { - eprintln!( - " {}: {:.1} MB", - "Size".bold(), - metadata.len() as f64 / 1_048_576.0 - ); + // Show metadata matching OCI worker style + eprintln!(" {}: {}", "Name".bold(), worker_name); + eprintln!(" {}: {}", "Version".bold(), version); + if !entry.description.is_empty() { + eprintln!(" {}: {}", "Description".bold(), entry.description); + } + eprintln!(" {}: {}", "Platform".bold(), target); + if let Ok(metadata) = std::fs::metadata(&install_path) { + eprintln!( + " {}: {:.1} MB", + "Size".bold(), + metadata.len() as f64 / 1_048_576.0 + ); + } } let config_yaml = entry @@ -120,21 +135,57 @@ pub async fn handle_binary_add(input: &str, _runtime: &str, _address: &str, _por return 1; } - eprintln!( - "\n {} Worker {} added to {}", - "✓".green(), - worker_name.bold(), - "config.yaml".dimmed(), - ); - eprintln!(" Start the engine to run it, or edit config.yaml to customize."); + if brief { + eprintln!(" {} {}", "✓".green(), worker_name.bold()); + } else { + eprintln!( + "\n {} Worker {} added to {}", + "✓".green(), + worker_name.bold(), + "config.yaml".dimmed(), + ); + eprintln!(" Start the engine to run it, or edit config.yaml to customize."); + } 0 } +pub async fn handle_managed_add_many(worker_names: &[String]) -> i32 { + let total = worker_names.len(); + let brief = total > 1; + let mut fail_count = 0; + + // Pre-fetch registry once for all workers (avoids N HTTP roundtrips). + let registry = fetch_registry().await.ok(); + + for (i, name) in worker_names.iter().enumerate() { + if brief { + eprintln!(" [{}/{}] Adding {}...", i + 1, total, name.bold()); + } + let result = handle_managed_add(name, brief, registry.as_ref()).await; + if result != 0 { + fail_count += 1; + } + } + + if total > 1 { + let succeeded = total - fail_count; + if fail_count == 0 { + eprintln!("\n Added {}/{} workers.", succeeded, total); + } else { + eprintln!( + "\n Added {}/{} workers. {} failed.", + succeeded, total, fail_count + ); + } + } + + if fail_count == 0 { 0 } else { 1 } +} + pub async fn handle_managed_add( image_or_name: &str, - _runtime: &str, - _address: &str, - _port: u16, + brief: bool, + cached_registry: Option<&RegistryV2>, ) -> i32 { // Check for engine-builtin workers first (no network needed). if let Some(default_yaml) = get_builtin_default(image_or_name) { @@ -143,45 +194,64 @@ pub async fn handle_managed_add( eprintln!("{} {}", "error:".red(), e); return 1; } - if already_exists { - eprintln!( - "\n {} Worker {} updated in {} (merged with builtin defaults)", - "✓".green(), - image_or_name.bold(), - "config.yaml".dimmed(), - ); + if brief { + if already_exists { + eprintln!(" {} {} (updated)", "✓".green(), image_or_name.bold()); + } else { + eprintln!(" {} {}", "✓".green(), image_or_name.bold()); + } } else { - eprintln!( - "\n {} Worker {} added to {}", - "✓".green(), - image_or_name.bold(), - "config.yaml".dimmed(), - ); + if already_exists { + eprintln!( + "\n {} Worker {} updated in {} (merged with builtin defaults)", + "✓".green(), + image_or_name.bold(), + "config.yaml".dimmed(), + ); + } else { + eprintln!( + "\n {} Worker {} added to {}", + "✓".green(), + image_or_name.bold(), + "config.yaml".dimmed(), + ); + } + eprintln!(" Start the engine to run it, or edit config.yaml to customize."); } - eprintln!(" Start the engine to run it, or edit config.yaml to customize."); return 0; } // Route binary workers to handle_binary_add; for OCI workers found in the - // registry, use the already-fetched entry to avoid a second HTTP roundtrip. + // registry, use the cached registry or fetch once if not provided. if !image_or_name.contains('/') && !image_or_name.contains(':') { let (name, _) = parse_worker_input(image_or_name); - if let Ok(registry) = fetch_registry().await + let fetched; + let registry = if let Some(r) = cached_registry { + Some(r) + } else { + fetched = fetch_registry().await.ok(); + fetched.as_ref() + }; + if let Some(registry) = registry && let Some(entry) = registry.workers.get(&name) { if matches!(entry.worker_type, Some(WorkerType::Binary)) { - return handle_binary_add(image_or_name, _runtime, _address, _port).await; + return handle_binary_add(image_or_name, brief, Some(registry)).await; } // OCI worker found in registry — use already-fetched entry if let (Some(img), Some(ver)) = (&entry.image, &entry.latest) { let image_ref = format!("{}:{}", img, ver); - eprintln!(" {} Resolved to {}", "✓".green(), image_ref.dimmed()); - return handle_oci_pull_and_add(&name, &image_ref).await; + if !brief { + eprintln!(" {} Resolved to {}", "✓".green(), image_ref.dimmed()); + } + return handle_oci_pull_and_add(&name, &image_ref, brief).await; } } } - eprintln!(" Resolving {}...", image_or_name.bold()); + if !brief { + eprintln!(" Resolving {}...", image_or_name.bold()); + } let (image_ref, name) = match resolve_image(image_or_name).await { Ok(v) => v, Err(e) => { @@ -189,14 +259,18 @@ pub async fn handle_managed_add( return 1; } }; - eprintln!(" {} Resolved to {}", "✓".green(), image_ref.dimmed()); - handle_oci_pull_and_add(&name, &image_ref).await + if !brief { + eprintln!(" {} Resolved to {}", "✓".green(), image_ref.dimmed()); + } + handle_oci_pull_and_add(&name, &image_ref, brief).await } -async fn handle_oci_pull_and_add(name: &str, image_ref: &str) -> i32 { +async fn handle_oci_pull_and_add(name: &str, image_ref: &str, brief: bool) -> i32 { let adapter = super::worker_manager::create_adapter("libkrun"); - eprintln!(" Pulling {}...", image_ref.bold()); + if !brief { + eprintln!(" Pulling {}...", image_ref.bold()); + } let pull_info = match adapter.pull(image_ref).await { Ok(info) => info, Err(e) => { @@ -214,24 +288,26 @@ async fn handle_oci_pull_and_add(name: &str, image_ref: &str) -> i32 { Err(_) => None, }; - if let Some(ref m) = manifest { - eprintln!(" {} Image pulled successfully", "✓".green()); - if let Some(v) = m.get("name").and_then(|v| v.as_str()) { - eprintln!(" {}: {}", "Name".bold(), v); - } - if let Some(v) = m.get("version").and_then(|v| v.as_str()) { - eprintln!(" {}: {}", "Version".bold(), v); - } - if let Some(v) = m.get("description").and_then(|v| v.as_str()) { - eprintln!(" {}: {}", "Description".bold(), v); - } - if let Some(size) = pull_info.size_bytes { - eprintln!(" {}: {:.1} MB", "Size".bold(), size as f64 / 1_048_576.0); - } - } else { - eprintln!(" {} Image pulled (no manifest found)", "✓".green()); - if let Some(size) = pull_info.size_bytes { - eprintln!(" {}: {:.1} MB", "Size".bold(), size as f64 / 1_048_576.0); + if !brief { + if let Some(ref m) = manifest { + eprintln!(" {} Image pulled successfully", "✓".green()); + if let Some(v) = m.get("name").and_then(|v| v.as_str()) { + eprintln!(" {}: {}", "Name".bold(), v); + } + if let Some(v) = m.get("version").and_then(|v| v.as_str()) { + eprintln!(" {}: {}", "Version".bold(), v); + } + if let Some(v) = m.get("description").and_then(|v| v.as_str()) { + eprintln!(" {}: {}", "Description".bold(), v); + } + if let Some(size) = pull_info.size_bytes { + eprintln!(" {}: {:.1} MB", "Size".bold(), size as f64 / 1_048_576.0); + } + } else { + eprintln!(" {} Image pulled (no manifest found)", "✓".green()); + if let Some(size) = pull_info.size_bytes { + eprintln!(" {}: {:.1} MB", "Size".bold(), size as f64 / 1_048_576.0); + } } } @@ -284,17 +360,51 @@ async fn handle_oci_pull_and_add(name: &str, image_ref: &str) -> i32 { eprintln!("{} Failed to update config.yaml: {}", "error:".red(), e); return 1; } - eprintln!( - "\n {} Worker {} added to {}", - "✓".green(), - name.bold(), - "config.yaml".dimmed(), - ); - eprintln!(" Start the engine to run it, or edit config.yaml to customize."); + if brief { + eprintln!(" {} {}", "✓".green(), name.bold()); + } else { + eprintln!( + "\n {} Worker {} added to {}", + "✓".green(), + name.bold(), + "config.yaml".dimmed(), + ); + eprintln!(" Start the engine to run it, or edit config.yaml to customize."); + } 0 } -pub async fn handle_managed_remove(worker_name: &str, _address: &str, _port: u16) -> i32 { +pub async fn handle_managed_remove_many(worker_names: &[String]) -> i32 { + let total = worker_names.len(); + let brief = total > 1; + let mut fail_count = 0; + + for (i, name) in worker_names.iter().enumerate() { + if brief { + eprintln!(" [{}/{}] Removing {}...", i + 1, total, name.bold()); + } + let result = handle_managed_remove(name, brief).await; + if result != 0 { + fail_count += 1; + } + } + + if total > 1 { + let succeeded = total - fail_count; + if fail_count == 0 { + eprintln!("\n Removed {}/{} workers.", succeeded, total); + } else { + eprintln!( + "\n Removed {}/{} workers. {} failed.", + succeeded, total, fail_count + ); + } + } + + if fail_count == 0 { 0 } else { 1 } +} + +pub async fn handle_managed_remove(worker_name: &str, brief: bool) -> i32 { if let Err(e) = super::registry::validate_worker_name(worker_name) { eprintln!("{} {}", "error:".red(), e); return 1; @@ -303,12 +413,16 @@ pub async fn handle_managed_remove(worker_name: &str, _address: &str, _port: u16 eprintln!("{} {}", "error:".red(), e); return 1; } - eprintln!( - " {} {} removed from {}", - "✓".green(), - worker_name.bold(), - "config.yaml".dimmed(), - ); + if brief { + eprintln!(" {} {}", "✓".green(), worker_name.bold()); + } else { + eprintln!( + " {} {} removed from {}", + "✓".green(), + worker_name.bold(), + "config.yaml".dimmed(), + ); + } 0 } diff --git a/crates/iii-worker/src/main.rs b/crates/iii-worker/src/main.rs index 9d9341b22..6bae7f960 100644 --- a/crates/iii-worker/src/main.rs +++ b/crates/iii-worker/src/main.rs @@ -19,20 +19,12 @@ async fn main() -> anyhow::Result<()> { let cli_args = Cli::parse(); let exit_code = match cli_args.command { - Commands::Add { - worker_name, - runtime, - address, - port, - } => { - iii_worker::cli::managed::handle_managed_add(&worker_name, &runtime, &address, port) - .await + Commands::Add { worker_names } => { + iii_worker::cli::managed::handle_managed_add_many(&worker_names).await + } + Commands::Remove { worker_names } => { + iii_worker::cli::managed::handle_managed_remove_many(&worker_names).await } - Commands::Remove { - worker_name, - address, - port, - } => iii_worker::cli::managed::handle_managed_remove(&worker_name, &address, port).await, Commands::Start { worker_name, address, diff --git a/crates/iii-worker/tests/config_file_integration.rs b/crates/iii-worker/tests/config_file_integration.rs index f9ce5a4d3..d09f13cf2 100644 --- a/crates/iii-worker/tests/config_file_integration.rs +++ b/crates/iii-worker/tests/config_file_integration.rs @@ -313,6 +313,47 @@ fn all_builtins_produce_valid_config_entries() { }); } +// ────────────────────────────────────────────────────────────────────────────── +// handle_managed_add_many flow tests +// ────────────────────────────────────────────────────────────────────────────── + +#[tokio::test] +async fn add_many_builtin_workers() { + in_temp_dir_async(|| async { + let names = vec!["iii-http".to_string(), "iii-state".to_string()]; + let exit_code = iii_worker::cli::managed::handle_managed_add_many(&names).await; + assert_eq!(exit_code, 0, "all builtin workers should succeed"); + + assert!( + iii_worker::cli::config_file::worker_exists("iii-http"), + "iii-http should be in config.yaml" + ); + assert!( + iii_worker::cli::config_file::worker_exists("iii-state"), + "iii-state should be in config.yaml" + ); + }) + .await; +} + +#[tokio::test] +async fn add_many_with_invalid_worker_returns_nonzero() { + in_temp_dir_async(|| async { + let names = vec![ + "iii-http".to_string(), + "definitely-not-a-real-worker-xyz".to_string(), + ]; + let exit_code = iii_worker::cli::managed::handle_managed_add_many(&names).await; + assert_ne!(exit_code, 0, "should fail when any worker fails"); + + assert!( + iii_worker::cli::config_file::worker_exists("iii-http"), + "iii-http should still be in config.yaml despite other failure" + ); + }) + .await; +} + // ────────────────────────────────────────────────────────────────────────────── // handle_managed_add flow tests // ────────────────────────────────────────────────────────────────────────────── @@ -320,9 +361,7 @@ fn all_builtins_produce_valid_config_entries() { #[tokio::test] async fn handle_managed_add_builtin_creates_config() { in_temp_dir_async(|| async { - let exit_code = - iii_worker::cli::managed::handle_managed_add("iii-http", "libkrun", "localhost", 49134) - .await; + let exit_code = iii_worker::cli::managed::handle_managed_add("iii-http", false, None).await; assert_eq!( exit_code, 0, "expected success exit code for builtin worker" @@ -351,7 +390,7 @@ async fn handle_managed_add_builtin_merges_existing() { .unwrap(); let exit_code = - iii_worker::cli::managed::handle_managed_add("iii-http", "libkrun", "localhost", 49134) + iii_worker::cli::managed::handle_managed_add("iii-http", false, None) .await; assert_eq!(exit_code, 0, "expected success exit code for merge"); @@ -372,9 +411,7 @@ async fn handle_managed_add_all_builtins_succeed() { for name in iii_worker::cli::builtin_defaults::BUILTIN_NAMES { let _ = std::fs::remove_file("config.yaml"); - let exit_code = - iii_worker::cli::managed::handle_managed_add(name, "libkrun", "localhost", 49134) - .await; + let exit_code = iii_worker::cli::managed::handle_managed_add(name, false, None).await; assert_eq!(exit_code, 0, "expected success for builtin '{}'", name); let content = std::fs::read_to_string("config.yaml").unwrap(); @@ -387,3 +424,53 @@ async fn handle_managed_add_all_builtins_succeed() { }) .await; } + +// ────────────────────────────────────────────────────────────────────────────── +// handle_managed_remove_many flow tests +// ────────────────────────────────────────────────────────────────────────────── + +#[tokio::test] +async fn remove_many_workers() { + in_temp_dir_async(|| async { + // Add two builtins first. + let names = vec!["iii-http".to_string(), "iii-state".to_string()]; + let exit_code = iii_worker::cli::managed::handle_managed_add_many(&names).await; + assert_eq!(exit_code, 0); + + // Remove both at once. + let exit_code = iii_worker::cli::managed::handle_managed_remove_many(&names).await; + assert_eq!(exit_code, 0, "all removals should succeed"); + + assert!( + !iii_worker::cli::config_file::worker_exists("iii-http"), + "iii-http should be removed" + ); + assert!( + !iii_worker::cli::config_file::worker_exists("iii-state"), + "iii-state should be removed" + ); + }) + .await; +} + +#[tokio::test] +async fn remove_many_with_missing_worker_returns_nonzero() { + in_temp_dir_async(|| async { + // Add one builtin. + let add_names = vec!["iii-http".to_string()]; + let exit_code = iii_worker::cli::managed::handle_managed_add_many(&add_names).await; + assert_eq!(exit_code, 0); + + // Remove existing + nonexistent. + let remove_names = vec!["iii-http".to_string(), "not-a-real-worker".to_string()]; + let exit_code = iii_worker::cli::managed::handle_managed_remove_many(&remove_names).await; + assert_ne!(exit_code, 0, "should fail when any removal fails"); + + // The valid one should still have been removed. + assert!( + !iii_worker::cli::config_file::worker_exists("iii-http"), + "iii-http should be removed despite other failure" + ); + }) + .await; +} diff --git a/crates/iii-worker/tests/worker_integration.rs b/crates/iii-worker/tests/worker_integration.rs index 370d6f910..392261a97 100644 --- a/crates/iii-worker/tests/worker_integration.rs +++ b/crates/iii-worker/tests/worker_integration.rs @@ -4,7 +4,7 @@ //! the crate library, ensuring any CLI changes are caught at compile time. use clap::Parser; -use iii_worker::{Cli, Commands, DEFAULT_PORT, VmBootArgs}; +use iii_worker::{Cli, Commands, VmBootArgs}; /// All 10 subcommands parse without error. #[test] @@ -56,21 +56,25 @@ fn cli_parses_all_subcommands() { fn add_subcommand_fields() { let cli = Cli::parse_from(["iii-worker", "add", "ghcr.io/iii-hq/node:latest"]); match cli.command { - Commands::Add { - worker_name, - runtime, - address, - port, - } => { - assert_eq!(worker_name, "ghcr.io/iii-hq/node:latest"); - assert_eq!(runtime, "libkrun"); - assert_eq!(address, "localhost"); - assert_eq!(port, DEFAULT_PORT); + Commands::Add { worker_names } => { + assert_eq!(worker_names, vec!["ghcr.io/iii-hq/node:latest".to_string()]); } _ => panic!("expected Add"), } } +/// `add` subcommand accepts multiple worker names as positional args. +#[test] +fn add_subcommand_multiple_workers() { + let cli = Cli::parse_from(["iii-worker", "add", "pdfkit", "iii-http", "iii-state"]); + match cli.command { + Commands::Add { worker_names } => { + assert_eq!(worker_names, vec!["pdfkit", "iii-http", "iii-state"]); + } + _ => panic!("Expected Add command"), + } +} + /// `dev` subcommand requires a path and supports all optional flags. #[test] fn dev_subcommand_all_flags() {