From 74f80b76a2e02540dd467ed8b6de85a8323260c0 Mon Sep 17 00:00:00 2001 From: meowjesty Date: Wed, 22 Jan 2025 18:46:01 -0300 Subject: [PATCH 1/8] Add tolerations for targetless agent. --- mirrord/config/src/agent.rs | 2 +- mirrord/kube/src/api/container/job.rs | 3 +++ mirrord/kube/src/api/container/pod.rs | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/mirrord/config/src/agent.rs b/mirrord/config/src/agent.rs index 9600edfcd4d..00997c87306 100644 --- a/mirrord/config/src/agent.rs +++ b/mirrord/config/src/agent.rs @@ -245,7 +245,7 @@ pub struct AgentConfig { /// ```json /// [ /// { - /// "operator": "Exists" + /// "key": "meow", "operator": "Exists", "effect": "NoSchedule" /// } /// ] /// ``` diff --git a/mirrord/kube/src/api/container/job.rs b/mirrord/kube/src/api/container/job.rs index d9958e6620b..d064224ded2 100644 --- a/mirrord/kube/src/api/container/job.rs +++ b/mirrord/kube/src/api/container/job.rs @@ -114,6 +114,8 @@ pub struct JobVariant { impl<'c> JobVariant> { pub fn new(agent: &'c AgentConfig, params: &'c ContainerParams) -> Self { + // TODO(alex) [high]: Here we also have agent config, which has tolerations. + // So why doesn't it propagate? JobVariant { inner: PodVariant::new(agent, params), } @@ -199,6 +201,7 @@ impl<'c> JobTargetedVariant<'c> { params: &'c ContainerParams, runtime_data: &'c RuntimeData, ) -> Self { + // TODO(alex) [high]: Here we have agent config, which has tolerations. let inner = PodTargetedVariant::new(agent, params, runtime_data); JobTargetedVariant { diff --git a/mirrord/kube/src/api/container/pod.rs b/mirrord/kube/src/api/container/pod.rs index f8461e8a002..9e44a69a5b1 100644 --- a/mirrord/kube/src/api/container/pod.rs +++ b/mirrord/kube/src/api/container/pod.rs @@ -195,6 +195,8 @@ impl ContainerVariant for PodTargetedVariant<'_> { let agent = self.agent_config(); let params = self.params(); + let tolerations = agent.tolerations.as_ref().unwrap_or(&DEFAULT_TOLERATIONS); + let env = self.runtime_data.mesh.map(|mesh_vendor| { let mut env = vec![EnvVar { name: "MIRRORD_AGENT_IN_SERVICE_MESH".into(), @@ -214,6 +216,7 @@ impl ContainerVariant for PodTargetedVariant<'_> { let update = Pod { spec: Some(PodSpec { restart_policy: Some("Never".to_string()), + tolerations: Some(tolerations.clone()), host_pid: Some(true), node_name: Some(runtime_data.node_name.clone()), volumes: Some(vec![ From 4e501a9b49ebdf99c70dc2a591057b41f8740e75 Mon Sep 17 00:00:00 2001 From: meowjesty Date: Thu, 23 Jan 2025 16:30:37 -0300 Subject: [PATCH 2/8] Out comment --- mirrord/kube/src/api/container/job.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/mirrord/kube/src/api/container/job.rs b/mirrord/kube/src/api/container/job.rs index d064224ded2..4b2061db252 100644 --- a/mirrord/kube/src/api/container/job.rs +++ b/mirrord/kube/src/api/container/job.rs @@ -114,8 +114,6 @@ pub struct JobVariant { impl<'c> JobVariant> { pub fn new(agent: &'c AgentConfig, params: &'c ContainerParams) -> Self { - // TODO(alex) [high]: Here we also have agent config, which has tolerations. - // So why doesn't it propagate? JobVariant { inner: PodVariant::new(agent, params), } From fce1896fe18cf233c546e9bf1742f46f3e54bef7 Mon Sep 17 00:00:00 2001 From: meowjesty Date: Thu, 23 Jan 2025 16:36:01 -0300 Subject: [PATCH 3/8] No default --- mirrord/kube/src/api/container/pod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mirrord/kube/src/api/container/pod.rs b/mirrord/kube/src/api/container/pod.rs index 9e44a69a5b1..120d205a5aa 100644 --- a/mirrord/kube/src/api/container/pod.rs +++ b/mirrord/kube/src/api/container/pod.rs @@ -195,7 +195,7 @@ impl ContainerVariant for PodTargetedVariant<'_> { let agent = self.agent_config(); let params = self.params(); - let tolerations = agent.tolerations.as_ref().unwrap_or(&DEFAULT_TOLERATIONS); + let tolerations = agent.tolerations.clone(); let env = self.runtime_data.mesh.map(|mesh_vendor| { let mut env = vec![EnvVar { @@ -216,7 +216,7 @@ impl ContainerVariant for PodTargetedVariant<'_> { let update = Pod { spec: Some(PodSpec { restart_policy: Some("Never".to_string()), - tolerations: Some(tolerations.clone()), + tolerations, host_pid: Some(true), node_name: Some(runtime_data.node_name.clone()), volumes: Some(vec![ From dbf30805d715d2a72c2abe46c71a2dc8cfd75ff7 Mon Sep 17 00:00:00 2001 From: meowjesty Date: Thu, 23 Jan 2025 16:36:49 -0300 Subject: [PATCH 4/8] out comment 2 --- mirrord/kube/src/api/container/job.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/mirrord/kube/src/api/container/job.rs b/mirrord/kube/src/api/container/job.rs index 4b2061db252..d9958e6620b 100644 --- a/mirrord/kube/src/api/container/job.rs +++ b/mirrord/kube/src/api/container/job.rs @@ -199,7 +199,6 @@ impl<'c> JobTargetedVariant<'c> { params: &'c ContainerParams, runtime_data: &'c RuntimeData, ) -> Self { - // TODO(alex) [high]: Here we have agent config, which has tolerations. let inner = PodTargetedVariant::new(agent, params, runtime_data); JobTargetedVariant { From 0e7d32c5b5604b400d7bc1ac184cd36870b1189f Mon Sep 17 00:00:00 2001 From: meowjesty Date: Fri, 24 Jan 2025 12:24:11 -0300 Subject: [PATCH 5/8] tolerations in targetless, but no default --- mirrord/config/src/agent.rs | 6 ++++-- mirrord/kube/src/api/container/pod.rs | 13 ++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/mirrord/config/src/agent.rs b/mirrord/config/src/agent.rs index 00997c87306..caf1d131a25 100644 --- a/mirrord/config/src/agent.rs +++ b/mirrord/config/src/agent.rs @@ -240,8 +240,10 @@ pub struct AgentConfig { /// ### agent.tolerations {#agent-tolerations} /// - /// Set pod tolerations. (not with ephemeral agents) - /// Default is + /// Set pod tolerations. (not with ephemeral agents). + /// + /// Defaults to `operator: Exists`. + /// /// ```json /// [ /// { diff --git a/mirrord/kube/src/api/container/pod.rs b/mirrord/kube/src/api/container/pod.rs index 120d205a5aa..984a3fd8afa 100644 --- a/mirrord/kube/src/api/container/pod.rs +++ b/mirrord/kube/src/api/container/pod.rs @@ -20,6 +20,7 @@ use crate::api::{ runtime::RuntimeData, }; +/// The `targetless` agent variant is created by this, see its [`PodVariant::as_update`]. pub struct PodVariant<'c> { agent: &'c AgentConfig, command_line: Vec, @@ -67,8 +68,6 @@ impl ContainerVariant for PodVariant<'_> { .. } = self; - let tolerations = agent.tolerations.as_ref().unwrap_or(&DEFAULT_TOLERATIONS); - let resources = agent.resources.clone().unwrap_or_else(|| { serde_json::from_value(serde_json::json!({ "requests": @@ -124,7 +123,7 @@ impl ContainerVariant for PodVariant<'_> { spec: Some(PodSpec { restart_policy: Some("Never".to_string()), image_pull_secrets, - tolerations: Some(tolerations.clone()), + tolerations: agent.tolerations.clone(), node_selector: Some(node_selector), service_account_name: agent.service_account.clone(), containers: vec![Container { @@ -148,6 +147,10 @@ impl ContainerVariant for PodVariant<'_> { } } +/// The `targeted` agent variant is created by this. +/// +/// It builds on top of [`PodVariant`], merging spec, etc from there. See +/// [`PodTargetedVariant::as_update`]. pub struct PodTargetedVariant<'c> { inner: PodVariant<'c>, runtime_data: &'c RuntimeData, @@ -195,7 +198,7 @@ impl ContainerVariant for PodTargetedVariant<'_> { let agent = self.agent_config(); let params = self.params(); - let tolerations = agent.tolerations.clone(); + let tolerations = agent.tolerations.as_ref().unwrap_or(&DEFAULT_TOLERATIONS); let env = self.runtime_data.mesh.map(|mesh_vendor| { let mut env = vec![EnvVar { @@ -216,7 +219,7 @@ impl ContainerVariant for PodTargetedVariant<'_> { let update = Pod { spec: Some(PodSpec { restart_policy: Some("Never".to_string()), - tolerations, + tolerations: Some(tolerations.clone()), host_pid: Some(true), node_name: Some(runtime_data.node_name.clone()), volumes: Some(vec![ From e232d667c09c673a3aa1a96204cabf9594cf93bb Mon Sep 17 00:00:00 2001 From: Razz4780 Date: Mon, 27 Jan 2025 11:57:33 +0100 Subject: [PATCH 6/8] Changelog added --- changelog.d/+targetless-tolerations.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/+targetless-tolerations.fixed.md diff --git a/changelog.d/+targetless-tolerations.fixed.md b/changelog.d/+targetless-tolerations.fixed.md new file mode 100644 index 00000000000..98d97eed309 --- /dev/null +++ b/changelog.d/+targetless-tolerations.fixed.md @@ -0,0 +1 @@ +mirrord no longer uses the default `{"operator": "Exists"}` tolerations when spawning targetless agent pods. From 1707856246ed91705a87671b4b0fd4b3f183090b Mon Sep 17 00:00:00 2001 From: Razz4780 Date: Mon, 27 Jan 2025 12:23:36 +0100 Subject: [PATCH 7/8] Updated schema and configuration.md --- mirrord-schema.json | 2 +- mirrord/config/configuration.md | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/mirrord-schema.json b/mirrord-schema.json index dee629fe493..0cebfb20ce9 100644 --- a/mirrord-schema.json +++ b/mirrord-schema.json @@ -461,7 +461,7 @@ }, "tolerations": { "title": "agent.tolerations {#agent-tolerations}", - "description": "Set pod tolerations. (not with ephemeral agents) Default is ```json [ { \"operator\": \"Exists\" } ] ```\n\nSet to an empty array to have no tolerations at all", + "description": "Set pod tolerations. (not with ephemeral agents).\n\nDefaults to `operator: Exists`.\n\n```json [ { \"key\": \"meow\", \"operator\": \"Exists\", \"effect\": \"NoSchedule\" } ] ```\n\nSet to an empty array to have no tolerations at all", "type": [ "array", "null" diff --git a/mirrord/config/configuration.md b/mirrord/config/configuration.md index e8f3f43c437..20d3dbc0e0e 100644 --- a/mirrord/config/configuration.md +++ b/mirrord/config/configuration.md @@ -394,12 +394,14 @@ Defaults to `60`. ### agent.tolerations {#agent-tolerations} -Set pod tolerations. (not with ephemeral agents) -Default is +Set pod tolerations. (not with ephemeral agents). + +Defaults to `operator: Exists`. + ```json [ { - "operator": "Exists" + "key": "meow", "operator": "Exists", "effect": "NoSchedule" } ] ``` From 7ce76d585a0a8313f646f3966afdf2b7cd6ca6fe Mon Sep 17 00:00:00 2001 From: Razz4780 Date: Mon, 27 Jan 2025 12:39:40 +0100 Subject: [PATCH 8/8] Fixed UT --- mirrord/kube/src/api/container/job.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/mirrord/kube/src/api/container/job.rs b/mirrord/kube/src/api/container/job.rs index d9958e6620b..7c3247a091f 100644 --- a/mirrord/kube/src/api/container/job.rs +++ b/mirrord/kube/src/api/container/job.rs @@ -285,7 +285,6 @@ mod test { "restartPolicy": "Never", "imagePullSecrets": agent.image_pull_secrets, "nodeSelector": {}, - "tolerations": *DEFAULT_TOLERATIONS, "serviceAccountName": agent.service_account, "containers": [ {