From ee5aadecc5ecc1094fbca15c0725315f0b1cd6df Mon Sep 17 00:00:00 2001 From: Mikimoto Date: Wed, 2 Sep 2026 06:58:34 +0000 Subject: [PATCH 1/3] fix(up): round fractional cpu limits to what container run accepts `container run --cpus` takes a whole number; Compose allows a fraction. A compose file using `cpus: "0.5"` failed outright: Error: The value '0.5' is invalid for '--cpus ' Rounded up rather than down, because down turns 0.5 into 0, which container also rejects, and a slightly loose cap fails less destructively than none. Reported on stdout, the same way the 200 MiB memory floor already is. Also fixes a quieter instance of the same input on the build path, which read the limit as `Int64(...) ?? 2`. Int64 cannot parse "0.5", so a service asking for half a CPU silently got two - four times its request, with no message. Only the run path failing loudly is why this was noticed. --- .../Commands/ComposeUp.swift | 22 +++++- .../CPULimitClampTests.swift | 70 +++++++++++++++++++ 2 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 Tests/Container-Compose-StaticTests/CPULimitClampTests.swift diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index d42efd7..200982d 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -478,6 +478,18 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { /// anything smaller with a confusing daemon error. Clamp sub-minimum values up /// to 200 MiB so common Docker values like `mem_limit: 128m` keep working. /// Returns the value to pass to `--memory` and whether it was raised. + /// `container run --cpus` takes a whole number, while Compose allows a + /// fraction. Any mapping is lossy, so this rounds up to the smallest + /// expressible limit rather than down: 0.5 would otherwise become 0, which + /// container rejects, and a slightly loose cap fails less destructively than + /// none at all. Callers report the change, as with the memory floor. + static func clampCPULimit(_ value: String) -> (value: String, clamped: Bool) { + guard let cpus = Double(value) else { return (value, false) } + let whole = max(1, Int(cpus.rounded(.up))) + guard Double(whole) != cpus else { return (value, false) } + return (String(whole), true) + } + static func clampMemoryLimit(_ value: String) -> (value: String, clamped: Bool) { let minimumBytes: Int64 = 200 * 1_024 * 1_024 if let bytes = parseMemoryToBytes(value), bytes < minimumBytes { @@ -1075,7 +1087,11 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { // the structured form. Both map to `container run --memory`. `mem_limit` takes // precedence when both are set, matching Docker Compose CLI behaviour. if let cpus = service.deploy?.resources?.limits?.cpus { - runCommandArgs.append(contentsOf: ["--cpus", cpus]) + let (cpuArg, didClamp) = Self.clampCPULimit(cpus) + if didClamp { + print("Note: Service '\(serviceName)' cpus '\(cpus)' is not a whole number; Apple Container needs one, rounding up to \(cpuArg).") + } + runCommandArgs.append(contentsOf: ["--cpus", cpuArg]) } let effectiveMemoryLimit = service.mem_limit ?? service.deploy?.resources?.limits?.memory if let memory = effectiveMemoryLimit { @@ -1354,7 +1370,9 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { commands.append(contentsOf: ["--tag", imageToRun]) // Add CPU & Memory - let cpuCount = Int64(service.deploy?.resources?.limits?.cpus ?? "2") ?? 2 + // Not `Int64(...) ?? 2`: that silently turns a fractional request such as + // "0.5" into 2, quadrupling it, because Int64 cannot parse it. + let cpuCount = Int64(Self.clampCPULimit(service.deploy?.resources?.limits?.cpus ?? "2").value) ?? 2 let memoryLimit = service.deploy?.resources?.limits?.memory ?? "2048MB" commands.append(contentsOf: ["--cpus", "\(cpuCount)"]) commands.append(contentsOf: ["--memory", memoryLimit]) diff --git a/Tests/Container-Compose-StaticTests/CPULimitClampTests.swift b/Tests/Container-Compose-StaticTests/CPULimitClampTests.swift new file mode 100644 index 0000000..9a2508f --- /dev/null +++ b/Tests/Container-Compose-StaticTests/CPULimitClampTests.swift @@ -0,0 +1,70 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2025 Morris Richman and the Container-Compose project authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Testing +import Foundation +@testable import ContainerComposeCore + +/// Companion to the memory floor in `clampMemoryLimit`: `container run --cpus` +/// takes a whole number while Compose allows a fraction, so the value has to be +/// adjusted and the adjustment reported. +@Suite("CPU Limit Clamping") +struct CPULimitClampTests { + + @Test("a fraction rounds up to the smallest expressible limit") + func fractionRoundsUp() { + let (value, clamped) = ComposeUp.clampCPULimit("0.5") + #expect(value == "1") + #expect(clamped) + } + + @Test("rounding is up, never down to zero") + func neverZero() { + // 0.1 down would be 0, which `container run` rejects outright. + #expect(ComposeUp.clampCPULimit("0.1").value == "1") + #expect(ComposeUp.clampCPULimit("0").value == "1") + for (input, expected) in [("1.5", "2"), ("2.5", "3"), ("3.01", "4")] { + #expect(ComposeUp.clampCPULimit(input).value == expected, "\(input)") + } + } + + @Test("whole numbers are passed through untouched and unreported") + func wholeNumbersUnchanged() { + for input in ["1", "2", "4", "16"] { + let (value, clamped) = ComposeUp.clampCPULimit(input) + #expect(value == input, "\(input)") + #expect(clamped == false, "\(input) should not be reported as clamped") + } + } + + @Test("an unparseable value is left alone rather than guessed at") + func unparseablePassesThrough() { + let (value, clamped) = ComposeUp.clampCPULimit("not-a-number") + #expect(value == "not-a-number") + #expect(clamped == false) + } + + /// Regression: the build path read the limit with `Int64(...) ?? 2`, so a + /// fractional request could not be parsed and silently became 2 - four times + /// what a service asking for 0.5 wanted, with no message. The run path failed + /// loudly instead, which is how this was noticed at all. + @Test("a fractional limit does not become the fallback on the build path") + func fractionalDoesNotBecomeFallback() { + let clamped = ComposeUp.clampCPULimit("0.5").value + #expect(Int64(clamped) == 1) + #expect(Int64(clamped) != 2) + } +} From 41dcded807d685396f90a4d0574441d6a4e00f71 Mon Sep 17 00:00:00 2001 From: Mikimoto Date: Thu, 3 Sep 2026 00:49:31 +0000 Subject: [PATCH 2/3] refactor(up): give the builder cpu count a test seam The regression test added with the previous commit called clampCPULimit and asserted on its result, which passes whether or not buildService uses it - mutation-checked: reverting that line to `Int64(...) ?? 2` left the suite green. Second time this shape appeared in this branch. Extracting builderCPUCount also separated two things the old expression conflated: the default for a service that declares no limit, and the fallback for a limit that cannot be parsed. They are now testable apart. --- .../Commands/ComposeUp.swift | 20 ++++++++-- .../CPULimitClampTests.swift | 38 +++++++++++++++++-- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index 200982d..dff0cb4 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -478,6 +478,22 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { /// anything smaller with a confusing daemon error. Clamp sub-minimum values up /// to 200 MiB so common Docker values like `mem_limit: 128m` keep working. /// Returns the value to pass to `--memory` and whether it was raised. + /// The CPU count the image builder is given for a service. + /// + /// Separate from `clampCPULimit` because it also carries the default for a + /// service that declares no limit at all. + /// + /// Not `Int64(limit) ?? 2`: Int64 cannot parse "0.5", so that form silently + /// handed a service asking for half a CPU the fallback of two - four times + /// its request, with nothing printed. The run path rejected the same input + /// loudly, which is the only reason it was noticed. + /// + /// Pure so the mapping is testable; buildService has no seam. + static func builderCPUCount(for service: Service) -> Int64 { + guard let declared = service.deploy?.resources?.limits?.cpus else { return 2 } + return Int64(clampCPULimit(declared).value) ?? 2 + } + /// `container run --cpus` takes a whole number, while Compose allows a /// fraction. Any mapping is lossy, so this rounds up to the smallest /// expressible limit rather than down: 0.5 would otherwise become 0, which @@ -1370,9 +1386,7 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { commands.append(contentsOf: ["--tag", imageToRun]) // Add CPU & Memory - // Not `Int64(...) ?? 2`: that silently turns a fractional request such as - // "0.5" into 2, quadrupling it, because Int64 cannot parse it. - let cpuCount = Int64(Self.clampCPULimit(service.deploy?.resources?.limits?.cpus ?? "2").value) ?? 2 + let cpuCount = Self.builderCPUCount(for: service) let memoryLimit = service.deploy?.resources?.limits?.memory ?? "2048MB" commands.append(contentsOf: ["--cpus", "\(cpuCount)"]) commands.append(contentsOf: ["--memory", memoryLimit]) diff --git a/Tests/Container-Compose-StaticTests/CPULimitClampTests.swift b/Tests/Container-Compose-StaticTests/CPULimitClampTests.swift index 9a2508f..a9cd9b3 100644 --- a/Tests/Container-Compose-StaticTests/CPULimitClampTests.swift +++ b/Tests/Container-Compose-StaticTests/CPULimitClampTests.swift @@ -16,6 +16,7 @@ import Testing import Foundation +@testable import Yams @testable import ContainerComposeCore /// Companion to the memory floor in `clampMemoryLimit`: `container run --cpus` @@ -61,10 +62,39 @@ struct CPULimitClampTests { /// fractional request could not be parsed and silently became 2 - four times /// what a service asking for 0.5 wanted, with no message. The run path failed /// loudly instead, which is how this was noticed at all. + /// An earlier version of this test called clampCPULimit and asserted on its + /// result, which passes whether or not the build path uses it - confirmed by + /// mutation: reverting that line to `Int64(...) ?? 2` left the test green. + /// It now goes through the build path's own function. @Test("a fractional limit does not become the fallback on the build path") - func fractionalDoesNotBecomeFallback() { - let clamped = ComposeUp.clampCPULimit("0.5").value - #expect(Int64(clamped) == 1) - #expect(Int64(clamped) != 2) + func fractionalDoesNotBecomeFallback() throws { + let svc = try YAMLDecoder().decode(Service.self, from: """ + image: alpine + deploy: + resources: + limits: + cpus: "0.5" + """) + let count = ComposeUp.builderCPUCount(for: svc) + #expect(count == 1) + #expect(count != 2, "0.5 must not silently become the fallback of 2") + } + + @Test("a service with no cpu limit gets the builder default") + func noLimitUsesDefault() throws { + let svc = try YAMLDecoder().decode(Service.self, from: "image: alpine") + #expect(ComposeUp.builderCPUCount(for: svc) == 2) + } + + @Test("a whole cpu limit reaches the builder unchanged") + func wholeLimitReachesBuilder() throws { + let svc = try YAMLDecoder().decode(Service.self, from: """ + image: alpine + deploy: + resources: + limits: + cpus: "4" + """) + #expect(ComposeUp.builderCPUCount(for: svc) == 4) } } From 49672c718aa8304bcfb423a8592e5309d09d8430 Mon Sep 17 00:00:00 2001 From: Mikimoto Date: Tue, 15 Sep 2026 06:09:15 +0000 Subject: [PATCH 3/3] fix(up): range-check the cpus value before converting it to Int MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Int(Double)` traps rather than saturating, so `clampCPULimit` crashed the process on three shapes a compose file can legally contain: `cpus: 1e400` parses as infinity, `cpus: nan` as NaN, and `cpus: 1e20` is finite but larger than `Int` can hold — the case an `isFinite` check alone still misses. A value this tool cannot express is now passed through untouched, so `container run` rejects it and names the service. That is a better outcome than a fatal error with no service in it, and it leaves the ordinary path unchanged: 0.5 still rounds to 1, 2.5 to 3, and a whole number is left alone. Six tests, one per shape plus the unchanged path and the builder fallback. --- .../Commands/ComposeUp.swift | 12 +++- .../CPULimitClampTests.swift | 57 +++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index dff0cb4..2eb4b58 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -499,9 +499,17 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { /// expressible limit rather than down: 0.5 would otherwise become 0, which /// container rejects, and a slightly loose cap fails less destructively than /// none at all. Callers report the change, as with the memory floor. + /// `Int(Double)` traps rather than saturating, so the range is checked before + /// the conversion: `cpus: 1e400` parses as infinity, `cpus: nan` as NaN, and + /// `cpus: 1e20` is finite but larger than `Int` can hold. All three crashed + /// here. A value this tool cannot express is passed through untouched so the + /// runtime rejects it by name, which is a better outcome than a fatal error + /// with no service in it. static func clampCPULimit(_ value: String) -> (value: String, clamped: Bool) { - guard let cpus = Double(value) else { return (value, false) } - let whole = max(1, Int(cpus.rounded(.up))) + guard let cpus = Double(value), cpus.isFinite else { return (value, false) } + let rounded = cpus.rounded(.up) + guard rounded >= Double(Int.min), rounded <= Double(Int.max) else { return (value, false) } + let whole = max(1, Int(rounded)) guard Double(whole) != cpus else { return (value, false) } return (String(whole), true) } diff --git a/Tests/Container-Compose-StaticTests/CPULimitClampTests.swift b/Tests/Container-Compose-StaticTests/CPULimitClampTests.swift index a9cd9b3..a09f884 100644 --- a/Tests/Container-Compose-StaticTests/CPULimitClampTests.swift +++ b/Tests/Container-Compose-StaticTests/CPULimitClampTests.swift @@ -98,3 +98,60 @@ struct CPULimitClampTests { #expect(ComposeUp.builderCPUCount(for: svc) == 4) } } + +/// `Int(Double)` traps instead of saturating, so every value that reaches the +/// conversion has to be range-checked first. Each case below crashed the process +/// before that check existed. +@Suite("CPU Limit Range Validation") +struct CPULimitRangeValidationTests { + + @Test("an infinite cpus value is passed through rather than crashing") + func infiniteIsPassedThrough() { + let (value, clamped) = ComposeUp.clampCPULimit("1e400") + #expect(value == "1e400") + #expect(!clamped) + } + + @Test("a NaN cpus value is passed through rather than crashing") + func nanIsPassedThrough() { + let (value, clamped) = ComposeUp.clampCPULimit("nan") + #expect(value == "nan") + #expect(!clamped) + } + + /// Finite, but larger than `Int` can hold — the case `isFinite` alone misses. + @Test("a finite value beyond Int's range is passed through rather than crashing") + func beyondIntRangeIsPassedThrough() { + let (value, clamped) = ComposeUp.clampCPULimit("1e20") + #expect(value == "1e20") + #expect(!clamped) + } + + @Test("a non-numeric cpus value is still passed through") + func nonNumericIsPassedThrough() { + let (value, clamped) = ComposeUp.clampCPULimit("plenty") + #expect(value == "plenty") + #expect(!clamped) + } + + /// The ordinary path is unchanged by the guard. + @Test("ordinary fractions still round up, whole numbers are left alone") + func ordinaryValuesUnchanged() { + #expect(ComposeUp.clampCPULimit("0.5") == ("1", true)) + #expect(ComposeUp.clampCPULimit("2.5") == ("3", true)) + #expect(ComposeUp.clampCPULimit("4") == ("4", false)) + } + + /// The builder default survives a value the clamp refuses to touch. + @Test("the builder falls back to its default for an unrepresentable value") + func builderFallsBack() throws { + let svc = try YAMLDecoder().decode(Service.self, from: """ + image: alpine + deploy: + resources: + limits: + cpus: "1e400" + """) + #expect(ComposeUp.builderCPUCount(for: svc) == 2) + } +}