Skip to content
This repository was archived by the owner on Feb 8, 2021. It is now read-only.

Commit d3eca44

Browse files
committed
Move responsibility of ls/inspect to volume driver
Makes `docker volume ls` and `docker volume inspect` ask the volume drivers rather than only using what is cached locally. Previously in order to use a volume from an external driver, one would either have to use `docker volume create` or have a container that is already using that volume for it to be visible to the other volume API's. For keeping uniqueness of volume names in the daemon, names are bound to a driver on a first come first serve basis. If two drivers have a volume with the same name, the first one is chosen, and a warning is logged about the second one. Adds 2 new methods to the plugin API, `List` and `Get`. If a plugin does not implement these endpoints, a user will not be able to find the specified volumes as well requests go through the drivers. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
1 parent 6c30931 commit d3eca44

30 files changed

+680
-265
lines changed

api/client/volume.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ func (cli *DockerCli) CmdVolumeLs(args ...string) error {
6565

6666
w := tabwriter.NewWriter(cli.out, 20, 1, 3, ' ', 0)
6767
if !*quiet {
68+
for _, warn := range volumes.Warnings {
69+
fmt.Fprintln(cli.err, warn)
70+
}
6871
fmt.Fprintf(w, "DRIVER \tVOLUME NAME")
6972
fmt.Fprintf(w, "\n")
7073
}
@@ -102,7 +105,7 @@ func (cli *DockerCli) CmdVolumeInspect(args ...string) error {
102105
return cli.inspectElements(*tmplStr, cmd.Args(), inspectSearcher)
103106
}
104107

105-
// CmdVolumeCreate creates a new container from a given image.
108+
// CmdVolumeCreate creates a new volume.
106109
//
107110
// Usage: docker volume create [OPTIONS]
108111
func (cli *DockerCli) CmdVolumeCreate(args ...string) error {
@@ -131,7 +134,7 @@ func (cli *DockerCli) CmdVolumeCreate(args ...string) error {
131134
return nil
132135
}
133136

134-
// CmdVolumeRm removes one or more containers.
137+
// CmdVolumeRm removes one or more volumes.
135138
//
136139
// Usage: docker volume rm VOLUME [VOLUME...]
137140
func (cli *DockerCli) CmdVolumeRm(args ...string) error {
@@ -140,6 +143,7 @@ func (cli *DockerCli) CmdVolumeRm(args ...string) error {
140143
cmd.ParseFlags(args, true)
141144

142145
var status = 0
146+
143147
for _, name := range cmd.Args() {
144148
if err := cli.client.VolumeRemove(name); err != nil {
145149
fmt.Fprintf(cli.err, "%s\n", err)

api/server/router/volume/backend.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
// Backend is the methods that need to be implemented to provide
99
// volume specific functionality
1010
type Backend interface {
11-
Volumes(filter string) ([]*types.Volume, error)
11+
Volumes(filter string) ([]*types.Volume, []string, error)
1212
VolumeInspect(name string) (*types.Volume, error)
1313
VolumeCreate(name, driverName string,
1414
opts map[string]string) (*types.Volume, error)

api/server/router/volume/volume_routes.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ func (v *volumeRouter) getVolumesList(ctx context.Context, w http.ResponseWriter
1414
return err
1515
}
1616

17-
volumes, err := v.backend.Volumes(r.Form.Get("filters"))
17+
volumes, warnings, err := v.backend.Volumes(r.Form.Get("filters"))
1818
if err != nil {
1919
return err
2020
}
21-
return httputils.WriteJSON(w, http.StatusOK, &types.VolumesListResponse{Volumes: volumes})
21+
return httputils.WriteJSON(w, http.StatusOK, &types.VolumesListResponse{Volumes: volumes, Warnings: warnings})
2222
}
2323

2424
func (v *volumeRouter) getVolumeByName(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {

api/types/types.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,8 @@ type Volume struct {
366366
// VolumesListResponse contains the response for the remote API:
367367
// GET "/volumes"
368368
type VolumesListResponse struct {
369-
Volumes []*Volume // Volumes is the list of volumes being returned
369+
Volumes []*Volume // Volumes is the list of volumes being returned
370+
Warnings []string // Warnings is a list of warnings that occurred when getting the list from the volume drivers
370371
}
371372

372373
// VolumeCreateRequest contains the response for the remote API:

daemon/create.go

+5-10
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"github.com/docker/docker/layer"
1111
"github.com/docker/docker/pkg/idtools"
1212
"github.com/docker/docker/pkg/stringid"
13-
"github.com/docker/docker/volume"
13+
volumestore "github.com/docker/docker/volume/store"
1414
"github.com/opencontainers/runc/libcontainer/label"
1515
)
1616

@@ -162,17 +162,12 @@ func (daemon *Daemon) VolumeCreate(name, driverName string, opts map[string]stri
162162

163163
v, err := daemon.volumes.Create(name, driverName, opts)
164164
if err != nil {
165+
if volumestore.IsNameConflict(err) {
166+
return nil, derr.ErrorVolumeNameTaken.WithArgs(name)
167+
}
165168
return nil, err
166169
}
167170

168-
// keep "docker run -v existing_volume:/foo --volume-driver other_driver" work
169-
if (driverName != "" && v.DriverName() != driverName) || (driverName == "" && v.DriverName() != volume.DefaultDriverName) {
170-
return nil, derr.ErrorVolumeNameTaken.WithArgs(name, v.DriverName())
171-
}
172-
173-
if driverName == "" {
174-
driverName = volume.DefaultDriverName
175-
}
176-
daemon.LogVolumeEvent(name, "create", map[string]string{"driver": driverName})
171+
daemon.LogVolumeEvent(v.Name(), "create", map[string]string{"driver": v.DriverName()})
177172
return volumeToAPIType(v), nil
178173
}

daemon/create_unix.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (daemon *Daemon) createContainerPlatformSpecificSettings(container *contain
5151
}
5252
}
5353

54-
v, err := daemon.createVolume(name, volumeDriver, nil)
54+
v, err := daemon.volumes.CreateWithRef(name, volumeDriver, container.ID, nil)
5555
if err != nil {
5656
return err
5757
}

daemon/create_windows.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (daemon *Daemon) createContainerPlatformSpecificSettings(container *contain
4242

4343
// Create the volume in the volume driver. If it doesn't exist,
4444
// a new one will be created.
45-
v, err := daemon.createVolume(mp.Name, volumeDriver, nil)
45+
v, err := daemon.volumes.CreateWithRef(mp.Name, volumeDriver, container.ID, nil)
4646
if err != nil {
4747
return err
4848
}

daemon/daemon.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -1470,10 +1470,7 @@ func configureVolumes(config *Config, rootUID, rootGID int) (*store.VolumeStore,
14701470
}
14711471

14721472
volumedrivers.Register(volumesDriver, volumesDriver.Name())
1473-
s := store.New()
1474-
s.AddAll(volumesDriver.List())
1475-
1476-
return s, nil
1473+
return store.New(), nil
14771474
}
14781475

14791476
// AuthenticateToRegistry checks the validity of credentials in authConfig

daemon/delete.go

+1
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ func (daemon *Daemon) VolumeRm(name string) error {
151151
if err != nil {
152152
return err
153153
}
154+
154155
if err := daemon.volumes.Remove(v); err != nil {
155156
if volumestore.IsInUse(err) {
156157
return derr.ErrorCodeRmVolumeInUse.WithArgs(err)

daemon/list.go

+10-7
Original file line numberDiff line numberDiff line change
@@ -392,24 +392,27 @@ func (daemon *Daemon) transformContainer(container *container.Container, ctx *li
392392

393393
// Volumes lists known volumes, using the filter to restrict the range
394394
// of volumes returned.
395-
func (daemon *Daemon) Volumes(filter string) ([]*types.Volume, error) {
395+
func (daemon *Daemon) Volumes(filter string) ([]*types.Volume, []string, error) {
396396
var volumesOut []*types.Volume
397397
volFilters, err := filters.FromParam(filter)
398398
if err != nil {
399-
return nil, err
399+
return nil, nil, err
400400
}
401401

402402
filterUsed := volFilters.Include("dangling") &&
403403
(volFilters.ExactMatch("dangling", "true") || volFilters.ExactMatch("dangling", "1"))
404404

405-
volumes := daemon.volumes.List()
405+
volumes, warnings, err := daemon.volumes.List()
406+
if err != nil {
407+
return nil, nil, err
408+
}
409+
if filterUsed {
410+
volumes = daemon.volumes.FilterByUsed(volumes)
411+
}
406412
for _, v := range volumes {
407-
if filterUsed && daemon.volumes.Count(v) > 0 {
408-
continue
409-
}
410413
volumesOut = append(volumesOut, volumeToAPIType(v))
411414
}
412-
return volumesOut, nil
415+
return volumesOut, warnings, nil
413416
}
414417

415418
func populateImageFilterByParents(ancestorMap map[image.ID]bool, imageID image.ID, getChildren func(image.ID) []image.ID) {

daemon/mounts.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ import (
1111
func (daemon *Daemon) prepareMountPoints(container *container.Container) error {
1212
for _, config := range container.MountPoints {
1313
if len(config.Driver) > 0 {
14-
v, err := daemon.createVolume(config.Name, config.Driver, nil)
14+
v, err := daemon.volumes.GetWithRef(config.Name, config.Driver, container.ID)
1515
if err != nil {
1616
return err
1717
}
18+
1819
config.Volume = v
1920
}
2021
}
@@ -27,10 +28,10 @@ func (daemon *Daemon) removeMountPoints(container *container.Container, rm bool)
2728
if m.Volume == nil {
2829
continue
2930
}
30-
daemon.volumes.Decrement(m.Volume)
31+
daemon.volumes.Dereference(m.Volume, container.ID)
3132
if rm {
3233
err := daemon.volumes.Remove(m.Volume)
33-
// ErrVolumeInUse is ignored because having this
34+
// Ignore volume in use errors because having this
3435
// volume being referenced by other container is
3536
// not an error, but an implementation detail.
3637
// This prevents docker from logging "ERROR: Volume in use"

daemon/volumes.go

+3-13
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,6 @@ func volumeToAPIType(v volume.Volume) *types.Volume {
3232
}
3333
}
3434

35-
// createVolume creates a volume.
36-
func (daemon *Daemon) createVolume(name, driverName string, opts map[string]string) (volume.Volume, error) {
37-
v, err := daemon.volumes.Create(name, driverName, opts)
38-
if err != nil {
39-
return nil, err
40-
}
41-
daemon.volumes.Increment(v)
42-
return v, nil
43-
}
44-
4535
// Len returns the number of mounts. Used in sorting.
4636
func (m mounts) Len() int {
4737
return len(m)
@@ -103,7 +93,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
10393
}
10494

10595
if len(cp.Source) == 0 {
106-
v, err := daemon.createVolume(cp.Name, cp.Driver, nil)
96+
v, err := daemon.volumes.GetWithRef(cp.Name, cp.Driver, container.ID)
10797
if err != nil {
10898
return err
10999
}
@@ -128,7 +118,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
128118

129119
if len(bind.Name) > 0 && len(bind.Driver) > 0 {
130120
// create the volume
131-
v, err := daemon.createVolume(bind.Name, bind.Driver, nil)
121+
v, err := daemon.volumes.CreateWithRef(bind.Name, bind.Driver, container.ID, nil)
132122
if err != nil {
133123
return err
134124
}
@@ -153,7 +143,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
153143
for _, m := range mountPoints {
154144
if m.BackwardsCompatible() {
155145
if mp, exists := container.MountPoints[m.Destination]; exists && mp.Volume != nil {
156-
daemon.volumes.Decrement(mp.Volume)
146+
daemon.volumes.Dereference(mp.Volume, container.ID)
157147
}
158148
}
159149
}

errors/daemon.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ var (
908908
// trying to create a volume that has existed using different driver.
909909
ErrorVolumeNameTaken = errcode.Register(errGroup, errcode.ErrorDescriptor{
910910
Value: "VOLUME_NAME_TAKEN",
911-
Message: "A volume named %q already exists with the %q driver. Choose a different volume name.",
911+
Message: "A volume named %s already exists. Choose a different volume name.",
912912
Description: "An attempt to create a volume using a driver but the volume already exists with a different driver",
913913
HTTPStatusCode: http.StatusInternalServerError,
914914
})

integration-cli/docker_cli_daemon_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1733,8 +1733,8 @@ func (s *DockerDaemonSuite) TestDaemonRestartRmVolumeInUse(c *check.C) {
17331733
c.Assert(s.d.Restart(), check.IsNil)
17341734

17351735
out, err = s.d.Cmd("volume", "rm", "test")
1736-
c.Assert(err, check.Not(check.IsNil), check.Commentf("should not be able to remove in use volume after daemon restart"))
1737-
c.Assert(strings.Contains(out, "in use"), check.Equals, true)
1736+
c.Assert(err, check.NotNil, check.Commentf("should not be able to remove in use volume after daemon restart"))
1737+
c.Assert(out, checker.Contains, "in use")
17381738
}
17391739

17401740
func (s *DockerDaemonSuite) TestDaemonRestartLocalVolumes(c *check.C) {

0 commit comments

Comments
 (0)