luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Fix allocdir Move test and make code more defensive 

A change in the behavior of `os.Rename` in Go 1.8 brought to light a
difference in the logic between `{Alloc,Task}Runner` and this test:

AllocRunner builds the alloc dir, moves dirs if necessary, and then lets
TaskRunner call TaskDir.Build().

This test called `TaskDir.Build` *before* `AllocDir.Move`, so in Go 1.8
it failed to `os.Rename over` the empty {data,local} dirs.

I updated the test to behave like the real code, but I defensively added
`os.Remove` calls as a subtle change in call order shouldn't break this
code. `os.Remove` won't remove a non-empty directory, so it's still
safe.
This commit is contained in:
Michael Schurter 2017-02-21 17:22:10 -08:00
parent 8a6e3fb706
commit e5a29b82db
2 changed files with 5 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
client/allocdir/alloc_dir.go
View File

@ -161,6 +161,7 @@ func (d *AllocDir) Move(other *AllocDir, tasks []*structs.Task) error {
otherDataDir := filepath.Join(other.SharedDir, SharedDataDir)
dataDir := filepath.Join(d.SharedDir, SharedDataDir)
if fileInfo, err := os.Stat(otherDataDir); fileInfo != nil && err == nil {
os.Remove(dataDir) // remove an empty data dir if it exists
if err := os.Rename(otherDataDir, dataDir); err != nil {
return fmt.Errorf("error moving data dir: %v", err)
}
@ -179,6 +180,7 @@ func (d *AllocDir) Move(other *AllocDir, tasks []*structs.Task) error {
return fmt.Errorf("error creating task %q dir: %v", task.Name, err)
}
localDir := filepath.Join(newTaskDir, TaskLocal)
os.Remove(localDir) // remove an empty local dir if it exists
if err := os.Rename(otherTaskLocal, localDir); err != nil {
return fmt.Errorf("error moving task %q local dir: %v", task.Name, err)
}

7
client/allocdir/alloc_dir_test.go
View File

@ -229,10 +229,9 @@ func TestAllocDir_Move(t *testing.T) {
t.Fatalf("TaskDir.Build() faild: %v", err)
}
td2 := d2.NewTaskDir(t1.Name)
if err := td2.Build(nil, cstructs.FSIsolationImage); err != nil {
t.Fatalf("TaskDir.Build() faild: %v", err)
}
// Create but don't build second task dir to mimic alloc/task runner
// behavior (AllocDir.Move() is called pre-TaskDir.Build).
d2.NewTaskDir(t1.Name)
dataDir := filepath.Join(d1.SharedDir, SharedDataDir)