diff --git a/docs/content/docs/inventory/_index.md b/docs/content/docs/inventory/_index.md index 2eb6cc2..dec87d5 100644 --- a/docs/content/docs/inventory/_index.md +++ b/docs/content/docs/inventory/_index.md @@ -38,6 +38,33 @@ So a secrets inventory loaded second can add (or override) variables without los > [!NOTE] > Each `-i` directory must contain a `groups/` subdirectory (it may be empty), or Jetpack will refuse to load it. +## Provision overlays + +A `provision:` block in `group_vars/` deep-merges onto each member host's +`host_vars` provision config — so you can set or override provision fields for a +whole fleet from one file. Host-specific fields win on conflict, exactly like +every other variable (more-specific wins). The same works ad-hoc from the CLI +with `-e` / `--extra-vars` (highest precedence): `-e provision.state=destroyed` +overrides every host's lifecycle state for that run, no inventory file touched. +(Values take the convenient `key=value` form, with dotted keys for nesting; JSON +and `@file` are still accepted for typed/structured values.) + +The common use is toggling a fleet's lifecycle state. Put each host's real +provision spec in its `host_vars` (no `state`, so it defaults to `present`), then +drive the whole group from one `group_vars` file: + +```yaml +# group_vars/test-k8s +provision: + state: destroyed # tear every member down (remove this block, or use + # `present`, to create them again) +``` + +Because the `host_vars` provision fields take precedence, only `state` is +overridden — `type`, `cluster`, `ip`, and the rest still come from each host. +This is how a repeatable test harness can reset and recreate a cluster by +flipping a single file between `destroyed` and `present`. + ## Inspecting inventory See exactly what an inventory resolves to before running anything: diff --git a/src/api.rs b/src/api.rs index a66b88b..c1a6c1d 100644 --- a/src/api.rs +++ b/src/api.rs @@ -75,8 +75,12 @@ impl PlaybookRunner { } else { let inv = Arc::new(RwLock::new(Inventory::new())); if !self.config.inventory_paths.read().unwrap().is_empty() { - load_inventory(&inv, Arc::clone(&self.config.inventory_paths)) - .map_err(JetpackError::Inventory)?; + load_inventory( + &inv, + Arc::clone(&self.config.inventory_paths), + self.config.extra_vars.clone(), + ) + .map_err(JetpackError::Inventory)?; } inv }; diff --git a/src/cli/confirm.rs b/src/cli/confirm.rs index 8f560a5..314360c 100644 --- a/src/cli/confirm.rs +++ b/src/cli/confirm.rs @@ -9,11 +9,16 @@ //! is skipped and the run proceeds as before. use crate::cli::parser::CliParser; +use crate::connection::no::NoFactory; use crate::inventory::hosts::Host; use crate::inventory::inventory::Inventory; +use crate::playbooks::context::PlaybookContext; +use crate::playbooks::traversal::{RunState, resolve_playbook_targets}; +use crate::playbooks::visitor::{CheckMode, PlaybookVisitor}; use crate::provisioners::ProvisionState; +use std::collections::{HashMap, HashSet}; use std::io::{IsTerminal, Write}; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, Mutex, RwLock}; /// Mutating execution modes that can actually destroy resources. The dry-run / /// validation modes (plan/check-*/syntax/simulate/show/pull) never mutate, so @@ -60,17 +65,19 @@ fn host_in_limit_scope(host: &Host, parser: &CliParser) -> bool { } /// Sorted list of in-scope hosts a run would destroy — those whose -/// `provision.state` is `absent` or `destroyed` and that pass the CLI -/// `--limit-hosts` / `--limit-groups` filters. Pure (no IO) so it is -/// unit-testable. +/// `provision.state` is `absent`/`destroyed`, passing the `--limit` filters, and +/// (when known) actually targeted by the specified playbook's plays. /// -/// Note: this cannot account for which groups a playbook's plays target (known -/// only after playbook parsing, post-dispatch), so without `--limit` it may -/// name destroy-declared hosts no play actually touches. That is a -/// safe-direction over-approximation — it never misses a real destroy. +/// `playbook_targets` scopes the list to the hosts the `-p` playbook resolves to, +/// so `apply -p repro.yml` only prompts for what repro.yml touches — not every +/// destroy-declared host in the inventory. It is `None` when no playbook was +/// given or its groups couldn't be resolved; in that case every destroy-bearing +/// host in `--limit` scope is listed, preserving the safe-direction +/// over-approximation (it never misses a real destroy). pub fn destroy_bearing_hosts( inventory: &Arc>, parser: &CliParser, + playbook_targets: Option<&std::collections::HashSet>, ) -> Vec { let inv = inventory.read().unwrap(); let mut names: Vec = inv @@ -78,10 +85,14 @@ pub fn destroy_bearing_hosts( .values() .filter_map(|h| { let h = h.read().unwrap(); - if host_is_destroy_bearing(&h) && host_in_limit_scope(&h, parser) { - Some(h.name.clone()) - } else { - None + if !host_is_destroy_bearing(&h) || !host_in_limit_scope(&h, parser) { + return None; + } + // Scope to the playbook's resolved targets when known; None = unknown, + // so keep the host (over-approximation — never under-prompt). + match playbook_targets { + Some(targets) if !targets.contains(&h.name) => None, + _ => Some(h.name.clone()), } }) .collect(); @@ -103,7 +114,11 @@ pub fn confirm_destroy_if_tty( if !std::io::stdin().is_terminal() { return Ok(()); } - let targets = destroy_bearing_hosts(inventory, parser); + // Scope the prompt to what the -p playbook actually targets (resolved play + // groups), not the whole inventory. None when there's no playbook or its + // groups can't be resolved → destroy_bearing_hosts falls back to inventory-wide. + let playbook_targets = resolve_playbook_targets_for_confirm(inventory, parser); + let targets = destroy_bearing_hosts(inventory, parser, playbook_targets.as_ref()); if targets.is_empty() { return Ok(()); } @@ -130,6 +145,54 @@ pub fn confirm_destroy_if_tty( } } +/// Resolve the `-p` playbook's target host set for the destroy prompt. Returns +/// `None` (→ inventory-wide over-approximation) when there's no playbook or +/// `resolve_playbook_targets` couldn't resolve every play's groups — so the +/// prompt never silently under-counts. +fn resolve_playbook_targets_for_confirm( + inventory: &Arc>, + parser: &CliParser, +) -> Option> { + if parser.playbook_paths.read().unwrap().is_empty() { + return None; + } + let run_state = resolution_run_state(inventory, parser); + match resolve_playbook_targets(&run_state) { + Ok(Some(set)) => Some(set), + Ok(None) | Err(_) => None, + } +} + +/// A `RunState` just capable of group resolution (no real connection factory) — +/// mirrors `playbook_syntax_check`'s build. Used only to drive +/// `resolve_target_groups` / `get_play_hosts` for the destroy prompt. +fn resolution_run_state(inventory: &Arc>, parser: &CliParser) -> Arc { + Arc::new(RunState { + inventory: Arc::clone(inventory), + playbook_paths: Arc::clone(&parser.playbook_paths), + role_paths: Arc::clone(&parser.role_paths), + module_paths: Arc::clone(&parser.module_paths), + limit_hosts: parser.limit_hosts.clone(), + limit_groups: parser.limit_groups.clone(), + batch_size: parser.batch_size, + context: Arc::new(RwLock::new(PlaybookContext::new(parser))), + visitor: Arc::new(RwLock::new(PlaybookVisitor::new(CheckMode::No))), + connection_factory: Arc::new(RwLock::new(NoFactory::new())), + tags: parser.tags.clone(), + allow_localhost_delegation: parser.allow_localhost_delegation, + is_pull_mode: false, + syntax_mode: false, + play_groups: parser.play_groups.clone(), + output_handler: None, + async_mode: parser.async_mode, + playbook_contents: Vec::new(), + processed_role_tasks: Arc::new(RwLock::new(std::collections::HashSet::new())), + processed_role_handlers: Arc::new(RwLock::new(std::collections::HashSet::new())), + role_processing_stack: Arc::new(RwLock::new(Vec::new())), + fetched_files: Arc::new(Mutex::new(HashMap::new())), + }) +} + #[cfg(test)] mod tests { use super::*; @@ -184,7 +247,7 @@ mod tests { ("db-bare", None, &["dbservers"]), ]); let parser = parser_with(&[], &[]); - let got = destroy_bearing_hosts(&inv, &parser); + let got = destroy_bearing_hosts(&inv, &parser, None); assert_eq!( got, vec!["web-absent".to_string(), "web-destroyed".to_string()] @@ -198,7 +261,7 @@ mod tests { ("db", None, &["dbservers"]), ]); let parser = parser_with(&[], &[]); - assert!(destroy_bearing_hosts(&inv, &parser).is_empty()); + assert!(destroy_bearing_hosts(&inv, &parser, None).is_empty()); } #[test] @@ -210,7 +273,7 @@ mod tests { // --limit web-destroyed only let parser = parser_with(&["web-destroyed"], &[]); assert_eq!( - destroy_bearing_hosts(&inv, &parser), + destroy_bearing_hosts(&inv, &parser, None), vec!["web-destroyed".to_string()] ); } @@ -224,8 +287,37 @@ mod tests { // --limit webservers only let parser = parser_with(&[], &["webservers"]); assert_eq!( - destroy_bearing_hosts(&inv, &parser), + destroy_bearing_hosts(&inv, &parser, None), vec!["web-destroyed".to_string()] ); } + + #[test] + fn playbook_targets_scope_the_destroy_prompt() { + let inv = inventory_with(&[ + ("web-destroyed", Some("destroyed"), &["webservers"]), + ("db-destroyed", Some("destroyed"), &["dbservers"]), + ("web-present", Some("present"), &["webservers"]), + ]); + let parser = parser_with(&[], &[]); + // A -p playbook resolving to only the webservers' hosts. + let targets: HashSet = ["web-destroyed", "web-present"] + .iter() + .map(|s| s.to_string()) + .collect(); + // Only the destroy-bearing host the playbook targets is prompted; the + // db-destroyed host (destroy-bearing, but not targeted) is excluded. + assert_eq!( + destroy_bearing_hosts(&inv, &parser, Some(&targets)), + vec!["web-destroyed".to_string()] + ); + // None (no playbook, or unresolvable groups) → inventory-wide + // over-approximation: every destroy-bearing host in scope is listed. + let mut all = destroy_bearing_hosts(&inv, &parser, None); + all.sort(); + assert_eq!( + all, + vec!["db-destroyed".to_string(), "web-destroyed".to_string()] + ); + } } diff --git a/src/cli/parser.rs b/src/cli/parser.rs index ff06c51..802ec09 100644 --- a/src/cli/parser.rs +++ b/src/cli/parser.rs @@ -19,7 +19,7 @@ include!(concat!(env!("OUT_DIR"), "/version.rs")); use crate::cli::config_file::{self, JetpackFileConfig}; -use crate::inventory::loading::convert_json_vars; +use crate::inventory::loading::{convert_json_vars, extra_vars_from_key_value}; use crate::util::io::directory_as_string; use crate::util::io::jet_file_open; use crate::util::io::read_local_file; @@ -1574,16 +1574,15 @@ impl CliParser { } }; blend_variables(&mut self.extra_vars, serde_yaml::Value::Mapping(parsed)); - } else { + } else if let Ok(actual) = serde_json::from_str::(value) { // input is inline JSON (as YAML wouldn't make sense with the newlines) - - let parsed: Result = serde_json::from_str(value); - let actual = match parsed { - Ok(x) => x, - Err(y) => return Err(format!("inline json is not valid: {}", y)), - }; let serde_map = convert_json_vars(&actual); blend_variables(&mut self.extra_vars, serde_yaml::Value::Mapping(serde_map)); + } else { + // key=value, dotted key → nested: the convenient form, e.g. + // `-e provision.state=destroyed` (use JSON for typed values). + let mapping = extra_vars_from_key_value(value)?; + blend_variables(&mut self.extra_vars, serde_yaml::Value::Mapping(mapping)); } Ok(()) diff --git a/src/cli/playbooks.rs b/src/cli/playbooks.rs index 5bc82cb..5b9634f 100644 --- a/src/cli/playbooks.rs +++ b/src/cli/playbooks.rs @@ -103,7 +103,11 @@ pub fn inventory_check(inventory: &Arc>, parser: &CliParser) - println!("inventory-check requires --inventory"); return 1; } - match load_inventory(inventory, Arc::clone(&parser.inventory_paths)) { + match load_inventory( + inventory, + Arc::clone(&parser.inventory_paths), + parser.extra_vars.clone(), + ) { Ok(_) => { let count = inventory.read().expect("inventory read").hosts.len(); println!("inventory OK ({} hosts)", count); diff --git a/src/cli/secrets_diagnostic.rs b/src/cli/secrets_diagnostic.rs index 5a1aab7..f6530f0 100644 --- a/src/cli/secrets_diagnostic.rs +++ b/src/cli/secrets_diagnostic.rs @@ -49,6 +49,8 @@ use crate::playbooks::ref_collector::collect_per_play; const RENDER_BUILTINS: &[&str] = &[ "jet_hostname", "jet_hostname_short", + "inventory_hostname", + "inventory_hostname_short", "jet_play_hosts", "jet_sudo_user", "jet_command", diff --git a/src/dns/mod.rs b/src/dns/mod.rs index 3825845..e480023 100644 --- a/src/dns/mod.rs +++ b/src/dns/mod.rs @@ -20,6 +20,26 @@ pub mod zone; use serde::Deserialize; use std::path::{Path, PathBuf}; use std::process::Command; +use std::sync::{LazyLock, Mutex}; + +/// Serializes DNS zone mutations across threads. +/// +/// Parallel host provisioning calls `add_host_record` / `remove_host_record` +/// concurrently; without this guard the zone-file read-modify-write in +/// `zone.rs` (and the octodns/gravity sync that follows it) would race and lose +/// updates. The record-mutating entry points below hold this lock for their +/// entire body — zone edit plus provider sync — so a host's DNS change is +/// atomic. Internal helpers (`add_ptr_record`, `sync`) deliberately take no +/// lock: they are only reached through these entry points, and +/// `std::sync::Mutex` is not reentrant. +static DNS_WRITE_LOCK: LazyLock> = LazyLock::new(|| Mutex::new(())); + +/// Acquire the process-wide DNS write lock, propagating poison as a `String`. +fn dns_write_lock() -> Result, String> { + DNS_WRITE_LOCK + .lock() + .map_err(|e| format!("DNS write lock poisoned: {}", e)) +} /// Source of truth for IP addresses #[derive(Debug, Clone, Deserialize, PartialEq, Default)] @@ -160,6 +180,7 @@ pub fn dns_config_from_vars( /// Add a DNS A record for a host, plus PTR if configured, and optionally sync /// inventory_name is the FQDN from inventory (e.g., "gravity01.island.lagun.co") pub fn add_host_record(config: &DnsConfig, inventory_name: &str, ip: &str) -> Result { + let _guard = dns_write_lock()?; let zone = config .zone .clone() @@ -211,6 +232,7 @@ pub fn add_host_record(config: &DnsConfig, inventory_name: &str, ip: &str) -> Re /// Remove a DNS A record for a host and optionally sync /// inventory_name is the FQDN from inventory (e.g., "gravity01.island.lagun.co") pub fn remove_host_record(config: &DnsConfig, inventory_name: &str) -> Result { + let _guard = dns_write_lock()?; let zone = config .zone .clone() @@ -252,6 +274,7 @@ pub fn set_service_records( service_name: &str, ips: &[String], ) -> Result { + let _guard = dns_write_lock()?; let changed = zone::set_a_records(&config.zones_path(), zone, service_name, ips)?; if changed && config.auto_sync { @@ -278,6 +301,7 @@ pub fn add_cname_alias( alias: &str, target: &str, ) -> Result { + let _guard = dns_write_lock()?; let changed = zone::add_cname_record(&config.zones_path(), zone, alias, target)?; if changed && config.auto_sync { @@ -315,6 +339,7 @@ pub fn add_ptr_record(config: &DnsConfig, ip: &str, fqdn: &str) -> Result Result { + let _guard = dns_write_lock()?; let changed = zone::remove_a_record(&config.zones_path(), zone, name)?; if changed && config.auto_sync { diff --git a/src/inventory/hosts.rs b/src/inventory/hosts.rs index 48db96c..5911ae1 100644 --- a/src/inventory/hosts.rs +++ b/src/inventory/hosts.rs @@ -220,27 +220,34 @@ impl Host { blend_variables(&mut blended, mine); blend_variables(&mut blended, self.facts.clone()); - // Add magic variables + // Magic variables. jet_hostname (the inventory identity, verbatim — the + // connection target is the separate jet_ssh_hostname) and + // jet_hostname_short are canonical. inventory_hostname / + // inventory_hostname_short are legacy Ansible-compat aliases that map + // onto them: Ansible's inventory_hostname means the same thing (the + // inventory identity, distinct from ansible_host), so the alias is a + // convenience for muscle memory, not a semantic lie. let mut result = match blended { serde_yaml::Value::Mapping(x) => x, _ => panic!("get_blended_variables produced a non-mapping (1)"), }; - // Full inventory hostname - result.insert( - serde_yaml::Value::String("jet_hostname".to_string()), - serde_yaml::Value::String(self.name.clone()), - ); - // Short hostname (first part before any dot) let short_name = self .name .split('.') .next() .unwrap_or(&self.name) .to_string(); - result.insert( - serde_yaml::Value::String("jet_hostname_short".to_string()), - serde_yaml::Value::String(short_name), - ); + for (key, value) in [ + ("jet_hostname", self.name.clone()), + ("jet_hostname_short", short_name.clone()), + ("inventory_hostname", self.name.clone()), + ("inventory_hostname_short", short_name), + ] { + result.insert( + serde_yaml::Value::String(key.to_string()), + serde_yaml::Value::String(value), + ); + } result } @@ -667,4 +674,32 @@ mod tests { assert_eq!(blended["service"], "nginx"); // From group assert_eq!(blended["hostname"], "webserver1"); // From host } + + #[test] + fn magic_variables_alias_inventory_hostname() { + use crate::playbooks::templar::{Templar, TemplateMode}; + + // The legacy Ansible-compat aliases (semantically identical to the + // jet_* builtins) must resolve in strict mode (regression: a task + // `msg: "...{{ inventory_hostname }}"` failed strict-mode templating + // with "variable in strict mode Some(\"inventory_hostname\")"). + let host = Host::new(&"web-01.lon.riff.cc".to_string()); + let blended = host.get_blended_variables(); + assert_eq!(blended["inventory_hostname"], "web-01.lon.riff.cc"); + assert_eq!(blended["inventory_hostname_short"], "web-01"); + // The jet_* builtins remain unchanged. + assert_eq!(blended["jet_hostname"], "web-01.lon.riff.cc"); + assert_eq!(blended["jet_hostname_short"], "web-01"); + + // Strict-mode render against a host's blended variables — the exact path + // a task `msg:` takes — resolves the alias without error. + let rendered = Templar::new() + .render( + "provisioned {{ inventory_hostname }}", + blended, + TemplateMode::Strict, + ) + .expect("inventory_hostname resolves in strict mode"); + assert_eq!(rendered, "provisioned web-01.lon.riff.cc"); + } } diff --git a/src/inventory/loading.rs b/src/inventory/loading.rs index c481337..fc88591 100644 --- a/src/inventory/loading.rs +++ b/src/inventory/loading.rs @@ -65,6 +65,7 @@ pub struct DynamicInventoryJsonEntry { pub fn load_inventory( inventory: &Arc>, inventory_paths: Arc>>, + extra_vars: serde_yaml::Value, ) -> Result<(), String> { { let mut inv_obj = inventory.write().unwrap(); @@ -108,7 +109,7 @@ pub fn load_inventory( // Now that every --inventory path has contributed its group_vars/host_vars, // fold each host's (now path-complete) ancestor-group variables into its own // stored variables. See `propagate_group_vars_to_hosts` for why this is needed. - propagate_group_vars_to_hosts(&inventory); + propagate_group_vars_to_hosts(inventory, &extra_vars); Ok(()) } @@ -136,7 +137,10 @@ pub fn load_inventory( /// re-blending the ancestor-group vars underneath the host's own vars yields the /// complete, correctly-precedenced view: host-specific vars win over group vars /// (host vars are layered on top last), group vars fill the gaps. -fn propagate_group_vars_to_hosts(inventory: &Arc>) { +fn propagate_group_vars_to_hosts( + inventory: &Arc>, + extra_vars: &serde_yaml::Value, +) { let host_names: Vec = { let inv = inventory.read().unwrap(); inv.hosts.keys().cloned().collect() @@ -162,7 +166,30 @@ fn propagate_group_vars_to_hosts(inventory: &Arc>) { let mine = serde_yaml::Value::from(host.get_variables()); crate::util::yaml::blend_variables(&mut blended, mine); + // Highest layer: -e / --extra-vars override everything (matching their + // precedence in templating). Blending them here — not just into the + // templating-time context blend — lets `-e provision.state=destroyed` + // reach Host.provision, so an ad-hoc CLI flag can drive a fleet's + // lifecycle without touching inventory files. + if !extra_vars.is_null() { + crate::util::yaml::blend_variables(&mut blended, extra_vars.clone()); + } + if let serde_yaml::Value::Mapping(resolved) = blended { + // A group-level `provision` overlay (e.g. `state: destroyed`) merges + // onto each member host's host_vars provision via the deep-blend + // above, so re-derive Host.provision from the resolved mapping — + // otherwise the provisioner reads the host_vars-only config parsed + // at load and the group overlay is silently ignored. Host fields win + // on conflict because the blend layers host vars on top of group + // vars (more-specific wins, matching variable resolution). + let provision_key = serde_yaml::Value::String("provision".to_string()); + if let Some(provision_value) = resolved.get(&provision_key) + && let Ok(provision_config) = + serde_yaml::from_value::(provision_value.clone()) + { + host.set_provision(provision_config); + } host.set_variables(resolved); } } @@ -339,11 +366,13 @@ fn load_vars_directory( } } - // Merge host-specific vars, excluding the provision block + // Merge host-specific vars, INCLUDING the provision block. Keeping + // the raw (default-free) provision mapping here lets a group-level + // `provision` overlay deep-merge onto it during + // propagate_group_vars_to_hosts; Host.provision is then re-derived + // from the resolved mapping so the provisioner sees the overlay. for (k, v) in yaml_result.iter() { - if k != &provision_key { - merged_vars.insert(k.clone(), v.clone()); - } + merged_vars.insert(k.clone(), v.clone()); } // Set the merged variables on the host @@ -446,6 +475,31 @@ pub fn convert_json_vars(input: &serde_json::Value) -> serde_yaml::Mapping { } } +/// Parse a `key=value` extra-vars string into a nested mapping, splitting the +/// key on `.` so `provision.state=destroyed` → `{provision: {state: "destroyed"}}`. +/// The value is taken as a string; use JSON (`-e '{"x":5}'`) for typed values. +pub fn extra_vars_from_key_value(input: &str) -> Result { + let (key, val) = input.split_once('=').ok_or_else(|| { + format!("--extra-vars: expected @file, inline JSON, or key=value; got {input:?}") + })?; + let key = key.trim(); + if key.is_empty() { + return Err(format!("--extra-vars: empty key in {input:?}")); + } + let val = val.trim(); + // Build the nested mapping from the dotted key segments (leaf = the value). + let mut node: serde_yaml::Value = serde_yaml::Value::String(val.to_string()); + for segment in key.split('.').rev() { + let mut mapping = serde_yaml::Mapping::new(); + mapping.insert(serde_yaml::Value::String(segment.to_string()), node); + node = serde_yaml::Value::Mapping(mapping); + } + match node { + serde_yaml::Value::Mapping(m) => Ok(m), + _ => unreachable!("a non-empty dotted key always yields a mapping"), + } +} + #[cfg(test)] mod tests { use super::*; @@ -475,7 +529,12 @@ mod tests { let (_keep_a, path_a) = inventory_tree_with_group_vars(first_yaml); let (_keep_b, path_b) = inventory_tree_with_group_vars(second_yaml); let paths = Arc::new(RwLock::new(vec![path_a, path_b])); - load_inventory(&inventory, paths).expect("load_inventory should succeed"); + load_inventory( + &inventory, + paths, + serde_yaml::Value::Mapping(serde_yaml::Mapping::new()), + ) + .expect("load_inventory should succeed"); inventory .read() .unwrap() @@ -565,7 +624,12 @@ mod tests { let inventory = Arc::new(RwLock::new(Inventory::new())); let paths = Arc::new(RwLock::new(vec![pub_path, sec_path])); - load_inventory(&inventory, paths).expect("load_inventory should succeed"); + load_inventory( + &inventory, + paths, + serde_yaml::Value::Mapping(serde_yaml::Mapping::new()), + ) + .expect("load_inventory should succeed"); let host_vars = inventory .read() @@ -588,6 +652,149 @@ mod tests { ); } + // Group-level `provision` overlay: a field set in a group's group_vars (e.g. + // `state: destroyed`) must merge onto each member host's host_vars provision + // config, so the whole fleet's lifecycle state can be toggled from one file + // (the PXE-starvation harness resets/recreates test-k8s this way). Host_vars + // fields are preserved. + #[test] + fn group_provision_overlay_merges_onto_host_provision_config() { + let (_keep, path) = inventory_tree_with_host( + "testgroup", + &["testhost"], + Some("provision:\n state: destroyed\n"), + &[( + "testhost", + "provision:\n type: proxmox_vm\n cluster: space\n", + )], + ); + + let inventory = Arc::new(RwLock::new(Inventory::new())); + let paths = Arc::new(RwLock::new(vec![path])); + load_inventory( + &inventory, + paths, + serde_yaml::Value::Mapping(serde_yaml::Mapping::new()), + ) + .expect("load_inventory should succeed"); + + let provision = inventory + .read() + .unwrap() + .get_host("testhost") + .read() + .unwrap() + .get_provision() + .expect("host has a provision config") + .clone(); + + assert_eq!( + provision.state, "destroyed", + "group provision.state must overlay onto the host config" + ); + assert_eq!(provision.provision_type, "proxmox_vm"); + assert_eq!(provision.cluster, "space"); + } + + // Precedence: when both the group and the host set the same provision field, + // the host's value wins (matches variable resolution — more specific wins). + #[test] + fn host_provision_field_wins_over_group_overlay() { + let (_keep, path) = inventory_tree_with_host( + "testgroup", + &["testhost"], + Some("provision:\n state: destroyed\n"), + &[( + "testhost", + "provision:\n type: proxmox_vm\n cluster: space\n state: present\n", + )], + ); + + let inventory = Arc::new(RwLock::new(Inventory::new())); + let paths = Arc::new(RwLock::new(vec![path])); + load_inventory( + &inventory, + paths, + serde_yaml::Value::Mapping(serde_yaml::Mapping::new()), + ) + .expect("load_inventory should succeed"); + + let provision = inventory + .read() + .unwrap() + .get_host("testhost") + .read() + .unwrap() + .get_provision() + .expect("host has a provision config") + .clone(); + + assert_eq!( + provision.state, "present", + "host_vars state wins over group" + ); + } + + // -e / --extra-vars reach the provisioner: a `provision` field injected via + // extra_vars merges onto each host's provision config at the highest + // precedence, so `jetpack apply -e provision.state=destroyed` drives a + // fleet's lifecycle without touching inventory files. Beats host_vars. + #[test] + fn extra_vars_provision_overlay_reaches_host_provision() { + let (_keep, path) = inventory_tree_with_host( + "testgroup", + &["testhost"], + None, + &[( + "testhost", + "provision:\n type: proxmox_vm\n cluster: space\n state: present\n", + )], + ); + let extra: serde_yaml::Value = + serde_yaml::from_str("provision:\n state: destroyed\n").unwrap(); + + let inventory = Arc::new(RwLock::new(Inventory::new())); + let paths = Arc::new(RwLock::new(vec![path])); + load_inventory(&inventory, paths, extra).expect("load_inventory should succeed"); + + let provision = inventory + .read() + .unwrap() + .get_host("testhost") + .read() + .unwrap() + .get_provision() + .expect("host has a provision config") + .clone(); + + // extra_vars (highest precedence) beats host_vars state:present. + assert_eq!(provision.state, "destroyed"); + assert_eq!(provision.provision_type, "proxmox_vm"); + } + + #[test] + fn extra_vars_key_value_dotted_nests() { + let m = extra_vars_from_key_value("provision.state=destroyed").unwrap(); + let s = serde_yaml::to_string(&m).unwrap(); + assert!(s.contains("provision:"), "{s}"); + assert!(s.contains("state: destroyed"), "{s}"); + } + + #[test] + fn extra_vars_key_value_keeps_equals_in_value() { + let m = extra_vars_from_key_value("token=a=b=c").unwrap(); + let s = serde_yaml::to_string(&m).unwrap(); + assert!(s.contains("token: a=b=c"), "{s}"); + } + + #[test] + fn extra_vars_key_value_rejects_malformed() { + // no '=', empty key, whitespace-only key + assert!(extra_vars_from_key_value("nokeyvalue").is_err()); + assert!(extra_vars_from_key_value("=value").is_err()); + assert!(extra_vars_from_key_value(" =val").is_err()); + } + // A directory with group_vars/ but no groups/ is a vars-only overlay — the // `secrets_inventory` shape — and must load in the same pass as the main // inventory (single propagate) so it layers with later-wins on conflicts. @@ -614,6 +821,7 @@ mod tests { load_inventory( &inventory, Arc::new(RwLock::new(vec![main_path, sec_dir.path().to_path_buf()])), + serde_yaml::Value::Mapping(serde_yaml::Mapping::new()), ) .expect("combined main + overlay load succeeds"); @@ -638,6 +846,7 @@ mod tests { let err = load_inventory( &inventory, Arc::new(RwLock::new(vec![empty.path().to_path_buf()])), + serde_yaml::Value::Mapping(serde_yaml::Mapping::new()), ) .unwrap_err(); assert!( diff --git a/src/main.rs b/src/main.rs index edf9a31..5df3187 100644 --- a/src/main.rs +++ b/src/main.rs @@ -74,6 +74,7 @@ fn liftoff() -> Result<(), String> { load_inventory( &inventory, Arc::new(RwLock::new(cli_parser.inventory_load_paths())), + cli_parser.extra_vars.clone(), )?; if !cli_parser.inventory_set { return Err(String::from( @@ -93,6 +94,7 @@ fn liftoff() -> Result<(), String> { load_inventory( &inventory, Arc::new(RwLock::new(cli_parser.inventory_load_paths())), + cli_parser.extra_vars.clone(), )?; } // Ensure localhost is in the inventory for local execution diff --git a/src/main_new.rs b/src/main_new.rs index a147327..de7bf6c 100644 --- a/src/main_new.rs +++ b/src/main_new.rs @@ -52,8 +52,12 @@ fn liftoff() -> Result<()> { // For show mode, handle it the old way since it's CLI-specific if cli_parser.mode == CLI_MODE_SHOW { let inventory: Arc> = Arc::new(RwLock::new(Inventory::new())); - load_inventory(&inventory, Arc::clone(&cli_parser.inventory_paths)) - .map_err(|e| JetpackError::Inventory(e))?; + load_inventory( + &inventory, + Arc::clone(&cli_parser.inventory_paths), + cli_parser.extra_vars.clone(), + ) + .map_err(|e| JetpackError::Inventory(e))?; if !cli_parser.inventory_set { return Err(JetpackError::Config("--inventory is required".into())); diff --git a/src/playbooks/context.rs b/src/playbooks/context.rs index 8b079eb..6b19e4b 100644 --- a/src/playbooks/context.rs +++ b/src/playbooks/context.rs @@ -186,6 +186,17 @@ impl PlaybookContext { self.failed_hosts.insert(hostname.clone(), Arc::clone(host)); } + // A host destroyed by its provisioner is removed from the task pool for the + // rest of the play. Unlike fail_host this is an intentional lifecycle + // outcome, not a task failure: it does not count against failed_tasks and + // is not recorded as a failed host, so a play that destroys all of its + // hosts can complete successfully instead of erroring "no hosts remaining". + + pub fn destroy_host(&mut self, host: &Arc>) { + let hostname = host.read().unwrap().name.clone(); + self.targetted_hosts.remove(&hostname); + } + pub fn set_playbook_path(&mut self, path: &Path) { self.playbook_path = Some(path_as_string(path)); self.playbook_directory = Some(directory_as_string(path)); diff --git a/src/playbooks/mod.rs b/src/playbooks/mod.rs index 1fd27a2..acf0365 100644 --- a/src/playbooks/mod.rs +++ b/src/playbooks/mod.rs @@ -19,6 +19,7 @@ pub mod async_ui; pub mod barrier; pub mod context; pub mod language; +pub mod provision_phase; pub mod ref_collector; pub mod role_tree; pub mod t_helpers; diff --git a/src/playbooks/provision_phase.rs b/src/playbooks/provision_phase.rs new file mode 100644 index 0000000..8bc3946 --- /dev/null +++ b/src/playbooks/provision_phase.rs @@ -0,0 +1,239 @@ +// Jetpack +// Copyright (C) Riff Labs Limited +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +//! Per-host provision phase. +//! +//! [`provision_host_with`] provisions a single host and bakes its SSH +//! connection variables (`jet_ssh_hostname` / `jet_ssh_user`) onto it, then +//! returns a [`ProvisionOutcome`]. It performs no task execution and touches no +//! barriers — control flow (abort-vs-continue, destroy handling) is owned by +//! the caller. It is parameterized over the provision function so it can be +//! exercised in tests without real infrastructure; production passes +//! [`ensure_host_provisioned`](crate::provisioners::ensure_host_provisioned). + +use crate::dns::DnsConfig; +use crate::inventory::hosts::Host; +use crate::inventory::inventory::Inventory; +use crate::playbooks::context::PlaybookContext; +use crate::playbooks::traversal::RunState; +use crate::provisioners::{ProvisionConfig, ProvisionResult, get_provisioner}; +use std::sync::{Arc, RwLock}; + +/// The result of attempting to provision a single host. +/// +/// Callers apply their own policy: `Ready` hosts run tasks; `Destroyed` hosts +/// are excluded from the task pool (the provisioner removed them intentionally); +/// `Failed` aborts the play (sequential) or is recorded as a per-host failure +/// (async). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ProvisionOutcome { + /// Provisioned (or needed no provisioning); ready for tasks. + Ready, + /// The provisioner destroyed this host; do not run tasks against it. + Destroyed, + /// Provisioning failed. Holds the raw error from the provision function. + Failed(String), +} + +/// Provision one host and set its connection variables. +/// +/// `provision` is [`ensure_host_provisioned`] in production and a fake in tests. +pub fn provision_host_with( + run_state: &Arc, + host: &Arc>, + provision: F, +) -> ProvisionOutcome +where + F: Fn( + &ProvisionConfig, + &str, + &Arc>, + Option<&DnsConfig>, + Option<&crate::output::OutputHandlerRef>, + ) -> Result, +{ + let (host_name, needs_provision, provision_config, host_vars) = { + let h = host.read().unwrap(); + ( + h.name.clone(), + h.needs_provisioning(), + h.get_provision().cloned(), + h.get_variables(), + ) + }; + + // No provision declared → nothing to do; the host is ready as-is. + if !needs_provision { + return ProvisionOutcome::Ready; + } + + let Some(config) = provision_config else { + return ProvisionOutcome::Ready; + }; + + // Dry-run: never create/update/destroy infrastructure in check mode. + if run_state.visitor.read().unwrap().is_check_mode() { + eprintln!( + " ~ {} => would provision ({}) — skipped (check mode)", + host_name, config.provision_type + ); + return ProvisionOutcome::Ready; + } + + let dns_key = serde_yaml::Value::String("dns".to_string()); + let automation_root = run_state.context.read().unwrap().automation_root.clone(); + let dns_config = host_vars + .get(&dns_key) + .and_then(|v| crate::dns::dns_config_from_vars(v, &automation_root)); + + match provision( + &config, + &host_name, + &run_state.inventory, + dns_config.as_ref(), + run_state.output_handler.as_ref(), + ) { + Ok(result) => { + run_state.visitor.read().unwrap().on_host_provisioned( + &run_state.context, + &host_name, + &result, + ); + + if matches!(result, ProvisionResult::Destroyed) { + return ProvisionOutcome::Destroyed; + } + + // Resolve the connection IP and bake it (plus the ssh user) onto the + // host so subsequent tasks connect to the right place. + let ip = get_provisioner(&config.provision_type).ok().and_then(|p| { + p.get_ip(&config, &host_name, &run_state.inventory) + .ok() + .flatten() + }); + + let mut h = host.write().unwrap(); + let mut vars = h.get_variables(); + let mut changed = false; + + if let Some(ip_addr) = ip { + let key = serde_yaml::Value::String("jet_ssh_hostname".to_string()); + if !vars.contains_key(&key) { + vars.insert(key, serde_yaml::Value::String(ip_addr)); + changed = true; + } + } + + if let Some(ssh_user) = &config.ssh_user { + let key = serde_yaml::Value::String("jet_ssh_user".to_string()); + if !vars.contains_key(&key) { + vars.insert(key, serde_yaml::Value::String(ssh_user.clone())); + changed = true; + } + } + + if changed { + h.set_variables(vars); + } + + ProvisionOutcome::Ready + } + Err(e) => ProvisionOutcome::Failed(e), + } +} + +/// Apply per-host provision outcomes to the task pool and decide whether the +/// play proceeds. +/// +/// - `Ready` → the host stays in the task pool. +/// - `Destroyed` → the host is removed from the pool (an intentional destroy, +/// not a failure). +/// - `Failed` → abort the play immediately (the sequential +/// abort-on-any-provision-failure policy). +/// +/// Returns the number of destroyed hosts so the caller can recognize an +/// all-destroyed play — a success with nothing left to configure — rather than +/// erroring "no hosts remaining". +pub fn apply_provision_outcomes( + context: &Arc>, + hosts: &[Arc>], + outcomes: &[ProvisionOutcome], +) -> Result { + let mut destroyed = 0usize; + let mut ctx = context.write().unwrap(); + for (host, outcome) in hosts.iter().zip(outcomes) { + match outcome { + ProvisionOutcome::Ready => {} + ProvisionOutcome::Destroyed => { + ctx.destroy_host(host); + destroyed += 1; + } + ProvisionOutcome::Failed(error) => { + let name = host.read().unwrap().name.clone(); + return Err(format!("Failed to provision host '{}': {}", name, error)); + } + } + } + Ok(destroyed) +} + +/// Format a one-line-per-host summary of the parallel provision phase, flagging +/// the slowest host as the straggler. Pure (no I/O) so it can be unit-tested. +/// +/// `timed[i]` corresponds to `hosts[i]` — rayon's `par_iter().collect()` into a +/// `Vec` preserves the original order. +pub fn format_provision_summary( + hosts: &[Arc>], + timed: &[(ProvisionOutcome, std::time::Duration)], + total: std::time::Duration, +) -> String { + use std::fmt::Write as _; + let (mut ready, mut destroyed, mut failed) = (0usize, 0usize, 0usize); + let mut slowest = std::time::Duration::ZERO; + let mut straggler_idx = None; + for (i, (outcome, dur)) in timed.iter().enumerate() { + match outcome { + ProvisionOutcome::Ready => ready += 1, + ProvisionOutcome::Destroyed => destroyed += 1, + ProvisionOutcome::Failed(_) => failed += 1, + } + if *dur > slowest { + slowest = *dur; + straggler_idx = Some(i); + } + } + let mut out = String::new(); + let _ = writeln!( + out, + "> provision phase: {} host(s) in {:.1}s (parallel) — ready:{} destroyed:{} failed:{}", + hosts.len(), + total.as_secs_f64(), + ready, + destroyed, + failed + ); + for (i, (host, (outcome, dur))) in hosts.iter().zip(timed.iter()).enumerate() { + let name = host.read().unwrap().name.clone(); + let label = match outcome { + ProvisionOutcome::Ready => "ready", + ProvisionOutcome::Destroyed => "destroyed", + ProvisionOutcome::Failed(_) => "failed", + }; + let flag = if straggler_idx == Some(i) && timed.len() > 1 { + " <- straggler" + } else { + "" + }; + let _ = writeln!( + out, + " {name:<28} {label:<10} {:>6.1}s{flag}", + dur.as_secs_f64() + ); + } + out +} diff --git a/src/playbooks/traversal.rs b/src/playbooks/traversal.rs index 6dfc2fb..433f319 100644 --- a/src/playbooks/traversal.rs +++ b/src/playbooks/traversal.rs @@ -22,6 +22,9 @@ use crate::playbooks::async_ui::{AsyncUi, HostEvent, TaskDisplayStatus}; use crate::playbooks::context::PlaybookContext; use crate::playbooks::language::Play; use crate::playbooks::language::{InstantiateSpec, RoleInvocation}; +use crate::playbooks::provision_phase::{ + apply_provision_outcomes, format_provision_summary, provision_host_with, +}; use crate::playbooks::role_tree::{ RoleSection, RoleWalkState, resolve_role_file, resolve_template_src, walk_role_tree, }; @@ -591,92 +594,45 @@ fn handle_batch( return async_handle_batch(run_state, play, hosts); } - // Sequential mode: provision all hosts before running tasks - for host_arc in hosts.iter() { - let (host_name, needs_provision, provision_config, host_vars) = { - let host = host_arc.read().unwrap(); - ( - host.name.clone(), - host.needs_provisioning(), - host.get_provision().cloned(), - host.get_variables(), - ) - }; - - if needs_provision && let Some(ref config) = provision_config { - // Dry-run: never create/update/destroy infrastructure in check mode. - if check_mode { - eprintln!( - " ~ {} => would provision ({}) — skipped (check mode)", - host_name, config.provision_type - ); - continue; - } - let dns_key = serde_yaml::Value::String("dns".to_string()); - let automation_root = run_state.context.read().unwrap().automation_root.clone(); - let dns_config = host_vars - .get(&dns_key) - .and_then(|v| crate::dns::dns_config_from_vars(v, &automation_root)); - - match ensure_host_provisioned( - config, - &host_name, - &run_state.inventory, - dns_config.as_ref(), - run_state.output_handler.as_ref(), - ) { - Ok(crate::provisioners::ProvisionResult::Destroyed) => { - run_state.visitor.read().unwrap().on_host_provisioned( - &run_state.context, - &host_name, - &crate::provisioners::ProvisionResult::Destroyed, - ); - continue; - } - Ok(result) => { - run_state.visitor.read().unwrap().on_host_provisioned( - &run_state.context, - &host_name, - &result, - ); - - let ip = crate::provisioners::get_provisioner(&config.provision_type) - .ok() - .and_then(|p| { - p.get_ip(config, &host_name, &run_state.inventory) - .ok() - .flatten() - }); + // Provision all hosts in parallel via the shared seam, then run the + // task-parallel phase against the survivors. Parallelizing provision means + // N hosts image concurrently — each gated on its own SSH readiness — instead + // of serially. The DNS write lock and the VMID lock make the concurrent + // infrastructure writes safe. + use rayon::prelude::*; + use std::time::Instant; + let phase_start = Instant::now(); + let timed: Vec<_> = hosts + .par_iter() + .map(|host| { + let start = Instant::now(); + let outcome = provision_host_with(run_state, host, ensure_host_provisioned); + (outcome, start.elapsed()) + }) + .collect(); + let outcomes: Vec<_> = timed.iter().map(|(o, _)| o.clone()).collect(); - let mut host = host_arc.write().unwrap(); - let mut vars = host.get_variables(); - let mut changed = false; + // Exclude destroyed hosts from the task pool; abort on any provision failure. + let destroyed = apply_provision_outcomes(&run_state.context, hosts, &outcomes)?; - if let Some(ref ip_addr) = ip { - let key = serde_yaml::Value::String("jet_ssh_hostname".to_string()); - if !vars.contains_key(&key) { - vars.insert(key, serde_yaml::Value::String(ip_addr.clone())); - changed = true; - } - } - - if let Some(ref ssh_user) = config.ssh_user { - let key = serde_yaml::Value::String("jet_ssh_user".to_string()); - if !vars.contains_key(&key) { - vars.insert(key, serde_yaml::Value::String(ssh_user.clone())); - changed = true; - } - } + // Controller-plane telemetry: per-host provision timing with the straggler flagged. + eprint!( + "{}", + format_provision_summary(hosts, &timed, phase_start.elapsed()) + ); - if changed { - host.set_variables(vars); - } - } - Err(e) => { - return Err(format!("Failed to provision host '{}': {}", host_name, e)); - } - } - } + // A play that intentionally destroyed every host has nothing left to + // configure — succeed without running tasks. (A genuinely empty play still + // falls through to the task phase, preserving the prior behavior.) + if destroyed > 0 + && run_state + .context + .read() + .unwrap() + .get_remaining_hosts() + .is_empty() + { + return Ok(()); } // Default mode: task-parallel execution (all hosts per task, then next task) @@ -1292,6 +1248,53 @@ fn resolve_target_groups(run_state: &Arc, play: &Play) -> Result, +) -> Result>, String> { + let playbook_paths = run_state.playbook_paths.read().unwrap().clone(); + if playbook_paths.is_empty() { + return Ok(None); + } + let mut targets = std::collections::HashSet::new(); + for path in &playbook_paths { + let file = match jet_file_open(path) { + Ok(f) => f, + Err(_) => return Ok(None), + }; + let plays: Vec = match serde_yaml::from_reader(file) { + Ok(p) => p, + Err(_) => return Ok(None), + }; + for (play_index, play) in plays.iter().enumerate() { + // Mirror the run: set play_index so a --groups per-play override + // resolves against the right index. + run_state.context.write().unwrap().play_index = play_index; + let groups = match resolve_target_groups(run_state, play) { + Ok(g) => g, + Err(_) => return Ok(None), + }; + if groups.is_empty() { + // A play with no declared groups is ambiguous → safe fallback. + return Ok(None); + } + for host in get_play_hosts(run_state, &groups) { + targets.insert(host.read().unwrap().name.clone()); + } + } + } + Ok(Some(targets)) +} + fn get_play_hosts(run_state: &Arc, groups: &[String]) -> Vec>> { // the hosts we want to talk to are the resolved play groups, possibly // further constrained by --limit-hosts / --limit-groups. Group resolution @@ -2211,7 +2214,12 @@ mod env_axis_groups_composition_tests { let inventory = Arc::new(RwLock::new(Inventory::new())); let load_list: Arc>> = Arc::new(RwLock::new(vec![main.clone(), overlay.clone()])); - load_inventory(&inventory, load_list).expect("main + env overlay load"); + load_inventory( + &inventory, + load_list, + serde_yaml::Value::Mapping(serde_yaml::Mapping::new()), + ) + .expect("main + env overlay load"); // The environment overlay's var reached group_vars/all (later-wins merge). let all_vars = inventory @@ -2250,8 +2258,12 @@ mod env_axis_groups_composition_tests { write_group(&main, "test-webservers", &["testweb1"]); let inventory = Arc::new(RwLock::new(Inventory::new())); - load_inventory(&inventory, Arc::new(RwLock::new(vec![main.clone()]))) - .expect("main inventory alone loads"); + load_inventory( + &inventory, + Arc::new(RwLock::new(vec![main.clone()])), + serde_yaml::Value::Mapping(serde_yaml::Mapping::new()), + ) + .expect("main inventory alone loads"); let rs = run_state_with_inventory(inventory); let play = play_with_groups("k3s", &["{{ target }}"]); diff --git a/src/provisioners/mod.rs b/src/provisioners/mod.rs index fcf4c62..400d4c1 100644 --- a/src/provisioners/mod.rs +++ b/src/provisioners/mod.rs @@ -39,7 +39,7 @@ pub mod proxmox_vm; use crate::dns::DnsConfig; use crate::inventory::inventory::Inventory; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::sync::Arc; use std::sync::RwLock; @@ -50,7 +50,7 @@ fn default_state() -> String { } /// Configuration for provisioning a host's underlying infrastructure -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct ProvisionConfig { /// The type of provisioner (e.g., "proxmox_lxc", "proxmox_vm", "docker") #[serde(rename = "type")] diff --git a/src/provisioners/proxmox_vm.rs b/src/provisioners/proxmox_vm.rs index 5abfe20..16b0f18 100644 --- a/src/provisioners/proxmox_vm.rs +++ b/src/provisioners/proxmox_vm.rs @@ -9,7 +9,17 @@ use crate::playbooks::templar::{Templar, TemplateMode}; use crate::provisioners::{ProvisionConfig, ProvisionResult, Provisioner}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, LazyLock, Mutex, RwLock}; + +/// Serializes VMID allocation + empty-VM creation. +/// +/// Proxmox `/cluster/nextid` returns max+1 without reserving it, so two hosts +/// created in parallel on the same cluster can be handed the same VMID and +/// collide at create time. Holding this across `get_next_vmid` → +/// `create_empty_vm` makes parallel provisioning safe. VM creation is a single +/// API call (~1s); PXE imaging (~90s) still runs in parallel afterward, so this +/// serializes only the cheap part. +static VMID_LOCK: LazyLock> = LazyLock::new(|| Mutex::new(())); pub struct ProxmoxVmProvisioner; @@ -968,16 +978,22 @@ impl Provisioner for ProxmoxVmProvisioner { // reimage with Dragonfly so it images on PXE boot. Best-effort — a // Dragonfly outage must never block VM creation; the VM still PXEs and // Dragonfly discovers it by MAC. - let vmid = if let Some(ref vmid_str) = config.vmid { - vmid_str + // Allocate the VMID and create the empty VM. When the VMID is + // auto-assigned, hold VMID_LOCK across nextid+create so parallel hosts + // on the same cluster don't collide on the non-reserving nextid API. + let (vmid, mac) = if let Some(ref vmid_str) = config.vmid { + let vmid = vmid_str .parse::() - .map_err(|_| "Invalid vmid".to_string())? + .map_err(|_| "Invalid vmid".to_string())?; + (vmid, self.create_empty_vm(&conn, &config, hostname, vmid)?) } else { - self.get_next_vmid(&conn)? + let _guard = VMID_LOCK + .lock() + .map_err(|e| format!("VMID lock poisoned: {}", e))?; + let vmid = self.get_next_vmid(&conn)?; + (vmid, self.create_empty_vm(&conn, &config, hostname, vmid)?) }; - let mac = self.create_empty_vm(&conn, &config, hostname, vmid)?; - if !mac.is_empty() { eprintln!("VM {} created with MAC: {}", hostname, mac); if let Some(ref dragonfly) = dragonfly { diff --git a/tests/dns_repo_root.rs b/tests/dns_repo_root.rs index 34053a1..49bfdd9 100644 --- a/tests/dns_repo_root.rs +++ b/tests/dns_repo_root.rs @@ -22,6 +22,8 @@ use serde_yaml::{Mapping, Value}; use std::fs; +use std::sync::Arc; +use std::thread; use tempfile::TempDir; #[test] @@ -78,3 +80,47 @@ fn dns_record_lands_at_automation_root_not_playbook_dir() { ); assert!(content.contains("10.0.0.5"), "ip recorded: {content}"); } + +/// Parallel host provisioning calls `add_host_record` concurrently against the +/// same zone file. The DNS write lock must serialize those calls so the +/// zone-file read-modify-write (and the sync that follows) does not lose +/// updates — and must not deadlock on the internally-called helpers. +#[test] +fn parallel_add_host_record_serializes_without_loss_or_deadlock() { + let repo = TempDir::new().unwrap(); + let automation_root = repo.path().to_path_buf(); + let mut dns_block = Mapping::new(); + dns_block.insert( + Value::String("path".to_string()), + Value::String("dns/test".to_string()), + ); + dns_block.insert(Value::String("auto_sync".to_string()), Value::Bool(false)); + let dns = Arc::new( + jetpack::dns::dns_config_from_vars(&Value::Mapping(dns_block), &automation_root) + .expect("dns block deserializes"), + ); + + const N: usize = 16; + let handles: Vec<_> = (1..=N) + .map(|i| { + let dns = Arc::clone(&dns); + thread::spawn(move || { + let fqdn = format!("node{i}.test"); + let ip = format!("10.0.0.{i}"); + jetpack::dns::add_host_record(&dns, &fqdn, &ip).expect("record write succeeds") + }) + }) + .collect(); + for h in handles { + h.join().expect("DNS writer thread must not panic"); + } + + let zone_file = repo.path().join("dns/test/zones/test.yaml"); + let content = fs::read_to_string(&zone_file).unwrap(); + for i in 1..=N { + assert!( + content.contains(&format!("node{i}")), + "node{i} record lost to a concurrent write: {content}" + ); + } +} diff --git a/tests/playbooks/context.rs b/tests/playbooks/context.rs index 65d5433..d64c577 100644 --- a/tests/playbooks/context.rs +++ b/tests/playbooks/context.rs @@ -1,14 +1,14 @@ -use jetpack::playbooks::context::*; use jetpack::cli::parser::CliParser; -use jetpack::playbooks::language::{Play, Role}; +use jetpack::inventory::hosts::Host; +use jetpack::playbooks::context::*; +use jetpack::playbooks::language::Role; use std::sync::{Arc, RwLock}; -use serde_yaml; #[test] fn test_playbook_context_new() { let parser = CliParser::new(); let context = PlaybookContext::new(&parser); - + assert_eq!(context.verbosity, 0); assert!(context.playbook_path.is_none()); assert!(context.playbook_directory.is_none()); @@ -24,10 +24,10 @@ fn test_playbook_context_new() { fn test_playbook_context_update_playbook_path() { let parser = CliParser::new(); let mut context = PlaybookContext::new(&parser); - + context.playbook_path = Some("/path/to/playbook.yml".to_string()); context.playbook_directory = Some("/path/to".to_string()); - + assert_eq!(context.playbook_path.unwrap(), "/path/to/playbook.yml"); assert_eq!(context.playbook_directory.unwrap(), "/path/to"); } @@ -36,11 +36,11 @@ fn test_playbook_context_update_playbook_path() { fn test_playbook_context_counters() { let parser = CliParser::new(); let mut context = PlaybookContext::new(&parser); - + context.play_count = 5; context.role_count = 3; context.task_count = 10; - + assert_eq!(context.play_count, 5); assert_eq!(context.role_count, 3); assert_eq!(context.task_count, 10); @@ -50,7 +50,7 @@ fn test_playbook_context_counters() { fn test_playbook_context_with_play() { let parser = CliParser::new(); let mut context = PlaybookContext::new(&parser); - + context.play = Some("Test Play".to_string()); assert_eq!(context.play.unwrap(), "Test Play"); } @@ -59,18 +59,17 @@ fn test_playbook_context_with_play() { fn test_playbook_context_with_role() { let parser = CliParser::new(); let mut context = PlaybookContext::new(&parser); - + let role_yaml = r#" name: test_role tasks: - - !echo - msg: "Hello from role" + - main.yml "#; - + let role: Role = serde_yaml::from_str(role_yaml).unwrap(); context.role = Some(role); context.role_path = Some("/path/to/roles/test_role".to_string()); - + assert!(context.role.is_some()); assert_eq!(context.role_path.unwrap(), "/path/to/roles/test_role"); } @@ -79,14 +78,49 @@ tasks: fn test_playbook_context_verbosity() { let parser = CliParser::new(); let mut context = PlaybookContext::new(&parser); - + // Test different verbosity levels context.verbosity = 0; assert_eq!(context.verbosity, 0); - + context.verbosity = 1; assert_eq!(context.verbosity, 1); - + context.verbosity = 3; assert_eq!(context.verbosity, 3); -} \ No newline at end of file +} + +fn host_named(name: &str) -> Arc> { + Arc::new(RwLock::new(Host::new(name))) +} + +#[test] +fn destroy_host_removes_from_task_pool_without_marking_failed() { + let parser = CliParser::new(); + let mut context = PlaybookContext::new(&parser); + let h1 = host_named("node1"); + let h2 = host_named("node2"); + context.set_targetted_hosts(&[Arc::clone(&h1), Arc::clone(&h2)]); + assert_eq!(context.get_remaining_hosts().len(), 2); + + context.destroy_host(&h1); + + let remaining = context.get_remaining_hosts(); + assert_eq!(remaining.len(), 1); + assert!(remaining.contains_key("node2")); + assert!(!remaining.contains_key("node1")); + // A destroy is an intentional lifecycle outcome, not a task failure. + assert_eq!(context.failed_tasks, 0); +} + +#[test] +fn destroy_host_on_unknown_host_is_a_no_op() { + let parser = CliParser::new(); + let mut context = PlaybookContext::new(&parser); + let h1 = host_named("node1"); + context.set_targetted_hosts(&[Arc::clone(&h1)]); + + context.destroy_host(&host_named("not-in-pool")); + + assert_eq!(context.get_remaining_hosts().len(), 1); +} diff --git a/tests/playbooks/mod.rs b/tests/playbooks/mod.rs index 2cbc80c..c6d07e0 100644 --- a/tests/playbooks/mod.rs +++ b/tests/playbooks/mod.rs @@ -1,4 +1,6 @@ +mod context; mod language; +mod provision_phase; mod t_helpers; mod templar; mod traversal; diff --git a/tests/playbooks/provision_phase.rs b/tests/playbooks/provision_phase.rs new file mode 100644 index 0000000..c8e278d --- /dev/null +++ b/tests/playbooks/provision_phase.rs @@ -0,0 +1,298 @@ +// Jetpack +// Copyright (C) Riff Labs Limited +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +//! Tests for the per-host provision seam (`provision_host_with`). +//! +//! These drive the helper with a fake provision function so the outcome +//! mapping, SSH-var setting, and check-mode / no-provision short-circuits can +//! be verified without any real infrastructure. + +use jetpack::cli::parser::CliParser; +use jetpack::connection::no::NoFactory; +use jetpack::dns::DnsConfig; +use jetpack::inventory::hosts::Host; +use jetpack::inventory::inventory::Inventory; +use jetpack::output::OutputHandlerRef; +use jetpack::playbooks::context::PlaybookContext; +use jetpack::playbooks::provision_phase::{ + ProvisionOutcome, apply_provision_outcomes, format_provision_summary, provision_host_with, +}; +use jetpack::playbooks::traversal::RunState; +use jetpack::playbooks::visitor::{CheckMode, PlaybookVisitor}; +use jetpack::provisioners::{ProvisionConfig, ProvisionResult}; +use serde_yaml::Value; +use std::collections::{HashMap, HashSet}; +use std::sync::atomic::{AtomicU32, Ordering}; +use std::sync::{Arc, Mutex, RwLock}; +use std::time::Duration; + +type ProvisionFn = fn( + &ProvisionConfig, + &str, + &Arc>, + Option<&DnsConfig>, + Option<&OutputHandlerRef>, +) -> Result; + +fn make_run_state(check_mode: bool) -> Arc { + let parser = CliParser::new(); + let mode = if check_mode { + CheckMode::Yes + } else { + CheckMode::No + }; + Arc::new(RunState { + inventory: Arc::new(RwLock::new(Inventory::new())), + playbook_paths: Arc::new(RwLock::new(Vec::new())), + role_paths: Arc::new(RwLock::new(Vec::new())), + module_paths: Arc::new(RwLock::new(Vec::new())), + limit_hosts: Vec::new(), + limit_groups: Vec::new(), + batch_size: None, + context: Arc::new(RwLock::new(PlaybookContext::new(&parser))), + visitor: Arc::new(RwLock::new(PlaybookVisitor::new(mode))), + connection_factory: Arc::new(RwLock::new(NoFactory::new())), + tags: None, + allow_localhost_delegation: false, + is_pull_mode: false, + syntax_mode: false, + play_groups: None, + processed_role_tasks: Arc::new(RwLock::new(HashSet::new())), + processed_role_handlers: Arc::new(RwLock::new(HashSet::new())), + role_processing_stack: Arc::new(RwLock::new(Vec::new())), + output_handler: None, + async_mode: false, + playbook_contents: Vec::new(), + fetched_files: Arc::new(Mutex::new(HashMap::new())), + }) +} + +fn provision_config(ip: &str) -> ProvisionConfig { + let yaml = + format!("type: proxmox_vm\ncluster: test\nstate: present\nssh_user: root\nip: {ip}\n"); + serde_yaml::from_str(&yaml).unwrap() +} + +fn host_with_provision(name: &str, ip: &str) -> Arc> { + let mut h = Host::new(name); + h.set_provision(provision_config(ip)); + Arc::new(RwLock::new(h)) +} + +fn host_without_provision(name: &str) -> Arc> { + Arc::new(RwLock::new(Host::new(name))) +} + +fn ssh_var(host: &Arc>, key: &str) -> Option { + let vars = host.read().unwrap().get_variables(); + vars.get(Value::String(key.to_string())) + .and_then(|v| v.as_str().map(|s| s.to_string())) +} + +fn fake_created( + _: &ProvisionConfig, + _: &str, + _: &Arc>, + _: Option<&DnsConfig>, + _: Option<&OutputHandlerRef>, +) -> Result { + Ok(ProvisionResult::Created) +} + +fn fake_destroyed( + _: &ProvisionConfig, + _: &str, + _: &Arc>, + _: Option<&DnsConfig>, + _: Option<&OutputHandlerRef>, +) -> Result { + Ok(ProvisionResult::Destroyed) +} + +fn fake_fails( + _: &ProvisionConfig, + _: &str, + _: &Arc>, + _: Option<&DnsConfig>, + _: Option<&OutputHandlerRef>, +) -> Result { + Err("provision blew up".to_string()) +} + +#[test] +fn ready_outcome_records_ssh_hostname_and_user() { + let rs = make_run_state(false); + let host = host_with_provision("node1", "10.0.0.5"); + + let outcome = provision_host_with(&rs, &host, fake_created as ProvisionFn); + + assert!(matches!(outcome, ProvisionOutcome::Ready)); + assert_eq!( + ssh_var(&host, "jet_ssh_hostname").as_deref(), + Some("10.0.0.5") + ); + assert_eq!(ssh_var(&host, "jet_ssh_user").as_deref(), Some("root")); +} + +#[test] +fn destroyed_outcome_does_not_set_ssh_vars() { + let rs = make_run_state(false); + let host = host_with_provision("node1", "10.0.0.5"); + + let outcome = provision_host_with(&rs, &host, fake_destroyed as ProvisionFn); + + assert!(matches!(outcome, ProvisionOutcome::Destroyed)); + assert!(ssh_var(&host, "jet_ssh_hostname").is_none()); + assert!(ssh_var(&host, "jet_ssh_user").is_none()); +} + +#[test] +fn failed_outcome_preserves_error_message() { + let rs = make_run_state(false); + let host = host_with_provision("node1", "10.0.0.5"); + + let outcome = provision_host_with(&rs, &host, fake_fails as ProvisionFn); + + match outcome { + ProvisionOutcome::Failed(msg) => assert_eq!(msg, "provision blew up"), + other => panic!("expected Failed, got {other:?}"), + } +} + +#[test] +fn host_without_provision_is_ready_without_calling_provision() { + let rs = make_run_state(false); + let host = host_without_provision("node1"); + + let calls = Arc::new(AtomicU32::new(0)); + let calls_clone = Arc::clone(&calls); + let fake = move |_: &ProvisionConfig, + _: &str, + _: &Arc>, + _: Option<&DnsConfig>, + _: Option<&OutputHandlerRef>| + -> Result { + calls_clone.fetch_add(1, Ordering::SeqCst); + Ok(ProvisionResult::Created) + }; + + let outcome = provision_host_with(&rs, &host, fake); + + assert!(matches!(outcome, ProvisionOutcome::Ready)); + assert_eq!(calls.load(Ordering::SeqCst), 0); +} + +#[test] +fn check_mode_is_ready_without_calling_provision() { + let rs = make_run_state(true); + let host = host_with_provision("node1", "10.0.0.5"); + + let calls = Arc::new(AtomicU32::new(0)); + let calls_clone = Arc::clone(&calls); + let fake = move |_: &ProvisionConfig, + _: &str, + _: &Arc>, + _: Option<&DnsConfig>, + _: Option<&OutputHandlerRef>| + -> Result { + calls_clone.fetch_add(1, Ordering::SeqCst); + Ok(ProvisionResult::Created) + }; + + let outcome = provision_host_with(&rs, &host, fake); + + assert!(matches!(outcome, ProvisionOutcome::Ready)); + assert_eq!(calls.load(Ordering::SeqCst), 0); +} + +#[test] +fn apply_outcomes_excludes_destroyed_keeps_ready() { + let rs = make_run_state(false); + let h1 = host_without_provision("node1"); + let h2 = host_without_provision("node2"); + let hosts = vec![Arc::clone(&h1), Arc::clone(&h2)]; + rs.context.write().unwrap().set_targetted_hosts(&hosts); + let outcomes = vec![ProvisionOutcome::Ready, ProvisionOutcome::Destroyed]; + + let destroyed = apply_provision_outcomes(&rs.context, &hosts, &outcomes).unwrap(); + + assert_eq!(destroyed, 1); + let remaining = rs.context.read().unwrap().get_remaining_hosts(); + assert_eq!(remaining.len(), 1); + assert!(remaining.contains_key("node1")); + assert!(!remaining.contains_key("node2")); +} + +#[test] +fn apply_outcomes_all_destroyed_empties_pool_and_counts() { + let rs = make_run_state(false); + let h1 = host_without_provision("node1"); + let h2 = host_without_provision("node2"); + let hosts = vec![Arc::clone(&h1), Arc::clone(&h2)]; + rs.context.write().unwrap().set_targetted_hosts(&hosts); + let outcomes = vec![ProvisionOutcome::Destroyed, ProvisionOutcome::Destroyed]; + + let destroyed = apply_provision_outcomes(&rs.context, &hosts, &outcomes).unwrap(); + + assert_eq!(destroyed, 2); + assert!(rs.context.read().unwrap().get_remaining_hosts().is_empty()); +} + +#[test] +fn apply_outcomes_aborts_on_first_failure() { + let rs = make_run_state(false); + let h1 = host_without_provision("node1"); + let hosts = vec![Arc::clone(&h1)]; + rs.context.write().unwrap().set_targetted_hosts(&hosts); + let outcomes = vec![ProvisionOutcome::Failed("provision blew up".to_string())]; + + let result = apply_provision_outcomes(&rs.context, &hosts, &outcomes); + + let err = result.unwrap_err(); + assert!(err.contains("node1"), "got: {err}"); + assert!(err.contains("provision blew up"), "got: {err}"); +} + +#[test] +fn summary_counts_outcomes_and_flags_the_slowest_straggler() { + let h1 = host_without_provision("node1"); + let h2 = host_without_provision("node2"); + let h3 = host_without_provision("node3"); + let hosts = vec![Arc::clone(&h1), Arc::clone(&h2), Arc::clone(&h3)]; + let timed = vec![ + (ProvisionOutcome::Ready, Duration::from_secs(10)), + (ProvisionOutcome::Destroyed, Duration::from_secs(20)), + ( + ProvisionOutcome::Failed("ssh timeout".to_string()), + Duration::from_secs(300), + ), + ]; + + let s = format_provision_summary(&hosts, &timed, Duration::from_secs(305)); + + assert!(s.contains("3 host(s)"), "{s}"); + assert!(s.contains("ready:1"), "{s}"); + assert!(s.contains("destroyed:1"), "{s}"); + assert!(s.contains("failed:1"), "{s}"); + // The slowest (node3) is flagged; node1 is not. + let node3_line = s.lines().find(|l| l.contains("node3")).unwrap(); + assert!(node3_line.contains("straggler"), "{node3_line}"); + let node1_line = s.lines().find(|l| l.contains("node1")).unwrap(); + assert!(!node1_line.contains("straggler"), "{node1_line}"); +} + +#[test] +fn summary_does_not_flag_a_straggler_for_a_single_host() { + let h = host_without_provision("solo"); + let hosts = vec![Arc::clone(&h)]; + let timed = vec![(ProvisionOutcome::Ready, Duration::from_secs(45))]; + + let s = format_provision_summary(&hosts, &timed, Duration::from_secs(45)); + + assert!(!s.contains("straggler"), "{s}"); +}