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 bf19b4fb3..20935ed41 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 { + volumeWithHost 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{ + volumeWithHost: 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,14 +296,21 @@ 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 { + // volumes with a host path + for hostPath, volName := range hostPaths.volumeWithHost { 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) } @@ -420,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 @@ -443,13 +458,19 @@ 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.volumeWithHost) + 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 + volumeName = volumes.volumeWithHost[hostPath] + + if volumeName == "" { + volumeName = getVolumeName(numVol) + volumes.volumeWithHost[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..597bc2708 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,10 +28,11 @@ 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" ) func TestConvertToTaskDefinition(t *testing.T) { @@ -262,7 +263,7 @@ func TestConvertToTaskDefinitionWithLogConfiguration(t *testing.T) { taskDefinition = convertToTaskDefinitionInTest(t, "name", serviceConfig) containerDef = *taskDefinition.ContainerDefinitions[0] if logDriver != aws.StringValue(containerDef.LogConfiguration.LogDriver) { - t.Errorf("Expected Log driver [%s]. But was [%s]", containerDef.LogConfiguration) + t.Errorf("Expected Log driver [%s]. But was [%s]", logDriver, aws.StringValue(containerDef.LogConfiguration.LogDriver)) } if !reflect.DeepEqual(logOpts, aws.StringValueMap(containerDef.LogConfiguration.Options)) { t.Errorf("Expected Log options [%v]. But was [%v]", logOpts, aws.StringValueMap(containerDef.LogConfiguration.Options)) @@ -350,14 +351,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{ + volumeWithHost: 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 { @@ -366,11 +372,23 @@ func TestConvertToMountPoints(t *testing.T) { if len(mountPointsIn.Volumes) != len(mountPointsOut) { 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[0], volumes, "", containerPath, false, 0) // 0 is the counter for the first volume with an empty host path + verifyMountPoint(t, mountPointsOut[1], volumes, "", containerPath2, false, 1) // 1 is the counter for the second volume with an empty host path + verifyMountPoint(t, mountPointsOut[2], volumes, hostPath, containerPath, false, 1) + verifyMountPoint(t, mountPointsOut[3], volumes, "", containerPath, true, 2) // 2 is the counter for the third volume with an empty host path + verifyMountPoint(t, mountPointsOut[4], volumes, hostPath, containerPath, true, 2) + verifyMountPoint(t, mountPointsOut[5], volumes, hostPath, containerPath, false, 2) + + 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 +405,22 @@ func TestConvertToMountPoints(t *testing.T) { } } -func verifyMountPoint(t *testing.T, output *ecs.MountPoint, volumes map[string]string, - hostPath, containerPath string, readonly bool) { - +func verifyMountPoint(t *testing.T, output *ecs.MountPoint, volumes *volumes, + hostPath, containerPath string, readonly bool, EmptyHostCtr int) { + sourceVolume := "" if containerPath != *output.ContainerPath { t.Errorf("Expected containerPath [%s] But was [%s]", containerPath, *output.ContainerPath) } - sourceVolume := volumes[hostPath] + if hostPath != "" { + sourceVolume = volumes.volumeWithHost[hostPath] + } else { + sourceVolume = volumes.volumeEmptyHost[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) } }