From 72cafcdc4b75aa5aeb7c3420f99352bdccd1160c Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 9 Sep 2026 05:49:06 +0000 Subject: [PATCH 1/2] Select remote Build Machines automatically and honor Compose defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plain `--remote` now selects a Build Machine instead of refusing. `--remote=` still pins one and `--local` forces the local default; Compose sets the preference between them with a top-level `x-build-machine:` that any of the three flags overrides. `auto` and `local` name modes wherever they appear, so a Machine called either is pinned by its ID. Selection is a pure function of observed evidence in `build_location`: visible Machines that advertise `ployz.build.v1`, ranked by how many requested `build.platforms` their reported architecture runs natively via the existing `platform_compatible`, with Machine ID breaking ties so the choice does not depend on observation order. The selected Machine is reported before any upload, along with the platforms this client could not confirm and any Machine that never answered; the Machine still admits or refuses the Build itself. Refusals separate the three reasons a Cluster offers no Build Machine — nothing visible, nothing answered, nothing capable — so a partition is not reported as a capability problem, and each carries the corrective action that actually applies. Capture runs before selection so the platforms it fixes decide the candidates; no source leaves the client before admission. Closes #804 --- README.md | 13 + crates/ployz/compose-helper/convert.go | 6 +- crates/ployz/src/build_location.rs | 378 ++++++++++++++++++++++ crates/ployz/src/cli/mod.rs | 2 +- crates/ployz/src/compose/build.rs | 10 + crates/ployz/src/compose/loader.rs | 29 ++ crates/ployz/src/compose/model.rs | 3 + crates/ployz/src/handlers/build.rs | 204 ++++++++++-- crates/ployz/src/handlers/deploy.rs | 28 +- crates/ployz/src/lib.rs | 1 + crates/ployz/tests/build_layer3/remote.rs | 133 ++++++++ crates/ployz/tests/cli_shape.rs | 41 ++- crates/ployz/tests/connect/support.rs | 16 +- crates/ployz/tests/remote_build_cli.rs | 171 +++++++++- 14 files changed, 979 insertions(+), 56 deletions(-) create mode 100644 crates/ployz/src/build_location.rs diff --git a/README.md b/README.md index e5318652be..2d22064fa5 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,19 @@ Railpack refuses `--check` and unsupported frontend settings by name. On this pinned frontend, `--no-cache` and `--pull` force a cold build by clearing the exclusive Ployz builder cache; unrelated Docker builder caches are untouched. +`ployz build` and `ployz deploy` run here by default. `--remote=` pins +one Machine, plain `--remote` selects one automatically, and `--local` forces the +local default. Compose sets the preference between them with a top-level +`x-build-machine:` of a Machine name or ID, `auto`, or `local`; any of the three +flags overrides it. Automatic selection uses visible Machines that advertise +remote Builds, prefers the one whose reported architecture runs the most +requested `build.platforms` natively, and breaks ties on Machine ID, so it does +not depend on observation order. `auto` and `local` name modes wherever they +appear, so a Machine called either is pinned by its ID. The selected Machine is +reported before any upload, along with any platform this client could not +confirm it runs natively and any Machine that never answered; it still admits or +refuses the Build itself, and Ployz never falls back to a local Build silently. + Remote Builds (`ployz build --remote=`) use one active slot per Machine and a FIFO of eight waiting attempts. Source and secrets stay on the client until admission. Configure the daemon environment and restart it: diff --git a/crates/ployz/compose-helper/convert.go b/crates/ployz/compose-helper/convert.go index a750b9795b..a23fa02d1d 100644 --- a/crates/ployz/compose-helper/convert.go +++ b/crates/ployz/compose-helper/convert.go @@ -83,7 +83,11 @@ func convert(p *types.Project, model, provisioned, secrets object) (object, erro } warnings = append(warnings, w...) } - return object{"name": p.Name, "working_dir": p.WorkingDir, "context": p.Extensions["x-context"], "services": services, "builds": builds, "dependencies": dependencies, "service_profiles": profiles, "warnings": warnings, "secrets": secrets, "environment": p.Environment}, nil + buildMachine, declared := p.Extensions["x-build-machine"] + if value, _ := buildMachine.(string); declared && value == "" { + return nil, fmt.Errorf("x-build-machine must be a Machine name or ID, 'auto', or 'local'") + } + return object{"name": p.Name, "working_dir": p.WorkingDir, "context": p.Extensions["x-context"], "build_machine": buildMachine, "services": services, "builds": builds, "dependencies": dependencies, "service_profiles": profiles, "warnings": warnings, "secrets": secrets, "environment": p.Environment}, nil } func service(p *types.Project, s types.ServiceConfig, raw, provisioned object) (object, error) { diff --git a/crates/ployz/src/build_location.rs b/crates/ployz/src/build_location.rs new file mode 100644 index 0000000000..9e0dcb5d7b --- /dev/null +++ b/crates/ployz/src/build_location.rs @@ -0,0 +1,378 @@ +//! Where a Build runs. The requested location comes from the CLI and Compose; +//! the automatic choice is a pure function of the Machines this client observed. +//! The selected Machine still admits or refuses the Build itself. + +use std::collections::BTreeSet; + +use ployz_core::{MachineId, MachineName, MachineTarget}; +use thiserror::Error; + +/// `--remote` given without a value. Shared with the clap definition in +/// `cli`, whose `default_missing_value` must stay this exact string for plain +/// `--remote` to mean automatic selection rather than a pin. +pub(crate) const AUTOMATIC: &str = ""; + +use crate::image::platform_compatible; + +/// A requested build location that names no Machine. The origin is part of the +/// message: the same text can come from a flag or from Compose. +#[derive(Debug, Error)] +#[error("{origin} {value:?} is not a Machine name or ID, `auto`, or `local`")] +pub(crate) struct InvalidLocation { + origin: &'static str, + value: String, +} + +/// Execution location a user asked for, before any Machine is observed. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) enum Location { + /// Build in the invoking client's own Docker. + Local, + /// Build on a Machine, once one is resolved. + Remote(Selection), +} + +/// How the Build Machine is chosen. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) enum Selection { + /// Choose one compatible Machine from what this client can observe. + Automatic, + /// Build on exactly this Machine. + Pinned(MachineTarget), +} + +impl Location { + /// Resolve the requested location. An explicit flag always beats Compose, + /// and Compose beats the local default. + /// + /// `remote` is the `--remote` value: absent when the flag was not given, + /// empty for plain `--remote`, which always asks for a fresh automatic + /// choice rather than the Compose pin. `preferred` is the Compose + /// `x-build-machine` value. `auto` and `local` name modes wherever they + /// appear, so a Machine called either is pinned by its ID instead. + /// + /// # Errors + /// + /// Returns [`InvalidLocation`] when a pinned target is not a Machine + /// identity, naming the flag or Compose key it was written in. + pub(crate) fn requested( + remote: Option<&str>, + local: bool, + preferred: Option<&str>, + ) -> Result { + if local { + return Ok(Self::Local); + } + let (origin, value) = match (remote, preferred) { + (Some(value), _) => ("--remote", value), + (None, Some(value)) => ("Compose x-build-machine", value), + (None, None) => return Ok(Self::Local), + }; + match value { + "local" => Ok(Self::Local), + AUTOMATIC | "auto" => Ok(Self::Remote(Selection::Automatic)), + target => MachineTarget::parse(target) + .map(|target| Self::Remote(Selection::Pinned(target))) + .map_err(|_| InvalidLocation { + origin, + value: target.to_owned(), + }), + } + } +} + +/// What one visible Machine offers a Build, as observed by this client. +#[derive(Debug)] +pub(crate) struct Candidate { + pub id: MachineId, + pub name: MachineName, + pub evidence: Evidence, +} + +/// Build capability of one Machine. Unanswered evidence stays visible: it is +/// never read as either support or refusal. +#[derive(Debug)] +pub(crate) enum Evidence { + /// The Machine advertises remote Builds. `architecture` is the kernel + /// architecture from the Cluster's discovery record, not part of the + /// Machine's answer and not Docker's own platform, so it can be stale or + /// absent. + Builds { architecture: String }, + /// The Machine answered and does not advertise remote Builds. + Refuses, + /// The Machine's membership structurally forbids an RPC, so it was never + /// asked. That is a known ineligibility, not a failed probe. + Ineligible(String), + /// The Machine could not be asked, with the reason it could not. + Unanswered(String), +} + +/// The Machine an automatic Build selected, and what it cannot prove about it. +#[derive(Debug)] +pub(crate) struct Choice<'list> { + pub machine: &'list Candidate, + /// Required platforms this client could not confirm the Machine runs + /// natively. The Machine confirms or refuses them at admission. + pub unconfirmed: Vec, + /// Visible Machines that never answered, so a better candidate may have + /// been passed over. One line of evidence each. + pub unanswered: Vec, +} + +/// Why no Machine could be chosen. The observed evidence belongs here; the +/// command that could fix it belongs to the CLI. +#[derive(Debug, Error)] +pub(crate) enum NoBuildMachine { + #[error("no Machine is visible, so no Build Machine can be selected")] + Invisible, + #[error("no visible Machine answered, so none could be selected. Observed {}", .unanswered.join("; "))] + Silent { unanswered: Vec }, + #[error("no visible Machine can run this Build. Observed {}", .observed.join("; "))] + Incapable { observed: Vec }, +} + +/// Pick one Machine for an automatic Build. +/// +/// Machines that never invite an RPC, do not answer, or answer without the +/// Build capability are not candidates. The Machine that natively runs the most required platforms wins, +/// and Machine ID breaks every remaining tie, so the choice does not depend on +/// the order the Machines were observed. +/// +/// # Errors +/// +/// Returns [`NoBuildMachine`] when no observed Machine can build, carrying every +/// Machine and the evidence that excluded it. +pub(crate) fn choose<'list>( + candidates: &'list [Candidate], + required: &BTreeSet, +) -> Result, NoBuildMachine> { + let mut eligible = Vec::new(); + let mut excluded = Vec::new(); + let mut unanswered = Vec::new(); + for candidate in candidates { + let name = &candidate.name; + let id = &candidate.id; + match &candidate.evidence { + Evidence::Builds { architecture } => { + eligible.push((candidate, unconfirmed(architecture, required))); + } + Evidence::Refuses => { + excluded.push(format!("{name} ({id}) does not run remote Builds")); + } + Evidence::Ineligible(reason) => excluded.push(format!("{name} ({id}) {reason}")), + Evidence::Unanswered(reason) => { + unanswered.push(format!("{name} ({id}) capability unknown: {reason}")); + } + } + } + let chosen = eligible + .into_iter() + .min_by(|(left, left_open), (right, right_open)| { + left_open + .len() + .cmp(&right_open.len()) + .then_with(|| left.id.as_str().cmp(right.id.as_str())) + }); + let Some((machine, unconfirmed)) = chosen else { + // Nothing answered is a reachability problem, not a capability one; + // telling that user to upgrade a Machine would be wrong advice. + return Err(match (excluded.is_empty(), unanswered.is_empty()) { + (true, true) => NoBuildMachine::Invisible, + (true, false) => NoBuildMachine::Silent { unanswered }, + _ => NoBuildMachine::Incapable { + observed: excluded.into_iter().chain(unanswered).collect(), + }, + }); + }; + Ok(Choice { + machine, + unconfirmed, + unanswered, + }) +} + +/// Required platforms this client cannot confirm the Machine runs natively. An +/// unreported or unrecognised architecture proves nothing, so every required +/// platform stays unconfirmed rather than being called emulated. +pub(crate) fn unconfirmed(architecture: &str, required: &BTreeSet) -> Vec { + required + .iter() + .filter(|platform| !platform_compatible(platform, architecture)) + .cloned() + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn candidate(hex: char, name: &str, evidence: Evidence) -> Candidate { + Candidate { + id: MachineId::parse(hex.to_string().repeat(32)).unwrap(), + name: MachineName::parse(name).unwrap(), + evidence, + } + } + + fn builds(architecture: &str) -> Evidence { + Evidence::Builds { + architecture: architecture.into(), + } + } + + fn platforms(values: [&str; N]) -> BTreeSet { + values.into_iter().map(ToOwned::to_owned).collect() + } + + #[test] + fn explicit_flags_override_compose_and_compose_overrides_the_local_default() { + let requested = Location::requested; + let pinned = |name: &str| Location::Remote(Selection::Pinned(name.parse().unwrap())); + assert_eq!(requested(None, false, None).unwrap(), Location::Local); + assert_eq!( + requested(None, false, Some("local")).unwrap(), + Location::Local + ); + assert_eq!( + requested(None, false, Some("auto")).unwrap(), + Location::Remote(Selection::Automatic) + ); + assert_eq!( + requested(None, false, Some("tower")).unwrap(), + pinned("tower") + ); + // Plain --remote asks for a fresh choice, never the configured pin. + assert_eq!( + requested(Some(""), false, Some("tower")).unwrap(), + Location::Remote(Selection::Automatic) + ); + assert_eq!( + requested(Some("edge"), false, Some("tower")).unwrap(), + pinned("edge") + ); + assert_eq!( + requested(None, true, Some("tower")).unwrap(), + Location::Local + ); + assert_eq!( + requested(None, true, Some("auto")).unwrap(), + Location::Local + ); + // The two mode words mean the same on the flag as in Compose. + assert_eq!( + requested(Some("auto"), false, Some("tower")).unwrap(), + Location::Remote(Selection::Automatic) + ); + assert_eq!( + requested(Some("local"), false, Some("tower")).unwrap(), + Location::Local + ); + assert!(requested(Some("*"), false, None).is_err()); + assert!(requested(None, false, Some("*")).is_err()); + } + + #[test] + fn automatic_selection_prefers_native_platforms_and_ignores_observation_order() { + let observed = |order: [char; 3]| { + order + .into_iter() + .map(|hex| match hex { + 'a' => candidate('a', "amd", builds("x86_64")), + 'b' => candidate('b', "arm", builds("aarch64")), + _ => candidate('c', "old", builds("")), + }) + .collect::>() + }; + for order in [['a', 'b', 'c'], ['c', 'b', 'a'], ['b', 'a', 'c']] { + let machines = observed(order); + let native = choose(&machines, &platforms(["linux/arm64"])).unwrap(); + assert_eq!(native.machine.name.as_str(), "arm"); + assert!(native.unconfirmed.is_empty() && native.unanswered.is_empty()); + // No native candidate: the lowest Machine ID wins and the platforms + // it cannot prove stay visible. + let emulating = choose(&machines, &platforms(["linux/riscv64"])).unwrap(); + assert_eq!(emulating.machine.name.as_str(), "amd"); + assert_eq!(emulating.unconfirmed, ["linux/riscv64"]); + // Nothing required: stable Machine-ID order alone decides. + assert_eq!( + choose(&machines, &BTreeSet::new()) + .unwrap() + .machine + .name + .as_str(), + "amd" + ); + } + } + + #[test] + fn incapable_and_unanswered_machines_are_not_candidates_but_stay_in_the_refusal() { + let machines = [ + candidate('a', "old", Evidence::Refuses), + candidate('b', "gone", Evidence::Unanswered("did not answer".into())), + ]; + let refusal = choose(&machines, &BTreeSet::new()).unwrap_err().to_string(); + assert!(refusal.contains("old"), "{refusal}"); + assert!(refusal.contains("does not run remote Builds"), "{refusal}"); + assert!(refusal.contains("gone"), "{refusal}"); + assert!(refusal.contains("did not answer"), "{refusal}"); + assert!( + matches!( + choose(&[], &BTreeSet::new()).unwrap_err(), + NoBuildMachine::Invisible + ), + "an empty Cluster is not a capability refusal" + ); + // Nothing answered at all is a reachability refusal, not a capability + // one. A Machine that was never eligible to be asked is neither. + let silent = [candidate( + 'b', + "gone", + Evidence::Unanswered("did not answer".into()), + )]; + assert!(matches!( + choose(&silent, &BTreeSet::new()).unwrap_err(), + NoBuildMachine::Silent { .. } + )); + let draining = [candidate( + 'b', + "gone", + Evidence::Ineligible("is draining".into()), + )]; + assert!( + matches!( + choose(&draining, &BTreeSet::new()).unwrap_err(), + NoBuildMachine::Incapable { .. } + ), + "a Machine that cannot be asked is not a connection problem" + ); + // One capable Machine among them is selected, and the Machine that + // never answered stays visible on that success. + let mut machines = Vec::from_iter(machines); + machines.push(candidate('c', "tower", builds("x86_64"))); + let choice = choose(&machines, &platforms(["linux/amd64"])).unwrap(); + assert_eq!(choice.machine.name.as_str(), "tower"); + assert!(choice.unconfirmed.is_empty()); + assert_eq!(choice.unanswered.len(), 1, "{choice:?}"); + assert!(choice.unanswered.concat().contains("gone"), "{choice:?}"); + } + + #[test] + fn a_multi_platform_request_prefers_the_machine_that_emulates_least() { + let machines = [ + // Reports no architecture: it would emulate both. + candidate('a', "old", builds("")), + candidate('b', "arm", builds("aarch64")), + ]; + let choice = choose(&machines, &platforms(["linux/amd64", "linux/arm64"])).unwrap(); + assert_eq!(choice.machine.name.as_str(), "arm"); + assert_eq!(choice.unconfirmed, ["linux/amd64"]); + // A variant suffix names the same execution platform. + assert!( + choose(&machines, &platforms(["linux/arm64/v8"])) + .unwrap() + .unconfirmed + .is_empty() + ); + } +} diff --git a/crates/ployz/src/cli/mod.rs b/crates/ployz/src/cli/mod.rs index 8228e280fb..bf34cf30a1 100644 --- a/crates/ployz/src/cli/mod.rs +++ b/crates/ployz/src/cli/mod.rs @@ -144,7 +144,7 @@ fn build_remote() -> Arg { .long("remote") .num_args(0..=1) .require_equals(true) - .default_missing_value("") + .default_missing_value(crate::build_location::AUTOMATIC) .value_name("MACHINE") .conflicts_with("local") } diff --git a/crates/ployz/src/compose/build.rs b/crates/ployz/src/compose/build.rs index dc4cf8d471..4abc72290a 100644 --- a/crates/ployz/src/compose/build.rs +++ b/crates/ployz/src/compose/build.rs @@ -611,6 +611,16 @@ fn effective_build_args( } impl CapturedBuild { + /// Platforms every captured target asked for. Empty means each target + /// builds for whatever platform its builder runs natively. + #[must_use] + pub fn platforms(&self) -> BTreeSet { + self.targets + .iter() + .flat_map(|target| target.platforms.iter().cloned()) + .collect() + } + /// Build this capture through the shared runner, without reading the /// original sources again. /// diff --git a/crates/ployz/src/compose/loader.rs b/crates/ployz/src/compose/loader.rs index e1a4aa4900..9dccdf483e 100644 --- a/crates/ployz/src/compose/loader.rs +++ b/crates/ployz/src/compose/loader.rs @@ -346,6 +346,35 @@ mod tests { use super::*; + #[test] + fn compose_carries_the_preferred_build_machine_and_refuses_a_non_string() { + let project = |preference: &str| { + parse_normalized( + &format!("name: demo\n{preference}services:\n api:\n image: busybox\n"), + ".", + ) + }; + assert_eq!(project("").unwrap().build_machine, None); + for preference in ["tower", "auto", "local"] { + assert_eq!( + project(&format!("x-build-machine: {preference}\n")) + .unwrap() + .build_machine + .as_deref(), + Some(preference) + ); + } + for invalid in ["x-build-machine: [tower]\n", "x-build-machine: \"\"\n"] { + assert!( + project(invalid) + .unwrap_err() + .to_string() + .contains("x-build-machine"), + "{invalid}" + ); + } + } + #[test] fn helper_contract_failures_do_not_suggest_editing_compose_settings() { let protocol = helper::(&serde_json::json!({"version": 2})).unwrap_err(); diff --git a/crates/ployz/src/compose/model.rs b/crates/ployz/src/compose/model.rs index 5128e19c96..00d3104cef 100644 --- a/crates/ployz/src/compose/model.rs +++ b/crates/ployz/src/compose/model.rs @@ -31,6 +31,9 @@ pub struct ComposeProject { pub name: String, pub working_dir: PathBuf, pub context: Option, + /// Preferred Build execution location from the Compose `x-build-machine` + /// extension: a Machine Target, `auto`, or `local`. + pub build_machine: Option, #[serde(deserialize_with = "deserialize_services")] pub services: BTreeMap, pub builds: BTreeMap, diff --git a/crates/ployz/src/handlers/build.rs b/crates/ployz/src/handlers/build.rs index 79ca86eb6c..45fdb86a24 100644 --- a/crates/ployz/src/handlers/build.rs +++ b/crates/ployz/src/handlers/build.rs @@ -1,6 +1,11 @@ +use std::collections::BTreeSet; + use clap::ArgMatches; use ployz_build::Output; +use ployz_core::MachineId; + +use crate::build_location::{self, Candidate, Evidence, Location, Selection}; use crate::compose::{ BuildOptions, LoadOptions, capture_build, execute_build, load_project, plan_build, @@ -23,17 +28,6 @@ pub(super) fn clear_cache(matches: &ArgMatches) -> Result<(), Error> { pub(super) fn run(matches: &ArgMatches) -> Result<(), Error> { let leaf = leaf_matches(matches); - let remote = leaf.get_one::("remote"); - if remote.is_some_and(String::is_empty) { - return Err(Error::usage( - "select a Build Machine with --remote=; automatic selection is not available", - )); - } - if remote.is_some() && leaf.get_flag("push") && !leaf.get_flag("check") { - return Err(Error::usage( - "remote build --push is not supported yet; remote build leaves the image on its selected Machine, or --push-registry publishes explicitly", - )); - } let load = LoadOptions { command: "build".into(), files: string_values(leaf, "file") @@ -55,26 +49,38 @@ pub(super) fn run(matches: &ArgMatches) -> Result<(), Error> { for warning in &project.warnings { eprintln!("WARNING: {warning}"); } + let location = Location::requested( + leaf.get_one::("remote").map(String::as_str), + leaf.get_flag("local"), + project.build_machine.as_deref(), + ) + .map_err(|error| Error::usage(error.to_string()))?; + if matches!(location, Location::Remote(_)) && leaf.get_flag("push") && !leaf.get_flag("check") { + return Err(Error::usage( + "--push is not supported yet for a Build on a Machine; that Build leaves its image on the selected Machine, --push-registry publishes explicitly, and --local builds here", + )); + } let plan = plan_build(&project, &options)?; if plan.is_empty() { println!("No buildable services selected."); return Ok(()); } - if let Some(target) = remote { - let target = ployz_core::MachineTarget::parse(target)?; + if let Location::Remote(selection) = location { let context = project .selected_context( leaf.get_one::("context").map(String::as_str), matches.get_one::("connect").map(String::as_str), ) .map(str::to_owned); + // Capture first: the platforms it fixes decide which Machines can run it. + let captured = capture_build(&plan, &options, &mut project)?; + let required = captured.platforms(); return runtime()?.block_on(async { let cancellation = super::cancellation_on_ctrl_c(); let mut client = connect_client(matches, context.as_deref()).await?; - let machine = select_build_machine(&mut client, &target).await?; - let captured = capture_build(&plan, &options, &mut project)?; + let machine = resolve_build_machine(&mut client, &selection, &required).await?; let outcome = captured - .execute_remote(&client, machine.id, cancellation.clone(), progress) + .execute_remote(&client, machine, cancellation.clone(), progress) .await; cancellation.cancel(); report_remote(outcome) @@ -140,27 +146,171 @@ pub(super) fn run(matches: &ArgMatches) -> Result<(), Error> { } } -pub(super) async fn select_build_machine( +/// One resolved Build Machine, with the evidence this client could not confirm. +struct Resolved { + name: ployz_core::MachineName, + id: MachineId, + /// Required platforms this client could not confirm run natively. + unconfirmed: Vec, + /// Visible Machines whose Build capability could not be observed. + unanswered: Vec, +} + +impl Resolved { + /// Name the Machine before any source leaves this client, then everything + /// the selection rests on but could not verify. + fn report(&self) { + let Self { + name, + id, + unconfirmed, + unanswered, + } = self; + println!("Selected Build Machine {name} ({id})"); + eprintln!("Build Machine: {id}"); + if !unconfirmed.is_empty() { + // Neither emulation support nor an unreported architecture is + // observable here, so claim only what was actually established. + eprintln!( + "Build Machine {name} is not confirmed to run {} natively; the Machine admits or refuses the Build itself", + unconfirmed.join(", ") + ); + } + if !unanswered.is_empty() { + // A silent Machine may have been the better candidate. Say so + // rather than let the choice look better evidenced than it is. + eprintln!( + "Build capability was not observed for {}", + unanswered.join("; ") + ); + } + } +} + +/// Resolve the Machine this Build runs on and report it before any upload. +/// +/// # Errors +/// +/// Fails when a named Machine is not visible, ambiguous, or cannot build, and +/// when no observed Machine can run an automatic Build. +pub(super) async fn resolve_build_machine( client: &mut crate::connect::Client, - target: &ployz_core::MachineTarget, -) -> Result { - let machine = client.build_machine(target).await?; - println!("Selected Build Machine {} ({})", machine.name, machine.id); - eprintln!("Build Machine: {}", machine.id); + selection: &Selection, + required: &BTreeSet, +) -> Result { + let resolved = match selection { + Selection::Pinned(target) => { + let machine = client.build_machine(target).await?; + let contract = describe(client, machine.id).await?; + if !contract.supports(ployz_core::BUILD_CAPABILITY) { + return Err(Error::usage(format!( + "Machine {} ({}) does not support remote Builds; name another with --remote=, or build here with --local", + machine.name, machine.id + ))); + } + Resolved { + unconfirmed: build_location::unconfirmed(&machine.runtime.architecture, required), + name: machine.name, + id: machine.id, + unanswered: Vec::new(), + } + } + Selection::Automatic => { + let candidates = observe_build_candidates(client).await?; + let choice = build_location::choose(&candidates, required).map_err(no_machine)?; + Resolved { + name: choice.machine.name.clone(), + id: choice.machine.id, + unconfirmed: choice.unconfirmed, + unanswered: choice.unanswered, + } + } + }; + resolved.report(); + Ok(resolved.id) +} + +/// The command that fixes an automatic selection with nothing to select. +fn no_machine(error: build_location::NoBuildMachine) -> Error { + let action = match error { + build_location::NoBuildMachine::Invisible => { + "add one with `ployz machine add`, or build here with --local" + } + build_location::NoBuildMachine::Silent { .. } => { + "check the Cluster connection and retry, or build here with --local" + } + build_location::NoBuildMachine::Incapable { .. } => { + "upgrade or restore a Machine from the evidence above, name one with --remote=, or build here with --local" + } + }; + Error::usage(format!("{error}; {action}")) +} + +/// Ask every visible Machine what it can build, keeping unanswered evidence. +/// +// ponytail: one concurrent probe per visible Machine, matching the Cluster +// fan-out in `cluster.rs`. Bound the concurrency if a Cluster ever grows past +// the handful of Machines the product targets; sequential probing would cost +// the 5s describe timeout per Machine on every automatic Build. +async fn observe_build_candidates( + client: &mut crate::connect::Client, +) -> Result, Error> { + let mut candidates = Vec::new(); + let mut probes = Vec::new(); + for observation in client.machines().await? { + let machine = observation.machine; + if !observation.membership.invites_rpc() { + candidates.push(Candidate { + id: machine.id, + name: machine.name, + evidence: Evidence::Ineligible(format!( + "has membership {} and does not invite an RPC", + observation.membership.as_str() + )), + }); + continue; + } + let probe = client.clone(); + probes.push(async move { + let evidence = match describe(&probe, machine.id).await { + Ok(contract) if contract.supports(ployz_core::BUILD_CAPABILITY) => { + Evidence::Builds { + architecture: machine.runtime.architecture, + } + } + Ok(_) => Evidence::Refuses, + Err(error) => Evidence::Unanswered(error.to_string()), + }; + Candidate { + id: machine.id, + name: machine.name, + evidence, + } + }); + } + candidates.extend(futures_util::future::join_all(probes).await); + Ok(candidates) +} + +/// Read one Machine's advertised contract, refusing an answer from another Machine. +async fn describe( + client: &crate::connect::Client, + machine_id: MachineId, +) -> Result { let contract = client .invoke::( ployz_core::DescribeContractRequest {}, - &ployz_core::MachineTarget::from(&machine.id), + &ployz_core::MachineTarget::from(&machine_id), Some(std::time::Duration::from_secs(5)), ) .await?; - if contract.machine_id != machine.id || !contract.supports(ployz_core::BUILD_CAPABILITY) { + if contract.machine_id != machine_id { return Err(Error::usage(format!( - "Machine {} does not support remote Builds", - machine.id + "Machine {machine_id} was answered by Machine {}", + contract.machine_id ))); } - Ok(machine) + Ok(contract) } pub(super) fn progress(event: ployz_build::Progress) { diff --git a/crates/ployz/src/handlers/deploy.rs b/crates/ployz/src/handlers/deploy.rs index b9b763622a..f23d7423ba 100644 --- a/crates/ployz/src/handlers/deploy.rs +++ b/crates/ployz/src/handlers/deploy.rs @@ -42,16 +42,15 @@ pub(super) fn run(root: &ArgMatches) -> Result<(), Error> { pub(super) fn deploy(root: &ArgMatches) -> Result<(), Error> { let matches = leaf_matches(root); - let remote = matches.get_one::("remote"); - if remote.is_some_and(String::is_empty) { - return Err(Error::usage( - "select a Build Machine with --remote=; automatic selection is not available", - )); - } - let remote = remote.map(ployz_core::MachineTarget::parse).transpose()?; let load = deploy_load(matches); let resolved = resolve_from_compose_load(matches, &load)?; let project = load_project(&load)?; + let location = crate::build_location::Location::requested( + matches.get_one::("remote").map(String::as_str), + matches.get_flag("local"), + project.build_machine.as_deref(), + ) + .map_err(|error| Error::usage(error.to_string()))?; let context = project .selected_context( matches.get_one::("context").map(String::as_str), @@ -72,13 +71,18 @@ pub(super) fn deploy(root: &ArgMatches) -> Result<(), Error> { .await?; // Every required Build finishes before preparation or application changes. let builds = match captured_build { - Some(build) => match remote { - Some(target) => { - let machine = super::build::select_build_machine(&mut client, &target).await?; + Some(build) => match &location { + crate::build_location::Location::Remote(selection) => { + let machine = super::build::resolve_build_machine( + &mut client, + selection, + &build.platforms(), + ) + .await?; let result = build .execute_remote_images( &client, - machine.id, + machine, cancellation.clone(), super::build::progress, ) @@ -91,7 +95,7 @@ pub(super) fn deploy(root: &ArgMatches) -> Result<(), Error> { } result.map_err(crate::deploy::DeployError::from)? } - None => build + crate::build_location::Location::Local => build .execute(load.docker.as_deref(), &cancellation) .map_err(crate::deploy::DeployError::from)?, }, diff --git a/crates/ployz/src/lib.rs b/crates/ployz/src/lib.rs index caf9f1dc6f..1956b22148 100644 --- a/crates/ployz/src/lib.rs +++ b/crates/ployz/src/lib.rs @@ -1,3 +1,4 @@ +mod build_location; mod cancellation; pub mod changes; pub mod cli; diff --git a/crates/ployz/tests/build_layer3/remote.rs b/crates/ployz/tests/build_layer3/remote.rs index 4c9ba995c4..f83c1bdf7f 100644 --- a/crates/ployz/tests/build_layer3/remote.rs +++ b/crates/ployz/tests/build_layer3/remote.rs @@ -551,3 +551,136 @@ async fn assert_temporary_tags_released(cluster: &Cluster) { .await .expect("command left temporary Build tags on a Machine"); } + +/// Two compatible candidates: automatic selection, an explicit pin, and the +/// local override that beats a Compose preference. +#[tokio::test] +#[ignore = "informing: requires the privileged Ployz testkit image with Buildx"] +async fn build_location_selects_automatically_honours_a_pin_and_yields_to_local() { + let cluster = Cluster::create( + ClusterPlan::new(&format!("l3-build-804-{}", std::process::id()), 2).unwrap(), + ) + .unwrap(); + let machines = cluster.initialize_two().await.unwrap(); + let first = machines.first().unwrap().id; + let second = machines.get(1).unwrap().id; + // Automatic selection breaks the tie between equally capable Machines on + // Machine ID alone, so the expected candidate is known before the run. + let (automatic, automatic_index) = if first.as_str() <= second.as_str() { + (first, 0) + } else { + (second, 1) + }; + let pinned_index = 1 - automatic_index; + let pinned = *[first, second].get(pinned_index).unwrap(); + let address = cluster.api_address(0).unwrap(); + let root = std::env::temp_dir().join(format!("ployz-build-804-{}", uuid::Uuid::new_v4())); + fs::create_dir_all(&root).unwrap(); + let image = format!("registry.invalid/ployz-804-{}:built", std::process::id()); + fs::write( + root.join("Dockerfile"), + "FROM alpine:3.23.3\nCOPY payload /payload\nCMD [\"sleep\", \"3600\"]\n", + ) + .unwrap(); + fs::write(root.join("payload"), "selected-machine-ran\n").unwrap(); + fs::write(root.join(".dockerignore"), "docker\nconfig.yaml\n").unwrap(); + fs::write( + root.join("docker"), + format!( + "#!/bin/sh\nprintf invoked > '{}'\nexit 99\n", + root.join("local-docker-called").display() + ), + ) + .unwrap(); + fs::set_permissions(root.join("docker"), fs::Permissions::from_mode(0o700)).unwrap(); + let compose = |preference: &str| { + fs::write( + root.join("compose.yaml"), + format!( + "name: remote\n{preference}services:\n app:\n image: {image}\n pull_policy: never\n x-machines: [{first}]\n build: .\n" + ), + ) + .unwrap(); + }; + let run = |args: Vec| { + let mut command = tokio::process::Command::new(env!("CARGO_BIN_EXE_ployz")); + command + .current_dir(&root) + .env("PATH", &root) + .env("HOME", &root) + .env("PLOYZ_CONFIG", root.join("config.yaml")) + .env("DOCKER_HOST", "unix:///no-local-docker.sock") + .args(["--connect", &address]) + .args(args) + .kill_on_drop(true); + command + }; + let succeed = async |args: Vec| { + let output = tokio::time::timeout(Duration::from_secs(240), run(args).output()) + .await + .unwrap() + .unwrap(); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!( + output.status.success(), + "{stdout}{}", + String::from_utf8_lossy(&output.stderr) + ); + stdout + }; + + // Plain --remote selects a compatible Machine and leaves the image there. + compose(""); + let stdout = succeed(vec!["build".into(), "--remote".into(), "app".into()]).await; + assert!(stdout.contains(automatic.as_str()), "{stdout}"); + let built = stdout + .split_whitespace() + .find(|word| word.starts_with("sha256:")) + .expect("remote exact image identity") + .to_owned(); + assert!( + cluster + .machine_shell(automatic_index, &format!("docker image inspect {built}")) + .is_ok() + ); + assert!( + cluster + .machine_shell(pinned_index, &format!("docker image inspect {built}")) + .is_err(), + "automatic selection left the image on both Machines" + ); + + // An explicit pin builds on the other Machine and still deploys from it. + let stdout = succeed(vec![ + "deploy".into(), + format!("--remote={pinned}"), + "--yes".into(), + "app".into(), + ]) + .await; + assert!(stdout.contains(pinned.as_str()), "{stdout}"); + let container = cluster + .machine_shell(0, "docker ps -q --filter label=ployz.service.name=app") + .unwrap(); + let container = container.trim(); + assert!(!container.is_empty(), "application was not running"); + assert_eq!( + cluster + .machine_shell(0, &format!("docker exec {container} cat /payload")) + .unwrap(), + "selected-machine-ran\n" + ); + + // --local overrides a Compose preference and runs on this host's Docker, + // which the fixture stub refuses. Compose precedence itself is rung 3. + compose(&format!("x-build-machine: {pinned}\n")); + assert!(!root.join("local-docker-called").exists()); + let output = run(vec!["build".into(), "--local".into(), "app".into()]) + .output() + .await + .unwrap(); + assert!(!output.status.success()); + assert!(root.join("local-docker-called").exists()); + assert_temporary_tags_released(&cluster).await; + fs::remove_dir_all(root).unwrap(); +} diff --git a/crates/ployz/tests/cli_shape.rs b/crates/ployz/tests/cli_shape.rs index 345023547c..275db54e50 100644 --- a/crates/ployz/tests/cli_shape.rs +++ b/crates/ployz/tests/cli_shape.rs @@ -176,9 +176,16 @@ fn remote_build_target_requires_equals_and_preserves_positional_service() { "api", ), (vec!["ployz", "build", "--remote", "api"], "", "api"), + ( + vec!["ployz", "deploy", "--remote=tower", "api"], + "tower", + "api", + ), + (vec!["ployz", "deploy", "--remote", "api"], "", "api"), ] { - let matches = ployz::cli::command().try_get_matches_from(args).unwrap(); - let build = matches.subcommand_matches("build").unwrap(); + let command = *args.get(1).unwrap(); + let matches = ployz::cli::command().try_get_matches_from(&args).unwrap(); + let build = matches.subcommand_matches(command).unwrap(); assert_eq!( build.get_one::("remote").map(String::as_str), Some(target) @@ -192,9 +199,29 @@ fn remote_build_target_requires_equals_and_preserves_positional_service() { [service] ); } - assert!( - ployz::cli::command() - .try_get_matches_from(["ployz", "build", "--local", "--remote=tower"]) - .is_err() - ); + // Both commands define --local, so the refusals below are conflicts rather + // than an unknown flag. + for command in ["build", "deploy"] { + let matches = ployz::cli::command() + .try_get_matches_from(["ployz", command, "--local", "api"]) + .unwrap(); + assert!( + matches + .subcommand_matches(command) + .unwrap() + .get_flag("local") + ); + } + for conflicting in [ + vec!["ployz", "build", "--local", "--remote=tower"], + vec!["ployz", "deploy", "--local", "--remote=tower"], + vec!["ployz", "deploy", "--no-build", "--remote=tower"], + ] { + assert!( + ployz::cli::command() + .try_get_matches_from(&conflicting) + .is_err(), + "{conflicting:?}" + ); + } } diff --git a/crates/ployz/tests/connect/support.rs b/crates/ployz/tests/connect/support.rs index 4f03fe89c7..b4ec7bfe67 100644 --- a/crates/ployz/tests/connect/support.rs +++ b/crates/ployz/tests/connect/support.rs @@ -142,6 +142,8 @@ fn send_watch_event(sender: &mpsc::Sender>, event: pub(super) struct DiscoveryService { pub(super) builds: Option>, description: ContractDescription, + /// Contracts answered per routed Machine. Absent Machines answer `description`. + pub(super) descriptions: BTreeMap, pub(super) describe_outcomes: Arc>>, pub(super) stream_opens: Arc, pub(super) watch_opens: Arc, @@ -179,6 +181,7 @@ impl DiscoveryService { Self { builds: None, description, + descriptions: BTreeMap::new(), describe_outcomes: Arc::new(Mutex::new(VecDeque::new())), stream_opens: Arc::new(AtomicUsize::new(0)), watch_opens: Arc::new(AtomicUsize::new(0)), @@ -406,6 +409,7 @@ impl MachineRpc for DiscoveryService { } None => {} } + let metadata = request.metadata().clone(); let request = request .into_inner() .decode_request() @@ -413,8 +417,18 @@ impl MachineRpc for DiscoveryService { if !matches!(request.body, RpcRequestBody::DescribeContract(_)) { return Err(Status::invalid_argument("expected discovery request")); } + let routed = match ployz_core::routing_from_metadata(&metadata) { + Ok(ployz_core::RoutingRequest::One(target)) => self + .machines + .iter() + .find(|observation| { + ployz_core::machine_matches_target(&observation.machine, &target) + }) + .and_then(|observation| self.descriptions.get(&observation.machine.id)), + _ => None, + }; Ok(Response::new( - RpcResponse::from(self.description.clone()) + RpcResponse::from(routed.unwrap_or(&self.description).clone()) .encode() .unwrap(), )) diff --git a/crates/ployz/tests/remote_build_cli.rs b/crates/ployz/tests/remote_build_cli.rs index 41f9b2eec9..3470a4bd0c 100644 --- a/crates/ployz/tests/remote_build_cli.rs +++ b/crates/ployz/tests/remote_build_cli.rs @@ -55,6 +55,8 @@ async fn standalone_remote_build_never_invokes_local_docker_and_keeps_the_servic for args in [ vec!["--remote=tower", "api"], vec!["--remote=tower", "--check", "api"], + // Plain --remote selects the only Machine that advertises Builds. + vec!["--remote", "api"], ] { let output = tokio::time::timeout(Duration::from_secs(20), run(&args).output()) .await @@ -78,7 +80,7 @@ async fn standalone_remote_build_never_invokes_local_docker_and_keeps_the_servic "{stdout}" ); } - assert_eq!(recorder.uploads.load(Ordering::SeqCst), 2); + assert_eq!(recorder.uploads.load(Ordering::SeqCst), 3); assert!( recorder .targets @@ -97,16 +99,16 @@ async fn standalone_remote_build_never_invokes_local_docker_and_keeps_the_servic == &RoutingRequest::One(MachineTarget::from(&support::machine_id('a')))) ); for args in [ - vec!["--remote", "api"], vec!["--remote=tower", "--local", "api"], vec!["--remote=missing", "api"], + vec!["--remote", "--push", "api"], ] { let output = run(&args).output().await.unwrap(); - assert!(!output.status.success()); + assert!(!output.status.success(), "{args:?}"); } assert_eq!( recorder.uploads.load(Ordering::SeqCst), - 2, + 3, "refusals must not resubmit source" ); assert!( @@ -138,7 +140,7 @@ async fn standalone_remote_build_never_invokes_local_docker_and_keeps_the_servic } assert_eq!( recorder.uploads.load(Ordering::SeqCst), - 2, + 3, "SSH refusal must precede upload" ); fs::write(root.join("key"), "captured-key").unwrap(); @@ -149,7 +151,7 @@ async fn standalone_remote_build_never_invokes_local_docker_and_keeps_the_servic "{}", String::from_utf8_lossy(&output.stderr) ); - assert_eq!(recorder.uploads.load(Ordering::SeqCst), 3); + assert_eq!(recorder.uploads.load(Ordering::SeqCst), 4); fs::write( root.join("compose.yaml"), "name: demo\nservices:\n api:\n build: {context: ., x-recipe: railpack}\n", @@ -161,7 +163,7 @@ async fn standalone_remote_build_never_invokes_local_docker_and_keeps_the_servic "{}", String::from_utf8_lossy(&output.stderr) ); - assert_eq!(recorder.uploads.load(Ordering::SeqCst), 4); + assert_eq!(recorder.uploads.load(Ordering::SeqCst), 5); assert!(!root.join("docker-called").exists()); server.abort(); fs::remove_dir_all(root).unwrap(); @@ -247,3 +249,158 @@ async fn remote_queue_outcomes_name_machine_and_unattempted_work() { } fs::remove_dir_all(root).unwrap(); } + +/// Automatic selection, Compose preference, and the overrides that beat it. +#[tokio::test] +async fn automatic_selection_uses_capable_machines_and_explicit_flags_beat_compose() { + let root = std::env::temp_dir().join(format!("ployz-auto-cli-{}", uuid::Uuid::new_v4())); + fs::create_dir_all(&root).unwrap(); + fs::write( + root.join("docker"), + format!( + "#!/bin/sh\nprintf called > '{}'\nexit 99\n", + root.join("docker-called").display() + ), + ) + .unwrap(); + fs::set_permissions(root.join("docker"), fs::Permissions::from_mode(0o700)).unwrap(); + fs::write(root.join("Dockerfile"), "FROM scratch\n").unwrap(); + let mut builds = support::test_description(); + builds.machine_id = support::machine_id('a'); + builds + .capabilities + .insert(BUILD_CAPABILITY.parse().unwrap()); + let mut idle = support::test_description(); + idle.machine_id = support::machine_id('b'); + let capable = |description: ployz_core::ContractDescription| { + let mut service = support::DiscoveryService::new(description); + service.machines = vec![ + support::machine('a', "tower"), + support::machine('b', "forge"), + ]; + service + .descriptions + .insert(support::machine_id('b'), idle.clone()); + service.builds = Some(Arc::new(support::BuildRecorder::default())); + service + }; + let compose = |preference: &str| { + fs::write( + root.join("compose.yaml"), + format!( + "name: demo\n{preference}services:\n api:\n image: example.test/api:built\n build: .\n" + ), + ) + .unwrap(); + }; + let run = |address: &std::net::SocketAddr, args: &[&str]| { + let mut command = tokio::process::Command::new(env!("CARGO_BIN_EXE_ployz")); + command + .current_dir(&root) + .env("PATH", &root) + .env("HOME", &root) + .env("PLOYZ_CONFIG", root.join("config.yaml")) + .env("DOCKER_HOST", "unix:///missing-local-docker.sock") + .args(["--connect", &format!("tcp://{address}"), "build"]) + .args(args); + command + }; + let (address, server) = support::serve_discovery(capable(builds.clone())).await; + // Automatic selection skips the Machine that does not advertise Builds, + // whether it was asked for on the command line or in Compose. + for (preference, args) in [ + ("", vec!["--remote", "api"]), + ("x-build-machine: auto\n", vec!["api"]), + // An explicit pin beats the Compose preference. + ("x-build-machine: forge\n", vec!["--remote=tower", "api"]), + ] { + compose(preference); + let output = tokio::time::timeout(Duration::from_secs(20), run(&address, &args).output()) + .await + .unwrap() + .unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + output.status.success(), + "{stdout}{}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(stdout.contains("tower"), "{stdout}"); + assert!( + stdout.contains(support::machine_id('a').as_str()), + "{stdout}" + ); + } + // An ambiguous name refuses the same way from the flag and from Compose, + // naming every Machine it matched. + let (ambiguous_address, ambiguous_server) = { + let mut service = support::DiscoveryService::new(builds.clone()); + service.machines = vec![ + support::machine('a', "tower"), + support::machine_named(&support::machine_id('b'), "tower"), + ]; + support::serve_discovery(service).await + }; + for (preference, args) in [ + ("", vec!["--remote=tower", "api"]), + ("x-build-machine: tower\n", vec!["api"]), + ] { + compose(preference); + let output = run(&ambiguous_address, &args).output().await.unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(!output.status.success(), "{stderr}"); + assert!( + stderr.contains("ambiguous") + && stderr.contains(support::machine_id('a').as_str()) + && stderr.contains(support::machine_id('b').as_str()), + "{stderr}" + ); + } + ambiguous_server.abort(); + + // A Compose preference naming an incapable Machine refuses with evidence. + compose("x-build-machine: forge\n"); + let output = run(&address, &["api"]).output().await.unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(!output.status.success(), "{stderr}"); + assert!( + stderr.contains("forge") && stderr.contains("--local"), + "{stderr}" + ); + assert!(!root.join("docker-called").exists(), "{stderr}"); + // --local overrides the Compose preference and runs here. + compose("x-build-machine: tower\n"); + assert!( + !run(&address, &["--local", "api"]) + .output() + .await + .unwrap() + .status + .success() + ); + assert!(root.join("docker-called").exists()); + server.abort(); + + // No visible Machine advertises Builds: every one of them answers without + // the capability, so the refusal names them all. + let mut refusing = capable(idle.clone()); + let mut without_builds = support::test_description(); + without_builds.machine_id = support::machine_id('a'); + refusing + .descriptions + .insert(support::machine_id('a'), without_builds); + let (address, server) = support::serve_discovery(refusing).await; + compose(""); + let output = run(&address, &["--remote", "api"]).output().await.unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(!output.status.success(), "{stderr}"); + assert!( + stderr.contains("tower") + && stderr.contains("forge") + && stderr.contains("does not run remote Builds") + && stderr.contains("--remote="), + "{stderr}" + ); + server.abort(); + fs::remove_dir_all(root).unwrap(); +} From 8737565f55cc5db0d478e96063f5ab9ce2646efb Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 9 Sep 2026 12:02:38 +0000 Subject: [PATCH 2/2] Drop the Compose build location and keep silence out of capability refusals Machine selectors are Cluster-local and may be ambiguous (CONTEXT.md:30, 33), so a committed Compose file cannot name one honestly: the same `x-build-machine: tower` means a different Machine in every checkout, and none at all in CI. Remove the extension rather than validate it. Build location is per invocation: `--remote=`, plain `--remote`, `--local`, or the local default. `auto` and `local` stop being reserved words, since no Compose key spells modes any more. Refusals no longer turn silence into a capability conclusion. A Machine that never answered may still have been able to build, so the "none can build" verdict is only available when every visible Machine answered for itself; otherwise the refusal is Inconclusive and advises waiting and retrying. Membership ineligibility counts as silence, not as a refusal: a Membership Observation is one Machine's potentially stale judgment of another and is not the observed Machine speaking (CONTEXT.md:50), so it cannot settle capability either. Both follow DESIGN.md bet 8, which permits a diagnosis to remain Unknown and flags inferring certainty the observer lacks. --- README.md | 21 ++- crates/ployz/compose-helper/convert.go | 6 +- crates/ployz/src/build_location.rs | 210 ++++++++++------------ crates/ployz/src/compose/loader.rs | 29 --- crates/ployz/src/compose/model.rs | 3 - crates/ployz/src/handlers/build.rs | 19 +- crates/ployz/src/handlers/deploy.rs | 1 - crates/ployz/tests/build_layer3/remote.rs | 7 +- crates/ployz/tests/remote_build_cli.rs | 36 ++-- 9 files changed, 134 insertions(+), 198 deletions(-) diff --git a/README.md b/README.md index 2d22064fa5..8cfc81686c 100644 --- a/README.md +++ b/README.md @@ -51,17 +51,16 @@ pinned frontend, `--no-cache` and `--pull` force a cold build by clearing the exclusive Ployz builder cache; unrelated Docker builder caches are untouched. `ployz build` and `ployz deploy` run here by default. `--remote=` pins -one Machine, plain `--remote` selects one automatically, and `--local` forces the -local default. Compose sets the preference between them with a top-level -`x-build-machine:` of a Machine name or ID, `auto`, or `local`; any of the three -flags overrides it. Automatic selection uses visible Machines that advertise -remote Builds, prefers the one whose reported architecture runs the most -requested `build.platforms` natively, and breaks ties on Machine ID, so it does -not depend on observation order. `auto` and `local` name modes wherever they -appear, so a Machine called either is pinned by its ID. The selected Machine is -reported before any upload, along with any platform this client could not -confirm it runs natively and any Machine that never answered; it still admits or -refuses the Build itself, and Ployz never falls back to a local Build silently. +one Machine, plain `--remote` selects one automatically, and `--local` states the +local default explicitly. Build location is per invocation: Machine selectors are +Cluster-local and may be ambiguous, so no Compose field pins one. Automatic +selection uses visible Machines that advertise remote Builds, prefers the one +whose reported architecture runs the most requested `build.platforms` natively, +and breaks ties on Machine ID, so it does not depend on observation order. The +selected Machine is reported before any upload, along with any platform this +client could not confirm it runs natively and any Machine that did not answer +for itself; it still admits or refuses the Build itself, and Ployz never falls +back to a local Build silently. Remote Builds (`ployz build --remote=`) use one active slot per Machine and a FIFO of eight waiting attempts. Source and secrets stay diff --git a/crates/ployz/compose-helper/convert.go b/crates/ployz/compose-helper/convert.go index a23fa02d1d..a750b9795b 100644 --- a/crates/ployz/compose-helper/convert.go +++ b/crates/ployz/compose-helper/convert.go @@ -83,11 +83,7 @@ func convert(p *types.Project, model, provisioned, secrets object) (object, erro } warnings = append(warnings, w...) } - buildMachine, declared := p.Extensions["x-build-machine"] - if value, _ := buildMachine.(string); declared && value == "" { - return nil, fmt.Errorf("x-build-machine must be a Machine name or ID, 'auto', or 'local'") - } - return object{"name": p.Name, "working_dir": p.WorkingDir, "context": p.Extensions["x-context"], "build_machine": buildMachine, "services": services, "builds": builds, "dependencies": dependencies, "service_profiles": profiles, "warnings": warnings, "secrets": secrets, "environment": p.Environment}, nil + return object{"name": p.Name, "working_dir": p.WorkingDir, "context": p.Extensions["x-context"], "services": services, "builds": builds, "dependencies": dependencies, "service_profiles": profiles, "warnings": warnings, "secrets": secrets, "environment": p.Environment}, nil } func service(p *types.Project, s types.ServiceConfig, raw, provisioned object) (object, error) { diff --git a/crates/ployz/src/build_location.rs b/crates/ployz/src/build_location.rs index 9e0dcb5d7b..0e3fb86291 100644 --- a/crates/ployz/src/build_location.rs +++ b/crates/ployz/src/build_location.rs @@ -14,14 +14,10 @@ pub(crate) const AUTOMATIC: &str = ""; use crate::image::platform_compatible; -/// A requested build location that names no Machine. The origin is part of the -/// message: the same text can come from a flag or from Compose. +/// A `--remote` value that names no Machine. #[derive(Debug, Error)] -#[error("{origin} {value:?} is not a Machine name or ID, `auto`, or `local`")] -pub(crate) struct InvalidLocation { - origin: &'static str, - value: String, -} +#[error("--remote {0:?} is not a Machine name or ID")] +pub(crate) struct InvalidLocation(String); /// Execution location a user asked for, before any Machine is observed. #[derive(Clone, Debug, Eq, PartialEq)] @@ -42,41 +38,25 @@ pub(crate) enum Selection { } impl Location { - /// Resolve the requested location. An explicit flag always beats Compose, - /// and Compose beats the local default. + /// Resolve the requested location. A Build runs on this host unless a flag + /// sends it to a Machine. /// /// `remote` is the `--remote` value: absent when the flag was not given, - /// empty for plain `--remote`, which always asks for a fresh automatic - /// choice rather than the Compose pin. `preferred` is the Compose - /// `x-build-machine` value. `auto` and `local` name modes wherever they - /// appear, so a Machine called either is pinned by its ID instead. + /// [`AUTOMATIC`] for plain `--remote`, otherwise the pinned Machine. /// /// # Errors /// - /// Returns [`InvalidLocation`] when a pinned target is not a Machine - /// identity, naming the flag or Compose key it was written in. - pub(crate) fn requested( - remote: Option<&str>, - local: bool, - preferred: Option<&str>, - ) -> Result { + /// Returns [`InvalidLocation`] when a pinned target is not a Machine identity. + pub(crate) fn requested(remote: Option<&str>, local: bool) -> Result { if local { return Ok(Self::Local); } - let (origin, value) = match (remote, preferred) { - (Some(value), _) => ("--remote", value), - (None, Some(value)) => ("Compose x-build-machine", value), - (None, None) => return Ok(Self::Local), - }; - match value { - "local" => Ok(Self::Local), - AUTOMATIC | "auto" => Ok(Self::Remote(Selection::Automatic)), - target => MachineTarget::parse(target) + match remote { + None => Ok(Self::Local), + Some(AUTOMATIC) => Ok(Self::Remote(Selection::Automatic)), + Some(target) => MachineTarget::parse(target) .map(|target| Self::Remote(Selection::Pinned(target))) - .map_err(|_| InvalidLocation { - origin, - value: target.to_owned(), - }), + .map_err(|_| InvalidLocation(target.to_owned())), } } } @@ -114,9 +94,9 @@ pub(crate) struct Choice<'list> { /// Required platforms this client could not confirm the Machine runs /// natively. The Machine confirms or refuses them at admission. pub unconfirmed: Vec, - /// Visible Machines that never answered, so a better candidate may have - /// been passed over. One line of evidence each. - pub unanswered: Vec, + /// Visible Machines that never answered for themselves, so a better + /// candidate may have been passed over. One line of evidence each. + pub unresolved: Vec, } /// Why no Machine could be chosen. The observed evidence belongs here; the @@ -125,10 +105,15 @@ pub(crate) struct Choice<'list> { pub(crate) enum NoBuildMachine { #[error("no Machine is visible, so no Build Machine can be selected")] Invisible, - #[error("no visible Machine answered, so none could be selected. Observed {}", .unanswered.join("; "))] - Silent { unanswered: Vec }, - #[error("no visible Machine can run this Build. Observed {}", .observed.join("; "))] - Incapable { observed: Vec }, + /// At least one Machine never spoke for itself, so "none can build" would + /// claim more than was observed. + #[error("no Machine that answered can run this Build, and {} did not answer. Observed {}", .unresolved.len(), observed(.refused, .unresolved))] + Inconclusive { + refused: Vec, + unresolved: Vec, + }, + #[error("no visible Machine can run this Build. Observed {}", .refused.join("; "))] + Incapable { refused: Vec }, } /// Pick one Machine for an automatic Build. @@ -147,8 +132,8 @@ pub(crate) fn choose<'list>( required: &BTreeSet, ) -> Result, NoBuildMachine> { let mut eligible = Vec::new(); - let mut excluded = Vec::new(); - let mut unanswered = Vec::new(); + let mut refused = Vec::new(); + let mut unresolved = Vec::new(); for candidate in candidates { let name = &candidate.name; let id = &candidate.id; @@ -157,11 +142,13 @@ pub(crate) fn choose<'list>( eligible.push((candidate, unconfirmed(architecture, required))); } Evidence::Refuses => { - excluded.push(format!("{name} ({id}) does not run remote Builds")); + refused.push(format!("{name} ({id}) does not run remote Builds")); } - Evidence::Ineligible(reason) => excluded.push(format!("{name} ({id}) {reason}")), + // Membership is one Machine's stale judgment of another, never the + // Machine speaking for itself, so it cannot settle capability. + Evidence::Ineligible(reason) => unresolved.push(format!("{name} ({id}) {reason}")), Evidence::Unanswered(reason) => { - unanswered.push(format!("{name} ({id}) capability unknown: {reason}")); + unresolved.push(format!("{name} ({id}) capability unknown: {reason}")); } } } @@ -174,23 +161,34 @@ pub(crate) fn choose<'list>( .then_with(|| left.id.as_str().cmp(right.id.as_str())) }); let Some((machine, unconfirmed)) = chosen else { - // Nothing answered is a reachability problem, not a capability one; - // telling that user to upgrade a Machine would be wrong advice. - return Err(match (excluded.is_empty(), unanswered.is_empty()) { + // Any silence leaves a Machine that might have built this, so the + // capability conclusion is only available when every Machine answered. + return Err(match (refused.is_empty(), unresolved.is_empty()) { (true, true) => NoBuildMachine::Invisible, - (true, false) => NoBuildMachine::Silent { unanswered }, - _ => NoBuildMachine::Incapable { - observed: excluded.into_iter().chain(unanswered).collect(), + (_, false) => NoBuildMachine::Inconclusive { + refused, + unresolved, }, + (false, true) => NoBuildMachine::Incapable { refused }, }); }; Ok(Choice { machine, unconfirmed, - unanswered, + unresolved, }) } +/// Every observed line, refusals before silence. +fn observed(refused: &[String], unresolved: &[String]) -> String { + refused + .iter() + .chain(unresolved) + .cloned() + .collect::>() + .join("; ") +} + /// Required platforms this client cannot confirm the Machine runs natively. An /// unreported or unrecognised architecture proves nothing, so every required /// platform stays unconfirmed rather than being called emulated. @@ -225,50 +223,27 @@ mod tests { } #[test] - fn explicit_flags_override_compose_and_compose_overrides_the_local_default() { + fn flags_choose_the_location_and_a_bad_pin_names_the_flag() { let requested = Location::requested; let pinned = |name: &str| Location::Remote(Selection::Pinned(name.parse().unwrap())); - assert_eq!(requested(None, false, None).unwrap(), Location::Local); - assert_eq!( - requested(None, false, Some("local")).unwrap(), - Location::Local - ); - assert_eq!( - requested(None, false, Some("auto")).unwrap(), - Location::Remote(Selection::Automatic) - ); - assert_eq!( - requested(None, false, Some("tower")).unwrap(), - pinned("tower") - ); - // Plain --remote asks for a fresh choice, never the configured pin. - assert_eq!( - requested(Some(""), false, Some("tower")).unwrap(), - Location::Remote(Selection::Automatic) - ); - assert_eq!( - requested(Some("edge"), false, Some("tower")).unwrap(), - pinned("edge") - ); - assert_eq!( - requested(None, true, Some("tower")).unwrap(), - Location::Local - ); - assert_eq!( - requested(None, true, Some("auto")).unwrap(), - Location::Local - ); - // The two mode words mean the same on the flag as in Compose. + assert_eq!(requested(None, false).unwrap(), Location::Local); + assert_eq!(requested(None, true).unwrap(), Location::Local); + // Plain --remote asks for a choice; a value pins one Machine. assert_eq!( - requested(Some("auto"), false, Some("tower")).unwrap(), + requested(Some(AUTOMATIC), false).unwrap(), Location::Remote(Selection::Automatic) ); - assert_eq!( - requested(Some("local"), false, Some("tower")).unwrap(), - Location::Local + assert_eq!(requested(Some("tower"), false).unwrap(), pinned("tower")); + // --local wins over a --remote clap already refuses to pair it with. + assert_eq!(requested(Some("tower"), true).unwrap(), Location::Local); + // Nothing reserves these words now that no Compose key spells modes. + assert_eq!(requested(Some("auto"), false).unwrap(), pinned("auto")); + assert_eq!(requested(Some("local"), false).unwrap(), pinned("local")); + let rejected = requested(Some("*"), false).unwrap_err().to_string(); + assert!( + rejected.contains("--remote") && rejected.contains("*"), + "{rejected}" ); - assert!(requested(Some("*"), false, None).is_err()); - assert!(requested(None, false, Some("*")).is_err()); } #[test] @@ -287,7 +262,7 @@ mod tests { let machines = observed(order); let native = choose(&machines, &platforms(["linux/arm64"])).unwrap(); assert_eq!(native.machine.name.as_str(), "arm"); - assert!(native.unconfirmed.is_empty() && native.unanswered.is_empty()); + assert!(native.unconfirmed.is_empty() && native.unresolved.is_empty()); // No native candidate: the lowest Machine ID wins and the platforms // it cannot prove stay visible. let emulating = choose(&machines, &platforms(["linux/riscv64"])).unwrap(); @@ -323,29 +298,38 @@ mod tests { ), "an empty Cluster is not a capability refusal" ); - // Nothing answered at all is a reachability refusal, not a capability - // one. A Machine that was never eligible to be asked is neither. - let silent = [candidate( - 'b', - "gone", + // Any silence leaves the capability question open, whether it came + // from a failed probe or from another Machine's judgment of it. + for unresolved in [ Evidence::Unanswered("did not answer".into()), - )]; + Evidence::Ineligible("has membership down and does not invite an RPC".into()), + ] { + let quiet = [candidate('b', "gone", unresolved)]; + assert!(matches!( + choose(&quiet, &BTreeSet::new()).unwrap_err(), + NoBuildMachine::Inconclusive { .. } + )); + // A definite refusal alongside silence still cannot conclude. + let mixed = [ + candidate('a', "old", Evidence::Refuses), + quiet.into_iter().next().unwrap(), + ]; + let error = choose(&mixed, &BTreeSet::new()).unwrap_err(); + assert!( + matches!(error, NoBuildMachine::Inconclusive { .. }), + "one silent Machine must not license a capability conclusion" + ); + assert!(error.to_string().contains("did not answer"), "{error}"); + } + // Only when every visible Machine answered is the conclusion available. assert!(matches!( - choose(&silent, &BTreeSet::new()).unwrap_err(), - NoBuildMachine::Silent { .. } + choose( + &[candidate('a', "old", Evidence::Refuses)], + &BTreeSet::new() + ) + .unwrap_err(), + NoBuildMachine::Incapable { .. } )); - let draining = [candidate( - 'b', - "gone", - Evidence::Ineligible("is draining".into()), - )]; - assert!( - matches!( - choose(&draining, &BTreeSet::new()).unwrap_err(), - NoBuildMachine::Incapable { .. } - ), - "a Machine that cannot be asked is not a connection problem" - ); // One capable Machine among them is selected, and the Machine that // never answered stays visible on that success. let mut machines = Vec::from_iter(machines); @@ -353,8 +337,8 @@ mod tests { let choice = choose(&machines, &platforms(["linux/amd64"])).unwrap(); assert_eq!(choice.machine.name.as_str(), "tower"); assert!(choice.unconfirmed.is_empty()); - assert_eq!(choice.unanswered.len(), 1, "{choice:?}"); - assert!(choice.unanswered.concat().contains("gone"), "{choice:?}"); + assert_eq!(choice.unresolved.len(), 1, "{choice:?}"); + assert!(choice.unresolved.concat().contains("gone"), "{choice:?}"); } #[test] diff --git a/crates/ployz/src/compose/loader.rs b/crates/ployz/src/compose/loader.rs index 9dccdf483e..e1a4aa4900 100644 --- a/crates/ployz/src/compose/loader.rs +++ b/crates/ployz/src/compose/loader.rs @@ -346,35 +346,6 @@ mod tests { use super::*; - #[test] - fn compose_carries_the_preferred_build_machine_and_refuses_a_non_string() { - let project = |preference: &str| { - parse_normalized( - &format!("name: demo\n{preference}services:\n api:\n image: busybox\n"), - ".", - ) - }; - assert_eq!(project("").unwrap().build_machine, None); - for preference in ["tower", "auto", "local"] { - assert_eq!( - project(&format!("x-build-machine: {preference}\n")) - .unwrap() - .build_machine - .as_deref(), - Some(preference) - ); - } - for invalid in ["x-build-machine: [tower]\n", "x-build-machine: \"\"\n"] { - assert!( - project(invalid) - .unwrap_err() - .to_string() - .contains("x-build-machine"), - "{invalid}" - ); - } - } - #[test] fn helper_contract_failures_do_not_suggest_editing_compose_settings() { let protocol = helper::(&serde_json::json!({"version": 2})).unwrap_err(); diff --git a/crates/ployz/src/compose/model.rs b/crates/ployz/src/compose/model.rs index 00d3104cef..5128e19c96 100644 --- a/crates/ployz/src/compose/model.rs +++ b/crates/ployz/src/compose/model.rs @@ -31,9 +31,6 @@ pub struct ComposeProject { pub name: String, pub working_dir: PathBuf, pub context: Option, - /// Preferred Build execution location from the Compose `x-build-machine` - /// extension: a Machine Target, `auto`, or `local`. - pub build_machine: Option, #[serde(deserialize_with = "deserialize_services")] pub services: BTreeMap, pub builds: BTreeMap, diff --git a/crates/ployz/src/handlers/build.rs b/crates/ployz/src/handlers/build.rs index 45fdb86a24..9f6636d9d2 100644 --- a/crates/ployz/src/handlers/build.rs +++ b/crates/ployz/src/handlers/build.rs @@ -52,7 +52,6 @@ pub(super) fn run(matches: &ArgMatches) -> Result<(), Error> { let location = Location::requested( leaf.get_one::("remote").map(String::as_str), leaf.get_flag("local"), - project.build_machine.as_deref(), ) .map_err(|error| Error::usage(error.to_string()))?; if matches!(location, Location::Remote(_)) && leaf.get_flag("push") && !leaf.get_flag("check") { @@ -153,7 +152,7 @@ struct Resolved { /// Required platforms this client could not confirm run natively. unconfirmed: Vec, /// Visible Machines whose Build capability could not be observed. - unanswered: Vec, + unresolved: Vec, } impl Resolved { @@ -164,7 +163,7 @@ impl Resolved { name, id, unconfirmed, - unanswered, + unresolved, } = self; println!("Selected Build Machine {name} ({id})"); eprintln!("Build Machine: {id}"); @@ -176,12 +175,12 @@ impl Resolved { unconfirmed.join(", ") ); } - if !unanswered.is_empty() { + if !unresolved.is_empty() { // A silent Machine may have been the better candidate. Say so // rather than let the choice look better evidenced than it is. eprintln!( "Build capability was not observed for {}", - unanswered.join("; ") + unresolved.join("; ") ); } } @@ -212,7 +211,7 @@ pub(super) async fn resolve_build_machine( unconfirmed: build_location::unconfirmed(&machine.runtime.architecture, required), name: machine.name, id: machine.id, - unanswered: Vec::new(), + unresolved: Vec::new(), } } Selection::Automatic => { @@ -222,7 +221,7 @@ pub(super) async fn resolve_build_machine( name: choice.machine.name.clone(), id: choice.machine.id, unconfirmed: choice.unconfirmed, - unanswered: choice.unanswered, + unresolved: choice.unresolved, } } }; @@ -236,11 +235,11 @@ fn no_machine(error: build_location::NoBuildMachine) -> Error { build_location::NoBuildMachine::Invisible => { "add one with `ployz machine add`, or build here with --local" } - build_location::NoBuildMachine::Silent { .. } => { - "check the Cluster connection and retry, or build here with --local" + build_location::NoBuildMachine::Inconclusive { .. } => { + "wait for those Machines and retry, name one with --remote=, or build here with --local" } build_location::NoBuildMachine::Incapable { .. } => { - "upgrade or restore a Machine from the evidence above, name one with --remote=, or build here with --local" + "upgrade a Machine, name one with --remote=, or build here with --local" } }; Error::usage(format!("{error}; {action}")) diff --git a/crates/ployz/src/handlers/deploy.rs b/crates/ployz/src/handlers/deploy.rs index f23d7423ba..338c625fa8 100644 --- a/crates/ployz/src/handlers/deploy.rs +++ b/crates/ployz/src/handlers/deploy.rs @@ -48,7 +48,6 @@ pub(super) fn deploy(root: &ArgMatches) -> Result<(), Error> { let location = crate::build_location::Location::requested( matches.get_one::("remote").map(String::as_str), matches.get_flag("local"), - project.build_machine.as_deref(), ) .map_err(|error| Error::usage(error.to_string()))?; let context = project diff --git a/crates/ployz/tests/build_layer3/remote.rs b/crates/ployz/tests/build_layer3/remote.rs index f83c1bdf7f..fbe9119d09 100644 --- a/crates/ployz/tests/build_layer3/remote.rs +++ b/crates/ployz/tests/build_layer3/remote.rs @@ -553,7 +553,7 @@ async fn assert_temporary_tags_released(cluster: &Cluster) { } /// Two compatible candidates: automatic selection, an explicit pin, and the -/// local override that beats a Compose preference. +/// local override. #[tokio::test] #[ignore = "informing: requires the privileged Ployz testkit image with Buildx"] async fn build_location_selects_automatically_honours_a_pin_and_yields_to_local() { @@ -671,9 +671,8 @@ async fn build_location_selects_automatically_honours_a_pin_and_yields_to_local( "selected-machine-ran\n" ); - // --local overrides a Compose preference and runs on this host's Docker, - // which the fixture stub refuses. Compose precedence itself is rung 3. - compose(&format!("x-build-machine: {pinned}\n")); + // --local runs on this host's Docker even with the Cluster reachable, which + // the fixture stub refuses. assert!(!root.join("local-docker-called").exists()); let output = run(vec!["build".into(), "--local".into(), "app".into()]) .output() diff --git a/crates/ployz/tests/remote_build_cli.rs b/crates/ployz/tests/remote_build_cli.rs index 3470a4bd0c..3e24a046e2 100644 --- a/crates/ployz/tests/remote_build_cli.rs +++ b/crates/ployz/tests/remote_build_cli.rs @@ -250,7 +250,7 @@ async fn remote_queue_outcomes_name_machine_and_unattempted_work() { fs::remove_dir_all(root).unwrap(); } -/// Automatic selection, Compose preference, and the overrides that beat it. +/// Automatic selection, an explicit pin, and the local override. #[tokio::test] async fn automatic_selection_uses_capable_machines_and_explicit_flags_beat_compose() { let root = std::env::temp_dir().join(format!("ployz-auto-cli-{}", uuid::Uuid::new_v4())); @@ -306,15 +306,10 @@ async fn automatic_selection_uses_capable_machines_and_explicit_flags_beat_compo command }; let (address, server) = support::serve_discovery(capable(builds.clone())).await; - // Automatic selection skips the Machine that does not advertise Builds, - // whether it was asked for on the command line or in Compose. - for (preference, args) in [ - ("", vec!["--remote", "api"]), - ("x-build-machine: auto\n", vec!["api"]), - // An explicit pin beats the Compose preference. - ("x-build-machine: forge\n", vec!["--remote=tower", "api"]), - ] { - compose(preference); + // Automatic selection skips the Machine that does not advertise Builds, and + // an explicit pin reaches the same Machine. + compose(""); + for args in [vec!["--remote", "api"], vec!["--remote=tower", "api"]] { let output = tokio::time::timeout(Duration::from_secs(20), run(&address, &args).output()) .await .unwrap() @@ -331,8 +326,7 @@ async fn automatic_selection_uses_capable_machines_and_explicit_flags_beat_compo "{stdout}" ); } - // An ambiguous name refuses the same way from the flag and from Compose, - // naming every Machine it matched. + // An ambiguous name refuses, naming every Machine it matched. let (ambiguous_address, ambiguous_server) = { let mut service = support::DiscoveryService::new(builds.clone()); service.machines = vec![ @@ -341,11 +335,8 @@ async fn automatic_selection_uses_capable_machines_and_explicit_flags_beat_compo ]; support::serve_discovery(service).await }; - for (preference, args) in [ - ("", vec!["--remote=tower", "api"]), - ("x-build-machine: tower\n", vec!["api"]), - ] { - compose(preference); + { + let args = vec!["--remote=tower", "api"]; let output = run(&ambiguous_address, &args).output().await.unwrap(); let stderr = String::from_utf8_lossy(&output.stderr); assert!(!output.status.success(), "{stderr}"); @@ -358,9 +349,11 @@ async fn automatic_selection_uses_capable_machines_and_explicit_flags_beat_compo } ambiguous_server.abort(); - // A Compose preference naming an incapable Machine refuses with evidence. - compose("x-build-machine: forge\n"); - let output = run(&address, &["api"]).output().await.unwrap(); + // Pinning the Machine that does not advertise Builds refuses with evidence. + let output = run(&address, &["--remote=forge", "api"]) + .output() + .await + .unwrap(); let stderr = String::from_utf8_lossy(&output.stderr); assert!(!output.status.success(), "{stderr}"); assert!( @@ -368,8 +361,7 @@ async fn automatic_selection_uses_capable_machines_and_explicit_flags_beat_compo "{stderr}" ); assert!(!root.join("docker-called").exists(), "{stderr}"); - // --local overrides the Compose preference and runs here. - compose("x-build-machine: tower\n"); + // --local runs here even with a Cluster available. assert!( !run(&address, &["--local", "api"]) .output()