Convert new compose volume type to swarm mount type

Signed-off-by: Daniel Nephin <dnephin@docker.com>

compose/convert/volume.go

@@ -1,21 +1,19 @@
 package convert
 
 import (
-	"fmt"
-	"strings"
-
 	"github.com/docker/docker/api/types/mount"
 	composetypes "github.com/docker/docker/cli/compose/types"
+	"github.com/pkg/errors"
 )
 
 type volumes map[string]composetypes.VolumeConfig
 
 // Volumes from compose-file types to engine api types
-func Volumes(serviceVolumes []string, stackVolumes volumes, namespace Namespace) ([]mount.Mount, error) {
+func Volumes(serviceVolumes []composetypes.ServiceVolumeConfig, stackVolumes volumes, namespace Namespace) ([]mount.Mount, error) {
 	var mounts []mount.Mount
 
-	for _, volumeSpec := range serviceVolumes {
-		mount, err := convertVolumeToMount(volumeSpec, stackVolumes, namespace)
+	for _, volumeConfig := range serviceVolumes {
+		mount, err := convertVolumeToMount(volumeConfig, stackVolumes, namespace)
 		if err != nil {
 			return nil, err
 		}
@@ -24,108 +22,65 @@ func Volumes(serviceVolumes []string, stackVolumes volumes, namespace Namespace)
 	return mounts, nil
 }
 
-func convertVolumeToMount(volumeSpec string, stackVolumes volumes, namespace Namespace) (mount.Mount, error) {
-	var source, target string
-	var mode []string
+func convertVolumeToMount(
+	volume composetypes.ServiceVolumeConfig,
+	stackVolumes volumes,
+	namespace Namespace,
+) (mount.Mount, error) {
+	result := mount.Mount{
+		Type:     mount.Type(volume.Type),
+		Source:   volume.Source,
+		Target:   volume.Target,
+		ReadOnly: volume.ReadOnly,
+	}
 
-	// TODO: split Windows path mappings properly
-	parts := strings.SplitN(volumeSpec, ":", 3)
+	// Anonymous volumes
+	if volume.Source == "" {
+		return result, nil
+	}
+	if volume.Type == "volume" && volume.Bind != nil {
+		return result, errors.New("bind options are incompatible with type volume")
+	}
+	if volume.Type == "bind" && volume.Volume != nil {
+		return result, errors.New("volume options are incompatible with type bind")
+	}
 
-	for _, part := range parts {
-		if strings.TrimSpace(part) == "" {
-			return mount.Mount{}, fmt.Errorf("invalid volume: %s", volumeSpec)
+	if volume.Bind != nil {
+		result.BindOptions = &mount.BindOptions{
+			Propagation: mount.Propagation(volume.Bind.Propagation),
 		}
 	}
-
-	switch len(parts) {
-	case 3:
-		source = parts[0]
-		target = parts[1]
-		mode = strings.Split(parts[2], ",")
-	case 2:
-		source = parts[0]
-		target = parts[1]
-	case 1:
-		target = parts[0]
+	// Binds volumes
+	if volume.Type == "bind" {
+		return result, nil
 	}
 
-	if source == "" {
-		// Anonymous volume
-		return mount.Mount{
-			Type:   mount.TypeVolume,
-			Target: target,
-		}, nil
-	}
-
-	// TODO: catch Windows paths here
-	if strings.HasPrefix(source, "/") {
-		return mount.Mount{
-			Type:        mount.TypeBind,
-			Source:      source,
-			Target:      target,
-			ReadOnly:    isReadOnly(mode),
-			BindOptions: getBindOptions(mode),
-		}, nil
-	}
-
-	stackVolume, exists := stackVolumes[source]
+	stackVolume, exists := stackVolumes[volume.Source]
 	if !exists {
-		return mount.Mount{}, fmt.Errorf("undefined volume: %s", source)
+		return result, errors.Errorf("undefined volume: %s", volume.Source)
 	}
 
-	var volumeOptions *mount.VolumeOptions
-	if stackVolume.External.Name != "" {
-		volumeOptions = &mount.VolumeOptions{
-			NoCopy: isNoCopy(mode),
-		}
-		source = stackVolume.External.Name
-	} else {
-		volumeOptions = &mount.VolumeOptions{
-			Labels: AddStackLabel(namespace, stackVolume.Labels),
-			NoCopy: isNoCopy(mode),
-		}
+	result.Source = namespace.Scope(volume.Source)
+	result.VolumeOptions = &mount.VolumeOptions{}
 
-		if stackVolume.Driver != "" {
-			volumeOptions.DriverConfig = &mount.Driver{
-				Name:    stackVolume.Driver,
-				Options: stackVolume.DriverOpts,
-			}
-		}
-		source = namespace.Scope(source)
+	if volume.Volume != nil {
+		result.VolumeOptions.NoCopy = volume.Volume.NoCopy
 	}
-	return mount.Mount{
-		Type:          mount.TypeVolume,
-		Source:        source,
-		Target:        target,
-		ReadOnly:      isReadOnly(mode),
-		VolumeOptions: volumeOptions,
-	}, nil
-}
 
-func modeHas(mode []string, field string) bool {
-	for _, item := range mode {
-		if item == field {
-			return true
+	// External named volumes
+	if stackVolume.External.External {
+		result.Source = stackVolume.External.Name
+		return result, nil
+	}
+
+	result.VolumeOptions.Labels = AddStackLabel(namespace, stackVolume.Labels)
+	if stackVolume.Driver != "" || stackVolume.DriverOpts != nil {
+		result.VolumeOptions.DriverConfig = &mount.Driver{
+			Name:    stackVolume.Driver,
+			Options: stackVolume.DriverOpts,
 		}
 	}
-	return false
-}
 
-func isReadOnly(mode []string) bool {
-	return modeHas(mode, "ro")
-}
-
-func isNoCopy(mode []string) bool {
-	return modeHas(mode, "nocopy")
-}
-
-func getBindOptions(mode []string) *mount.BindOptions {
-	for _, item := range mode {
-		for _, propagation := range mount.Propagations {
-			if mount.Propagation(item) == propagation {
-				return &mount.BindOptions{Propagation: mount.Propagation(item)}
-			}
-		}
-	}
-	return nil
+	// Named volumes
+	return result, nil
 }

compose/convert/volume_test.go

@@ -8,51 +8,48 @@ import (
 	"github.com/docker/docker/pkg/testutil/assert"
 )
 
-func TestIsReadOnly(t *testing.T) {
-	assert.Equal(t, isReadOnly([]string{"foo", "bar", "ro"}), true)
-	assert.Equal(t, isReadOnly([]string{"ro"}), true)
-	assert.Equal(t, isReadOnly([]string{}), false)
-	assert.Equal(t, isReadOnly([]string{"foo", "rw"}), false)
-	assert.Equal(t, isReadOnly([]string{"foo"}), false)
-}
-
-func TestIsNoCopy(t *testing.T) {
-	assert.Equal(t, isNoCopy([]string{"foo", "bar", "nocopy"}), true)
-	assert.Equal(t, isNoCopy([]string{"nocopy"}), true)
-	assert.Equal(t, isNoCopy([]string{}), false)
-	assert.Equal(t, isNoCopy([]string{"foo", "rw"}), false)
-}
-
-func TestGetBindOptions(t *testing.T) {
-	opts := getBindOptions([]string{"slave"})
-	expected := mount.BindOptions{Propagation: mount.PropagationSlave}
-	assert.Equal(t, *opts, expected)
-}
-
-func TestGetBindOptionsNone(t *testing.T) {
-	opts := getBindOptions([]string{"ro"})
-	assert.Equal(t, opts, (*mount.BindOptions)(nil))
-}
-
 func TestConvertVolumeToMountAnonymousVolume(t *testing.T) {
-	stackVolumes := volumes{}
-	namespace := NewNamespace("foo")
+	config := composetypes.ServiceVolumeConfig{
+		Type:   "volume",
+		Target: "/foo/bar",
+	}
 	expected := mount.Mount{
 		Type:   mount.TypeVolume,
 		Target: "/foo/bar",
 	}
-	mount, err := convertVolumeToMount("/foo/bar", stackVolumes, namespace)
+	mount, err := convertVolumeToMount(config, volumes{}, NewNamespace("foo"))
 	assert.NilError(t, err)
 	assert.DeepEqual(t, mount, expected)
 }
 
-func TestConvertVolumeToMountInvalidFormat(t *testing.T) {
+func TestConvertVolumeToMountConflictingOptionsBind(t *testing.T) {
 	namespace := NewNamespace("foo")
-	invalids := []string{"::", "::cc", ":bb:", "aa::", "aa::cc", "aa:bb:", " : : ", " : :cc", " :bb: ", "aa: : ", "aa: :cc", "aa:bb: "}
-	for _, vol := range invalids {
-		_, err := convertVolumeToMount(vol, volumes{}, namespace)
-		assert.Error(t, err, "invalid volume: "+vol)
+
+	config := composetypes.ServiceVolumeConfig{
+		Type:   "volume",
+		Source: "foo",
+		Target: "/target",
+		Bind: &composetypes.ServiceVolumeBind{
+			Propagation: "slave",
+		},
 	}
+	_, err := convertVolumeToMount(config, volumes{}, namespace)
+	assert.Error(t, err, "bind options are incompatible")
+}
+
+func TestConvertVolumeToMountConflictingOptionsVolume(t *testing.T) {
+	namespace := NewNamespace("foo")
+
+	config := composetypes.ServiceVolumeConfig{
+		Type:   "bind",
+		Source: "/foo",
+		Target: "/target",
+		Volume: &composetypes.ServiceVolumeVolume{
+			NoCopy: true,
+		},
+	}
+	_, err := convertVolumeToMount(config, volumes{}, namespace)
+	assert.Error(t, err, "volume options are incompatible")
 }
 
 func TestConvertVolumeToMountNamedVolume(t *testing.T) {
@@ -84,9 +81,19 @@ func TestConvertVolumeToMountNamedVolume(t *testing.T) {
 					"opt": "value",
 				},
 			},
+			NoCopy: true,
 		},
 	}
-	mount, err := convertVolumeToMount("normal:/foo:ro", stackVolumes, namespace)
+	config := composetypes.ServiceVolumeConfig{
+		Type:     "volume",
+		Source:   "normal",
+		Target:   "/foo",
+		ReadOnly: true,
+		Volume: &composetypes.ServiceVolumeVolume{
+			NoCopy: true,
+		},
+	}
+	mount, err := convertVolumeToMount(config, stackVolumes, namespace)
 	assert.NilError(t, err)
 	assert.DeepEqual(t, mount, expected)
 }
@@ -109,7 +116,12 @@ func TestConvertVolumeToMountNamedVolumeExternal(t *testing.T) {
 			NoCopy: false,
 		},
 	}
-	mount, err := convertVolumeToMount("outside:/foo", stackVolumes, namespace)
+	config := composetypes.ServiceVolumeConfig{
+		Type:   "volume",
+		Source: "outside",
+		Target: "/foo",
+	}
+	mount, err := convertVolumeToMount(config, stackVolumes, namespace)
 	assert.NilError(t, err)
 	assert.DeepEqual(t, mount, expected)
 }
@@ -132,7 +144,15 @@ func TestConvertVolumeToMountNamedVolumeExternalNoCopy(t *testing.T) {
 			NoCopy: true,
 		},
 	}
-	mount, err := convertVolumeToMount("outside:/foo:nocopy", stackVolumes, namespace)
+	config := composetypes.ServiceVolumeConfig{
+		Type:   "volume",
+		Source: "outside",
+		Target: "/foo",
+		Volume: &composetypes.ServiceVolumeVolume{
+			NoCopy: true,
+		},
+	}
+	mount, err := convertVolumeToMount(config, stackVolumes, namespace)
 	assert.NilError(t, err)
 	assert.DeepEqual(t, mount, expected)
 }
@@ -147,13 +167,26 @@ func TestConvertVolumeToMountBind(t *testing.T) {
 		ReadOnly:    true,
 		BindOptions: &mount.BindOptions{Propagation: mount.PropagationShared},
 	}
-	mount, err := convertVolumeToMount("/bar:/foo:ro,shared", stackVolumes, namespace)
+	config := composetypes.ServiceVolumeConfig{
+		Type:     "bind",
+		Source:   "/bar",
+		Target:   "/foo",
+		ReadOnly: true,
+		Bind:     &composetypes.ServiceVolumeBind{Propagation: "shared"},
+	}
+	mount, err := convertVolumeToMount(config, stackVolumes, namespace)
 	assert.NilError(t, err)
 	assert.DeepEqual(t, mount, expected)
 }
 
 func TestConvertVolumeToMountVolumeDoesNotExist(t *testing.T) {
 	namespace := NewNamespace("foo")
-	_, err := convertVolumeToMount("unknown:/foo:ro", volumes{}, namespace)
+	config := composetypes.ServiceVolumeConfig{
+		Type:     "volume",
+		Source:   "unknown",
+		Target:   "/foo",
+		ReadOnly: true,
+	}
+	_, err := convertVolumeToMount(config, volumes{}, namespace)
 	assert.Error(t, err, "undefined volume: unknown")
 }

compose/schema/schema.go

@@ -81,13 +81,18 @@ func toError(result *gojsonschema.Result) error {
 	return err
 }
 
+const (
+	jsonschemaOneOf = "number_one_of"
+	jsonschemaAnyOf = "number_any_of"
+)
+
 func getDescription(err validationError) string {
 	switch err.parent.Type() {
 	case "invalid_type":
 		if expectedType, ok := err.parent.Details()["expected"].(string); ok {
 			return fmt.Sprintf("must be a %s", humanReadableType(expectedType))
 		}
-	case "number_one_of", "number_any_of":
+	case jsonschemaOneOf, jsonschemaAnyOf:
 		if err.child == nil {
 			return err.parent.Description()
 		}