This PR resolves#1343.
This PR depends on apple/container-builder-shim#72.
Do not create staging directory under build context, but pass
dockerignore file bytes to the container-builder-shim.
## Type of Change
- [x] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
## Motivation and Context
[Why is this change needed?]
## Testing
- [x] Tested locally
- [ ] Added/updated tests
- [ ] Added/updated docs
- Closes#874.
- Image save now fails fast with a clear error when the requested
platform is not available locally.
- The message lists the platforms that ARE available so the user
knows what they can save.
This PR fixes#357, passing `SSH_AUTH_SOCK` env variable from current
terminal to the `SandboxService` so that the container can mount the
correct ssh auth socket. For that, it introduces `env` parameters to
`bootstrap` RPC of both `ContainersService` and `SandboxService`. This
parameter is used only for passing `SSH_AUTH_SOCK` now, but can be
extended to pass more runtime env variables.
This PR is a follow up PR of #1214.
## Type of Change
- [X] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
## Motivation and Context
Container run `--ssh` was inheriting `SSH_AUTH_SOCK` env variable from
launchd, not from current terminal.
## Testing
- [X] Tested locally
- [ ] Added/updated tests
- [ ] Added/updated docs
This change migrates plugin config format away from using JSON to use
TOML.
- Addresses the plugin portion of discussion #1336
## Type of Change
- [ ] Bug fix
- [X] New feature
- [ ] Breaking change
- [ ] Documentation update
## Motivation and Context
Context within https://github.com/apple/container/discussions/1336
> We're looking to improve the user experience and overall functionality
of setting defaults for container to better enable future use cases.
>
> Today container uses macOS's
[UserDefaults](https://developer.apple.com/documentation/foundation/userdefaults)
to configure settings needed at runtime. As mentioned in
https://github.com/apple/container/issues/608, UserDefaults may be
idiomatic for macOS, but they apply globally to all sessions and do not
handle representing complex, hierarchical data well. Additionally, we
currently have two ways of setting these defaults, either directly with
macOS's defaults command or through container system property.
>
> https://github.com/apple/container/issues/608 proposes moving to use
environment variables in place of UserDefaults. However, we do not
believe this is sufficient. Environment variables are not in a
consistent location, are not sourced from data, and also do not handle
representing complex, hierarchical data well.
As part of this migration to TOML for user settings we want to also move
plugin settings to use TOML so that we can have a singular file format
for `container` configurations.
## Testing
- [X] Tested locally
- [X] Added/updated tests
- [ ] Added/updated docs
- Adds a color progress output mode (`--progress color`)
that renders ANSI-colored progress output with visual
differentiation between progress states.
- Like `ansi` mode, this feature requires a TTY.
- Closes#1366
Closes#1352
Containerization has had support for a bit, it was just never brought
over here. It's exposed on the CLI via the classic `--cap-add` and
`--cap-drop` UX.
- Closes#1359.
- Create a ResourceLabels type and extract the label validation from
NetworkConfiguration into the new type.
- Create a base AppError type that is compatible with structured logging
and delegates message presentation to the error receiver.
- Define LabelError over AppError for label validation.
- Slightly reworks NetworkConfiguration entity migration code in
NetworksService.
## Motivation and Context
This PR consolidates duplicated list-output formatting across the CLI
into shared rendering infrastructure.
Currently, each list command has its own copy of the same
json/quiet/table branching. This PR pulls that into shared rendering
infrastructure in `ContainerCommands`:
- `ListDisplayable` protocol for table + quiet output
- `renderJSON`, `renderTable`, `renderList` as pure functions that
return strings
- `emit()` as the single stdout boundary (no-ops on empty strings to
avoid blank-line regressions)
- `JSONOptions` so all JSON encoding goes through one path, including
volume inspect's pretty + ISO 8601 case
JSON encoding remains separate from display formatting: each command
still chooses its own JSON model, while `ListDisplayable` is used only
for table and quiet output. `ImageList` remains the intentional
exception for quiet mode so it can avoid unnecessary async work.
This change also replaces inline `JSONEncoder` usage with `renderJSON`,
removes the old `Codable+JSON.swift` helper, and moves `TableOutput` and
`ListFormat` into `ContainerCommands`. It also adds unit tests for the
shared rendering helpers and expands integration coverage for image,
network, and registry list formatting.
## Testing
- [x] Tested locally
- [x] Added/updated tests
- [ ] Added/updated docs
- Closes#387.
- Override the swift-argument-parser `help` command
with a version that prints the reformatted help that
includes plugin information.
---------
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
- Clarified the `.plain` progress contract. This also
documents how `clearOnFinish` interacts with plain
output. Added test coverage for the current behavior.
- Adds a `plain` progress output mode (`--progress plain`)
that outputs newline-separated progress lines without
ANSI escape sequences. This is useful for CI environments,
piped output, and non-TTY contexts where ANSI cursor
control is not supported. Unlike `--progress ansi`, plain
mode works even when stderr is not a TTY.
- Closes#1365
- Related: #641, #808
- TestCLIStatus isn't in the integration test.
- Update the output to align with the current output.
- Follow up #1193
---------
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
Same as we'd done with kill.
```
➜ container git:(stop-kill-throw-error) ✗ ./bin/container stop foo bar baz
Warning! Running debug build. Performance may be degraded.
Error: internalError: "failed to stop container" (cause: "notFound: "container with ID baz not found"")
internalError: "failed to stop container" (cause: "notFound: "container with ID bar not found"")
internalError: "failed to stop container" (cause: "notFound: "container with ID foo not found"")
➜ container git:(stop-kill-throw-error) ✗ ./bin/container kill foo bar baz
Warning! Running debug build. Performance may be degraded.
Error: internalError: "failed to kill container" (cause: "notFound: "container with ID foo not found"")
internalError: "failed to kill container" (cause: "notFound: "container with ID bar not found"")
internalError: "failed to kill container" (cause: "notFound: "container with ID baz not found"")
➜ container git:(stop-kill-throw-error) ✗ ./bin/container delete foo bar baz
Warning! Running debug build. Performance may be degraded.
Error: internalError: "failed to delete container" (cause: "notFound: "container with ID foo not found"")
internalError: "failed to delete container" (cause: "notFound: "container with ID baz not found"")
internalError: "failed to delete container" (cause: "notFound: "container with ID bar not found"")
```
## Type of Change
- [x] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
## Motivation and Context
`TestCLIVersion` isn't in the integration test; we should include this.
And update the output to align with the real output.
Follow up #911
## Testing
- [x] Tested locally
- [x] Added/updated tests
- [ ] Added/updated docs
---------
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
- Closes#1268.
- The types we were using weren't very usable with Swift 6 structured
concurrency.
- Implements just the subset of records that we use.
- Use notImplemented instead of formatError for unknown record types.
- Use pure actor for LocalhostDNSHandler now that we have sendable
types.
- Use DNSName as key for table lookups in LocalhostDNSHandler and
HostTableResolver.
- Utilize dot-suffixed domain names everywhere in the lookup chain.
- Huge thanks to @manojmahapatra and @katiewasnothere for their
diligent and patient review :D
This PR fixes `container build` to use docker specific ignore (#1169).
This PR relies on apple/container-builder-shim#68.
When docker specific ignore file is detected, it creates a hidden docker
directory (i.e., `.hidden-docker-dir`) under the build context, and
places Dockerfile and ignore file under there. This hidden directory is
passed to the builder shim and used by buildkit daemon to correctly
figure out the docker specific ignore file.
Docker specific ignore file is detected regardless of whether i) they
are under nested directory, ii) outside build context, iii) using custom
names. If docker specific ignore is not provided, hidden directory is
also not created, and buildkit daemon will use the docker ignore file at
build context root as usual.
## Type of Change
- [X] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
## Motivation and Context
Docker specific ignore works.
## Testing
- [X] Tested locally
- [ ] Added/updated tests
- [ ] Added/updated docs
- Closes#1252 (part of #913).
- Commands that accept `--platform` currently require it
to be passed explicitly every time. Users working
consistently with a non-native platform (e.g. `linux/amd64`
on Apple Silicon) have no way to set a default, unlike
Docker's `DOCKER_DEFAULT_PLATFORM`.
- This adds support for the `CONTAINER_DEFAULT_PLATFORM`
environment variable as a fallback when `--platform` is not
explicitly provided.
- Previously we weren't tolerating a throw from
the check.
- We have #1250 open for root cause analysis of
this particular throw that trips the test up.
This PR fixes#1277.
`SandboxService.waiters` had a consistency issue (not exactly race).
`SandboxService.wait` XPC can be executed on arbitrary `id`, and it will
hang forever if no other handler resumes it. Without knowing this
internal, the high level entity can run into this issue, and deadlock.
This PR simplifies the mental model: **`SandboxService.waiters[id]:
ExitWaiter(continuations, exitCode)` can only be in three states: i)
non-existing, ii) existing with nil `exitCode`, and iii) existing with
concrete `exitCode`.**
**If it is non-existing, no handler has been registered to resume it
later. If existing with nil `exitCode`, It is guaranteed the registered
`continuations` will be resumed later with a concrete `exitCode`.
Finally, if already a concrete `exitCode`, a handler has been
registered, and already resumed (with that `exitCode`).**
Thus, `SandboxService.wait` should return immediately if `waiters[id]`
is non-existing or existing with a concrete `exitCode` (as no handler
will resume it later). It should only block when `waiters[id]` is
existing with nil `exitCode` as it is guaranteed to be resumed later. By
doing so, we can guarantee there is no deadlock at all.
For that this PR does followings:
1. Introduce `ExitMonitor` class to updates `continuations` and
`exitCode` all together atomically. Initially, `state` variable saved
the `exitCode`, but it cannot be tied with `continuations` as they are
protected by different primitives (i.e., lock and actor).
2. Gather `waiters` related operations into a single actor method,
guaranteeing those are performed atomically under actor
protection---i.e., we actually don't need Mutex here.
3. Ensure initialized `waiters` are released (i.e., resumed) later
(under any possible circumstances).
4. Move `process.wait` after `process.start` in `io.handleProcess` to
run `SandboxService.wait` only after the `waiters[id]` is initialized.
By doing fourth step, we can guarantee `SandboxService.wait` can meet
only one of two following `ExitMonitor` state: i) existing with nil
`exitCode`, or ii) existing with concrete `exitCode` (in case the
process exited too early). In both cases, `exitCode` is preserved and
returned.
## Type of Change
- [X] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
## Motivation and Context
[Why is this change needed?]
## Testing
- [X] Tested locally
- [ ] Added/updated tests
- [ ] Added/updated docs
Fix container hang when starting an invalid executable (#1277).
Ensure the `SandboxService.waiters` updated atomically, preventing a slow `wait` from registering continuation after the `SandboxService` has been stopped.
Current `ProcessIO.handleProcess` has races between `process.wait()` and
`process.start()`. In `process.wait`, `SandboxService.wait` registers
and block on a continuation so that later exit of that `LinuxContainer`
can resume it.
Even if `SandboxService.wait` checks the container state, there can be a
race so that `SandboxService.startInitProcess` fails earlier with no
continuation to resume, then `SandboxService.wait` registers the
continuation, which forever hang the thread.
- Closes#1270.
- The current test is a bit baroque. It tries to run `nc`, either to
`8.8.8.8:53` or a proxy server port if `HTTP_PROXY` is in the
environment. It seems that for an alpine image, a proxy-aware
`Dockerfile` and a `RUN apk install curl` directive is a more natural
test of "network access".
- Current DirectoryWatcher fails if `/etc/resolver` does not exist. This
PR fixes DirectoryWatcher to handle non-existing `/etc/resolver`
directory. If that directory does not exist, it first watches `/etc`
directory to check if `/etc/resolver` directory is created later. Once
it detects new `/etc/resolver` directory, it starts watching new DNS
resolver files there.
- This PR also fixes to log the exception thrown by API server's tasks.
- Closes#1207
Closes#1225
Add a flag to signify that we'd like to run a minimal init process that
can reap zombie processes. The actual support for this is in the
Containerization library so the plumbing here is very simple.
- This reverts commit 1942a399e3.
- This approach to getting variant into the runtime helper was fine for
proof of concept work but we need a persistent runtime config file so
that the variant is available for `container start`.
- Adds plugin variant selection to bootstrap() on the Container API
service in such a way that we can revert the change soon without
compatibility issues when we work out a more permanent approach, and
requires no persistent data migration.
- Restore lexical ordering on ManagementFlags (except `--runtime`, will
take care of that next PR).
- Add a couple plugin loader tests to improve coverage.
## Type of Change
- [x] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
## Motivation and Context
As reported in [1212](https://github.com/apple/container/issues/1212),
environment variables defined in a Dockerfile (ENV) were not being
overridden when passed via -e at runtime, resulting in duplicate entries
in the container's environment.
**Root cause:** `Parser.allEnv()` simply merged image, env-file, and
user-provided environment variables without deduplication. Since
`getenv()` returns the first match, the image default took precedence
over the user override.
**Fix:** `allEnv()` now deduplicates by key, with later sources
overriding earlier ones
## Testing
- [x] Tested locally
- [x] Added/updated tests
- [ ] Added/updated docs
- Adds a a `--log-root` option to `swift system start`, propagating the
value as `CONTAINER_LOG_ROOT` to services for logging to files instead
of the OS log facility. This is not a "production" capability as it
neither merges nor rotates logs.
- Currently we don't collect logs on CI builds, and we don't have
permission to run the `log` command there. The PR adds `--log-root` to
the CI test phase, archives the results, and uploads the archive as an
artifact.
- Use FilePath from swift-system for the log root. Foundation URL is a
bit of a footgun for filesystem paths, so unless we identify a
showstopper, we should incrementally transition to this type everywhere
except where we really need network URLs.
- Output the hostname of the CI runner at the start of the test phase so
we can identify runner-specific issues where they exist.
- Fix formatting for log messages with multiple metadata items, and fix
unstructured messages on instances that weren't found using `grep -r
'log\.' Sources`.
- Adds command reference documentation for `--log-root`.
- Closes#1206.
- Closes#1185.
- Closes#507.
- Addresses existing log messages for #642.
- Nondeterministic CI errors are resulting from very slow launch times
for the first runtime helper, which causes ContainersService to be
locked for longer than our 20 sec timeout. Bumping the timeout to 60
seconds addresses this case for now.
- Since many log messages needed to be changed to troubleshoot the
issue, updated all log messages to use structured logging, and
implemented consistent entry/exit logging for all service operations.
- Added logging for ContainerService lock acquisition to help with
finding root cause for the slow service startup.
- Plumbed the `--debug` flag on both `container system start` and
`container system logs` so that the flag is actually useful.
- Updated the `install-init.sh` script so that can install in a custom
app root directory.
## Type of Change
- [x] New feature
- [x] Breaking change
## Motivation and Context
We want to be able to support using multiple network plugins during
`container`'s lifetime. This additionally means needing to pick an
interface strategy to interpret a network attachment based on what
network plugin was used to create that attachment. This PR will
potentially replace https://github.com/apple/container/pull/1081.
Followups:
- doc updates to include the ability to specify plugin in the network
creation cli
## Testing
- [x] Tested locally
- [x] Added/updated tests
## Type of Change
- [ ] Bug fix
- [x] New feature
- [ ] Breaking change
- [ ] Documentation update
## Motivation and Context
Closes#1046 -- Right now we're creating container bundles in
ContainersService. Move this to the SandboxService to make it easier to
support different container bundle types.
## Testing
- [x] Tested locally
- [x] Added/updated tests
- [ ] Added/updated docs
## Type of Change
- [x] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
## Motivation and Context
- CI build is failing because runners don't have an /etc/resolver
directory, causing the directory monitor to fail. This occurs while the
install-kernel make target is running, so it appears that kernel
download is failing when it's just that the API server is dying. Created
#1207 for the issue.
- Fixing the initial scan for the directory just moves the problem to
the filesystem watch loop, likely because we're not testing the result
of `open()` for errors.
- Right now the priority is getting CI running and PRs merged, so just
commenting out the realhost DNS server setup.
- Also seeing that under some conditions it can take quite a while for
launchd to start the helper for the default network (8 seconds or more).
With the 10 second health check timeout after API server registration,
this means that some CI runs can exhibit this failure mode. Added a
`--timeout` option to SystemStart and set a 60 second timeout for
install-kernel and integration Makefile targets.
- Fixed a bug where `--debug` was being placed in the wrong location in
the api server startup args.
- Disabled all network CLI tests due to container bootstrap errors when
trying to run the container immediately after creating the network. The
slow network helper launch could be the reason behind the failures that
drove us to serialize these tests. Filed #1206 for this issue.
## Testing
- [x] Tested locally
- [ ] Added/updated tests
- [ ] Added/updated docs
jwhur