Merge pull request #19528 from cpuguy83/19475_abck_compat_for_vol_drivers

Add back compat for volume drivers `Get` and `Ls`

integration-cli/docker_cli_volume_driver_compat_unix_test.go

@@ -0,0 +1,215 @@
+// +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 DockerExternalVolumeSuiteCompatV1_1 struct {
+	server *httptest.Server
+	ds     *DockerSuite
+	d      *Daemon
+	ec     *eventCounter
+}
+
+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
+	}
+
+	type pluginResp struct {
+		Mountpoint string `json:",omitempty"`
+		Err        string `json:",omitempty"`
+	}
+
+	type vol struct {
+		Name       string
+		Mountpoint string
+	}
+	var volList []vol
+
+	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
+		}
+		volList = append(volList, vol{Name: pr.Name})
+		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 volList {
+			if v.Name == pr.Name {
+				if err := os.RemoveAll(hostVolumePath(v.Name)); err != nil {
+					send(w, fmt.Sprintf(`{"Err": "%v"}`, err))
+					return
+				}
+				volList = append(volList[:i], 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))
+}

pkg/plugins/client.go

@@ -3,7 +3,6 @@ package plugins
 import (
 	"bytes"
 	"encoding/json"
-	"fmt"
 	"io"
 	"io/ioutil"
 	"net/http"
@@ -124,7 +123,7 @@ func (c *Client) callWithRetry(serviceMethod string, data io.Reader, retry bool)
 		if resp.StatusCode != http.StatusOK {
 			b, err := ioutil.ReadAll(resp.Body)
 			if err != nil {
-				return nil, fmt.Errorf("%s: %s", serviceMethod, err)
+				return nil, &statusError{resp.StatusCode, serviceMethod, err.Error()}
 			}
 
 			// Plugins' Response(s) should have an Err field indicating what went
@@ -136,11 +135,11 @@ func (c *Client) callWithRetry(serviceMethod string, data io.Reader, retry bool)
 			remoteErr := responseErr{}
 			if err := json.Unmarshal(b, &remoteErr); err == nil {
 				if remoteErr.Err != "" {
-					return nil, fmt.Errorf("%s: %s", serviceMethod, remoteErr.Err)
+					return nil, &statusError{resp.StatusCode, serviceMethod, remoteErr.Err}
 				}
 			}
 			// old way...
-			return nil, fmt.Errorf("%s: %s", serviceMethod, string(b))
+			return nil, &statusError{resp.StatusCode, serviceMethod, string(b)}
 		}
 		return resp.Body, nil
 	}

pkg/plugins/errors.go

@@ -0,0 +1,33 @@
+package plugins
+
+import (
+	"fmt"
+	"net/http"
+)
+
+type statusError struct {
+	status int
+	method string
+	err    string
+}
+
+// Error returns a formated string for this error type
+func (e *statusError) Error() string {
+	return fmt.Sprintf("%s: %v", e.method, e.err)
+}
+
+// IsNotFound indicates if the passed in error is from an http.StatusNotFound from the plugin
+func IsNotFound(err error) bool {
+	return isStatusError(err, http.StatusNotFound)
+}
+
+func isStatusError(err error, status int) bool {
+	if err == nil {
+		return false
+	}
+	e, ok := err.(*statusError)
+	if !ok {
+		return false
+	}
+	return e.status == status
+}

volume/drivers/adapter.go

@@ -1,6 +1,9 @@
 package volumedrivers
 
-import "github.com/docker/docker/volume"
+import (
+	"github.com/docker/docker/pkg/plugins"
+	"github.com/docker/docker/volume"
+)
 
 type volumeDriverAdapter struct {
 	name  string
@@ -47,7 +50,11 @@ 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 {
-		return nil, err
+		// TODO: remove this hack. Allows back compat with volume drivers that don't support this call
+		if !plugins.IsNotFound(err) {
+			return nil, err
+		}
+		return a.Create(name, nil)
 	}
 
 	return &volumeAdapter{

volume/store/store.go

@@ -14,23 +14,23 @@ import (
 func New() *VolumeStore {
 	return &VolumeStore{
 		locks: &locker.Locker{},
-		names: make(map[string]string),
+		names: make(map[string]volume.Volume),
 		refs:  make(map[string][]string),
 	}
 }
 
-func (s *VolumeStore) getNamed(name string) (string, bool) {
+func (s *VolumeStore) getNamed(name string) (volume.Volume, bool) {
 	s.globalLock.Lock()
-	driverName, exists := s.names[name]
+	v, exists := s.names[name]
 	s.globalLock.Unlock()
-	return driverName, exists
+	return v, exists
 }
 
-func (s *VolumeStore) setNamed(name, driver, ref string) {
+func (s *VolumeStore) setNamed(v volume.Volume, ref string) {
 	s.globalLock.Lock()
-	s.names[name] = driver
+	s.names[v.Name()] = v
 	if len(ref) > 0 {
-		s.refs[name] = append(s.refs[name], ref)
+		s.refs[v.Name()] = append(s.refs[v.Name()], ref)
 	}
 	s.globalLock.Unlock()
 }
@@ -48,7 +48,7 @@ type VolumeStore struct {
 	globalLock sync.Mutex
 	// names stores the volume name -> driver name relationship.
 	// This is used for making lookups faster so we don't have to probe all drivers
-	names map[string]string
+	names map[string]volume.Volume
 	// refs stores the volume name and the list of things referencing it
 	refs map[string][]string
 }
@@ -67,12 +67,12 @@ func (s *VolumeStore) List() ([]volume.Volume, []string, error) {
 		name := normaliseVolumeName(v.Name())
 
 		s.locks.Lock(name)
-		driverName, exists := s.getNamed(name)
+		storedV, exists := s.getNamed(name)
 		if !exists {
-			s.setNamed(name, v.DriverName(), "")
+			s.setNamed(v, "")
 		}
-		if exists && driverName != v.DriverName() {
-			logrus.Warnf("Volume name %s already exists for driver %s, not including volume returned by %s", v.Name(), driverName, v.DriverName())
+		if exists && storedV.DriverName() != v.DriverName() {
+			logrus.Warnf("Volume name %s already exists for driver %s, not including volume returned by %s", v.Name(), storedV.DriverName(), v.DriverName())
 			s.locks.Unlock(v.Name())
 			continue
 		}
@@ -95,8 +95,9 @@ func (s *VolumeStore) list() ([]volume.Volume, []string, error) {
 	)
 
 	type vols struct {
-		vols []volume.Volume
-		err  error
+		vols       []volume.Volume
+		err        error
+		driverName string
 	}
 	chVols := make(chan vols, len(drivers))
 
@@ -104,23 +105,32 @@ func (s *VolumeStore) list() ([]volume.Volume, []string, error) {
 		go func(d volume.Driver) {
 			vs, err := d.List()
 			if err != nil {
-				chVols <- vols{err: &OpErr{Err: err, Name: d.Name(), Op: "list"}}
+				chVols <- vols{driverName: d.Name(), err: &OpErr{Err: err, Name: d.Name(), Op: "list"}}
 				return
 			}
 			chVols <- vols{vols: vs}
 		}(vd)
 	}
 
+	badDrivers := make(map[string]struct{})
 	for i := 0; i < len(drivers); i++ {
 		vs := <-chVols
 
 		if vs.err != nil {
 			warnings = append(warnings, vs.err.Error())
+			badDrivers[vs.driverName] = struct{}{}
 			logrus.Warn(vs.err)
-			continue
 		}
 		ls = append(ls, vs.vols...)
 	}
+
+	if len(badDrivers) > 0 {
+		for _, v := range s.names {
+			if _, exists := badDrivers[v.DriverName()]; exists {
+				ls = append(ls, v)
+			}
+		}
+	}
 	return ls, warnings, nil
 }
 
@@ -137,7 +147,7 @@ func (s *VolumeStore) CreateWithRef(name, driverName, ref string, opts map[strin
 		return nil, &OpErr{Err: err, Name: name, Op: "create"}
 	}
 
-	s.setNamed(name, v.DriverName(), ref)
+	s.setNamed(v, ref)
 	return v, nil
 }
 
@@ -151,7 +161,7 @@ func (s *VolumeStore) Create(name, driverName string, opts map[string]string) (v
 	if err != nil {
 		return nil, &OpErr{Err: err, Name: name, Op: "create"}
 	}
-	s.setNamed(name, v.DriverName(), "")
+	s.setNamed(v, "")
 	return v, nil
 }
 
@@ -169,12 +179,11 @@ func (s *VolumeStore) create(name, driverName string, opts map[string]string) (v
 		return nil, &OpErr{Err: errInvalidName, Name: name, Op: "create"}
 	}
 
-	vdName, exists := s.getNamed(name)
-	if exists {
-		if vdName != driverName && driverName != "" && driverName != volume.DefaultDriverName {
+	if v, exists := s.getNamed(name); exists {
+		if v.DriverName() != driverName && driverName != "" && driverName != volume.DefaultDriverName {
 			return nil, errNameConflict
 		}
-		driverName = vdName
+		return v, nil
 	}
 
 	logrus.Debugf("Registering new volume reference: driver %s, name %s", driverName, name)
@@ -207,7 +216,7 @@ func (s *VolumeStore) GetWithRef(name, driverName, ref string) (volume.Volume, e
 		return nil, &OpErr{Err: err, Name: name, Op: "get"}
 	}
 
-	s.setNamed(name, v.DriverName(), ref)
+	s.setNamed(v, ref)
 	return v, nil
 }
 
@@ -221,6 +230,7 @@ func (s *VolumeStore) Get(name string) (volume.Volume, error) {
 	if err != nil {
 		return nil, &OpErr{Err: err, Name: name, Op: "get"}
 	}
+	s.setNamed(v, "")
 	return v, nil
 }
 
@@ -229,8 +239,8 @@ func (s *VolumeStore) Get(name string) (volume.Volume, error) {
 // it is expected that callers of this function hold any neccessary locks
 func (s *VolumeStore) getVolume(name string) (volume.Volume, error) {
 	logrus.Debugf("Getting volume reference for name: %s", name)
-	if vdName, exists := s.names[name]; exists {
-		vd, err := volumedrivers.GetDriver(vdName)
+	if v, exists := s.names[name]; exists {
+		vd, err := volumedrivers.GetDriver(v.DriverName())
 		if err != nil {
 			return nil, err
 		}