Skip to content

Commit

Permalink
merge main
Browse files Browse the repository at this point in the history
  • Loading branch information
meowjesty committed Nov 18, 2024
2 parents 8c6b083 + 06b81a4 commit c8a8395
Show file tree
Hide file tree
Showing 20 changed files with 115 additions and 89 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Changes the Result alias to CliResult, and config to layer_config (in some places).
1 change: 1 addition & 0 deletions changelog.d/+mirrord-container-publish.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Print a warning to the user when `-p` is provided as part of `mirrord container` run command that it may cause an issue because of our usage of container type network mode.
12 changes: 12 additions & 0 deletions mirrord/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,18 @@ impl ContainerCommand {
runtime_args: runtime_args.into_iter().map(T::into).collect(),
}
}

pub fn has_publish(&self) -> bool {
let ContainerCommand::Run { runtime_args } = self;

let mut hit_trailing_token = false;

runtime_args.iter().any(|runtime_arg| {
hit_trailing_token = hit_trailing_token || runtime_arg == "--";

!hit_trailing_token && matches!(runtime_arg.as_str(), "-p" | "--publish")
})
}
}

#[derive(Args, Debug)]
Expand Down
10 changes: 5 additions & 5 deletions mirrord/cli/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use mirrord_protocol::{ClientMessage, DaemonMessage};
use tokio::sync::mpsc;
use tracing::Level;

use crate::{CliError, Result};
use crate::{CliError, CliResult};

pub const AGENT_CONNECT_INFO_ENV_KEY: &str = "MIRRORD_AGENT_CONNECT_INFO";

Expand All @@ -35,7 +35,7 @@ async fn try_connect_using_operator<P, R>(
config: &LayerConfig,
progress: &P,
analytics: &mut R,
) -> Result<Option<OperatorSessionConnection>>
) -> CliResult<Option<OperatorSessionConnection>>
where
P: Progress,
R: Reporter,
Expand Down Expand Up @@ -114,7 +114,7 @@ pub(crate) async fn create_and_connect<P, R: Reporter>(
config: &LayerConfig,
progress: &mut P,
analytics: &mut R,
) -> Result<(AgentConnectInfo, AgentConnection)>
) -> CliResult<(AgentConnectInfo, AgentConnection)>
where
P: Progress + Send + Sync,
{
Expand Down Expand Up @@ -208,7 +208,7 @@ fn user_persistent_random_message_select() -> bool {
== 0
}

pub(crate) fn show_multipod_warning<P>(progress: &mut P) -> Result<(), CliError>
pub(crate) fn show_multipod_warning<P>(progress: &mut P) -> CliResult<(), CliError>
where
P: Progress + Send + Sync,
{
Expand Down Expand Up @@ -238,7 +238,7 @@ where
Ok(())
}

pub(crate) fn show_http_filter_warning<P>(progress: &mut P) -> Result<(), CliError>
pub(crate) fn show_http_filter_warning<P>(progress: &mut P) -> CliResult<(), CliError>
where
P: Progress + Send + Sync,
{
Expand Down
18 changes: 12 additions & 6 deletions mirrord/cli/src/container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::{
config::{ContainerCommand, ExecParams, RuntimeArgs},
connection::AGENT_CONNECT_INFO_ENV_KEY,
container::command_builder::RuntimeCommandBuilder,
error::{ContainerError, Result},
error::{CliResult, ContainerError},
execution::{
MirrordExecution, LINUX_INJECTION_ENV_VAR, MIRRORD_CONNECT_TCP_ENV,
MIRRORD_EXECUTION_KIND_ENV,
Expand All @@ -51,7 +51,9 @@ fn format_command(command: &Command) -> String {

/// Execute a [`Command`] and read first line from stdout
#[tracing::instrument(level = Level::TRACE, ret)]
async fn exec_and_get_first_line(command: &mut Command) -> Result<Option<String>, ContainerError> {
async fn exec_and_get_first_line(
command: &mut Command,
) -> CliResult<Option<String>, ContainerError> {
let mut child = command
.stdin(Stdio::null())
.stdout(Stdio::piped())
Expand Down Expand Up @@ -110,7 +112,7 @@ async fn exec_and_get_first_line(command: &mut Command) -> Result<Option<String>
/// Create a temp file with a json serialized [`LayerConfig`] to be loaded by container and external
/// proxy
#[tracing::instrument(level = Level::TRACE, ret)]
fn create_composed_config(config: &LayerConfig) -> Result<NamedTempFile, ContainerError> {
fn create_composed_config(config: &LayerConfig) -> CliResult<NamedTempFile, ContainerError> {
let mut composed_config_file = tempfile::Builder::new()
.suffix(".json")
.tempfile()
Expand All @@ -126,7 +128,7 @@ fn create_composed_config(config: &LayerConfig) -> Result<NamedTempFile, Contain
#[tracing::instrument(level = Level::TRACE, ret)]
fn create_self_signed_certificate(
subject_alt_names: Vec<String>,
) -> Result<(NamedTempFile, NamedTempFile), ContainerError> {
) -> CliResult<(NamedTempFile, NamedTempFile), ContainerError> {
let geerated = rcgen::generate_simple_self_signed(subject_alt_names)
.map_err(ContainerError::SelfSignedCertificate)?;

Expand All @@ -153,7 +155,7 @@ async fn create_sidecar_intproxy(
config: &LayerConfig,
base_command: &RuntimeCommandBuilder,
connection_info: Vec<(&str, &str)>,
) -> Result<(String, SocketAddr), ContainerError> {
) -> CliResult<(String, SocketAddr), ContainerError> {
let mut sidecar_command = base_command.clone();

sidecar_command.add_env(MIRRORD_INTPROXY_CONTAINER_MODE_ENV, "true");
Expand Down Expand Up @@ -219,9 +221,13 @@ pub(crate) async fn container_command(
runtime_args: RuntimeArgs,
exec_params: ExecParams,
watch: drain::Watch,
) -> Result<()> {
) -> CliResult<()> {
let 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");
}

progress.warning("mirrord container is currently an unstable feature");

for (name, value) in exec_params.as_env_vars()? {
Expand Down
10 changes: 5 additions & 5 deletions mirrord/cli/src/diagnose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ use tokio::{sync::mpsc, time::Instant};
use tracing::Level;

use crate::{
connection::create_and_connect, util::remove_proxy_env, CliError, DiagnoseArgs,
DiagnoseCommand, Result,
connection::create_and_connect, util::remove_proxy_env, CliError, CliResult, DiagnoseArgs,
DiagnoseCommand,
};

/// Sends a ping the connection and expects a pong.
async fn ping(
sender: &mpsc::Sender<ClientMessage>,
receiver: &mut mpsc::Receiver<DaemonMessage>,
) -> Result<()> {
) -> CliResult<()> {
sender.send(ClientMessage::Ping).await.map_err(|_| {
CliError::PingPongFailed(
"failed to send ping message - agent unexpectedly closed connection".to_string(),
Expand Down Expand Up @@ -47,7 +47,7 @@ async fn ping(

/// Create a targetless session and run pings to diagnose network latency.
#[tracing::instrument(level = Level::TRACE, ret)]
async fn diagnose_latency(config: Option<&Path>) -> Result<()> {
async fn diagnose_latency(config: Option<&Path>) -> CliResult<()> {
let mut progress = ProgressTracker::from_env("mirrord network diagnosis");

let mut cfg_context = ConfigContext::default();
Expand Down Expand Up @@ -98,7 +98,7 @@ async fn diagnose_latency(config: Option<&Path>) -> Result<()> {
}

/// Handle commands related to the operator `mirrord diagnose ...`
pub(crate) async fn diagnose_command(args: DiagnoseArgs) -> Result<()> {
pub(crate) async fn diagnose_command(args: DiagnoseArgs) -> CliResult<()> {
match args.command {
DiagnoseCommand::Latency { config_file } => diagnose_latency(config_file.as_deref()).await,
}
Expand Down
2 changes: 1 addition & 1 deletion mirrord/cli/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use thiserror::Error;

use crate::port_forward::PortForwardError;

pub(crate) type Result<T, E = CliError> = core::result::Result<T, E>;
pub(crate) type CliResult<T, E = CliError> = core::result::Result<T, E>;

const GENERAL_HELP: &str = r#"
Expand Down
14 changes: 7 additions & 7 deletions mirrord/cli/src/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::{
error::CliError,
extract::extract_library,
util::remove_proxy_env,
Result,
CliResult,
};

/// Env variable mirrord-layer uses to connect to intproxy
Expand Down Expand Up @@ -171,7 +171,7 @@ impl MirrordExecution {
#[cfg(target_os = "macos")] executable: Option<&str>,
progress: &mut P,
analytics: &mut AnalyticsReporter,
) -> Result<Self>
) -> CliResult<Self>
where
P: Progress + Send + Sync,
{
Expand Down Expand Up @@ -325,7 +325,7 @@ impl MirrordExecution {
})
}

async fn get_agent_version(connection: &mut AgentConnection) -> Result<Version> {
async fn get_agent_version(connection: &mut AgentConnection) -> CliResult<Version> {
let Ok(_) = connection
.sender
.send(ClientMessage::SwitchProtocolVersion(
Expand Down Expand Up @@ -354,7 +354,7 @@ impl MirrordExecution {
config: &LayerConfig,
progress: &mut P,
analytics: &mut AnalyticsReporter,
) -> Result<Self>
) -> CliResult<Self>
where
P: Progress + Send + Sync,
{
Expand Down Expand Up @@ -447,7 +447,7 @@ impl MirrordExecution {
async fn fetch_env_vars(
config: &LayerConfig,
connection: &mut AgentConnection,
) -> Result<HashMap<String, String>> {
) -> CliResult<HashMap<String, String>> {
let mut env_vars = HashMap::new();

let (env_vars_exclude, env_vars_include) = match (
Expand Down Expand Up @@ -500,7 +500,7 @@ impl MirrordExecution {
connection: &mut AgentConnection,
env_vars_filter: HashSet<String>,
env_vars_select: HashSet<String>,
) -> Result<HashMap<String, String>> {
) -> CliResult<HashMap<String, String>> {
connection
.sender
.send(ClientMessage::GetEnvVarsRequest(GetEnvVarsRequest {
Expand Down Expand Up @@ -552,7 +552,7 @@ impl MirrordExecution {
/// cleans up the process when the parent process exits, so we need the parent to stay alive
/// while the internal proxy is running.
/// See <https://github.com/metalbear-co/mirrord/issues/1211>
pub(crate) async fn wait(mut self) -> Result<()> {
pub(crate) async fn wait(mut self) -> CliResult<()> {
self.child
.wait()
.await
Expand Down
6 changes: 3 additions & 3 deletions mirrord/cli/src/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use mirrord_analytics::{AnalyticsError, AnalyticsReporter, Reporter};
use mirrord_config::{LayerConfig, MIRRORD_CONFIG_FILE_ENV};
use mirrord_progress::{JsonProgress, Progress, ProgressTracker};

use crate::{config::ExtensionExecArgs, error::CliError, execution::MirrordExecution, Result};
use crate::{config::ExtensionExecArgs, error::CliError, execution::MirrordExecution, CliResult};

/// Actually facilitate execution after all preparations were complete
async fn mirrord_exec<P>(
Expand All @@ -13,7 +13,7 @@ async fn mirrord_exec<P>(
config: LayerConfig,
mut progress: P,
analytics: &mut AnalyticsReporter,
) -> Result<()>
) -> CliResult<()>
where
P: Progress + Send + Sync,
{
Expand All @@ -37,7 +37,7 @@ where
}

/// Facilitate the execution of a process using mirrord by an IDE extension
pub(crate) async fn extension_exec(args: ExtensionExecArgs, watch: drain::Watch) -> Result<()> {
pub(crate) async fn extension_exec(args: ExtensionExecArgs, watch: drain::Watch) -> CliResult<()> {
let progress = ProgressTracker::try_from_env("mirrord preparing to launch")
.unwrap_or_else(|| JsonProgress::new("mirrord preparing to launch").into());
let mut env: HashMap<String, String> = HashMap::new();
Expand Down
10 changes: 5 additions & 5 deletions mirrord/cli/src/external_proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use tracing_subscriber::EnvFilter;

use crate::{
connection::AGENT_CONNECT_INFO_ENV_KEY,
error::{ExternalProxyError, Result},
error::{CliResult, ExternalProxyError},
execution::MIRRORD_EXECUTION_KIND_ENV,
internal_proxy::connect_and_ping,
util::{create_listen_socket, detach_io},
Expand All @@ -59,7 +59,7 @@ fn print_addr(listener: &TcpListener) -> io::Result<()> {
Ok(())
}

pub async fn proxy(listen_port: u16, watch: drain::Watch) -> Result<()> {
pub async fn proxy(listen_port: u16, watch: drain::Watch) -> CliResult<()> {
let config = LayerConfig::from_env()?;

tracing::info!(?config, "external_proxy starting");
Expand Down Expand Up @@ -185,7 +185,7 @@ pub async fn proxy(listen_port: u16, watch: drain::Watch) -> Result<()> {

async fn create_external_proxy_tls_acceptor(
config: &LayerConfig,
) -> Result<Option<tokio_rustls::TlsAcceptor>, ExternalProxyError> {
) -> CliResult<Option<tokio_rustls::TlsAcceptor>, ExternalProxyError> {
if !config.external_proxy.tls_enable {
return Ok(None);
}
Expand All @@ -205,7 +205,7 @@ async fn create_external_proxy_tls_acceptor(
})
.map_err(ExternalProxyError::Tls)?,
))
.collect::<Result<Vec<_>, _>>()
.collect::<CliResult<Vec<_>, _>>()
.map_err(|error| ConnectionTlsError::ParsingPem(client_tls_certificate.to_path_buf(), error))
.map_err(ExternalProxyError::Tls)?;

Expand All @@ -214,7 +214,7 @@ async fn create_external_proxy_tls_acceptor(
.map_err(|error| ConnectionTlsError::MissingPem(tls_certificate.to_path_buf(), error))
.map_err(ExternalProxyError::Tls)?,
))
.collect::<Result<Vec<_>, _>>()
.collect::<CliResult<Vec<_>, _>>()
.map_err(|error| ConnectionTlsError::ParsingPem(tls_certificate.to_path_buf(), error))
.map_err(ExternalProxyError::Tls)?;

Expand Down
6 changes: 3 additions & 3 deletions mirrord/cli/src/extract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use const_random::const_random;
use mirrord_progress::Progress;
use tracing::debug;

use crate::{error::CliError, Result};
use crate::{error::CliError, CliResult};

/// For some reason loading dylib from $TMPDIR can get the process killed somehow..?
#[cfg(target_os = "macos")]
Expand All @@ -35,7 +35,7 @@ pub(crate) fn extract_library<P>(
dest_dir: Option<String>,
progress: &P,
prefix: bool,
) -> Result<PathBuf>
) -> CliResult<PathBuf>
where
P: Progress + Send + Sync,
{
Expand Down Expand Up @@ -72,7 +72,7 @@ where
/// If prefix is true, add a random prefix to the file name that identifies the specific build
/// of the layer. This is useful for debug purposes usually.
#[cfg(all(target_os = "macos", target_arch = "aarch64"))]
pub(crate) fn extract_arm64<P>(progress: &P, prefix: bool) -> Result<PathBuf>
pub(crate) fn extract_arm64<P>(progress: &P, prefix: bool) -> CliResult<PathBuf>
where
P: Progress + Send + Sync,
{
Expand Down
9 changes: 6 additions & 3 deletions mirrord/cli/src/internal_proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use tracing_subscriber::EnvFilter;

use crate::{
connection::AGENT_CONNECT_INFO_ENV_KEY,
error::{InternalProxyError, Result},
error::{CliResult, InternalProxyError},
execution::MIRRORD_EXECUTION_KIND_ENV,
util::{create_listen_socket, detach_io},
};
Expand All @@ -50,7 +50,10 @@ fn print_addr(listener: &TcpListener) -> io::Result<()> {

/// Main entry point for the internal proxy.
/// It listens for inbound layer connect and forwards to agent.
pub(crate) async fn proxy(listen_port: u16, watch: drain::Watch) -> Result<(), InternalProxyError> {
pub(crate) async fn proxy(
listen_port: u16,
watch: drain::Watch,
) -> CliResult<(), InternalProxyError> {
let config = LayerConfig::from_env()?;

tracing::info!(?config, "internal_proxy starting");
Expand Down Expand Up @@ -153,7 +156,7 @@ pub(crate) async fn connect_and_ping(
config: &LayerConfig,
connect_info: Option<AgentConnectInfo>,
analytics: &mut AnalyticsReporter,
) -> Result<AgentConnection, InternalProxyError> {
) -> CliResult<AgentConnection, InternalProxyError> {
let mut agent_conn = AgentConnection::new(config, connect_info, analytics)
.await
.map_err(IntProxyError::from)?;
Expand Down
Loading

0 comments on commit c8a8395

Please sign in to comment.