Skip to content

Commit

Permalink
Parse Docker mounts correctly
Browse files Browse the repository at this point in the history 
This PR fixes the parsing of Docker mounts and adds testing to ensure no
regressions.

Fixes #3156
  • Loading branch information
@dadgar
dadgar committed Sep 5, 2017
1 parent 2111a84 commit 918fe70
Show file tree
Hide file tree
Showing 2 changed files with 235 additions and 27 deletions.
82 changes: 55 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,21 @@ 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 {
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: m.VolumeOptions[0].NoCopy,
}

if len(m.VolumeOptions[0].DriverConfig) == 1 {
hm.VolumeOptions.DriverConfig = docker.VolumeDriverConfig{
Name: m.VolumeOptions[0].DriverConfig[0].Name,
}
if len(m.VolumeOptions[0].DriverConfig[0].Options) == 1 {
hm.VolumeOptions.DriverConfig.Options = m.VolumeOptions[0].DriverConfig[0].Options[0]
}
}
if len(m.VolumeOptions[0].Labels) == 1 {
hm.VolumeOptions.Labels = m.VolumeOptions[0].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 918fe70

Please sign in to comment.