Add tolerations for targetless agent. (#3033)

* Add tolerations for targetless agent. * Out comment * No default * out comment 2 * tolerations in targetless, but no default * Changelog added * Updated schema and configuration.md * Fixed UT --------- Co-authored-by: Razz4780 <[email protected]>
metalbear-co · Jan 27, 2025 · c416f79 · c416f79
1 parent db91583
commit c416f79
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 11 deletions.
diff --git a/changelog.d/+targetless-tolerations.fixed.md b/changelog.d/+targetless-tolerations.fixed.md
@@ -0,0 +1 @@
+mirrord no longer uses the default `{"operator": "Exists"}` tolerations when spawning targetless agent pods.
diff --git 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
@@ -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"
   }
 ]
 ```

diff --git a/mirrord/config/src/agent.rs b/mirrord/config/src/agent.rs
@@ -240,12 +240,14 @@ 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
     /// [
     ///   {
-    ///     "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
@@ -285,7 +285,6 @@ mod test {
                         "restartPolicy": "Never",
                         "imagePullSecrets": agent.image_pull_secrets,
                         "nodeSelector": {},
-                        "tolerations": *DEFAULT_TOLERATIONS,
                         "serviceAccountName": agent.service_account,
                         "containers": [
                             {

diff --git 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<String>,
@@ -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,6 +198,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 +219,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![
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		mirrord no longer uses the default `{"operator": "Exists"}` tolerations when spawning targetless agent pods.