fix Bind-mounts not properly registering after daemon restart #10147

cpuguy83 · 2015-01-16T22:15:18Z

Volume config is not restored if we couldn't find it with the graph
driver, but bind-mounts would never be found by the graph driver since
they aren't in that dir
container volumes were only being restored if they were found in the
volumes repo, but volumes created by old daemons wouldn't be in the
repo until the container is at least started.

crosbymichael · 2015-01-20T01:05:51Z

daemon/volumes.go

+			v.AddContainer(container.ID)
+		} else {
+			// if container was created with an old daemon, this volume may not be registered
+			v, err := container.daemon.volumes.FindOrCreateVolume(path, container.VolumesRW[path])


What if the volume is not a rw volume in VolumesRW?

Ahh, from what I can tell this is only used for very old daemons

It would be pre 1.3 daemons, so not that old.

I'll update this VolumesRW thing to make sure we don't panic here. It shouldn't be nil, but it could be.

cpuguy83 · 2015-01-20T02:27:34Z

@crosbymichael Updated

LK4D4 · 2015-01-20T20:39:17Z

@cpuguy83 Is it possible to write test for this? We have test-daemon for restarting.

cpuguy83 · 2015-01-20T20:40:16Z

@LK4D4 There is a test there.

LK4D4 · 2015-01-20T20:42:08Z

@cpuguy83 Yup, sorry :)

LK4D4 · 2015-01-20T20:47:27Z

daemon/volumes.go

+			v.AddContainer(container.ID)
+		} else {
+			// if container was created with an old daemon, this volume may not be registered
+			var mode = true


maybe mode := true? Also mode is pretty weird name, maybe writtable?

Fixes moby#9629 moby#9768 A couple of issues: 1) Volume config is not restored if we couldn't find it with the graph driver, but bind-mounts would never be found by the graph driver since they aren't in that dir 2) container volumes were only being restored if they were found in the volumes repo, but volumes created by old daemons wouldn't be in the repo until the container is at least started. Signed-off-by: Brian Goff <cpuguy83@gmail.com>

cpuguy83 · 2015-01-20T21:00:10Z

@LK4D4 Updated

LK4D4 · 2015-01-20T22:50:14Z

LGTM

crosbymichael · 2015-01-20T23:37:53Z

LGTM

jessfraz · 2015-01-20T23:37:53Z

LGTM

…d-container fix Bind-mounts not properly registering after daemon restart

grossws · 2015-01-20T23:55:47Z

Does it fix #9801?

jessfraz · 2015-01-20T23:58:06Z

thanks @grossws I have closed that issue.

HuKeping · 2015-01-21T12:54:19Z

It will cause a panic with this patch.

cpuguy83 · 2015-01-21T13:19:58Z

How so?

HuKeping · 2015-01-22T01:06:17Z

hi @cpuguy83 , when u restart the daemon without -g to specify another new storage path, u would like to get this:

goroutine 8 [running]:
runtime.panic(0xb22d80, 0x164e5e8)
    /usr/local/go/src/pkg/runtime/panic.c:266 +0xb6
github.com/docker/docker/volumes.(*Volume).createIfNotExist(0xc2101b16e0, 0x0, 0x27)
    /go/src/github.com/docker/docker/volumes/volume.go:102 +0x2b2
github.com/docker/docker/volumes.(*Volume).initialize(0xc2101b16e0, 0x0, 0x0)
    /go/src/github.com/docker/docker/volumes/volume.go:130 +0x83
github.com/docker/docker/volumes.(*Repository).restore(0xc210245810, 0x0, 0xc210245840)
    /go/src/github.com/docker/docker/volumes/repository.go:110 +0x6fe
github.com/docker/docker/volumes.NewRepository(0xc21013d160, 0x17, 0x7f29b9c9c928, 0xc21017d0b0, 0x17, ...)
    /go/src/github.com/docker/docker/volumes/repository.go:39 +0x178
github.com/docker/docker/daemon.NewDaemonFromDirectory(0x1670b20, 0xc21008da50, 0x0, 0x47, 0xb9)
    /go/src/github.com/docker/docker/daemon/daemon.go:892 +0xed6
github.com/docker/docker/daemon.NewDaemon(0x1670b20, 0xc21008da50, 0x0, 0x2, 0x0)
    /go/src/github.com/docker/docker/daemon/daemon.go:779 +0x31
main.func·001()
    /go/src/github.com/docker/docker/docker/daemon.go:55 +0x43
created by main.mainDaemon
    /go/src/github.com/docker/docker/docker/daemon.go:78 +0x290

goroutine 1 [chan receive]:
github.com/docker/docker/api/server.ServeApi(0xc210047880, 0x7f29b9c91040)
    /go/src/github.com/docker/docker/api/server/server.go:1639 +0x440
github.com/docker/docker/engine.(*Job).Run(0xc210047880, 0x0, 0x0)
    /go/src/github.com/docker/docker/engine/job.go:83 +0x76e
main.mainDaemon()
    /go/src/github.com/docker/docker/docker/daemon.go:93 +0x514
main.main()
goroutine 3 [syscall]:
os/signal.loop()
    /usr/local/go/src/pkg/os/signal/signal_unix.go:21 +0x1e
created by os/signal.init·1
    /usr/local/go/src/pkg/os/signal/signal_unix.go:27 +0x31

goroutine 4 [syscall]:
runtime.goexit()
    /usr/local/go/src/pkg/runtime/proc.c:1394

goroutine 10 [runnable]:
runtime.gosched()
    /usr/local/go/src/pkg/runtime/proc.c:1368 +0x27
runtime.gc(0x1)
    /usr/local/go/src/pkg/runtime/mgc0.c:2047 +0x27c
runfinq()
    /usr/local/go/src/pkg/runtime/mgc0.c:2325 +0x1f2
runtime.goexit()
    /usr/local/go/src/pkg/runtime/proc.c:1394

goroutine 7 [chan receive]:
github.com/docker/docker/pkg/signal.func·002()
    /go/src/github.com/docker/docker/pkg/signal/trap.go:30 +0x6c
created by github.com/docker/docker/pkg/signal.Trap
    /go/src/github.com/docker/docker/pkg/signal/trap.go:53 +0x26c

goroutine 9 [chan receive]:
github.com/docker/docker/pkg/listenbuffer.(*defaultListener).Accept(0xc210185720, 0x0, 0x1000000a349c0, 0xc210185dc0, 0x2100efd70)
    /go/src/github.com/docker/docker/pkg/listenbuffer/buffer.go:43 +0x7c
net/http.(*Server).Serve(0xc210265dc0, 0x7f29b9c9c7e0, 0xc210185720, 0x0, 0x0)
    /usr/local/go/src/pkg/net/http/server.go:1622 +0x91
github.com/docker/docker/api/server.(*HttpServer).Serve(0xc210185dc0, 0x4, 0xc2100b5967)
    /go/src/github.com/docker/docker/api/server/server.go:51 +0x3a
github.com/docker/docker/api/server.func·006()
    /go/src/github.com/docker/docker/api/server/server.go:1634 +0x206
created by github.com/docker/docker/api/server.ServeApi
    /go/src/github.com/docker/docker/api/server/server.go:1635 +0x3f1

And i think this was caused by nil pointer dereference in volumes/volume.go function createIfNotExist(), since the Path has already been removed by the patch, the nil.xxx() will cause a panic.

cpuguy83 · 2015-01-22T01:07:48Z

@HuKeping Hi, thanks for the report! This got resolved in #10243

HuKeping · 2015-01-22T01:14:56Z

OK, so fast........

cpuguy83 changed the title ~~9629 volumes from unregistered container~~ Bind-mounts not properly registered after daemon restart Jan 17, 2015

cpuguy83 changed the title ~~Bind-mounts not properly registered after daemon restart~~ fix Bind-mounts not properly registering after daemon restart Jan 17, 2015

cpuguy83 force-pushed the 9629_volumes-from_unregistered-container branch from 477b6f9 to 16f9a04 Compare January 17, 2015 16:22

tiborvass added this to the 1.5.0 milestone Jan 19, 2015

cpuguy83 force-pushed the 9629_volumes-from_unregistered-container branch from 16f9a04 to a3de790 Compare January 19, 2015 17:51

crosbymichael reviewed Jan 20, 2015
View reviewed changes

cpuguy83 force-pushed the 9629_volumes-from_unregistered-container branch from a3de790 to 7b2752b Compare January 20, 2015 02:27

LK4D4 reviewed Jan 20, 2015
View reviewed changes

cpuguy83 force-pushed the 9629_volumes-from_unregistered-container branch from 7b2752b to c837416 Compare January 20, 2015 20:54

cpuguy83 force-pushed the 9629_volumes-from_unregistered-container branch from c837416 to e744b0d Compare January 20, 2015 20:55

crosbymichael added a commit that referenced this pull request Jan 20, 2015

Merge pull request #10147 from cpuguy83/9629_volumes-from_unregistere…

cc66fa3

…d-container fix Bind-mounts not properly registering after daemon restart

crosbymichael merged commit cc66fa3 into moby:master Jan 20, 2015

cpuguy83 mentioned this pull request Jan 20, 2015

Docker creates new volume instead of use one from volumes-from after upgrading from 1.2.0 to 1.3.2 #9768

Closed

jessfraz mentioned this pull request Jan 20, 2015

Docker bind mount is made on old partition instead of new mounted on the host after docker daemon started #9801

Closed

SvenDowideit mentioned this pull request Jan 27, 2015

Post 1.4.1 docs update 2 #9739

Merged

cpuguy83 deleted the 9629_volumes-from_unregistered-container branch September 20, 2017 16:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix Bind-mounts not properly registering after daemon restart #10147

fix Bind-mounts not properly registering after daemon restart #10147

cpuguy83 commented Jan 16, 2015

crosbymichael Jan 20, 2015

crosbymichael Jan 20, 2015

cpuguy83 Jan 20, 2015

cpuguy83 commented Jan 20, 2015

LK4D4 commented Jan 20, 2015

cpuguy83 commented Jan 20, 2015

LK4D4 commented Jan 20, 2015

LK4D4 Jan 20, 2015

cpuguy83 commented Jan 20, 2015

LK4D4 commented Jan 20, 2015

crosbymichael commented Jan 20, 2015

jessfraz commented Jan 20, 2015

grossws commented Jan 20, 2015

jessfraz commented Jan 20, 2015

HuKeping commented Jan 21, 2015

cpuguy83 commented Jan 21, 2015

HuKeping commented Jan 22, 2015

cpuguy83 commented Jan 22, 2015

HuKeping commented Jan 22, 2015

fix Bind-mounts not properly registering after daemon restart #10147

fix Bind-mounts not properly registering after daemon restart #10147

Conversation

cpuguy83 commented Jan 16, 2015

crosbymichael Jan 20, 2015

Choose a reason for hiding this comment

crosbymichael Jan 20, 2015

Choose a reason for hiding this comment

cpuguy83 Jan 20, 2015

Choose a reason for hiding this comment

cpuguy83 commented Jan 20, 2015

LK4D4 commented Jan 20, 2015

cpuguy83 commented Jan 20, 2015

LK4D4 commented Jan 20, 2015

LK4D4 Jan 20, 2015

Choose a reason for hiding this comment

cpuguy83 commented Jan 20, 2015

LK4D4 commented Jan 20, 2015

crosbymichael commented Jan 20, 2015

jessfraz commented Jan 20, 2015

grossws commented Jan 20, 2015

jessfraz commented Jan 20, 2015

HuKeping commented Jan 21, 2015

cpuguy83 commented Jan 21, 2015

HuKeping commented Jan 22, 2015

cpuguy83 commented Jan 22, 2015

HuKeping commented Jan 22, 2015