Skip to content

Commit

Permalink
Switch to pre-0.9 behaviour for handling volumes
Browse files Browse the repository at this point in the history
In Nomad 0.9, we made volume driver handling the same for `""`, and
`"local"` volumes. Prior to Nomad 0.9 however these had slightly different
behaviour for relative paths and named volumes.

Prior to 0.9 the empty string would expand relative paths within the task
dir, and `"local"` volumes that are not absolute paths would be treated
as docker named volumes.

This commit reverts to the previous behaviour as follows:

| Nomad Version | Driver  |   Volume Spec    | Behaviour                 |
|-------------------------------------------------------------------------
| all           | ""      | testing:/testing | allocdir/testing          |
| 0.8.7         | "local" | testing:/testing | "testing" as named volume |
| 0.9.0         | "local" | testing:/testing | allocdir/testing          |
| 0.9.1         | "local" | testing:/testing | "testing" as named volume |
  • Loading branch information
endocrimes committed Apr 17, 2019
1 parent c07b729 commit 161ed44
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 22 deletions.
15 changes: 9 additions & 6 deletions drivers/docker/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,9 +603,9 @@ func (d *Driver) containerBinds(task *drivers.TaskConfig, driverConfig *TaskConf
secretDirBind := fmt.Sprintf("%s:%s", task.TaskDir().SecretsDir, task.Env[taskenv.SecretsDir])
binds := []string{allocDirBind, taskLocalBind, secretDirBind}

localBindVolume := driverConfig.VolumeDriver == "" || driverConfig.VolumeDriver == "local"
taskLocalBindVolume := driverConfig.VolumeDriver == ""

if !d.config.Volumes.Enabled && !localBindVolume {
if !d.config.Volumes.Enabled && !taskLocalBindVolume {
return nil, fmt.Errorf("volumes are not enabled; cannot use volume driver %q", driverConfig.VolumeDriver)
}

Expand All @@ -618,12 +618,15 @@ func (d *Driver) containerBinds(task *drivers.TaskConfig, driverConfig *TaskConf
// Paths inside task dir are always allowed, Relative paths are always allowed as they mount within a container
// When a VolumeDriver is set, we assume we receive a binding in the format volume-name:container-dest
// Otherwise, we assume we receive a relative path binding in the format relative/to/task:/also/in/container
if localBindVolume {
if taskLocalBindVolume {
parts[0] = expandPath(task.TaskDir().Dir, parts[0])
} else {
// Resolve dotted path segments
parts[0] = filepath.Clean(parts[0])
}

if !d.config.Volumes.Enabled && !isParentPath(task.AllocDir, parts[0]) {
return nil, fmt.Errorf("volumes are not enabled; cannot mount host paths: %+q", userbind)
}
if !d.config.Volumes.Enabled && !isParentPath(task.AllocDir, parts[0]) {
return nil, fmt.Errorf("volumes are not enabled; cannot mount host paths: %+q", userbind)
}

binds = append(binds, strings.Join(parts, ":"))
Expand Down
18 changes: 2 additions & 16 deletions drivers/docker/driver_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,10 @@ func TestDockerDriver_BindMountsHonorVolumesEnabledFlag(t *testing.T) {
},
{
name: "relative local driver",
requiresVolumes: false,
requiresVolumes: true,
volumeDriver: "local",
volumes: []string{"test-path:/tmp/taskpath"},
expectedVolumes: []string{"/tmp/nomad/alloc-dir/demo/test-path:/tmp/taskpath"},
expectedVolumes: []string{"test-path:/tmp/taskpath"},
},
{
name: "relative outside task-dir default driver",
Expand All @@ -311,27 +311,13 @@ func TestDockerDriver_BindMountsHonorVolumesEnabledFlag(t *testing.T) {
volumes: []string{"../test-path:/tmp/taskpath"},
expectedVolumes: []string{"/tmp/nomad/alloc-dir/test-path:/tmp/taskpath"},
},
{
name: "relative outside task-dir local driver",
requiresVolumes: false,
volumeDriver: "local",
volumes: []string{"../test-path:/tmp/taskpath"},
expectedVolumes: []string{"/tmp/nomad/alloc-dir/test-path:/tmp/taskpath"},
},
{
name: "relative outside alloc-dir default driver",
requiresVolumes: true,
volumeDriver: "",
volumes: []string{"../../test-path:/tmp/taskpath"},
expectedVolumes: []string{"/tmp/nomad/test-path:/tmp/taskpath"},
},
{
name: "relative outside task-dir local driver",
requiresVolumes: true,
volumeDriver: "local",
volumes: []string{"../../test-path:/tmp/taskpath"},
expectedVolumes: []string{"/tmp/nomad/test-path:/tmp/taskpath"},
},
}

t.Run("with volumes enabled", func(t *testing.T) {
Expand Down

0 comments on commit 161ed44

Please sign in to comment.