diff --git a/tools/packaging/kata-deploy/binary/src/config.rs b/tools/packaging/kata-deploy/binary/src/config.rs index ee94c8808a..8c7ffe4e73 100644 --- a/tools/packaging/kata-deploy/binary/src/config.rs +++ b/tools/packaging/kata-deploy/binary/src/config.rs @@ -204,6 +204,17 @@ pub struct Config { /// Enable dm-verity integrity for EROFS lower layers. /// Independent of rw-layer backing; works with both disk and memory modes. pub erofs_dmverity: bool, + /// Startup taints to remove from the node once Kata is installed and the + /// node has been labeled `katacontainers.io/kata-runtime=true`. Each entry + /// is either a bare taint key (matches any effect) or `key:effect` (matches + /// only that effect). Empty means "remove nothing" and is the default, so + /// the behavior is opt-in and a no-op for users who don't configure it. + /// + /// This lets a node be provisioned with a startup taint that keeps Kata + /// workloads from being scheduled before the runtime binaries exist; kata-deploy + /// removes the taint as its final install step, closing the window in which a + /// pod could land on a not-yet-ready node. + pub startup_taints: Vec, } impl Config { @@ -352,6 +363,15 @@ impl Config { .trim() .eq_ignore_ascii_case("dmverity"); + // Startup taints to remove after install+label. Comma- or whitespace-separated + // list of `key` or `key:effect` entries. Empty/unset means "remove nothing". + let startup_taints = env::var("STARTUP_TAINTS") + .unwrap_or_default() + .split([',', ' ', '\t', '\n']) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect(); + let config = Config { node_name, debug, @@ -382,6 +402,7 @@ impl Config { custom_runtimes, erofs_snapshotter_mode, erofs_dmverity, + startup_taints, }; // Validate the configuration @@ -620,6 +641,7 @@ impl Config { "* CUSTOM_RUNTIMES_ENABLED: {}", self.custom_runtimes_enabled ); + info!("* STARTUP_TAINTS: {}", self.startup_taints.join(" ")); if !self.custom_runtimes.is_empty() { info!("* CUSTOM_RUNTIMES:"); for runtime in &self.custom_runtimes { @@ -948,6 +970,7 @@ mod tests { "EXPERIMENTAL_FORCE_GUEST_PULL_S390X", "EXPERIMENTAL_FORCE_GUEST_PULL_PPC64LE", "CONTAINERD_CONFIG_FILE_NAME", + "STARTUP_TAINTS", ]; for var in &vars { std::env::remove_var(var); diff --git a/tools/packaging/kata-deploy/binary/src/k8s/client.rs b/tools/packaging/kata-deploy/binary/src/k8s/client.rs index 7a196c84d2..e624b32d2e 100644 --- a/tools/packaging/kata-deploy/binary/src/k8s/client.rs +++ b/tools/packaging/kata-deploy/binary/src/k8s/client.rs @@ -116,6 +116,60 @@ impl K8sClient { Ok(()) } + /// Remove taints from the bound node. + /// + /// `matchers` is a list of `key` or `key:effect` entries. A bare key removes + /// every taint with that key regardless of effect; `key:effect` removes only + /// the taint matching both. Taints not matched are left untouched. + /// + /// Returns the matcher labels that matched and were removed. A matcher that + /// matches nothing is not an error: the node simply had no such taint, which + /// is the expected steady state on re-runs and pod restarts. + pub async fn remove_node_taints(&self, matchers: &[String]) -> Result> { + if matchers.is_empty() { + return Ok(Vec::new()); + } + + let node = self.get_node().await?; + let current = node + .spec + .as_ref() + .and_then(|s| s.taints.clone()) + .unwrap_or_default(); + + if current.is_empty() { + return Ok(Vec::new()); + } + + let (retained, removed) = partition_taints(current, matchers); + + if removed.is_empty() { + return Ok(removed); + } + + for label in &removed { + info!("Removing taint {} from node {}", label, self.node_name); + } + + // `.spec.taints` is an atomic list server-side, so we replace it wholesale + // with the retained set. A JSON-merge patch on the whole array is + // equivalent here; we use a merge patch for consistency with label_node + // and to avoid resourceVersion juggling. + let patch = Patch::Merge(json!({ + "spec": { + "taints": retained, + } + })); + + let pp = PatchParams::default(); + self.node_api + .patch(&self.node_name, &pp, &patch) + .await + .with_context(|| format!("Failed to patch node {} to remove taints", self.node_name))?; + + Ok(removed) + } + /// Returns whether a non-terminating DaemonSet with this exact name /// exists in the current namespace. Used to decide whether this pod is /// being restarted (true) or uninstalled (false). @@ -504,6 +558,49 @@ impl K8sClient { } } +/// Split `taints` into (retained, removed-labels) according to `matchers`. +/// +/// Each matcher is `key` (matches any effect) or `key:effect` (matches only that +/// effect). Pure and cluster-free so the matching rules can be unit-tested; the +/// async `remove_node_taints` method wraps this with the apiserver read/patch. +fn partition_taints( + taints: Vec, + matchers: &[String], +) -> (Vec, Vec) { + // Split each matcher into (key, optional effect) once up front. + let parsed: Vec<(&str, Option<&str>)> = matchers + .iter() + .map(|m| match m.split_once(':') { + Some((k, e)) => (k.trim(), Some(e.trim())), + None => (m.trim(), None), + }) + .filter(|(k, _)| !k.is_empty()) + .collect(); + + let mut removed = Vec::new(); + let retained = taints + .into_iter() + .filter(|taint| { + let hit = parsed.iter().find(|(key, effect)| { + taint.key == *key && effect.map(|e| e == taint.effect).unwrap_or(true) + }); + match hit { + Some((key, effect)) => { + let label = match effect { + Some(e) => format!("{key}:{e}"), + None => (*key).to_string(), + }; + removed.push(label); + false + } + None => true, + } + }) + .collect(); + + (retained, removed) +} + // Public API functions that use the client pub async fn get_container_runtime_version(config: &Config) -> Result { let client = K8sClient::new(&config.node_name).await?; @@ -542,6 +639,11 @@ pub async fn label_node( client.label_node(label_key, label_value, overwrite).await } +pub async fn remove_node_taints(config: &Config, matchers: &[String]) -> Result> { + let client = K8sClient::new(&config.node_name).await?; + client.remove_node_taints(matchers).await +} + pub async fn own_daemonset_exists(config: &Config) -> Result { let client = K8sClient::new(&config.node_name).await?; client.own_daemonset_exists(&config.daemonset_name).await @@ -585,3 +687,103 @@ pub async fn update_runtimeclass( let client = K8sClient::new(&config.node_name).await?; client.update_runtimeclass(runtimeclass).await } + +#[cfg(test)] +mod tests { + use super::partition_taints; + use k8s_openapi::api::core::v1::Taint; + use rstest::rstest; + + fn taint(key: &str, effect: &str) -> Taint { + Taint { + key: key.to_string(), + effect: effect.to_string(), + value: None, + time_added: None, + } + } + + fn build(pairs: &[(&str, &str)]) -> Vec { + pairs.iter().map(|(k, e)| taint(k, e)).collect() + } + + fn keys(taints: &[Taint]) -> Vec<(String, String)> { + taints + .iter() + .map(|t| (t.key.clone(), t.effect.clone())) + .collect() + } + + /// `partition_taints` keeps every taint except those matched by a matcher. + /// A bare key matches any effect; `key:effect` matches only that effect; + /// matchers are trimmed; blank matchers and non-matches remove nothing. + #[rstest] + // bare key removes every effect for that key, leaving others untouched + #[case::bare_key_removes_all_effects( + &[("kata.io/not-ready", "NoSchedule"), ("kata.io/not-ready", "NoExecute"), ("other", "NoSchedule")], + &["kata.io/not-ready"], + &[("other", "NoSchedule")], + &["kata.io/not-ready", "kata.io/not-ready"], + )] + // key:effect removes only the matching effect + #[case::key_effect_removes_only_matching_effect( + &[("kata.io/not-ready", "NoSchedule"), ("kata.io/not-ready", "NoExecute")], + &["kata.io/not-ready:NoSchedule"], + &[("kata.io/not-ready", "NoExecute")], + &["kata.io/not-ready:NoSchedule"], + )] + // no matcher matches: everything retained, nothing removed + #[case::no_match_retains_everything( + &[("some-other-taint", "NoSchedule")], + &["kata.io/not-ready"], + &[("some-other-taint", "NoSchedule")], + &[], + )] + // key matches but effect differs: not removed + #[case::effect_mismatch_is_not_removed( + &[("kata.io/not-ready", "NoExecute")], + &["kata.io/not-ready:NoSchedule"], + &[("kata.io/not-ready", "NoExecute")], + &[], + )] + // empty / whitespace-only matchers remove nothing + #[case::blank_matchers_remove_nothing( + &[("kata.io/not-ready", "NoSchedule")], + &["", " "], + &[("kata.io/not-ready", "NoSchedule")], + &[], + )] + // surrounding whitespace in a key:effect matcher is trimmed before matching + #[case::whitespace_around_matcher_is_trimmed( + &[("kata.io/not-ready", "NoSchedule")], + &[" kata.io/not-ready : NoSchedule "], + &[], + &["kata.io/not-ready:NoSchedule"], + )] + fn test_partition_taints( + #[case] taints: &[(&str, &str)], + #[case] matchers: &[&str], + #[case] expected_retained: &[(&str, &str)], + #[case] expected_removed: &[&str], + ) { + let matchers: Vec = matchers.iter().map(|s| s.to_string()).collect(); + let (retained, removed) = partition_taints(build(taints), &matchers); + + assert_eq!( + keys(&retained), + build(expected_retained) + .iter() + .map(|t| (t.key.clone(), t.effect.clone())) + .collect::>(), + "retained taints mismatch", + ); + assert_eq!( + removed, + expected_removed + .iter() + .map(|s| s.to_string()) + .collect::>(), + "removed labels mismatch", + ); + } +} diff --git a/tools/packaging/kata-deploy/binary/src/main.rs b/tools/packaging/kata-deploy/binary/src/main.rs index d3cdde3d20..baee18489b 100644 --- a/tools/packaging/kata-deploy/binary/src/main.rs +++ b/tools/packaging/kata-deploy/binary/src/main.rs @@ -497,6 +497,13 @@ async fn install_stage_cri(config: &config::Config, runtime: &str) -> Result<()> /// Install stage 3 (label): apply the kata-runtime node label. Unprivileged, /// Kubernetes API only. Skips re-applying when the label is already correct. +/// +/// As the very last action, once the label is confirmed present, remove any +/// configured startup taints (`STARTUP_TAINTS`). This is what makes the +/// scheduling handshake safe: a node can be provisioned with a startup taint +/// that keeps kata workloads off it until the runtime exists, and that taint is +/// only lifted here, strictly after artifacts are installed, the CRI runtime is +/// configured and restarted, and the node is labeled kata-capable. async fn install_stage_label(config: &config::Config) -> Result<()> { info!("install (label): applying node label"); @@ -506,18 +513,62 @@ async fn install_stage_label(config: &config::Config) -> Result<()> { "install (label): node already labeled {}=true, skipping", KATA_RUNTIME_LABEL ); - return Ok(()); } // Any other state (absent, different value, or a transient read error) // falls through to label_node_with_retry, which applies and verifies. - _ => {} + _ => { + label_node_with_retry(config, KATA_RUNTIME_LABEL, "true").await?; + } } - label_node_with_retry(config, KATA_RUNTIME_LABEL, "true").await?; + remove_startup_taints(config).await; Ok(()) } +/// Remove the configured startup taints from this node, if any. +/// +/// Best-effort by design: failing to remove a taint must not fail the install +/// (the runtime is already in place and the node is labeled). We log a warning +/// and let the next reconcile/retry try again. Leaving the taint in place is the +/// safe failure mode, since it only keeps workloads off the node rather than +/// admitting them prematurely. +async fn remove_startup_taints(config: &config::Config) { + if config.startup_taints.is_empty() { + return; + } + + info!( + "install (label): removing startup taint(s): {}", + config.startup_taints.join(", ") + ); + + match k8s::remove_node_taints(config, &config.startup_taints).await { + Ok(removed) if removed.is_empty() => { + info!( + "install (label): no matching startup taint present on node {} (nothing to remove)", + config.node_name + ); + } + Ok(removed) => { + info!( + "install (label): removed startup taint(s) [{}] from node {}", + removed.join(", "), + config.node_name + ); + } + Err(e) => { + log::warn!( + "install (label): failed to remove startup taint(s) [{}] from node {}: {}; \ + leaving them in place (workloads stay gated). Will retry on next install run.", + config.startup_taints.join(", "), + config.node_name, + e + ); + } + } +} + /// Label the node and verify the label sticks, retrying if necessary. /// /// On rke2/k3s a CRI restart also restarts the kubelet, and `wait_till_node_is_ready` diff --git a/tools/packaging/kata-deploy/helm-chart/kata-deploy/templates/_helpers.tpl b/tools/packaging/kata-deploy/helm-chart/kata-deploy/templates/_helpers.tpl index aa6678df30..21b3ea7037 100644 --- a/tools/packaging/kata-deploy/helm-chart/kata-deploy/templates/_helpers.tpl +++ b/tools/packaging/kata-deploy/helm-chart/kata-deploy/templates/_helpers.tpl @@ -625,6 +625,10 @@ e.g. `{{- include "kata-deploy.commonEnv" . | nindent 8 }}`. - name: CUSTOM_RUNTIMES_ENABLED value: "true" {{- end }} +{{- with .Values.startupTaints }} +- name: STARTUP_TAINTS + value: {{ join "," . | quote }} +{{- end }} {{- end -}} {{/* diff --git a/tools/packaging/kata-deploy/helm-chart/kata-deploy/values.yaml b/tools/packaging/kata-deploy/helm-chart/kata-deploy/values.yaml index 4bbeb2341f..3a12ae30af 100644 --- a/tools/packaging/kata-deploy/helm-chart/kata-deploy/values.yaml +++ b/tools/packaging/kata-deploy/helm-chart/kata-deploy/values.yaml @@ -170,6 +170,34 @@ containerd: nodeSelector: {} tolerations: [] +# Startup taints to remove from a node once Kata is installed on it and the node +# has been labeled katacontainers.io/kata-runtime=true (the final install step). +# +# This closes a scheduling race: kata-deploy installs the runtime asynchronously +# via a DaemonSet (or per-node Jobs), so on a freshly provisioned node a pod that +# requests a Kata RuntimeClass can be scheduled before the runtime binaries exist +# and fail to start. Provision such nodes with a startup taint that keeps Kata +# workloads off them, list that taint here, and kata-deploy will remove it only +# after the runtime is ready, at which point the scheduler admits the workloads. +# +# Each entry is either a bare taint key (removes that key for ANY effect) or +# "key:effect" (removes only the taint with that key AND effect). Removal is +# best-effort and idempotent: a taint that is already absent is simply skipped, +# and a failure to remove leaves the taint in place (workloads stay gated) rather +# than admitting them prematurely. +# +# For this to work, kata-deploy itself must tolerate the taint so its own pods can +# land on the node. Add a matching entry under `tolerations` above. +# +# Example (pairs with a Karpenter NodePool `startupTaints` entry of the same key): +# tolerations: +# - key: "katacontainers.io/kata-runtime" +# operator: "Exists" +# effect: "NoSchedule" +# startupTaints: +# - "katacontainers.io/kata-runtime:NoSchedule" +startupTaints: [] + # Priority class name for the kata-deploy DaemonSet pods. # # kata-deploy is an infrastructure DaemonSet that installs Kata runtime