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 9, 2017
1 parent 8aef474 commit 4de8b48
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 40 deletions.
65 changes: 40 additions & 25 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 {
volumeHasHost 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{
volumeHasHost: 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.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
}
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,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{
Expand Down
54 changes: 39 additions & 15 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,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
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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)
}
}

Expand Down

0 comments on commit 4de8b48

Please sign in to comment.