DNS names were only set up for user-defined networks. On Linux, none
of the built-in networks (bridge/host/none) have built-in DNS, so they
don't need DNS names.
But, on Windows, the default network is "nat" and it does need the DNS
names.
Signed-off-by: Rob Murray <rob.murray@docker.com>
The API's EndpointConfig struct has a MacAddress field that's used for
both the configured address, and the current address (which may be generated).
A configured address must be restored when a container is restarted, but a
generated address must not.
The previous attempt to differentiate between the two, without adding a field
to the API's EndpointConfig that would show up in 'inspect' output, was a
field in the daemon's version of EndpointSettings, MACOperational. It did
not work, MACOperational was set to true when a configured address was
used. So, while it ensured addresses were regenerated, it failed to preserve
a configured address.
So, this change removes that code, and adds DesiredMacAddress to the wrapped
version of EndpointSettings, where it is persisted but does not appear in
'inspect' results. Its value is copied from MacAddress (the API field) when
a container is created.
Signed-off-by: Rob Murray <rob.murray@docker.com>
This is a follow-up to 2cf230951f, adding
more directives to adjust for some new code added since:
Before this patch:
make -C ./internal/gocompat/
GO111MODULE=off go generate .
GO111MODULE=on go mod tidy
GO111MODULE=on go test -v
# github.com/docker/docker/internal/sliceutil
internal/sliceutil/sliceutil.go:3:12: type parameter requires go1.18 or later (-lang was set to go1.16; check go.mod)
internal/sliceutil/sliceutil.go:3:14: predeclared comparable requires go1.18 or later (-lang was set to go1.16; check go.mod)
internal/sliceutil/sliceutil.go:4:19: invalid map key type T (missing comparable constraint)
# github.com/docker/docker/libnetwork
libnetwork/endpoint.go:252:17: implicit function instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
# github.com/docker/docker/daemon
daemon/container_operations.go:682:9: implicit function instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
daemon/inspect.go:42:18: implicit function instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
With this patch:
make -C ./internal/gocompat/
GO111MODULE=off go generate .
GO111MODULE=on go mod tidy
GO111MODULE=on go test -v
=== RUN TestModuleCompatibllity
main_test.go:321: all packages have the correct go version specified through //go:build
--- PASS: TestModuleCompatibllity (0.00s)
PASS
ok gocompat 0.031s
make: Leaving directory '/go/src/github.com/docker/docker/internal/gocompat'
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The MAC address of a running container was stored in the same place as
the configured address for a container.
When starting a stopped container, a generated address was treated as a
configured address. If that generated address (based on an IPAM-assigned
IP address) had been reused, the containers ended up with duplicate MAC
addresses.
So, remember whether the MAC address was explicitly configured, and
clear it if not.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Since v25.0 (commit ff50388), we validate endpoint settings when
containers are created, instead of doing so when containers are started.
However, a container created prior to that release would still trigger
validation error at start-time. In such case, the API returns a 500
status code because the Go error isn't wrapped into an InvalidParameter
error. This is now fixed.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The output var was used in a `defer`, but named `err` and shadowed in various
places. Rename the var to a more explicit name to make clear where it's used
and to prevent it being accidentally shadowed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Instead of special-casing anonymous endpoints in libnetwork, let the
daemon specify what (non fully qualified) DNS names should be associated
to container's endpoints.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Use a strong type for the DNS IP-addresses so that we can use flags.IPSliceVar,
instead of implementing our own option-type and validation.
Behavior should be the same, although error-messages have slightly changed:
Before this patch:
dockerd --dns 1.1.1.1oooo --validate
Status: invalid argument "1.1.1.1oooo" for "--dns" flag: 1.1.1.1oooo is not an ip address
See 'dockerd --help'., Code: 125
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1"]}
dockerd --dns 2.2.2.2 --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: dns: (from flag: [2.2.2.2], from file: [1.1.1.1])
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1oooo"]}
dockerd --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: merged configuration validation from file and command line flags failed: 1.1.1.1ooooo is not an ip address
With this patch:
dockerd --dns 1.1.1.1oooo --validate
Status: invalid argument "1.1.1.1oooo" for "--dns" flag: invalid string being converted to IP address: 1.1.1.1oooo
See 'dockerd --help'., Code: 125
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1"]}
dockerd --dns 2.2.2.2 --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: dns: (from flag: [2.2.2.2], from file: [1.1.1.1])
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1oooo"]}
dockerd --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: invalid IP address: 1.1.1.1oooo
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Prior to this commit, only container.Config had a MacAddress field and
it's used only for the first network the container connects to. It's a
relic of old times where custom networks were not supported.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
When the default bridge is disabled by setting dockerd's `--bridge=none`
option, the daemon still creates a sandbox for containers with no
network attachment specified. In that case `NetworkDisabled` will be set
to true.
However, currently the `releaseNetwork` call will early return if
NetworkDisabled is true. Thus, these sandboxes won't be deleted until
the daemon is restarted. If a high number of such containers are
created, the daemon would then take few minutes to start.
See https://github.com/moby/moby/issues/42461.
Signed-off-by: payall4u <payall4u@qq.com>
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
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 issue wasn't caught on ContainerCreate or NetworkConnect (when
container wasn't started yet).
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Thus far, validation code would stop as soon as a bad value was found.
Now, we try to validate as much as we can, to return all errors to the
API client.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
So far, only a subset of NetworkingConfig was validated when calling
ContainerCreate. Other parameters would be validated when the container
was started. And the same goes for EndpointSettings on NetworkConnect.
This commit adds two validation steps:
1. Check if the IP addresses set in endpoint's IPAMConfig are valid,
when ContainerCreate and ConnectToNetwork is called ;
2. Check if the network allows static IP addresses, only on
ConnectToNetwork as we need the libnetwork's Network for that and it
might not exist until NetworkAttachment requests are sent to the
Swarm leader (which happens only when starting the container) ;
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The daemon would pass an EndpointCreateOption to set the interface MAC
address if the network name and the provided network mode were matching.
Obviously, if the network mode is a network ID, it won't work. To make
things worse, the network mode is never normalized if it's a partial ID.
To fix that: 1. the condition under what the container's mac-address is
applied is updated to also match the full ID; 2. the network mode is
normalized to a full ID when it's only a partial one.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Define consts for the Actions we use for events, instead of "ad-hoc" strings.
Having these consts makes it easier to find where specific events are triggered,
makes the events less error-prone, and allows documenting each Action (if needed).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function never returns an error, so let's remove the error-return,
and give it a slightly more to-the-point name.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Various parts of the code were using "walkers" to iterate over the
controller's sandboxes, and the only condition for all of them was
to find the sandbox for a given container-ID. Iterating over all
sandboxes was also sub-optimal, because on Windows, the ContainerID
is used as Sandbox-ID, which can be used to lookup the sandbox from
the "sandboxes" map on the controller.
This patch implements a GetSandbox method on the controller that
looks up the sandbox for a given container-ID, using the most optimal
approach (depending on the platform).
The new method can return errors for invalid (empty) container-IDs, and
a "not found" error to allow consumers to detect non-existing sandboxes,
or potentially invalid IDs.
This new method replaces the (non-exported) Daemon.getNetworkSandbox(),
which was only used internally, in favor of directly accessing the
controller's method.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Now that we removed the interface, there's no need to cast the Network
to a NetworkInfo interface, so we can remove uses of the `Info()` method.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Remove some intermediate vars, move vars closer to where they're used,
and introduce local var for `nw.Name()` to reduce some locking/unlocking in:
- `Daemon.allocateNetwork()`
- `Daemon.releaseNetwork()`
- `Daemon.connectToNetwork()`
- `Daemon.disconnectFromNetwork()`
- `Daemon.findAndAttachNetwork()`
Also un-wrapping some lines to make it slightly easier to read the conditions.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Remove intermediate variable
- Optimize the order of checks in the condition; check for unmanaged containers
first, before getting information about cluster state and network information.
- Simplify the log messages, as the error would already contain the same
information about the network (name or ID) and container (ID), so would
print the network ID twice:
error detaching from network <ID>: could not find network attachment for container <ID> to network <name or ID>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The function was declaring an err variable which was shadowed. It was
intended for directly assigning to a struct field, but as this function
is directly mutating an existing object, and the err variable was declared
far away from its use, let's use an intermediate var for that to make it
slightly more atomic.
While at it, also combined two "if" branches.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This code was initializing a new PortBinding, and creating a deep copy
for each binding. It's unclear what the intent was here, but at least
PortBinding.GetCopy() wasn't adding much value, as it created a new
PortBinding, [copying all values from the original][1], which includes
a [copy of IPAddresses in it][2]. Our original "template" did not have any
of that, so let's forego that, and just create new PortBindings as we go.
[1]: 454b6a7cf5/libnetwork/types/types.go (L110-L120)
[2]: 454b6a7cf5/libnetwork/types/types.go (L236-L244)
Benchmarking before/after;
BenchmarkPortBindingCopy-10 166752 6230 ns/op 1600 B/op 100 allocs/op
BenchmarkPortBindingNoCopy-10 226989 5056 ns/op 1600 B/op 100 allocs/op
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These were not adding much, so just getting rid of them. Also added a
TODO to move this code to the type.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Move variables closer to where they're used instead of defining them all
at the start of the function.
Also removing some intermediate variables, unwrapped some lines, and combined
some checks to a single check.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Outside of some tests, these options are the only code setting these fields,
so we can update them to set the value, instead of appending.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was created as a "method", but didn't use the Daemon in any
way, and all other options were checked inline, so let's not pretend this
function is more "special" than the other checks, and inline the code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The "Capability" type defines DataScope and ConnectivityScope fields,
but their value was set from consts in the datastore package, which
required importing that package and its dependencies for the consts
only.
This patch:
- Moves the consts to a separate "scope" package
- Adds aliases for the consts in the datastore package.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These aliases were not needed, and only used in a couple of places,
which made it inconsistent, so let's use the import without aliasing.
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>
Basically every exported method which takes a libnetwork.Sandbox
argument asserts that the value's concrete type is *sandbox. Passing any
other implementation of the interface is a runtime error! This interface
is a footgun, and clearly not necessary. Export and use the concrete
type instead.
Signed-off-by: Cory Snider <csnider@mirantis.com>
