Skip to content

Commit 6e13bda

Browse files
allisaurusallisaurus
committed
Wire up top-level Volumes for Compose 3
1 parent 33687ee commit 6e13bda

File tree

5 files changed

+227
-40
lines changed

5 files changed

+227
-40
lines changed

‎ecs-cli/modules/cli/compose/adapter/convert.go

Lines changed: 66 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ package adapter
1616
import (
1717
"encoding/json"
1818
"fmt"
19+
"reflect"
1920
"regexp"
2021
"strconv"
2122
"strings"
2223

2324
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/utils"
2425
"github.com/aws/aws-sdk-go/aws"
2526
"github.com/aws/aws-sdk-go/service/ecs"
27+
"github.com/docker/cli/cli/compose/types"
2628
"github.com/docker/go-units"
2729
"github.com/docker/libcompose/config"
2830
"github.com/docker/libcompose/project"
@@ -151,26 +153,9 @@ func ConvertToMountPoints(cfgVolumes *yaml.Volumes, volumes *Volumes) ([]*ecs.Mo
151153
}
152154
}
153155

154-
var volumeName string
155-
numVol := len(volumes.VolumeWithHost) + len(volumes.VolumeEmptyHost)
156-
if source == "" {
157-
// add mount point for volumes with an empty host path
158-
volumeName = getVolumeName(numVol)
159-
volumes.VolumeEmptyHost = append(volumes.VolumeEmptyHost, volumeName)
160-
} else if project.IsNamedVolume(source) {
161-
if !utils.InSlice(source, volumes.VolumeEmptyHost) {
162-
return nil, fmt.Errorf(
163-
"named volume [%s] is used but no declaration was found in the volumes section", cfgVolume)
164-
}
165-
volumeName = source
166-
} else {
167-
// add mount point for volumes with a host path
168-
volumeName = volumes.VolumeWithHost[source]
169-
170-
if volumeName == "" {
171-
volumeName = getVolumeName(numVol)
172-
volumes.VolumeWithHost[source] = volumeName
173-
}
156+
volumeName, err := GetSourcePathAndUpdateVolumes(source, volumes)
157+
if err != nil {
158+
return nil, err
174159
}
175160

176161
mountPoints = append(mountPoints, &ecs.MountPoint{
@@ -182,6 +167,34 @@ func ConvertToMountPoints(cfgVolumes *yaml.Volumes, volumes *Volumes) ([]*ecs.Mo
182167
return mountPoints, nil
183168
}
184169

170+
// GetSourcePathAndUpdateVolumes checks for & creates an ECS Volume for a mount point without
171+
// a source volume and returns the appropriate source volume name
172+
func GetSourcePathAndUpdateVolumes(source string, volumes *Volumes) (string, error) {
173+
var volumeName string
174+
numVol := len(volumes.VolumeWithHost) + len(volumes.VolumeEmptyHost)
175+
if source == "" {
176+
// add mount point for volumes with an empty source path
177+
volumeName = getVolumeName(numVol)
178+
volumes.VolumeEmptyHost = append(volumes.VolumeEmptyHost, volumeName)
179+
} else if project.IsNamedVolume(source) {
180+
if !utils.InSlice(source, volumes.VolumeEmptyHost) {
181+
return "", fmt.Errorf(
182+
"named volume [%s] is used but no declaration was found in the volumes section", source)
183+
}
184+
volumeName = source
185+
} else {
186+
// add mount point for volumes with a source path
187+
volumeName = volumes.VolumeWithHost[source]
188+
189+
if volumeName == "" {
190+
volumeName = getVolumeName(numVol)
191+
volumes.VolumeWithHost[source] = volumeName
192+
}
193+
}
194+
195+
return volumeName, nil
196+
}
197+
185198
// ConvertToPortMappings transforms the yml ports string slice to ecs compatible PortMappings slice
186199
func ConvertToPortMappings(serviceName string, cfgPorts []string) ([]*ecs.PortMapping, error) {
187200
portMappings := []*ecs.PortMapping{}
@@ -310,24 +323,47 @@ func ConvertToVolumes(volumeConfigs map[string]*config.VolumeConfig) (*Volumes,
310323
if volumeConfigs != nil {
311324
for name, config := range volumeConfigs {
312325
if config != nil {
313-
// NOTE: If Driver field is not empty, this
314-
// will add a prefix to the named volume on the container
315-
if config.Driver != "" {
316-
return nil, errors.New("Volume driver is not supported")
317-
}
318-
// Driver Options must relate to a specific volume driver
319-
if len(config.DriverOpts) != 0 {
320-
return nil, errors.New("Volume driver options is not supported")
321-
}
322-
return nil, errors.New("External option is not supported")
326+
return nil, logOutUnsupportedVolumeFields(config.Driver, config.DriverOpts, nil)
323327
}
324328
volumes.VolumeEmptyHost = append(volumes.VolumeEmptyHost, name)
325329
}
326330
}
331+
return volumes, nil
332+
}
333+
334+
// ConvertToV3Volumes converts the VolumesConfig map in a docker/cli config into a
335+
// Volumes struct and populates the VolumesEmptyHost field with any names volumes
336+
func ConvertToV3Volumes(volConfig map[string]types.VolumeConfig) (*Volumes, error) {
337+
volumes := NewVolumes()
327338

339+
// Add named volume configs:
340+
if len(volConfig) != 0 {
341+
for name, config := range volConfig {
342+
if !reflect.DeepEqual(config, types.VolumeConfig{}) {
343+
return nil, logOutUnsupportedVolumeFields(config.Driver, config.DriverOpts, config.Labels)
344+
}
345+
volumes.VolumeEmptyHost = append(volumes.VolumeEmptyHost, name)
346+
}
347+
}
328348
return volumes, nil
329349
}
330350

351+
func logOutUnsupportedVolumeFields(driver string, driverOpts map[string]string, labels map[string]string) error {
352+
// NOTE: If Driver field is not empty, this
353+
// will add a prefix to the named volume on the container
354+
if driver != "" {
355+
return errors.New("Volume driver is not supported")
356+
}
357+
// Driver Options must relate to a specific volume driver
358+
if len(driverOpts) != 0 {
359+
return errors.New("Volume driver options is not supported")
360+
}
361+
if labels != nil && len(labels) != 0 {
362+
return errors.New("Volume labels are not supported")
363+
}
364+
return errors.New("External option is not supported")
365+
}
366+
331367
// ConvertToVolumesFrom transforms the yml volumes from to ecs compatible VolumesFrom slice
332368
// Examples for compose format v2:
333369
// volumes_from:

‎ecs-cli/modules/cli/compose/adapter/convert_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,34 @@ func TestConvertToMountPointsWithNoCorrespondingNamedVolume(t *testing.T) {
344344
assert.Error(t, err, "Expected error converting MountPoints")
345345
}
346346

347+
func TestGetSourcePathAndUpdateVolumesWithEmptySourcePath(t *testing.T) {
348+
expectedSourcePath := "volume-0"
349+
volumes := &Volumes{
350+
VolumeWithHost: make(map[string]string),
351+
VolumeEmptyHost: []string{}, // Top-level named volumes is empty
352+
}
353+
354+
observedSourcePath, err := GetSourcePathAndUpdateVolumes("", volumes)
355+
356+
assert.NoError(t, err, "Unexpected error getting Mount Point source path")
357+
assert.Equal(t, expectedSourcePath, observedSourcePath)
358+
assert.Equal(t, expectedSourcePath, volumes.VolumeEmptyHost[0])
359+
}
360+
361+
func TestGetSourcePathAndUpdateVolumesWithNamedVol(t *testing.T) {
362+
namedSourcePath := "logging"
363+
volumes := &Volumes{
364+
VolumeWithHost: make(map[string]string),
365+
VolumeEmptyHost: []string{namedSourcePath},
366+
}
367+
368+
observedSourcePath, err := GetSourcePathAndUpdateVolumes(namedSourcePath, volumes)
369+
370+
assert.NoError(t, err, "Unexpected error getting Mount Point source path")
371+
assert.Equal(t, namedSourcePath, observedSourcePath)
372+
assert.Equal(t, namedSourcePath, volumes.VolumeEmptyHost[0])
373+
}
374+
347375
func TestConvertToExtraHosts(t *testing.T) {
348376
hostname := "test.local"
349377
ipAddress := "127.10.10.10"

‎ecs-cli/modules/cli/compose/project/project_parseV1V2.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package project
22

33
import (
44
"fmt"
5+
56
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/cli/compose/adapter"
67
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/cli/compose/logger"
78
"github.com/aws/aws-sdk-go/aws"

‎ecs-cli/modules/cli/compose/project/project_parseV3.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,22 @@ func (p *ecsProject) parseV3() (*[]adapter.ContainerConfig, error) {
2424
return nil, err
2525
}
2626

27+
servVols, err := adapter.ConvertToV3Volumes(v3Config.Volumes)
28+
if err != nil {
29+
return nil, err
30+
}
31+
p.volumes = servVols
32+
2733
// convert ServiceConfigs to ContainerConfigs
2834
conConfigs := []adapter.ContainerConfig{}
2935
for _, service := range v3Config.Services {
30-
cCon, err := convertToContainerConfig(service)
36+
cCon, err := convertToContainerConfig(service, p.volumes)
3137
if err != nil {
3238
return nil, err
3339
}
3440
conConfigs = append(conConfigs, *cCon)
3541
}
3642

37-
// TODO: process v3Config.Volumes as well
3843
return &conConfigs, nil
3944
}
4045

@@ -78,7 +83,7 @@ func getV3Config(composeFiles []string) (*types.Config, error) {
7883
return config, nil
7984
}
8085

81-
func convertToContainerConfig(serviceConfig types.ServiceConfig) (*adapter.ContainerConfig, error) {
86+
func convertToContainerConfig(serviceConfig types.ServiceConfig, serviceVols *adapter.Volumes) (*adapter.ContainerConfig, error) {
8287
//TODO: Add Healthcheck, Devices to ContainerConfig
8388
c := &adapter.ContainerConfig{
8489
CapAdd: serviceConfig.CapAdd,
@@ -154,11 +159,19 @@ func convertToContainerConfig(serviceConfig types.ServiceConfig) (*adapter.Conta
154159
mountPoints := []*ecs.MountPoint{}
155160

156161
for _, volConfig := range serviceConfig.Volumes {
157-
if volConfig.Type == "volume" {
162+
if volConfig.Type == "volume" || volConfig.Type == "bind" {
163+
164+
sourceVolName, err := adapter.GetSourcePathAndUpdateVolumes(volConfig.Source, serviceVols)
165+
if err != nil {
166+
return nil, err
167+
}
168+
containerPath := volConfig.Target
169+
readOnly := volConfig.ReadOnly
170+
158171
mp := &ecs.MountPoint{
159-
ContainerPath: &volConfig.Target,
160-
SourceVolume: &volConfig.Source,
161-
ReadOnly: &volConfig.ReadOnly,
172+
ContainerPath: &containerPath,
173+
SourceVolume: &sourceVolName,
174+
ReadOnly: &readOnly,
162175
}
163176
mountPoints = append(mountPoints, mp)
164177
} else {

‎ecs-cli/modules/cli/compose/project/project_parseV3_test.go

Lines changed: 112 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package project
33
import (
44
"io/ioutil"
55
"os"
6+
"strings"
67
"testing"
78

89
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/cli/compose/adapter"
@@ -153,6 +154,110 @@ services:
153154
}
154155
}
155156

157+
func TestParseV3WithTopLevelVolume(t *testing.T) {
158+
// set up file
159+
composeFileString := `version: '3'
160+
services:
161+
wordpress:
162+
cap_add:
163+
- ALL
164+
cap_drop:
165+
- NET_ADMIN
166+
command: echo "hello world"
167+
image: wordpress
168+
entrypoint: /wordpress/entry
169+
ports: ["80:80"]
170+
links:
171+
- mysql
172+
tmpfs:
173+
- "/run:rw,size=1gb"
174+
read_only: true
175+
mysql:
176+
image: mysql
177+
labels:
178+
- "com.example.mysql=mysqllabel"
179+
- "com.example.mysql2=anothermysql label"
180+
user: mysqluser
181+
working_dir: /mysqltestdir
182+
privileged: true
183+
volumes:
184+
- /opt/data:/var/lib/mysql
185+
- logging:/test/place:ro
186+
- /var/lib/mysql
187+
extra_hosts:
188+
- "mysqlexhost:10.0.0.0"
189+
volumes:
190+
logging:`
191+
192+
tmpfile, err := ioutil.TempFile("", "test")
193+
assert.NoError(t, err, "Unexpected error in creating test file")
194+
195+
defer os.Remove(tmpfile.Name())
196+
197+
_, err = tmpfile.Write([]byte(composeFileString))
198+
assert.NoError(t, err, "Unexpected error writing file")
199+
200+
err = tmpfile.Close()
201+
assert.NoError(t, err, "Unexpected error closing file")
202+
203+
// add files to projects
204+
project := setupTestProject(t)
205+
project.ecsContext.ComposeFiles = append(project.ecsContext.ComposeFiles, tmpfile.Name())
206+
207+
// get map of docker-parsed ServiceConfigs
208+
expectedConfigs := getServiceConfigMap(t, project.ecsContext.ComposeFiles)
209+
210+
// assert # and content of container configs matches expected services
211+
actualConfigs, err := project.parseV3()
212+
assert.NoError(t, err, "Unexpected error parsing file")
213+
214+
assert.Equal(t, len(expectedConfigs), len(*actualConfigs))
215+
for _, containerConfig := range *actualConfigs {
216+
verifyConvertToContainerConfigOutput(t, expectedConfigs[containerConfig.Name], containerConfig)
217+
}
218+
}
219+
220+
func TestParseV3_ErrorWithExternalVolume(t *testing.T) {
221+
// set up file with invalid Volume config ("external")
222+
composeFileString := `version: '3'
223+
services:
224+
httpd:
225+
cap_add:
226+
- ALL
227+
cap_drop:
228+
- NET_ADMIN
229+
command: echo "hello world"
230+
image: httpd
231+
entrypoint: /web/entry
232+
ports: ["80:80"]
233+
volumes:
234+
- /opt/data:/var/lib/mysql
235+
- logging:/test/place:ro
236+
- /var/lib/test
237+
volumes:
238+
logging:
239+
external:true`
240+
241+
tmpfile, err := ioutil.TempFile("", "test")
242+
assert.NoError(t, err, "Unexpected error in creating test file")
243+
244+
defer os.Remove(tmpfile.Name())
245+
246+
_, err = tmpfile.Write([]byte(composeFileString))
247+
assert.NoError(t, err, "Unexpected error writing file")
248+
249+
err = tmpfile.Close()
250+
assert.NoError(t, err, "Unexpected error closing file")
251+
252+
// add files to projects
253+
project := setupTestProject(t)
254+
project.ecsContext.ComposeFiles = append(project.ecsContext.ComposeFiles, tmpfile.Name())
255+
256+
// assert error when parsing v3 project
257+
_, err = project.parseV3()
258+
assert.Error(t, err, "Expected error when parsing project with invalid Volume configuration")
259+
}
260+
156261
func TestThrowErrorOnBadYaml(t *testing.T) {
157262
badPortsYaml := `version: '2'
158263
services:
@@ -333,9 +438,13 @@ func verifyConvertToContainerConfigOutput(t *testing.T, expected types.ServiceCo
333438
}
334439

335440
func verifyMountPoint(t *testing.T, servVolume types.ServiceVolumeConfig, mountPoint ecs.MountPoint) {
336-
assert.Equal(t, servVolume.Target, mountPoint.ContainerPath, "Expected volume Target and mount point ContainerPath to match")
337-
assert.Equal(t, servVolume.Source, mountPoint.SourceVolume, "Expected volume Source and mount point SourceVolume to match")
338-
assert.Equal(t, servVolume.ReadOnly, mountPoint.ReadOnly, "Expected volume and mount point readOnly to match")
441+
if servVolume.Source == "" {
442+
assert.True(t, strings.HasPrefix(*mountPoint.SourceVolume, "volume-"), "Expected volume Source to being with 'volume-'")
443+
} else {
444+
assert.Equal(t, servVolume.Source, *mountPoint.SourceVolume, "Expected volume Source and mount point SourceVolume to match")
445+
}
446+
assert.Equal(t, servVolume.Target, *mountPoint.ContainerPath, "Expected volume Target and mount point ContainerPath to match")
447+
assert.Equal(t, servVolume.ReadOnly, *mountPoint.ReadOnly, "Expected volume and mount point readOnly to match")
339448
}
340449

341450
func getServiceConfigMap(t *testing.T, composeFiles []string) map[string]types.ServiceConfig {

0 commit comments

Comments
 (0)