Mostly for readability, and to avoid linters suggesting to move the
default condition outside of the if/else.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These comments were added to enforce using the correct import path for
our packages ("github.com/docker/docker", not "github.com/moby/moby").
However, when working in go module mode (not GOPATH / vendor), they have
no effect, so their impact is limited.
Remove these imports in preparation of migrating our code to become an
actual go module.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `Cause()` function is not part of the standard library and should
not be used or relied on. `pkg/errors` already has compatibility with
the standard library.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Rename the ouput variable to prevent accidental shadowing, and simplify how
we check for the `syscall.ENOTDIR` error; `errors.Is()` will already unwrap
the error, so no type-casting is needed;
package main
import (
"errors"
"fmt"
"os"
"syscall"
)
func main() {
err := &os.PathError{Op: "mkdir", Path: "/hello/world", Err: syscall.ENOTDIR}
if errors.Is(err, syscall.ENOTDIR) {
fmt.Println(err)
}
}
While at it, also improve the code-comment that outlines the intent.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Adding a `default` statement so that disabling the "default-signifies-exhaustive"
linter option will make it show up.
volume/mounts/linux_parser.go:353:2: missing cases in switch of type mount.Type: mount.TypeNamedPipe, mount.TypeCluster (exhaustive)
switch cfg.Type {
^
volume/mounts/windows_parser.go:392:2: missing cases in switch of type mount.Type: mount.TypeTmpfs, mount.TypeCluster, mount.TypeImage (exhaustive)
switch cfg.Type {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This error implemented the Causer interface, but did not implement
the go1.13 unwrapper, which could prevent errors from being matched.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
If we have an error type that we're checking a substring against, we
should really be checking using ErrorContains to indicate the right
semantics to assert.
Mostly done using these transforms:
find . -type f -name "*_test.go" | \
xargs gofmt -w -r 'assert.Assert(t, is.ErrorContains(e, s)) -> assert.ErrorContains(t, e, s)'
find . -type f -name "*_test.go" | \
xargs gofmt -w -r 'assert.Assert(t, is.Contains(err.Error(), s)) -> assert.ErrorContains(t, err, s)'
find . -type f -name "*_test.go" | \
xargs gofmt -w -r 'assert.Check(t, is.Contains(err.Error(), s)) -> assert.Check(t, is.ErrorContains(err, s))'
As well as some small fixups to helpers that were doing
strings.Contains explicitly.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
If a values is non-nil when we don't expect it, it would be quite
helpful to get an error message explaining what happened.
find . -type f -name "*_test.go" | \
xargs gofmt -w -r "assert.Assert(t, a == nil) -> assert.Assert(t, is.Nil(a))"
find . -type f -name "*_test.go" | \
xargs gofmt -w -r "assert.Check(t, a == nil) -> assert.Check(t, is.Nil(a))"
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
volume/local/local_linux_test.go:232:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
volume/local/local_linux_test.go:316:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
volume/mounts/lcow_parser_test.go:260:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
volume/mounts/linux_parser_test.go:253:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
volume/mounts/parser_test.go:82:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
volume/mounts/validate_test.go:59:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
volume/mounts/validate_test.go:117:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
volume/mounts/windows_parser_test.go:283:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The alias is not needed as the package is already named `units`.
It was also not aliases consistently across the project.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Anonymous volumes get a unique, 64-character name, and intended to be a new
volume (not an existing one). While it's theoretically possible for this name
to exist in other volume drivers, this would be very unlikely, so we should
not need to check other drivers to have this volume.
This patch uses the default ("local") volume-driver for anonymous volumes,
unless the user explicitly asked for a specific driver to use. Setting the
driver skips looking up existing volumes in other drivers.
Before this patch:
DEBU[2024-10-26T15:51:12.681547126Z] container mounted via snapshotter: /var/lib/docker/rootfs/overlayfs/7e947822249514b6239657a0c54d091d90e0fed4b09da472f3f6258f2b4920bc container=7e947822249514b6239657a0c54d091d90e0fed4b09da472f3f6258f2b4920bc
DEBU[2024-10-26T15:51:12.681616084Z] Creating anonymous volume volume-name=fd46d688247c3e7d39d9bae4532d6b2dc69e82e354c4a3bf305c50bbfb9ebc6c
DEBU[2024-10-26T15:51:12.681638959Z] Probing all drivers for volume volume-name=fd46d688247c3e7d39d9bae4532d6b2dc69e82e354c4a3bf305c50bbfb9ebc6c
DEBU[2024-10-26T15:51:12.681688917Z] Registering new volume reference driver=local volume-name=fd46d688247c3e7d39d9bae4532d6b2dc69e82e354c4a3bf305c50bbfb9ebc6c
With this patch:
DEBU[2024-10-27T17:28:28.574956716Z] container mounted via snapshotter: /var/lib/docker/rootfs/overlayfs/7085cb3991b61cbb79edffcb6980ad926f99f6b6b3be617cc3e3b92673cc2eb8 container=7085cb3991b61cbb79edffcb6980ad926f99f6b6b3be617cc3e3b92673cc2eb8
DEBU[2024-10-27T17:28:28.575002549Z] Creating anonymous volume driver=local volume-name=db11c053566362499103213542402af2770a6622fe7a90b9a938a5bed84ca937
DEBU[2024-10-27T17:28:28.575016299Z] Registering new volume reference driver=local volume-name=db11c053566362499103213542402af2770a6622fe7a90b9a938a5bed84ca937
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Make sure we're asserting the right fields to be propagated, as there
are some fields that shadow top-level fields (by design).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
ParseMountRaw returns a nil value on error, so there's nothing to
check other than it not returning a value.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These errors used a backtick, which is not commonly used in our errors.
Change them to use single quotes instead.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These parsers can be compiled and used cross-platform, but follow platform-
specific semantics. Remove the use of runtime.GOOS, as it would result in
confusing errors ("linux does not support tmpfs").
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These log-entries were added in 10d57fde44,
but it looks like I accidentally left them as Error-logs following some
debugging (whoops!).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
`Parser.ParseMountRaw()` labels anonymous volumes with a `AnonymousLabel` label
(`com.docker.volume.anonymous`) label based on whether a volume has a name
(named volume) or no name (anonymous) (see [1]).
However both `VolumesService.Create()` (see [1]) and `Parser.ParseMountRaw()`
(see [2], [3]) were generating a random name for anonymous volumes. The latter
is called before `VolumesService.Create()` is called, resulting in such volumes
not being labeled as anonymous.
Generating the name was originally done in Create (fc7b904dce),
but duplicated in b3b7eb2723 with the introduction
of the new Mounts field in HostConfig. Duplicating this effort didn't have a
real effect until (`Create` would just skip generating the name), until
618f26ccbc introduced the `AnonymousLabel` in
(v24.0.0, backported to v23.0.0).
Parsing generally should not fill in defaults / generate names, so this patch;
- Removes generating volume names from `Parser.ParseMountRaw()`
- Adds a debug-log entry to `VolumesService.Create()`
- Touches up some logs to use structured logs for easier correlating logs
With this patch applied:
docker run --rm --mount=type=volume,target=/toto hello-world
DEBU[2024-10-24T22:50:36.359990376Z] creating anonymous volume volume-name=0cfd63d4df363571e7b3e9c04e37c74054cc16ff1d00d9a005232d83e92eda02
DEBU[2024-10-24T22:50:36.360069209Z] probing all drivers for volume volume-name=0cfd63d4df363571e7b3e9c04e37c74054cc16ff1d00d9a005232d83e92eda02
DEBU[2024-10-24T22:50:36.360341209Z] Registering new volume reference driver=local volume-name=0cfd63d4df363571e7b3e9c04e37c74054cc16ff1d00d9a005232d83e92eda02
[1]: 032721ff75/volume/service/service.go (L72-L83)
[2]: 032721ff75/volume/mounts/linux_parser.go (L330-L336)
[3]: 032721ff75/volume/mounts/windows_parser.go (L394-L400)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
volume/testutils/testutils.go:98:26: printf: non-constant format string in call to fmt.Errorf (govet)
return nil, fmt.Errorf(opts["error"])
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
If the system (or Docker) crashes while saivng the volume options, on
restart the daemon will error out when trying to read the options file
because it doesn't contain valid JSON.
In such a crash scenario, the new volume will be treated as though it
has the default options configuration. This is not ideal, but volumes
created on very old Docker versions (pre-1.11[1], circa 2016) do not
have opts.json and so doing some kind of cleanup when loading the volume
store (even if we take care to only delete empty volumes) could delete
existing volumes carried over from very old Docker versions that users
would not expect to disappear.
Ultimately, if a user creates a volume and the system crashes, a volume
that has the wrong config is better than Docker not being able to start.
[1]: commit b05b237075 ("Support mount opts for `local` volume driver")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
