From 8d7be4545220d2e084bd5f5440980b8c4e67107b Mon Sep 17 00:00:00 2001 From: Janet Kuo Date: Tue, 2 Aug 2016 17:00:54 -0700 Subject: [PATCH 1/2] Convert volumes in [name:][host:]container[:access_mode] format --- cli/app/app.go | 82 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 24 deletions(-) diff --git a/cli/app/app.go b/cli/app/app.go index c7471546..466cc461 100644 --- a/cli/app/app.go +++ b/cli/app/app.go @@ -509,36 +509,70 @@ func configEnvs(name string, service ServiceConfig) []api.EnvVar { func configVolumes(service ServiceConfig) ([]api.VolumeMount, []api.Volume) { volumesMount := []api.VolumeMount{} volumes := []api.Volume{} + volumeSource := api.VolumeSource{} for _, volume := range service.Volumes { - character := ":" - if strings.Contains(volume, character) { - containerDir := volume[strings.Index(volume, character)+1:] - containerDir = strings.TrimSpace(containerDir) - - // check if ro/rw mode is defined, default rw - readonly := false - if strings.Index(volume, character) != strings.LastIndex(volume, character) { - mode := volume[strings.LastIndex(volume, character)+1:] - if strings.Compare(mode, "ro") == 0 { - readonly = true - } - containerDir = containerDir[0:strings.Index(containerDir, character)] - } - - // volumeName = random string of 20 chars - volumeName := RandStringBytes(20) - - volumesMount = append(volumesMount, api.VolumeMount{Name: volumeName, ReadOnly: readonly, MountPath: containerDir}) - - emptyDir := &api.EmptyDirVolumeSource{} - volumeSource := api.VolumeSource{EmptyDir: emptyDir} - - volumes = append(volumes, api.Volume{Name: volumeName, VolumeSource: volumeSource}) + name, host, container, mode, err := parseVolume(volume) + if err != nil { + logrus.Warningf("Failed to configure container volume: %v", err) + continue } + + // if volume name isn't specified, set it to a random string of 20 chars + if len(name) == 0 { + name = RandStringBytes(20) + } + // check if ro/rw mode is defined, default rw + readonly := len(mode) > 0 && mode == "ro" + + volumesMount = append(volumesMount, api.VolumeMount{Name: name, ReadOnly: readonly, MountPath: container}) + + if len(host) > 0 { + volumeSource = api.VolumeSource{HostPath: &api.HostPathVolumeSource{Path: host}} + } else { + volumeSource = api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}} + } + + volumes = append(volumes, api.Volume{Name: name, VolumeSource: volumeSource}) } return volumesMount, volumes } +// parseVolume parse a given volume, which might be [name:][host:]container[:access_mode] +func parseVolume(volume string) (name, host, container, mode string, err error) { + separator := ":" + volumeStrings := strings.Split(volume, separator) + if len(volumeStrings) == 0 { + return + } + // Set name if existed + if !isPath(volumeStrings[0]) { + name = volumeStrings[0] + volumeStrings = volumeStrings[1:] + } + if len(volumeStrings) == 0 { + err = fmt.Errorf("invalid volume format: %s", volume) + return + } + if volumeStrings[len(volumeStrings)-1] == "rw" || volumeStrings[len(volumeStrings)-1] == "ro" { + mode = volumeStrings[len(volumeStrings)-1] + volumeStrings = volumeStrings[:len(volumeStrings)-1] + } + container = volumeStrings[len(volumeStrings)-1] + volumeStrings = volumeStrings[:len(volumeStrings)-1] + if len(volumeStrings) == 1 { + host = volumeStrings[0] + } + if !isPath(container) || (len(host) > 0 && !isPath(host)) || len(volumeStrings) > 1 { + err = fmt.Errorf("invalid volume format: %s", volume) + return + } + return +} + +func isPath(substring string) bool { + return strings.Contains(substring, "/") +} + // Configure the container ports. func configPorts(name string, service ServiceConfig) []api.ContainerPort { ports := []api.ContainerPort{} From eab6d004f8c993059355c565ef45fe6ac1ff2654 Mon Sep 17 00:00:00 2001 From: Janet Kuo Date: Wed, 3 Aug 2016 13:37:51 -0700 Subject: [PATCH 2/2] Add an unit test for parseVolume --- cli/app/app_test.go | 119 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 cli/app/app_test.go diff --git a/cli/app/app_test.go b/cli/app/app_test.go new file mode 100644 index 00000000..b02d9b81 --- /dev/null +++ b/cli/app/app_test.go @@ -0,0 +1,119 @@ +/* +Copyright 2016 Skippbox, Ltd All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package app + +import ( + "fmt" + "testing" +) + +func TestParseVolume(t *testing.T) { + name1 := "datavolume" + host1 := "./cache" + host2 := "~/configs" + container1 := "/tmp/cache" + container2 := "/etc/configs/" + mode := "rw" + + tests := []struct { + test, volume, name, host, container, mode string + }{ + { + "name:host:container:mode", + fmt.Sprintf("%s:%s:%s:%s", name1, host1, container1, mode), + name1, + host1, + container1, + mode, + }, + { + "host:container:mode", + fmt.Sprintf("%s:%s:%s", host2, container2, mode), + "", + host2, + container2, + mode, + }, + { + "name:container:mode", + fmt.Sprintf("%s:%s:%s", name1, container1, mode), + name1, + "", + container1, + mode, + }, + { + "name:host:container", + fmt.Sprintf("%s:%s:%s", name1, host1, container1), + name1, + host1, + container1, + "", + }, + { + "host:container", + fmt.Sprintf("%s:%s", host1, container1), + "", + host1, + container1, + "", + }, + { + "container:mode", + fmt.Sprintf("%s:%s", container2, mode), + "", + "", + container2, + mode, + }, + { + "name:container", + fmt.Sprintf("%s:%s", name1, container1), + name1, + "", + container1, + "", + }, + { + "container", + fmt.Sprintf("%s", container2), + "", + "", + container2, + "", + }, + } + + for _, test := range tests { + name, host, container, mode, err := parseVolume(test.volume) + if err != nil { + t.Errorf("In test case %q, returned unexpected error %v", test.test, err) + } + if name != test.name { + t.Errorf("In test case %q, returned volume name %s, expected %s", name, test.name) + } + if host != test.host { + t.Errorf("In test case %q, returned host path %s, expected %s", host, test.host) + } + if container != test.container { + t.Errorf("In test case %q, returned container path %s, expected %s", container, test.container) + } + if mode != test.mode { + t.Errorf("In test case %q, returned access mode %s, expected %s", mode, test.mode) + } + } +}