Merge pull request #20198 from cpuguy83/check_drivers_b4_vol_create

Check drivers before vol create

daemon/volumes.go

@@ -117,7 +117,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
 			return derr.ErrorCodeMountDup.WithArgs(bind.Destination)
 		}
 
-		if len(bind.Name) > 0 && len(bind.Driver) > 0 {
+		if len(bind.Name) > 0 {
 			// create the volume
 			v, err := daemon.volumes.CreateWithRef(bind.Name, bind.Driver, container.ID, nil)
 			if err != nil {

integration-cli/docker_cli_start_volume_driver_unix_test.go

@@ -16,6 +16,7 @@ import (
 	"time"
 
 	"github.com/docker/docker/pkg/integration/checker"
+	"github.com/docker/engine-api/types"
 	"github.com/go-check/check"
 )
 
@@ -410,3 +411,15 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverGet(c *check.C) {
 	c.Assert(s.ec.gets, check.Equals, 1)
 	c.Assert(out, checker.Contains, "No such volume")
 }
+
+func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverWithDaemnRestart(c *check.C) {
+	dockerCmd(c, "volume", "create", "-d", "test-external-volume-driver", "--name", "abc")
+	err := s.d.Restart()
+	c.Assert(err, checker.IsNil)
+
+	dockerCmd(c, "run", "--name=test", "-v", "abc:/foo", "busybox", "true")
+	var mounts []types.MountPoint
+	inspectFieldAndMarshall(c, "test", "Mounts", &mounts)
+	c.Assert(mounts, checker.HasLen, 1)
+	c.Assert(mounts[0].Driver, checker.Equals, "test-external-volume-driver")
+}

integration-cli/docker_cli_volume_driver_compat_unix_test.go

@@ -1,234 +0,0 @@
-// +build !windows
-
-package main
-
-import (
-	"encoding/json"
-	"fmt"
-	"io"
-	"io/ioutil"
-	"net/http"
-	"net/http/httptest"
-	"os"
-	"path/filepath"
-	"strings"
-
-	"github.com/docker/docker/pkg/integration/checker"
-
-	"github.com/go-check/check"
-)
-
-func init() {
-	check.Suite(&DockerExternalVolumeSuiteCompatV1_1{
-		ds: &DockerSuite{},
-	})
-}
-
-type vol struct {
-	Name       string
-	Mountpoint string
-	Opts       map[string]string
-}
-
-type DockerExternalVolumeSuiteCompatV1_1 struct {
-	server  *httptest.Server
-	ds      *DockerSuite
-	d       *Daemon
-	ec      *eventCounter
-	volList []vol
-}
-
-func (s *DockerExternalVolumeSuiteCompatV1_1) SetUpTest(c *check.C) {
-	s.d = NewDaemon(c)
-	s.ec = &eventCounter{}
-}
-
-func (s *DockerExternalVolumeSuiteCompatV1_1) TearDownTest(c *check.C) {
-	s.d.Stop()
-	s.ds.TearDownTest(c)
-}
-
-func (s *DockerExternalVolumeSuiteCompatV1_1) SetUpSuite(c *check.C) {
-	mux := http.NewServeMux()
-	s.server = httptest.NewServer(mux)
-
-	type pluginRequest struct {
-		Name string
-		Opts map[string]string
-	}
-
-	type pluginResp struct {
-		Mountpoint string `json:",omitempty"`
-		Err        string `json:",omitempty"`
-	}
-
-	read := func(b io.ReadCloser) (pluginRequest, error) {
-		defer b.Close()
-		var pr pluginRequest
-		if err := json.NewDecoder(b).Decode(&pr); err != nil {
-			return pr, err
-		}
-		return pr, nil
-	}
-
-	send := func(w http.ResponseWriter, data interface{}) {
-		switch t := data.(type) {
-		case error:
-			http.Error(w, t.Error(), 500)
-		case string:
-			w.Header().Set("Content-Type", "application/vnd.docker.plugins.v1+json")
-			fmt.Fprintln(w, t)
-		default:
-			w.Header().Set("Content-Type", "application/vnd.docker.plugins.v1+json")
-			json.NewEncoder(w).Encode(&data)
-		}
-	}
-
-	mux.HandleFunc("/Plugin.Activate", func(w http.ResponseWriter, r *http.Request) {
-		s.ec.activations++
-		send(w, `{"Implements": ["VolumeDriver"]}`)
-	})
-
-	mux.HandleFunc("/VolumeDriver.Create", func(w http.ResponseWriter, r *http.Request) {
-		s.ec.creations++
-		pr, err := read(r.Body)
-		if err != nil {
-			send(w, err)
-			return
-		}
-		s.volList = append(s.volList, vol{Name: pr.Name, Opts: pr.Opts})
-		send(w, nil)
-	})
-
-	mux.HandleFunc("/VolumeDriver.Remove", func(w http.ResponseWriter, r *http.Request) {
-		s.ec.removals++
-		pr, err := read(r.Body)
-		if err != nil {
-			send(w, err)
-			return
-		}
-
-		if err := os.RemoveAll(hostVolumePath(pr.Name)); err != nil {
-			send(w, &pluginResp{Err: err.Error()})
-			return
-		}
-
-		for i, v := range s.volList {
-			if v.Name == pr.Name {
-				if err := os.RemoveAll(hostVolumePath(v.Name)); err != nil {
-					send(w, fmt.Sprintf(`{"Err": "%v"}`, err))
-					return
-				}
-				s.volList = append(s.volList[:i], s.volList[i+1:]...)
-				break
-			}
-		}
-		send(w, nil)
-	})
-
-	mux.HandleFunc("/VolumeDriver.Path", func(w http.ResponseWriter, r *http.Request) {
-		s.ec.paths++
-
-		pr, err := read(r.Body)
-		if err != nil {
-			send(w, err)
-			return
-		}
-		p := hostVolumePath(pr.Name)
-		send(w, &pluginResp{Mountpoint: p})
-	})
-
-	mux.HandleFunc("/VolumeDriver.Mount", func(w http.ResponseWriter, r *http.Request) {
-		s.ec.mounts++
-
-		pr, err := read(r.Body)
-		if err != nil {
-			send(w, err)
-			return
-		}
-
-		p := hostVolumePath(pr.Name)
-		if err := os.MkdirAll(p, 0755); err != nil {
-			send(w, &pluginResp{Err: err.Error()})
-			return
-		}
-
-		if err := ioutil.WriteFile(filepath.Join(p, "test"), []byte(s.server.URL), 0644); err != nil {
-			send(w, err)
-			return
-		}
-
-		send(w, &pluginResp{Mountpoint: p})
-	})
-
-	mux.HandleFunc("/VolumeDriver.Unmount", func(w http.ResponseWriter, r *http.Request) {
-		s.ec.unmounts++
-
-		_, err := read(r.Body)
-		if err != nil {
-			send(w, err)
-			return
-		}
-
-		send(w, nil)
-	})
-
-	err := os.MkdirAll("/etc/docker/plugins", 0755)
-	c.Assert(err, checker.IsNil)
-
-	err = ioutil.WriteFile("/etc/docker/plugins/test-external-volume-driver.spec", []byte(s.server.URL), 0644)
-	c.Assert(err, checker.IsNil)
-}
-
-func (s *DockerExternalVolumeSuiteCompatV1_1) TearDownSuite(c *check.C) {
-	s.server.Close()
-
-	err := os.RemoveAll("/etc/docker/plugins")
-	c.Assert(err, checker.IsNil)
-}
-
-func (s *DockerExternalVolumeSuiteCompatV1_1) TestExternalVolumeDriverCompatV1_1(c *check.C) {
-	err := s.d.StartWithBusybox()
-	c.Assert(err, checker.IsNil)
-
-	out, err := s.d.Cmd("run", "--name=test", "-v", "foo:/bar", "--volume-driver", "test-external-volume-driver", "busybox", "sh", "-c", "echo hello > /bar/hello")
-	c.Assert(err, checker.IsNil, check.Commentf(out))
-	out, err = s.d.Cmd("rm", "test")
-	c.Assert(err, checker.IsNil, check.Commentf(out))
-
-	out, err = s.d.Cmd("run", "--name=test2", "-v", "foo:/bar", "busybox", "cat", "/bar/hello")
-	c.Assert(err, checker.IsNil, check.Commentf(out))
-	c.Assert(strings.TrimSpace(out), checker.Equals, "hello")
-
-	err = s.d.Restart()
-	c.Assert(err, checker.IsNil)
-
-	out, err = s.d.Cmd("start", "-a", "test2")
-	c.Assert(strings.TrimSpace(out), checker.Equals, "hello")
-
-	out, err = s.d.Cmd("rm", "test2")
-	c.Assert(err, checker.IsNil, check.Commentf(out))
-
-	out, err = s.d.Cmd("volume", "inspect", "foo")
-	c.Assert(err, checker.IsNil, check.Commentf(out))
-
-	out, err = s.d.Cmd("volume", "rm", "foo")
-	c.Assert(err, checker.IsNil, check.Commentf(out))
-}
-
-func (s *DockerExternalVolumeSuiteCompatV1_1) TestExternalVolumeDriverCompatOptionsV1_1(c *check.C) {
-	err := s.d.StartWithBusybox()
-	c.Assert(err, checker.IsNil)
-
-	out, err := s.d.Cmd("volume", "create", "--name", "optvol", "--driver", "test-external-volume-driver", "--opt", "opt1=opt1val", "--opt", "opt2=opt2val")
-	c.Assert(err, checker.IsNil, check.Commentf(out))
-
-	out, err = s.d.Cmd("volume", "inspect", "optvol")
-	c.Assert(err, checker.IsNil, check.Commentf(out))
-
-	c.Assert(s.volList[0].Opts["opt1"], checker.Equals, "opt1val")
-	c.Assert(s.volList[0].Opts["opt2"], checker.Equals, "opt2val")
-
-	out, err = s.d.Cmd("volume", "rm", "optvol")
-	c.Assert(err, checker.IsNil, check.Commentf(out))
-}

volume/drivers/adapter.go

@@ -1,9 +1,6 @@
 package volumedrivers
 
-import (
+import "github.com/docker/docker/volume"
-	"github.com/docker/docker/pkg/plugins"
-	"github.com/docker/docker/volume"
-)
 
 type volumeDriverAdapter struct {
 	name  string
@@ -15,21 +12,7 @@ func (a *volumeDriverAdapter) Name() string {
 }
 
 func (a *volumeDriverAdapter) Create(name string, opts map[string]string) (volume.Volume, error) {
-	// First try a Get. For drivers that support Get this will return any
+	if err := a.proxy.Create(name, opts); err != nil {
-	// existing volume.
-	v, err := a.proxy.Get(name)
-	if v != nil {
-		return &volumeAdapter{
-			proxy:      a.proxy,
-			name:       v.Name,
-			driverName: a.Name(),
-			eMount:     v.Mountpoint,
-		}, nil
-	}
-
-	// Driver didn't support Get or volume didn't exist. Perform Create.
-	err = a.proxy.Create(name, opts)
-	if err != nil {
 		return nil, err
 	}
 	return &volumeAdapter{
@@ -63,11 +46,7 @@ func (a *volumeDriverAdapter) List() ([]volume.Volume, error) {
 func (a *volumeDriverAdapter) Get(name string) (volume.Volume, error) {
 	v, err := a.proxy.Get(name)
 	if err != nil {
-		// TODO: remove this hack. Allows back compat with volume drivers that don't support this call
+		return nil, err
-		if !plugins.IsNotFound(err) {
-			return nil, err
-		}
-		return a.Create(name, nil)
 	}
 
 	return &volumeAdapter{

volume/store/store.go

@@ -186,12 +186,23 @@ func (s *VolumeStore) create(name, driverName string, opts map[string]string) (v
 		return v, nil
 	}
 
-	logrus.Debugf("Registering new volume reference: driver %s, name %s", driverName, name)
+	// Since there isn't a specified driver name, let's see if any of the existing drivers have this volume name
+	if driverName == "" {
+		v, _ := s.getVolume(name)
+		if v != nil {
+			return v, nil
+		}
+	}
+
+	logrus.Debugf("Registering new volume reference: driver %q, name %q", driverName, name)
 	vd, err := volumedrivers.GetDriver(driverName)
 	if err != nil {
 		return nil, &OpErr{Op: "create", Name: name, Err: err}
 	}
 
+	if v, _ := vd.Get(name); v != nil {
+		return v, nil
+	}
 	return vd.Create(name, opts)
 }
 

volume/volume_test.go

@@ -167,10 +167,10 @@ func TestParseMountSpecSplit(t *testing.T) {
 			{"/tmp:/tmp2:ro", "", "/tmp2", "/tmp", "", "", false, false},
 			{"/tmp:/tmp3:rw", "", "/tmp3", "/tmp", "", "", true, false},
 			{"/tmp:/tmp4:foo", "", "", "", "", "", false, true},
-			{"name:/named1", "", "/named1", "", "name", "local", true, false},
+			{"name:/named1", "", "/named1", "", "name", "", true, false},
 			{"name:/named2", "external", "/named2", "", "name", "external", true, false},
 			{"name:/named3:ro", "local", "/named3", "", "name", "local", false, false},
-			{"local/name:/tmp:rw", "", "/tmp", "", "local/name", "local", true, false},
+			{"local/name:/tmp:rw", "", "/tmp", "", "local/name", "", true, false},
 			{"/tmp:tmp", "", "", "", "", "", true, true},
 		}
 	}

volume/volume_unix.go

@@ -97,9 +97,6 @@ func ParseMountSpec(spec, volumeDriver string) (*MountPoint, error) {
 	if len(source) == 0 {
 		mp.Source = "" // Clear it out as we previously assumed it was not a name
 		mp.Driver = volumeDriver
-		if len(mp.Driver) == 0 {
-			mp.Driver = DefaultDriverName
-		}
 		// Named volumes can't have propagation properties specified.
 		// Their defaults will be decided by docker. This is just a
 		// safeguard. Don't want to get into situations where named