Merge pull request #15917 from srust/volume_create_dont_lock_around_volume_driver

Don't hold lock around volume driver for volume create.

daemon/volumes.go

@@ -133,11 +133,12 @@ func getVolumeDriver(name string) (volume.Driver, error) {
 // Create tries to find an existing volume with the given name or create a new one from the passed in driver
 func (s *volumeStore) Create(name, driverName string, opts map[string]string) (volume.Volume, error) {
 	s.mu.Lock()
-	defer s.mu.Unlock()
-
 	if vc, exists := s.vols[name]; exists {
-		return vc.Volume, nil
+		v := vc.Volume
+		s.mu.Unlock()
+		return v, nil
 	}
+	s.mu.Unlock()
 
 	vd, err := getVolumeDriver(driverName)
 	if err != nil {
@@ -149,7 +150,10 @@ func (s *volumeStore) Create(name, driverName string, opts map[string]string) (v
 		return nil, err
 	}
 
+	s.mu.Lock()
 	s.vols[v.Name()] = &volumeCounter{v, 0}
+	s.mu.Unlock()
+
 	return v, nil
 }
 

integration-cli/docker_cli_start_volume_driver_unix_test.go

@@ -9,8 +9,10 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/go-check/check"
 )
@@ -271,3 +273,40 @@ func (s *DockerExternalVolumeSuite) TestStartExternalNamedVolumeDriverCheckBindL
 	c.Assert(s.ec.mounts, check.Equals, 1)
 	c.Assert(s.ec.unmounts, check.Equals, 1)
 }
+
+// Make sure a request to use a down driver doesn't block other requests
+func (s *DockerExternalVolumeSuite) TestStartExternalVolumeDriverLookupNotBlocked(c *check.C) {
+	specPath := "/etc/docker/plugins/down-driver.spec"
+	err := ioutil.WriteFile("/etc/docker/plugins/down-driver.spec", []byte("tcp://127.0.0.7:9999"), 0644)
+	c.Assert(err, check.IsNil)
+	defer os.RemoveAll(specPath)
+
+	chCmd1 := make(chan struct{})
+	chCmd2 := make(chan error)
+	cmd1 := exec.Command(dockerBinary, "volume", "create", "-d", "down-driver")
+	cmd2 := exec.Command(dockerBinary, "volume", "create")
+
+	c.Assert(cmd1.Start(), check.IsNil)
+	defer cmd1.Process.Kill()
+	time.Sleep(100 * time.Millisecond) // ensure API has been called
+	c.Assert(cmd2.Start(), check.IsNil)
+
+	go func() {
+		cmd1.Wait()
+		close(chCmd1)
+	}()
+	go func() {
+		chCmd2 <- cmd2.Wait()
+	}()
+
+	select {
+	case <-chCmd1:
+		cmd2.Process.Kill()
+		c.Fatalf("volume create with down driver finished unexpectedly")
+	case err := <-chCmd2:
+		c.Assert(err, check.IsNil, check.Commentf("error creating volume"))
+	case <-time.After(5 * time.Second):
+		c.Fatal("volume creates are blocked by previous create requests when previous driver is down")
+		cmd2.Process.Kill()
+	}
+}