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>
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>
These tests were made parallel to speed up the execution, but this
turned out to be flaky, because they mutate some shared state.
The tests use shared `storage` variable without any synchronization.
However, adding synchronization is not enough in all cases, some tests
register the same plugin, so they can't be run in parallel to each
other.
This commit adds the synchronization around `storage` variable
modification and removes parallel from the tests where it's not enough.
Before:
```
$ go test -race -v . -count 1
...
--- FAIL: TestGet (15.02s)
--- FAIL: TestGet/not_implemented (0.00s)
testing.go:1446: race detected during execution of test
testing.go:1446: race detected during execution of test
FAIL
FAIL github.com/docker/docker/pkg/plugins 17.655s
FAIL
```
After:
```
$ go test -race -v . -count 1
ok github.com/docker/docker/pkg/plugins 32.702s
```
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This variable was only accessed from within LocalRegistry methods, but
due to being a package-level variable, tests had to deal with setting
and resetting it.
Move it to be a field scoped to the LocalRegistry. This simplifies the
tests, and to make this more transparent, also removing the "Setup()"
helper (which, wasn't marked as a t.Helper()).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The client's transport can only be set by newClientWithTransport, which
is not exported, and always uses a transport.HTTPTransport.
However, requestFactory is mocked in one of the tests, so keep the interface,
but make it a local, non-exported one.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The interface is not consumed anywhere, and only non-exported functions
produced one, so we can remove it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This field was exported, but never mutated outside of the package, and
effectively a rather "creative" way to define a method on LocalRegistry.
While un-exporting also store these paths in a field, instead of constructing
them on every call, as the results won't change during the lifecycle of the
LocalRegistry.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Split the exported SpecsPaths from the platform-specific implementations,
so that documentation can be maintained in a single location.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since 0e5eaf8ee3, these implementations
were fully identical, so removing the duplicate, and move it to a
platform-agnostic file.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
For local communications (npipe://, unix://), the hostname is not used,
but we need valid and meaningful hostname.
The current code used the socket path as hostname, which gets rejected by
go1.20.6 and go1.19.11 because of a security fix for [CVE-2023-29406 ][1],
which was implemented in https://go.dev/issue/60374.
Prior versions go Go would clean the host header, and strip slashes in the
process, but go1.20.6 and go1.19.11 no longer do, and reject the host
header.
Before this patch, tests would fail on go1.20.6:
=== FAIL: pkg/authorization TestAuthZRequestPlugin (15.01s)
time="2023-07-12T12:53:45Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 1s"
time="2023-07-12T12:53:46Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 2s"
time="2023-07-12T12:53:48Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 4s"
time="2023-07-12T12:53:52Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 8s"
authz_unix_test.go:82: Failed to authorize request Post "http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq": http: invalid Host header
[1]: https://github.com/advisories/GHSA-f8f7-69v5-w4vx
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Some tests are testing timeouts and take a long time to run. Run the tests
in parallel, so that the test-suite takes shorter to run.
Before:
ok github.com/docker/docker/pkg/plugins 34.013s
After:
ok github.com/docker/docker/pkg/plugins 17.945s
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Refactor setupRemotePluginServer() to be a helper, and to spin up a test-
server for each test instead of sharing the same instance between tests.
This allows the tests to be run in parallel without stepping on each-other's
toes (tearing down the server).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
TestClientWithRequestTimeout has been observed to flake in CI. The
timing in the test is quite tight, only giving the client a 10ms window
to time out, which could potentially be missed if the host is under
load and the goroutine scheduling is unlucky. Give the client a full
five seconds of grace to time out before failing the test.
Signed-off-by: Cory Snider <csnider@mirantis.com>
pkg/directory/directory.go:9:49: empty-lines: extra empty line at the start of a block (revive)
pkg/pubsub/publisher.go:8:48: empty-lines: extra empty line at the start of a block (revive)
pkg/loopback/attach_loopback.go:96:69: empty-lines: extra empty line at the start of a block (revive)
pkg/devicemapper/devmapper_wrapper.go:136:48: empty-lines: extra empty line at the start of a block (revive)
pkg/devicemapper/devmapper.go:391:35: empty-lines: extra empty line at the end of a block (revive)
pkg/devicemapper/devmapper.go:676:35: empty-lines: extra empty line at the end of a block (revive)
pkg/archive/changes_posix_test.go:15:38: empty-lines: extra empty line at the end of a block (revive)
pkg/devicemapper/devmapper.go:241:51: empty-lines: extra empty line at the start of a block (revive)
pkg/fileutils/fileutils_test.go:17:47: empty-lines: extra empty line at the end of a block (revive)
pkg/fileutils/fileutils_test.go:34:48: empty-lines: extra empty line at the end of a block (revive)
pkg/fileutils/fileutils_test.go:318:32: empty-lines: extra empty line at the end of a block (revive)
pkg/tailfile/tailfile.go:171:6: empty-lines: extra empty line at the end of a block (revive)
pkg/tarsum/fileinfosums_test.go:16:41: empty-lines: extra empty line at the end of a block (revive)
pkg/tarsum/tarsum_test.go:198:42: empty-lines: extra empty line at the start of a block (revive)
pkg/tarsum/tarsum_test.go:294:25: empty-lines: extra empty line at the start of a block (revive)
pkg/tarsum/tarsum_test.go:407:34: empty-lines: extra empty line at the end of a block (revive)
pkg/ioutils/fswriters_test.go:52:45: empty-lines: extra empty line at the end of a block (revive)
pkg/ioutils/writers_test.go:24:39: empty-lines: extra empty line at the end of a block (revive)
pkg/ioutils/bytespipe_test.go:78:26: empty-lines: extra empty line at the end of a block (revive)
pkg/sysinfo/sysinfo_linux_test.go:13:37: empty-lines: extra empty line at the end of a block (revive)
pkg/archive/archive_linux_test.go:57:64: empty-lines: extra empty line at the end of a block (revive)
pkg/archive/changes.go:248:72: empty-lines: extra empty line at the start of a block (revive)
pkg/archive/changes_posix_test.go:15:38: empty-lines: extra empty line at the end of a block (revive)
pkg/archive/copy.go:248:124: empty-lines: extra empty line at the end of a block (revive)
pkg/archive/diff_test.go:198:44: empty-lines: extra empty line at the end of a block (revive)
pkg/archive/archive.go:304:12: empty-lines: extra empty line at the end of a block (revive)
pkg/archive/archive.go:749:37: empty-lines: extra empty line at the end of a block (revive)
pkg/archive/archive.go:812:81: empty-lines: extra empty line at the start of a block (revive)
pkg/archive/copy_unix_test.go:347:34: empty-lines: extra empty line at the end of a block (revive)
pkg/system/path.go:11:39: empty-lines: extra empty line at the end of a block (revive)
pkg/system/meminfo_linux.go:29:21: empty-lines: extra empty line at the end of a block (revive)
pkg/plugins/plugins.go:135:32: empty-lines: extra empty line at the end of a block (revive)
pkg/authorization/response.go:71:48: empty-lines: extra empty line at the start of a block (revive)
pkg/authorization/api_test.go:18:51: empty-lines: extra empty line at the end of a block (revive)
pkg/authorization/middleware_test.go:23:44: empty-lines: extra empty line at the end of a block (revive)
pkg/authorization/middleware_unix_test.go:17:46: empty-lines: extra empty line at the end of a block (revive)
pkg/authorization/api_test.go:57:45: empty-lines: extra empty line at the end of a block (revive)
pkg/authorization/response.go:83:50: empty-lines: extra empty line at the start of a block (revive)
pkg/authorization/api_test.go:66:47: empty-lines: extra empty line at the end of a block (revive)
pkg/authorization/middleware_unix_test.go:45:48: empty-lines: extra empty line at the end of a block (revive)
pkg/authorization/response.go:145:75: empty-lines: extra empty line at the start of a block (revive)
pkg/authorization/middleware_unix_test.go:56:51: empty-lines: extra empty line at the end of a block (revive)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The 1.16 `io/fs` compatibility code was being built on 1.18 and 1.19.
Drop it completely as 1.16 is long EOL, and additionally drop 1.17 as it
has been EOL for a month and 1.18 is both the minimum Go supported by
the 20.10 branch, as well as a very easy jump from 1.17.
Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
Older versions of Go don't format comments, so committing this as
a separate commit, so that we can already make these changes before
we upgrade to Go 1.19.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function is marked deprecated in Go 1.18; however, the suggested replacement
brings in a large amount of new code, and most strings we generate will be ASCII,
so this would only be in case it's used for some user-provided string. We also
don't have a language to use, so would be using the "default".
Adding a `//nolint` comment to suppress the linting failure instead.
daemon/logger/templates/templates.go:23:14: SA1019: strings.Title is deprecated: The rule Title uses for word boundaries does not handle Unicode punctuation properly. Use golang.org/x/text/cases instead. (staticcheck)
"title": strings.Title,
^
pkg/plugins/pluginrpc-gen/template.go:67:9: SA1019: strings.Title is deprecated: The rule Title uses for word boundaries does not handle Unicode punctuation properly. Use golang.org/x/text/cases instead. (staticcheck)
return strings.Title(s)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
commit c55a4ac779 changed the ioutil utilities
to use the new os variants, per recommendation from the go 1.16 release notes:
https://golang.org/doc/go1.16#ioutil
> we encourage new code to use the new definitions in the io and os packages.
> Here is a list of the new locations of the names exported by io/ioutil:
However, the devil is in the detail, and io.ReadDir() is not a direct
replacement for ioutil.ReadDir();
> ReadDir => os.ReadDir (note: returns a slice of os.DirEntry rather than a slice of fs.FileInfo)
go1.16 added a io.FileInfoToDirEntry() utility to concert a DirEntry to
a FileInfo, but it's not available in go1.16
This patch copies the FileInfoToDirEntry code, and uses it for go1.16.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The io/ioutil package has been deprecated in Go 1.16. This commit
replaces the existing io/ioutil functions with their new definitions in
io and os packages.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
The test sometimes failed because no error was returned:
=== Failed
=== FAIL: pkg/plugins TestClientWithRequestTimeout (0.00s)
client_test.go:254: assertion failed: expected an error, got nil: expected error
Possibly caused by a race condition, as the sleep was just 1 ms longer than the timeout;
this patch is increasing the sleep in the response to try to reduce flakiness.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Seeing this `ErrNotImplements` in some of our logs and it's not very
helpful because we don't know what plugin is causing it or even what the
requested interface is.
```
{"message":"legacy plugin: Plugin does not implement the requested driver"}
```
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This can happen when you have --config-only network
Such attempt will fail anyway and it will create 15s delay in container
startup
Signed-off-by: Pavel Matěja <pavel@verotel.cz>
