diff --git a/crates/clickhousectl/src/local/docker.rs b/crates/clickhousectl/src/local/docker.rs index 4265642..80c78b8 100644 --- a/crates/clickhousectl/src/local/docker.rs +++ b/crates/clickhousectl/src/local/docker.rs @@ -17,6 +17,7 @@ use crate::error::{Error, Result}; use bollard::Docker; use futures_util::StreamExt; use std::collections::HashMap; +use std::io::{self, IsTerminal, Write}; pub const LABEL_ENGINE: &str = "clickhousectl.engine"; pub const LABEL_NAME: &str = "clickhousectl.name"; @@ -48,20 +49,123 @@ pub async fn connect() -> Result { Ok(docker) } -/// Pull `postgres:`, streaming progress to stderr. -pub async fn pull_image(docker: &Docker, tag: &str) -> Result<()> { +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum PullProgressMode { + Interactive, + Collapsed, +} + +fn pull_progress_mode( + stdout_is_terminal: bool, + stderr_is_terminal: bool, + structured_output: bool, +) -> PullProgressMode { + if stdout_is_terminal && stderr_is_terminal && !structured_output { + PullProgressMode::Interactive + } else { + PullProgressMode::Collapsed + } +} + +struct PullReporter { + image: String, + mode: PullProgressMode, +} + +impl PullReporter { + fn new(image: String, mode: PullProgressMode) -> Self { + Self { image, mode } + } + + fn start(&self, output: &mut impl Write) { + match self.mode { + PullProgressMode::Interactive => { + let _ = writeln!(output, "Pulling {}...", self.image); + } + PullProgressMode::Collapsed => { + let _ = write!(output, "Pulling {}...", self.image); + let _ = output.flush(); + } + } + } + + fn event(&self, info: &bollard::models::CreateImageInfo, output: &mut impl Write) { + if self.mode != PullProgressMode::Interactive { + return; + } + + let Some(status) = info.status.as_deref() else { + return; + }; + let _ = write!(output, " "); + if let Some(id) = info.id.as_deref() { + let _ = write!(output, "{id}: "); + } + let _ = write!(output, "{status}"); + if let Some((current, total)) = info.progress_detail.as_ref().and_then(|progress| { + progress + .current + .zip(progress.total) + .filter(|(_, total)| *total > 0) + }) { + let percent = current.saturating_mul(100) / total; + let _ = write!(output, " ({current}/{total} bytes, {percent}%)"); + } + let _ = writeln!(output); + } + + fn finish(&self, output: &mut impl Write) { + match self.mode { + PullProgressMode::Interactive => { + let _ = writeln!(output, "Pulled {}", self.image); + } + PullProgressMode::Collapsed => { + let _ = writeln!(output, " done"); + } + } + } + + fn fail(&self, output: &mut impl Write) { + match self.mode { + PullProgressMode::Interactive => { + let _ = writeln!(output, "Failed to pull {}", self.image); + } + PullProgressMode::Collapsed => { + let _ = writeln!(output, " failed"); + } + } + } +} + +/// Pull `postgres:`, keeping full progress for interactive terminals and +/// collapsing it to one bounded summary line for redirected or structured output. +pub async fn pull_image(docker: &Docker, tag: &str, structured_output: bool) -> Result<()> { use bollard::query_parameters::CreateImageOptionsBuilder; let from = format!("postgres:{}", tag); + let mode = pull_progress_mode( + io::stdout().is_terminal(), + io::stderr().is_terminal(), + structured_output, + ); + let reporter = PullReporter::new(from.clone(), mode); + let stderr = io::stderr(); + reporter.start(&mut stderr.lock()); + let opts = CreateImageOptionsBuilder::default() .from_image(&from) .build(); let mut stream = docker.create_image(Some(opts), None, None); while let Some(item) = stream.next().await { - let info = item.map_err(|e| Error::DockerError(e.to_string()))?; - if let Some(status) = info.status.as_deref() { - eprintln!(" {}", status); - } + let info = match item { + Ok(info) => info, + Err(error) => { + reporter.fail(&mut stderr.lock()); + return Err(Error::DockerError(error.to_string())); + } + }; + reporter.event(&info, &mut stderr.lock()); } + reporter.finish(&mut stderr.lock()); Ok(()) } @@ -721,3 +825,96 @@ pub fn recover_project_postgres_blocking(project_cwd: &str) { Ok(()) }); } + +#[cfg(test)] +mod tests { + use super::*; + use bollard::models::{CreateImageInfo, ProgressDetail}; + + #[test] + fn pull_progress_is_interactive_only_for_human_ttys() { + assert_eq!( + pull_progress_mode(true, true, false), + PullProgressMode::Interactive + ); + assert_eq!( + pull_progress_mode(false, true, false), + PullProgressMode::Collapsed + ); + assert_eq!( + pull_progress_mode(true, false, false), + PullProgressMode::Collapsed + ); + assert_eq!( + pull_progress_mode(true, true, true), + PullProgressMode::Collapsed + ); + } + + #[test] + fn collapsed_pull_progress_is_one_bounded_summary_line() { + let mut output = Vec::new(); + let reporter = PullReporter::new("postgres:18".to_string(), PullProgressMode::Collapsed); + reporter.start(&mut output); + for (id, status) in [ + ("layer-a", "Pulling fs layer"), + ("layer-a", "Downloading"), + ("layer-b", "Pulling fs layer"), + ("layer-b", "Pull complete"), + ] { + reporter.event( + &CreateImageInfo { + id: Some(id.to_string()), + status: Some(status.to_string()), + ..Default::default() + }, + &mut output, + ); + } + reporter.finish(&mut output); + + assert_eq!( + String::from_utf8(output).unwrap(), + "Pulling postgres:18... done\n" + ); + } + + #[test] + fn interactive_pull_progress_keeps_layer_and_byte_context() { + let mut output = Vec::new(); + let reporter = PullReporter::new("postgres:18".to_string(), PullProgressMode::Interactive); + reporter.start(&mut output); + reporter.event( + &CreateImageInfo { + id: Some("layer-a".to_string()), + status: Some("Downloading".to_string()), + progress_detail: Some(ProgressDetail { + current: Some(50), + total: Some(100), + }), + ..Default::default() + }, + &mut output, + ); + reporter.finish(&mut output); + + assert_eq!( + String::from_utf8(output).unwrap(), + "Pulling postgres:18...\n layer-a: Downloading (50/100 bytes, 50%)\nPulled postgres:18\n" + ); + } + + #[test] + fn collapsed_pull_failure_closes_the_summary_line() { + let mut output = Vec::new(); + let reporter = + PullReporter::new("postgres:missing".to_string(), PullProgressMode::Collapsed); + reporter.start(&mut output); + reporter.fail(&mut output); + + assert_eq!( + String::from_utf8(output).unwrap(), + "Pulling postgres:missing... failed\n" + ); + } +} diff --git a/crates/clickhousectl/src/local/mod.rs b/crates/clickhousectl/src/local/mod.rs index f6156ca..e4b93c4 100644 --- a/crates/clickhousectl/src/local/mod.rs +++ b/crates/clickhousectl/src/local/mod.rs @@ -72,8 +72,7 @@ async fn install_postgres(tag: &str, force: bool, json: bool) -> Result<()> { return Ok(()); } - eprintln!("Pulling postgres:{tag}..."); - docker::pull_image(&docker, tag).await?; + docker::pull_image(&docker, tag, json).await?; let out = output::InstallOutput { version: format!("postgres@{tag}"), diff --git a/crates/clickhousectl/src/local/postgres.rs b/crates/clickhousectl/src/local/postgres.rs index fd1796c..3c1d9fb 100644 --- a/crates/clickhousectl/src/local/postgres.rs +++ b/crates/clickhousectl/src/local/postgres.rs @@ -180,8 +180,7 @@ async fn start( // Fresh create. if !docker::image_exists(&docker, tag).await? { - eprintln!("Pulling postgres:{tag}..."); - docker::pull_image(&docker, tag).await?; + docker::pull_image(&docker, tag, json).await?; } let host_port = resolve_port(port)?; diff --git a/crates/clickhousectl/tests/local_docker_pull_progress_test.rs b/crates/clickhousectl/tests/local_docker_pull_progress_test.rs new file mode 100644 index 0000000..ccdc123 --- /dev/null +++ b/crates/clickhousectl/tests/local_docker_pull_progress_test.rs @@ -0,0 +1,158 @@ +//! Subprocess coverage for non-TTY Docker pull reporting. + +use std::io::{ErrorKind, Read, Write}; +use std::os::unix::net::{UnixListener, UnixStream}; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output, Stdio}; +use std::thread::{self, JoinHandle}; +use std::time::{Duration, Instant}; + +fn clickhousectl_binary() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_clickhousectl")) +} + +fn write_response(stream: &mut UnixStream, content_type: &str, body: &str) { + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: {content_type}\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + stream + .write_all(response.as_bytes()) + .expect("write fake Docker response"); +} + +fn read_request(stream: &mut UnixStream) -> String { + let mut request = Vec::new(); + let mut buffer = [0_u8; 1024]; + while !request.windows(4).any(|window| window == b"\r\n\r\n") { + let bytes = stream.read(&mut buffer).expect("read Docker request"); + assert!(bytes > 0, "Docker request ended before its headers"); + request.extend_from_slice(&buffer[..bytes]); + } + String::from_utf8(request).expect("Docker request is UTF-8") +} + +fn accept_connection(listener: &UnixListener, operation: &str) -> UnixStream { + let deadline = Instant::now() + Duration::from_secs(5); + loop { + match listener.accept() { + Ok((stream, _)) => { + stream + .set_nonblocking(false) + .expect("make Docker connection blocking"); + return stream; + } + Err(error) if error.kind() == ErrorKind::WouldBlock && Instant::now() < deadline => { + thread::sleep(Duration::from_millis(10)); + } + Err(error) => panic!("accept Docker {operation}: {error}"), + } + } +} + +fn spawn_fake_docker(socket_path: &Path, pull_response: String) -> JoinHandle<()> { + let listener = UnixListener::bind(socket_path).expect("bind fake Docker socket"); + listener + .set_nonblocking(true) + .expect("make fake Docker socket nonblocking"); + thread::spawn(move || { + let mut ping = accept_connection(&listener, "ping"); + let request = read_request(&mut ping); + assert!( + request.starts_with("GET /_ping "), + "unexpected request: {request}" + ); + write_response(&mut ping, "text/plain", "OK"); + + let mut pull = accept_connection(&listener, "pull"); + let request = read_request(&mut pull); + assert!( + request.starts_with("POST /images/create?") + && request.contains("fromImage=postgres%3A"), + "unexpected request: {request}" + ); + write_response(&mut pull, "application/json", &pull_response); + }) +} + +fn run_install(tag: &str, pull_response: &str) -> Output { + let tempdir = tempfile::tempdir().expect("create tempdir"); + let socket_path = tempdir.path().join("docker.sock"); + let daemon = spawn_fake_docker(&socket_path, pull_response.to_string()); + + let mut command = Command::new(clickhousectl_binary()); + command + .env_clear() + .env("DO_NOT_TRACK", "1") + .env("HOME", tempdir.path()) + .env("DOCKER_HOST", format!("unix://{}", socket_path.display())) + .args(["local", "install", &format!("postgres@{tag}"), "--force"]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let mut child = command.spawn().expect("run clickhousectl"); + for _ in 0..100 { + if child.try_wait().expect("poll clickhousectl").is_some() { + let output = child + .wait_with_output() + .expect("collect clickhousectl output"); + daemon.join().expect("fake Docker daemon"); + return output; + } + thread::sleep(Duration::from_millis(50)); + } + child.kill().expect("kill timed out clickhousectl"); + let output = child.wait_with_output().expect("collect timed out output"); + panic!( + "clickhousectl timed out\nstderr: {}\nstdout: {}", + String::from_utf8_lossy(&output.stderr), + String::from_utf8_lossy(&output.stdout) + ); +} + +#[test] +fn non_tty_pull_collapses_repeated_layer_events() { + let output = run_install( + "18", + concat!( + "{\"status\":\"Pulling fs layer\",\"id\":\"layer-a\"}\n", + "{\"status\":\"Downloading\",\"id\":\"layer-a\",\"progressDetail\":{\"current\":1,\"total\":10}}\n", + "{\"status\":\"Pulling fs layer\",\"id\":\"layer-b\"}\n", + "{\"status\":\"Extracting\",\"id\":\"layer-b\",\"progressDetail\":{\"current\":5,\"total\":10}}\n", + "{\"status\":\"Pull complete\",\"id\":\"layer-a\"}\n", + "{\"status\":\"Pull complete\",\"id\":\"layer-b\"}\n" + ), + ); + + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + String::from_utf8(output.stderr).unwrap(), + "Pulling postgres:18... done\n" + ); +} + +#[test] +fn non_tty_pull_reports_failure_once_and_preserves_diagnostics() { + let output = run_install( + "18-missing", + "{\"errorDetail\":{\"message\":\"manifest for postgres:18-missing not found\"}}\n", + ); + + let stderr = String::from_utf8(output.stderr).unwrap(); + assert_eq!(output.status.code(), Some(1), "stderr: {stderr}"); + assert!( + stderr.starts_with("Pulling postgres:18-missing... failed\n"), + "unexpected stderr: {stderr}" + ); + assert!( + stderr.contains("Docker stream error: manifest for postgres:18-missing not found"), + "missing Docker diagnostics: {stderr}" + ); + assert!( + !stderr.contains("Pulling fs layer"), + "progress leaked: {stderr}" + ); +}