Commit graph

47039 commits

Author SHA1 Message Date
Sebastiaan van Stijn
87fff769f4
client: Client.checkResponseErr: change errorMessage to an error
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-09 20:30:25 +02:00
Sebastiaan van Stijn
89b542b421
Merge pull request #46175 from vvoland/pkg-plugins-gotest
pkg/plugins: Rewrite with assert.Check
2023-08-09 13:44:05 +02:00
Sebastiaan van Stijn
cfc117826a
Merge pull request #46168 from vvoland/hack-integrationcli-dont-failfast
hack/test: Don't fail-fast before integration-cli
2023-08-09 13:43:43 +02:00
Sebastiaan van Stijn
1614b534fe
Merge pull request #46169 from vvoland/hack-integration-testfilter-fixes
hack/test: Fix `TEST_FILTER` with regex matching integration and integration-cli tests
2023-08-09 13:29:46 +02:00
Paweł Gronowski
11a0c2779b
pkg/plugins: Rewrite with assert.Check
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2023-08-09 12:22:25 +02:00
Sebastiaan van Stijn
6598cba32f
Merge pull request #46174 from thaJeztah/libnetwork_osl_cleanups
libnetwork/osl: remove redundant locks, and assorted cleanups
2023-08-09 12:17:54 +02:00
Paweł Gronowski
43956e1b71
hack/test: Don't exit early when all tests are filtered out
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>
2023-08-09 09:47:02 +02:00
Paweł Gronowski
ae008570ff
hack/test: Split -test.run for integration and integration-cli
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>
2023-08-09 09:47:01 +02:00
Paweł Gronowski
510cac5f5b
hack/test: Fix checking if integration-cli are filtered out
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>
2023-08-09 09:47:00 +02:00
Sebastiaan van Stijn
2cccb1f02c
Merge pull request #46075 from thaJeztah/libnetwork_remove_NetworkInfo
libnetwork: remove Network.Info() and remove NetworkInfo interface
2023-08-09 01:15:44 +02:00
Sebastiaan van Stijn
8a1ca49657
libnetwork/osl: nwIface: add godoc
Copy the godoc from the interface to the implementation.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 23:05:42 +02:00
Sebastiaan van Stijn
16785b9b7b
libnetwork/osl: move all networkNamespace methods together
These methods were sprinkled throughout the code; let's move
them together.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 23:04:19 +02:00
Sebastiaan van Stijn
5b0fa7aaca
libnetwork/osl: some minor nits
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 22:33:29 +02:00
Sebastiaan van Stijn
972d80b596
libnetwork/osl: clean up newInfo() a bit
Use struct-literals in some places to make it slightly more visible
what we're creating where.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 22:33:28 +02:00
Sebastiaan van Stijn
0da721ec38
libnetwork/osl: make newKey and newInfo a t.Helper()
Both were passed testing.T, but it was not used, so let's make use of it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 22:33:28 +02:00
Sebastiaan van Stijn
d9442aab88
libnetwork/osl: nwIface: remove mutex altogether
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>
2023-08-08 22:33:07 +02:00
Sebastiaan van Stijn
2afe18d2ce
libnetwork/osl: nwIface: unexport sync.Mutex
Don't make the mutex public. This also gives a better clue
if the mutex is used externally.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 22:32:26 +02:00
Sebastiaan van Stijn
8b989ac665
libnetwork/osl: let's not do this, etc.
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>
2023-08-08 22:15:13 +02:00
Sebastiaan van Stijn
3d0a7d819c
libnetwork: remove Network.Info() and remove NetworkInfo interface
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 22:05:32 +02:00
Sebastiaan van Stijn
74354043ff
remove uses of libnetwork/Network.Info()
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>
2023-08-08 22:05:30 +02:00
Sebastiaan van Stijn
2b449e0e65
Merge pull request #46173 from akerouanton/remove-unused-ipam-errors
libnet/ipamapi: Remove unused errors
2023-08-08 21:56:08 +02:00
Albin Kerouanton
36a0946aa9
libnet/ipamapi: Remove unused errors
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>
2023-08-08 19:55:47 +02:00
Sebastiaan van Stijn
2884018e7f
Merge pull request #46051 from thaJeztah/ipam_clean
libnetwork/ipam: assorted cleanup and refactor
2023-08-08 18:39:00 +02:00
Sebastiaan van Stijn
2ef7b479a5
Merge pull request #46166 from thaJeztah/pkg_plugin_cleanup_STEP3
pkg/plugins: override timeouts during tests
2023-08-08 18:38:08 +02:00
Sebastiaan van Stijn
0485b8f0fb
Merge pull request #46170 from akerouanton/daemon-network-replace-pkg-errors
daemon/network.go: Remove github.com/pkg/errors pkg
2023-08-08 18:37:31 +02:00
Albin Kerouanton
fd4ec26313
daemon/network.go: Fix error capitalization
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2023-08-08 17:40:19 +02:00
Albin Kerouanton
64e01c627f
daemon/network.go: Remove github.com/pkg/errors pkg
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>
2023-08-08 17:39:53 +02:00
Sebastiaan van Stijn
94dc10378d
libnetwork: network.requestPoolHelper: slightly optimize order of checks
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>
2023-08-08 15:57:24 +02:00
Sebastiaan van Stijn
67e2c1d482
libnetwork: network.requestPoolHelper: remove dead code
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>
2023-08-08 15:57:24 +02:00
Sebastiaan van Stijn
ad68883c5a
libnetwork: network.requestPoolHelper: don't defer in a loop
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>
2023-08-08 15:57:24 +02:00
Sebastiaan van Stijn
32fcde6d9e
libnetwork: network.IpamConfig, network.IpamInfo: name output vars
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>
2023-08-08 15:57:24 +02:00
Sebastiaan van Stijn
df03357d19
libnetwork/ipam: move PoolID.FromString() to a PoolIDFromString() func
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>
2023-08-08 15:57:24 +02:00
Sebastiaan van Stijn
808fed550d
libnetwork/ipam: PoolID.String(): don't use fmt.Sprintf
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>
2023-08-08 15:57:24 +02:00
Sebastiaan van Stijn
87fc8c772b
libnetwork/ipam: Allocator.RequestPool: name args, output vars
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>
2023-08-08 15:57:20 +02:00
Sebastiaan van Stijn
6dbc9c1c53
libnetwork/ipam: Allocator.RequestPool: mark options arg as unused
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>
2023-08-08 15:56:24 +02:00
Sebastiaan van Stijn
7047964bd6
libnetwork/ipam: Allocator.RequestPool: make parseErr only handle errors
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>
2023-08-08 15:56:24 +02:00
Sebastiaan van Stijn
821ef5cbaf
libnetwork/ipams/null: use consts for fixed values
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 15:56:22 +02:00
Sebastiaan van Stijn
05ef5559c3
pkg/plugins: override timeouts during tests
Some tests were testing non-existing plugins, but therefore triggered
the retry-loop, which times out after 15-30 seconds. Add some options
to allow overriding this timeout during tests.

Before:

    go test -v -run '^(TestGet|TestNewClientWithTimeout)$'
    === RUN   TestGet
    === RUN   TestGet/success
    === RUN   TestGet/not_implemented
    === RUN   TestGet/not_exists
    WARN[0000] Unable to locate plugin: vegetable, retrying in 1s
    WARN[0001] Unable to locate plugin: vegetable, retrying in 2s
    WARN[0003] Unable to locate plugin: vegetable, retrying in 4s
    WARN[0007] Unable to locate plugin: vegetable, retrying in 8s
    --- PASS: TestGet (15.02s)
        --- PASS: TestGet/success (0.00s)
        --- PASS: TestGet/not_implemented (0.00s)
        --- PASS: TestGet/not_exists (15.02s)
    === RUN   TestNewClientWithTimeout
        client_test.go:166: started remote plugin server listening on: http://127.0.0.1:36275
    WARN[0015] Unable to connect to plugin: 127.0.0.1:36275/Test.Echo: Post "http://127.0.0.1:36275/Test.Echo": context deadline exceeded (Client.Timeout exceeded while awaiting headers), retrying in 1s
    WARN[0017] Unable to connect to plugin: 127.0.0.1:36275/Test.Echo: Post "http://127.0.0.1:36275/Test.Echo": context deadline exceeded (Client.Timeout exceeded while awaiting headers), retrying in 2s
    WARN[0019] Unable to connect to plugin: 127.0.0.1:36275/Test.Echo: Post "http://127.0.0.1:36275/Test.Echo": net/http: request canceled (Client.Timeout exceeded while awaiting headers), retrying in 4s
    WARN[0024] Unable to connect to plugin: 127.0.0.1:36275/Test.Echo: Post "http://127.0.0.1:36275/Test.Echo": net/http: request canceled (Client.Timeout exceeded while awaiting headers), retrying in 8s
    --- PASS: TestNewClientWithTimeout (17.64s)
    PASS
    ok  	github.com/docker/docker/pkg/plugins	32.664s

After:

    go test -v -run '^(TestGet|TestNewClientWithTimeout)$'
    === RUN   TestGet
    === RUN   TestGet/success
    === RUN   TestGet/not_implemented
    === RUN   TestGet/not_exists
    WARN[0000] Unable to locate plugin: this-plugin-does-not-exist, retrying in 1s
    --- PASS: TestGet (1.00s)
        --- PASS: TestGet/success (0.00s)
        --- PASS: TestGet/not_implemented (0.00s)
        --- PASS: TestGet/not_exists (1.00s)
    === RUN   TestNewClientWithTimeout
        client_test.go:167: started remote plugin server listening on: http://127.0.0.1:45973
    --- PASS: TestNewClientWithTimeout (0.04s)
    PASS
    ok  	github.com/docker/docker/pkg/plugins	1.050s

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 15:41:11 +02:00
Sebastiaan van Stijn
63d477b20e
Merge pull request #46039 from thaJeztah/cleanup_bridge
libnetwork/drivers/bridge: assorted cleanups
2023-08-08 14:06:50 +02:00
Sebastiaan van Stijn
f9cae2acbe
Merge pull request #46165 from akerouanton/remove-api-CheckDuplicate-warning
api: Remove duplicated check on CheckDuplicate
2023-08-08 13:42:17 +02:00
Paweł Gronowski
6841a53d17
hack/test: Don't fail-fast before integration-cli
If TEST_INTEGRATION_FAIL_FAST is not set, run the integration-cli tests
even if integration tests failed.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2023-08-08 13:29:54 +02:00
Sebastiaan van Stijn
4ab4330677
Merge pull request #46080 from thaJeztah/pkg_plugin_cleanup_STEP2
pkg/plugins: some cleaning up (step 2)
2023-08-08 12:28:16 +02:00
Sebastiaan van Stijn
2aa24519da
ibnetwork/drivers/bridge: newLink: validate before creating
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 11:50:40 +02:00
Sebastiaan van Stijn
5d722b35d9
libnetwork/drivers/bridge: bridgeNetwork.getEndpoint(): move lock
Don't lock if there's no need to.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 11:50:39 +02:00
Sebastiaan van Stijn
eba15fe905
libnetwork/drivers/bridge: driver.link: don't defer in a loop
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>
2023-08-08 11:50:39 +02:00
Sebastiaan van Stijn
76b736c242
libnetwork/drivers/bridge: driver.link: name return var for defer handling
Name the return variable to prevent accidental shadowing of the error,
which is used in defers.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 11:50:39 +02:00
Sebastiaan van Stijn
ea5f21ceac
libnetwork/drivers/bridge: don't convert IP to string and back again
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 11:50:39 +02:00
Sebastiaan van Stijn
8b6203b613
libnetwork/drivers/bridge: link.Enable: don't register reload on error
Only register a reload function if we actually managed to enable the link.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-08 11:50:34 +02:00
Sebastiaan van Stijn
0f4ba145ee
libnetwork/drivers/bridge: link.Enable, link.Disable use iptables.Action
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>
2023-08-08 11:47:31 +02:00
Sebastiaan van Stijn
f2be77a5ab
Merge pull request #46058 from thaJeztah/cleanup_buildSandboxOptions
daemon: cleanup Daemon.buildSandboxOptions
2023-08-08 11:38:24 +02:00