diff --git a/changelog.d/+targetless-namespace.changed.md b/changelog.d/+targetless-namespace.changed.md new file mode 100644 index 00000000000..12189203701 --- /dev/null +++ b/changelog.d/+targetless-namespace.changed.md @@ -0,0 +1,2 @@ +Namespace for `targetless` runs is now be specified with the `target.namespace` config field (or the `MIRRORD_TARGET_NAMESPACE` environment variable). +`agent.namespace` field is ignored in targetless runs. diff --git a/mirrord-schema.json b/mirrord-schema.json index 6a8bb2d6def..f5535604247 100644 --- a/mirrord-schema.json +++ b/mirrord-schema.json @@ -250,7 +250,7 @@ "additionalProperties": false }, "AgentFileConfig": { - "description": "Configuration for the mirrord-agent pod that is spawned in the Kubernetes cluster.\n\nWe provide sane defaults for this option, so you don't have to set up anything here.\n\n```json { \"agent\": { \"log_level\": \"info\", \"json_log\": false, \"namespace\": \"default\", \"image\": \"ghcr.io/metalbear-co/mirrord:latest\", \"image_pull_policy\": \"IfNotPresent\", \"image_pull_secrets\": [ { \"secret-key\": \"secret\" } ], \"ttl\": 30, \"ephemeral\": false, \"communication_timeout\": 30, \"startup_timeout\": 360, \"network_interface\": \"eth0\", \"flush_connections\": false } } ```", + "description": "Configuration for the mirrord-agent pod that is spawned in the Kubernetes cluster.\n\n**Note:** this configuration is ignored when using the mirrord Operator. Agent configuration is done by the cluster admin.\n\nWe provide sane defaults for this option, so you don't have to set up anything here.\n\n```json { \"agent\": { \"log_level\": \"info\", \"json_log\": false, \"namespace\": \"default\", \"image\": \"ghcr.io/metalbear-co/mirrord:latest\", \"image_pull_policy\": \"IfNotPresent\", \"image_pull_secrets\": [ { \"secret-key\": \"secret\" } ], \"ttl\": 30, \"ephemeral\": false, \"communication_timeout\": 30, \"startup_timeout\": 360, \"network_interface\": \"eth0\", \"flush_connections\": false } } ```", "type": "object", "properties": { "annotations": { @@ -306,7 +306,7 @@ }, "ephemeral": { "title": "agent.ephemeral {#agent-ephemeral}", - "description": "Runs the agent as an [ephemeral container](https://kubernetes.io/docs/concepts/workloads/pods/ephemeral-containers/)\n\nDefaults to `false`.", + "description": "Runs the agent as an [ephemeral container](https://kubernetes.io/docs/concepts/workloads/pods/ephemeral-containers/).\n\nNot compatible with targetless runs.\n\nDefaults to `false`.", "type": [ "boolean", "null" @@ -388,7 +388,7 @@ }, "namespace": { "title": "agent.namespace {#agent-namespace}", - "description": "Namespace where the agent shall live. Note: Doesn't work with ephemeral containers. Defaults to the current kubernetes namespace.", + "description": "Namespace where the agent shall live.\n\n**Note:** ignored in targetless runs or when the agent is run as an ephemeral container.\n\nDefaults to the current kubernetes namespace.", "type": [ "string", "null" diff --git a/mirrord/config/configuration.md b/mirrord/config/configuration.md index e1aa8368a35..a6f7ff259b8 100644 --- a/mirrord/config/configuration.md +++ b/mirrord/config/configuration.md @@ -140,6 +140,9 @@ If not provided, mirrord will use value from the kubeconfig. Configuration for the mirrord-agent pod that is spawned in the Kubernetes cluster. +**Note:** this configuration is ignored when using the mirrord Operator. +Agent configuration is done by the cluster admin. + We provide sane defaults for this option, so you don't have to set up anything here. ```json @@ -204,7 +207,9 @@ as targetless agent containers have no capabilities. ### agent.ephemeral {#agent-ephemeral} Runs the agent as an -[ephemeral container](https://kubernetes.io/docs/concepts/workloads/pods/ephemeral-containers/) +[ephemeral container](https://kubernetes.io/docs/concepts/workloads/pods/ephemeral-containers/). + +Not compatible with targetless runs. Defaults to `false`. @@ -323,7 +328,9 @@ configured to scrape for metrics. ### agent.namespace {#agent-namespace} Namespace where the agent shall live. -Note: Doesn't work with ephemeral containers. + +**Note:** ignored in targetless runs or when the agent is run as an ephemeral container. + Defaults to the current kubernetes namespace. ### agent.network_interface {#agent-network_interface} @@ -1587,7 +1594,7 @@ Please note that: - `job`, `cronjob`, `statefulset` and `service` targets require the mirrord Operator - `job` and `cronjob` targets require the [`copy_target`](#feature-copy_target) feature -Shortened setup: +Shortened setup with a target: ```json { @@ -1598,7 +1605,7 @@ Shortened setup: The setup above will result in a session targeting the `bear-pod` Kubernetes pod in the user's default namespace. A target container will be chosen by mirrord. -Shortened setup with target container: +Shortened setup with a target container: ```json { @@ -1609,7 +1616,7 @@ Shortened setup with target container: The setup above will result in a session targeting the `bear-pod-container` container in the `bear-pod` Kubernetes pod in the user's default namespace. -Complete setup: +Complete setup with a target container: ```json { @@ -1626,16 +1633,34 @@ Complete setup: The setup above will result in a session targeting the `bear-pod-container` container in the `bear-pod` Kubernetes pod in the `bear-pod-namespace` namespace. +Setup with a namespace for a targetless run: + +```json +{ + "target": { + "path": "targetless", + "namespace": "bear-namespace" + } +} +``` + +The setup above will result in a session without any target. +Remote outgoing traffic and DNS will be done from the `bear-namespace` namespace. + ### target.namespace {#target-namespace} Namespace where the target lives. +For targetless runs, this the namespace in which remote networking is done. + Defaults to the Kubernetes user's default namespace (defined in Kubernetes context). ### target.path {#target-path} Specifies the Kubernetes resource to target. +If not given, defaults to `targetless`. + Note: targeting services and whole workloads is available only in mirrord for Teams. If you target a workload without the mirrord Operator, it will choose a random pod replica to work with. @@ -1652,7 +1677,7 @@ Supports: - `statefulset/{statefulset-name}[/container/{container-name}]`; (requires mirrord Operator) - `service/{service-name}[/container/{container-name}]`; (requires mirrord Operator) -- `replicaset/{replicaset-name}[/container/{replicaset-name}]`; (requires mirrord Operator) +- `replicaset/{replicaset-name}[/container/{container-name}]`; (requires mirrord Operator) ## telemetry {#root-telemetry} Controls whether or not mirrord sends telemetry data to MetalBear cloud. diff --git a/mirrord/config/src/agent.rs b/mirrord/config/src/agent.rs index 7f8cbfa0967..4562ada4167 100644 --- a/mirrord/config/src/agent.rs +++ b/mirrord/config/src/agent.rs @@ -47,6 +47,9 @@ impl fmt::Display for LinuxCapability { /// Configuration for the mirrord-agent pod that is spawned in the Kubernetes cluster. /// +/// **Note:** this configuration is ignored when using the mirrord Operator. +/// Agent configuration is done by the cluster admin. +/// /// We provide sane defaults for this option, so you don't have to set up anything here. /// /// ```json @@ -106,7 +109,9 @@ pub struct AgentConfig { /// ### agent.namespace {#agent-namespace} /// /// Namespace where the agent shall live. - /// Note: Doesn't work with ephemeral containers. + /// + /// **Note:** ignored in targetless runs or when the agent is run as an ephemeral container. + /// /// Defaults to the current kubernetes namespace. #[config(env = "MIRRORD_AGENT_NAMESPACE")] pub namespace: Option, @@ -183,7 +188,9 @@ pub struct AgentConfig { /// ### agent.ephemeral {#agent-ephemeral} /// /// Runs the agent as an - /// [ephemeral container](https://kubernetes.io/docs/concepts/workloads/pods/ephemeral-containers/) + /// [ephemeral container](https://kubernetes.io/docs/concepts/workloads/pods/ephemeral-containers/). + /// + /// Not compatible with targetless runs. /// /// Defaults to `false`. #[config(env = "MIRRORD_EPHEMERAL_CONTAINER", default = false)] diff --git a/mirrord/config/src/lib.rs b/mirrord/config/src/lib.rs index d0384d834b5..ad788879c43 100644 --- a/mirrord/config/src/lib.rs +++ b/mirrord/config/src/lib.rs @@ -500,14 +500,13 @@ impl LayerConfig { Err(ConfigError::TargetJobWithoutCopyTarget)? } - if self.target.path.is_none() && !context.ide { - // In the IDE, a target may be selected after `mirrord verify-config` is run, so we - // for this case we treat these as warnings. They'll become errors once mirrord proper - // tries to start (if the user somehow managed to not select a target by then). - if self.target.namespace.is_some() { - Err(ConfigError::TargetNamespaceWithoutTarget)? - } + let is_targetless = match self.target.path.as_ref() { + Some(Target::Targetless) => true, + None => !context.ide, + _ => false, + }; + if is_targetless { if self.feature.network.incoming.is_steal() { Err(ConfigError::Conflict("Steal mode is not compatible with a targetless agent, please either disable this option or specify a target.".into()))? } @@ -520,6 +519,10 @@ impl LayerConfig { .into(), ))? } + + if self.agent.namespace.is_some() { + context.add_warning("Agent namespace is ignored in targetless runs".into()); + } } if self.feature.copy_target.enabled { @@ -532,7 +535,7 @@ impl LayerConfig { } // Target may also be set later in the UI. - if self.target.path.is_none() && !context.ide { + if is_targetless { return Err(ConfigError::Conflict( "The copy target feature is not compatible with a targetless agent, \ please either disable this option or specify a target." diff --git a/mirrord/config/src/target.rs b/mirrord/config/src/target.rs index 0ca815e0019..1a99da6e2cc 100644 --- a/mirrord/config/src/target.rs +++ b/mirrord/config/src/target.rs @@ -87,7 +87,7 @@ fn make_simple_target_custom_schema(gen: &mut SchemaGenerator) -> schemars::sche /// - `job`, `cronjob`, `statefulset` and `service` targets require the mirrord Operator /// - `job` and `cronjob` targets require the [`copy_target`](#feature-copy_target) feature /// -/// Shortened setup: +/// Shortened setup with a target: /// ///```json /// { @@ -98,7 +98,7 @@ fn make_simple_target_custom_schema(gen: &mut SchemaGenerator) -> schemars::sche /// The setup above will result in a session targeting the `bear-pod` Kubernetes pod /// in the user's default namespace. A target container will be chosen by mirrord. /// -/// Shortened setup with target container: +/// Shortened setup with a target container: /// /// ```json /// { @@ -109,7 +109,7 @@ fn make_simple_target_custom_schema(gen: &mut SchemaGenerator) -> schemars::sche /// The setup above will result in a session targeting the `bear-pod-container` container /// in the `bear-pod` Kubernetes pod in the user's default namespace. /// -/// Complete setup: +/// Complete setup with a target container: /// /// ```json /// { @@ -125,6 +125,20 @@ fn make_simple_target_custom_schema(gen: &mut SchemaGenerator) -> schemars::sche /// /// The setup above will result in a session targeting the `bear-pod-container` container /// in the `bear-pod` Kubernetes pod in the `bear-pod-namespace` namespace. +/// +/// Setup with a namespace for a targetless run: +/// +/// ```json +/// { +/// "target": { +/// "path": "targetless", +/// "namespace": "bear-namespace" +/// } +/// } +/// ``` +/// +/// The setup above will result in a session without any target. +/// Remote outgoing traffic and DNS will be done from the `bear-namespace` namespace. #[derive(Serialize, Deserialize, Clone, Eq, PartialEq, Hash, Debug)] #[serde(deny_unknown_fields)] pub struct TargetConfig { @@ -132,6 +146,8 @@ pub struct TargetConfig { /// /// Specifies the Kubernetes resource to target. /// + /// If not given, defaults to `targetless`. + /// /// Note: targeting services and whole workloads is available only in mirrord for Teams. /// If you target a workload without the mirrord Operator, it will choose a random pod replica /// to work with. @@ -156,6 +172,8 @@ pub struct TargetConfig { /// /// Namespace where the target lives. /// + /// For targetless runs, this the namespace in which remote networking is done. + /// /// Defaults to the Kubernetes user's default namespace (defined in Kubernetes context). #[serde(skip_serializing_if = "Option::is_none")] pub namespace: Option, @@ -322,23 +340,6 @@ impl FromStr for Target { } impl Target { - /// Get the target name - pod name, deployment name, rollout name.. - pub fn get_target_name(&self) -> String { - match self { - Target::Deployment(target) => target.deployment.clone(), - Target::Pod(target) => target.pod.clone(), - Target::Rollout(target) => target.rollout.clone(), - Target::Job(target) => target.job.clone(), - Target::CronJob(target) => target.cron_job.clone(), - Target::StatefulSet(target) => target.stateful_set.clone(), - Target::Service(target) => target.service.clone(), - Target::ReplicaSet(target) => target.replica_set.clone(), - Target::Targetless => { - unreachable!("this shouldn't happen - called from operator on a flow where it's not targetless.") - } - } - } - /// `true` if this [`Target`] is only supported when the copy target feature is enabled. pub(super) fn requires_copy(&self) -> bool { matches!(self, Target::Job(_) | Target::CronJob(_)) diff --git a/mirrord/kube/src/api/kubernetes.rs b/mirrord/kube/src/api/kubernetes.rs index 86a84f7e4ba..822cb6297b0 100644 --- a/mirrord/kube/src/api/kubernetes.rs +++ b/mirrord/kube/src/api/kubernetes.rs @@ -41,6 +41,10 @@ pub struct KubernetesAPI { } impl KubernetesAPI { + /// Creates a new instance from the given [`LayerConfig`]. + /// + /// If [`LayerConfig::target`] specifies a targetless run, + /// replaces [`AgentConfig::namespace`] with the target namespace. pub async fn create(config: &LayerConfig) -> Result { let client = create_kube_config( config.accept_invalid_certificates, @@ -50,7 +54,17 @@ impl KubernetesAPI { .await? .try_into()?; - Ok(KubernetesAPI::new(client, config.agent.clone())) + let mut agent = config.agent.clone(); + if config + .target + .path + .as_ref() + .is_none_or(|path| matches!(path, Target::Targetless)) + { + agent.namespace = config.target.namespace.clone(); + } + + Ok(KubernetesAPI::new(client, agent)) } pub fn new(client: Client, agent: AgentConfig) -> Self { diff --git a/mirrord/kube/src/resolved.rs b/mirrord/kube/src/resolved.rs index c96b59e6962..954dc109b76 100644 --- a/mirrord/kube/src/resolved.rs +++ b/mirrord/kube/src/resolved.rs @@ -6,7 +6,7 @@ use k8s_openapi::api::{ core::v1::{Pod, Service}, }; use kube::{Client, Resource, ResourceExt}; -use mirrord_config::{feature::network::incoming::ConcurrentSteal, target::Target}; +use mirrord_config::target::Target; use tracing::Level; use super::{ @@ -532,37 +532,3 @@ impl ResolvedTarget { } } } - -impl ResolvedTarget { - pub fn connect_url( - &self, - use_proxy: bool, - concurrent_steal: ConcurrentSteal, - api_version: &str, - plural: &str, - url_path: &str, - ) -> String { - let name = self.urlfied_name(); - let namespace = self.namespace().unwrap_or("default"); - - if use_proxy { - format!("/apis/{api_version}/proxy/namespaces/{namespace}/{plural}/{name}?on_concurrent_steal={concurrent_steal}&connect=true") - } else { - format!("{url_path}/{name}?on_concurrent_steal={concurrent_steal}&connect=true") - } - } - - pub fn urlfied_name(&self) -> String { - let mut url = self.type_().to_string(); - - if let Some(target_name) = self.name() { - url.push_str(&format!(".{target_name}")); - } - - if let Some(container) = self.container() { - url.push_str(&format!(".container.{container}")); - } - - url - } -} diff --git a/mirrord/operator/src/client.rs b/mirrord/operator/src/client.rs index 26f3d66e5b1..970c92e49c6 100644 --- a/mirrord/operator/src/client.rs +++ b/mirrord/operator/src/client.rs @@ -15,7 +15,11 @@ use mirrord_auth::{ credential_store::{CredentialStoreSync, UserIdentity}, credentials::LicenseValidity, }; -use mirrord_config::{feature::split_queues::SplitQueuesConfig, target::Target, LayerConfig}; +use mirrord_config::{ + feature::{network::incoming::ConcurrentSteal, split_queues::SplitQueuesConfig}, + target::Target, + LayerConfig, +}; use mirrord_kube::{ api::{kubernetes::create_kube_config, runtime::RuntimeDataProvider}, error::KubeApiError, @@ -507,20 +511,6 @@ where HeaderValue::try_from(as_base64) .map_err(|error| OperatorApiError::ClientCertError(error.to_string())) } - - /// Returns a namespace of the target based on the given [`LayerConfig`] and default namespace - /// of [`Client`] used by this instance. - fn target_namespace<'a>(&'a self, layer_config: &'a LayerConfig) -> &'a str { - let namespace_opt = if layer_config.target.path.is_some() { - // Not a targetless run, we use target's namespace. - layer_config.target.namespace.as_deref() - } else { - // A targetless run, we use the namespace where the agent should live. - layer_config.agent.namespace.as_deref() - }; - - namespace_opt.unwrap_or(self.client.default_namespace()) - } } impl OperatorApi { @@ -587,7 +577,11 @@ impl OperatorApi { .clone() .unwrap_or(Target::Targetless); let scale_down = layer_config.feature.copy_target.scale_down; - let namespace = self.target_namespace(layer_config); + let namespace = layer_config + .target + .namespace + .as_deref() + .unwrap_or(self.client.default_namespace()); let copied = self .copy_target( target, @@ -633,12 +627,9 @@ impl OperatorApi { } ( - target.connect_url( - use_proxy_api, + self.target_connect_url( + &target, layer_config.feature.network.incoming.on_concurrent_steal, - &TargetCrd::api_version(&()), - &TargetCrd::plural(&()), - &TargetCrd::url_path(&(), target.namespace()), ), None, ) @@ -671,6 +662,43 @@ impl OperatorApi { Ok(OperatorSessionConnection { session, tx, rx }) } + /// Produces the URL for making a connection request to the operator. + fn target_connect_url( + &self, + target: &ResolvedTarget, + concurrent_steal: ConcurrentSteal, + ) -> String { + let use_proxy = self + .operator + .spec + .supported_features() + .contains(&NewOperatorFeature::ProxyApi); + + let name = { + let mut urlfied_name = target.type_().to_string(); + if let Some(target_name) = target.name() { + urlfied_name.push('.'); + urlfied_name.push_str(target_name); + } + if let Some(target_container) = target.container() { + urlfied_name.push('.'); + urlfied_name.push_str(target_container); + } + urlfied_name + }; + + let namespace = target.namespace().unwrap_or("default"); + + if use_proxy { + let api_version = TargetCrd::api_version(&()); + let plural = TargetCrd::plural(&()); + format!("/apis/{api_version}/proxy/namespaces/{namespace}/{plural}/{name}?on_concurrent_steal={concurrent_steal}&connect=true") + } else { + let url_path = TargetCrd::url_path(&(), Some(namespace)); + format!("{url_path}/{name}?on_concurrent_steal={concurrent_steal}&connect=true") + } + } + /// Returns client cert's public key in a base64 encoded string (no padding same like in /// operator logic) pub fn get_user_id_str(&self) -> String {