The mutex is only used on reads, but there's nothing protecting writes,
and it looks like nothing is mutating fields after creation, so let's
remove this altogether.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
No context in the commit that added it, but PR discussion shows that
the API was mostly exploratory, and it was 8 Years go, so let's not
head in that direction :) b646784859
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>
These errors aren't used in our repo and seem unused by the OSS
community (this was checked with Sourcegraph).
- ErrIpamInternalError has never been used
- ErrInvalidRequest is unused since moby/libnetwork@c85356efa
- ErrPoolNotFound has never been used
- ErrOverlapPool has never been used
- ErrNoAvailablePool has never been used
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Check the preferredPool first, as other checks could be doing more
(such as locking, or validating / parsing). Also adding a note, as
it's unclear why we're ignoring invalid pools here.
The "invalid" conditions was added in [libnetwork#1095][1], which
moved code to reduce os-specific dependencies in the ipam package,
but also introduced a types.IsIPNetValid() function, which considers
"0.0.0.0/0" invalid, and added it to the condition to return early.
Unfortunately review does not mention this change, so there's no
context why. Possibly this was done to prevent errors further down
the line (when checking for overlaps), but returning an error here
instead would likely have avoided that as well, so we can only guess.
To make this code slightly more transparent, this patch also inlines
the "types.IsIPNetValid" function, as it's not used anywhere else,
and inlining it makes it more visible.
[1]: 5ca79d6b87 (diff-bdcd879439d041827d334846f9aba01de6e3683ed8fdd01e63917dae6df23846)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This code was only run if no preferred pool was specified, however,
since [libnetwork#1162][2], the function would already return early
if a preferred pools was set (and the overlap check to be skipped),
so this was now just dead code.
[2]: 9cc3385f44
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function intentionally holds a lock / lease on address-pools to
prevent trying the same pool repeatedly.
Let's try to make this logic slightly more transparent, and prevent
defining defers in a loop. Releasing all the pools in a singe defer
also allows us to get the network-name once, which prevents locking
and unlocking the network for each iteration.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Both functions have multiple output vars with generic types, which made
it hard to grasp what's what.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This makes it easier to consume, without first having to create an empty
PoolID.
Performance is the same:
BenchmarkPoolIDFromString-10 6100345 196.5 ns/op 112 B/op 3 allocs/op
BenchmarkPoolIDFromString-10 6252750 192.0 ns/op 112 B/op 3 allocs/op
Note that I opted not to change the return-type to a pointer, as that seems
to perform less;
BenchmarkPoolIDFromString-10 6252750 192.0 ns/op 112 B/op 3 allocs/op
BenchmarkPoolIDFromString-10 5288682 226.6 ns/op 192 B/op 4 allocs/op
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
As this function may be called repeatedly to convert to/from a string,
it may be worth optimizing it a bit. Adding a minimal Benchmark for
it as well.
Before/after:
BenchmarkPoolIDToString-10 2842830 424.3 ns/op 232 B/op 12 allocs/op
BenchmarkPoolIDToString-10 7176738 166.8 ns/op 112 B/op 7 allocs/op
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
network.requestPoolHelper and Allocator.RequestPool have many args and
output vars with generic types. Add names for them to make it easier to
grasp what's what.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The options are unused, other than for debug-logging, which made it look
as if they were actually consumed anywhere, but they aren't.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This makes it slightly more readable to see what's returned in each of
the code-paths. Also move validation of pool/subpool earlier in the
function.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Collect a list of all the links we successfully enabled (if any), and
use a single defer to disable them.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The iptables package has types defined for these actions; use them directly
instead of creating a string only to convert it to a known value.
As the linkContainers() function is only used internally, and with fixed
values, we can also remove the validation, and InvalidIPTablesCfgError
error, which is now unused.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Partially revert commit 94b880f.
The CheckDuplicate field has been introduced in commit 2ab94e1. At that
time, this check was done in the network router. It was then moved to
the daemon package in commit 3ca2982. However, commit 94b880f duplicated
the logic into the network router for no apparent reason. Finally,
commit ab18718 made sure a 409 would be returned instead of a 500.
As this logic is first done by the daemon, the error -> warning
conversion can't happen because CheckDuplicate has to be true for the
daemon package to return an error. If it's false, the daemon proceed
with the network creation, set the Warning field of its return value and
return no error.
Thus, the CheckDuplicate logic in the api is removed and
libnetwork.NetworkNameError now implements the ErrConflict interface.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The MediaType was changed twice in;
- b3b7eb2723 ("application/vnd.docker.plugins.v1+json" -> "application/vnd.docker.plugins.v1.1+json")
- 54587d861d ("application/vnd.docker.plugins.v1.1+json" -> "application/vnd.docker.plugins.v1.2+json")
But the (integration) tests were still using the old version, so let's
use the VersionMimeType const that's defined, and use the updated version.
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>
IPv4AddrNoMatchError and IPv6AddrNoMatchError are currently implementing
BadRequestError. They are returned in two cases, and none are due to a
bad user request:
- When calling daemon's CreateNetwork route, if the bridge's IPv4
address or none of the bridge's IPv6 addresses match what's requested.
If that happens, there's a big issue somewhere in libnetwork or the
kernel.
- When restoring a network, for the same reason. In that case, the
on-disk state drifted from the interface state.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This error can only be reached because of an error in our code, so it's
not a "bad user request". As it's never type asserted, no need to keep
it around.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This error is only used in defensive checks whereas the precondition is
already checked by caller. If we reach it, we messed something else. So
it's definitely not a BadRequest. Also, it's not type asserted anywhere,
so just inline it.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
