diff --git a/changelog.d/2927.fixed.md b/changelog.d/2927.fixed.md new file mode 100644 index 00000000000..66f98075187 --- /dev/null +++ b/changelog.d/2927.fixed.md @@ -0,0 +1 @@ +Manually call `docker start ` if after our sidecar `run` command the container hasn't started yet and is in "created" status. diff --git a/mirrord/cli/src/container.rs b/mirrord/cli/src/container.rs index 1a3af5eb01c..1a2afa27a76 100644 --- a/mirrord/cli/src/container.rs +++ b/mirrord/cli/src/container.rs @@ -1,6 +1,5 @@ use std::{io::Write, net::SocketAddr, path::Path, process::Stdio, time::Duration}; -use exec::execvp; use local_ip_address::local_ip; use mirrord_analytics::{ AnalyticsError, AnalyticsReporter, CollectAnalytics, ExecutionKind, Reporter, @@ -22,7 +21,7 @@ use tokio::{ use tracing::Level; use crate::{ - config::{ContainerCommand, ExecParams, RuntimeArgs}, + config::{ContainerCommand, ContainerRuntime, ExecParams, RuntimeArgs}, connection::AGENT_CONNECT_INFO_ENV_KEY, container::command_builder::RuntimeCommandBuilder, error::{CliResult, ContainerError}, @@ -193,6 +192,46 @@ async fn create_sidecar_intproxy( ) })?; + // For Docker runtime sometimes the sidecar doesn't start so we double check. + // See [#2927](https://github.com/metalbear-co/mirrord/issues/2927) + if matches!(base_command.runtime(), ContainerRuntime::Docker) { + let mut container_inspect_command = Command::new(&runtime_binary); + container_inspect_command + .args(["inspect", &sidecar_container_id]) + .stdout(Stdio::piped()); + + let container_inspect_output = container_inspect_command.output().await.map_err(|err| { + ContainerError::UnsuccesfulCommandOutput( + format_command(&container_inspect_command), + err.to_string(), + ) + })?; + + let (container_inspection,) = + serde_json::from_slice::<(serde_json::Value,)>(&container_inspect_output.stdout) + .unwrap_or_default(); + + let container_status = container_inspection + .get("State") + .and_then(|inspect| inspect.get("Status")); + + if container_status + .map(|status| status == "created") + .unwrap_or(false) + { + let mut container_start_command = Command::new(&runtime_binary); + + container_start_command.args(["start", &sidecar_container_id]); + + let _ = container_start_command.status().await.map_err(|err| { + ContainerError::UnsuccesfulCommandOutput( + format_command(&container_start_command), + err.to_string(), + ) + })?; + } + } + // After spawning sidecar with -d flag it prints container_id, now we need the address of // intproxy running in sidecar to be used by mirrord-layer in execution container let intproxy_address: SocketAddr = { @@ -221,8 +260,8 @@ pub(crate) async fn container_command( runtime_args: RuntimeArgs, exec_params: ExecParams, watch: drain::Watch, -) -> CliResult<()> { - let progress = ProgressTracker::from_env("mirrord container"); +) -> CliResult { + let mut progress = ProgressTracker::from_env("mirrord container"); if runtime_args.command.has_publish() { progress.warning("mirrord container may have problems with \"-p\" directly container in command, please add to \"contanier.cli_extra_args\" in config if you are planning to publish ports"); @@ -342,13 +381,14 @@ pub(crate) async fn container_command( ); runtime_command.add_env(MIRRORD_CONFIG_FILE_ENV, "/tmp/mirrord-config.json"); - runtime_command.add_volume(composed_config_file.path(), "/tmp/mirrord-config.json"); + runtime_command + .add_volume::(composed_config_file.path(), "/tmp/mirrord-config.json"); let mut load_env_and_mount_pem = |env: &str, path: &Path| { let container_path = format!("/tmp/{}.pem", env.to_lowercase()); runtime_command.add_env(env, &container_path); - runtime_command.add_volume(path, container_path); + runtime_command.add_volume::(path, container_path); }; if let Some(path) = config.internal_proxy.client_tls_certificate.as_ref() { @@ -381,14 +421,28 @@ pub(crate) async fn container_command( sidecar_intproxy_address.to_string(), ); + progress.success(None); + let (binary, binary_args) = runtime_command .with_command(runtime_args.command) - .into_execvp_args(); + .into_command_args(); + + let runtime_command_result = Command::new(binary) + .args(binary_args) + .stdin(Stdio::inherit()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .status() + .await; - let err = execvp(binary, binary_args); - tracing::error!("Couldn't execute {:?}", err); + let _ = composed_config_file.close(); - analytics.set_error(AnalyticsError::BinaryExecuteFailed); + match runtime_command_result { + Err(err) => { + analytics.set_error(AnalyticsError::BinaryExecuteFailed); - Ok(()) + Err(ContainerError::UnableToExecuteCommand(err).into()) + } + Ok(status) => Ok(status.code().unwrap_or_default()), + } } diff --git a/mirrord/cli/src/container/command_builder.rs b/mirrord/cli/src/container/command_builder.rs index 37282650104..e4cd6d843af 100644 --- a/mirrord/cli/src/container/command_builder.rs +++ b/mirrord/cli/src/container/command_builder.rs @@ -17,6 +17,10 @@ pub struct RuntimeCommandBuilder { } impl RuntimeCommandBuilder { + pub(super) fn runtime(&self) -> &ContainerRuntime { + &self.runtime + } + fn push_arg(&mut self, value: V) where V: Into, @@ -34,7 +38,7 @@ impl RuntimeCommandBuilder { } } - pub fn add_env(&mut self, key: K, value: V) + pub(super) fn add_env(&mut self, key: K, value: V) where K: AsRef, V: AsRef, @@ -46,7 +50,7 @@ impl RuntimeCommandBuilder { self.push_arg(format!("{key}={value}")) } - pub fn add_envs(&mut self, iter: I) + pub(super) fn add_envs(&mut self, iter: I) where I: IntoIterator, K: AsRef, @@ -57,7 +61,7 @@ impl RuntimeCommandBuilder { } } - pub fn add_volume(&mut self, host_path: H, container_path: C) + pub(super) fn add_volume(&mut self, host_path: H, container_path: C) where H: AsRef, C: AsRef, @@ -65,16 +69,25 @@ impl RuntimeCommandBuilder { match self.runtime { ContainerRuntime::Podman | ContainerRuntime::Docker | ContainerRuntime::Nerdctl => { self.push_arg("-v"); - self.push_arg(format!( - "{}:{}", - host_path.as_ref().display(), - container_path.as_ref().display() - )); + + if READONLY { + self.push_arg(format!( + "{}:{}:ro", + host_path.as_ref().display(), + container_path.as_ref().display() + )); + } else { + self.push_arg(format!( + "{}:{}", + host_path.as_ref().display(), + container_path.as_ref().display() + )); + } } } } - pub fn add_volumes_from(&mut self, volumes_from: V) + pub(super) fn add_volumes_from(&mut self, volumes_from: V) where V: Into, { @@ -86,7 +99,7 @@ impl RuntimeCommandBuilder { } } - pub fn add_network(&mut self, network: N) + pub(super) fn add_network(&mut self, network: N) where N: Into, { @@ -98,7 +111,10 @@ impl RuntimeCommandBuilder { } } - pub fn with_command(self, command: ContainerCommand) -> RuntimeCommandBuilder { + pub(super) fn with_command( + self, + command: ContainerCommand, + ) -> RuntimeCommandBuilder { let RuntimeCommandBuilder { runtime, extra_args, @@ -115,7 +131,7 @@ impl RuntimeCommandBuilder { impl RuntimeCommandBuilder { /// Return completed command command with updated arguments - pub fn into_command_args(self) -> (String, impl Iterator) { + pub(super) fn into_command_args(self) -> (String, impl Iterator) { let RuntimeCommandBuilder { runtime, extra_args, @@ -133,12 +149,4 @@ impl RuntimeCommandBuilder { .chain(runtime_args), ) } - - /// Same as [`RuntimeCommandBuilder::::into_command_args`] execvp expects first arg - /// to be binary so we pass the same value as runtime as first element - pub fn into_execvp_args(self) -> (String, impl Iterator) { - let (runtime, args) = self.into_command_args(); - - (runtime.clone(), std::iter::once(runtime).chain(args)) - } } diff --git a/mirrord/cli/src/main.rs b/mirrord/cli/src/main.rs index 04048229b6e..e02b713ee64 100644 --- a/mirrord/cli/src/main.rs +++ b/mirrord/cli/src/main.rs @@ -682,7 +682,11 @@ fn main() -> miette::Result<()> { Commands::Diagnose(args) => diagnose_command(*args).await?, Commands::Container(args) => { let (runtime_args, exec_params) = args.into_parts(); - container_command(runtime_args, exec_params, watch).await? + let exit_code = container_command(runtime_args, exec_params, watch).await?; + + if exit_code != 0 { + std::process::exit(exit_code); + } } Commands::ExternalProxy { port } => external_proxy::proxy(port, watch).await?, Commands::PortForward(args) => port_forward(&args, watch).await?,