Browse code

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

Signed-off-by: Stephen Rust <srust@blockbridge.com>

Stephen Rust authored on 2015/08/29 03:16:18
Showing 2 changed files
... ...
@@ -133,11 +133,12 @@ func getVolumeDriver(name string) (volume.Driver, error) {
133 133
 // Create tries to find an existing volume with the given name or create a new one from the passed in driver
134 134
 func (s *volumeStore) Create(name, driverName string, opts map[string]string) (volume.Volume, error) {
135 135
 	s.mu.Lock()
136
-	defer s.mu.Unlock()
137
-
138 136
 	if vc, exists := s.vols[name]; exists {
139
-		return vc.Volume, nil
137
+		v := vc.Volume
138
+		s.mu.Unlock()
139
+		return v, nil
140 140
 	}
141
+	s.mu.Unlock()
141 142
 
142 143
 	vd, err := getVolumeDriver(driverName)
143 144
 	if err != nil {
... ...
@@ -149,7 +150,10 @@ func (s *volumeStore) Create(name, driverName string, opts map[string]string) (v
149 149
 		return nil, err
150 150
 	}
151 151
 
152
+	s.mu.Lock()
152 153
 	s.vols[v.Name()] = &volumeCounter{v, 0}
154
+	s.mu.Unlock()
155
+
153 156
 	return v, nil
154 157
 }
155 158
 
... ...
@@ -9,8 +9,10 @@ import (
9 9
 	"net/http"
10 10
 	"net/http/httptest"
11 11
 	"os"
12
+	"os/exec"
12 13
 	"path/filepath"
13 14
 	"strings"
15
+	"time"
14 16
 
15 17
 	"github.com/go-check/check"
16 18
 )
... ...
@@ -271,3 +273,40 @@ func (s *DockerExternalVolumeSuite) TestStartExternalNamedVolumeDriverCheckBindL
271 271
 	c.Assert(s.ec.mounts, check.Equals, 1)
272 272
 	c.Assert(s.ec.unmounts, check.Equals, 1)
273 273
 }
274
+
275
+// Make sure a request to use a down driver doesn't block other requests
276
+func (s *DockerExternalVolumeSuite) TestStartExternalVolumeDriverLookupNotBlocked(c *check.C) {
277
+	specPath := "/etc/docker/plugins/down-driver.spec"
278
+	err := ioutil.WriteFile("/etc/docker/plugins/down-driver.spec", []byte("tcp://127.0.0.7:9999"), 0644)
279
+	c.Assert(err, check.IsNil)
280
+	defer os.RemoveAll(specPath)
281
+
282
+	chCmd1 := make(chan struct{})
283
+	chCmd2 := make(chan error)
284
+	cmd1 := exec.Command(dockerBinary, "volume", "create", "-d", "down-driver")
285
+	cmd2 := exec.Command(dockerBinary, "volume", "create")
286
+
287
+	c.Assert(cmd1.Start(), check.IsNil)
288
+	defer cmd1.Process.Kill()
289
+	time.Sleep(100 * time.Millisecond) // ensure API has been called
290
+	c.Assert(cmd2.Start(), check.IsNil)
291
+
292
+	go func() {
293
+		cmd1.Wait()
294
+		close(chCmd1)
295
+	}()
296
+	go func() {
297
+		chCmd2 <- cmd2.Wait()
298
+	}()
299
+
300
+	select {
301
+	case <-chCmd1:
302
+		cmd2.Process.Kill()
303
+		c.Fatalf("volume create with down driver finished unexpectedly")
304
+	case err := <-chCmd2:
305
+		c.Assert(err, check.IsNil, check.Commentf("error creating volume"))
306
+	case <-time.After(5 * time.Second):
307
+		c.Fatal("volume creates are blocked by previous create requests when previous driver is down")
308
+		cmd2.Process.Kill()
309
+	}
310
+}