Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning when mirrord exec docker is detected, hinting the user towards mirrord container. #2663

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions changelog.d/2599.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add warning when user tries to mirrord exec [container], pointing them to use mirrord container instead.
1 change: 1 addition & 0 deletions mirrord/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ rustls-pemfile = "2"
tokio-rustls = "0.26"
tokio-stream = { workspace = true, features = ["net"] }
tokio-retry = "0.3"
regex.workspace = true


[target.'cfg(target_os = "macos")'.dependencies]
Expand Down
9 changes: 8 additions & 1 deletion mirrord/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ use mirrord_config::{
};
use mirrord_kube::api::{container::SKIP_NAMES, kubernetes::create_kube_config};
use mirrord_operator::client::OperatorApi;
use mirrord_progress::{Progress, ProgressTracker};
use mirrord_progress::{messages::EXEC_CONTAINER_BINARY, Progress, ProgressTracker};
use operator::operator_command;
use port_forward::PortForwarder;
use regex::Regex;
use semver::Version;
use serde_json::json;
use tracing::{error, info, warn};
Expand Down Expand Up @@ -321,6 +322,12 @@ async fn exec(args: &ExecArgs, watch: drain::Watch) -> Result<()> {
args.binary, args.binary_args
);

let container_detection =
Regex::new("docker|podman|nerdctl").expect("Failed building container detection regex!");
if container_detection.is_match(&args.binary) {
progress.warning(EXEC_CONTAINER_BINARY);
}

if !(args.params.no_tcp_outgoing || args.params.no_udp_outgoing) && args.params.no_remote_dns {
warn!("TCP/UDP outgoing enabled without remote DNS might cause issues when local machine has IPv6 enabled but remote cluster doesn't")
}
Expand Down
6 changes: 6 additions & 0 deletions mirrord/progress/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,9 @@ pub const MULTIPOD_WARNING: (&str, &str) = (
You can get started with mirrord for Teams at this link: \
https://mirrord.dev/docs/overview/teams/",
);

/// Warning when user tries to run `mirrord exec docker` (for example), instead of the correct
/// `mirrord container ...`.
pub const EXEC_CONTAINER_BINARY: &str = "`mirrord exec <docker|podman|nerdctl> ...` detected! \
If you try to run a container with mirrord, please use \
`mirrord container [options] -- <docker|podman|nerdctl> ...` instead.";
Loading