From c8a8366f7977760c4b48916b12825b0e1ee8fc02 Mon Sep 17 00:00:00 2001 From: us Date: Sat, 4 Jul 2026 13:52:19 +0300 Subject: [PATCH] fix(runtime): honor --memory and compose mem_limit - Normalize --memory/mem_limit to the uppercase K/M/G/T/P suffix format Apple's container CLI documents (mocker's own --help advertised Docker-style lowercase like "3g", risking silent rejection by the underlying runtime). - mocker update -m/--cpus/--memory-reservation/--memory-swap now fails loudly instead of printing a warning and exiting 0: Apple Containerization has no update path, so the old behavior looked like success while changing nothing. - mocker run now warns that --memory-swap/--memory-swappiness/ --memory-reservation are unsupported (Apple Containerization has no swap concept), instead of silently accepting them as no-ops. - mocker compose config no longer strips mem_limit / deploy.resources.limits and reservations from its printed output; ComposeOrchestrator.startService already threads mem_limit into the container's -m flag on `up`, so `config` was misleadingly implying the value was dropped. Closes #62 --- Sources/Mocker/Commands/Compose.swift | 49 ++++++++++--- Sources/Mocker/Commands/Run.swift | 6 ++ Sources/Mocker/Commands/Update.swift | 19 ++++-- .../MockerKit/Container/ContainerEngine.swift | 36 +++++++++- .../MockerKitTests/ContainerEngineTests.swift | 56 +++++++++++++++ Tests/MockerTests/CLITests.swift | 20 ++++++ Tests/MockerTests/ComposeConfigTests.swift | 68 +++++++++++++++++++ 7 files changed, 240 insertions(+), 14 deletions(-) create mode 100644 Tests/MockerTests/ComposeConfigTests.swift diff --git a/Sources/Mocker/Commands/Compose.swift b/Sources/Mocker/Commands/Compose.swift index 8e7ac6b..a285eb2 100644 --- a/Sources/Mocker/Commands/Compose.swift +++ b/Sources/Mocker/Commands/Compose.swift @@ -1190,16 +1190,49 @@ struct ComposeConfig: AsyncParsableCommand { return } - // Print resolved compose file as YAML-like output - print("name: \(options.projectName ?? "default")") - print("services:") + print(Self.renderConfig(composeFile: composeFile, projectName: options.projectName)) + } + + /// Render the resolved Compose file as YAML-like output, mirroring what `up` + /// will actually apply. Extracted as a pure function (rather than inline + /// `print` calls) so it's directly unit-testable without capturing stdout. + /// + /// Resource limits (`mem_limit`/`cpus` and their `deploy.resources` equivalents) + /// are wired into the container's `-m`/`-c` flags by `ComposeOrchestrator. + /// startService` — this must surface them too, or `config` misleadingly implies + /// they're dropped (see #62). + static func renderConfig(composeFile: ComposeFile, projectName: String?) -> String { + var lines: [String] = [] + lines.append("name: \(projectName ?? "default")") + lines.append("services:") for (name, svc) in composeFile.services.sorted(by: { $0.key < $1.key }) { - print(" \(name):") - if let image = svc.image { print(" image: \(image)") } - if let build = svc.build { print(" build: \(build)") } - if !svc.ports.isEmpty { print(" ports:"); svc.ports.forEach { print(" - \($0)") } } - if !svc.environment.isEmpty { print(" environment:"); svc.environment.forEach { print(" - \($0)") } } + lines.append(" \(name):") + if let image = svc.image { lines.append(" image: \(image)") } + if let build = svc.build { lines.append(" build: \(build)") } + if !svc.ports.isEmpty { + lines.append(" ports:") + svc.ports.forEach { lines.append(" - \($0)") } + } + if !svc.environment.isEmpty { + lines.append(" environment:") + svc.environment.forEach { lines.append(" - \($0)") } + } + if svc.memLimit != nil || svc.cpus != nil || svc.memReservation != nil || svc.cpusReservation != nil { + lines.append(" deploy:") + lines.append(" resources:") + if svc.memLimit != nil || svc.cpus != nil { + lines.append(" limits:") + if let cpus = svc.cpus { lines.append(" cpus: \"\(cpus)\"") } + if let mem = svc.memLimit { lines.append(" memory: \(mem)") } + } + if svc.memReservation != nil || svc.cpusReservation != nil { + lines.append(" reservations:") + if let cpus = svc.cpusReservation { lines.append(" cpus: \"\(cpus)\"") } + if let mem = svc.memReservation { lines.append(" memory: \(mem)") } + } + } } + return lines.joined(separator: "\n") } } diff --git a/Sources/Mocker/Commands/Run.swift b/Sources/Mocker/Commands/Run.swift index 1f65c22..fa22c5c 100644 --- a/Sources/Mocker/Commands/Run.swift +++ b/Sources/Mocker/Commands/Run.swift @@ -346,6 +346,12 @@ struct Run: AsyncParsableCommand { if !deviceWriteIops.isEmpty { unsupported.append("--device-write-iops") } if healthCmd != nil { unsupported.append("--health-cmd") } if `init` { unsupported.append("--init") } + // Apple Containerization has no swap concept (each container is a VM with a + // fixed memory allocation, not a cgroup) — accepting these silently would hide + // that a swap-based safety net for over-limit spikes never actually exists. See #62. + if memorySwap != nil { unsupported.append("--memory-swap") } + if memorySwappiness != nil { unsupported.append("--memory-swappiness") } + if memoryReservation != nil { unsupported.append("--memory-reservation") } if !unsupported.isEmpty { let flags = unsupported.joined(separator: ", ") FileHandle.standardError.write(Data("WARNING: flags not supported by Apple Containerization runtime (ignored): \(flags)\n".utf8)) diff --git a/Sources/Mocker/Commands/Update.swift b/Sources/Mocker/Commands/Update.swift index 84b0501..2dc8b24 100644 --- a/Sources/Mocker/Commands/Update.swift +++ b/Sources/Mocker/Commands/Update.swift @@ -53,12 +53,23 @@ struct Update: AsyncParsableCommand { var memorySwap: String? func run() async throws { - // Accept the flags for compatibility but warn if not applicable + // Apple's Containerization runtime has no `container update` equivalent: a + // container's VM (and therefore its memory/CPU allocation) is sized once at + // `run` time and cannot be resized in place. Silently printing the container + // ids and a warning (as mocker used to) looks like success but never actually + // changes anything — see https://github.com/us/mocker/issues/62. Fail loudly + // instead so callers don't believe the limit was applied. + if memory != nil || cpus != nil || memoryReservation != nil || memorySwap != nil { + throw MockerError.operationFailed( + "mocker update cannot resize memory/CPU limits: Apple Containerization sizes " + + "a container's VM once at 'mocker run' time and has no live-update support. " + + "Recreate the container with the desired --memory/--cpus instead " + + "(e.g. 'mocker rm && mocker run --memory 3g ...')." + ) + } + for identifier in containers { print(identifier) } - if memory != nil || cpus != nil { - FileHandle.standardError.write(Data("WARNING: Resource limits are managed at VM level with Apple Containerization\n".utf8)) - } } } diff --git a/Sources/MockerKit/Container/ContainerEngine.swift b/Sources/MockerKit/Container/ContainerEngine.swift index a5d03d1..299cbc4 100644 --- a/Sources/MockerKit/Container/ContainerEngine.swift +++ b/Sources/MockerKit/Container/ContainerEngine.swift @@ -137,8 +137,8 @@ public actor ContainerEngine { args += ["-c", cpuCount] } - if let memory = containerConfig.memory { - args += ["-m", memory] + if let memory = containerConfig.memory, let normalized = Self.sanitizeMemory(memory) { + args += ["-m", normalized] } for t in containerConfig.tmpfs { @@ -188,6 +188,38 @@ public actor ContainerEngine { return count >= 1 ? String(count) : nil } + /// Normalize a Docker-style memory value (e.g. `512m`, `3g`, `1.5G`, plain bytes) + /// into the form Apple's `container` CLI accepts for `-m/--memory`: an integer + /// amount with an optional uppercase `K`/`M`/`G`/`T`/`P` suffix (1 MiByte + /// granularity). Apple's parser only documents the uppercase suffixes, so a + /// lowercase Docker-style value (which mocker's own `--help` text advertises, + /// e.g. "512m, 1g") risked being silently rejected/ignored by the underlying + /// runtime, defaulting the VM back to its built-in 1 GiB size. See #62. + /// Returns `nil` (dropping the flag) only for genuinely unparsable input. + static func sanitizeMemory(_ value: String) -> String? { + let trimmed = value.trimmingCharacters(in: .whitespaces) + guard !trimmed.isEmpty else { return nil } + + let suffixes: Set = ["b", "B", "k", "K", "m", "M", "g", "G", "t", "T", "p", "P"] + guard let last = trimmed.last else { return nil } + + if suffixes.contains(last) { + let numberPart = trimmed.dropLast() + guard !numberPart.isEmpty, Double(numberPart) != nil else { return nil } + // Docker allows a trailing "b" (bytes) which Apple's CLI doesn't document; + // drop it since bytes are the implicit unit when no suffix is given. + let upper = last.uppercased() + if upper == "B" { + return String(numberPart) + } + return "\(numberPart)\(upper)" + } + + // No suffix: must be a plain byte count. + guard Double(trimmed) != nil else { return nil } + return trimmed + } + // MARK: - List public func list(all: Bool = false) async throws -> [ContainerInfo] { diff --git a/Tests/MockerKitTests/ContainerEngineTests.swift b/Tests/MockerKitTests/ContainerEngineTests.swift index 11773e3..7ffa00a 100644 --- a/Tests/MockerKitTests/ContainerEngineTests.swift +++ b/Tests/MockerKitTests/ContainerEngineTests.swift @@ -162,6 +162,62 @@ struct ContainerEngineTests { #expect(ContainerEngine.sanitizeCpuCount("3") == "3") } + // MARK: - sanitizeMemory + + @Test("sanitizeMemory uppercases Docker-style lowercase suffixes") + func sanitizeMemoryLowercaseSuffix() { + // Apple's container CLI only documents uppercase K/M/G/T/P suffixes; mocker's + // own --help text advertises Docker-style lowercase ("512m, 1g"), so these must + // be normalized or the flag risks being silently rejected/ignored (#62). + #expect(ContainerEngine.sanitizeMemory("3g") == "3G") + #expect(ContainerEngine.sanitizeMemory("512m") == "512M") + #expect(ContainerEngine.sanitizeMemory("2k") == "2K") + } + + @Test("sanitizeMemory preserves already-uppercase suffixes") + func sanitizeMemoryUppercaseSuffix() { + #expect(ContainerEngine.sanitizeMemory("3G") == "3G") + #expect(ContainerEngine.sanitizeMemory("512M") == "512M") + } + + @Test("sanitizeMemory strips a trailing bytes suffix") + func sanitizeMemoryBytesSuffix() { + #expect(ContainerEngine.sanitizeMemory("1024b") == "1024") + #expect(ContainerEngine.sanitizeMemory("1024B") == "1024") + } + + @Test("sanitizeMemory passes through plain byte counts") + func sanitizeMemoryPlainBytes() { + #expect(ContainerEngine.sanitizeMemory("1073741824") == "1073741824") + } + + @Test("sanitizeMemory returns nil for invalid input") + func sanitizeMemoryInvalid() { + #expect(ContainerEngine.sanitizeMemory("abc") == nil) + #expect(ContainerEngine.sanitizeMemory("") == nil) + #expect(ContainerEngine.sanitizeMemory("g") == nil) + } + + @Test("Run arguments emit a normalized -m flag when memory is set") + func testRunArgumentsIncludeNormalizedMemory() { + let config = ContainerConfig(image: "alpine", memory: "3g") + + let args = ContainerEngine.buildRunArguments(name: "capped", config: config) + + let idx = args.firstIndex(of: "-m") + #expect(idx != nil) + if let idx { #expect(args[idx + 1] == "3G") } + } + + @Test("Run arguments omit -m when memory is unset") + func testRunArgumentsOmitMemoryWhenUnset() { + let config = ContainerConfig(image: "alpine") + + let args = ContainerEngine.buildRunArguments(name: "no-limit", config: config) + + #expect(!args.contains("-m")) + } + @Test("Run arguments include nested virtualization flags") func testRunArgumentsIncludeVirtualizationFlags() { let config = ContainerConfig( diff --git a/Tests/MockerTests/CLITests.swift b/Tests/MockerTests/CLITests.swift index 8ea7d76..8f4e5f5 100644 --- a/Tests/MockerTests/CLITests.swift +++ b/Tests/MockerTests/CLITests.swift @@ -15,6 +15,26 @@ struct CLITests { // x-release-please-end } + @Test("Update errors instead of silently ignoring --memory") + func updateMemoryErrorsInsteadOfNoOp() async throws { + // Apple Containerization sizes a container's VM once at `run` time and has no + // live-update path, so `mocker update -m` used to just print a warning and + // exit 0 — a silent no-op that looked like success. It must now fail loudly + // instead of pretending the limit changed. See #62. + let command = try Update.parse(["--memory", "3g", "some-container"]) + await #expect(throws: MockerError.self) { + try await command.run() + } + } + + @Test("Update errors instead of silently ignoring --cpus") + func updateCpusErrorsInsteadOfNoOp() async throws { + let command = try Update.parse(["--cpus", "2", "some-container"]) + await #expect(throws: MockerError.self) { + try await command.run() + } + } + @Test("Run command accepts --env-file flag") func runEnvFileFlag() throws { let command = try Run.parse(["--env-file", "test.env", "alpine"]) diff --git a/Tests/MockerTests/ComposeConfigTests.swift b/Tests/MockerTests/ComposeConfigTests.swift new file mode 100644 index 0000000..da8a86a --- /dev/null +++ b/Tests/MockerTests/ComposeConfigTests.swift @@ -0,0 +1,68 @@ +import Testing +import Foundation +import MockerKit +@testable import Mocker + +/// `mocker compose config` is meant to show the resolved configuration that `up` +/// will actually apply. Before this fix it silently dropped `mem_limit` and +/// `deploy.resources.limits/reservations` — the exact "compose mem_limit stripped" +/// complaint in #62 — even though `ComposeOrchestrator.startService` already wires +/// `service.memLimit` into the container's `-m` flag. These tests lock in that the +/// printed config now reflects the resource limits that get applied. +@Suite("Compose Config Tests") +struct ComposeConfigTests { + + @Test("compose config prints mem_limit as deploy.resources.limits.memory") + func composeConfigPrintsMemLimit() throws { + let composeFile = try ComposeFile.parse(""" + services: + app: + image: nginx:latest + mem_limit: 2g + """) + + let output = ComposeConfig.renderConfig(composeFile: composeFile, projectName: nil) + + #expect(output.contains("deploy:")) + #expect(output.contains("limits:")) + #expect(output.contains("memory: 2g")) + } + + @Test("compose config prints deploy.resources.limits.cpus and reservations") + func composeConfigPrintsDeployResources() throws { + let composeFile = try ComposeFile.parse(""" + services: + app: + image: nginx:latest + deploy: + resources: + limits: + cpus: "0.50" + memory: 512M + reservations: + cpus: "0.25" + memory: 256M + """) + + let output = ComposeConfig.renderConfig(composeFile: composeFile, projectName: nil) + + #expect(output.contains("cpus: \"0.50\"")) + #expect(output.contains("memory: 512M")) + #expect(output.contains("reservations:")) + #expect(output.contains("cpus: \"0.25\"")) + #expect(output.contains("memory: 256M")) + } + + @Test("compose config omits deploy block when no resource limits are set") + func composeConfigOmitsDeployWhenUnset() throws { + let composeFile = try ComposeFile.parse(""" + services: + app: + image: nginx:latest + """) + + let output = ComposeConfig.renderConfig(composeFile: composeFile, projectName: nil) + + #expect(!output.contains("deploy:")) + } +}