new file mode 100644

@@ -0,0 +1 @@

+package cmd

new file mode 100644

@@ -0,0 +1,23 @@

+package main

+

+import (

+	"net"

+	"net/http"

+)

+

+func main() {

+	l, err := net.Listen("unix", "/run/docker/plugins/plugin.sock")

+	if err != nil {

+		panic(err)

+	}

+

+	mux := http.NewServeMux()

+	server := http.Server{

+		Addr:    l.Addr().String(),

+		Handler: http.NewServeMux(),

+	}

+	mux.HandleFunc("/VolumeDriver.Create", func(w http.ResponseWriter, r *http.Request) {

+		http.Error(w, "error during create", http.StatusInternalServerError)

+	})

+	server.Serve(l)

+}

new file mode 100644

@@ -0,0 +1 @@

+package main

new file mode 100644

@@ -0,0 +1,51 @@

+// +build linux

+

+package volume

+

+import (

+	"context"

+	"testing"

+

+	"github.com/docker/docker/api/types"

+	"github.com/docker/docker/api/types/volume"

+	"github.com/docker/docker/integration-cli/daemon"

+)

+

+// TestCreateDerefOnError ensures that if a volume create fails, that the plugin is dereferenced

+// Normally 1 volume == 1 reference to a plugin, which prevents a plugin from being removed.

+// If the volume create fails, we should make sure to dereference the plugin.

+func TestCreateDerefOnError(t *testing.T) {

+	t.Parallel()

+

+	d := daemon.New(t, "", dockerdBinary, daemon.Config{})

+	d.Start(t)

+	defer d.Stop(t)

+

+	c, err := d.NewClient()

+	if err != nil {

+		t.Fatal(err)

+	}

+

+	pName := "testderef"

+	createPlugin(t, c, pName, "create-error", asVolumeDriver)

+

+	if err := c.PluginEnable(context.Background(), pName, types.PluginEnableOptions{Timeout: 30}); err != nil {

+		t.Fatal(err)

+	}

+

+	_, err = c.VolumeCreate(context.Background(), volume.VolumesCreateBody{

+		Driver: pName,

+		Name:   "fake",

+	})

+	if err == nil {

+		t.Fatal("volume create should have failed")

+	}

+

+	if err := c.PluginDisable(context.Background(), pName, types.PluginDisableOptions{}); err != nil {

+		t.Fatal(err)

+	}

+

+	if err := c.PluginRemove(context.Background(), pName, types.PluginRemoveOptions{}); err != nil {

+		t.Fatal(err)

+	}

+}

new file mode 100644

@@ -0,0 +1,69 @@

+package volume

+

+import (

+	"context"

+	"os"

+	"os/exec"

+	"path/filepath"

+	"testing"

+	"time"

+

+	"github.com/docker/docker/api/types"

+	"github.com/docker/docker/integration-cli/fixtures/plugin"

+	"github.com/docker/docker/pkg/locker"

+	"github.com/pkg/errors"

+)

+

+const dockerdBinary = "dockerd"

+

+var pluginBuildLock = locker.New()

+

+func ensurePlugin(t *testing.T, name string) string {

+	pluginBuildLock.Lock(name)

+	defer pluginBuildLock.Unlock(name)

+

+	installPath := filepath.Join(os.Getenv("GOPATH"), "bin", name)

+	if _, err := os.Stat(installPath); err == nil {

+		return installPath

+	}

+

+	goBin, err := exec.LookPath("go")

+	if err != nil {

+		t.Fatal(err)

+	}

+

+	cmd := exec.Command(goBin, "build", "-o", installPath, "./"+filepath.Join("cmd", name))

+	cmd.Env = append(cmd.Env, "CGO_ENABLED=0")

+	if out, err := cmd.CombinedOutput(); err != nil {

+		t.Fatal(errors.Wrapf(err, "error building basic plugin bin: %s", string(out)))

+	}

+

+	return installPath

+}

+

+func asVolumeDriver(cfg *plugin.Config) {

+	cfg.Interface.Types = []types.PluginInterfaceType{

+		{Capability: "volumedriver", Prefix: "docker", Version: "1.0"},

+	}

+}

+

+func withSockPath(name string) func(*plugin.Config) {

+	return func(cfg *plugin.Config) {

+		cfg.Interface.Socket = name

+	}

+}

+

+func createPlugin(t *testing.T, client plugin.CreateClient, alias, bin string, opts ...plugin.CreateOpt) {

+	pluginBin := ensurePlugin(t, bin)

+

+	opts = append(opts, withSockPath("plugin.sock"))

+	opts = append(opts, plugin.WithBinary(pluginBin))

+

+	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)

+	err := plugin.Create(ctx, client, alias, opts...)

+	cancel()

+

+	if err != nil {

+		t.Fatal(err)

+	}

+}

@@ -1,6 +1,8 @@

 package plugingetter

-import "github.com/docker/docker/pkg/plugins"

+import (

+	"github.com/docker/docker/pkg/plugins"

+)

 const (

 	// Lookup doesn't update RefCount

@@ -115,10 +115,15 @@ func (ps *Store) Get(name, capability string, mode int) (plugingetter.CompatPlug

 	if ps != nil {

 		p, err := ps.GetV2Plugin(name)

 		if err == nil {

-			p.AddRefCount(mode)

 			if p.IsEnabled() {

-				return p.FilterByCap(capability)

+				fp, err := p.FilterByCap(capability)

+				if err != nil {

+					return nil, err

+				}

+				p.AddRefCount(mode)

+				return fp, nil

 			}

+

 			// Plugin was found but it is disabled, so we should not fall back to legacy plugins

 			// but we should error out right away

 			return nil, errDisabled(name)

@@ -4,6 +4,7 @@ import (

 	"testing"

 	"github.com/docker/docker/api/types"

+	"github.com/docker/docker/pkg/plugingetter"

 	"github.com/docker/docker/plugin/v2"

 )

@@ -31,3 +32,33 @@ func TestFilterByCapPos(t *testing.T) {

 		t.Fatalf("expected no error, got %v", err)

 	}

 }

+

+func TestStoreGetPluginNotMatchCapRefs(t *testing.T) {

+	s := NewStore()

+	p := v2.Plugin{PluginObj: types.Plugin{Name: "test:latest"}}

+

+	iType := types.PluginInterfaceType{Capability: "whatever", Prefix: "docker", Version: "1.0"}

+	i := types.PluginConfigInterface{Socket: "plugins.sock", Types: []types.PluginInterfaceType{iType}}

+	p.PluginObj.Config.Interface = i

+

+	if err := s.Add(&p); err != nil {

+		t.Fatal(err)

+	}

+

+	if _, err := s.Get("test", "volumedriver", plugingetter.Acquire); err == nil {

+		t.Fatal("exepcted error when getting plugin that doesn't match the passed in capability")

+	}

+

+	if refs := p.GetRefCount(); refs != 0 {

+		t.Fatalf("reference count should be 0, got: %d", refs)

+	}

+

+	p.PluginObj.Enabled = true

+	if _, err := s.Get("test", "volumedriver", plugingetter.Acquire); err == nil {

+		t.Fatal("exepcted error when getting plugin that doesn't match the passed in capability")

+	}

+

+	if refs := p.GetRefCount(); refs != 0 {

+		t.Fatalf("reference count should be 0, got: %d", refs)

+	}

+}

@@ -11,6 +11,7 @@ import (

 	getter "github.com/docker/docker/pkg/plugingetter"

 	"github.com/docker/docker/volume"

 	"github.com/pkg/errors"

+	"github.com/sirupsen/logrus"

 )

 // currently created by hand. generation tool would generate this like:

@@ -130,6 +131,12 @@ func lookup(name string, mode int) (volume.Driver, error) {

 		d := NewVolumeDriver(p.Name(), p.BasePath(), p.Client())

 		if err := validateDriver(d); err != nil {

+			if mode > 0 {

+				// Undo any reference count changes from the initial `Get`

+				if _, err := drivers.plugingetter.Get(name, extName, mode*-1); err != nil {

+					logrus.WithError(err).WithField("action", "validate-driver").WithField("plugin", name).Error("error releasing reference to plugin")

+				}

+			}

 			return nil, err

 		}

@@ -169,9 +176,9 @@ func CreateDriver(name string) (volume.Driver, error) {

 	return lookup(name, getter.Acquire)

 }

-// RemoveDriver returns a volume driver by its name and decrements RefCount..

+// ReleaseDriver returns a volume driver by its name and decrements RefCount..

 // If the driver is empty, it looks for the local driver.

-func RemoveDriver(name string) (volume.Driver, error) {

+func ReleaseDriver(name string) (volume.Driver, error) {

 	if name == "" {

 		name = volume.DefaultDriverName

 	}

@@ -145,7 +145,7 @@ func (s *VolumeStore) Purge(name string) {

 	s.globalLock.Lock()

 	v, exists := s.names[name]

 	if exists {

-		if _, err := volumedrivers.RemoveDriver(v.DriverName()); err != nil {

+		if _, err := volumedrivers.ReleaseDriver(v.DriverName()); err != nil {

 			logrus.Errorf("Error dereferencing volume driver: %v", err)

 		}

 	}

@@ -409,6 +409,9 @@ func (s *VolumeStore) create(name, driverName string, opts, labels map[string]st

 	}

 	v, err = vd.Create(name, opts)

 	if err != nil {

+		if _, err := volumedrivers.ReleaseDriver(driverName); err != nil {

+			logrus.WithError(err).WithField("driver", driverName).Error("Error releasing reference to volume driver")

+		}

 		return nil, err

 	}

 	s.globalLock.Lock()