From 69cd6761bb6c64246b8b2db6287ee102ab483658 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 17 Aug 2016 20:36:17 -0400 Subject: [PATCH] Fix volume not working after daemon restart When the daemon is started, it looks at all the volumes and checks to see if any of them have mount options persisted to disk, and loads them from disk if it does. In some cases a volume will be created with an empty map causing the options file to be persisted and volume options set to a non-nil value on daemon restart... this causes problems later when the driver checks for a non-nil value to determine if it should try and mount with the persisted volume options. Ensures 2 things: 1. Instead of only checking nilness for the opts map, use `len` to make sure it is not an empty map, which we don't really need to persit. 2. An empty (or nulled) opts.json will not inadvertnatly set volume options on daemon restart. Signed-off-by: Brian Goff Upstream-commit: 246d1eb58e807f2cf2d2b387e267dcfa228f96a8 Component: engine --- components/engine/volume/local/local.go | 10 ++- components/engine/volume/local/local_test.go | 64 ++++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/components/engine/volume/local/local.go b/components/engine/volume/local/local.go index 6b1c4f76e6..73039259f6 100644 --- a/components/engine/volume/local/local.go +++ b/components/engine/volume/local/local.go @@ -9,6 +9,7 @@ import ( "io/ioutil" "os" "path/filepath" + "reflect" "sync" "github.com/Sirupsen/logrus" @@ -90,10 +91,13 @@ func New(scope string, rootUID, rootGID int) (*Root, error) { r.volumes[name] = v optsFilePath := filepath.Join(rootDirectory, name, "opts.json") if b, err := ioutil.ReadFile(optsFilePath); err == nil { - v.opts = &optsConfig{} - if err := json.Unmarshal(b, v.opts); err != nil { + opts := optsConfig{} + if err := json.Unmarshal(b, &opts); err != nil { return nil, err } + if !reflect.DeepEqual(opts, optsConfig{}) { + v.opts = &opts + } // unmount anything that may still be mounted (for example, from an unclean shutdown) for _, info := range mountInfos { @@ -178,7 +182,7 @@ func (r *Root) Create(name string, opts map[string]string) (volume.Volume, error path: path, } - if opts != nil { + if len(opts) != 0 { if err = setOpts(v, opts); err != nil { return nil, err } diff --git a/components/engine/volume/local/local_test.go b/components/engine/volume/local/local_test.go index fd1af411d5..2ba42fe678 100644 --- a/components/engine/volume/local/local_test.go +++ b/components/engine/volume/local/local_test.go @@ -3,6 +3,7 @@ package local import ( "io/ioutil" "os" + "path/filepath" "reflect" "runtime" "strings" @@ -133,6 +134,11 @@ func TestCreate(t *testing.T) { } } } + + r, err = New(rootDir, 0, 0) + if err != nil { + t.Fatal(err) + } } func TestValidateName(t *testing.T) { @@ -263,3 +269,61 @@ func TestCreateWithOpts(t *testing.T) { t.Fatal("missing volume options on restart") } } + +func TestRealodNoOpts(t *testing.T) { + rootDir, err := ioutil.TempDir("", "volume-test-reload-no-opts") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(rootDir) + + r, err := New(rootDir, 0, 0) + if err != nil { + t.Fatal(err) + } + + if _, err := r.Create("test1", nil); err != nil { + t.Fatal(err) + } + if _, err := r.Create("test2", nil); err != nil { + t.Fatal(err) + } + // make sure a file with `null` (.e.g. empty opts map from older daemon) is ok + if err := ioutil.WriteFile(filepath.Join(rootDir, "test2"), []byte("null"), 600); err != nil { + t.Fatal(err) + } + + if _, err := r.Create("test3", nil); err != nil { + t.Fatal(err) + } + // make sure an empty opts file doesn't break us too + if err := ioutil.WriteFile(filepath.Join(rootDir, "test3"), nil, 600); err != nil { + t.Fatal(err) + } + + if _, err := r.Create("test4", map[string]string{}); err != nil { + t.Fatal(err) + } + + r, err = New(rootDir, 0, 0) + if err != nil { + t.Fatal(err) + } + + for _, name := range []string{"test1", "test2", "test3", "test4"} { + v, err := r.Get(name) + if err != nil { + t.Fatal(err) + } + lv, ok := v.(*localVolume) + if !ok { + t.Fatalf("expected *localVolume got: %v", reflect.TypeOf(v)) + } + if lv.opts != nil { + t.Fatalf("expected opts to be nil, got: %v", lv.opts) + } + if _, err := lv.Mount("1234"); err != nil { + t.Fatal(err) + } + } +}