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

Support targetless agents. #1343

Merged
merged 5 commits into from May 17, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/ci.yaml
Expand Up @@ -544,7 +544,10 @@ jobs:
with:
container-runtime: ${{matrix.container-runtime}}
- run: minikube image load /tmp/test.tar
- name: cargo test containerd
# By running the test of the targetless agent first, we prove it works on an empty cluster without any pods.
- name: Run targetless E2E test.
run: cargo test -p tests targetless
- name: Run all E2E test
Razz4780 marked this conversation as resolved.
Show resolved Hide resolved
run: cargo test -p tests
- name: Collect logs
if: ${{ failure() }}
Expand Down
1 change: 1 addition & 0 deletions changelog.d/574.added.md
@@ -0,0 +1 @@
Support for targetless execution: when not specifying any target for the agent, mirrord now spins up an independent agent. This can be useful e.g. if you are just interested in getting the cluster's dns resolution and outgoing connectivity but don't want any pod's incoming traffic or FS.
7 changes: 1 addition & 6 deletions mirrord/cli/src/config.rs
Expand Up @@ -86,12 +86,7 @@
}

#[derive(Args, Debug)]
#[command(group(
ArgGroup::new("exec")
.required(true)
.multiple(true)
.args(&["target", "config_file"]),
))]
#[command(group(ArgGroup::new("exec")))]
pub(super) struct ExecArgs {
/// Target name to mirror.
/// Target can either be a deployment or a pod.
Expand Down Expand Up @@ -177,7 +172,7 @@
#[arg(long)]
pub no_udp_outgoing: bool,

/// Disable telemetry - this also disables version check. See https://github.com/metalbear-co/mirrord/blob/main/TELEMETRY.md

Check warning on line 175 in mirrord/cli/src/config.rs

View workflow job for this annotation

GitHub Actions / check-rust-docs

this URL is not a hyperlink
#[arg(long)]
pub no_telemetry: bool,

Expand Down Expand Up @@ -222,7 +217,7 @@
/// NOTE: You don't need to install the operator to use open source mirrord features.
#[command(override_usage = "mirrord operator setup [OPTIONS] | kubectl apply -f -")]
Setup {
/// ToS can be read here https://metalbear.co/legal/terms

Check warning on line 220 in mirrord/cli/src/config.rs

View workflow job for this annotation

GitHub Actions / check-rust-docs

this URL is not a hyperlink
#[arg(long)]
accept_tos: bool,

Expand Down
2 changes: 1 addition & 1 deletion mirrord/cli/src/connection.rs
Expand Up @@ -71,7 +71,7 @@ where
AgentConnection { sender, receiver },
))
} else {
if matches!(config.target.path, Some(mirrord_config::target::Target::Deployment{..})) {
if matches!(config.target, Some(mirrord_config::target::TargetConfig{ path: mirrord_config::target::Target::Deployment{..}, ..})) {
// progress.subtask("text").done_with("text");
eprintln!("When targeting multi-pod deployments, mirrord impersonates the first pod in the deployment.\n \
Support for multi-pod impersonation requires the mirrord operator, which is part of mirrord for Teams.\n \
Expand Down
2 changes: 2 additions & 0 deletions mirrord/cli/src/error.rs
Expand Up @@ -198,4 +198,6 @@ pub(crate) enum CliError {
#[error("Waitlist registration failed.")]
#[diagnostic(help("Please check the email provided and internet connection.{GENERAL_HELP}"))]
WaitlistError(reqwest::Error),
#[error("{0} is not compatible with a targetless agent, please either disable this option or specify a target.")]
IncompatibleWithTargetless(String),
}
28 changes: 27 additions & 1 deletion mirrord/cli/src/execution.rs
Expand Up @@ -17,7 +17,7 @@

use crate::{
connection::{create_and_connect, AgentConnectInfo},
error::CliError,
error::{CliError, CliError::IncompatibleWithTargetless},
extract::extract_library,
Result,
};
Expand Down Expand Up @@ -52,6 +52,7 @@
where
P: Progress + Send + Sync,
{
Self::check_config_for_targetless_agent(config)?;
let lib_path = extract_library(None, progress, true)?;
let mut env_vars = HashMap::new();
let (connect_info, mut connection) = create_and_connect(config, progress).await?;
Expand Down Expand Up @@ -189,11 +190,36 @@
})
}

/// Some features are incompatible with targetless agents, return error if they are set and
/// there is no target.
///
/// # Errors
///
/// [`IncompatibleWithTargetless`]
fn check_config_for_targetless_agent(config: &LayerConfig) -> Result<()> {
if config.target.is_none() {
if config.feature.network.incoming.is_steal() {
Err(IncompatibleWithTargetless("Steal mode".into()))?
}
if config.agent.ephemeral {
Err(IncompatibleWithTargetless(
"Using an ephemeral container for the agent".into(),
))?
}
if config.agent.pause {
Err(IncompatibleWithTargetless(
"The target pause feature".into(),
))?
}
}
Ok(())
}

/// Wait for the internal proxy to exit.
/// Required when called from extension since sometimes the extension
/// 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

Check warning on line 222 in mirrord/cli/src/execution.rs

View workflow job for this annotation

GitHub Actions / check-rust-docs

this URL is not a hyperlink
pub(crate) async fn wait(mut self) -> Result<()> {
self.child
.wait()
Expand Down
4 changes: 3 additions & 1 deletion mirrord/cli/src/main.rs
Expand Up @@ -267,7 +267,9 @@ async fn print_pod_targets(args: &ListTargetArgs) -> Result<()> {
(
layer_config.accept_invalid_certificates,
layer_config.kubeconfig,
layer_config.target.namespace,
layer_config
.target
.and_then(|target_conf| target_conf.namespace),
)
} else {
(false, None, None)
Expand Down
10 changes: 10 additions & 0 deletions mirrord/config/src/config.rs
Expand Up @@ -11,6 +11,16 @@ pub enum ConfigError {
#[error("invalid target provided `{0}`!")]
InvalidTarget(String),

#[error(
"A target namespace was specified, but no target was specified. If you want to set the
namespace in which the agent will be created, please set the agent namespace, not the
target namespace. That value can be set with agent.namespace in the configuration file,
the -a argument of the CLI, or the MIRRORD_AGENT_NAMESPACE environment variable.

If you are not trying to run targetless, please specify a target instead."
)]
TargetNamespaceWithoutTarget,

#[error("value for {1:?} not provided in {0:?} (env override {2:?})")]
ValueNotProvided(&'static str, &'static str, Option<&'static str>),

Expand Down
2 changes: 1 addition & 1 deletion mirrord/config/src/lib.rs
Expand Up @@ -97,7 +97,7 @@ pub struct LayerConfig {
/// - `podname/{sample-pod}/[container]/{sample-container}`;
/// - `deployment/{sample-deployment}/[container]/{sample-container}`;
#[config(nested)]
pub target: TargetConfig,
pub target: Option<TargetConfig>,

/// IP:PORT to connect to instead of using k8s api, for testing purposes.
#[config(env = "MIRRORD_CONNECT_TCP")]
Expand Down
127 changes: 98 additions & 29 deletions mirrord/config/src/target.rs
Expand Up @@ -31,6 +31,9 @@ pub enum TargetFileConfig {
// we need default else target value will be required in some scenarios.
Simple(#[serde(default, deserialize_with = "string_or_struct_option")] Option<Target>),
Advanced {
// Path is optional so that it can also be specified via env var instead of via conf file,
// but it is not optional in a resulting [`TargetConfig`] object - either there is a path,
// or the target configuration is `None`.
#[serde(default, deserialize_with = "string_or_struct_option")]
path: Option<Target>,
namespace: Option<String>,
Expand All @@ -40,7 +43,7 @@ pub enum TargetFileConfig {
#[derive(Serialize, Deserialize, Clone, Eq, PartialEq, Hash, Debug)]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
pub struct TargetConfig {
pub path: Option<Target>,
pub path: Target,
pub namespace: Option<String>,
}

Expand All @@ -54,33 +57,69 @@ impl FromMirrordConfig for TargetConfig {
type Generator = TargetFileConfig;
}

impl TargetFileConfig {
fn get_optional_path(path_from_config_file: Option<Target>) -> Result<Option<Target>> {
FromEnvWithError::new("MIRRORD_IMPERSONATED_TARGET")
.or(path_from_config_file)
.source_value()
.transpose()
}
}

impl MirrordConfig for TargetFileConfig {
type Generated = TargetConfig;
type Generated = Option<TargetConfig>;

/// Generate the final config object, out of the configuration parsed from a configuration file,
/// factoring in environment variables (which are also set by the front end - CLI/IDE-plugin).
///
/// `None` if no target specified.
/// Specifying target namespace without target is not allowed and results in an error that
/// explains to the user what to do instead.
fn generate_config(self) -> Result<Self::Generated> {
let config = match self {
TargetFileConfig::Simple(path) => TargetConfig {
path: FromEnvWithError::new("MIRRORD_IMPERSONATED_TARGET")
.or(path)
.source_value()
.transpose()?,
namespace: FromEnv::new("MIRRORD_TARGET_NAMESPACE")
TargetFileConfig::Simple(path) => {
// Namespace was not specified via file, get it from env var if set.
let namespace: Option<String> = FromEnv::new("MIRRORD_TARGET_NAMESPACE")
.source_value()
.transpose()?,
},
TargetFileConfig::Advanced { path, namespace } => TargetConfig {
path: FromEnvWithError::new("MIRRORD_IMPERSONATED_TARGET")
.or(path)
.source_value()
.transpose()?,
namespace: FromEnv::new("MIRRORD_TARGET_NAMESPACE")
.or(namespace)
.source_value()
.transpose()?,
},
.transpose()?;

let path = if let Some(path) = Self::get_optional_path(path)? {
path
} else {
// No path specified, neither via file, nor via env. So no `TargetConfig`,
// running targetless.

if let Some(namespace_from_env) = namespace {
// env var (or CLI flag) was set.

if !namespace_from_env.is_empty() {
// And the value is not the empty string.

return Err(ConfigError::TargetNamespaceWithoutTarget);
}
}
return Ok(None); // Run targetless.
};
TargetConfig { path, namespace }
}
TargetFileConfig::Advanced { path, namespace } => {
debug_assert!(namespace.is_some()); // Should only be advanced if namespace there.
let path = if let Some(path) = Self::get_optional_path(path)? {
path
} else {
return Err(ConfigError::TargetNamespaceWithoutTarget);
};
TargetConfig {
path,
namespace: FromEnv::new("MIRRORD_TARGET_NAMESPACE")
.or(namespace)
.source_value()
.transpose()?,
}
}
};

Ok(config)
Ok(Some(config))
}
}

Expand Down Expand Up @@ -218,21 +257,51 @@ mod tests {
use crate::{config::MirrordConfig, util::testing::with_env_vars};

#[rstest]
#[case(None, None, None)] // Nothing specified - no target config (targetless mode).
#[should_panic]
#[case(None, Some("ns"), None)] // Namespace without target - error.
#[case(
Some("pod/foo"),
None,
Some(TargetConfig{
path: Target::Pod(PodTarget {pod: "foo".to_string(), container: None}),
namespace: None
})
)] // Only pod specified
#[case(
Some("pod/foo/container/bar"),
None,
Some(TargetConfig{
path: Target::Pod(PodTarget {
pod: "foo".to_string(),
container: Some("bar".to_string())
}),
namespace: None
})
)] // Pod and container specified.
#[case(
Some("pod/foo"),
Some("baz"),
Some(TargetConfig{
path: Target::Pod(PodTarget {pod: "foo".to_string(), container: None}),
namespace: Some("baz".to_string())
})
)] // Pod and namespace specified.
fn default(
#[values((None, None), (Some("pod/foobar"), Some(Target::Pod(PodTarget { pod: "foobar".to_string(), container: None }))))]
path: (Option<&str>, Option<Target>),
#[values((None, None), (Some("foo"), Some("foo")))] namespace: (Option<&str>, Option<&str>),
#[case] path_env: Option<&str>,
#[case] namespace_env: Option<&str>,
#[case] expected_target_config: Option<TargetConfig>,
) {
with_env_vars(
vec![
("MIRRORD_IMPERSONATED_TARGET", path.0),
("MIRRORD_TARGET_NAMESPACE", namespace.0),
("MIRRORD_IMPERSONATED_TARGET", path_env),
("MIRRORD_TARGET_NAMESPACE", namespace_env),
],
|| {
let target = TargetFileConfig::default().generate_config().unwrap();
let generated_target_config =
TargetFileConfig::default().generate_config().unwrap();

assert_eq!(target.path, path.1);
assert_eq!(target.namespace.as_deref(), namespace.1);
assert_eq!(expected_target_config, generated_target_config);
},
);
}
Expand Down
26 changes: 24 additions & 2 deletions mirrord/kube/src/api.rs
@@ -1,7 +1,7 @@
use actix_codec::{AsyncRead, AsyncWrite};
use futures::{SinkExt, StreamExt};
use k8s_openapi::NamespaceResourceScope;
use kube::{Api, Client};
use k8s_openapi::{api::core::v1::Namespace, NamespaceResourceScope};
use kube::{api::ListParams, Api, Client};
use mirrord_progress::Progress;
use mirrord_protocol::{ClientCodec, ClientMessage, DaemonMessage, LogLevel};
use tokio::{
Expand Down Expand Up @@ -30,6 +30,28 @@ where
}
}

/// Get a vector of namespaces from an optional namespace. If the given namespace is Some, then
/// fetch its Namespace object, and return a vector only with that.
/// If the namespace is None - return all namespaces.
pub async fn get_namespaces(
client: &Client,
namespace: Option<&str>,
lp: &ListParams,
) -> Result<Vec<Namespace>> {
let api: Api<Namespace> = Api::all(client.clone());
Ok(if let Some(namespace) = namespace {
vec![api.get(namespace).await?]
} else {
api.list(lp).await?.items
})
}

/// Check if the client can see a given namespace.
pub async fn namespace_exists_for_client(namespace: &str, client: &Client) -> bool {
let api: Api<Namespace> = Api::all(client.clone());
api.get(namespace).await.is_ok()
}

/// Creates the task that handles the messaging between layer/agent.
/// It does the encoding/decoding of protocol.
pub fn wrap_raw_connection(
Expand Down