Skip to content

Commit f7b639a

Browse files
vsilentCopilot
andcommitted
fix(cli): skip docker call in dry-run mode for local deploy
In dry-run mode, artifacts (Dockerfile, docker-compose.yml) are already generated before the strategy is invoked. Calling 'docker compose config' was optional validation that fails on CI runners without the Docker Compose plugin. Since dry-run semantically means 'preview without executing', skip the docker call entirely and return success immediately after file generation. Updated unit tests accordingly. Fixes: cli_deploy integration tests on self-hosted CI runner Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent facbdaf commit f7b639a

1 file changed

Lines changed: 52 additions & 19 deletions

File tree

src/cli/install_runner.rs

Lines changed: 52 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,20 @@ pub fn strategy_for(target: &DeployTarget) -> Box<dyn DeployStrategy> {
152152
// LocalDeploy — docker compose up/down
153153
// ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
154154

155+
/// Detect which compose invocation is available on this host.
156+
///
157+
/// Returns `("docker", vec!["compose"])` when the Docker Compose plugin is
158+
/// installed (`docker compose version` exits 0), or `("docker-compose", vec![])`
159+
/// when only the standalone tool is available.
160+
fn resolve_compose_cmd(executor: &dyn CommandExecutor) -> (&'static str, Vec<&'static str>) {
161+
if let Ok(out) = executor.execute("docker", &["compose", "version"]) {
162+
if out.success() {
163+
return ("docker", vec!["compose"]);
164+
}
165+
}
166+
("docker-compose", vec![])
167+
}
168+
155169
pub struct LocalDeploy;
156170

157171
impl DeployStrategy for LocalDeploy {
@@ -167,9 +181,25 @@ impl DeployStrategy for LocalDeploy {
167181
context: &DeployContext,
168182
executor: &dyn CommandExecutor,
169183
) -> Result<DeployResult, CliError> {
184+
// In dry-run mode, artifacts have already been generated.
185+
// Skip calling docker compose — it may not be available in all environments,
186+
// and "dry run" means "preview, don't execute".
187+
if context.dry_run {
188+
return Ok(DeployResult {
189+
target: DeployTarget::Local,
190+
message: "Local deployment previewed successfully (dry-run)".to_string(),
191+
server_ip: None,
192+
deployment_id: None,
193+
project_id: None,
194+
server_name: None,
195+
});
196+
}
197+
170198
let compose_path = context.compose_path.to_string_lossy().to_string();
171199

172-
let mut args: Vec<String> = vec!["compose".into()];
200+
let (cmd, base_args) = resolve_compose_cmd(executor);
201+
let mut args: Vec<String> = base_args.iter().map(|s| s.to_string()).collect();
202+
173203
if let Some(ref env_file) = config.env_file {
174204
let env_file_path = if env_file.is_absolute() {
175205
env_file.clone()
@@ -181,17 +211,12 @@ impl DeployStrategy for LocalDeploy {
181211
}
182212
args.push("--file".into());
183213
args.push(compose_path.clone());
184-
185-
if context.dry_run {
186-
args.push("config".into());
187-
} else {
188-
args.push("up".into());
189-
args.push("-d".into());
190-
args.push("--build".into());
191-
}
214+
args.push("up".into());
215+
args.push("-d".into());
216+
args.push("--build".into());
192217

193218
let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect();
194-
let output = executor.execute("docker", &args_refs)?;
219+
let output = executor.execute(cmd, &args_refs)?;
195220

196221
if !output.stdout.trim().is_empty() {
197222
println!("{}", output.stdout);
@@ -207,10 +232,9 @@ impl DeployStrategy for LocalDeploy {
207232
});
208233
}
209234

210-
let action = if context.dry_run { "validated" } else { "started" };
211235
Ok(DeployResult {
212236
target: DeployTarget::Local,
213-
message: format!("Local deployment {} successfully", action),
237+
message: "Local deployment started successfully".to_string(),
214238
server_ip: None,
215239
deployment_id: None,
216240
project_id: None,
@@ -225,7 +249,10 @@ impl DeployStrategy for LocalDeploy {
225249
executor: &dyn CommandExecutor,
226250
) -> Result<(), CliError> {
227251
let compose_path = context.compose_path.to_string_lossy().to_string();
228-
let mut args: Vec<String> = vec!["compose".into()];
252+
253+
let (cmd, base_args) = resolve_compose_cmd(executor);
254+
let mut args: Vec<String> = base_args.iter().map(|s| s.to_string()).collect();
255+
229256
if let Some(ref env_file) = config.env_file {
230257
let env_file_path = if env_file.is_absolute() {
231258
env_file.clone()
@@ -240,7 +267,7 @@ impl DeployStrategy for LocalDeploy {
240267
args.push("down".into());
241268
args.push("--volumes".into());
242269
let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect();
243-
let output = executor.execute("docker", &args_refs)?;
270+
let output = executor.execute(cmd, &args_refs)?;
244271

245272
if !output.success() {
246273
return Err(CliError::DeployFailed {
@@ -1669,11 +1696,17 @@ mod tests {
16691696

16701697
let result = strategy.deploy(&config, &context, &executor).unwrap();
16711698
assert_eq!(result.target, DeployTarget::Local);
1672-
assert!(result.message.contains("validated"));
1699+
assert!(result.message.contains("dry-run") || result.message.contains("previewed"),
1700+
"dry-run message should indicate preview, got: {}", result.message);
16731701

1674-
let args = executor.last_args();
1675-
assert!(args.contains(&"config".to_string()));
1676-
assert!(!args.contains(&"up".to_string()));
1702+
// Dry-run should NOT invoke docker at all (no compose call)
1703+
let recorded = executor.recorded_calls.lock().unwrap();
1704+
// Only the compose-version probe may have been called (from resolve_compose_cmd),
1705+
// but the actual compose up/config should NOT be called.
1706+
assert!(!recorded.iter().any(|(_, args)| args.contains(&"up".to_string())),
1707+
"dry-run must not call docker compose up");
1708+
assert!(!recorded.iter().any(|(_, args)| args.contains(&"config".to_string())),
1709+
"dry-run must not call docker compose config");
16771710
}
16781711

16791712
#[test]
@@ -1725,7 +1758,7 @@ mod tests {
17251758
.env_file(".env")
17261759
.build()
17271760
.unwrap();
1728-
let context = sample_context(true);
1761+
let context = sample_context(false); // real deploy, not dry-run
17291762
let executor = MockExecutor::success();
17301763
let strategy = LocalDeploy;
17311764

0 commit comments

Comments
 (0)