fix(service): add mem_limit and extra_hosts support#125
Conversation
Implements two commonly-used Compose keys that container-compose v1.0.0
does not support:
**mem_limit** — top-level memory limit shorthand (e.g. `mem_limit: 2g`).
Decoded as a String; also accepts bare integer bytes (YAML `mem_limit:
536870912`). Mapped to `--memory` in ComposeUp, taking precedence over
`deploy.resources.limits.memory` when both are set.
**extra_hosts** — additional /etc/hosts entries. Accepts both Compose
spec forms: list (`["hostname:IP"]`) and map (`{hostname: IP}`). The
special token `host-gateway` is resolved to the host machine's default
gateway IP via `route -n get default`; resolution runs lazily (only when
at least one entry actually uses the token). Passed to `container run`
as `--add-host hostname:IP` (colon separator, per the OCI standard).
Tests cover: string mem_limit, gigabyte mem_limit, integer bytes mem_limit,
absent mem_limit, list extra_hosts, host-gateway token, map form
normalisation, absent extra_hosts, multiple entries, and resolveHostGatewayIP
return type. Test helper updated to use flatMap({ $0 }) to correctly
unwrap Service?? from the services dictionary.
Cyb3rDudu
left a comment
There was a problem hiding this comment.
@ryan106 The parsing and the mem_limit → --memory work look good, and the
tests are solid. One thing I'd like to nail down before merging: the
host-gateway resolution in resolveHostGatewayIP.
route -n get default returns the macOS host's default-route gateway -
i.e. the LAN router - not the host itself. On my machine that's 192.168.1.1
while the host is 192.168.1.254, so
extra_hosts: ["logto.localhost:host-gateway"] would resolve
logto.localhost to the router, not the host-side proxy it's meant to
reach. Docker's host-gateway means "the host as seen from the container
network," and the Mac's outbound default route isn't that — under Apple
container's VM networking the container reaches the host via the VM bridge,
which route -n get default doesn't describe.
A couple of ways forward:
- Smoke-test it:
container runwith the--add-host, then check from inside
the container whether the resolved IP actually reaches the macOS host. - Or check whether Apple
container runacceptshost-gatewayliterally /
exposes the correct gateway directly - if so, passing the token through is
more robust than parsingroute.
Minor, related: needsGateway checks the raw (pre-resolveVariable) entries,
so a variable wrapping the whole entry ("${HOST_ENTRY}" → "foo:host-gateway")
would leave hostGatewayIP empty and emit --add-host foo:. Checking the
resolved value instead would close that.
Happy to merge once the host-gateway value is verified (or switched to whatever
Apple container natively supports).
… route -n route -n get default reports the Mac's LAN default-route gateway, which is not the same address as the vmnet bridge gateway containers actually use to reach the host — verified: route -n get default returns 192.168.68.1 (the router) while container network inspect default reports 192.168.64.1 (the real gateway) on the same machine. Query the network directly instead, with route -n get default kept only as a last-resort fallback. Also fixes needsGateway checking extra_hosts entries before resolveVariable runs, which missed entries fully wrapped in a variable, and replaces --add-host (container run has no such flag — confirmed it fails with "Unknown option '--add-host'") with a hosts file bind-mounted over /etc/hosts.
Awesome thanks for the comments! Changes pushed. Thanks Again! |
Cyb3rDudu
left a comment
There was a problem hiding this comment.
We are getting there.
Bind-mounting over /etc/hosts replaces the container's entire hosts file, including the entry Apple container normally generates for the container's own hostname/IP. The generated file only has localhost + the extra_hosts lines, so anything inside the container that resolves its own hostname loses that mapping (Docker's --add-host appends; this overwrites). For the PR's use case (reaching a host proxy) it's fine, and there's no clean way to read the runtime-generated /etc/hosts before container run to merge — so it's somewhat inherent to the workaround.
Can you
- add a one-line print warning when extra_hosts is used (e.g. "Note: extra_hosts replaces the container's /etc/hosts; the container's own hostname entry is overridden"), and/or
- include the container's own hostname in the generated file (e.g. 127.0.0.1 ) so self-resolution still works.
Minor
- The temp hosts file (NSTemporaryDirectory()...) is never cleaned up. One per service per up. Negligible (OS temp), but trivial to remove after the run if timing allows.
- Gateway uses service.networks?.first ?? "default"; for a service on multiple custom networks the first network's gateway may not be the host-reachable one. Edge case.
…es /etc/hosts Bind-mounting over /etc/hosts (required because container run has no --add-host to append to) replaces the daemon-generated file wholesale, including the entry for the container's own hostname. Print a note explaining this, and re-add <containerName> -> 127.0.0.1 (or the resolved service.hostname when set) so self-resolution keeps working, skipping it if extra_hosts already defines that name explicitly. Also remove the generated hosts file in `down`, once the container it was mounted into is actually stopped -- doing it any earlier risks removing it out from under a still-running container's bind mount.
Problem
Two commonly-used Compose keys are silently ignored by
container-compose:mem_limit— the top-level memory limit shorthand (mem_limit: 2g). Services with high memory requirements (e.g. litellm, which spikes above 1 GB on startup) start without any limit and may be OOM-killed.extra_hosts— additional/etc/hostsentries (extra_hosts: ["logto.localhost:host-gateway"]). Without this, services that need to reach a host-side reverse proxy by hostname can't resolve it inside the container.Solution
mem_limitpublic let mem_limit: String?toServiceString; also handles bare integer bytes (YAMLmem_limit: 536870912) via anIntfallback--memoryinComposeUp, taking precedence overdeploy.resources.limits.memorywhen both are presentextra_hostspublic let extra_hosts: [String]?toService["hostname:IP"]) and map ({hostname: IP})host-gatewayis resolved to the host machine's default gateway IP viaroute -n get default(lazy — only runs when at least one entry uses the token)container runas--add-host hostname:IP(colon separator, per OCI standard)Changes
Sources/Container-Compose/Codable Structs/Service.swift— new properties,CodingKeys, memberwise init params, and decoder branchesSources/Container-Compose/Commands/ComposeUp.swift—--memoryflag frommem_limit;--add-hostloop with lazyhost-gatewayresolution;resolveHostGatewayIP()helperTests/Container-Compose-StaticTests/MemLimitExtraHostsTests.swift— 10 new tests covering string/integer/absentmem_limit, list/map/host-gateway/absent/multipleextra_hosts, andresolveHostGatewayIPreturn typeTests
All 10 tests pass; existing tests unaffected.