Merge pull request #35512 from cpuguy83/replace_plugun_integration_with_unit

Replace vol plugin integration test w/ unit test

integration/plugin/volume/cmd/cmd_test.go

@@ -1 +0,0 @@
-package cmd

integration/plugin/volume/cmd/create-error/main.go

@@ -1,23 +0,0 @@
-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)
-}

integration/plugin/volume/cmd/create-error/main_test.go

@@ -1 +0,0 @@
-package main

integration/plugin/volume/create_test.go

@@ -1,51 +0,0 @@
-// +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)
-	}
-}

integration/plugin/volume/main_test.go

@@ -1,69 +0,0 @@
-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)
-	}
-}

volume/store/store_test.go

@@ -2,7 +2,9 @@ package store
 
 import (
 	"errors"
+	"fmt"
 	"io/ioutil"
+	"net"
 	"os"
 	"strings"
 	"testing"
@@ -266,3 +268,55 @@ func TestCreateKeepOptsLabelsWhenExistsRemotely(t *testing.T) {
 		t.Fatalf("got unexpected type: %T", v)
 	}
 }
+
+func TestDefererencePluginOnCreateError(t *testing.T) {
+	var (
+		l   net.Listener
+		err error
+	)
+
+	for i := 32768; l == nil && i < 40000; i++ {
+		l, err = net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", i))
+	}
+	if l == nil {
+		t.Fatalf("could not create listener: %v", err)
+	}
+	defer l.Close()
+
+	d := volumetestutils.NewFakeDriver("TestDefererencePluginOnCreateError")
+	p, err := volumetestutils.MakeFakePlugin(d, l)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	pg := volumetestutils.NewFakePluginGetter(p)
+	volumedrivers.RegisterPluginGetter(pg)
+
+	dir, err := ioutil.TempDir("", "test-plugin-deref-err")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	s, err := New(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// create a good volume so we have a plugin reference
+	_, err = s.Create("fake1", d.Name(), nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Now create another one expecting an error
+	_, err = s.Create("fake2", d.Name(), map[string]string{"error": "some error"}, nil)
+	if err == nil || !strings.Contains(err.Error(), "some error") {
+		t.Fatalf("expected an error on create: %v", err)
+	}
+
+	// There should be only 1 plugin reference
+	if refs := volumetestutils.FakeRefs(p); refs != 1 {
+		t.Fatalf("expected 1 plugin reference, got: %d", refs)
+	}
+}

volume/testutils/testutils.go

@@ -1,9 +1,15 @@
 package testutils
 
 import (
+	"encoding/json"
+	"errors"
 	"fmt"
+	"net"
+	"net/http"
 	"time"
 
+	"github.com/docker/docker/pkg/plugingetter"
+	"github.com/docker/docker/pkg/plugins"
 	"github.com/docker/docker/volume"
 )
 
@@ -121,3 +127,99 @@ func (d *FakeDriver) Get(name string) (volume.Volume, error) {
 func (*FakeDriver) Scope() string {
 	return "local"
 }
+
+type fakePlugin struct {
+	client *plugins.Client
+	name   string
+	refs   int
+}
+
+// MakeFakePlugin creates a fake plugin from the passed in driver
+// Note: currently only "Create" is implemented because that's all that's needed
+// so far. If you need it to test something else, add it here, but probably you
+// shouldn't need to use this except for very specific cases with v2 plugin handling.
+func MakeFakePlugin(d volume.Driver, l net.Listener) (plugingetter.CompatPlugin, error) {
+	c, err := plugins.NewClient(l.Addr().Network()+"://"+l.Addr().String(), nil)
+	if err != nil {
+		return nil, err
+	}
+	mux := http.NewServeMux()
+
+	mux.HandleFunc("/VolumeDriver.Create", func(w http.ResponseWriter, r *http.Request) {
+		createReq := struct {
+			Name string
+			Opts map[string]string
+		}{}
+		if err := json.NewDecoder(r.Body).Decode(&createReq); err != nil {
+			fmt.Fprintf(w, `{"Err": "%s"}`, err.Error())
+			return
+		}
+		_, err := d.Create(createReq.Name, createReq.Opts)
+		if err != nil {
+			fmt.Fprintf(w, `{"Err": "%s"}`, err.Error())
+			return
+		}
+		w.Write([]byte("{}"))
+	})
+
+	go http.Serve(l, mux)
+	return &fakePlugin{client: c, name: d.Name()}, nil
+}
+
+func (p *fakePlugin) Client() *plugins.Client {
+	return p.client
+}
+
+func (p *fakePlugin) Name() string {
+	return p.name
+}
+
+func (p *fakePlugin) IsV1() bool {
+	return false
+}
+
+func (p *fakePlugin) BasePath() string {
+	return ""
+}
+
+type fakePluginGetter struct {
+	plugins map[string]plugingetter.CompatPlugin
+}
+
+// NewFakePluginGetter returns a plugin getter for fake plugins
+func NewFakePluginGetter(pls ...plugingetter.CompatPlugin) plugingetter.PluginGetter {
+	idx := make(map[string]plugingetter.CompatPlugin, len(pls))
+	for _, p := range pls {
+		idx[p.Name()] = p
+	}
+	return &fakePluginGetter{plugins: idx}
+}
+
+// This ignores the second argument since we only care about volume drivers here,
+// there shouldn't be any other kind of plugin in here
+func (g *fakePluginGetter) Get(name, _ string, mode int) (plugingetter.CompatPlugin, error) {
+	p, ok := g.plugins[name]
+	if !ok {
+		return nil, errors.New("not found")
+	}
+	p.(*fakePlugin).refs += mode
+	return p, nil
+}
+
+func (g *fakePluginGetter) GetAllByCap(capability string) ([]plugingetter.CompatPlugin, error) {
+	panic("GetAllByCap shouldn't be called")
+}
+
+func (g *fakePluginGetter) GetAllManagedPluginsByCap(capability string) []plugingetter.CompatPlugin {
+	panic("GetAllManagedPluginsByCap should not be called")
+}
+
+func (g *fakePluginGetter) Handle(capability string, callback func(string, *plugins.Client)) {
+	panic("Handle should not be called")
+}
+
+// FakeRefs checks ref count on a fake plugin.
+func FakeRefs(p plugingetter.CompatPlugin) int {
+	// this should panic if something other than a `*fakePlugin` is passed in
+	return p.(*fakePlugin).refs
+}