diff --git a/Cargo.lock b/Cargo.lock index 98c3d01..380803b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -30,6 +30,15 @@ version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" +[[package]] +name = "android_system_properties" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311" +dependencies = [ + "libc", +] + [[package]] name = "anstream" version = "0.6.21" @@ -233,6 +242,12 @@ dependencies = [ "generic-array", ] +[[package]] +name = "bumpalo" +version = "3.20.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d20789868f4b01b2f2caec9f5c4e0213b41e3e5702a50157d699ae31ced2fcb" + [[package]] name = "byteorder" version = "1.5.0" @@ -298,8 +313,12 @@ version = "0.4.44" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c673075a2e0e5f4a1dde27ce9dee1ea4558c7ffe648f576438a20ca1d2acc4b0" dependencies = [ + "iana-time-zone", + "js-sys", "num-traits", "serde", + "wasm-bindgen", + "windows-link", ] [[package]] @@ -1106,6 +1125,30 @@ dependencies = [ "tracing", ] +[[package]] +name = "iana-time-zone" +version = "0.1.65" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e31bc9ad994ba00e440a8aa5c9ef0ec67d5cb5e5cb0cc7f8b744a35b389cc470" +dependencies = [ + "android_system_properties", + "core-foundation-sys", + "iana-time-zone-haiku", + "js-sys", + "log", + "wasm-bindgen", + "windows-core", +] + +[[package]] +name = "iana-time-zone-haiku" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f31827a206f56af32e590ba56d5d2d085f558508192593743f16b2306495269f" +dependencies = [ + "cc", +] + [[package]] name = "id-arena" version = "2.3.0" @@ -1170,6 +1213,16 @@ version = "1.0.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "92ecc6618181def0457392ccd0ee51198e065e016d1d527a7ac1b6dc7c1f09d2" +[[package]] +name = "js-sys" +version = "0.3.91" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b49715b7073f385ba4bc528e5747d02e66cb39c6146efb66b781f131f0fb399c" +dependencies = [ + "once_cell", + "wasm-bindgen", +] + [[package]] name = "json-patch" version = "3.0.1" @@ -1985,6 +2038,7 @@ dependencies = [ "anyhow", "async-trait", "axum", + "chrono", "clap", "containerd-shim", "containerd-shim-protos", @@ -2964,6 +3018,51 @@ dependencies = [ "wit-bindgen", ] +[[package]] +name = "wasm-bindgen" +version = "0.2.114" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6532f9a5c1ece3798cb1c2cfdba640b9b3ba884f5db45973a6f442510a87d38e" +dependencies = [ + "cfg-if 1.0.4", + "once_cell", + "rustversion", + "wasm-bindgen-macro", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-macro" +version = "0.2.114" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "18a2d50fcf105fb33bb15f00e7a77b772945a2ee45dcf454961fd843e74c18e6" +dependencies = [ + "quote", + "wasm-bindgen-macro-support", +] + +[[package]] +name = "wasm-bindgen-macro-support" +version = "0.2.114" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "03ce4caeaac547cdf713d280eda22a730824dd11e6b8c3ca9e42247b25c631e3" +dependencies = [ + "bumpalo", + "proc-macro2", + "quote", + "syn 2.0.117", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-shared" +version = "0.2.114" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75a326b8c223ee17883a4251907455a2431acc2791c98c26279376490c378c16" +dependencies = [ + "unicode-ident", +] + [[package]] name = "wasm-encoder" version = "0.244.0" @@ -3044,12 +3143,65 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows-core" +version = "0.62.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8e83a14d34d0623b51dce9581199302a221863196a1dde71a7663a4c2be9deb" +dependencies = [ + "windows-implement", + "windows-interface", + "windows-link", + "windows-result", + "windows-strings", +] + +[[package]] +name = "windows-implement" +version = "0.60.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + +[[package]] +name = "windows-interface" +version = "0.59.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f316c4a2570ba26bbec722032c4099d8c8bc095efccdc15688708623367e358" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "windows-link" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-result" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7781fa89eaf60850ac3d2da7af8e5242a5ea78d1a11c49bf2910bb5a73853eb5" +dependencies = [ + "windows-link", +] + +[[package]] +name = "windows-strings" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7837d08f69c77cf6b07689544538e017c1bfcf57e34b4c0ff58e6c2cd3b37091" +dependencies = [ + "windows-link", +] + [[package]] name = "windows-sys" version = "0.48.0" diff --git a/Cargo.toml b/Cargo.toml index da6fc62..ce00b46 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,9 +29,10 @@ k8s-openapi = { version = "0.24", features = ["v1_31"], optional = true } prometheus-client = { version = "0.23", optional = true } axum = { version = "0.8", optional = true } futures = { version = "0.3", optional = true } +chrono = { version = "0.4", optional = true } [features] -agent = ["kube", "k8s-openapi", "prometheus-client", "axum", "futures"] +agent = ["kube", "k8s-openapi", "prometheus-client", "axum", "futures", "chrono"] [dev-dependencies] tempfile = "3" diff --git a/deploy/kubernetes/reaper-agent.yaml b/deploy/kubernetes/reaper-agent.yaml index 5320cc4..b8fa570 100644 --- a/deploy/kubernetes/reaper-agent.yaml +++ b/deploy/kubernetes/reaper-agent.yaml @@ -46,6 +46,12 @@ rules: - apiGroups: [""] resources: ["namespaces"] verbs: ["get", "list"] + - apiGroups: [""] + resources: ["nodes"] + verbs: ["get"] + - apiGroups: [""] + resources: ["nodes/status"] + verbs: ["patch"] --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding @@ -102,6 +108,11 @@ spec: imagePullPolicy: IfNotPresent securityContext: runAsUser: 0 + env: + - name: NODE_NAME + valueFrom: + fieldRef: + fieldPath: spec.nodeName args: - --config-namespace=reaper-system - --config-name=reaper-config diff --git a/docs/TODO.md b/docs/TODO.md index 3a55916..f340cba 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -22,6 +22,6 @@ List of tasks to do, not ordered in any specific way. - [x] Evaluate if Reaper can be configured using a Kubernetes ConfigMap instead of relying on a node-level config file. (Implemented via `reaper-agent` DaemonSet — PR #27) - [x] reaper-agent Phase 2: Overlay GC — reconcile overlay namespaces against Kubernetes API, delete overlays for namespaces that no longer exist - [ ] reaper-agent Phase 2: Binary self-update — watch ConfigMap version field, download and replace shim/runtime binaries -- [ ] reaper-agent Phase 2: Node condition reporting — patch Node object with `ReaperReady` condition +- [x] reaper-agent Phase 2: Node condition reporting — patch Node object with `ReaperReady` condition - [x] reaper-agent Phase 2: Mount namespace cleanup — detect and unmount stale `/run/reaper/ns/*` bind-mounts - [ ] Fix known bugs documented in [docs/BUGS.md](BUGS.md) \ No newline at end of file diff --git a/scripts/lib/test-integration-suite.sh b/scripts/lib/test-integration-suite.sh index 8abf48e..4e7791b 100644 --- a/scripts/lib/test-integration-suite.sh +++ b/scripts/lib/test-integration-suite.sh @@ -2504,6 +2504,136 @@ test_agent_ns_cleanup_metrics() { log_verbose "ns cleanup metrics verified: all expected metrics present" } +test_agent_node_condition_set() { + # The agent should have patched the node with a ReaperReady condition. + # Wait for the condition to appear (initial patch happens at startup). + local node_name + node_name=$(kubectl get nodes -o jsonpath='{.items[0].metadata.name}' 2>/dev/null) + + if [[ -z "$node_name" ]]; then + log_error "Could not determine node name" + return 1 + fi + + local max_wait=60 + for i in $(seq 1 "$max_wait"); do + local condition_status + condition_status=$(kubectl get node "$node_name" \ + -o jsonpath='{.status.conditions[?(@.type=="ReaperReady")].status}' 2>/dev/null || true) + + if [[ "$condition_status" == "True" ]]; then + log_verbose "ReaperReady condition is True on node $node_name" + return 0 + fi + sleep 1 + done + + log_error "ReaperReady condition not set to True within ${max_wait}s" + kubectl get node "$node_name" -o jsonpath='{.status.conditions}' >> "$LOG_FILE" 2>&1 || true + return 1 +} + +test_agent_node_condition_reflects_health() { + # Temporarily rename the shim binary to make health check fail, + # then verify the condition transitions to False, then restore and verify True. + local node_name + node_name=$(kubectl get nodes -o jsonpath='{.items[0].metadata.name}' 2>/dev/null) + + if [[ -z "$node_name" ]]; then + log_error "Could not determine node name" + return 1 + fi + + # Break the shim binary (rename it) + docker exec "$NODE_ID" mv /usr/local/bin/containerd-shim-reaper-v2 \ + /usr/local/bin/containerd-shim-reaper-v2.bak >> "$LOG_FILE" 2>&1 + + # Wait for condition to flip to False (node condition interval = 30s in defaults, + # but the agent re-evaluates health each cycle) + local max_wait=90 + local flipped_false=false + for i in $(seq 1 "$max_wait"); do + local condition_status + condition_status=$(kubectl get node "$node_name" \ + -o jsonpath='{.status.conditions[?(@.type=="ReaperReady")].status}' 2>/dev/null || true) + + if [[ "$condition_status" == "False" ]]; then + log_verbose "ReaperReady condition flipped to False after shim removal" + flipped_false=true + break + fi + sleep 1 + done + + # Restore the shim binary immediately + docker exec "$NODE_ID" mv /usr/local/bin/containerd-shim-reaper-v2.bak \ + /usr/local/bin/containerd-shim-reaper-v2 >> "$LOG_FILE" 2>&1 + + if ! $flipped_false; then + log_error "ReaperReady condition did not flip to False within ${max_wait}s after shim removal" + return 1 + fi + + # Wait for condition to return to True + for i in $(seq 1 "$max_wait"); do + local condition_status + condition_status=$(kubectl get node "$node_name" \ + -o jsonpath='{.status.conditions[?(@.type=="ReaperReady")].status}' 2>/dev/null || true) + + if [[ "$condition_status" == "True" ]]; then + log_verbose "ReaperReady condition returned to True after shim restore" + return 0 + fi + sleep 1 + done + + log_error "ReaperReady condition did not return to True within ${max_wait}s after shim restore" + return 1 +} + +test_agent_node_condition_metrics() { + local agent_pod + agent_pod=$(kubectl get pods -n reaper-system -l app.kubernetes.io/name=reaper-agent \ + -o jsonpath='{.items[0].metadata.name}' 2>/dev/null) + + if [[ -z "$agent_pod" ]]; then + log_error "No reaper-agent pod found" + return 1 + fi + + # Use port-forward to reach the metrics endpoint + local local_port=19104 + kubectl port-forward -n reaper-system "$agent_pod" ${local_port}:9100 >> "$LOG_FILE" 2>&1 & + local pf_pid=$! + sleep 2 + + local metrics_response + metrics_response=$(curl -sf http://localhost:${local_port}/metrics 2>/dev/null || echo "FAILED") + + kill "$pf_pid" 2>/dev/null || true + wait "$pf_pid" 2>/dev/null || true + + if [[ "$metrics_response" == "FAILED" ]]; then + log_error "Failed to fetch metrics from agent" + return 1 + fi + + local missing=() + for metric in reaper_agent_node_condition_updates_total reaper_agent_node_condition_healthy; do + if ! echo "$metrics_response" | grep -q "$metric"; then + missing+=("$metric") + fi + done + + if [[ ${#missing[@]} -gt 0 ]]; then + log_error "Missing node condition metrics: ${missing[*]}" + log_error "Metrics output: $metrics_response" + return 1 + fi + + log_verbose "node condition metrics verified: all expected metrics present" +} + cleanup_agent() { kubectl delete -f deploy/kubernetes/reaper-agent.yaml --ignore-not-found >> "$LOG_FILE" 2>&1 || true docker exec "$NODE_ID" rm -rf /run/reaper/stale-gc-test >> "$LOG_FILE" 2>&1 || true @@ -2555,6 +2685,9 @@ phase_agent_tests() { run_test test_agent_ns_cleanup_metrics "Agent ns cleanup metrics" --hard-fail run_test test_agent_ns_cleanup_stale_file "Agent ns cleanup stale file" --hard-fail run_test test_agent_ns_cleanup_preserves_active "Agent ns cleanup preserves active" --hard-fail + run_test test_agent_node_condition_set "Agent node condition ReaperReady set" --hard-fail + run_test test_agent_node_condition_reflects_health "Agent node condition reflects health" --hard-fail + run_test test_agent_node_condition_metrics "Agent node condition metrics" --hard-fail # Cleanup agent resources cleanup_agent diff --git a/src/bin/reaper-agent/main.rs b/src/bin/reaper-agent/main.rs index 5af5206..cb6b7bb 100644 --- a/src/bin/reaper-agent/main.rs +++ b/src/bin/reaper-agent/main.rs @@ -8,6 +8,7 @@ mod config_sync; mod gc; mod health; mod metrics; +mod node_condition; mod overlay_gc; // config.rs is available as shared module but not needed by the agent @@ -95,6 +96,26 @@ struct Cli { env = "REAPER_AGENT_RUNTIME_PATH" )] runtime_path: String, + + /// Enable node condition reporting (patch Node with ReaperReady condition) + #[arg( + long, + default_value = "true", + env = "REAPER_AGENT_NODE_CONDITION_ENABLED" + )] + node_condition_enabled: bool, + + /// Node condition update interval in seconds + #[arg( + long, + default_value = "30", + env = "REAPER_AGENT_NODE_CONDITION_INTERVAL" + )] + node_condition_interval: u64, + + /// Node name (set via downward API). Required when node condition reporting is enabled. + #[arg(long, env = "NODE_NAME")] + node_name: Option, } #[tokio::main] @@ -155,6 +176,38 @@ async fn main() -> anyhow::Result<()> { } }); + // Spawn node condition reporting loop (patch Node with ReaperReady condition) + let node_condition_handle = if cli.node_condition_enabled { + match &cli.node_name { + Some(name) => { + let nc_node = name.clone(); + let nc_shim = cli.shim_path.clone(); + let nc_runtime = cli.runtime_path.clone(); + let nc_state_dir = cli.state_dir.clone(); + let nc_metrics = metrics_state.clone(); + let nc_interval = cli.node_condition_interval; + Some(tokio::spawn(async move { + node_condition::node_condition_loop( + &nc_node, + &nc_shim, + &nc_runtime, + &nc_state_dir, + nc_interval, + &nc_metrics, + ) + .await; + })) + } + None => { + error!("node condition reporting enabled but NODE_NAME not set; disable with --node-condition-enabled=false or set NODE_NAME via downward API"); + None + } + } + } else { + info!("node condition reporting disabled via --node-condition-enabled=false"); + None + }; + // Spawn overlay GC loop (reconcile overlay dirs against K8s namespaces) let overlay_gc_handle = if cli.overlay_gc_enabled { let ogc_state_dir = cli.state_dir.clone(); @@ -197,6 +250,9 @@ async fn main() -> anyhow::Result<()> { health_handle.abort(); sync_handle.abort(); server_handle.abort(); + if let Some(h) = node_condition_handle { + h.abort(); + } if let Some(h) = overlay_gc_handle { h.abort(); } diff --git a/src/bin/reaper-agent/metrics.rs b/src/bin/reaper-agent/metrics.rs index b0441f8..a42ec29 100644 --- a/src/bin/reaper-agent/metrics.rs +++ b/src/bin/reaper-agent/metrics.rs @@ -38,7 +38,12 @@ struct MetricsInner { // Namespace cleanup metrics ns_cleanup_runs_total: Counter, + #[allow(dead_code)] // used on Linux only (run_ns_cleanup) ns_cleaned_total: Counter, + + // Node condition reporting metrics + node_condition_updates_total: Counter, + node_condition_healthy: Gauge, } impl MetricsState { @@ -56,6 +61,8 @@ impl MetricsState { let overlay_namespaces = Gauge::default(); let ns_cleanup_runs_total = Counter::default(); let ns_cleaned_total = Counter::default(); + let node_condition_updates_total = Counter::default(); + let node_condition_healthy = Gauge::default(); registry.register( "reaper_containers_created", @@ -112,6 +119,16 @@ impl MetricsState { "Total number of stale namespace bind-mount files removed", ns_cleaned_total.clone(), ); + registry.register( + "reaper_agent_node_condition_updates_total", + "Total number of node condition patch operations", + node_condition_updates_total.clone(), + ); + registry.register( + "reaper_agent_node_condition_healthy", + "Whether the last node condition patch reported healthy (1=healthy, 0=unhealthy)", + node_condition_healthy.clone(), + ); Self { inner: Arc::new(MetricsInner { @@ -127,6 +144,8 @@ impl MetricsState { overlay_namespaces, ns_cleanup_runs_total, ns_cleaned_total, + node_condition_updates_total, + node_condition_healthy, }), } } @@ -168,12 +187,23 @@ impl MetricsState { self.inner.ns_cleanup_runs_total.inc(); } + #[allow(dead_code)] // used on Linux only (run_ns_cleanup) pub fn inc_ns_cleaned(&self, count: u64) { for _ in 0..count { self.inner.ns_cleaned_total.inc(); } } + pub fn inc_node_condition_updates(&self) { + self.inner.node_condition_updates_total.inc(); + } + + pub fn set_node_condition_healthy(&self, healthy: bool) { + self.inner + .node_condition_healthy + .set(if healthy { 1 } else { 0 }); + } + pub fn set_overlay_namespaces(&self, count: u64) { self.inner.overlay_namespaces.set(count as i64); } diff --git a/src/bin/reaper-agent/node_condition.rs b/src/bin/reaper-agent/node_condition.rs new file mode 100644 index 0000000..2862b6d --- /dev/null +++ b/src/bin/reaper-agent/node_condition.rs @@ -0,0 +1,142 @@ +use anyhow::{Context, Result}; +use k8s_openapi::api::core::v1::Node; +use k8s_openapi::apimachinery::pkg::apis::meta::v1::Time; +use kube::{api::Api, Client}; +use serde_json::json; +use tracing::{debug, error, info, warn}; + +use crate::health; +use crate::metrics::MetricsState; + +const CONDITION_TYPE: &str = "ReaperReady"; + +/// Patch the Node's status conditions with the current ReaperReady state. +/// +/// Uses a strategic merge patch on the `/status` subresource so that the +/// `ReaperReady` condition is upserted by its `type` field without +/// disturbing other conditions (Ready, MemoryPressure, etc.). +async fn patch_node_condition( + api: &Api, + node_name: &str, + healthy: bool, + details: &[String], +) -> Result<()> { + let now = Time(chrono::Utc::now()); + + let (status, reason, message) = if healthy { + ( + "True", + "ReaperHealthy", + "Reaper binaries and state directory are present and healthy".to_string(), + ) + } else { + let msg = if details.is_empty() { + "Reaper health check failed".to_string() + } else { + details.join("; ") + }; + ("False", "ReaperUnhealthy", msg) + }; + + let patch = json!({ + "status": { + "conditions": [{ + "type": CONDITION_TYPE, + "status": status, + "lastHeartbeatTime": now, + "lastTransitionTime": now, + "reason": reason, + "message": message, + }] + } + }); + + let patch_params = kube::api::PatchParams::default(); + api.patch_status( + node_name, + &patch_params, + &kube::api::Patch::Strategic(patch), + ) + .await + .with_context(|| format!("patching node {} status condition", node_name))?; + + debug!( + node = node_name, + status = status, + "patched ReaperReady condition" + ); + + Ok(()) +} + +/// Run a single node condition update cycle. +pub async fn update_node_condition( + client: &Client, + node_name: &str, + shim_path: &str, + runtime_path: &str, + state_dir: &str, + metrics: &MetricsState, +) { + let result = health::check_health(shim_path, runtime_path, state_dir); + let api: Api = Api::all(client.clone()); + + match patch_node_condition(&api, node_name, result.healthy, &result.details).await { + Ok(()) => { + metrics.inc_node_condition_updates(); + metrics.set_node_condition_healthy(result.healthy); + } + Err(e) => { + warn!(error = %e, node = node_name, "failed to patch node condition"); + } + } +} + +/// Run the node condition reporting loop at the configured interval. +pub async fn node_condition_loop( + node_name: &str, + shim_path: &str, + runtime_path: &str, + state_dir: &str, + interval_secs: u64, + metrics: &MetricsState, +) { + let client = match Client::try_default().await { + Ok(c) => c, + Err(e) => { + error!(error = %e, "failed to create Kubernetes client, node condition reporting disabled"); + return; + } + }; + + info!( + node = node_name, + interval_secs = interval_secs, + "node condition reporting starting" + ); + + // Initial patch + update_node_condition( + &client, + node_name, + shim_path, + runtime_path, + state_dir, + metrics, + ) + .await; + + let interval = tokio::time::Duration::from_secs(interval_secs); + loop { + tokio::time::sleep(interval).await; + update_node_condition( + &client, + node_name, + shim_path, + runtime_path, + state_dir, + metrics, + ) + .await; + } +} diff --git a/src/bin/reaper-agent/overlay_gc.rs b/src/bin/reaper-agent/overlay_gc.rs index 5f686af..4ed9fa6 100644 --- a/src/bin/reaper-agent/overlay_gc.rs +++ b/src/bin/reaper-agent/overlay_gc.rs @@ -142,6 +142,7 @@ fn try_lock_nonblocking(lock_path: &Path) -> Option (&str, Option<&str>) { match name.split_once("--") { Some((ns, group)) => (ns, Some(group)), @@ -151,6 +152,7 @@ fn parse_ns_filename(name: &str) -> (&str, Option<&str>) { /// Check whether ANY container state directory has status "running" with a live PID, /// regardless of namespace. Used for the legacy `shared-mnt-ns` file. +#[cfg(target_os = "linux")] fn has_any_running_container(state_dir: &str) -> bool { let base = Path::new(state_dir); let entries = match fs::read_dir(base) {