It was only returned in a few places, and not used any different than
a "invalid parameter" error, so let's use a standard errdefs.ErrInvalidParameter
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It was only returned in a few places, and not used any different than
a "invalid parameter" error, so let's use a standard errdefs.ErrInvalidParameter
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
diagnosticServer is only written to during controller.New, and the
diagnostic server itself already has a mutex on EnableDiagnostic,
DisableDiagnostic, and IsDiagnosticEnabled, which should prevent
issues trying to concurrently change its state.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Handle situations where the server is already started or stopped internally,
instead of requiring the caller to do this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
A gateway address is always reserved before the network driver is
asked to create the network. But, the driver doesn't always need a
gateway address, so the address reservation can be unnecessary.
This means, for example, an "--internal" IPv4 "/31" network cannot
be used as a point-to-point link, because one of its two addresses
is always reserved for a gateway.
So, before allocating a gateway address, ask the network driver if
it will need one (based on options that only the network driver can
interpret). Implement that as an optional interface for network
drivers.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Before that change, we were passing the datastore to network drivers
through a `map[string]interface{}`. Then, each driver that needed the
store would cast the datastore to the correct type.
This was not a good design, as it was not clear which drivers were using
the store and which were not. Not all unit tests were passing the store,
leading to logs about uninitialized store being written.
This change makes the store a parameter of the `RegisterX` functions.
All unit tests are now passing a valid datastore to the drivers. A new
testutil func is added for that purpose.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
It no longer needs to be moved to the top of the filter-FORWARD
chain after creating a new bridge network. But, it does need to
be rearranged after setting up Swarm ingress.
Similarly, the jump to DOCKER-INGRESS no longer needs to be
shuffled back to the top of the FORWARD chain after adding a
new network.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Commit 3ec19ff62b introduced a GC goroutine to delete files where netns
were mounted. It was primarly added to work around a race in kernel
3.18-4.0.1. Since no distros we support are using such old kernels,
there's no need to keep this code around.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
In 27.x and earlier releases libnetwork's database file was in a
sub-directory "network/files" under the daemon's root data dir.
That part of the path got lost in commit ed08486ec7
So, libnet data ended up in the daemon's main Bolt db. Then, on
upgrade, config in the original file was no longer accessible.
libnet doesn't need access to any data outside its sub-dir, so
change the meaning of its OptionDataDir - it now points at libnet's
sub-dir, so the db will be created in the right place. Also, update
other uses of that data dir to match.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Currently, the DOCKER-USER chains are set up on firewall reload or network
creation. If there are running containers at startup, configureNetworking won't
be called (daemon/daemon_unix.go), so the user chains won't be setup.
This commit puts the setup logic on a separate function, and calls it on the
original place and on initNetworkController.
Signed-off-by: Andrés Maldonado <maldonado@codelutin.com>
That function was needlessly complex. Instead of relying on a struct and
a sub-struct, it now just takes two string params: a path and a bucket
name.
Libnetwork config is now initialized with default values. A new struct
is introduced in libnetwork/config to let tests customize the path and
bucket name.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
libnetwork/controller.go:1054:32: printf: non-constant format string in call to github.com/docker/docker/libnetwork/types.NotFoundErrorf (govet)
return types.NotFoundErrorf(err.Error())
^
libnetwork/controller.go:1073:32: printf: non-constant format string in call to github.com/docker/docker/libnetwork/types.NotFoundErrorf (govet)
return types.NotFoundErrorf(err.Error())
^
libnetwork/sandbox_externalkey_unix.go:113:21: printf: non-constant format string in call to fmt.Errorf (govet)
return fmt.Errorf(string(buf[0:n]))
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Previous commit made it clear that c.store can't be nil. Hence,
`c.store.Close()` can be called without checking if c.store is nil.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This was done in a separate method, called by the ctrler constructor.
This method was returning a nil datastore when c.cfg was nil -- but that
can't happen in practice!
This was giving the impression that the controller could be run without
a datastore properly configured. It's not the case, so make it explicit
by instantiating the datastore before `Controller`.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Prior to this change, cnmallocator would call
`ConfigGlobalScopeDefaultNetworks` right before initializing its
IPAM drivers. This function was mutating some global state used
during drivers init.
This change just remove the global state, and adds an arg to
ipams.Register and defaultipam.Register to pass the global pools
by arguments instead.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The bbolt library wants exclusive access to the boltdb file and uses
file locking to assure that is the case. The controller and each network
driver that needs persistent storage instantiates its own unique
datastore instance, backed by the same boltdb file. The boltdb kvstore
implementation works around multiple access to the same boltdb file by
aggressively closing the boltdb file between each transaction. This is
very inefficient. Have the controller pass its datastore instance into
the drivers and enable the PersistConnection option to disable closing
the boltdb between transactions.
Set data-dir in unit tests which instantiate libnetwork controllers so
they don't hang trying to lock the default boltdb database file.
Signed-off-by: Cory Snider <csnider@mirantis.com>
I was trying to find out why `docker info` was sometimes slow so
plumbing a context through to propagate trace data through.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Its only remaining purpose is to elide removing the endpoint from the
service records if it was not previously added. Deleting the service
records is an idempotent operation so it is harmless to delete service
records which do not exist.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The meaning of the (*Controller).isDistributedControl() method is not
immediately clear from the name, and it does not have any doc comment.
It returns true if and only if the controller is neither a manager node
nor an agent node -- that is, if the daemon is _not_ participating in a
Swarm cluster. The method name likely comes from the old abandoned
datastore-as-IPC control plane architecture for libnetwork. Refactor
c.isDistributedControl() -> !c.isSwarmNode()
to make it easier to understand code which consumes the method.
Signed-off-by: Cory Snider <csnider@mirantis.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>
It was only used in a single place, and it was defined far away from
where it was used.
Move the code inline, so that it's clear at a glance what it's doing.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Controller.getNetworkFromStore() already returns a ErrNoSuchNetwork if
no network was found, so we don't need to convert the existing error.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Fixes#18864, #20648, #33561, #40901.
[This GH comment][1] makes clear network name uniqueness has never been
enforced due to the eventually consistent nature of Classic Swarm
datastores:
> there is no guaranteed way to check for duplicates across a cluster of
> docker hosts.
And this is further confirmed by other comments made by @mrjana in that
same issue, eg. [this one][2]:
> we want to adopt a schema which can pave the way in the future for a
> completely decentralized cluster of docker hosts (if scalability is
> needed).
This decentralized model is what Classic Swarm was trying to be. It's
been superseded since then by Docker Swarm, which has a centralized
control plane.
To circumvent this drawback, the `NetworkCreate` endpoint accepts a
`CheckDuplicate` flag. However it's not perfectly reliable as it won't
catch concurrent requests.
Due to this design decision, API clients like Compose have to implement
workarounds to make sure names are really unique (eg.
docker/compose#9585). And the daemon itself has seen a string of issues
due to that decision, including some that aren't fixed to this day (for
instance moby/moby#40901):
> The problem is, that if you specify a network for a container using
> the ID, it will add that network to the container but it will then
> change it to reference the network by using the name.
To summarize, this "feature" is broken, has no practical use and is a
source of pain for Docker users and API consumers. So let's just remove
it for _all_ API versions.
[1]: https://github.com/moby/moby/issues/18864#issuecomment-167201414
[2]: https://github.com/moby/moby/issues/18864#issuecomment-167202589
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
There is no meaningful distinction between driverapi.Registerer and
drvregistry.DriverNotifyFunc. They are both used to register a network
driver with an interested party. They have the same function signature.
The only difference is that the latter could be satisfied by an
anonymous closure. However, in practice the only implementation of
drvregistry.DriverNotifyFunc is the
(*libnetwork.Controller).RegisterDriver method. This same method also
makes the libnetwork.Controller type satisfy the Registerer interface,
therefore the DriverNotifyFunc type is redundant. Change
drvregistry.Networks to notify a Registerer and drop the
DriverNotifyFunc type.
Signed-off-by: Cory Snider <csnider@mirantis.com>
osl.NewSandbox() always returns a nil interface on Windows (and other non-Linux
platforms). This means that any code that these fields are always nil, and
any code using these fields must be considered Linux-only;
- libnetwork/Controller.defOsSbox
- libnetwork/Sandbox.osSbox
Ideally, these fields would live in Linux-only files, but they're referenced
in various platform-neutral parts of the code, so let's start with moving
the initialization code to Linux-only files.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Windows uses the container-iD as ID for sandboxes, so it's not needed to
generate an ID when running on Windows.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The Controller.Sandboxes method was used by some SandboxWalkers. Now
that those have been removed, there are no longer any consumers of this
method, so let's remove it for now.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This functionality has been replaced with Controller.GetSandbox, and is
no longer used anywhere.
This patch removes:
- the Controller.WalkSandboxes method
- the SandboxContainerWalker SandboxWalker
- the SandboxWalker type
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>
