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>
If the lease doesn't exit (for example when creating the container
failed), just ignore the not found error.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Currently, IPAM config is never validated by the API. Some checks
are done by the CLI, but they're not exhaustive. And some of these
misconfigurations might be caught early by libnetwork (ie. when the
network is created), and others only surface when connecting a container
to a misconfigured network. In both cases, the API would return a 500.
Although the `NetworkCreate` endpoint might already return warnings,
these are never displayed by the CLI. As such, it was decided during a
maintainer's call to return validation errors _for all API versions_.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
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>
If the image for the wanted platform doesn't exist then the lease
doesn't exist either. Returning this error hides the real error, so
let's not return it.
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
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>
Fix a failure to inspect image if any of its present manifest references
an image config which isn't present locally.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This goroutine was added in c458bca6dc, and
looks for errors from the wait channel. If no error is returned, it attempts
to start the container, and *updates* the error if a failure happened while
doing so, so that the code below it can update the container's status, and
perform auto-remove (if set for the container).
However, due to the formatting of the code, it was easy to overlook that
the "err" variable was not local to the "if" statement.
This patch breaks up the if-statement in an attempt to make it clearer that
this is not a local "err" variable, and adds a code-comment explaining the
logic.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The function was checking in a loop if networking for the container was
disabled. Change the function to return early, and to only set hooks
if one needs to be set.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Removing this type, because:
- containerNotModifiedError is not an actual error, and abstracting it away
was hiding some of these details. It also wasn't used as a sentinel error
anywhere, so doesn't have to be its own type.
- Defining a type just to toggle the error-message between "not running"
and "not stopped" felt a bit over-the-top, as each variant was only used once.
- So "it only had one job", and it didn't even do that right; it produced
capitalized error messages, which makes linters unhappy.
So, let's just inline what it does in the two places it was used.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
There's no need for this to be a closure; let's just make it a regular
function. While moving it out, also make some minor code-changes and
add some code-comments to describe the flow / intent, which may not
be trivial for people that are not familiar with these details.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The container rw layer may already be mounted, so it's not safe to use
it in another overlay mount. Use the ref counted mounter (which will
reuse the existing mount if it exists) to avoid that.
Also, mount the parent mounts (layers of the base image) in a read-only
mode.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
To prevent mounting the container rootfs in a rw mode if it's already
mounted. This can't use `mount.WithReadonlyTempMount` because the
archive code does a chroot with a pivot_root, which creates a new
directory in the rootfs.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Any error that occurs while creating the spec, even if it's the
result of an invalid container config, must be considered a System
error (internal server error), as it's not an error with the request
to start the container.
Invalid configuration in the config itself must be validated when
creating the container (creating its config), but some errors are
dependent on the current state, for example when starting a container
that shares a namespace with another container, and that container
is not running (or missing).
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>
PR moby/moby#45759 is going to use the new `errors.Join` function to
return a list of validation errors.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
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>
store network.Name() in a variable to reduce repeatedly locking/unlocking
of the network (although this is very, very minimal in the grand scheme
of things).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function is called by `daemon.containerCreate()` which is already
wrapping errors coming from `verifyNetworkingConfig()` with
`errdefs.InvalidParameter()`. So `verifyNetworkingConfig()` should only
return standard errors.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
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>
