Merge pull request #138 from vdemeester/carry-pr-8

Carry #8 docker: support relative paths for volumes

Author: vdemeester

docker/container.go

@@ -302,7 +302,7 @@ func volumeBinds(volumes map[string]struct{}, container *dockerclient.Container)
 }
 
 func (c *Container) createContainer(imageName, oldContainer string) (*dockerclient.APIContainers, error) {
-	createOpts, err := ConvertToAPI(c.service.serviceConfig, c.name)
+	createOpts, err := ConvertToAPI(c.service, c.name)
 	if err != nil {
 		return nil, err
 	}

docker/convert.go

@@ -31,8 +31,8 @@ func isVolume(s string) bool {
 }
 
 // ConvertToAPI converts a service configuration to a docker API container configuration.
-func ConvertToAPI(c *project.ServiceConfig, name string) (*dockerclient.CreateContainerOptions, error) {
-	config, hostConfig, err := Convert(c)
+func ConvertToAPI(s *Service, name string) (*dockerclient.CreateContainerOptions, error) {
+	config, hostConfig, err := Convert(s.serviceConfig, s.context)
 	if err != nil {
 		return nil, err
 	}
@@ -45,12 +45,15 @@ func ConvertToAPI(c *project.ServiceConfig, name string) (*dockerclient.CreateCo
 	return &result, nil
 }
 
-func volumes(c *project.ServiceConfig) map[string]struct{} {
-	vs := Filter(c.Volumes, isVolume)
+func volumes(c *project.ServiceConfig, ctx *Context) map[string]struct{} {
+	volumes := make(map[string]struct{}, len(c.Volumes))
+	for k, v := range c.Volumes {
+		vol := ctx.ResourceLookup.ResolvePath(v, ctx.ComposeFile)
 
-	volumes := make(map[string]struct{}, len(vs))
-	for _, v := range vs {
-		volumes[v] = struct{}{}
+		c.Volumes[k] = vol
+		if isVolume(vol) {
+			volumes[vol] = struct{}{}
+		}
 	}
 	return volumes
 }
@@ -95,7 +98,7 @@ func ports(c *project.ServiceConfig) (map[dockerclient.Port]struct{}, map[docker
 }
 
 // Convert converts a service configuration to an docker API structures (Config and HostConfig)
-func Convert(c *project.ServiceConfig) (*dockerclient.Config, *dockerclient.HostConfig, error) {
+func Convert(c *project.ServiceConfig, ctx *Context) (*dockerclient.Config, *dockerclient.HostConfig, error) {
 	restartPolicy, err := restartPolicy(c)
 	if err != nil {
 		return nil, nil, err
@@ -125,7 +128,7 @@ func Convert(c *project.ServiceConfig) (*dockerclient.Config, *dockerclient.Host
 		OpenStdin:    c.StdinOpen,
 		WorkingDir:   c.WorkingDir,
 		VolumeDriver: c.VolumeDriver,
-		Volumes:      volumes(c),
+		Volumes:      volumes(c, ctx),
 	}
 	hostConfig := &dockerclient.HostConfig{
 		VolumesFrom: utils.CopySlice(c.VolumesFrom),

docker/convert_test.go

@@ -1,10 +1,13 @@
 package docker
 
 import (
+	"path/filepath"
+	"testing"
+
+	"github.com/docker/libcompose/lookup"
 	"github.com/docker/libcompose/project"
 	shlex "github.com/flynn/go-shlex"
 	"github.com/stretchr/testify/assert"
-	"testing"
 )
 
 func TestParseCommand(t *testing.T) {
@@ -15,18 +18,33 @@ func TestParseCommand(t *testing.T) {
 }
 
 func TestParseBindsAndVolumes(t *testing.T) {
+	ctx := &Context{}
+	ctx.ComposeFile = "foo/docker-compose.yml"
+	ctx.ResourceLookup = &lookup.FileConfigLookup{}
+
+	abs, err := filepath.Abs(".")
+	assert.Nil(t, err)
+	cfg, hostCfg, err := Convert(&project.ServiceConfig{
+		Volumes: []string{"/foo", "/home:/home", "/bar/baz", ".:/home", "/usr/lib:/usr/lib:ro"},
+	}, ctx)
+	assert.Nil(t, err)
+	assert.Equal(t, map[string]struct{}{"/foo": {}, "/bar/baz": {}}, cfg.Volumes)
+	assert.Equal(t, []string{"/home:/home", abs + "/foo:/home", "/usr/lib:/usr/lib:ro"}, hostCfg.Binds)
+}
+
+func TestParseLabels(t *testing.T) {
+	ctx := &Context{}
+	ctx.ComposeFile = "foo/docker-compose.yml"
+	ctx.ResourceLookup = &lookup.FileConfigLookup{}
 	bashCmd := "bash"
 	fooLabel := "foo.label"
 	fooLabelValue := "service.config.value"
 	sc := &project.ServiceConfig{
 		Entrypoint: project.NewCommand(bashCmd),
-		Volumes:    []string{"/foo", "/home:/home", "/bar/baz", "/usr/lib:/usr/lib:ro"},
 		Labels:     project.NewSliceorMap(map[string]string{fooLabel: "service.config.value"}),
 	}
-	cfg, hostCfg, err := Convert(sc)
+	cfg, _, err := Convert(sc, ctx)
 	assert.Nil(t, err)
-	assert.Equal(t, map[string]struct{}{"/foo": {}, "/bar/baz": {}}, cfg.Volumes)
-	assert.Equal(t, []string{"/home:/home", "/usr/lib:/usr/lib:ro"}, hostCfg.Binds)
 
 	cfg.Labels[fooLabel] = "FUN"
 	cfg.Entrypoint[0] = "less"

docker/project.go

@@ -9,8 +9,8 @@ import (
 
 // NewProject creates a Project with the specified context.
 func NewProject(context *Context) (*project.Project, error) {
-	if context.ConfigLookup == nil {
-		context.ConfigLookup = &lookup.FileConfigLookup{}
+	if context.ResourceLookup == nil {
+		context.ResourceLookup = &lookup.FileConfigLookup{}
 	}
 
 	if context.EnvironmentLookup == nil {

integration/assets/volumes/relative-volumes.yml

@@ -0,0 +1,4 @@
+server:
+    image: busybox
+    volumes:
+    - .:/path

integration/up_test.go

@@ -2,6 +2,7 @@ package integration
 
 import (
 	"fmt"
+	"path/filepath"
 	"strings"
 
 	"github.com/docker/libcompose/utils"
@@ -278,3 +279,21 @@ func (s *RunSuite) TestLink(c *C) {
 		fmt.Sprintf("/%s:/%s/%s", serverName, clientName, serverName),
 	}))
 }
+
+func (s *RunSuite) TestRelativeVolume(c *C) {
+	p := s.ProjectFromText(c, "up", `
+	server:
+	  image: busybox
+	  volumes:
+	    - .:/path
+	`)
+
+	absPath, err := filepath.Abs(".")
+	c.Assert(err, IsNil)
+	serverName := fmt.Sprintf("%s_%s_1", p, "server")
+	cn := s.GetContainerByName(c, serverName)
+
+	c.Assert(cn, NotNil)
+	c.Assert(len(cn.Mounts), DeepEquals, 1)
+	c.Assert(cn.Mounts[0].Source, DeepEquals, absPath)
+}

lookup/file.go

@@ -2,13 +2,44 @@ package lookup
 
 import (
 	"io/ioutil"
+	"os"
 	"path"
+	"path/filepath"
 	"strings"
 
 	"github.com/Sirupsen/logrus"
 )
 
-// FileConfigLookup is a "bare" structure that implements the project.ConfigLookup interface
+// relativePath returns the proper relative path for the given file path. If
+// the relativeTo string equals "-", then it means that it's from the stdin,
+// and the returned path will be the current working directory. Otherwise, if
+// file is really an absolute path, then it will be returned without any
+// changes. Otherwise, the returned path will be a combination of relativeTo
+// and file.
+func relativePath(file, relativeTo string) string {
+	// stdin: return the current working directory if possible.
+	if relativeTo == "-" {
+		if cwd, err := os.Getwd(); err == nil {
+			return cwd
+		}
+	}
+
+	// If the given file is already an absolute path, just return it.
+	// Otherwise, the returned path will be relative to the given relativeTo
+	// path.
+	if filepath.IsAbs(file) {
+		return file
+	}
+
+	abs, err := filepath.Abs(filepath.Join(path.Dir(relativeTo), file))
+	if err != nil {
+		logrus.Errorf("Failed to get absolute directory: %s", err)
+		return file
+	}
+	return abs
+}
+
+// FileConfigLookup is a "bare" structure that implements the project.ResourceLookup interface
 type FileConfigLookup struct {
 }
 
@@ -17,14 +48,27 @@ type FileConfigLookup struct {
 // If file starts with a slash ('/'), it tries to load it, otherwise it will build a
 // filename using the folder part of relativeTo joined with file.
 func (f *FileConfigLookup) Lookup(file, relativeTo string) ([]byte, string, error) {
-	if strings.HasPrefix(file, "/") {
-		logrus.Debugf("Reading file %s", file)
-		bytes, err := ioutil.ReadFile(file)
-		return bytes, file, err
+	file = relativePath(file, relativeTo)
+	logrus.Debugf("Reading file %s", file)
+	bytes, err := ioutil.ReadFile(file)
+	return bytes, file, err
+}
+
+// ResolvePath returns the path to be used for the given path volume. This
+// function already takes care of relative paths.
+func (f *FileConfigLookup) ResolvePath(path, inFile string) string {
+	vs := strings.SplitN(path, ":", 2)
+	if len(vs) != 2 || filepath.IsAbs(vs[0]) {
+		return path
 	}
 
-	fileName := path.Join(path.Dir(relativeTo), file)
-	logrus.Debugf("Reading file %s relative to %s", fileName, relativeTo)
-	bytes, err := ioutil.ReadFile(fileName)
-	return bytes, fileName, err
+	if !strings.HasPrefix(vs[0], "./") && !strings.HasPrefix(vs[0], "~/") &&
+		!strings.HasPrefix(vs[0], "/") {
+
+		logrus.Warnf("The mapping \"%s\" is ambiguous. In a future version of Docker, it will "+
+			"designate a \"named\" volume (see https://github.com/docker/docker/pull/14242). "+
+			"To prevent unexpected behaviour, change it to \"./%s\".", vs[0], vs[0])
+	}
+	vs[0] = relativePath(vs[0], inFile)
+	return strings.Join(vs, ":")
 }

lookup/file_test.go

@@ -1,6 +1,7 @@
 package lookup
 
 import (
+	"fmt"
 	"io/ioutil"
 	"path/filepath"
 	"testing"
@@ -12,8 +13,12 @@ type input struct {
 }
 
 func TestLookupError(t *testing.T) {
+	abs, err := filepath.Abs(".")
+	if err != nil {
+		t.Fatalf("Failed to get absolute directory: %s", err)
+	}
 	invalids := map[input]string{
-		input{"", ""}:                             "read .: is a directory",
+		input{"", ""}:                             fmt.Sprintf("read %s: is a directory", abs),
 		input{"", "/tmp/"}:                        "read /tmp: is a directory",
 		input{"file", "/does/not/exists/"}:        "open /does/not/exists/file: no such file or directory",
 		input{"file", "/does/not/something"}:      "open /does/not/file: no such file or directory",

project/context.go

@@ -31,7 +31,7 @@ type Context struct {
 	isOpen              bool
 	ServiceFactory      ServiceFactory
 	EnvironmentLookup   EnvironmentLookup
-	ConfigLookup        ConfigLookup
+	ResourceLookup      ResourceLookup
 	LoggerFactory       logger.Factory
 	IgnoreMissingConfig bool
 	Project             *Project

project/merge.go

@@ -44,7 +44,7 @@ func mergeProject(p *Project, bytes []byte) (map[string]*ServiceConfig, error) {
 	}
 
 	for name, data := range datas {
-		data, err := parse(p.context.ConfigLookup, p.context.EnvironmentLookup, p.File, data, datas)
+		data, err := parse(p.context.ResourceLookup, p.context.EnvironmentLookup, p.File, data, datas)
 		if err != nil {
 			logrus.Errorf("Failed to parse service %s: %v", name, err)
 			return nil, err
@@ -70,7 +70,7 @@ func adjustValues(configs map[string]*ServiceConfig) {
 	}
 }
 
-func readEnvFile(configLookup ConfigLookup, inFile string, serviceData rawService) (rawService, error) {
+func readEnvFile(resourceLookup ResourceLookup, inFile string, serviceData rawService) (rawService, error) {
 	var config ServiceConfig
 
 	if err := utils.Convert(serviceData, &config); err != nil {
@@ -81,15 +81,15 @@ func readEnvFile(configLookup ConfigLookup, inFile string, serviceData rawServic
 		return serviceData, nil
 	}
 
-	if configLookup == nil {
+	if resourceLookup == nil {
 		return nil, fmt.Errorf("Can not use env_file in file %s no mechanism provided to load files", inFile)
 	}
 
 	vars := config.Environment.Slice()
 
 	for i := len(config.EnvFile.Slice()) - 1; i >= 0; i-- {
 		envFile := config.EnvFile.Slice()[i]
-		content, _, err := configLookup.Lookup(envFile, inFile)
+		content, _, err := resourceLookup.Lookup(envFile, inFile)
 		if err != nil {
 			return nil, err
 		}
@@ -154,8 +154,8 @@ func resolveBuild(inFile string, serviceData rawService) (rawService, error) {
 	return serviceData, nil
 }
 
-func parse(configLookup ConfigLookup, environmentLookup EnvironmentLookup, inFile string, serviceData rawService, datas rawServiceMap) (rawService, error) {
-	serviceData, err := readEnvFile(configLookup, inFile, serviceData)
+func parse(resourceLookup ResourceLookup, environmentLookup EnvironmentLookup, inFile string, serviceData rawService, datas rawServiceMap) (rawService, error) {
+	serviceData, err := readEnvFile(resourceLookup, inFile, serviceData)
 	if err != nil {
 		return nil, err
 	}
@@ -175,7 +175,7 @@ func parse(configLookup ConfigLookup, environmentLookup EnvironmentLookup, inFil
 		return serviceData, nil
 	}
 
-	if configLookup == nil {
+	if resourceLookup == nil {
 		return nil, fmt.Errorf("Can not use extends in file %s no mechanism provided to files", inFile)
 	}
 
@@ -190,12 +190,12 @@ func parse(configLookup ConfigLookup, environmentLookup EnvironmentLookup, inFil
 
 	if file == "" {
 		if serviceData, ok := datas[service]; ok {
-			baseService, err = parse(configLookup, environmentLookup, inFile, serviceData, datas)
+			baseService, err = parse(resourceLookup, environmentLookup, inFile, serviceData, datas)
 		} else {
 			return nil, fmt.Errorf("Failed to find service %s to extend", service)
 		}
 	} else {
-		bytes, resolved, err := configLookup.Lookup(file, inFile)
+		bytes, resolved, err := resourceLookup.Lookup(file, inFile)
 		if err != nil {
 			logrus.Errorf("Failed to lookup file %s: %v", file, err)
 			return nil, err
@@ -216,7 +216,7 @@ func parse(configLookup ConfigLookup, environmentLookup EnvironmentLookup, inFil
 			return nil, fmt.Errorf("Failed to find service %s in file %s", service, file)
 		}
 
-		baseService, err = parse(configLookup, environmentLookup, resolved, baseService, baseRawServices)
+		baseService, err = parse(resourceLookup, environmentLookup, resolved, baseService, baseRawServices)
 	}
 
 	if err != nil {