Now that we're raising the minimum API version, we can also update
the CLI used in our integration-cli tests.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Make invalid states unrepresentable by moving away from stringly-typed
MAC address values in API structs. As go.dev/issue/29678 has not yet
been implemented, provide our own HardwareAddr byte-slice type which
implements TextMarshaler and TextUnmarshaler to retain compatibility
with the API wire format.
When stdlib's net.HardwareAddr type implements TextMarshaler and
TextUnmarshaler and GODEBUG=netmarshal becomes the default, we should be
able to make the type a straight alias for stdlib net.HardwareAddr as a
non-breaking change.
Signed-off-by: Cory Snider <csnider@mirantis.com>
These utilities are very handy to use in integration tests, too. Move
the package so it can be imported by them.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Clients should not make assumptions about the validity of an API struct
as the set of well-formed values may differ across daemon versions.
Remove it from the API module so client-application authors are not
tempted to apply it, which would restrict the forward compatibility of
the client.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The Container.State struct holds the container's state, and most of
its fields are expected to change dynamically. Some o these state-changes
are explicit, for example, setting the container to be "stopped". Other
state changes can be more explicit, for example due to the containers'
process exiting or being "OOM" killed by the kernel.
The distinction between explicit ("desired") state changes and "state"
("actual state") is sometimes vague; for some properties, we clearly
separated them, for example if a user requested the container to be
stopped or restarted, we store state in the Container object itself;
HasBeenManuallyStopped bool // used for unless-stopped restart policy
HasBeenManuallyRestarted bool `json:"-"` // used to distinguish restart caused by restart policy from the manual one
Other properties are more ambiguous. such as "HasBeenStartedBefore" and
"RestartCount", which are stored on the Container (and persisted to
disk), but may be more related to "actual" state, and likely should
not be persisted;
RestartCount int
HasBeenStartedBefore bool
Given that (per the above) concurrency must be taken into account, most
changes to the `container.State` struct should be protected; here's where
things get blurry. While the `State` type provides various accessor methods,
only some of them take concurrency into account; for example, [State.IsRunning]
and [State.GetPID] acquire a lock, whereas [State.ExitCodeValue] does not.
Even the (commonly used) [State.StateString] has no locking at all.
The way to handle this is error-prone; [container.State] contains a mutex,
and it's exported. Given that its embedded in the [container.Container]
struct, it's also exposed as an exported mutex for the container. The
assumption here is that by "merging" the two, the caller to acquire a lock
when either the container _or_ its state must be mutated. However, because
some methods on `container.State` handle their own locking, consumers must
be deeply familiar with the internals; if both changes to the `Container`
AND `Container.State` must be made. This gets amplified more as some
(exported!) methods, such as [container.SetRunning] mutate multiple fields,
but don't acquire a lock (so expect the caller to hold one), but their
(also exported) counterpart (e.g. [State.IsRunning]) do.
It should be clear from the above, that this needs some architectural
changes; a clearer separation between "desired" and "actual" state (opening
the potential to update the container's config without manually touching
its `State`), possibly a method to obtain a read-only copy of the current
state (for those querying state), and reviewing which fields belong where
(and should be persisted to disk, or only remain in memory).
This PR preserves the status quo; it makes no structural changes, other
than exposing where we access the container's state. Where previously the
State fields and methods were referred to as "part of the container"
(e.g. `ctr.IsRunning()` or `ctr.Running`), we now explicitly reference
the embedded `State` (`ctr.State.IsRunning`, `ctr.State.Running`).
The exception (for now) is the mutex, which is still referenced through
the embedded struct (`ctr.Lock()` instead of `ctr.State.Lock()`), as this
is (mostly) by design to protect the container, and what's in it (including
its `State`).
[State.IsRunning]: c4afa77157/daemon/container/state.go (L205-L209)
[State.GetPID]: c4afa77157/daemon/container/state.go (L211-L216)
[State.ExitCodeValue]: c4afa77157/daemon/container/state.go (L218-L228)
[State.StateString]: c4afa77157/daemon/container/state.go (L102-L131)
[container.State]: c4afa77157/daemon/container/state.go (L15-L23)
[container.Container]: c4afa77157/daemon/container/container.go (L67-L75)
[container.SetRunning]: c4afa77157/daemon/container/state.go (L230-L277)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When running:
docker network create --ipv6 b46
docker run --rm -ti \
--network name=b46,driver-opt=com.docker.network.endpoint.sysctls=net.ipv6.conf.IFNAME.disable_ipv6=1 \
busybox
IPv6 is enabled in the container and the network, so an IPv6 address
will be allocated for the endpoint.
But, when the sysctl is applied, the IPv6 address will be removed
from the interface ... so, no unsolicited neighbour advertisement
should be (or can be) sent and, the endpoint should not be treated
as dual-stack when selecting a gateway endpoint and, if it is
selected as the gateway endpoint, setting up an IPv6 route via the
network will fail.
So, if the IPv6 address disappears after sysctls have been applied,
release the address and remove it from the endpoint's config.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Complete the removal of the deprecated network structs by dropping the
remaining references in daemon code.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The buildSandboxPlatformOptions function was given a pointer to the
sboxOptions and modified it in-place.
Similarly, a pointer to the container was passed and `container.HostsPath`
and `container.ResolvConfPath` mutated. In cases where either of those
failed, we would return an error, but the container (and sboxOptions)
would already be modified.
This patch;
- updates the signature of buildSandboxPlatformOptions to return a fresh
slice of sandbox options, which can be appended to the sboxOptions by
the caller.
- uses intermediate variables for `hostsPath` and `resolvConfPath`, and
only mutates the container if both were obtained successfully.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Modernize the field and allow using it as-is in some places, or
convert it to a string (which won't produce an error down the line).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Staticcheck is suggesting to cast the type or to directly copy, but
doesn't account for nat.SortPortMap mutating the second argument, so
mutating the HostConfig.PortBindings. From the code, it looks like the
intent here was to prevent that (creating a deep copy), so let's keep
that.
daemon/container_operations.go:109:39: S1016: should convert bb (type github.com/docker/docker/vendor/github.com/docker/go-connections/nat.PortBinding) to github.com/docker/docker/vendor/github.com/moby/moby/api/types/container.PortBinding instead of using struct literal (staticcheck)
bindings[p] = append(bindings[p], containertypes.PortBinding{
^
daemon/network.go:952:39: S1016: should convert bb (type github.com/docker/docker/vendor/github.com/docker/go-connections/nat.PortBinding) to github.com/docker/docker/vendor/github.com/moby/moby/api/types/container.PortBinding instead of using struct literal (staticcheck)
bindings[p] = append(bindings[p], containertypes.PortBinding{
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Follow-up to 494677f93f, which added
the aliases, but did not yet replace our own use of the nat types.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These errors were not used as sentinel error, and used as any other
"invalid parameter" / "invalid argument" error, so remove them, and
just produce errors where used.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The stringid package is used in many places; while it's trivial
to implement a similar utility, let's just provide it as a utility
package in the client, removing the daemon-specific logic.
For integration tests, I opted to use the implementation in the
client, as those should not ideally not make assumptions about
the daemon implementation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These comments were added to enforce using the correct import path for
our packages ("github.com/docker/docker", not "github.com/moby/moby").
However, when working in go module mode (not GOPATH / vendor), they have
no effect, so their impact is limited.
Remove these imports in preparation of migrating our code to become an
actual go module.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Go maintainers started to unconditionally update the minimum go version
for golang.org/x/ dependencies to go1.23, which means that we'll no longer
be able to support any version below that when updating those dependencies;
> all: upgrade go directive to at least 1.23.0 [generated]
>
> By now Go 1.24.0 has been released, and Go 1.22 is no longer supported
> per the Go Release Policy (https://go.dev/doc/devel/release#policy).
>
> For golang/go#69095.
This updates our minimum version to go1.23, as we won't be able to maintain
compatibility with older versions because of the above.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The function `getEndpointPortMapInfo` is called by `updateJoinInfo` to
update the field `NetworkSettings.Ports` of a container.
However, `updateJoinInfo` is only called by `connectToNetwork` which is
also calling `getPortMapInfo` (which in turn calls
`getEndpointPortMapInfo`).
So, remove the call to `getEndpointPortMapInfo` from `updateJoinInfo`.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Split the `ErrConflictHostNetwork` error into two distinct errors:
1. `ErrConflictConnectToHostNetwork` when attempting to change the
network mode of a running container from a different mode to `host`
2. `ErrConflictDisconnectFromHostNetwork` when the network mode of a
running container is `host` and attempting to disconnect from `host`
This commit clarifies error messaging by differentiating between the two
errors, making it clearer which operation failed and how to fix it.
Signed-off-by: Brendon Smith <bws@bws.bio>
Prior to commit fe856b9, containers' network sandbox and interfaces were
created before the containerd task. Now, it's created after.
If this step fails, the containerd task is forcefully deleted, and an
event is sent to the c8d event monitor, which triggers `handleContainerExit`.
Then this method tries to restart the faulty container.
This leads to containers with a published port already in use to be
stuck in a tight restart loop (if they're started with
`--restart=always`) until the port is available. This is needlessly
spamming the daemon logs.
Prior to that commit, a published port already in use wouldn't trigger
the restart process.
This commit adds a check to `handleContainerExit` to ignore exit events
if the latest container error is related to networking setup.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
When failing to resolve the network container, a "not found" error should
not return a 404, but either a "invalid parameter" (400) or "system" (500)
error.
Given that this function is called on container start, not container create,
a 500 (internal server error) is more appropriate, because the API request
(start the container) is valid, but the state of the container isn't.
While working on this, I discovered that no validation happens during container
create; TODO's were added to look into that, but this may be partially
by design (allow a container to be created before the "donor" network
container is created).
Before this patch:
docker container create --name hello --network=container:nosuchcontainer alpine
docker container start hello
Error response from daemon: No such container: nosuchcontainer
Error: failed to start containers: hello
# daemon logs:
DEBU[2025-01-30T11:32:33.595636043Z] error response for POST request error-response="No such container: nosuchcontainer" method=POST module=api request-url=/v1.47/containers/hello/start status=404 vars="map[name:hello version:1.47]"
docker container create --name hello2 --network=container:hello2 alpine
docker container start hello2
Error response from daemon: cannot join own network
Error: failed to start containers: hello2
# daemon logs:
DEBU[2025-01-30T11:33:19.545287551Z] FIXME: Got an API for which error does not match any expected type!!! error="cannot join own network" error_type="*errors.errorString" module=api
DEBU[2025-01-30T11:33:19.545346093Z] error response for POST request error-response="cannot join own network" method=POST module=api request-url=/v1.47/containers/hello2/start status=500 vars="map[name:hello2 version:1.47]"
DEBU[2025-01-30T11:33:19.545369968Z] FIXME: Got an API for which error does not match any expected type!!! error="cannot join own network" error_type="*errors.errorString" module=api
ERRO[2025-01-30T11:33:19.545375426Z] Handler for POST /v1.47/containers/hello2/start returned error: cannot join own network
With this patch:
docker container create --name hello --network=container:nosuchcontainer alpine
docker container start hello
Error response from daemon: joining network of container: No such container: nosuchcontainer
Error: failed to start containers: hello
# daemon logs:
DEBU[2025-01-30T11:35:50.406462760Z] error response for POST request error-response="joining network of container: No such container: nosuchcontainer" method=POST module=api request-url=/v1.47/containers/hello/start status=500 vars="map[name:hello version:1.47]"
ERRO[2025-01-30T11:35:50.406501468Z] Handler for POST /v1.47/containers/hello/start returned error: joining network of container: No such container: nosuchcontainer
docker container create --name hello2 --network=container:hello2 alpine
docker container start hello2
Error response from daemon: cannot join own network namespace
Error: failed to start containers: hello2
# daemon logs:
DEBU[2025-01-30T11:36:15.178475049Z] error response for POST request error-response="cannot join own network" method=POST module=api request-url=/v1.47/containers/hello2/start status=500 vars="map[name:hello2 version:1.47]"
ERRO[2025-01-30T11:36:15.178536507Z] Handler for POST /v1.47/containers/hello2/start returned error: cannot join own network
docker run --name exitedcontainer alpine
docker run --rm --network=container:exitedcontainer alpine
docker: Error response from daemon: cannot join network namespace of a non running container: container exitedcontainer is exited.
# daemon logs:
DEBU[2025-01-30T12:54:28.040637429Z] error response for POST request error-response="cannot join network namespace of a non running container: container exitedcontainer is exited" method=POST module=api request-url=/v1.47/containers/hello2/start status=409 vars="map[name:hello2 version:1.47]"
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Running a container with "--add-host blah:host-gateway" adds an /etc/hosts
entry for host "blah" and an address on the docker host - to give the
container a convenient way of reaching the host.
If no --host-gateway-ip option is supplied, the IPv4 address of the
default bridge is used - and that's been fine until now, it's a host
address we know will exist. But, in a container that's only connected
to IPv6-only networks, that doesn't work.
So:
- if the default bridge has an IPv6 address, create an additional
/etc/hosts entry with that adddress
- allow two --host-gateway-ip options
- at most one IPv4 and one IPv6 address
- in daemon.json, allow a JSON array value in --host-gateway-ips (plural)
- for a single address, a JSON string is also allowed
For example:
--host-gateway-ip 192.0.2.1 --host-gateway-ip 2001:db8::1111
And the daemon.json version would be:
"host-gateway-ips": ["192.0.2.1", "2001:db8::1111"]
But, this is also still valid:
"host-gateway-ip": "192.0.2.1"
Note that the /etc/hosts entries follow the usual rules. If IPv6 is
disabled in a container (by sysctl, or lack of kernel support), IPv6
addresses are not included in the file. In other cases, IPv4 and IPv6
addresses will both be included, whether or not the container currently
has network endpoints that support IPv4 or IPv6.
buildx has its own code to interpret the host-gateway-ip option. When
it's updated to understand two addresses, moby will need to pass it
both. For now, it passes an IPv4 address if there is one, else IPv6.
Signed-off-by: Rob Murray <rob.murray@docker.com>
commit a0807e7cfe configured golangci-lint
to use go1.23 semantics, which alowed linters like `copyloopvar` to lint
using thee correct semantics.
go1.22 now creates a copy of variables when assigned in a loop; make sure we
don't have files that may downgrade semantics to go1.21 in case that also means
disabling that feature; https://go.dev/ref/spec#Go_1.22
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
For Linux, delay construction and configuration of network endpoints
until the container has been created (but not started).
Signed-off-by: Rob Murray <rob.murray@docker.com>
If config for legacy links needs to be added to a libnetwork.Sandbox,
add it when constructing the Endpoint that needs it - removing the
constraint on ordering of Endpoint construction, and the dependency
between Endpoint and Sandbox construction.
So, now a Sandbox can be constructed in one place, before the first
Endpoint.
Signed-off-by: Rob Murray <rob.murray@docker.com>
