Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Switch to pre-0.9 behaviour for handling volumes #5572

Merged
merged 1 commit into from
Apr 18, 2019
Merged

Switch to pre-0.9 behaviour for handling volumes #5572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 15 additions & 9 deletions drivers/docker/driver.go
View file
Open in desktop
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 {
endocrimes marked this conversation as resolved.
Show resolved Hide resolved
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])
endocrimes marked this conversation as resolved.
Show resolved Hide resolved
}

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
View file
Open in desktop
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