From f5670f20be108a1e3ba5cac2a8805d155001c9b6 Mon Sep 17 00:00:00 2001 From: Maxime Van Hees Date: Wed, 3 Sep 2025 15:38:52 +0200 Subject: [PATCH] rewrite builder pattern + clean script as template --- packages/system/virt/src/cloudhv/mod.rs | 41 +--------- packages/system/virt/src/image_prep/mod.rs | 4 +- packages/system/virt/src/rhai/cloudhv.rs | 16 ++++ .../system/virt/src/rhai/cloudhv_builder.rs | 71 +++++++++------- .../virt/tests/rhai/vm_clean_launch.rhai | 82 +++++++++++++++++++ 5 files changed, 144 insertions(+), 70 deletions(-) create mode 100644 packages/system/virt/tests/rhai/vm_clean_launch.rhai diff --git a/packages/system/virt/src/cloudhv/mod.rs b/packages/system/virt/src/cloudhv/mod.rs index 0bb2b8f..af0a9c6 100644 --- a/packages/system/virt/src/cloudhv/mod.rs +++ b/packages/system/virt/src/cloudhv/mod.rs @@ -427,10 +427,8 @@ pub fn vm_start(id: &str) -> Result<(), CloudHvError> { ipv6_bridge_cidr = Some(cidr); } else { let if_hint = nat.mycelium_if.clone().unwrap_or_else(|| "mycelium".into()); - println!("auto-deriving mycelium address..."); let (_ifname, myc_addr) = net::mycelium_ipv6_addr(&if_hint)?; let (_pfx, router_cidr) = net::derive_ipv6_prefix_from_mycelium(&myc_addr)?; - println!("derived router cidr for bridge: {}", router_cidr); ipv6_bridge_cidr = Some(router_cidr); } } @@ -467,9 +465,7 @@ pub fn vm_start(id: &str) -> Result<(), CloudHvError> { // TAP + NIC args let tap_name = net::ensure_tap_for_vm(&nat.bridge_name, id)?; - println!("TAP device for vm called: {tap_name}"); let mac = net::stable_mac_from_id(id); - println!("MAC for vm: {mac}"); parts.push("--net".into()); parts.push(format!("tap={},mac={}", tap_name, mac)); } @@ -484,9 +480,7 @@ pub fn vm_start(id: &str) -> Result<(), CloudHvError> { net::ensure_bridge(&bridge_name, &bridge_addr_cidr, opts.bridge_ipv6_cidr.as_deref())?; // TAP + NIC only, no NAT/DHCP let tap_name = net::ensure_tap_for_vm(&bridge_name, id)?; - println!("TAP device for vm called: {tap_name}"); let mac = net::stable_mac_from_id(id); - println!("MAC for vm: {mac}"); parts.push("--net".into()); parts.push(format!("tap={},mac={}", tap_name, mac)); @@ -516,9 +510,8 @@ pub fn vm_start(id: &str) -> Result<(), CloudHvError> { log_file, vm_pid_path(id).to_string_lossy() ); - println!("executing command:\n{heredoc}"); // Execute command; this will background cloud-hypervisor and return - let result = sal_process::run(&heredoc).execute(); + let result = sal_process::run(&heredoc).silent(true).execute(); match result { Ok(res) => { if !res.success { @@ -541,7 +534,6 @@ pub fn vm_start(id: &str) -> Result<(), CloudHvError> { Ok(s) => s.trim().parse::().ok(), Err(_) => None, }; - println!("reading PID back: {} - (if 0 == not found)", pid.unwrap_or(0)); // Quick health check: ensure process did not exit immediately due to CLI errors (e.g., duplicate flags) if let Some(pid_num) = pid { @@ -549,7 +541,6 @@ pub fn vm_start(id: &str) -> Result<(), CloudHvError> { if !proc_exists(pid_num) { // Tail log to surface the error cause let tail_cmd = format!("tail -n 200 {}", shell_escape(&log_file)); - println!("executing tail_cmd command:\n{tail_cmd}"); let tail = sal_process::run(&tail_cmd).die(false).silent(true).execute(); let mut log_snip = String::new(); if let Ok(res) = tail { @@ -578,10 +569,8 @@ pub fn vm_start(id: &str) -> Result<(), CloudHvError> { let value = serde_json::to_value(&rec).map_err(|e| CloudHvError::JsonError(e.to_string()))?; write_json(&vm_json_path(id), &value)?; - println!("wrote JSON for VM"); - // Best-effort: discover and print guest IPv4/IPv6 addresses (default-net path) - println!("waiting 5 secs for DHCP/ND"); + // Best-effort: discover guest IPv4/IPv6 addresses (default-net path) thread::sleep(Duration::from_millis(5000)); let mac_lower = net::stable_mac_from_id(id).to_lowercase(); @@ -591,29 +580,7 @@ pub fn vm_start(id: &str) -> Result<(), CloudHvError> { .unwrap_or_else(|_| format!("/var/lib/misc/dnsmasq-hero-{}.leases", bridge_name)) }); let ipv4 = net::discover_ipv4_from_leases(&lease_path, &mac_lower, 12); - println!( - "Got IPv4 from dnsmasq lease ({}): {}", - lease_path, - ipv4.clone().unwrap_or("not found".to_string()) - ); - let ipv6 = net::discover_ipv6_on_bridge(&bridge_name, &mac_lower); - println!( - "Got IPv6 from neighbor table on bridge: {}", - ipv6.clone().unwrap_or("not found".to_string()) - ); - - println!( - "[cloudhv] VM '{}' guest addresses: IPv4={}, IPv6={}", - id, - ipv4.as_deref().unwrap_or(""), - ipv6.as_deref().unwrap_or("") - ); - } else { - println!( - "[cloudhv] VM '{}' guest addresses discovery skipped (no default bridge in use)", - id - ); } Ok(()) @@ -828,7 +795,6 @@ fn get_mycelium_ipv6_addr(iface_hint: &str) -> Result<(String, String), CloudHvE let parts: Vec<&str> = lt.split_whitespace().collect(); if let Some(addr_cidr) = parts.get(1) { let addr_only = addr_cidr.split('/').next().unwrap_or("").trim(); - println!("got addr from host: {addr_only}"); if !addr_only.is_empty() && addr_only.parse::().is_ok() { return Ok((iface, addr_only.to_string())); } @@ -981,8 +947,7 @@ fi // Use a unique heredoc delimiter to avoid clashing with inner < Ok(()), Ok(res) => Err(CloudHvError::CommandFailed(format!( diff --git a/packages/system/virt/src/image_prep/mod.rs b/packages/system/virt/src/image_prep/mod.rs index 4fb9b1b..22cc02d 100644 --- a/packages/system/virt/src/image_prep/mod.rs +++ b/packages/system/virt/src/image_prep/mod.rs @@ -731,9 +731,7 @@ exit 0 disable_ci_net = if disable_ci_net { "true" } else { "false" } ); - // image prep script printout for debugging: - println!("{script}"); - + // image prep script executed silently let res = run_script(&script)?; // Prefer a RESULT:-prefixed line (robust against extra stdout noise) let mut marker: Option = None; diff --git a/packages/system/virt/src/rhai/cloudhv.rs b/packages/system/virt/src/rhai/cloudhv.rs index 2461207..d123c1d 100644 --- a/packages/system/virt/src/rhai/cloudhv.rs +++ b/packages/system/virt/src/rhai/cloudhv.rs @@ -170,6 +170,20 @@ pub fn cloudhv_vm_info(id: &str) -> Result> { Ok(vmrecord_to_map(&rec)) } +pub fn cloudhv_discover_ipv4_from_leases(lease_path: &str, mac_lower: &str, timeout_secs: i64) -> Dynamic { + match crate::cloudhv::net::discover_ipv4_from_leases(lease_path, mac_lower, timeout_secs as u64) { + Some(ip) => ip.into(), + None => Dynamic::UNIT, + } +} + +pub fn cloudhv_discover_ipv6_on_bridge(bridge_name: &str, mac_lower: &str) -> Dynamic { + match crate::cloudhv::net::discover_ipv6_on_bridge(bridge_name, mac_lower) { + Some(ip) => ip.into(), + None => Dynamic::UNIT, + } +} + // Module registration pub fn register_cloudhv_module(engine: &mut Engine) -> Result<(), Box> { @@ -179,5 +193,7 @@ pub fn register_cloudhv_module(engine: &mut Engine) -> Result<(), Box CloudHvBuilder { +// Improved functional-style builder with better method names for fluent feel +fn cloudhv_builder(id: &str) -> CloudHvBuilder { CloudHvBuilder::new(id) } -// Functional, chainable-style helpers (consume and return the builder) -fn builder_memory_mb(mut b: CloudHvBuilder, mb: i64) -> CloudHvBuilder { +fn memory_mb(b: CloudHvBuilder, mb: i64) -> CloudHvBuilder { + let mut b = b; if mb > 0 { b.memory_mb(mb as u32); } b } -fn builder_vcpus(mut b: CloudHvBuilder, v: i64) -> CloudHvBuilder { +fn vcpus(b: CloudHvBuilder, v: i64) -> CloudHvBuilder { + let mut b = b; if v > 0 { b.vcpus(v as u32); } b } -fn builder_disk(mut b: CloudHvBuilder, path: &str) -> CloudHvBuilder { +fn disk(b: CloudHvBuilder, path: &str) -> CloudHvBuilder { + let mut b = b; b.disk(path); b } -fn builder_disk_from_flavor(mut b: CloudHvBuilder, flavor: &str) -> CloudHvBuilder { +fn disk_from_flavor(b: CloudHvBuilder, flavor: &str) -> CloudHvBuilder { + let mut b = b; b.disk_from_flavor(flavor); b } -fn builder_cmdline(mut b: CloudHvBuilder, c: &str) -> CloudHvBuilder { +fn cmdline(b: CloudHvBuilder, c: &str) -> CloudHvBuilder { + let mut b = b; b.cmdline(c); b } -fn builder_extra_arg(mut b: CloudHvBuilder, a: &str) -> CloudHvBuilder { +fn extra_arg(b: CloudHvBuilder, a: &str) -> CloudHvBuilder { + let mut b = b; b.extra_arg(a); b } -fn builder_no_default_net(mut b: CloudHvBuilder) -> CloudHvBuilder { +fn no_default_net(b: CloudHvBuilder) -> CloudHvBuilder { + let mut b = b; b.no_default_net(); b } -// New networking profile helpers -fn builder_network_default_nat(mut b: CloudHvBuilder) -> CloudHvBuilder { +fn network_default_nat(b: CloudHvBuilder) -> CloudHvBuilder { + let mut b = b; b.network_default_nat(); b } -fn builder_network_none(mut b: CloudHvBuilder) -> CloudHvBuilder { + +fn network_none(b: CloudHvBuilder) -> CloudHvBuilder { + let mut b = b; b.network_none(); b } -fn builder_network_bridge_only(mut b: CloudHvBuilder) -> CloudHvBuilder { + +fn network_bridge_only(b: CloudHvBuilder) -> CloudHvBuilder { + let mut b = b; b.network_bridge_only(); b } -fn builder_network_custom(mut b: CloudHvBuilder, args: Array) -> CloudHvBuilder { + +fn network_custom(b: CloudHvBuilder, args: Array) -> CloudHvBuilder { + let mut b = b; let mut v: Vec = Vec::new(); for it in args { if it.is_string() { @@ -71,7 +84,7 @@ fn builder_network_custom(mut b: CloudHvBuilder, args: Array) -> CloudHvBuilder b } -fn builder_launch(mut b: CloudHvBuilder) -> Result> { +fn launch(mut b: CloudHvBuilder) -> Result> { b.launch().map_err(|e| { Box::new(EvalAltResult::ErrorRuntime( format!("cloudhv builder launch failed: {}", e).into(), @@ -141,24 +154,24 @@ pub fn register_cloudhv_builder_module(engine: &mut Engine) -> Result<(), Box("CloudHvBuilder"); // Factory - engine.register_fn("cloudhv_builder", builder_new); + engine.register_fn("cloudhv_builder", cloudhv_builder); - // Chainable methods (functional style) - engine.register_fn("memory_mb", builder_memory_mb); - engine.register_fn("vcpus", builder_vcpus); - engine.register_fn("disk", builder_disk); - engine.register_fn("disk_from_flavor", builder_disk_from_flavor); - engine.register_fn("cmdline", builder_cmdline); - engine.register_fn("extra_arg", builder_extra_arg); - engine.register_fn("no_default_net", builder_no_default_net); + // Chainable methods (fluent functional style) + engine.register_fn("memory_mb", memory_mb); + engine.register_fn("vcpus", vcpus); + engine.register_fn("disk", disk); + engine.register_fn("disk_from_flavor", disk_from_flavor); + engine.register_fn("cmdline", cmdline); + engine.register_fn("extra_arg", extra_arg); + engine.register_fn("no_default_net", no_default_net); // Networking profiles - engine.register_fn("network_default_nat", builder_network_default_nat); - engine.register_fn("network_none", builder_network_none); - engine.register_fn("network_bridge_only", builder_network_bridge_only); - engine.register_fn("network_custom", builder_network_custom); + engine.register_fn("network_default_nat", network_default_nat); + engine.register_fn("network_none", network_none); + engine.register_fn("network_bridge_only", network_bridge_only); + engine.register_fn("network_custom", network_custom); // Action - engine.register_fn("launch", builder_launch); + engine.register_fn("launch", launch); // One-shot wrapper engine.register_fn("vm_easy_launch", vm_easy_launch); diff --git a/packages/system/virt/tests/rhai/vm_clean_launch.rhai b/packages/system/virt/tests/rhai/vm_clean_launch.rhai new file mode 100644 index 0000000..87cdc89 --- /dev/null +++ b/packages/system/virt/tests/rhai/vm_clean_launch.rhai @@ -0,0 +1,82 @@ +// Clean VM Launch Script +// Creates a VM using builder pattern with concise output + +let vm_id = "vm-clean-test"; + +// Phase 1: Host check +print("Checking system requirements..."); +let hc = host_check(); +if !(hc.ok == true) { + print("❌ System check failed - missing dependencies:"); + if hc.critical != () && hc.critical.len() > 0 { + print("Critical:"); + for dep in hc.critical { + print(" - " + dep); + } + } + if hc.optional != () && hc.optional.len() > 0 { + print("Optional:"); + for dep in hc.optional { + print(" - " + dep); + } + } + throw "Host check failed: missing dependencies"; +} +print("✅ System requirements met"); + +// Phase 2: Create VM using fluent builder pattern +print("Preparing Ubuntu image and configuring VM..."); +let vm_id_actual = ""; +try { + vm_id_actual = cloudhv_builder(vm_id) + .disk_from_flavor("ubuntu") + .network_default_nat() + .memory_mb(4096) + .vcpus(2) + .launch(); +} catch (e) { + throw "VM launch failed: " + e.to_string(); +} + +// Phase 3: Wait for VM to boot and get network configuration +print("✅ VM launched successfully"); +print("⏳ Waiting 10 seconds for VM to boot and configure network..."); +sleep(10); + +// Phase 4: Discover VM IP addresses +print("🔍 Discovering VM network addresses..."); +let mac_addr = "a2:26:1e:ac:96:3a"; // This should be derived from vm_id_actual +let ipv4 = cloudhv_discover_ipv4_from_leases("/var/lib/misc/dnsmasq-hero-br-hero.leases", mac_addr, 30); +let ipv6 = cloudhv_discover_ipv6_on_bridge("br-hero", mac_addr); + +// Phase 5: Display connection info +print("✅ VM " + vm_id_actual + " is ready!"); +print(""); +print("🌐 Network Information:"); +if ipv4 != () && ipv4 != "" { + print(" IPv4: " + ipv4); +} else { + print(" IPv4: Not assigned yet (VM may still be configuring)"); +} +if ipv6 != () && ipv6 != "" { + print(" IPv6: " + ipv6); +} else { + print(" IPv6: Not available"); +} +print(""); +print("💡 VM is running in the background. To connect:"); +print(" SSH: ssh ubuntu@" + (if ipv4 != () && ipv4 != "" { ipv4 } else { "" })); +print(""); +print("🛑 To stop the VM later:"); +print(" cloudhv_vm_stop(\"" + vm_id_actual + "\", false);"); +print(" cloudhv_vm_delete(\"" + vm_id_actual + "\", true);"); + +/* +try { + cloudhv_vm_stop(vm_id_actual, false); + cloudhv_vm_delete(vm_id_actual, true); + print("VM stopped and cleaned up."); +} catch (e) { + print("Warning: cleanup failed: " + e.to_string()); +} +*/ \ No newline at end of file