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
@@ -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)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user