Improve DX: confirmations, progress, validate, completions

Batch 1 of developer experience improvements:

- Destroy requires typing zone name to confirm (or --yes/-y for scripts)
- Non-interactive (non-TTY) mode refuses destructive ops without --yes
- Create/destroy print numbered step-by-step progress
- Partial failure during create prints cleanup guidance (orphaned VNICs etc)
- New `zmgr validate` checks KDL syntax, referential integrity, gateway
  in network, range in network, duplicate IPs, pool utilization warnings
- New `zmgr completions <shell>` generates bash/zsh/fish completions
- Added --version flag
- Dry-run notes that IP allocation is tentative

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Till Wegmueller 2026-03-22 15:30:02 +01:00
parent 430be11b13
commit 382823b228
No known key found for this signature in database
4 changed files with 560 additions and 42 deletions

67
Cargo.lock generated
View file

@ -169,6 +169,15 @@ dependencies = [
"strsim",
]
[[package]]
name = "clap_complete"
version = "4.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "19c9f1dde76b736e3681f28cec9d5a61299cbaae0fce80a68e43724ad56031eb"
dependencies = [
"clap",
]
[[package]]
name = "clap_derive"
version = "4.6.0"
@ -269,6 +278,12 @@ version = "1.70.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695"
[[package]]
name = "itoa"
version = "1.0.18"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682"
[[package]]
name = "js-sys"
version = "0.3.91"
@ -496,6 +511,49 @@ version = "1.0.22"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d"
[[package]]
name = "serde"
version = "1.0.228"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e"
dependencies = [
"serde_core",
"serde_derive",
]
[[package]]
name = "serde_core"
version = "1.0.228"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad"
dependencies = [
"serde_derive",
]
[[package]]
name = "serde_derive"
version = "1.0.228"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79"
dependencies = [
"proc-macro2",
"quote",
"syn",
]
[[package]]
name = "serde_json"
version = "1.0.149"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "83fc039473c5595ace860d8c4fafa220ff474b3fc6bfdb4293327f1a37e94d86"
dependencies = [
"itoa",
"memchr",
"serde",
"serde_core",
"zmij",
]
[[package]]
name = "shlex"
version = "1.3.0"
@ -812,8 +870,17 @@ version = "0.1.0"
dependencies = [
"chrono",
"clap",
"clap_complete",
"ipnet",
"kdl",
"miette",
"serde",
"serde_json",
"thiserror",
]
[[package]]
name = "zmij"
version = "1.0.21"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b8848ee67ecc8aedbaf3e4122217aff892639231befc6a1b58d29fff4c2cabaa"

View file

@ -6,7 +6,10 @@ edition = "2024"
[dependencies]
chrono = "0.4.44"
clap = { version = "4.6.0", features = ["derive"] }
clap_complete = "4.6.0"
ipnet = "2.12.0"
kdl = "6.5.0"
miette = { version = "7.6.0", features = ["fancy"] }
serde = { version = "1.0.228", features = ["derive"] }
serde_json = "1.0.149"
thiserror = "2.0.18"

View file

@ -10,13 +10,16 @@ mod kdl_util;
mod pool;
mod publisher;
mod template;
mod validate;
mod zone;
use std::collections::HashSet;
use std::io::{IsTerminal, Write};
use std::net::Ipv4Addr;
use std::path::Path;
use clap::{Parser, Subcommand};
use clap::{CommandFactory, Parser, Subcommand};
use clap_complete::Shell;
use crate::config::Config;
use crate::error::{Result, ZmgrError};
@ -26,7 +29,7 @@ use crate::template::Template;
use crate::zone::{Zone, ZoneNet};
#[derive(Parser)]
#[command(name = "zmgr", about = "illumos zone manager")]
#[command(name = "zmgr", version, about = "illumos zone manager")]
struct Cli {
/// Registry path (default: /etc/zmgr)
#[arg(long, default_value = "/etc/zmgr")]
@ -36,6 +39,10 @@ struct Cli {
#[arg(long, short = 'n')]
dry_run: bool,
/// Skip confirmation prompts (for scripting)
#[arg(long, short = 'y')]
yes: bool,
#[command(subcommand)]
command: Commands,
}
@ -73,6 +80,8 @@ enum Commands {
/// Only import this specific zone
name: Option<String>,
},
/// Validate registry configuration
Validate,
/// Manage templates
Template {
#[command(subcommand)]
@ -88,6 +97,11 @@ enum Commands {
#[command(subcommand)]
action: PublisherAction,
},
/// Generate shell completions
Completions {
/// Shell to generate completions for
shell: Shell,
},
}
#[derive(Subcommand)]
@ -129,16 +143,18 @@ fn main() -> miette::Result<()> {
let registry = std::path::PathBuf::from(&cli.registry);
let dry_run = cli.dry_run;
let yes = cli.yes;
match cli.command {
Commands::Init { force } => cmd_init(&registry, force),
Commands::Create { name, template } => {
cmd_create(&registry, &name, template.as_deref(), dry_run)
}
Commands::Destroy { name } => cmd_destroy(&registry, &name, dry_run),
Commands::Destroy { name } => cmd_destroy(&registry, &name, dry_run, yes),
Commands::List => cmd_list(&registry),
Commands::Status { name } => cmd_status(&registry, name.as_deref()),
Commands::Import { name } => cmd_import(&registry, name.as_deref()),
Commands::Validate => cmd_validate(&registry),
Commands::Template { action } => match action {
TemplateAction::List => cmd_template_list(&registry),
TemplateAction::Show { name } => cmd_template_show(&registry, &name),
@ -152,9 +168,53 @@ fn main() -> miette::Result<()> {
PublisherAction::Add { name, origin } => cmd_publisher_add(&registry, &name, &origin),
PublisherAction::Remove { name } => cmd_publisher_remove(&registry, &name),
},
Commands::Completions { shell } => {
clap_complete::generate(
shell,
&mut Cli::command(),
"zmgr",
&mut std::io::stdout(),
);
Ok(())
}
}
}
// --- Confirmation helpers ---
/// Ask the user to type the zone name to confirm a destructive operation.
/// Returns Ok(true) if confirmed, Ok(false) if declined.
fn confirm_destroy(name: &str, zone: &Zone) -> Result<bool> {
if !std::io::stdin().is_terminal() {
eprintln!("error: destructive operation requires --yes (-y) in non-interactive mode");
return Ok(false);
}
println!("Zone '{name}' will be permanently destroyed:");
for net in &zone.nets {
println!(" net \"{}\": {} via {}", net.name, net.address, net.vnic);
}
println!();
println!(" - zoneadm halt + uninstall");
println!(" - zonecfg delete");
for net in &zone.nets {
println!(" - dladm delete-vnic {}", net.vnic);
}
println!(" - remove registry entry");
println!();
print!("Type zone name to confirm: ");
std::io::stdout().flush().map_err(ZmgrError::Io)?;
let mut input = String::new();
std::io::stdin()
.read_line(&mut input)
.map_err(ZmgrError::Io)?;
Ok(input.trim() == name)
}
// --- Commands ---
fn cmd_init(registry: &Path, force: bool) -> Result<()> {
if registry.join("config.kdl").exists() && !force {
return Err(ZmgrError::AlreadyInitialized {
@ -214,13 +274,17 @@ fn cmd_create(
let gateway = pool.gateway.to_string();
let stub = pool.stub.clone();
zone_nets.push((tnet, pool, ZoneNet {
zone_nets.push((
tnet,
pool,
ZoneNet {
name: tnet.name.clone(),
address,
gateway,
vnic,
stub,
}));
},
));
}
// Build zonecfg commands
@ -282,23 +346,72 @@ fn cmd_create(
println!();
println!("Registry entry that would be written:");
println!(" {}/zones/{name}.kdl", registry.display());
println!();
println!("Note: IP allocation is tentative. Actual address may differ if");
println!("other zones are created first.");
return Ok(());
}
// --- Execute with step-by-step progress ---
let total_steps = zone_nets.len() + 3 + if tmpl.autoboot { 1 } else { 0 };
let mut step = 0;
println!("Creating zone '{name}' (template: {tmpl_name})...");
// Track created VNICs for rollback
let mut created_vnics: Vec<String> = Vec::new();
// Create VNICs
for (_, _, znet) in &zone_nets {
exec::dladm_create_vnic(&znet.vnic, &znet.stub)?;
step += 1;
print!(" [{step}/{total_steps}] Creating VNIC {} on {}... ", znet.vnic, znet.stub);
std::io::stdout().flush().map_err(ZmgrError::Io)?;
match exec::dladm_create_vnic(&znet.vnic, &znet.stub) {
Ok(()) => {
println!("done");
created_vnics.push(znet.vnic.clone());
}
Err(e) => {
println!("FAILED");
print_partial_failure(name, &created_vnics, false, false);
return Err(e);
}
}
}
exec::zonecfg_create(name, &zonecfg_cmds)?;
exec::zoneadm_install(name)?;
// Configure zone
step += 1;
print!(" [{step}/{total_steps}] Configuring zone via zonecfg... ");
std::io::stdout().flush().map_err(ZmgrError::Io)?;
match exec::zonecfg_create(name, &zonecfg_cmds) {
Ok(()) => println!("done"),
Err(e) => {
println!("FAILED");
print_partial_failure(name, &created_vnics, false, false);
return Err(e);
}
}
// Install zone
step += 1;
print!(" [{step}/{total_steps}] Installing zone (this may take a while)... ");
std::io::stdout().flush().map_err(ZmgrError::Io)?;
match exec::zoneadm_install(name) {
Ok(()) => println!("done"),
Err(e) => {
println!("FAILED");
print_partial_failure(name, &created_vnics, true, false);
return Err(e);
}
}
// Write registry entry
step += 1;
print!(" [{step}/{total_steps}] Writing registry entry... ");
std::io::stdout().flush().map_err(ZmgrError::Io)?;
let today = chrono::Local::now().format("%Y-%m-%d").to_string();
let nets: Vec<ZoneNet> = zone_nets
.into_iter()
.map(|(_, _, znet)| znet)
.collect();
let nets: Vec<ZoneNet> = zone_nets.into_iter().map(|(_, _, znet)| znet).collect();
let zone = Zone {
name: name.to_string(),
template: tmpl_name.to_string(),
@ -306,26 +419,61 @@ fn cmd_create(
created: today,
};
zone.write(registry)?;
println!("done");
println!("Created zone '{name}'");
println!(" template: {tmpl_name}");
println!(" brand: {}", tmpl.brand);
for net in &zone.nets {
println!(" net \"{}\": {} via {} ({})", net.name, net.address, net.vnic, net.gateway);
// Boot if autoboot
if tmpl.autoboot {
step += 1;
print!(" [{step}/{total_steps}] Booting zone... ");
std::io::stdout().flush().map_err(ZmgrError::Io)?;
exec::zoneadm_boot(name)?;
println!("done");
}
if tmpl.autoboot {
exec::zoneadm_boot(name)?;
println!(" status: booted");
} else {
println!(" status: installed (autoboot=false)");
println!(" hint: run `zoneadm -z {name} boot` to start");
println!();
println!("Zone '{name}' created successfully.");
for net in &zone.nets {
println!(
" net \"{}\": {} via {} (gw {})",
net.name, net.address, net.vnic, net.gateway
);
}
Ok(())
}
fn cmd_destroy(registry: &Path, name: &str, dry_run: bool) -> Result<()> {
/// Print cleanup guidance after a partial failure during create.
fn print_partial_failure(
name: &str,
created_vnics: &[String],
zone_configured: bool,
zone_installed: bool,
) {
eprintln!();
eprintln!("Partial state left behind:");
if zone_installed {
eprintln!(" - zone '{name}' is installed");
}
if zone_configured {
eprintln!(" - zone '{name}' is configured in zonecfg");
}
for vnic in created_vnics {
eprintln!(" - VNIC '{vnic}' was created");
}
eprintln!();
eprintln!("To clean up manually:");
if zone_installed {
eprintln!(" zoneadm -z {name} uninstall -F");
}
if zone_configured {
eprintln!(" echo 'delete -F' | zonecfg -z {name}");
}
for vnic in created_vnics {
eprintln!(" dladm delete-vnic {vnic}");
}
}
fn cmd_destroy(registry: &Path, name: &str, dry_run: bool, yes: bool) -> Result<()> {
config::ensure_initialized(registry)?;
let zone = Zone::load(registry, name)?;
@ -352,30 +500,77 @@ fn cmd_destroy(registry: &Path, name: &str, dry_run: bool) -> Result<()> {
return Ok(());
}
// Halt if running (ignore errors — zone may already be halted)
let _ = exec::zoneadm_halt(name);
// Require confirmation unless --yes
if !yes {
if !confirm_destroy(name, &zone)? {
println!("Aborted.");
return Ok(());
}
println!();
}
let vnic_count = zone.nets.len();
let total_steps = 3 + vnic_count + 1; // halt+uninstall+zonecfg + vnics + registry
let mut step = 0;
println!("Destroying zone '{name}'...");
// Halt
step += 1;
print!(" [{step}/{total_steps}] Halting zone... ");
std::io::stdout().flush().map_err(ZmgrError::Io)?;
match exec::zoneadm_halt(name) {
Ok(()) => println!("done"),
Err(_) => println!("skipped (not running)"),
}
// Uninstall
if let Err(e) = exec::zoneadm_uninstall(name) {
eprintln!("warning: zoneadm uninstall: {e}");
step += 1;
print!(" [{step}/{total_steps}] Uninstalling zone... ");
std::io::stdout().flush().map_err(ZmgrError::Io)?;
match exec::zoneadm_uninstall(name) {
Ok(()) => println!("done"),
Err(e) => {
println!("warning");
eprintln!(" {e}");
}
}
// Delete zonecfg
if let Err(e) = exec::zonecfg_delete(name) {
eprintln!("warning: zonecfg delete: {e}");
step += 1;
print!(" [{step}/{total_steps}] Deleting zone configuration... ");
std::io::stdout().flush().map_err(ZmgrError::Io)?;
match exec::zonecfg_delete(name) {
Ok(()) => println!("done"),
Err(e) => {
println!("warning");
eprintln!(" {e}");
}
}
// Delete all VNICs
// Delete VNICs
for net in &zone.nets {
if let Err(e) = exec::dladm_delete_vnic(&net.vnic) {
eprintln!("warning: dladm delete-vnic {}: {e}", net.vnic);
step += 1;
print!(" [{step}/{total_steps}] Deleting VNIC {}... ", net.vnic);
std::io::stdout().flush().map_err(ZmgrError::Io)?;
match exec::dladm_delete_vnic(&net.vnic) {
Ok(()) => println!("done"),
Err(e) => {
println!("warning");
eprintln!(" {e}");
}
}
}
// Remove from registry
step += 1;
print!(" [{step}/{total_steps}] Removing registry entry... ");
std::io::stdout().flush().map_err(ZmgrError::Io)?;
Zone::delete(registry, name)?;
println!("done");
println!("Destroyed zone '{name}'");
println!();
println!("Zone '{name}' destroyed.");
Ok(())
}
@ -469,7 +664,8 @@ fn cmd_status(registry: &Path, name: Option<&str>) -> Result<()> {
let addrs: Vec<&str> = r.nets.iter().map(|n| n.address.as_str()).collect();
println!(
"{:<20} {:<10} {:<12} {:<10} {}",
r.name, r.template, state, "yes", addrs.join(", ")
r.name, r.template, state, "yes",
addrs.join(", ")
);
seen.insert(r.name.clone());
}
@ -507,6 +703,18 @@ fn cmd_import(registry: &Path, name: Option<&str>) -> Result<()> {
Ok(())
}
fn cmd_validate(registry: &Path) -> Result<()> {
config::ensure_initialized(registry)?;
println!("Validating registry at {}...", registry.display());
let ok = validate::run(registry)?;
if !ok {
std::process::exit(1);
}
Ok(())
}
fn cmd_template_list(registry: &Path) -> Result<()> {
config::ensure_initialized(registry)?;
@ -587,7 +795,6 @@ fn cmd_pool_show(registry: &Path, name: &str) -> Result<()> {
println!(" stub: {}", pool.stub);
println!(" addresses: {}", pool.source_description());
// Find allocations: zone nets whose IP falls in this pool
let mut allocations = Vec::new();
for z in &zones {
for net in &z.nets {

241
src/validate.rs Normal file
View file

@ -0,0 +1,241 @@
use std::collections::HashMap;
use std::net::Ipv4Addr;
use std::path::Path;
use crate::config::Config;
use crate::error::Result;
use crate::pool::Pool;
use crate::template::Template;
use crate::zone::Zone;
struct ValidationResult {
errors: Vec<String>,
warnings: Vec<String>,
}
impl ValidationResult {
fn new() -> Self {
Self {
errors: Vec::new(),
warnings: Vec::new(),
}
}
fn error(&mut self, msg: impl Into<String>) {
self.errors.push(msg.into());
}
fn warn(&mut self, msg: impl Into<String>) {
self.warnings.push(msg.into());
}
}
/// Run all validation checks and print results. Returns true if no errors.
pub fn run(registry: &Path) -> Result<bool> {
let mut result = ValidationResult::new();
// 1. Config
print!(" config.kdl ");
match Config::load(registry) {
Ok(cfg) => {
println!("{:.<30} ok", "");
validate_config(&cfg, registry, &mut result);
}
Err(e) => {
println!("{:.<30} ERROR", "");
result.error(format!("config.kdl: {e}"));
}
}
// 2. Templates
let template_names = Template::list(registry).unwrap_or_default();
let mut templates: HashMap<String, Template> = HashMap::new();
for name in &template_names {
let label = format!("templates/{name}.kdl ");
print!(" {label}");
match Template::load(registry, name) {
Ok(tmpl) => {
println!("{:.<30} ok", "");
templates.insert(name.clone(), tmpl);
}
Err(e) => {
println!("{:.<30} ERROR", "");
result.error(format!("templates/{name}.kdl: {e}"));
}
}
}
// 3. Pools
let pool_names = Pool::list(registry).unwrap_or_default();
let mut pools: HashMap<String, Pool> = HashMap::new();
for name in &pool_names {
let label = format!("pools/{name}.kdl ");
print!(" {label}");
match Pool::load(registry, name) {
Ok(pool) => {
println!("{:.<30} ok", "");
pools.insert(name.clone(), pool);
}
Err(e) => {
println!("{:.<30} ERROR", "");
result.error(format!("pools/{name}.kdl: {e}"));
}
}
}
// 4. Zones
let zones = Zone::list(registry).unwrap_or_default();
let zones_dir = registry.join("zones");
if zones_dir.exists() {
let count = zones.len();
print!(" zones/ ({count} zones) ");
println!("{:.<25} ok", "");
}
// --- Cross-file validation ---
// Template → Pool referential integrity
for (tname, tmpl) in &templates {
for net in &tmpl.nets {
if !pools.contains_key(&net.pool) {
result.error(format!(
"templates/{tname}.kdl: net '{}' references pool '{}' which does not exist",
net.name, net.pool
));
}
}
}
// Config → Template referential integrity
if let Ok(cfg) = Config::load(registry) {
if !templates.contains_key(&cfg.default_template) && !template_names.is_empty() {
result.warn(format!(
"config.kdl: default-template '{}' does not exist",
cfg.default_template
));
}
}
// Pool sanity checks
for (pname, pool) in &pools {
// Gateway in network
if !pool.network.contains(&pool.gateway) {
result.error(format!(
"pools/{pname}.kdl: gateway {} is not in network {}",
pool.gateway, pool.network
));
}
// Range/addresses within network
match &pool.source {
crate::pool::AddressSource::Range { start, end } => {
if !pool.network.contains(start) {
result.error(format!(
"pools/{pname}.kdl: range-start {start} is not in network {}",
pool.network
));
}
if !pool.network.contains(end) {
result.error(format!(
"pools/{pname}.kdl: range-end {end} is not in network {}",
pool.network
));
}
if u32::from(*start) > u32::from(*end) {
result.error(format!(
"pools/{pname}.kdl: range-start {start} is after range-end {end}"
));
}
}
crate::pool::AddressSource::List(addrs) => {
for addr in addrs {
if !pool.network.contains(addr) {
result.error(format!(
"pools/{pname}.kdl: address {addr} is not in network {}",
pool.network
));
}
}
}
}
// Pool utilization warning
let used: usize = zones
.iter()
.flat_map(|z| &z.nets)
.filter_map(|n| {
n.address
.split('/')
.next()
.and_then(|s| s.parse::<Ipv4Addr>().ok())
})
.filter(|ip| pool.network.contains(ip))
.count();
let total = pool.total_addresses() as usize;
if total > 0 {
let pct = (used * 100) / total;
if pct >= 90 {
result.warn(format!(
"pools/{pname}.kdl: {used}/{total} addresses allocated ({pct}%)"
));
}
}
}
// Duplicate IP check across all zones
let mut ip_owners: HashMap<Ipv4Addr, Vec<String>> = HashMap::new();
for z in &zones {
for net in &z.nets {
if let Some(ip) = net
.address
.split('/')
.next()
.and_then(|s| s.parse::<Ipv4Addr>().ok())
{
ip_owners
.entry(ip)
.or_default()
.push(format!("{}:{}", z.name, net.name));
}
}
}
for (ip, owners) in &ip_owners {
if owners.len() > 1 {
result.error(format!(
"zones: duplicate IP {ip} allocated to: {}",
owners.join(", ")
));
}
}
// Print summary
println!();
if result.warnings.is_empty() && result.errors.is_empty() {
println!("No issues found.");
} else {
for w in &result.warnings {
println!(" warning: {w}");
}
for e in &result.errors {
println!(" error: {e}");
}
println!();
println!(
"{} error(s), {} warning(s)",
result.errors.len(),
result.warnings.len()
);
}
Ok(result.errors.is_empty())
}
fn validate_config(cfg: &Config, _registry: &Path, result: &mut ValidationResult) {
// Check zonepath prefix is absolute
if !cfg.zonepath_prefix.starts_with('/') {
result.warn(format!(
"config.kdl: zonepath-prefix '{}' is not an absolute path",
cfg.zonepath_prefix
));
}
}