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>
This utility was only used for a single test, and it was very limited
in functionality as it only allowed for a certain error-string to be
matched.
Let's change it into a more generic function; a helper that allows a
container to be created from a `TestContainerConfig` (which can be
constructed using `NewTestConfig`) and that returns the response from
client.ContainerCreate(), so that any result from that can be tested,
leaving it up to the test to check the results.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Introduce a NewTestConfig utility, to allow using the available utilities
for constructing a config, and use them with the regular API client.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `client` variable was colliding with the `client` import. In some cases
the confusing `cli` name (it's not the "cli") was used. Given that such names
can easily start spreading (through copy/paste, or "code by example"), let's
make a one-time pass through all of them in this package to use the same name.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `client` variable was colliding with the `client` import in various
files. While it didn't conflict in all files, there was inconsistency
in the naming, sometimes using the confusing `cli` name (it's not the
"cli"), and such names can easily start spreading (through copy/paste,
or "code by example").
Let's make a one-time pass through all of them in this package to use
the same name.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In these cases, continuing after a non nil error will result in a nil
dereference in panic.
Change the `assert.Check` to `assert.NilError` to avoid that.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This test was testing the client-side validation, so might as well
move it there, and validate that the client invalidates before
trying to make an API call.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Attach the context to the request while we're creating it, instead of
creating the context first, and adding the context later.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Re-use the request, and change the method to GET instead of building
a new request "from scratch".
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Don't exit immediately (due to `set -e` bash behavior) when grep returns
with a non-zero exit code. Use empty dirs instead and let it print
messages about all tests being filtered out.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
To avoid passing the `/` prefix in the -test.run to the integration test
suite, which for some reason executes all tests, but works fine with
integration-cli.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Previous check checked if ANY of the test directories isn't
integration-cli. This means it was true if TEST_FILTER matched multiple
tests from both integration and integration-cli suite.
Remove the grep `-v` inversion and replace it with a bash negation, so
it actually checks if there is no `integration-cli` in test dirs.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
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>
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>
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>
