The container platform migration tries to deduce the platform data from
the containerd content store if it's available.
However, on Windows we currently default to a non-containerd runtime
setup, so the containerd client is nil and accessing its content store
paniced:
```
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x0 pc=0x11b48e4]
goroutine 87 [running]:
github.com/containerd/containerd/v2/client.(*Client).ContentStore(0xc0003a0008?)
/go/src/github.com/docker/docker/vendor/github.com/containerd/containerd/v2/client/client.go:645 +0x24
github.com/docker/docker/daemon.(*Daemon).load(0xc00026e488, {0xc000c13d40, 0x40})
/go/src/github.com/docker/docker/daemon/container.go:84 +0x289
github.com/docker/docker/daemon.(*Daemon).restore.func1({0xc000c13d40, 0x40})
/go/src/github.com/docker/docker/daemon/daemon.go:236 +0x207
created by github.com/docker/docker/daemon.(*Daemon).restore in goroutine 1
/go/src/github.com/docker/docker/daemon/daemon.go:229 +0x1a7
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x0 pc=0x11b48e4]
goroutine 90 [running]:
github.com/containerd/containerd/v2/client.(*Client).ContentStore(0xc000313608?)
/go/src/github.com/docker/docker/vendor/github.com/containerd/containerd/v2/client/client.go:645 +0x24
github.com/docker/docker/daemon.(*Daemon).load(0xc00026e488, {0xc000c13e00, 0x40})
/go/src/github.com/docker/docker/daemon/container.go:84 +0x289
github.com/docker/docker/daemon.(*Daemon).restore.func1({0xc000c13e00, 0x40})
/go/src/github.com/docker/docker/daemon/daemon.go:236 +0x207
created by github.com/docker/docker/daemon.(*Daemon).restore in goroutine 1
/go/src/github.com/docker/docker/daemon/daemon.go:229 +0x1a7
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x0 pc=0x11b48e4]
```
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
It was not using the daemon, so can be a regular function. While at it,
also changed the parameter type to accept a regular string-slice, as
we don't need strslice.StrSlice's json.Unmarshaler implementation, and
reversed the logic for the early return.
Finally, for uses where the entrypoint was always nil, this patch removes
the use of this utility altogether.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Change the persistent container metadata to store the whole platform
(as defined by OCI) instead of only the operating system.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Daemon.containerRoot was only used in a single place, but defined far
from where it was used, and Daemon.newBaseContainer was a _very_ thin
abstraction on top of container.NewBaseContainer.
Let's remove these.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was poorly documented as it uses fuzzy matching under the hood,
and it's no longer used. Mark it as deprecated, and to be removed in the
next release.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was poorly documented as it uses fuzzy matching under the hood,
and it's no longer used. Mark it as deprecated, and to be removed in the
next release.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
generateHostname was only used here, and defined as a method on Daemon
without depending on the daemon type. Inline the function and simplify
the logic (as both the "network=host" and non-host code only had to be
executed if no hostname was set).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was only used internally in the daemon. This patch splits
the implementation to a non-exported version and deprecates the exported
one.
While at it, also pass through the context (which is used for tracing),
and added a note about the function potentially not being atomic.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Remove uses of this function and mark it deprecated. There's no known
consumers of this function, but let's stay on the safe side, and mark
it deprected for 1 release.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Don't mutate the container's `Config.WorkingDir` permanently with a
cleaned path when creating a working directory.
Move the `filepath.Clean` to the `translateWorkingDir` instead.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Don't change the behavior for older clients and keep the same behavior.
Otherwise client can't opt-out (because `ReadOnlyNonRecursive` is
unsupported before 1.44).
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The semantics of an "anonymous" endpoint has always been weird: it was
set on endpoints which name shouldn't be taken into account when
inserting DNS records into libnetwork's `Controller.svcRecords` (and
into the NetworkDB). However, in that case the endpoint's aliases would
still be used to create DNS records; thus, making those "anonymous
endpoints" not so anonymous.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This is a follow-up to 2216d3ca8d, which
implemented the StartInterval for health-checks, but did not add validation
for the minimum accepted interval;
> The time to wait between checks in nanoseconds during the start period.
> It should be 0 or at least 1000000 (1 ms). 0 means inherit.
This patch adds validation for the minimum accepted interval (1ms).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The github.com/containerd/containerd/log package was moved to a separate
module, which will also be used by upcoming (patch) releases of containerd.
This patch moves our own uses of the package to use the new module.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was added in 12485d62ee to save some
duplication, but was really over-engineered to save a few lines of code,
at the cost of hiding away what it does and also potentially returning
inconsistent errors (not addressed in this patch). Let's start with
inlining these.
This removes;
- Daemon.checkContainer
- daemon.containerIsRunning
- daemon.containerIsNotRestarting
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also move the validation function to live with the type definition,
which allows it to be used outside of the daemon as well.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The existing runtimes reload logic went to great lengths to replace the
directory containing runtime wrapper scripts as atomically as possible
within the limitations of the Linux filesystem ABI. Trouble is,
atomically swapping the wrapper scripts directory solves the wrong
problem! The runtime configuration is "locked in" when a container is
started, including the path to the runC binary. If a container is
started with a runtime which requires a daemon-managed wrapper script
and then the daemon is reloaded with a config which no longer requires
the wrapper script (i.e. some args -> no args, or the runtime is dropped
from the config), that container would become unmanageable. Any attempts
to stop, exec or otherwise perform lifecycle management operations on
the container are likely to fail due to the wrapper script no longer
existing at its original path.
Atomically swapping the wrapper scripts is also incompatible with the
read-copy-update paradigm for reloading configuration. A handler in the
daemon could retain a reference to the pre-reload configuration for an
indeterminate amount of time after the daemon configuration has been
reloaded and updated. It is possible for the daemon to attempt to start
a container using a deleted wrapper script if a request to run a
container races a reload.
Solve the problem of deleting referenced wrapper scripts by ensuring
that all wrapper scripts are *immutable* for the lifetime of the daemon
process. Any given runtime wrapper script must always exist with the
same contents, no matter how many times the daemon config is reloaded,
or what changes are made to the config. This is accomplished by using
everyone's favourite design pattern: content-addressable storage. Each
wrapper script file name is suffixed with the SHA-256 digest of its
contents to (probabilistically) guarantee immutability without needing
any concurrency control. Stale runtime wrapper scripts are only cleaned
up on the next daemon restart.
Split the derived runtimes configuration from the user-supplied
configuration to have a place to store derived state without mutating
the user-supplied configuration or exposing daemon internals in API
struct types. Hold the derived state and the user-supplied configuration
in a single struct value so that they can be updated as an atomic unit.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Ensure data-race-free access to the daemon configuration without
locking by mutating a deep copy of the config and atomically storing
a pointer to the copy into the daemon-wide configStore value. Any
operations which need to read from the daemon config must capture the
configStore value only once and pass it around to guarantee a consistent
view of the config.
Signed-off-by: Cory Snider <csnider@mirantis.com>
- Split these options to a separate struct, so that we can handle them in isolation.
- Change some tests to use subtests, and improve coverage
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(*Container).CheckpointTo() upserts a snapshot of the container to the
daemon's in-memory ViewDB and also persists the snapshot to disk. It
does not register the live container object with the daemon's container
store, however. The ViewDB and container store are used as the source of
truth for different operations, so having a container registered in one
but not the other can result in inconsistencies. In particular, the List
Containers API uses the ViewDB as its source of truth and the Container
Inspect API uses the container store.
The (*Daemon).setHostConfig() method is called fairly early in the
process of creating a container, long before the container is registered
in the daemon's container store. Due to a rogue CheckpointTo() call
inside setHostConfig(), there is a window of time where a container can
be included in a List Containers API response but "not exist" according
to the Container Inspect API and similar endpoints which operate on a
particular container. Remove the rogue call so that the caller has full
control over when the container is checkpointed and update callers to
checkpoint explicitly. No changes to (*Daemon).create() are needed as it
checkpoints the fully-created container via (*Daemon).Register().
Fixes#44512.
Signed-off-by: Cory Snider <csnider@mirantis.com>
GetContainer() would return (nil, nil) when looking up a container
if the container was inserted into the containersReplica ViewDB but not
the containers Store at the time of the lookup. Callers which reasonably
assume that the returned err == nil implies returned container != nil
would dereference a nil pointer and panic. Change GetContainer() so that
it always returns a container or an error.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Make the function name more generic, as it's no longer used only
for graphdrivers but also for snapshotters.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Older versions of Go don't format comments, so committing this as
a separate commit, so that we can already make these changes before
we upgrade to Go 1.19.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This changes mounts.NewParser() to create a parser for the current operatingsystem,
instead of one specific to a (possibly non-matching, in case of LCOW) OS.
With the OS-specific handling being removed, the "OS" parameter is also removed
from `daemon.verifyContainerSettings()`, and various other container-related
functions.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
After dicussing with maintainers, it was decided putting the burden of
providing the full cap list on the client is not a good design.
Instead we decided to follow along with the container API and use cap
add/drop.
This brings in the changes already merged into swarmkit.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
