diff --git a/ecs-cli/modules/compose/ecs/utils/convert_task_definition.go b/ecs-cli/modules/compose/ecs/utils/convert_task_definition.go index 15db2ca73..b90eb21d1 100644 --- a/ecs-cli/modules/compose/ecs/utils/convert_task_definition.go +++ b/ecs-cli/modules/compose/ecs/utils/convert_task_definition.go @@ -48,6 +48,11 @@ var supportedComposeYamlOptions = []string{ var supportedComposeYamlOptionsMap = getSupportedComposeYamlOptionsMap() +type volumes struct { + volumeHasHost map[string]string + volumeEmptyHost []string +} + func getSupportedComposeYamlOptionsMap() map[string]bool { optionsMap := make(map[string]bool) for _, value := range supportedComposeYamlOptions { @@ -67,7 +72,9 @@ func ConvertToTaskDefinition(taskDefinitionName string, context *project.Context logUnsupportedConfigFields(context.Project) containerDefinitions := []*ecs.ContainerDefinition{} - volumes := make(map[string]string) // map with key:=hostSourcePath value:=VolumeName + volumes := &volumes{ + volumeHasHost: make(map[string]string), // map with key:=hostSourcePath value:=VolumeName + } for _, name := range serviceConfigs.Keys() { serviceConfig, ok := serviceConfigs.Get(name) @@ -83,6 +90,7 @@ func ConvertToTaskDefinition(taskDefinitionName string, context *project.Context } containerDefinitions = append(containerDefinitions, containerDef) } + taskDefinition := &ecs.TaskDefinition{ Family: aws.String(taskDefinitionName), ContainerDefinitions: containerDefinitions, @@ -156,7 +164,7 @@ func isZero(v reflect.Value) bool { // convertToContainerDef transforms each service in the compose yml // to an equivalent container definition func convertToContainerDef(context *project.Context, inputCfg *config.ServiceConfig, - volumes map[string]string, outputContDef *ecs.ContainerDefinition) error { + volumes *volumes, outputContDef *ecs.ContainerDefinition) error { // setting memory var mem int64 if inputCfg.MemLimit != 0 { @@ -288,23 +296,23 @@ func createKeyValuePair(key, value string) *ecs.KeyValuePair { } // convertToECSVolumes transforms the map of hostPaths to the format of ecs.Volume -func convertToECSVolumes(hostPaths map[string]string) []*ecs.Volume { +func convertToECSVolumes(hostPaths *volumes) []*ecs.Volume { output := []*ecs.Volume{} - for hostPath, volName := range hostPaths { - if hostPath == "" { - ecsVolume := &ecs.Volume{ - Name: aws.String(volName), - } - output = append(output, ecsVolume) - } else { - ecsVolume := &ecs.Volume{ - Name: aws.String(volName), - Host: &ecs.HostVolumeProperties{ - SourcePath: aws.String(hostPath), - }, - } - output = append(output, ecsVolume) + // volumes with a host path + for hostPath, volName := range hostPaths.volumeHasHost { + ecsVolume := &ecs.Volume{ + Name: aws.String(volName), + Host: &ecs.HostVolumeProperties{ + SourcePath: aws.String(hostPath), + }} + output = append(output, ecsVolume) + } + // volumes with an empty host path + for _, volName := range hostPaths.volumeEmptyHost { + ecsVolume := &ecs.Volume{ + Name: aws.String(volName), } + output = append(output, ecsVolume) } return output } @@ -427,7 +435,7 @@ func convertToVolumesFrom(cfgVolumesFrom []string) ([]*ecs.VolumeFrom, error) { // convertToMountPoints transforms the yml volumes slice to ecs compatible MountPoints slice // It also uses the hostPath from volumes if present, else adds one to it -func convertToMountPoints(cfgVolumes *yaml.Volumes, volumes map[string]string) ([]*ecs.MountPoint, error) { +func convertToMountPoints(cfgVolumes *yaml.Volumes, volumes *volumes) ([]*ecs.MountPoint, error) { mountPoints := []*ecs.MountPoint{} if cfgVolumes == nil { return mountPoints, nil @@ -450,13 +458,20 @@ func convertToMountPoints(cfgVolumes *yaml.Volumes, volumes map[string]string) ( } var volumeName string - if len(volumes) > 0 { - volumeName = volumes[hostPath] - } - - if volumeName == "" { - volumeName = getVolumeName(len(volumes)) - volumes[hostPath] = volumeName + numVol := len(volumes.volumeHasHost) + len(volumes.volumeEmptyHost) + if hostPath == "" { + // add mount point for volumes with an empty host path + volumeName = getVolumeName(numVol) + volumes.volumeEmptyHost = append(volumes.volumeEmptyHost, volumeName) + } else { + // add mount point for volumes with a host path + if len(volumes.volumeHasHost) > 0 { + volumeName = volumes.volumeHasHost[hostPath] + } + if volumeName == "" { + volumeName = getVolumeName(numVol) + volumes.volumeHasHost[hostPath] = volumeName + } } mountPoints = append(mountPoints, &ecs.MountPoint{ diff --git a/ecs-cli/modules/compose/ecs/utils/convert_task_definition_test.go b/ecs-cli/modules/compose/ecs/utils/convert_task_definition_test.go index fed925f7b..6a1418b37 100644 --- a/ecs-cli/modules/compose/ecs/utils/convert_task_definition_test.go +++ b/ecs-cli/modules/compose/ecs/utils/convert_task_definition_test.go @@ -28,12 +28,15 @@ import ( ) const ( - portNumber = 8000 - portMapping = "8000:8000" - containerPath = "/tmp/cache" - hostPath = "./cache" + portNumber = 8000 + portMapping = "8000:8000" + containerPath = "/tmp/cache" + containerPath2 = "/tmp/cache2" + hostPath = "./cache" ) +var emptyHostCtr = 0 + func TestConvertToTaskDefinition(t *testing.T) { name := "mysql" cpu := int64(131072) // 128 * 1024 @@ -350,14 +353,19 @@ func verifyPortMapping(t *testing.T, output *ecs.PortMapping, hostPort, containe func TestConvertToMountPoints(t *testing.T) { onlyContainerPath := yaml.Volume{Destination: containerPath} - hostAndContainerPath := yaml.Volume{Source: hostPath, Destination: containerPath} // "./cache:/tmp/cache" + onlyContainerPath2 := yaml.Volume{Destination: containerPath2} + hostAndContainerPath := yaml.Volume{Source: hostPath, Destination: containerPath} // "./cache:/tmp/cache" + onlyContainerPathWithRO := yaml.Volume{Destination: containerPath, AccessMode: "ro"} hostAndContainerPathWithRO := yaml.Volume{Source: hostPath, Destination: containerPath, AccessMode: "ro"} // "./cache:/tmp/cache:ro" hostAndContainerPathWithRW := yaml.Volume{Source: hostPath, Destination: containerPath, AccessMode: "rw"} - volumes := make(map[string]string) + volumes := &volumes{ + volumeHasHost: make(map[string]string), // map with key:=hostSourcePath value:=VolumeName + } - mountPointsIn := yaml.Volumes{Volumes: []*yaml.Volume{&onlyContainerPath, &hostAndContainerPath, - &hostAndContainerPathWithRO, &hostAndContainerPathWithRW}} + // Valid inputs with host and container paths set + mountPointsIn := yaml.Volumes{Volumes: []*yaml.Volume{&onlyContainerPath, &onlyContainerPath2, &hostAndContainerPath, + &onlyContainerPathWithRO, &hostAndContainerPathWithRO, &hostAndContainerPathWithRW}} mountPointsOut, err := convertToMountPoints(&mountPointsIn, volumes) if err != nil { @@ -367,10 +375,21 @@ func TestConvertToMountPoints(t *testing.T) { t.Errorf("Incorrect conversion. Input [%v] Output [%v]", mountPointsIn, mountPointsOut) } verifyMountPoint(t, mountPointsOut[0], volumes, "", containerPath, false) - verifyMountPoint(t, mountPointsOut[1], volumes, hostPath, containerPath, false) - verifyMountPoint(t, mountPointsOut[2], volumes, hostPath, containerPath, true) - verifyMountPoint(t, mountPointsOut[3], volumes, hostPath, containerPath, false) + verifyMountPoint(t, mountPointsOut[1], volumes, "", containerPath2, false) + verifyMountPoint(t, mountPointsOut[2], volumes, hostPath, containerPath, false) + verifyMountPoint(t, mountPointsOut[3], volumes, "", containerPath, true) + verifyMountPoint(t, mountPointsOut[4], volumes, hostPath, containerPath, true) + verifyMountPoint(t, mountPointsOut[5], volumes, hostPath, containerPath, false) + + if mountPointsOut[0].SourceVolume == mountPointsOut[1].SourceVolume { + t.Errorf("Expected volume %v (onlyContainerPath) and %v (onlyContainerPath2) to be different", mountPointsOut[0].SourceVolume, mountPointsOut[1].SourceVolume) + } + if mountPointsOut[1].SourceVolume == mountPointsOut[3].SourceVolume { + t.Errorf("Expected volume %v (onlyContainerPath2) and %v (onlyContainerPathWithRO) to be different", mountPointsOut[0].SourceVolume, mountPointsOut[1].SourceVolume) + } + + // Invalid access mode input hostAndContainerPathWithIncorrectAccess := yaml.Volume{Source: hostPath, Destination: containerPath, AccessMode: "readonly"} mountPointsIn = yaml.Volumes{Volumes: []*yaml.Volume{&hostAndContainerPathWithIncorrectAccess}} mountPointsOut, err = convertToMountPoints(&mountPointsIn, volumes) @@ -387,18 +406,23 @@ func TestConvertToMountPoints(t *testing.T) { } } -func verifyMountPoint(t *testing.T, output *ecs.MountPoint, volumes map[string]string, +func verifyMountPoint(t *testing.T, output *ecs.MountPoint, volumes *volumes, hostPath, containerPath string, readonly bool) { - + sourceVolume := "" if containerPath != *output.ContainerPath { t.Errorf("Expected containerPath [%s] But was [%s]", containerPath, *output.ContainerPath) } - sourceVolume := volumes[hostPath] + if hostPath != "" { + sourceVolume = volumes.volumeHasHost[hostPath] + } else { + sourceVolume = volumes.volumeEmptyHost[emptyHostCtr] + emptyHostCtr++ + } if sourceVolume != *output.SourceVolume { t.Errorf("Expected sourceVolume [%s] But was [%s]", sourceVolume, *output.SourceVolume) } if readonly != *output.ReadOnly { - t.Errorf("Expected readonly [%s] But was [%s]", readonly, *output.ReadOnly) + t.Errorf("Expected readonly [%v] But was [%v]", readonly, *output.ReadOnly) } }