Extract volume interaction to a volumes service

This cleans up some of the package API's used for interacting with volumes, and simplifies management. Signed-off-by: Brian Goff <[email protected]>
rdx0992 · May 25, 2018 · e4b6adc · e4b6adc
1 parent 9c2c887
commit e4b6adc
Show file tree

Hide file tree

Showing 50 changed files with 1,533 additions and 639 deletions.
diff --git a/api/server/router/volume/backend.go b/api/server/router/volume/backend.go
@@ -3,6 +3,7 @@ package volume // import "github.com/docker/docker/api/server/router/volume"
 import (
 	"context"
 
+	"github.com/docker/docker/volume/service/opts"
 	// TODO return types need to be refactored into pkg
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
@@ -11,9 +12,9 @@ import (
 // Backend is the methods that need to be implemented to provide
 // volume specific functionality
 type Backend interface {
-	Volumes(filter string) ([]*types.Volume, []string, error)
-	VolumeInspect(name string) (*types.Volume, error)
-	VolumeCreate(name, driverName string, opts, labels map[string]string) (*types.Volume, error)
-	VolumeRm(name string, force bool) error
-	VolumesPrune(ctx context.Context, pruneFilters filters.Args) (*types.VolumesPruneReport, error)
+	List(ctx context.Context, filter filters.Args) ([]*types.Volume, []string, error)
+	Get(ctx context.Context, name string, opts ...opts.GetOption) (*types.Volume, error)
+	Create(ctx context.Context, name, driverName string, opts ...opts.CreateOption) (*types.Volume, error)
+	Remove(ctx context.Context, name string, opts ...opts.RemoveOption) error
+	Prune(ctx context.Context, pruneFilters filters.Args) (*types.VolumesPruneReport, error)
 }
diff --git a/api/server/router/volume/volume_routes.go b/api/server/router/volume/volume_routes.go
@@ -3,22 +3,27 @@ package volume // import "github.com/docker/docker/api/server/router/volume"
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"io"
 	"net/http"
 
 	"github.com/docker/docker/api/server/httputils"
 	"github.com/docker/docker/api/types/filters"
 	volumetypes "github.com/docker/docker/api/types/volume"
 	"github.com/docker/docker/errdefs"
+	"github.com/docker/docker/volume/service/opts"
+	"github.com/pkg/errors"
 )
 
 func (v *volumeRouter) getVolumesList(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	if err := httputils.ParseForm(r); err != nil {
 		return err
 	}
 
-	volumes, warnings, err := v.backend.Volumes(r.Form.Get("filters"))
+	filters, err := filters.FromJSON(r.Form.Get("filters"))
+	if err != nil {
+		return errdefs.InvalidParameter(errors.Wrap(err, "error reading volume filters"))
+	}
+	volumes, warnings, err := v.backend.List(ctx, filters)
 	if err != nil {
 		return err
 	}
@@ -30,7 +35,7 @@ func (v *volumeRouter) getVolumeByName(ctx context.Context, w http.ResponseWrite
 		return err
 	}
 
-	volume, err := v.backend.VolumeInspect(vars["name"])
+	volume, err := v.backend.Get(ctx, vars["name"], opts.WithGetResolveStatus)
 	if err != nil {
 		return err
 	}
@@ -54,7 +59,7 @@ func (v *volumeRouter) postVolumesCreate(ctx context.Context, w http.ResponseWri
 		return err
 	}
 
-	volume, err := v.backend.VolumeCreate(req.Name, req.Driver, req.DriverOpts, req.Labels)
+	volume, err := v.backend.Create(ctx, req.Name, req.Driver, opts.WithCreateOptions(req.DriverOpts), opts.WithCreateLabels(req.Labels))
 	if err != nil {
 		return err
 	}
@@ -66,7 +71,7 @@ func (v *volumeRouter) deleteVolumes(ctx context.Context, w http.ResponseWriter,
 		return err
 	}
 	force := httputils.BoolValue(r, "force")
-	if err := v.backend.VolumeRm(vars["name"], force); err != nil {
+	if err := v.backend.Remove(ctx, vars["name"], opts.WithPurgeOnError(force)); err != nil {
 		return err
 	}
 	w.WriteHeader(http.StatusNoContent)
@@ -83,7 +88,7 @@ func (v *volumeRouter) postVolumesPrune(ctx context.Context, w http.ResponseWrit
 		return err
 	}
 
-	pruneReport, err := v.backend.VolumesPrune(ctx, pruneFilters)
+	pruneReport, err := v.backend.Prune(ctx, pruneFilters)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go
@@ -453,7 +453,7 @@ func initRouter(opts routerOptions) {
 		container.NewRouter(opts.daemon, decoder),
 		image.NewRouter(opts.daemon.ImageService()),
 		systemrouter.NewRouter(opts.daemon, opts.cluster, opts.buildCache),
-		volume.NewRouter(opts.daemon),
+		volume.NewRouter(opts.daemon.VolumesService()),
 		build.NewRouter(opts.buildBackend, opts.daemon),
 		sessionrouter.NewRouter(opts.sessionManager),
 		swarmrouter.NewRouter(opts.cluster),
@@ -595,6 +595,7 @@ func createAndStartCluster(cli *DaemonCli, d *daemon.Daemon) (*cluster.Cluster,
 		Root:                   cli.Config.Root,
 		Name:                   name,
 		Backend:                d,
+		VolumeBackend:          d.VolumesService(),
 		ImageBackend:           d.ImageService(),
 		PluginBackend:          d.PluginManager(),
 		NetworkSubnetsProvider: d,

diff --git a/container/container_unix.go b/container/container_unix.go
@@ -127,7 +127,7 @@ func (container *Container) CopyImagePathContent(v volume.Volume, destination st
 		return err
 	}
 
-	if _, err = ioutil.ReadDir(rootfs); err != nil {
+	if _, err := os.Stat(rootfs); err != nil {
 		if os.IsNotExist(err) {
 			return nil
 		}

diff --git a/daemon/cluster/cluster.go b/daemon/cluster/cluster.go
@@ -85,6 +85,7 @@ type Config struct {
 	Backend                executorpkg.Backend
 	ImageBackend           executorpkg.ImageBackend
 	PluginBackend          plugin.Backend
+	VolumeBackend          executorpkg.VolumeBackend
 	NetworkSubnetsProvider NetworkSubnetsProvider
 
 	// DefaultAdvertiseAddr is the default host/IP or network interface to use

diff --git a/daemon/cluster/executor/backend.go b/daemon/cluster/executor/backend.go
@@ -18,6 +18,7 @@ import (
 	clustertypes "github.com/docker/docker/daemon/cluster/provider"
 	networkSettings "github.com/docker/docker/daemon/network"
 	"github.com/docker/docker/plugin"
+	volumeopts "github.com/docker/docker/volume/service/opts"
 	"github.com/docker/libnetwork"
 	"github.com/docker/libnetwork/cluster"
 	networktypes "github.com/docker/libnetwork/types"
@@ -47,7 +48,6 @@ type Backend interface {
 	SetContainerSecretReferences(name string, refs []*swarmtypes.SecretReference) error
 	SetContainerConfigReferences(name string, refs []*swarmtypes.ConfigReference) error
 	SystemInfo() (*types.Info, error)
-	VolumeCreate(name, driverName string, opts, labels map[string]string) (*types.Volume, error)
 	Containers(config *types.ContainerListOptions) ([]*types.Container, error)
 	SetNetworkBootstrapKeys([]*networktypes.EncryptionKey) error
 	DaemonJoinsCluster(provider cluster.Provider)
@@ -62,6 +62,11 @@ type Backend interface {
 	GetAttachmentStore() *networkSettings.AttachmentStore
 }
 
+// VolumeBackend is used by an executor to perform volume operations
+type VolumeBackend interface {
+	Create(ctx context.Context, name, driverName string, opts ...volumeopts.CreateOption) (*types.Volume, error)
+}
+
 // ImageBackend is used by an executor to perform image operations
 type ImageBackend interface {
 	PullImage(ctx context.Context, image, tag, platform string, metaHeaders map[string][]string, authConfig *types.AuthConfig, outStream io.Writer) error

diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go
@@ -22,6 +22,7 @@ import (
 	"github.com/docker/docker/daemon"
 	"github.com/docker/docker/daemon/cluster/convert"
 	executorpkg "github.com/docker/docker/daemon/cluster/executor"
+	volumeopts "github.com/docker/docker/volume/service/opts"
 	"github.com/docker/libnetwork"
 	"github.com/docker/swarmkit/agent/exec"
 	"github.com/docker/swarmkit/api"
@@ -36,23 +37,25 @@ import (
 // are mostly naked calls to the client API, seeded with information from
 // containerConfig.
 type containerAdapter struct {
-	backend      executorpkg.Backend
-	imageBackend executorpkg.ImageBackend
-	container    *containerConfig
-	dependencies exec.DependencyGetter
+	backend       executorpkg.Backend
+	imageBackend  executorpkg.ImageBackend
+	volumeBackend executorpkg.VolumeBackend
+	container     *containerConfig
+	dependencies  exec.DependencyGetter
 }
 
-func newContainerAdapter(b executorpkg.Backend, i executorpkg.ImageBackend, task *api.Task, node *api.NodeDescription, dependencies exec.DependencyGetter) (*containerAdapter, error) {
+func newContainerAdapter(b executorpkg.Backend, i executorpkg.ImageBackend, v executorpkg.VolumeBackend, task *api.Task, node *api.NodeDescription, dependencies exec.DependencyGetter) (*containerAdapter, error) {
 	ctnr, err := newContainerConfig(task, node)
 	if err != nil {
 		return nil, err
 	}
 
 	return &containerAdapter{
-		container:    ctnr,
-		backend:      b,
-		imageBackend: i,
-		dependencies: dependencies,
+		container:     ctnr,
+		backend:       b,
+		imageBackend:  i,
+		volumeBackend: v,
+		dependencies:  dependencies,
 	}, nil
 }
 
@@ -388,7 +391,10 @@ func (c *containerAdapter) createVolumes(ctx context.Context) error {
 		req := c.container.volumeCreateRequest(&mount)
 
 		// Check if this volume exists on the engine
-		if _, err := c.backend.VolumeCreate(req.Name, req.Driver, req.DriverOpts, req.Labels); err != nil {
+		if _, err := c.volumeBackend.Create(ctx, req.Name, req.Driver,
+			volumeopts.WithCreateOptions(req.DriverOpts),
+			volumeopts.WithCreateLabels(req.Labels),
+		); err != nil {
 			// TODO(amitshukla): Today, volume create through the engine api does not return an error
 			// when the named volume with the same parameters already exists.
 			// It returns an error if the driver name is different - that is a valid error

diff --git a/daemon/cluster/executor/container/attachment.go b/daemon/cluster/executor/container/attachment.go
@@ -21,8 +21,8 @@ type networkAttacherController struct {
 	closed  chan struct{}
 }
 
-func newNetworkAttacherController(b executorpkg.Backend, i executorpkg.ImageBackend, task *api.Task, node *api.NodeDescription, dependencies exec.DependencyGetter) (*networkAttacherController, error) {
-	adapter, err := newContainerAdapter(b, i, task, node, dependencies)
+func newNetworkAttacherController(b executorpkg.Backend, i executorpkg.ImageBackend, v executorpkg.VolumeBackend, task *api.Task, node *api.NodeDescription, dependencies exec.DependencyGetter) (*networkAttacherController, error) {
+	adapter, err := newContainerAdapter(b, i, v, task, node, dependencies)
 	if err != nil {
 		return nil, err
 	}

diff --git a/daemon/cluster/executor/container/controller.go b/daemon/cluster/executor/container/controller.go
@@ -40,8 +40,8 @@ type controller struct {
 var _ exec.Controller = &controller{}
 
 // NewController returns a docker exec runner for the provided task.
-func newController(b executorpkg.Backend, i executorpkg.ImageBackend, task *api.Task, node *api.NodeDescription, dependencies exec.DependencyGetter) (*controller, error) {
-	adapter, err := newContainerAdapter(b, i, task, node, dependencies)
+func newController(b executorpkg.Backend, i executorpkg.ImageBackend, v executorpkg.VolumeBackend, task *api.Task, node *api.NodeDescription, dependencies exec.DependencyGetter) (*controller, error) {
+	adapter, err := newContainerAdapter(b, i, v, task, node, dependencies)
 	if err != nil {
 		return nil, err
 	}

diff --git a/daemon/cluster/executor/container/executor.go b/daemon/cluster/executor/container/executor.go
@@ -28,17 +28,19 @@ type executor struct {
 	backend       executorpkg.Backend
 	imageBackend  executorpkg.ImageBackend
 	pluginBackend plugin.Backend
+	volumeBackend executorpkg.VolumeBackend
 	dependencies  exec.DependencyManager
 	mutex         sync.Mutex // This mutex protects the following node field
 	node          *api.NodeDescription
 }
 
 // NewExecutor returns an executor from the docker client.
-func NewExecutor(b executorpkg.Backend, p plugin.Backend, i executorpkg.ImageBackend) exec.Executor {
+func NewExecutor(b executorpkg.Backend, p plugin.Backend, i executorpkg.ImageBackend, v executorpkg.VolumeBackend) exec.Executor {
 	return &executor{
 		backend:       b,
 		pluginBackend: p,
 		imageBackend:  i,
+		volumeBackend: v,
 		dependencies:  agent.NewDependencyManager(),
 	}
 }
@@ -211,7 +213,7 @@ func (e *executor) Controller(t *api.Task) (exec.Controller, error) {
 	e.mutex.Unlock()
 
 	if t.Spec.GetAttachment() != nil {
-		return newNetworkAttacherController(e.backend, e.imageBackend, t, nodeDescription, dependencyGetter)
+		return newNetworkAttacherController(e.backend, e.imageBackend, e.volumeBackend, t, nodeDescription, dependencyGetter)
 	}
 
 	var ctlr exec.Controller
@@ -240,7 +242,7 @@ func (e *executor) Controller(t *api.Task) (exec.Controller, error) {
 			return ctlr, fmt.Errorf("unsupported runtime type: %q", runtimeKind)
 		}
 	case *api.TaskSpec_Container:
-		c, err := newController(e.backend, e.imageBackend, t, nodeDescription, dependencyGetter)
+		c, err := newController(e.backend, e.imageBackend, e.volumeBackend, t, nodeDescription, dependencyGetter)
 		if err != nil {
 			return ctlr, err
 		}

diff --git a/daemon/cluster/executor/container/health_test.go b/daemon/cluster/executor/container/health_test.go
@@ -52,7 +52,7 @@ func TestHealthStates(t *testing.T) {
 		EventsService: e,
 	}
 
-	controller, err := newController(daemon, nil, task, nil, nil)
+	controller, err := newController(daemon, nil, nil, task, nil, nil)
 	if err != nil {
 		t.Fatalf("create controller fail %v", err)
 	}

diff --git a/daemon/cluster/executor/container/validate_test.go b/daemon/cluster/executor/container/validate_test.go
@@ -12,7 +12,7 @@ import (
 )
 
 func newTestControllerWithMount(m api.Mount) (*controller, error) {
-	return newController(&daemon.Daemon{}, nil, &api.Task{
+	return newController(&daemon.Daemon{}, nil, nil, &api.Task{
 		ID:        stringid.GenerateRandomID(),
 		ServiceID: stringid.GenerateRandomID(),
 		Spec: api.TaskSpec{

diff --git a/daemon/cluster/noderunner.go b/daemon/cluster/noderunner.go
@@ -123,7 +123,9 @@ func (n *nodeRunner) start(conf nodeStartConfig) error {
 		Executor: container.NewExecutor(
 			n.cluster.config.Backend,
 			n.cluster.config.PluginBackend,
-			n.cluster.config.ImageBackend),
+			n.cluster.config.ImageBackend,
+			n.cluster.config.VolumeBackend,
+		),
 		HeartbeatTick: n.cluster.config.RaftHeartbeatTick,
 		// Recommended value in etcd/raft is 10 x (HeartbeatTick).
 		// Lower values were seen to have caused instability because of

diff --git a/daemon/create.go b/daemon/create.go
@@ -7,19 +7,17 @@ import (
 	"strings"
 	"time"
 
-	"github.com/pkg/errors"
-
 	"github.com/docker/docker/api/types"
 	containertypes "github.com/docker/docker/api/types/container"
 	networktypes "github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/idtools"
-	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/runconfig"
 	"github.com/opencontainers/selinux/go-selinux/label"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
@@ -255,24 +253,6 @@ func (daemon *Daemon) generateSecurityOpt(hostConfig *containertypes.HostConfig)
 	return nil, nil
 }
 
-// VolumeCreate creates a volume with the specified name, driver, and opts
-// This is called directly from the Engine API
-func (daemon *Daemon) VolumeCreate(name, driverName string, opts, labels map[string]string) (*types.Volume, error) {
-	if name == "" {
-		name = stringid.GenerateNonCryptoID()
-	}
-
-	v, err := daemon.volumes.Create(name, driverName, opts, labels)
-	if err != nil {
-		return nil, err
-	}
-
-	daemon.LogVolumeEvent(v.Name(), "create", map[string]string{"driver": v.DriverName()})
-	apiV := volumeToAPIType(v)
-	apiV.Mountpoint = v.Path()
-	return apiV, nil
-}
-
 func (daemon *Daemon) mergeAndVerifyConfig(config *containertypes.Config, img *image.Image) error {
 	if img != nil && img.Config != nil {
 		if err := merge(config, img.Config); err != nil {

diff --git a/daemon/create_unix.go b/daemon/create_unix.go
@@ -3,6 +3,7 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -11,6 +12,7 @@ import (
 	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/pkg/stringid"
+	volumeopts "github.com/docker/docker/volume/service/opts"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/sirupsen/logrus"
 )
@@ -46,16 +48,16 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con
 			return fmt.Errorf("cannot mount volume over existing file, file exists %s", path)
 		}
 
-		v, err := daemon.volumes.CreateWithRef(name, hostConfig.VolumeDriver, container.ID, nil, nil)
+		v, err := daemon.volumes.Create(context.TODO(), name, hostConfig.VolumeDriver, volumeopts.WithCreateReference(container.ID))
 		if err != nil {
 			return err
 		}
 
-		if err := label.Relabel(v.Path(), container.MountLabel, true); err != nil {
+		if err := label.Relabel(v.Mountpoint, container.MountLabel, true); err != nil {
 			return err
 		}
 
-		container.AddMountPointWithVolume(destination, v, true)
+		container.AddMountPointWithVolume(destination, &volumeWrapper{v: v, s: daemon.volumes}, true)
 	}
 	return daemon.populateVolumes(container)
 }