fix(runtime): honor --memory and compose mem_limit#66
Merged
Conversation
- 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Investigation of #62 ("Container memory hard-capped at 1 GiB, no swap; --memory ignored and compose mem_limit stripped") found three real, distinct bugs, and one architectural limitation that's now surfaced honestly instead of silently:
mocker update -m/--cpuswas a complete no-op that looked like success.Update.run()printed the container id(s) and a stderr warning, then exited0— it never touched the stored config or the runtime. Apple's Containerization runtime has nocontainer updateequivalent at all (a container's VM is sized once atruntime and can't be resized live or on restart), so this now fails loudly with a clear error telling the user to recreate the container instead of pretending the change took effect.mocker compose configsilently droppedmem_limit/deploy.resources.limits/reservationsfrom its printed output, even thoughComposeOrchestrator.startServicealready correctly threadsservice.memLimitinto the container's-mflag onup. This madeconfigmisleadingly imply the value was discarded. Fixed by rendering the resolveddeploy.resourcesblock.--memory/mem_limitvalues could be silently mis-honored by the underlying runtime due to a case mismatch. Apple'scontainerCLI docs only mention uppercaseK/M/G/T/Psuffixes, while mocker's own--helptext (and Docker convention) advertises lowercase (512m,1g). AddedContainerEngine.sanitizeMemory, mirroring the existingsanitizeCpuCounthelper, to normalize the value before it reachescontainer run -m.--memory-swap/--memory-swappiness/--memory-reservationwere silently accepted no-ops. Apple Containerization has no swap concept (each container is its own VM, not a cgroup), so these can never actually be honored.mocker runnow surfaces them in the existing "flags not supported by Apple Containerization runtime" warning instead of pretending to accept them.Root cause for the 1 GiB ceiling itself: that's Apple's
containerCLI's own documented default when no-mis passed — not something mocker can raise on its own. The real, fixable bugs were the update no-op, the compose config display gap, and the case-format mismatch risk for--memory.Test plan
swift buildsucceedsswift test— all 314 tests pass, including new coverage:sanitizeMemorynormalization (lowercase/uppercase/byte-suffix/invalid input)buildRunArgumentsemits a normalized-mflagmocker update --memory/--cpusnow throws instead of silently succeedingmocker compose configprintsmem_limit/deploy.resourcesfor limits and reservations, and omits the block when unsetCloses #62