From 00d801dd85b486eb46eff7bd041c33f04e373699 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 15 Nov 2017 13:13:22 -0500 Subject: [PATCH] Replace vol plugin integration test w/ unit test Signed-off-by: Brian Goff --- integration/plugin/volume/cmd/cmd_test.go | 1 - .../plugin/volume/cmd/create-error/main.go | 23 ---- .../volume/cmd/create-error/main_test.go | 1 - integration/plugin/volume/create_test.go | 51 --------- integration/plugin/volume/main_test.go | 69 ------------ volume/store/store_test.go | 54 ++++++++++ volume/testutils/testutils.go | 102 ++++++++++++++++++ 7 files changed, 156 insertions(+), 145 deletions(-) delete mode 100644 integration/plugin/volume/cmd/cmd_test.go delete mode 100644 integration/plugin/volume/cmd/create-error/main.go delete mode 100644 integration/plugin/volume/cmd/create-error/main_test.go delete mode 100644 integration/plugin/volume/create_test.go delete mode 100644 integration/plugin/volume/main_test.go diff --git a/integration/plugin/volume/cmd/cmd_test.go b/integration/plugin/volume/cmd/cmd_test.go deleted file mode 100644 index 1d619dd05e..0000000000 --- a/integration/plugin/volume/cmd/cmd_test.go +++ /dev/null @@ -1 +0,0 @@ -package cmd diff --git a/integration/plugin/volume/cmd/create-error/main.go b/integration/plugin/volume/cmd/create-error/main.go deleted file mode 100644 index f23be51d66..0000000000 --- a/integration/plugin/volume/cmd/create-error/main.go +++ /dev/null @@ -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) -} diff --git a/integration/plugin/volume/cmd/create-error/main_test.go b/integration/plugin/volume/cmd/create-error/main_test.go deleted file mode 100644 index 06ab7d0f9a..0000000000 --- a/integration/plugin/volume/cmd/create-error/main_test.go +++ /dev/null @@ -1 +0,0 @@ -package main diff --git a/integration/plugin/volume/create_test.go b/integration/plugin/volume/create_test.go deleted file mode 100644 index ce9b4dca43..0000000000 --- a/integration/plugin/volume/create_test.go +++ /dev/null @@ -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) - } -} diff --git a/integration/plugin/volume/main_test.go b/integration/plugin/volume/main_test.go deleted file mode 100644 index 8cfbf37978..0000000000 --- a/integration/plugin/volume/main_test.go +++ /dev/null @@ -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) - } -} diff --git a/volume/store/store_test.go b/volume/store/store_test.go index eb78c85cbe..7d5294043d 100644 --- a/volume/store/store_test.go +++ b/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) + } +} diff --git a/volume/testutils/testutils.go b/volume/testutils/testutils.go index 359d923822..84ab55ff77 100644 --- a/volume/testutils/testutils.go +++ b/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 +}