Plumb context from the API down to libnet driver method `CreateNetwork`,
and add an OTel span to the bridge driver's `createNetwork` method.
Include a few attributes describing the network configuration (e.g.
IPv4/IPv6, ICC, internal and MTU).
A new util function, `RecordStatus`, is added to the `otelutil` package
to easily record any error, and update the span status accordingly.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
To support this, a new netlabel is added: `com.docker.network.endpoint.ifname`.
It gives the ability to specify the interface name to be set by
netdrivers when the interface is added / moved into the container's
network namespace.
All builtin netdrivers support it.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
It was only used in a single place, and did not implement an errdef;
the type itself was not used as sentinel error.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since commit 933fcc9 (Re-remove the SetKey OCI prestart hook),
the network namespace will be set up before endpoints are
created in most cases, apart from build containers.
So, when possible, create the veth with one end in that netns
to save moving it in later. On my host, that saves about 20ms
for each bridge network a container is connected to.
Signed-off-by: Rob Murray <rob.murray@docker.com>
This code has some gnarly string-matching to detect "not found" errors
returned by hcsshim.
Hcsshim at some point changed this error to lowercase;
6d67a30859
It looks like we ran into that problem in integration tests, which was
fixed in c530c9cbb0 when updating hcsshim,
however, it was only fixed in tests, and hiding the actual issue in our
code.
It looks like hcsshim has some utilities to detect error-types, such as the
IsElementNotFoundError function in hcn, which is the newer API that also wraps
the "HNS" service;
d9a4231b9d/hcn/hcnerrors.go (L75-L77)
But unfortunately, the hns API used by us, does not return typed errors, and
returns HNS errors as a untyped formatted string.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also updated some existing ones to use `%v` instead of `%s` for consistency.
caught by go1.24
# github.com/docker/docker/libnetwork/drivers/windows/overlay
# github.com/docker/docker/libnetwork/drivers/windows/overlay/ov_network_windows.go:206:32: non-constant format string in call to github.com/docker/docker/libnetwork/types.ForbiddenErrorf
FAIL github.com/docker/docker/libnetwork/drivers/windows/overlay [build failed]
# github.com/docker/docker/libnetwork/drivers/windows
# github.com/docker/docker/libnetwork/drivers/windows/windows.go:449:33: non-constant format string in call to github.com/docker/docker/libnetwork/types.ForbiddenErrorf
FAIL github.com/docker/docker/libnetwork/drivers/windows [build failed]
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
This wrapper is now a plain alias for hcsshim.HNSEndpointRequest, so let's
remove the extra abstraction.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This synchronisation was added in [libnetwork@0a61693]:
> Adding synchronization around peerAdd and peerDelete to prevent network
> connectivity issue
>
> When multiple networks are present in a Swarm Cluster, multiple peerAdd
> or peerDelete calls are an issue for different remote endpoints. These
> threads are updating the remote endpoint to HNS parallelly. In 2016 HNS
> code base, we don't have synchronization around remoteEndpoint addition
> and deletion. So serializing the peerAdd and peerDelete calls from docker
> network driver.
We no longer support and test Windows 2016, as it reached EOL / end of
[standard support][1], so we can remove this special condition.
[libnetwork@0a61693]: c90114ce7c
[1]: https://en.wikipedia.org/wiki/Windows_10,_version_1607
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The bridge driver now does its own port-mapping, rather than using the
portmapper module (which ran as two completely separate instances, for
IPv4 and IPv6).
When asked for a mapping from any host address (0.0.0.0/0) with a range
of host ports, the same port will be allocated for IPv4 and IPv6, or the
mapping will fail with an error if that's not possible.
The bridge driver now manages its own port mappings. So, remove
linux-specific PortMapper code and make what's left Windows-only.
Also, replace the portmapper.userlandProxy interface with StartProxy().
Signed-off-by: Rob Murray <rob.murray@docker.com>
A common pattern in libnetwork is to delete an object using
`DeleteAtomic`, ie. to check the optimistic lock, but put in a retry
loop to refresh the data and the version index used by the optimistic
lock.
This commit introduces a new `Delete` method to delete without
checking the optimistic lock. It focuses only on the few places where
it's obvious the calling code doesn't rely on the side-effects of the
retry loop (ie. refreshing the object to be deleted).
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>
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>
None of the code using this function was setting the value, so let's
simplify and remove the argument.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
InvalidParameter is now compatible with errdefs.InvalidParameter. Thus,
these errors will now return a 400 status code instead of a 500.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Follow-up to fca38bcd0a, which made the
Discover API optional for drivers to implement, but forgot to remove the
stubs from the Windows drivers, which didn't implement this API.
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>
It only had a single implementation, so let's remove the interface.
While changing, also renaming;
- datastore.DataStore -> datastore.Store
- datastore.NewDataStore -> datastore.New
- datastore.NewDataStoreFromConfig -> datastore.FromConfig
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
...which ignore the config argument. Notably, none of the network
drivers referenced by Swarmkit use config, which is good as Swarmkit
unconditionally passes nil for the config when registering drivers.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Remove the intermediate variable, and move the option closer
to where it's used, as in some cases we created the variable,
but could return with an error before it was used.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The overlay driver was creating a global store whenever
netlabel.GlobalKVClient was specified in its config argument. This
specific label is unused anymore since 142b522 (moby/moby#44875).
It was also creating a local store whenever netlabel.LocalKVClient was
specificed in its config argument. This store is unused since
moby/libnetwork@9e72136 (moby/libnetwork#1636).
Finally, the sync.Once properties are never used and thus can be
deleted.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Per the Interface Segregation Principle, network drivers should not have
to depend on GetPluginGetter methods they do not use. The remote network
driver is the only one which needs a PluginGetter, and it is already
special-cased in Controller so there is no sense warping the interfaces
to achieve a foolish consistency. Replace all other network drivers' Init
functions with Register functions which take a driverapi.Registerer
argument instead of a driverapi.DriverCallback. Add back in Init wrapper
functions for only the drivers which Swarmkit references so that
Swarmkit can continue to build.
Refactor the libnetwork Controller to use the new drvregistry.Networks
and drvregistry.IPAMs driver registries in place of the legacy ones.
Signed-off-by: Cory Snider <csnider@mirantis.com>
These package-level variables were copied over from the Linux
implementation; drop them for clarity's sake.
Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
strings.ReplaceAll(s, old, new) is a wrapper function for
strings.Replace(s, old, new, -1). But strings.ReplaceAll is more
readable and removes the hardcoded -1.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
