Fix volume creates blocked by stale cache entries
| ... | ... |
@@ -17,10 +17,13 @@ import ( |
| 17 | 17 |
|
| 18 | 18 |
"github.com/docker/docker/api/types" |
| 19 | 19 |
"github.com/docker/docker/pkg/integration/checker" |
| 20 |
+ "github.com/docker/docker/pkg/stringid" |
|
| 20 | 21 |
"github.com/docker/docker/volume" |
| 21 | 22 |
"github.com/go-check/check" |
| 22 | 23 |
) |
| 23 | 24 |
|
| 25 |
+const volumePluginName = "test-external-volume-driver" |
|
| 26 |
+ |
|
| 24 | 27 |
func init() {
|
| 25 | 28 |
check.Suite(&DockerExternalVolumeSuite{
|
| 26 | 29 |
ds: &DockerSuite{},
|
| ... | ... |
@@ -40,10 +43,9 @@ type eventCounter struct {
|
| 40 | 40 |
} |
| 41 | 41 |
|
| 42 | 42 |
type DockerExternalVolumeSuite struct {
|
| 43 |
- server *httptest.Server |
|
| 44 |
- ds *DockerSuite |
|
| 45 |
- d *Daemon |
|
| 46 |
- ec *eventCounter |
|
| 43 |
+ ds *DockerSuite |
|
| 44 |
+ d *Daemon |
|
| 45 |
+ *volumePlugin |
|
| 47 | 46 |
} |
| 48 | 47 |
|
| 49 | 48 |
func (s *DockerExternalVolumeSuite) SetUpTest(c *check.C) {
|
| ... | ... |
@@ -57,8 +59,29 @@ func (s *DockerExternalVolumeSuite) TearDownTest(c *check.C) {
|
| 57 | 57 |
} |
| 58 | 58 |
|
| 59 | 59 |
func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) {
|
| 60 |
+ s.volumePlugin = newVolumePlugin(c, volumePluginName) |
|
| 61 |
+} |
|
| 62 |
+ |
|
| 63 |
+type volumePlugin struct {
|
|
| 64 |
+ ec *eventCounter |
|
| 65 |
+ *httptest.Server |
|
| 66 |
+ vols map[string]vol |
|
| 67 |
+} |
|
| 68 |
+ |
|
| 69 |
+type vol struct {
|
|
| 70 |
+ Name string |
|
| 71 |
+ Mountpoint string |
|
| 72 |
+ Ninja bool // hack used to trigger a null volume return on `Get` |
|
| 73 |
+ Status map[string]interface{}
|
|
| 74 |
+} |
|
| 75 |
+ |
|
| 76 |
+func (p *volumePlugin) Close() {
|
|
| 77 |
+ p.Server.Close() |
|
| 78 |
+} |
|
| 79 |
+ |
|
| 80 |
+func newVolumePlugin(c *check.C, name string) *volumePlugin {
|
|
| 60 | 81 |
mux := http.NewServeMux() |
| 61 |
- s.server = httptest.NewServer(mux) |
|
| 82 |
+ s := &volumePlugin{Server: httptest.NewServer(mux), ec: &eventCounter{}, vols: make(map[string]vol)}
|
|
| 62 | 83 |
|
| 63 | 84 |
type pluginRequest struct {
|
| 64 | 85 |
Name string |
| ... | ... |
@@ -71,14 +94,6 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) {
|
| 71 | 71 |
Err string `json:",omitempty"` |
| 72 | 72 |
} |
| 73 | 73 |
|
| 74 |
- type vol struct {
|
|
| 75 |
- Name string |
|
| 76 |
- Mountpoint string |
|
| 77 |
- Ninja bool // hack used to trigger a null volume return on `Get` |
|
| 78 |
- Status map[string]interface{}
|
|
| 79 |
- } |
|
| 80 |
- var volList []vol |
|
| 81 |
- |
|
| 82 | 74 |
read := func(b io.ReadCloser) (pluginRequest, error) {
|
| 83 | 75 |
defer b.Close() |
| 84 | 76 |
var pr pluginRequest |
| ... | ... |
@@ -115,14 +130,14 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) {
|
| 115 | 115 |
} |
| 116 | 116 |
_, isNinja := pr.Opts["ninja"] |
| 117 | 117 |
status := map[string]interface{}{"Hello": "world"}
|
| 118 |
- volList = append(volList, vol{Name: pr.Name, Ninja: isNinja, Status: status})
|
|
| 118 |
+ s.vols[pr.Name] = vol{Name: pr.Name, Ninja: isNinja, Status: status}
|
|
| 119 | 119 |
send(w, nil) |
| 120 | 120 |
}) |
| 121 | 121 |
|
| 122 | 122 |
mux.HandleFunc("/VolumeDriver.List", func(w http.ResponseWriter, r *http.Request) {
|
| 123 | 123 |
s.ec.lists++ |
| 124 |
- vols := []vol{}
|
|
| 125 |
- for _, v := range volList {
|
|
| 124 |
+ vols := make([]vol, 0, len(s.vols)) |
|
| 125 |
+ for _, v := range s.vols {
|
|
| 126 | 126 |
if v.Ninja {
|
| 127 | 127 |
continue |
| 128 | 128 |
} |
| ... | ... |
@@ -139,19 +154,19 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) {
|
| 139 | 139 |
return |
| 140 | 140 |
} |
| 141 | 141 |
|
| 142 |
- for _, v := range volList {
|
|
| 143 |
- if v.Name == pr.Name {
|
|
| 144 |
- if v.Ninja {
|
|
| 145 |
- send(w, map[string]vol{})
|
|
| 146 |
- return |
|
| 147 |
- } |
|
| 142 |
+ v, exists := s.vols[pr.Name] |
|
| 143 |
+ if !exists {
|
|
| 144 |
+ send(w, `{"Err": "no such volume"}`)
|
|
| 145 |
+ } |
|
| 148 | 146 |
|
| 149 |
- v.Mountpoint = hostVolumePath(pr.Name) |
|
| 150 |
- send(w, map[string]vol{"Volume": v})
|
|
| 151 |
- return |
|
| 152 |
- } |
|
| 147 |
+ if v.Ninja {
|
|
| 148 |
+ send(w, map[string]vol{})
|
|
| 149 |
+ return |
|
| 153 | 150 |
} |
| 154 |
- send(w, `{"Err": "no such volume"}`)
|
|
| 151 |
+ |
|
| 152 |
+ v.Mountpoint = hostVolumePath(pr.Name) |
|
| 153 |
+ send(w, map[string]vol{"Volume": v})
|
|
| 154 |
+ return |
|
| 155 | 155 |
}) |
| 156 | 156 |
|
| 157 | 157 |
mux.HandleFunc("/VolumeDriver.Remove", func(w http.ResponseWriter, r *http.Request) {
|
| ... | ... |
@@ -162,16 +177,17 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) {
|
| 162 | 162 |
return |
| 163 | 163 |
} |
| 164 | 164 |
|
| 165 |
- for i, v := range volList {
|
|
| 166 |
- if v.Name == pr.Name {
|
|
| 167 |
- if err := os.RemoveAll(hostVolumePath(v.Name)); err != nil {
|
|
| 168 |
- send(w, &pluginResp{Err: err.Error()})
|
|
| 169 |
- return |
|
| 170 |
- } |
|
| 171 |
- volList = append(volList[:i], volList[i+1:]...) |
|
| 172 |
- break |
|
| 173 |
- } |
|
| 165 |
+ v, ok := s.vols[pr.Name] |
|
| 166 |
+ if !ok {
|
|
| 167 |
+ send(w, nil) |
|
| 168 |
+ return |
|
| 169 |
+ } |
|
| 170 |
+ |
|
| 171 |
+ if err := os.RemoveAll(hostVolumePath(v.Name)); err != nil {
|
|
| 172 |
+ send(w, &pluginResp{Err: err.Error()})
|
|
| 173 |
+ return |
|
| 174 | 174 |
} |
| 175 |
+ delete(s.vols, v.Name) |
|
| 175 | 176 |
send(w, nil) |
| 176 | 177 |
}) |
| 177 | 178 |
|
| ... | ... |
@@ -202,7 +218,7 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) {
|
| 202 | 202 |
return |
| 203 | 203 |
} |
| 204 | 204 |
|
| 205 |
- if err := ioutil.WriteFile(filepath.Join(p, "test"), []byte(s.server.URL), 0644); err != nil {
|
|
| 205 |
+ if err := ioutil.WriteFile(filepath.Join(p, "test"), []byte(s.Server.URL), 0644); err != nil {
|
|
| 206 | 206 |
send(w, err) |
| 207 | 207 |
return |
| 208 | 208 |
} |
| ... | ... |
@@ -242,12 +258,13 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) {
|
| 242 | 242 |
err := os.MkdirAll("/etc/docker/plugins", 0755)
|
| 243 | 243 |
c.Assert(err, checker.IsNil) |
| 244 | 244 |
|
| 245 |
- err = ioutil.WriteFile("/etc/docker/plugins/test-external-volume-driver.spec", []byte(s.server.URL), 0644)
|
|
| 245 |
+ err = ioutil.WriteFile("/etc/docker/plugins/"+name+".spec", []byte(s.Server.URL), 0644)
|
|
| 246 | 246 |
c.Assert(err, checker.IsNil) |
| 247 |
+ return s |
|
| 247 | 248 |
} |
| 248 | 249 |
|
| 249 | 250 |
func (s *DockerExternalVolumeSuite) TearDownSuite(c *check.C) {
|
| 250 |
- s.server.Close() |
|
| 251 |
+ s.volumePlugin.Close() |
|
| 251 | 252 |
|
| 252 | 253 |
err := os.RemoveAll("/etc/docker/plugins")
|
| 253 | 254 |
c.Assert(err, checker.IsNil) |
| ... | ... |
@@ -257,9 +274,9 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverNamed(c *check.C) {
|
| 257 | 257 |
err := s.d.StartWithBusybox() |
| 258 | 258 |
c.Assert(err, checker.IsNil) |
| 259 | 259 |
|
| 260 |
- out, err := s.d.Cmd("run", "--rm", "--name", "test-data", "-v", "external-volume-test:/tmp/external-volume-test", "--volume-driver", "test-external-volume-driver", "busybox:latest", "cat", "/tmp/external-volume-test/test")
|
|
| 260 |
+ out, err := s.d.Cmd("run", "--rm", "--name", "test-data", "-v", "external-volume-test:/tmp/external-volume-test", "--volume-driver", volumePluginName, "busybox:latest", "cat", "/tmp/external-volume-test/test")
|
|
| 261 | 261 |
c.Assert(err, checker.IsNil, check.Commentf(out)) |
| 262 |
- c.Assert(out, checker.Contains, s.server.URL) |
|
| 262 |
+ c.Assert(out, checker.Contains, s.Server.URL) |
|
| 263 | 263 |
|
| 264 | 264 |
_, err = s.d.Cmd("volume", "rm", "external-volume-test")
|
| 265 | 265 |
c.Assert(err, checker.IsNil) |
| ... | ... |
@@ -280,9 +297,9 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverUnnamed(c *check.C) |
| 280 | 280 |
err := s.d.StartWithBusybox() |
| 281 | 281 |
c.Assert(err, checker.IsNil) |
| 282 | 282 |
|
| 283 |
- out, err := s.d.Cmd("run", "--rm", "--name", "test-data", "-v", "/tmp/external-volume-test", "--volume-driver", "test-external-volume-driver", "busybox:latest", "cat", "/tmp/external-volume-test/test")
|
|
| 283 |
+ out, err := s.d.Cmd("run", "--rm", "--name", "test-data", "-v", "/tmp/external-volume-test", "--volume-driver", volumePluginName, "busybox:latest", "cat", "/tmp/external-volume-test/test")
|
|
| 284 | 284 |
c.Assert(err, checker.IsNil, check.Commentf(out)) |
| 285 |
- c.Assert(out, checker.Contains, s.server.URL) |
|
| 285 |
+ c.Assert(out, checker.Contains, s.Server.URL) |
|
| 286 | 286 |
|
| 287 | 287 |
c.Assert(s.ec.activations, checker.Equals, 1) |
| 288 | 288 |
c.Assert(s.ec.creations, checker.Equals, 1) |
| ... | ... |
@@ -295,7 +312,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverVolumesFrom(c *check |
| 295 | 295 |
err := s.d.StartWithBusybox() |
| 296 | 296 |
c.Assert(err, checker.IsNil) |
| 297 | 297 |
|
| 298 |
- out, err := s.d.Cmd("run", "--name", "vol-test1", "-v", "/foo", "--volume-driver", "test-external-volume-driver", "busybox:latest")
|
|
| 298 |
+ out, err := s.d.Cmd("run", "--name", "vol-test1", "-v", "/foo", "--volume-driver", volumePluginName, "busybox:latest")
|
|
| 299 | 299 |
c.Assert(err, checker.IsNil, check.Commentf(out)) |
| 300 | 300 |
|
| 301 | 301 |
out, err = s.d.Cmd("run", "--rm", "--volumes-from", "vol-test1", "--name", "vol-test2", "busybox", "ls", "/tmp")
|
| ... | ... |
@@ -315,7 +332,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverDeleteContainer(c *c |
| 315 | 315 |
err := s.d.StartWithBusybox() |
| 316 | 316 |
c.Assert(err, checker.IsNil) |
| 317 | 317 |
|
| 318 |
- out, err := s.d.Cmd("run", "--name", "vol-test1", "-v", "/foo", "--volume-driver", "test-external-volume-driver", "busybox:latest")
|
|
| 318 |
+ out, err := s.d.Cmd("run", "--name", "vol-test1", "-v", "/foo", "--volume-driver", volumePluginName, "busybox:latest")
|
|
| 319 | 319 |
c.Assert(err, checker.IsNil, check.Commentf(out)) |
| 320 | 320 |
|
| 321 | 321 |
out, err = s.d.Cmd("rm", "-fv", "vol-test1")
|
| ... | ... |
@@ -388,7 +405,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverRetryNotImmediatelyE |
| 388 | 388 |
// wait for a retry to occur, then create spec to allow plugin to register |
| 389 | 389 |
time.Sleep(2000 * time.Millisecond) |
| 390 | 390 |
// no need to check for an error here since it will get picked up by the timeout later |
| 391 |
- ioutil.WriteFile(specPath, []byte(s.server.URL), 0644) |
|
| 391 |
+ ioutil.WriteFile(specPath, []byte(s.Server.URL), 0644) |
|
| 392 | 392 |
}() |
| 393 | 393 |
|
| 394 | 394 |
select {
|
| ... | ... |
@@ -409,7 +426,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverRetryNotImmediatelyE |
| 409 | 409 |
} |
| 410 | 410 |
|
| 411 | 411 |
func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverBindExternalVolume(c *check.C) {
|
| 412 |
- dockerCmd(c, "volume", "create", "-d", "test-external-volume-driver", "foo") |
|
| 412 |
+ dockerCmd(c, "volume", "create", "-d", volumePluginName, "foo") |
|
| 413 | 413 |
dockerCmd(c, "run", "-d", "--name", "testing", "-v", "foo:/bar", "busybox", "top") |
| 414 | 414 |
|
| 415 | 415 |
var mounts []struct {
|
| ... | ... |
@@ -420,18 +437,18 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverBindExternalVolume(c |
| 420 | 420 |
c.Assert(json.NewDecoder(strings.NewReader(out)).Decode(&mounts), checker.IsNil) |
| 421 | 421 |
c.Assert(len(mounts), checker.Equals, 1, check.Commentf(out)) |
| 422 | 422 |
c.Assert(mounts[0].Name, checker.Equals, "foo") |
| 423 |
- c.Assert(mounts[0].Driver, checker.Equals, "test-external-volume-driver") |
|
| 423 |
+ c.Assert(mounts[0].Driver, checker.Equals, volumePluginName) |
|
| 424 | 424 |
} |
| 425 | 425 |
|
| 426 | 426 |
func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverList(c *check.C) {
|
| 427 |
- dockerCmd(c, "volume", "create", "-d", "test-external-volume-driver", "abc3") |
|
| 427 |
+ dockerCmd(c, "volume", "create", "-d", volumePluginName, "abc3") |
|
| 428 | 428 |
out, _ := dockerCmd(c, "volume", "ls") |
| 429 | 429 |
ls := strings.Split(strings.TrimSpace(out), "\n") |
| 430 | 430 |
c.Assert(len(ls), check.Equals, 2, check.Commentf("\n%s", out))
|
| 431 | 431 |
|
| 432 | 432 |
vol := strings.Fields(ls[len(ls)-1]) |
| 433 | 433 |
c.Assert(len(vol), check.Equals, 2, check.Commentf("%v", vol))
|
| 434 |
- c.Assert(vol[0], check.Equals, "test-external-volume-driver") |
|
| 434 |
+ c.Assert(vol[0], check.Equals, volumePluginName) |
|
| 435 | 435 |
c.Assert(vol[1], check.Equals, "abc3") |
| 436 | 436 |
|
| 437 | 437 |
c.Assert(s.ec.lists, check.Equals, 1) |
| ... | ... |
@@ -440,10 +457,10 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverList(c *check.C) {
|
| 440 | 440 |
func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverGet(c *check.C) {
|
| 441 | 441 |
out, _, err := dockerCmdWithError("volume", "inspect", "dummy")
|
| 442 | 442 |
c.Assert(err, check.NotNil, check.Commentf(out)) |
| 443 |
- c.Assert(s.ec.gets, check.Equals, 1) |
|
| 444 | 443 |
c.Assert(out, checker.Contains, "No such volume") |
| 444 |
+ c.Assert(s.ec.gets, check.Equals, 1) |
|
| 445 | 445 |
|
| 446 |
- dockerCmd(c, "volume", "create", "test", "-d", "test-external-volume-driver") |
|
| 446 |
+ dockerCmd(c, "volume", "create", "test", "-d", volumePluginName) |
|
| 447 | 447 |
out, _ = dockerCmd(c, "volume", "inspect", "test") |
| 448 | 448 |
|
| 449 | 449 |
type vol struct {
|
| ... | ... |
@@ -458,7 +475,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverGet(c *check.C) {
|
| 458 | 458 |
} |
| 459 | 459 |
|
| 460 | 460 |
func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverWithDaemonRestart(c *check.C) {
|
| 461 |
- dockerCmd(c, "volume", "create", "-d", "test-external-volume-driver", "abc1") |
|
| 461 |
+ dockerCmd(c, "volume", "create", "-d", volumePluginName, "abc1") |
|
| 462 | 462 |
err := s.d.Restart() |
| 463 | 463 |
c.Assert(err, checker.IsNil) |
| 464 | 464 |
|
| ... | ... |
@@ -466,7 +483,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverWithDaemonRestart(c |
| 466 | 466 |
var mounts []types.MountPoint |
| 467 | 467 |
inspectFieldAndMarshall(c, "test", "Mounts", &mounts) |
| 468 | 468 |
c.Assert(mounts, checker.HasLen, 1) |
| 469 |
- c.Assert(mounts[0].Driver, checker.Equals, "test-external-volume-driver") |
|
| 469 |
+ c.Assert(mounts[0].Driver, checker.Equals, volumePluginName) |
|
| 470 | 470 |
} |
| 471 | 471 |
|
| 472 | 472 |
// Ensures that the daemon handles when the plugin responds to a `Get` request with a null volume and a null error. |
| ... | ... |
@@ -474,7 +491,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverWithDaemonRestart(c |
| 474 | 474 |
func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverGetEmptyResponse(c *check.C) {
|
| 475 | 475 |
c.Assert(s.d.Start(), checker.IsNil) |
| 476 | 476 |
|
| 477 |
- out, err := s.d.Cmd("volume", "create", "-d", "test-external-volume-driver", "abc2", "--opt", "ninja=1")
|
|
| 477 |
+ out, err := s.d.Cmd("volume", "create", "-d", volumePluginName, "abc2", "--opt", "ninja=1")
|
|
| 478 | 478 |
c.Assert(err, checker.IsNil, check.Commentf(out)) |
| 479 | 479 |
|
| 480 | 480 |
out, err = s.d.Cmd("volume", "inspect", "abc2")
|
| ... | ... |
@@ -505,7 +522,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverMountID(c *check.C) |
| 505 | 505 |
err := s.d.StartWithBusybox() |
| 506 | 506 |
c.Assert(err, checker.IsNil) |
| 507 | 507 |
|
| 508 |
- out, err := s.d.Cmd("run", "--rm", "-v", "external-volume-test:/tmp/external-volume-test", "--volume-driver", "test-external-volume-driver", "busybox:latest", "cat", "/tmp/external-volume-test/test")
|
|
| 508 |
+ out, err := s.d.Cmd("run", "--rm", "-v", "external-volume-test:/tmp/external-volume-test", "--volume-driver", volumePluginName, "busybox:latest", "cat", "/tmp/external-volume-test/test")
|
|
| 509 | 509 |
c.Assert(err, checker.IsNil, check.Commentf(out)) |
| 510 | 510 |
c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "") |
| 511 | 511 |
} |
| ... | ... |
@@ -516,7 +533,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverCapabilities(c *chec |
| 516 | 516 |
c.Assert(s.ec.caps, checker.Equals, 0) |
| 517 | 517 |
|
| 518 | 518 |
for i := 0; i < 3; i++ {
|
| 519 |
- out, err := s.d.Cmd("volume", "create", "-d", "test-external-volume-driver", fmt.Sprintf("test%d", i))
|
|
| 519 |
+ out, err := s.d.Cmd("volume", "create", "-d", volumePluginName, fmt.Sprintf("test%d", i))
|
|
| 520 | 520 |
c.Assert(err, checker.IsNil, check.Commentf(out)) |
| 521 | 521 |
c.Assert(s.ec.caps, checker.Equals, 1) |
| 522 | 522 |
out, err = s.d.Cmd("volume", "inspect", "--format={{.Scope}}", fmt.Sprintf("test%d", i))
|
| ... | ... |
@@ -524,3 +541,24 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverCapabilities(c *chec |
| 524 | 524 |
c.Assert(strings.TrimSpace(out), checker.Equals, volume.GlobalScope) |
| 525 | 525 |
} |
| 526 | 526 |
} |
| 527 |
+ |
|
| 528 |
+func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverOutOfBandDelete(c *check.C) {
|
|
| 529 |
+ driverName := stringid.GenerateNonCryptoID() |
|
| 530 |
+ p := newVolumePlugin(c, driverName) |
|
| 531 |
+ defer p.Close() |
|
| 532 |
+ |
|
| 533 |
+ c.Assert(s.d.StartWithBusybox(), checker.IsNil) |
|
| 534 |
+ |
|
| 535 |
+ out, err := s.d.Cmd("volume", "create", "-d", driverName, "--name", "test")
|
|
| 536 |
+ c.Assert(err, checker.IsNil, check.Commentf(out)) |
|
| 537 |
+ |
|
| 538 |
+ out, err = s.d.Cmd("volume", "create", "-d", "local", "--name", "test")
|
|
| 539 |
+ c.Assert(err, checker.NotNil, check.Commentf(out)) |
|
| 540 |
+ c.Assert(out, checker.Contains, "volume named test already exists") |
|
| 541 |
+ |
|
| 542 |
+ // simulate out of band volume deletion on plugin level |
|
| 543 |
+ delete(p.vols, "test") |
|
| 544 |
+ |
|
| 545 |
+ out, err = s.d.Cmd("volume", "create", "-d", "local", "--name", "test")
|
|
| 546 |
+ c.Assert(err, checker.IsNil, check.Commentf(out)) |
|
| 547 |
+} |
| ... | ... |
@@ -1,8 +1,9 @@ |
| 1 | 1 |
package store |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 |
- "errors" |
|
| 5 | 4 |
"strings" |
| 5 |
+ |
|
| 6 |
+ "github.com/pkg/errors" |
|
| 6 | 7 |
) |
| 7 | 8 |
|
| 8 | 9 |
var ( |
| ... | ... |
@@ -64,11 +65,12 @@ func IsNameConflict(err error) bool {
|
| 64 | 64 |
} |
| 65 | 65 |
|
| 66 | 66 |
func isErr(err error, expected error) bool {
|
| 67 |
+ err = errors.Cause(err) |
|
| 67 | 68 |
switch pe := err.(type) {
|
| 68 | 69 |
case nil: |
| 69 | 70 |
return false |
| 70 | 71 |
case *OpErr: |
| 71 |
- err = pe.Err |
|
| 72 |
+ err = errors.Cause(pe.Err) |
|
| 72 | 73 |
} |
| 73 | 74 |
return err == expected |
| 74 | 75 |
} |
| ... | ... |
@@ -3,6 +3,7 @@ package store |
| 3 | 3 |
import ( |
| 4 | 4 |
"bytes" |
| 5 | 5 |
"encoding/json" |
| 6 |
+ "net" |
|
| 6 | 7 |
"os" |
| 7 | 8 |
"path/filepath" |
| 8 | 9 |
"sync" |
| ... | ... |
@@ -117,6 +118,15 @@ func (s *VolumeStore) setNamed(v volume.Volume, ref string) {
|
| 117 | 117 |
s.globalLock.Unlock() |
| 118 | 118 |
} |
| 119 | 119 |
|
| 120 |
+// getRefs gets the list of refs for a given name |
|
| 121 |
+// Callers of this function are expected to hold the name lock. |
|
| 122 |
+func (s *VolumeStore) getRefs(name string) []string {
|
|
| 123 |
+ s.globalLock.Lock() |
|
| 124 |
+ refs := s.refs[name] |
|
| 125 |
+ s.globalLock.Unlock() |
|
| 126 |
+ return refs |
|
| 127 |
+} |
|
| 128 |
+ |
|
| 120 | 129 |
// Purge allows the cleanup of internal data on docker in case |
| 121 | 130 |
// the internal data is out of sync with volumes driver plugins. |
| 122 | 131 |
func (s *VolumeStore) Purge(name string) {
|
| ... | ... |
@@ -251,9 +261,77 @@ func (s *VolumeStore) Create(name, driverName string, opts, labels map[string]st |
| 251 | 251 |
return s.CreateWithRef(name, driverName, "", opts, labels) |
| 252 | 252 |
} |
| 253 | 253 |
|
| 254 |
+// checkConflict checks the local cache for name collisions with the passed in name, |
|
| 255 |
+// for existing volumes with the same name but in a different driver. |
|
| 256 |
+// This is used by `Create` as a best effort to prevent name collisions for volumes. |
|
| 257 |
+// If a matching volume is found that is not a conflict that is returned so the caller |
|
| 258 |
+// does not need to perform an additional lookup. |
|
| 259 |
+// When no matching volume is found, both returns will be nil |
|
| 260 |
+// |
|
| 261 |
+// Note: This does not probe all the drivers for name collisions because v1 plugins |
|
| 262 |
+// are very slow, particularly if the plugin is down, and cause other issues, |
|
| 263 |
+// particularly around locking the store. |
|
| 264 |
+// TODO(cpuguy83): With v2 plugins this shouldn't be a problem. Could also potentially |
|
| 265 |
+// use a connect timeout for this kind of check to ensure we aren't blocking for a |
|
| 266 |
+// long time. |
|
| 267 |
+func (s *VolumeStore) checkConflict(name, driverName string) (volume.Volume, error) {
|
|
| 268 |
+ // check the local cache |
|
| 269 |
+ v, _ := s.getNamed(name) |
|
| 270 |
+ if v != nil {
|
|
| 271 |
+ vDriverName := v.DriverName() |
|
| 272 |
+ if driverName != "" && vDriverName != driverName {
|
|
| 273 |
+ // we have what looks like a conflict |
|
| 274 |
+ // let's see if there are existing refs to this volume, if so we don't need |
|
| 275 |
+ // to go any further since we can assume the volume is legit. |
|
| 276 |
+ if len(s.getRefs(name)) > 0 {
|
|
| 277 |
+ return nil, errors.Wrapf(errNameConflict, "driver '%s' already has volume '%s'", vDriverName, name) |
|
| 278 |
+ } |
|
| 279 |
+ |
|
| 280 |
+ // looks like there is a conflict, but nothing is referencing it... |
|
| 281 |
+ // let's check if the found volume ref |
|
| 282 |
+ // is stale by checking with the driver if it still exists |
|
| 283 |
+ vd, err := volumedrivers.GetDriver(vDriverName) |
|
| 284 |
+ if err != nil {
|
|
| 285 |
+ // play it safe and return the error |
|
| 286 |
+ // TODO(cpuguy83): maybe when when v2 plugins are ubiquitous, we should |
|
| 287 |
+ // just purge this from the cache |
|
| 288 |
+ return nil, errors.Wrapf(errNameConflict, "found reference to volume '%s' in driver '%s', but got an error while checking the driver: %v", name, vDriverName, err) |
|
| 289 |
+ } |
|
| 290 |
+ |
|
| 291 |
+ // now check if it still exists in the driver |
|
| 292 |
+ v2, err := vd.Get(name) |
|
| 293 |
+ err = errors.Cause(err) |
|
| 294 |
+ if err != nil {
|
|
| 295 |
+ if _, ok := err.(net.Error); ok {
|
|
| 296 |
+ // got some error related to the driver connectivity |
|
| 297 |
+ // play it safe and return the error |
|
| 298 |
+ // TODO(cpuguy83): When when v2 plugins are ubiquitous, maybe we should |
|
| 299 |
+ // just purge this from the cache |
|
| 300 |
+ return nil, errors.Wrapf(errNameConflict, "found reference to volume '%s' in driver '%s', but got an error while checking the driver: %v", name, vDriverName, err) |
|
| 301 |
+ } |
|
| 302 |
+ |
|
| 303 |
+ // a driver can return whatever it wants, so let's make sure this is nil |
|
| 304 |
+ if v2 == nil {
|
|
| 305 |
+ // purge this reference from the cache |
|
| 306 |
+ s.Purge(name) |
|
| 307 |
+ return nil, nil |
|
| 308 |
+ } |
|
| 309 |
+ } |
|
| 310 |
+ if v2 != nil {
|
|
| 311 |
+ return nil, errors.Wrapf(errNameConflict, "driver '%s' already has volume '%s'", vDriverName, name) |
|
| 312 |
+ } |
|
| 313 |
+ } |
|
| 314 |
+ return v, nil |
|
| 315 |
+ } |
|
| 316 |
+ |
|
| 317 |
+ return nil, nil |
|
| 318 |
+} |
|
| 319 |
+ |
|
| 254 | 320 |
// create asks the given driver to create a volume with the name/opts. |
| 255 | 321 |
// If a volume with the name is already known, it will ask the stored driver for the volume. |
| 256 |
-// If the passed in driver name does not match the driver name which is stored for the given volume name, an error is returned. |
|
| 322 |
+// If the passed in driver name does not match the driver name which is stored |
|
| 323 |
+// for the given volume name, an error is returned after checking if the reference is stale. |
|
| 324 |
+// If the reference is stale, it will be purged and this create can continue. |
|
| 257 | 325 |
// It is expected that callers of this function hold any necessary locks. |
| 258 | 326 |
func (s *VolumeStore) create(name, driverName string, opts, labels map[string]string) (volume.Volume, error) {
|
| 259 | 327 |
// Validate the name in a platform-specific manner |
| ... | ... |
@@ -265,10 +343,11 @@ func (s *VolumeStore) create(name, driverName string, opts, labels map[string]st |
| 265 | 265 |
return nil, &OpErr{Err: errInvalidName, Name: name, Op: "create"}
|
| 266 | 266 |
} |
| 267 | 267 |
|
| 268 |
- if v, exists := s.getNamed(name); exists {
|
|
| 269 |
- if v.DriverName() != driverName && driverName != "" && driverName != volume.DefaultDriverName {
|
|
| 270 |
- return nil, errNameConflict |
|
| 271 |
- } |
|
| 268 |
+ v, err := s.checkConflict(name, driverName) |
|
| 269 |
+ if err != nil {
|
|
| 270 |
+ return nil, err |
|
| 271 |
+ } |
|
| 272 |
+ if v != nil {
|
|
| 272 | 273 |
return v, nil |
| 273 | 274 |
} |
| 274 | 275 |
|
| ... | ... |
@@ -291,7 +370,7 @@ func (s *VolumeStore) create(name, driverName string, opts, labels map[string]st |
| 291 | 291 |
if v, _ := vd.Get(name); v != nil {
|
| 292 | 292 |
return v, nil |
| 293 | 293 |
} |
| 294 |
- v, err := vd.Create(name, opts) |
|
| 294 |
+ v, err = vd.Create(name, opts) |
|
| 295 | 295 |
if err != nil {
|
| 296 | 296 |
return nil, err |
| 297 | 297 |
} |
| ... | ... |
@@ -432,7 +511,8 @@ func (s *VolumeStore) Remove(v volume.Volume) error {
|
| 432 | 432 |
s.locks.Lock(name) |
| 433 | 433 |
defer s.locks.Unlock(name) |
| 434 | 434 |
|
| 435 |
- if refs, exists := s.refs[name]; exists && len(refs) > 0 {
|
|
| 435 |
+ refs := s.getRefs(name) |
|
| 436 |
+ if len(refs) > 0 {
|
|
| 436 | 437 |
return &OpErr{Err: errVolumeInUse, Name: v.Name(), Op: "remove", Refs: refs}
|
| 437 | 438 |
} |
| 438 | 439 |
|
| ... | ... |
@@ -473,13 +553,7 @@ func (s *VolumeStore) Refs(v volume.Volume) []string {
|
| 473 | 473 |
s.locks.Lock(v.Name()) |
| 474 | 474 |
defer s.locks.Unlock(v.Name()) |
| 475 | 475 |
|
| 476 |
- s.globalLock.Lock() |
|
| 477 |
- defer s.globalLock.Unlock() |
|
| 478 |
- refs, exists := s.refs[v.Name()] |
|
| 479 |
- if !exists {
|
|
| 480 |
- return nil |
|
| 481 |
- } |
|
| 482 |
- |
|
| 476 |
+ refs := s.getRefs(v.Name()) |
|
| 483 | 477 |
refsOut := make([]string, len(refs)) |
| 484 | 478 |
copy(refsOut, refs) |
| 485 | 479 |
return refsOut |
| ... | ... |
@@ -511,7 +585,7 @@ func (s *VolumeStore) FilterByDriver(name string) ([]volume.Volume, error) {
|
| 511 | 511 |
func (s *VolumeStore) FilterByUsed(vols []volume.Volume, used bool) []volume.Volume {
|
| 512 | 512 |
return s.filter(vols, func(v volume.Volume) bool {
|
| 513 | 513 |
s.locks.Lock(v.Name()) |
| 514 |
- l := len(s.refs[v.Name()]) |
|
| 514 |
+ l := len(s.getRefs(v.Name())) |
|
| 515 | 515 |
s.locks.Unlock(v.Name()) |
| 516 | 516 |
if (used && l > 0) || (!used && l == 0) {
|
| 517 | 517 |
return true |
| ... | ... |
@@ -270,7 +270,6 @@ func ParseMountSpec(cfg mounttypes.Mount, options ...func(*validateOpts)) (*Moun |
| 270 | 270 |
} |
| 271 | 271 |
mp.CopyData = DefaultCopyMode |
| 272 | 272 |
|
| 273 |
- mp.Driver = DefaultDriverName |
|
| 274 | 273 |
if cfg.VolumeOptions != nil {
|
| 275 | 274 |
if cfg.VolumeOptions.DriverConfig != nil {
|
| 276 | 275 |
mp.Driver = cfg.VolumeOptions.DriverConfig.Name |
| ... | ... |
@@ -163,8 +163,8 @@ func TestParseMountRawSplit(t *testing.T) {
|
| 163 | 163 |
{`name:d::rw`, "local", `d:`, ``, `name`, "local", true, false},
|
| 164 | 164 |
{`name:d:`, "local", `d:`, ``, `name`, "local", true, false},
|
| 165 | 165 |
// TODO Windows post TP5 - Add readonly support {`name:d::ro`, "local", `d:`, ``, `name`, "local", false, false},
|
| 166 |
- {`name:c:`, "", ``, ``, ``, DefaultDriverName, true, true},
|
|
| 167 |
- {`driver/name:c:`, "", ``, ``, ``, DefaultDriverName, true, true},
|
|
| 166 |
+ {`name:c:`, "", ``, ``, ``, "", true, true},
|
|
| 167 |
+ {`driver/name:c:`, "", ``, ``, ``, "", true, true},
|
|
| 168 | 168 |
} |
| 169 | 169 |
} else {
|
| 170 | 170 |
cases = []testParseMountRaw{
|
| ... | ... |
@@ -172,10 +172,10 @@ func TestParseMountRawSplit(t *testing.T) {
|
| 172 | 172 |
{"/tmp:/tmp2:ro", "", "/tmp2", "/tmp", "", "", false, false},
|
| 173 | 173 |
{"/tmp:/tmp3:rw", "", "/tmp3", "/tmp", "", "", true, false},
|
| 174 | 174 |
{"/tmp:/tmp4:foo", "", "", "", "", "", false, true},
|
| 175 |
- {"name:/named1", "", "/named1", "", "name", DefaultDriverName, true, false},
|
|
| 175 |
+ {"name:/named1", "", "/named1", "", "name", "", true, false},
|
|
| 176 | 176 |
{"name:/named2", "external", "/named2", "", "name", "external", true, false},
|
| 177 | 177 |
{"name:/named3:ro", "local", "/named3", "", "name", "local", false, false},
|
| 178 |
- {"local/name:/tmp:rw", "", "/tmp", "", "local/name", DefaultDriverName, true, false},
|
|
| 178 |
+ {"local/name:/tmp:rw", "", "/tmp", "", "local/name", "", true, false},
|
|
| 179 | 179 |
{"/tmp:tmp", "", "", "", "", "", true, true},
|
| 180 | 180 |
} |
| 181 | 181 |
} |
| ... | ... |
@@ -207,7 +207,7 @@ func TestParseMountRawSplit(t *testing.T) {
|
| 207 | 207 |
t.Fatalf("Expected name '%s', was '%s' for spec '%s'", c.expName, m.Name, c.bind)
|
| 208 | 208 |
} |
| 209 | 209 |
|
| 210 |
- if (m.Driver != c.expDriver) || (m.Driver == DefaultDriverName && c.expDriver == "") {
|
|
| 210 |
+ if m.Driver != c.expDriver {
|
|
| 211 | 211 |
t.Fatalf("Expected driver '%s', was '%s', for spec '%s'", c.expDriver, m.Driver, c.bind)
|
| 212 | 212 |
} |
| 213 | 213 |
|
| ... | ... |
@@ -233,8 +233,8 @@ func TestParseMountSpec(t *testing.T) {
|
| 233 | 233 |
{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath, RW: true}},
|
| 234 | 234 |
{mount.Mount{Type: mount.TypeBind, Source: testDir + string(os.PathSeparator), Target: testDestinationPath, ReadOnly: true}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath}},
|
| 235 | 235 |
{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath + string(os.PathSeparator), ReadOnly: true}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath}},
|
| 236 |
- {mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath}, MountPoint{Type: mount.TypeVolume, Destination: testDestinationPath, RW: true, Driver: DefaultDriverName, CopyData: DefaultCopyMode}},
|
|
| 237 |
- {mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath + string(os.PathSeparator)}, MountPoint{Type: mount.TypeVolume, Destination: testDestinationPath, RW: true, Driver: DefaultDriverName, CopyData: DefaultCopyMode}},
|
|
| 236 |
+ {mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath}, MountPoint{Type: mount.TypeVolume, Destination: testDestinationPath, RW: true, CopyData: DefaultCopyMode}},
|
|
| 237 |
+ {mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath + string(os.PathSeparator)}, MountPoint{Type: mount.TypeVolume, Destination: testDestinationPath, RW: true, CopyData: DefaultCopyMode}},
|
|
| 238 | 238 |
} |
| 239 | 239 |
|
| 240 | 240 |
for i, c := range cases {
|