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 18, 2019
1 parent 15c6487 commit ccce364
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 22 deletions.
24 changes: 15 additions & 9 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 @@ -615,15 +615,21 @@ func (d *Driver) containerBinds(task *drivers.TaskConfig, driverConfig *TaskConf
return nil, fmt.Errorf("invalid docker volume: %q", userbind)
}

// 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 {
// Paths inside task dir are always allowed when using the default driver,
// 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 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
19 changes: 6 additions & 13 deletions drivers/docker/driver_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,11 @@ func TestDockerDriver_BindMountsHonorVolumesEnabledFlag(t *testing.T) {
expectedVolumes: []string{"/tmp/nomad/alloc-dir/demo/test-path:/tmp/taskpath"},
},
{
name: "relative local driver",
requiresVolumes: false,
name: "named volume local driver",
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,13 +311,6 @@ 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,
Expand All @@ -326,11 +319,11 @@ func TestDockerDriver_BindMountsHonorVolumesEnabledFlag(t *testing.T) {
expectedVolumes: []string{"/tmp/nomad/test-path:/tmp/taskpath"},
},
{
name: "relative outside task-dir local driver",
name: "clean path local driver",
requiresVolumes: true,
volumeDriver: "local",
volumes: []string{"../../test-path:/tmp/taskpath"},
expectedVolumes: []string{"/tmp/nomad/test-path:/tmp/taskpath"},
volumes: []string{"/tmp/nomad/../test-path:/tmp/taskpath"},
expectedVolumes: []string{"/tmp/test-path:/tmp/taskpath"},
},
}

Expand Down

0 comments on commit ccce364

Please sign in to comment.