Browse code
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 <cpuguy83@gmail.com>
Brian Goff authored on 2016/08/18 09:36:17Showing 2 changed files
| ... | ... |
@@ -9,6 +9,7 @@ import ( |
| 9 | 9 |
"io/ioutil" |
| 10 | 10 |
"os" |
| 11 | 11 |
"path/filepath" |
| 12 |
+ "reflect" |
|
| 12 | 13 |
"sync" |
| 13 | 14 |
|
| 14 | 15 |
"github.com/Sirupsen/logrus" |
| ... | ... |
@@ -90,10 +91,13 @@ func New(scope string, rootUID, rootGID int) (*Root, error) {
|
| 90 | 90 |
r.volumes[name] = v |
| 91 | 91 |
optsFilePath := filepath.Join(rootDirectory, name, "opts.json") |
| 92 | 92 |
if b, err := ioutil.ReadFile(optsFilePath); err == nil {
|
| 93 |
- v.opts = &optsConfig{}
|
|
| 94 |
- if err := json.Unmarshal(b, v.opts); err != nil {
|
|
| 93 |
+ opts := optsConfig{}
|
|
| 94 |
+ if err := json.Unmarshal(b, &opts); err != nil {
|
|
| 95 | 95 |
return nil, err |
| 96 | 96 |
} |
| 97 |
+ if !reflect.DeepEqual(opts, optsConfig{}) {
|
|
| 98 |
+ v.opts = &opts |
|
| 99 |
+ } |
|
| 97 | 100 |
|
| 98 | 101 |
// unmount anything that may still be mounted (for example, from an unclean shutdown) |
| 99 | 102 |
for _, info := range mountInfos {
|
| ... | ... |
@@ -178,7 +182,7 @@ func (r *Root) Create(name string, opts map[string]string) (volume.Volume, error |
| 178 | 178 |
path: path, |
| 179 | 179 |
} |
| 180 | 180 |
|
| 181 |
- if opts != nil {
|
|
| 181 |
+ if len(opts) != 0 {
|
|
| 182 | 182 |
if err = setOpts(v, opts); err != nil {
|
| 183 | 183 |
return nil, err |
| 184 | 184 |
} |
| ... | ... |
@@ -3,6 +3,7 @@ package local |
| 3 | 3 |
import ( |
| 4 | 4 |
"io/ioutil" |
| 5 | 5 |
"os" |
| 6 |
+ "path/filepath" |
|
| 6 | 7 |
"reflect" |
| 7 | 8 |
"runtime" |
| 8 | 9 |
"strings" |
| ... | ... |
@@ -133,6 +134,11 @@ func TestCreate(t *testing.T) {
|
| 133 | 133 |
} |
| 134 | 134 |
} |
| 135 | 135 |
} |
| 136 |
+ |
|
| 137 |
+ r, err = New(rootDir, 0, 0) |
|
| 138 |
+ if err != nil {
|
|
| 139 |
+ t.Fatal(err) |
|
| 140 |
+ } |
|
| 136 | 141 |
} |
| 137 | 142 |
|
| 138 | 143 |
func TestValidateName(t *testing.T) {
|
| ... | ... |
@@ -263,3 +269,61 @@ func TestCreateWithOpts(t *testing.T) {
|
| 263 | 263 |
t.Fatal("missing volume options on restart")
|
| 264 | 264 |
} |
| 265 | 265 |
} |
| 266 |
+ |
|
| 267 |
+func TestRealodNoOpts(t *testing.T) {
|
|
| 268 |
+ rootDir, err := ioutil.TempDir("", "volume-test-reload-no-opts")
|
|
| 269 |
+ if err != nil {
|
|
| 270 |
+ t.Fatal(err) |
|
| 271 |
+ } |
|
| 272 |
+ defer os.RemoveAll(rootDir) |
|
| 273 |
+ |
|
| 274 |
+ r, err := New(rootDir, 0, 0) |
|
| 275 |
+ if err != nil {
|
|
| 276 |
+ t.Fatal(err) |
|
| 277 |
+ } |
|
| 278 |
+ |
|
| 279 |
+ if _, err := r.Create("test1", nil); err != nil {
|
|
| 280 |
+ t.Fatal(err) |
|
| 281 |
+ } |
|
| 282 |
+ if _, err := r.Create("test2", nil); err != nil {
|
|
| 283 |
+ t.Fatal(err) |
|
| 284 |
+ } |
|
| 285 |
+ // make sure a file with `null` (.e.g. empty opts map from older daemon) is ok |
|
| 286 |
+ if err := ioutil.WriteFile(filepath.Join(rootDir, "test2"), []byte("null"), 600); err != nil {
|
|
| 287 |
+ t.Fatal(err) |
|
| 288 |
+ } |
|
| 289 |
+ |
|
| 290 |
+ if _, err := r.Create("test3", nil); err != nil {
|
|
| 291 |
+ t.Fatal(err) |
|
| 292 |
+ } |
|
| 293 |
+ // make sure an empty opts file doesn't break us too |
|
| 294 |
+ if err := ioutil.WriteFile(filepath.Join(rootDir, "test3"), nil, 600); err != nil {
|
|
| 295 |
+ t.Fatal(err) |
|
| 296 |
+ } |
|
| 297 |
+ |
|
| 298 |
+ if _, err := r.Create("test4", map[string]string{}); err != nil {
|
|
| 299 |
+ t.Fatal(err) |
|
| 300 |
+ } |
|
| 301 |
+ |
|
| 302 |
+ r, err = New(rootDir, 0, 0) |
|
| 303 |
+ if err != nil {
|
|
| 304 |
+ t.Fatal(err) |
|
| 305 |
+ } |
|
| 306 |
+ |
|
| 307 |
+ for _, name := range []string{"test1", "test2", "test3", "test4"} {
|
|
| 308 |
+ v, err := r.Get(name) |
|
| 309 |
+ if err != nil {
|
|
| 310 |
+ t.Fatal(err) |
|
| 311 |
+ } |
|
| 312 |
+ lv, ok := v.(*localVolume) |
|
| 313 |
+ if !ok {
|
|
| 314 |
+ t.Fatalf("expected *localVolume got: %v", reflect.TypeOf(v))
|
|
| 315 |
+ } |
|
| 316 |
+ if lv.opts != nil {
|
|
| 317 |
+ t.Fatalf("expected opts to be nil, got: %v", lv.opts)
|
|
| 318 |
+ } |
|
| 319 |
+ if _, err := lv.Mount("1234"); err != nil {
|
|
| 320 |
+ t.Fatal(err) |
|
| 321 |
+ } |
|
| 322 |
+ } |
|
| 323 |
+} |