Skip to content

Commit

Permalink
Fixes aws#11: Creates new volume when there's no host path
Browse files Browse the repository at this point in the history
  • Loading branch information
@tiffanyfay
tiffanyfay committed Feb 10, 2017
1 parent 8aef474 commit 060a6de
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 42 deletions.
62 changes: 38 additions & 24 deletions ecs-cli/modules/compose/ecs/utils/convert_task_definition.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -83,6 +90,7 @@ func ConvertToTaskDefinition(taskDefinitionName string, context *project.Context
}
containerDefinitions = append(containerDefinitions, containerDef)
}

taskDefinition := &ecs.TaskDefinition{
Family: aws.String(taskDefinitionName),
ContainerDefinitions: containerDefinitions,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.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)
}
return output
}
Expand Down Expand Up @@ -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
Expand All @@ -450,13 +458,19 @@ func convertToMountPoints(cfgVolumes *yaml.Volumes, volumes map[string]string) (
}

var volumeName string
if len(volumes) > 0 {
volumeName = volumes[hostPath]
}
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(len(volumes))
volumes[hostPath] = volumeName
if volumeName == "" {
volumeName = getVolumeName(numVol)
volumes.volumeWithHost[hostPath] = volumeName
}
}

mountPoints = append(mountPoints, &ecs.MountPoint{
Expand Down
60 changes: 42 additions & 18 deletions ecs-cli/modules/compose/ecs/utils/convert_task_definition_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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 {
Expand All @@ -366,11 +372,24 @@ 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)

emptyHostCtr := 0
verifyMountPoint(t, mountPointsOut[0], volumes, "", containerPath, false, &emptyHostCtr)
verifyMountPoint(t, mountPointsOut[1], volumes, "", containerPath2, false, &emptyHostCtr)
verifyMountPoint(t, mountPointsOut[2], volumes, hostPath, containerPath, false, &emptyHostCtr)
verifyMountPoint(t, mountPointsOut[3], volumes, "", containerPath, true, &emptyHostCtr)
verifyMountPoint(t, mountPointsOut[4], volumes, hostPath, containerPath, true, &emptyHostCtr)
verifyMountPoint(t, mountPointsOut[5], volumes, hostPath, containerPath, false, &emptyHostCtr)

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)
Expand All @@ -387,18 +406,23 @@ 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]
*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)
}
}

Expand Down

0 comments on commit 060a6de

Please sign in to comment.