Merge pull request #6186 from thaJeztah/remove_more_aliases

opts: minor cleanup in tests, and remove some import aliases
This commit is contained in:
Sebastiaan van Stijn
2025-07-17 17:18:15 +02:00
committed by GitHub
5 changed files with 143 additions and 134 deletions
+12 -12
View File
@@ -16,8 +16,8 @@ import (
"github.com/docker/cli/internal/lazyregexp"
"github.com/docker/cli/opts"
"github.com/docker/docker/api/types/container"
mounttypes "github.com/docker/docker/api/types/mount"
networktypes "github.com/docker/docker/api/types/network"
"github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/network"
"github.com/docker/go-connections/nat"
"github.com/pkg/errors"
"github.com/spf13/pflag"
@@ -323,7 +323,7 @@ func addFlags(flags *pflag.FlagSet) *containerOptions {
type containerConfig struct {
Config *container.Config
HostConfig *container.HostConfig
NetworkingConfig *networktypes.NetworkingConfig
NetworkingConfig *network.NetworkingConfig
}
// parse parses the args for the specified command and generates a Config,
@@ -372,7 +372,7 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
if parsed.Source != "" {
toBind := bind
if parsed.Type == string(mounttypes.TypeBind) {
if parsed.Type == string(mount.TypeBind) {
if hostPart, targetPath, ok := strings.Cut(bind, ":"); ok {
if !filepath.IsAbs(hostPart) && strings.HasPrefix(hostPart, ".") {
if absHostPart, err := filepath.Abs(hostPart); err == nil {
@@ -706,8 +706,8 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
config.StdinOnce = true
}
networkingConfig := &networktypes.NetworkingConfig{
EndpointsConfig: make(map[string]*networktypes.EndpointSettings),
networkingConfig := &network.NetworkingConfig{
EndpointsConfig: make(map[string]*network.EndpointSettings),
}
networkingConfig.EndpointsConfig, err = parseNetworkOpts(copts)
@@ -735,9 +735,9 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
// this function may return _multiple_ endpoints, which is not currently supported
// by the daemon, but may be in future; it's up to the daemon to produce an error
// in case that is not supported.
func parseNetworkOpts(copts *containerOptions) (map[string]*networktypes.EndpointSettings, error) {
func parseNetworkOpts(copts *containerOptions) (map[string]*network.EndpointSettings, error) {
var (
endpoints = make(map[string]*networktypes.EndpointSettings, len(copts.netMode.Value()))
endpoints = make(map[string]*network.EndpointSettings, len(copts.netMode.Value()))
hasUserDefined, hasNonUserDefined bool
)
@@ -783,7 +783,7 @@ func parseNetworkOpts(copts *containerOptions) (map[string]*networktypes.Endpoin
// and only a single network is specified, omit the endpoint-configuration
// on the client (the daemon will still create it when creating the container)
if i == 0 && len(copts.netMode.Value()) == 1 {
if ep == nil || reflect.DeepEqual(*ep, networktypes.EndpointSettings{}) {
if ep == nil || reflect.DeepEqual(*ep, network.EndpointSettings{}) {
continue
}
}
@@ -841,7 +841,7 @@ func applyContainerOptions(n *opts.NetworkAttachmentOpts, copts *containerOption
return nil
}
func parseNetworkAttachmentOpt(ep opts.NetworkAttachmentOpts) (*networktypes.EndpointSettings, error) {
func parseNetworkAttachmentOpt(ep opts.NetworkAttachmentOpts) (*network.EndpointSettings, error) {
if strings.TrimSpace(ep.Target) == "" {
return nil, errors.New("no name set for network")
}
@@ -854,7 +854,7 @@ func parseNetworkAttachmentOpt(ep opts.NetworkAttachmentOpts) (*networktypes.End
}
}
epConfig := &networktypes.EndpointSettings{
epConfig := &network.EndpointSettings{
GwPriority: ep.GwPriority,
}
epConfig.Aliases = append(epConfig.Aliases, ep.Aliases...)
@@ -866,7 +866,7 @@ func parseNetworkAttachmentOpt(ep opts.NetworkAttachmentOpts) (*networktypes.End
epConfig.Links = ep.Links
}
if ep.IPv4Address != "" || ep.IPv6Address != "" || len(ep.LinkLocalIPs) > 0 {
epConfig.IPAMConfig = &networktypes.EndpointIPAMConfig{
epConfig.IPAMConfig = &network.EndpointIPAMConfig{
IPv4Address: ep.IPv4Address,
IPv6Address: ep.IPv6Address,
LinkLocalIPs: ep.LinkLocalIPs,
+2 -2
View File
@@ -11,7 +11,7 @@ import (
"github.com/docker/cli/cli/command/formatter"
"github.com/docker/cli/cli/command/inspect"
"github.com/docker/docker/api/types/container"
mounttypes "github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/network"
"github.com/docker/docker/api/types/swarm"
"github.com/docker/go-units"
@@ -461,7 +461,7 @@ func (ctx *serviceInspectContext) ContainerInit() bool {
return *ctx.Service.Spec.TaskTemplate.ContainerSpec.Init
}
func (ctx *serviceInspectContext) ContainerMounts() []mounttypes.Mount {
func (ctx *serviceInspectContext) ContainerMounts() []mount.Mount {
return ctx.Service.Spec.TaskTemplate.ContainerSpec.Mounts
}
+13 -13
View File
@@ -13,7 +13,7 @@ import (
"github.com/docker/cli/opts"
"github.com/docker/cli/opts/swarmopts"
"github.com/docker/docker/api/types/container"
mounttypes "github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/network"
"github.com/docker/docker/api/types/swarm"
"github.com/docker/docker/api/types/versions"
@@ -943,33 +943,33 @@ func removeItems(
return newSeq
}
func updateMounts(flags *pflag.FlagSet, mounts *[]mounttypes.Mount) error {
mountsByTarget := map[string]mounttypes.Mount{}
func updateMounts(flags *pflag.FlagSet, mounts *[]mount.Mount) error {
mountsByTarget := map[string]mount.Mount{}
if flags.Changed(flagMountAdd) {
values := flags.Lookup(flagMountAdd).Value.(*opts.MountOpt).Value()
for _, mount := range values {
if _, ok := mountsByTarget[mount.Target]; ok {
for _, mnt := range values {
if _, ok := mountsByTarget[mnt.Target]; ok {
return errors.Errorf("duplicate mount target")
}
mountsByTarget[mount.Target] = mount
mountsByTarget[mnt.Target] = mnt
}
}
// Add old list of mount points minus updated one.
for _, mount := range *mounts {
if _, ok := mountsByTarget[mount.Target]; !ok {
mountsByTarget[mount.Target] = mount
for _, mnt := range *mounts {
if _, ok := mountsByTarget[mnt.Target]; !ok {
mountsByTarget[mnt.Target] = mnt
}
}
newMounts := []mounttypes.Mount{}
newMounts := make([]mount.Mount, 0, len(mountsByTarget))
toRemove := buildToRemoveSet(flags, flagMountRemove)
for _, mount := range mountsByTarget {
if _, exists := toRemove[mount.Target]; !exists {
newMounts = append(newMounts, mount)
for _, mnt := range mountsByTarget {
if _, exists := toRemove[mnt.Target]; !exists {
newMounts = append(newMounts, mnt)
}
}
sort.Slice(newMounts, func(i, j int) bool {
+8 -8
View File
@@ -9,7 +9,7 @@ import (
"time"
"github.com/docker/docker/api/types/container"
mounttypes "github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/network"
"github.com/docker/docker/api/types/swarm"
"gotest.tools/v3/assert"
@@ -237,9 +237,9 @@ func TestUpdateMounts(t *testing.T) {
flags.Set("mount-add", "type=volume,source=vol2,target=/toadd")
flags.Set("mount-rm", "/toremove")
mounts := []mounttypes.Mount{
{Target: "/toremove", Source: "vol1", Type: mounttypes.TypeBind},
{Target: "/tokeep", Source: "vol3", Type: mounttypes.TypeBind},
mounts := []mount.Mount{
{Target: "/toremove", Source: "vol1", Type: mount.TypeBind},
{Target: "/tokeep", Source: "vol3", Type: mount.TypeBind},
}
updateMounts(flags, &mounts)
@@ -252,10 +252,10 @@ func TestUpdateMountsWithDuplicateMounts(t *testing.T) {
flags := newUpdateCommand(nil).Flags()
flags.Set("mount-add", "type=volume,source=vol4,target=/toadd")
mounts := []mounttypes.Mount{
{Target: "/tokeep1", Source: "vol1", Type: mounttypes.TypeBind},
{Target: "/toadd", Source: "vol2", Type: mounttypes.TypeBind},
{Target: "/tokeep2", Source: "vol3", Type: mounttypes.TypeBind},
mounts := []mount.Mount{
{Target: "/tokeep1", Source: "vol1", Type: mount.TypeBind},
{Target: "/toadd", Source: "vol2", Type: mount.TypeBind},
{Target: "/tokeep2", Source: "vol3", Type: mount.TypeBind},
}
updateMounts(flags, &mounts)
+108 -99
View File
@@ -5,28 +5,28 @@ import (
"path/filepath"
"testing"
mounttypes "github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/mount"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)
func TestMountOptString(t *testing.T) {
mount := MountOpt{
values: []mounttypes.Mount{
m := MountOpt{
values: []mount.Mount{
{
Type: mounttypes.TypeBind,
Type: mount.TypeBind,
Source: "/home/path",
Target: "/target",
},
{
Type: mounttypes.TypeVolume,
Type: mount.TypeVolume,
Source: "foo",
Target: "/target/foo",
},
},
}
expected := "bind /home/path /target, volume foo /target/foo"
assert.Check(t, is.Equal(expected, mount.String()))
assert.Check(t, is.Equal(expected, m.String()))
}
func TestMountRelative(t *testing.T) {
@@ -57,15 +57,15 @@ func TestMountRelative(t *testing.T) {
},
} {
t.Run(testcase.name, func(t *testing.T) {
var mount MountOpt
assert.NilError(t, mount.Set(testcase.bind))
var m MountOpt
assert.NilError(t, m.Set(testcase.bind))
mounts := mount.Value()
mounts := m.Value()
assert.Assert(t, is.Len(mounts, 1))
abs, err := filepath.Abs(testcase.path)
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(mounttypes.Mount{
Type: mounttypes.TypeBind,
assert.Check(t, is.DeepEqual(mount.Mount{
Type: mount.TypeBind,
Source: abs,
Target: "/target",
}, mounts[0]))
@@ -73,77 +73,83 @@ func TestMountRelative(t *testing.T) {
}
}
// TestMountOptSetBindNoErrorBind tests several aliases that should have
// the same result.
func TestMountOptSetBindNoErrorBind(t *testing.T) {
for _, testcase := range []string{
// tests several aliases that should have same result.
for _, tc := range []string{
"type=bind,target=/target,source=/source",
"type=bind,src=/source,dst=/target",
"type=bind,source=/source,dst=/target",
"type=bind,src=/source,target=/target",
} {
var mount MountOpt
t.Run(tc, func(t *testing.T) {
var m MountOpt
assert.NilError(t, mount.Set(testcase))
assert.NilError(t, m.Set(tc))
mounts := mount.Value()
assert.Assert(t, is.Len(mounts, 1))
assert.Check(t, is.DeepEqual(mounttypes.Mount{
Type: mounttypes.TypeBind,
Source: "/source",
Target: "/target",
}, mounts[0]))
mounts := m.Value()
assert.Assert(t, is.Len(mounts, 1))
assert.Check(t, is.DeepEqual(mount.Mount{
Type: mount.TypeBind,
Source: "/source",
Target: "/target",
}, mounts[0]))
})
}
}
// TestMountOptSetVolumeNoError tests several aliases that should have
// the same result.
func TestMountOptSetVolumeNoError(t *testing.T) {
for _, testcase := range []string{
// tests several aliases that should have same result.
for _, tc := range []string{
"type=volume,target=/target,source=/source",
"type=volume,src=/source,dst=/target",
"type=volume,source=/source,dst=/target",
"type=volume,src=/source,target=/target",
} {
var mount MountOpt
t.Run(tc, func(t *testing.T) {
var m MountOpt
assert.NilError(t, mount.Set(testcase))
assert.NilError(t, m.Set(tc))
mounts := mount.Value()
assert.Assert(t, is.Len(mounts, 1))
assert.Check(t, is.DeepEqual(mounttypes.Mount{
Type: mounttypes.TypeVolume,
Source: "/source",
Target: "/target",
}, mounts[0]))
mounts := m.Value()
assert.Assert(t, is.Len(mounts, 1))
assert.Check(t, is.DeepEqual(mount.Mount{
Type: mount.TypeVolume,
Source: "/source",
Target: "/target",
}, mounts[0]))
})
}
}
// TestMountOptDefaultType ensures that a mount without the type defaults to a
// volume mount.
func TestMountOptDefaultType(t *testing.T) {
var mount MountOpt
assert.NilError(t, mount.Set("target=/target,source=/foo"))
assert.Check(t, is.Equal(mounttypes.TypeVolume, mount.values[0].Type))
var m MountOpt
assert.NilError(t, m.Set("target=/target,source=/foo"))
assert.Check(t, is.Equal(mount.TypeVolume, m.values[0].Type))
}
func TestMountOptSetErrorNoTarget(t *testing.T) {
var mount MountOpt
assert.Error(t, mount.Set("type=volume,source=/foo"), "target is required")
var m MountOpt
assert.Error(t, m.Set("type=volume,source=/foo"), "target is required")
}
func TestMountOptSetErrorInvalidKey(t *testing.T) {
var mount MountOpt
assert.Error(t, mount.Set("type=volume,bogus=foo"), "unexpected key 'bogus' in 'bogus=foo'")
var m MountOpt
assert.Error(t, m.Set("type=volume,bogus=foo"), "unexpected key 'bogus' in 'bogus=foo'")
}
func TestMountOptSetErrorInvalidField(t *testing.T) {
var mount MountOpt
assert.Error(t, mount.Set("type=volume,bogus"), "invalid field 'bogus' must be a key=value pair")
var m MountOpt
assert.Error(t, m.Set("type=volume,bogus"), "invalid field 'bogus' must be a key=value pair")
}
func TestMountOptSetErrorInvalidReadOnly(t *testing.T) {
var mount MountOpt
assert.Error(t, mount.Set("type=volume,readonly=no"), "invalid value for readonly: no")
assert.Error(t, mount.Set("type=volume,readonly=invalid"), "invalid value for readonly: invalid")
var m MountOpt
assert.Error(t, m.Set("type=volume,readonly=no"), "invalid value for readonly: no")
assert.Error(t, m.Set("type=volume,readonly=invalid"), "invalid value for readonly: invalid")
}
func TestMountOptDefaultEnableReadOnly(t *testing.T) {
@@ -200,46 +206,49 @@ func TestMountOptTypeConflict(t *testing.T) {
}
func TestMountOptSetImageNoError(t *testing.T) {
for _, testcase := range []string{
for _, tc := range []string{
"type=image,source=foo,target=/target,image-subpath=/bar",
} {
var mount MountOpt
var m MountOpt
assert.NilError(t, mount.Set(testcase))
assert.NilError(t, m.Set(tc))
mounts := mount.Value()
mounts := m.Value()
assert.Assert(t, is.Len(mounts, 1))
assert.Check(t, is.DeepEqual(mounttypes.Mount{
Type: mounttypes.TypeImage,
assert.Check(t, is.DeepEqual(mount.Mount{
Type: mount.TypeImage,
Source: "foo",
Target: "/target",
ImageOptions: &mounttypes.ImageOptions{
ImageOptions: &mount.ImageOptions{
Subpath: "/bar",
},
}, mounts[0]))
}
}
// TestMountOptSetTmpfsNoError tests several aliases that should have
// the same result.
func TestMountOptSetTmpfsNoError(t *testing.T) {
for _, testcase := range []string{
// tests several aliases that should have same result.
for _, tc := range []string{
"type=tmpfs,target=/target,tmpfs-size=1m,tmpfs-mode=0700",
"type=tmpfs,target=/target,tmpfs-size=1MB,tmpfs-mode=700",
} {
var mount MountOpt
t.Run(tc, func(t *testing.T) {
var m MountOpt
assert.NilError(t, mount.Set(testcase))
assert.NilError(t, m.Set(tc))
mounts := mount.Value()
assert.Assert(t, is.Len(mounts, 1))
assert.Check(t, is.DeepEqual(mounttypes.Mount{
Type: mounttypes.TypeTmpfs,
Target: "/target",
TmpfsOptions: &mounttypes.TmpfsOptions{
SizeBytes: 1024 * 1024, // not 1000 * 1000
Mode: os.FileMode(0o700),
},
}, mounts[0]))
mounts := m.Value()
assert.Assert(t, is.Len(mounts, 1))
assert.Check(t, is.DeepEqual(mount.Mount{
Type: mount.TypeTmpfs,
Target: "/target",
TmpfsOptions: &mount.TmpfsOptions{
SizeBytes: 1024 * 1024, // not 1000 * 1000
Mode: os.FileMode(0o700),
},
}, mounts[0]))
})
}
}
@@ -251,84 +260,84 @@ func TestMountOptSetTmpfsError(t *testing.T) {
}
func TestMountOptSetBindNonRecursive(t *testing.T) {
var mount MountOpt
assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-nonrecursive"))
assert.Check(t, is.DeepEqual([]mounttypes.Mount{
var m MountOpt
assert.NilError(t, m.Set("type=bind,source=/foo,target=/bar,bind-nonrecursive"))
assert.Check(t, is.DeepEqual([]mount.Mount{
{
Type: mounttypes.TypeBind,
Type: mount.TypeBind,
Source: "/foo",
Target: "/bar",
BindOptions: &mounttypes.BindOptions{
BindOptions: &mount.BindOptions{
NonRecursive: true,
},
},
}, mount.Value()))
}, m.Value()))
}
func TestMountOptSetBindRecursive(t *testing.T) {
t.Run("enabled", func(t *testing.T) {
var mount MountOpt
assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=enabled"))
assert.Check(t, is.DeepEqual([]mounttypes.Mount{
var m MountOpt
assert.NilError(t, m.Set("type=bind,source=/foo,target=/bar,bind-recursive=enabled"))
assert.Check(t, is.DeepEqual([]mount.Mount{
{
Type: mounttypes.TypeBind,
Type: mount.TypeBind,
Source: "/foo",
Target: "/bar",
},
}, mount.Value()))
}, m.Value()))
})
t.Run("disabled", func(t *testing.T) {
var mount MountOpt
assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=disabled"))
assert.Check(t, is.DeepEqual([]mounttypes.Mount{
var m MountOpt
assert.NilError(t, m.Set("type=bind,source=/foo,target=/bar,bind-recursive=disabled"))
assert.Check(t, is.DeepEqual([]mount.Mount{
{
Type: mounttypes.TypeBind,
Type: mount.TypeBind,
Source: "/foo",
Target: "/bar",
BindOptions: &mounttypes.BindOptions{
BindOptions: &mount.BindOptions{
NonRecursive: true,
},
},
}, mount.Value()))
}, m.Value()))
})
t.Run("writable", func(t *testing.T) {
var mount MountOpt
assert.Error(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=writable"),
var m MountOpt
assert.Error(t, m.Set("type=bind,source=/foo,target=/bar,bind-recursive=writable"),
"option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction")
assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=writable,readonly"))
assert.Check(t, is.DeepEqual([]mounttypes.Mount{
assert.NilError(t, m.Set("type=bind,source=/foo,target=/bar,bind-recursive=writable,readonly"))
assert.Check(t, is.DeepEqual([]mount.Mount{
{
Type: mounttypes.TypeBind,
Type: mount.TypeBind,
Source: "/foo",
Target: "/bar",
ReadOnly: true,
BindOptions: &mounttypes.BindOptions{
BindOptions: &mount.BindOptions{
ReadOnlyNonRecursive: true,
},
},
}, mount.Value()))
}, m.Value()))
})
t.Run("readonly", func(t *testing.T) {
var mount MountOpt
assert.Error(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly"),
var m MountOpt
assert.Error(t, m.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly"),
"option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction")
assert.Error(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly,readonly"),
assert.Error(t, m.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly,readonly"),
"option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction")
assert.NilError(t, mount.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly,readonly,bind-propagation=rprivate"))
assert.Check(t, is.DeepEqual([]mounttypes.Mount{
assert.NilError(t, m.Set("type=bind,source=/foo,target=/bar,bind-recursive=readonly,readonly,bind-propagation=rprivate"))
assert.Check(t, is.DeepEqual([]mount.Mount{
{
Type: mounttypes.TypeBind,
Type: mount.TypeBind,
Source: "/foo",
Target: "/bar",
ReadOnly: true,
BindOptions: &mounttypes.BindOptions{
BindOptions: &mount.BindOptions{
ReadOnlyForceRecursive: true,
Propagation: mounttypes.PropagationRPrivate,
Propagation: mount.PropagationRPrivate,
},
},
}, mount.Value()))
}, m.Value()))
})
}