diff --git a/core/container/src/podman_client.rs b/core/container/src/podman_client.rs index d81759d0..6ee0f707 100644 --- a/core/container/src/podman_client.rs +++ b/core/container/src/podman_client.rs @@ -313,6 +313,33 @@ impl PodmanClient { ) })?; + // Build resource_limits conditionally: if the manifest has no memory or + // cpu limit, OMIT the field entirely rather than sending 0. The podman + // libpod HTTP API treats `memory.limit: 0` as "set MemoryMax=0" which + // systemd then rejects at container-start time. Absent = unlimited. + let mut resource_limits = serde_json::Map::new(); + if let Some(mem_bytes) = manifest + .app + .resources + .memory_limit + .as_ref() + .and_then(|m| parse_memory_limit(m)) + { + resource_limits.insert( + "memory".to_string(), + serde_json::json!({ "limit": mem_bytes }), + ); + } + if let Some(cpu) = manifest.app.resources.cpu_limit { + resource_limits.insert( + "cpu".to_string(), + serde_json::json!({ + "quota": (cpu as i64) * 100_000, + "period": 100_000u64, + }), + ); + } + let body = serde_json::json!({ "name": name, "image": image_ref, @@ -323,19 +350,7 @@ impl PodmanClient { "devices": manifest.app.devices.iter().map(|d| { serde_json::json!({"path": d}) }).collect::>(), - "resource_limits": { - "memory": { - "limit": manifest.app.resources.memory_limit.as_ref() - .and_then(|m| parse_memory_limit(m)) - .unwrap_or(0), - }, - "cpu": { - "quota": manifest.app.resources.cpu_limit - .map(|c| (c as i64) * 100000) - .unwrap_or(0), - "period": 100000u64, - } - }, + "resource_limits": resource_limits, "cap_add": cap_add, "cap_drop": cap_drop, "read_only_filesystem": manifest.app.security.readonly_root, @@ -578,26 +593,106 @@ fn parse_port_bindings(bindings: &serde_json::Value) -> Vec { } fn parse_memory_limit(limit: &str) -> Option { - let limit = limit.trim().to_lowercase(); - if limit.ends_with('g') { - limit - .trim_end_matches('g') - .parse::() - .ok() - .map(|v| (v * 1_073_741_824.0) as i64) - } else if limit.ends_with('m') { - limit - .trim_end_matches('m') - .parse::() - .ok() - .map(|v| (v * 1_048_576.0) as i64) - } else if limit.ends_with('k') { - limit - .trim_end_matches('k') - .parse::() - .ok() - .map(|v| (v * 1024.0) as i64) - } else { - limit.parse::().ok() + // Supports the Kubernetes-style suffixes used throughout apps/*/manifest.yml + // (IEC binary: Ki/Mi/Gi/Ti) as well as the shorter docker-style k/m/g/t. + // Longest suffix matched first so "Mi" isn't mis-matched as "m". + // + // Historical bug: we used to lowercase+trim_end_matches('m'), which turned + // "128Mi" into "128i" → parse:: failed → None → .unwrap_or(0) wrote + // memory.limit:0 into the OCI spec, which systemd then rejected at start + // time with "MemoryMax is out of range" on rootless podman. See + // docs/rust-orchestrator-migration.md Step 9 notes. + let trimmed = limit.trim(); + if trimmed.is_empty() { + return None; + } + const UNITS: &[(&str, i64)] = &[ + ("Ki", 1024), + ("Mi", 1024 * 1024), + ("Gi", 1024 * 1024 * 1024), + ("Ti", 1024i64 * 1024 * 1024 * 1024), + ("kB", 1000), + ("MB", 1_000_000), + ("GB", 1_000_000_000), + ("TB", 1_000_000_000_000), + ("k", 1024), + ("K", 1024), + ("m", 1024 * 1024), + ("M", 1024 * 1024), + ("g", 1024 * 1024 * 1024), + ("G", 1024 * 1024 * 1024), + ("t", 1024i64 * 1024 * 1024 * 1024), + ("T", 1024i64 * 1024 * 1024 * 1024), + ("b", 1), + ("B", 1), + ]; + for (suffix, multiplier) in UNITS { + if let Some(num) = trimmed.strip_suffix(suffix) { + let num = num.trim(); + return num + .parse::() + .ok() + .map(|v| (v * (*multiplier as f64)) as i64) + .filter(|n| *n > 0); + } + } + // No recognised suffix — treat as raw bytes. + trimmed.parse::().ok().filter(|n| *n > 0) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_memory_limit_iec_binary_suffixes() { + // Kubernetes-style — this is what apps/*/manifest.yml uses. + assert_eq!(parse_memory_limit("128Mi"), Some(128 * 1024 * 1024)); + assert_eq!(parse_memory_limit("64Mi"), Some(64 * 1024 * 1024)); + assert_eq!(parse_memory_limit("4Gi"), Some(4i64 * 1024 * 1024 * 1024)); + assert_eq!(parse_memory_limit("512Ki"), Some(512 * 1024)); + } + + #[test] + fn parse_memory_limit_shorthand_suffixes() { + // Docker-style shorthand — treated as IEC binary for backwards compat. + assert_eq!(parse_memory_limit("128m"), Some(128 * 1024 * 1024)); + assert_eq!(parse_memory_limit("128M"), Some(128 * 1024 * 1024)); + assert_eq!(parse_memory_limit("2g"), Some(2i64 * 1024 * 1024 * 1024)); + assert_eq!(parse_memory_limit("2G"), Some(2i64 * 1024 * 1024 * 1024)); + } + + #[test] + fn parse_memory_limit_si_decimal_suffixes() { + assert_eq!(parse_memory_limit("1MB"), Some(1_000_000)); + assert_eq!(parse_memory_limit("1GB"), Some(1_000_000_000)); + } + + #[test] + fn parse_memory_limit_raw_bytes() { + assert_eq!(parse_memory_limit("134217728"), Some(134_217_728)); + assert_eq!(parse_memory_limit(" 134217728 "), Some(134_217_728)); + } + + #[test] + fn parse_memory_limit_invalid_returns_none() { + // Regression guard: the old implementation returned Some(0) for "128Mi" + // because lowercase+trim_end_matches('m') left "128i" which parse:: + // rejected. The new implementation must never return Some(0) or Some of + // a negative number from any input. + assert_eq!(parse_memory_limit(""), None); + assert_eq!(parse_memory_limit(" "), None); + assert_eq!(parse_memory_limit("abc"), None); + assert_eq!(parse_memory_limit("0"), None); + assert_eq!(parse_memory_limit("0Mi"), None); + assert_eq!(parse_memory_limit("-1Mi"), None); + } + + #[test] + fn parse_memory_limit_tolerates_whitespace_and_fractional() { + assert_eq!( + parse_memory_limit(" 1.5Gi "), + Some((1.5 * (1024.0 * 1024.0 * 1024.0)) as i64) + ); } }