fix: parse_memory_limit accepts Ki/Mi/Gi IEC binary suffixes

The libpod HTTP API path (PodmanClient::create_container) ran manifest
memory_limit values like "128Mi" through parse_memory_limit which
lowercased+trim_end_matches("m"), leaving "128i" which parse::<f64>()
rejected. The resulting None became 0 via .unwrap_or(0), and podman
serialised that into the OCI config as memory.limit:0. At container
start time systemd then rejected MemoryMax=0 with "Value specified in
MemoryMax is out of range".

Silently wrong for every manifest in apps/ that uses Kubernetes-style
suffixes (all of them). Became visible on .228 when Step 9 first
exercised the ProdContainerOrchestrator path for bitcoin-ui and lnd-ui
installs \u2014 the old first-boot-containers.sh bash script used podman
run --memory 128m directly, which podman-the-CLI parses correctly, so
the bug never surfaced before.

Two parts:
- parse_memory_limit now recognises Ki/Mi/Gi/Ti (IEC binary, what k8s
  and our manifests use), kB/MB/GB/TB (SI decimal), k/K/m/M/g/G/t/T
  (docker shorthand, treated as IEC binary for backwards compat), and
  bare byte integers. Filters out zero/negative results.
- create_container omits the memory/cpu fields entirely when the
  manifest has no limit or parsing fails, rather than emitting 0. The
  libpod API treats absent as unlimited; 0 is "set MemoryMax=0" which
  systemd rightly rejects. Defence in depth against the next weird
  suffix someone puts in a manifest.

Six regression tests in the new tests module cover IEC, SI, shorthand,
raw bytes, invalid input (empty/garbage/0/negative), and whitespace.
This commit is contained in:
archipelago 2026-04-23 03:44:23 -04:00
parent c396be8068
commit 8acf7d1112

View File

@ -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::<Vec<_>>(),
"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<String> {
}
fn parse_memory_limit(limit: &str) -> Option<i64> {
let limit = limit.trim().to_lowercase();
if limit.ends_with('g') {
limit
.trim_end_matches('g')
.parse::<f64>()
.ok()
.map(|v| (v * 1_073_741_824.0) as i64)
} else if limit.ends_with('m') {
limit
.trim_end_matches('m')
.parse::<f64>()
.ok()
.map(|v| (v * 1_048_576.0) as i64)
} else if limit.ends_with('k') {
limit
.trim_end_matches('k')
.parse::<f64>()
.ok()
.map(|v| (v * 1024.0) as i64)
} else {
limit.parse::<i64>().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::<f64> 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::<f64>()
.ok()
.map(|v| (v * (*multiplier as f64)) as i64)
.filter(|n| *n > 0);
}
}
// No recognised suffix — treat as raw bytes.
trimmed.parse::<i64>().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::<f64>
// 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)
);
}
}