Skip to content

Commit

Permalink
Parse Docker mounts correctly (#3163)
Browse files Browse the repository at this point in the history 
* Parse Docker mounts correctly

This PR fixes the parsing of Docker mounts and adds testing to ensure no
regressions.

Fixes #3156

* Review feedback
  • Loading branch information
@dadgar
dadgar committed Sep 5, 2017
1 parent 949df54 commit b9f51ce
Show file tree
Hide file tree
Showing 2 changed files with 237 additions and 27 deletions.
84 changes: 57 additions & 27 deletions client/driver/docker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,22 +136,22 @@ type DockerLoggingOpts struct {
}

type DockerMount struct {
Target string `mapstructure:"target"`
Source string `mapstructure:"source"`
ReadOnly bool `mapstructure:"readonly"`
VolumeOptions *DockerVolumeOptions `mapstructure:"volume_options"`
Target string `mapstructure:"target"`
Source string `mapstructure:"source"`
ReadOnly bool `mapstructure:"readonly"`
VolumeOptions []*DockerVolumeOptions `mapstructure:"volume_options"`
}

type DockerVolumeOptions struct {
NoCopy bool `mapstructure:"no_copy"`
Labels map[string]string `mapstructure:"labels"`
DriverConfig DockerVolumeDriverConfig `mapstructure:"driver_config"`
NoCopy bool `mapstructure:"no_copy"`
Labels []map[string]string `mapstructure:"labels"`
DriverConfig []DockerVolumeDriverConfig `mapstructure:"driver_config"`
}

// VolumeDriverConfig holds a map of volume driver specific options
type DockerVolumeDriverConfig struct {
Name string `mapstructure:"name"`
Options map[string]string `mapstructure:"options"`
Name string `mapstructure:"name"`
Options []map[string]string `mapstructure:"options"`
}

type DockerDriverConfig struct {
Expand Down Expand Up @@ -259,22 +259,43 @@ func NewDockerDriverConfig(task *structs.Task, env *env.TaskEnv) (*DockerDriverC
for i, m := range dconf.Mounts {
dconf.Mounts[i].Target = env.ReplaceEnv(m.Target)
dconf.Mounts[i].Source = env.ReplaceEnv(m.Source)
if m.VolumeOptions != nil {
if m.VolumeOptions.Labels != nil {
for k, v := range m.VolumeOptions.Labels {

if len(m.VolumeOptions) > 1 {
return nil, fmt.Errorf("Only one volume_options stanza allowed")
}

if len(m.VolumeOptions) == 1 {
vo := m.VolumeOptions[0]
if len(vo.Labels) > 1 {
return nil, fmt.Errorf("labels may only be specified once in volume_options stanza")
}

if len(vo.Labels) == 1 {
for k, v := range vo.Labels[0] {
if k != env.ReplaceEnv(k) {
delete(dconf.Mounts[i].VolumeOptions.Labels, k)
delete(vo.Labels[0], k)
}
dconf.Mounts[i].VolumeOptions.Labels[env.ReplaceEnv(k)] = env.ReplaceEnv(v)
vo.Labels[0][env.ReplaceEnv(k)] = env.ReplaceEnv(v)
}
}
dconf.Mounts[i].VolumeOptions.DriverConfig.Name = env.ReplaceEnv(m.VolumeOptions.DriverConfig.Name)
if m.VolumeOptions.DriverConfig.Options != nil {
for k, v := range m.VolumeOptions.DriverConfig.Options {
if k != env.ReplaceEnv(k) {
delete(dconf.Mounts[i].VolumeOptions.DriverConfig.Options, k)

if len(vo.DriverConfig) > 1 {
return nil, fmt.Errorf("volume driver config may only be specified once")
}
if len(vo.DriverConfig) == 1 {
vo.DriverConfig[0].Name = env.ReplaceEnv(vo.DriverConfig[0].Name)
if len(vo.DriverConfig[0].Options) > 1 {
return nil, fmt.Errorf("volume driver options may only be specified once")
}

if len(vo.DriverConfig[0].Options) == 1 {
options := vo.DriverConfig[0].Options[0]
for k, v := range options {
if k != env.ReplaceEnv(k) {
delete(options, k)
}
options[env.ReplaceEnv(k)] = env.ReplaceEnv(v)
}
dconf.Mounts[i].VolumeOptions.DriverConfig.Options[env.ReplaceEnv(k)] = env.ReplaceEnv(v)
}
}
}
Expand Down Expand Up @@ -995,14 +1016,23 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas
Type: "volume", // Only type supported
ReadOnly: m.ReadOnly,
}
if m.VolumeOptions != nil {
if len(m.VolumeOptions) == 1 {
vo := m.VolumeOptions[0]
hm.VolumeOptions = &docker.VolumeOptions{
NoCopy: m.VolumeOptions.NoCopy,
Labels: m.VolumeOptions.Labels,
DriverConfig: docker.VolumeDriverConfig{
Name: m.VolumeOptions.DriverConfig.Name,
Options: m.VolumeOptions.DriverConfig.Options,
},
NoCopy: vo.NoCopy,
}

if len(vo.DriverConfig) == 1 {
dc := vo.DriverConfig[0]
hm.VolumeOptions.DriverConfig = docker.VolumeDriverConfig{
Name: dc.Name,
}
if len(dc.Options) == 1 {
hm.VolumeOptions.DriverConfig.Options = dc.Options[0]
}
}
if len(vo.Labels) == 1 {
hm.VolumeOptions.Labels = vo.Labels[0]
}
}
hostConfig.Mounts = append(hostConfig.Mounts, hm)
Expand Down
180 changes: 180 additions & 0 deletions client/driver/docker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1376,6 +1376,186 @@ func TestDockerDriver_VolumesEnabled(t *testing.T) {
}
}

func TestDockerDriver_Mounts(t *testing.T) {
if !tu.IsTravis() {
t.Parallel()
}
if !testutil.DockerIsConnected(t) {
t.SkipNow()
}

goodMount := map[string]interface{}{
"target": "/nomad",
"volume_options": []interface{}{
map[string]interface{}{
"labels": []interface{}{
map[string]string{"foo": "bar"},
},
"driver_config": []interface{}{
map[string]interface{}{
"name": "local",
"options": []interface{}{
map[string]interface{}{
"foo": "bar",
},
},
},
},
},
},
"readonly": true,
"source": "test",
}

cases := []struct {
Name string
Mounts []interface{}
Error string
}{
{
Name: "good-one",
Error: "",
Mounts: []interface{}{goodMount},
},
{
Name: "good-many",
Error: "",
Mounts: []interface{}{goodMount, goodMount, goodMount},
},
{
Name: "multiple volume options",
Error: "Only one volume_options stanza allowed",
Mounts: []interface{}{
map[string]interface{}{
"target": "/nomad",
"volume_options": []interface{}{
map[string]interface{}{
"driver_config": []interface{}{
map[string]interface{}{
"name": "local",
},
},
},
map[string]interface{}{
"driver_config": []interface{}{
map[string]interface{}{
"name": "local",
},
},
},
},
},
},
},
{
Name: "multiple driver configs",
Error: "volume driver config may only be specified once",
Mounts: []interface{}{
map[string]interface{}{
"target": "/nomad",
"volume_options": []interface{}{
map[string]interface{}{
"driver_config": []interface{}{
map[string]interface{}{
"name": "local",
},
map[string]interface{}{
"name": "local",
},
},
},
},
},
},
},
{
Name: "multiple volume labels",
Error: "labels may only be",
Mounts: []interface{}{
map[string]interface{}{
"target": "/nomad",
"volume_options": []interface{}{
map[string]interface{}{
"labels": []interface{}{
map[string]string{"foo": "bar"},
map[string]string{"baz": "bam"},
},
},
},
},
},
},
{
Name: "multiple driver options",
Error: "driver options may only",
Mounts: []interface{}{
map[string]interface{}{
"target": "/nomad",
"volume_options": []interface{}{
map[string]interface{}{
"driver_config": []interface{}{
map[string]interface{}{
"name": "local",
"options": []interface{}{
map[string]interface{}{
"foo": "bar",
},
map[string]interface{}{
"bam": "bar",
},
},
},
},
},
},
},
},
},
}

task := &structs.Task{
Name: "redis-demo",
Driver: "docker",
Config: map[string]interface{}{
"image": "busybox",
"load": "busybox.tar",
"command": "/bin/sleep",
"args": []string{"10000"},
},
Resources: &structs.Resources{
MemoryMB: 256,
CPU: 512,
},
LogConfig: &structs.LogConfig{
MaxFiles: 10,
MaxFileSizeMB: 10,
},
}

for _, c := range cases {
t.Run(c.Name, func(t *testing.T) {
// Build the task
task.Config["mounts"] = c.Mounts

ctx := testDockerDriverContexts(t, task)
driver := NewDockerDriver(ctx.DriverCtx)
copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar")
defer ctx.AllocDir.Destroy()

_, err := driver.Prestart(ctx.ExecCtx, task)
if err == nil && c.Error != "" {
t.Fatalf("expected error: %v", c.Error)
} else if err != nil {
if c.Error == "" {
t.Fatalf("unexpected error in prestart: %v", err)
} else if !strings.Contains(err.Error(), c.Error) {
t.Fatalf("expected error %q; got %v", c.Error, err)
}
}
})
}
}

// TestDockerDriver_Cleanup ensures Cleanup removes only downloaded images.
func TestDockerDriver_Cleanup(t *testing.T) {
if !tu.IsTravis() {
Expand Down

0 comments on commit b9f51ce

Please sign in to comment.