Skip to content

Commit 9377f44

Browse files
allisaurusallisaurus
committed
Refactor convert_task_definition to use params struct
1 parent a426586 commit 9377f44

File tree

3 files changed

+92
-27
lines changed

3 files changed

+92
-27
lines changed

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -186,14 +186,16 @@ func (p *ecsProject) transformTaskDefinition() error {
186186
requiredCompatibilities := ecsContext.CommandConfig.LaunchType
187187
taskDefinitionName := ecsContext.ProjectName
188188

189-
taskDefinition, err := utils.ConvertToTaskDefinition(
190-
taskDefinitionName,
191-
p.VolumeConfigs(),
192-
p.ContainerConfigs(), // TODO Change to pointer on project?
193-
taskRoleArn,
194-
requiredCompatibilities,
195-
ecsContext.ECSParams,
196-
)
189+
convertParams := utils.ConvertTaskDefParams{
190+
TaskDefName: taskDefinitionName,
191+
TaskRoleArn: taskRoleArn,
192+
RequiredCompatibilites: requiredCompatibilities,
193+
Volumes: p.VolumeConfigs(),
194+
ContainerConfigs: p.ContainerConfigs(), // TODO Change to pointer on project?
195+
ECSParams: ecsContext.ECSParams,
196+
}
197+
198+
taskDefinition, err := utils.ConvertToTaskDefinition(convertParams)
197199

198200
if err != nil {
199201
return err

‎ecs-cli/modules/utils/compose/convert_task_definition.go

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,28 +37,37 @@ type TaskDefParams struct {
3737
executionRoleArn string
3838
}
3939

40+
// ConvertTaskDefParams contains the inputs required to convert compose & ECS inputs into an ECS task definition
41+
type ConvertTaskDefParams struct {
42+
TaskDefName string
43+
TaskRoleArn string
44+
RequiredCompatibilites string
45+
Volumes *adapter.Volumes
46+
ContainerConfigs []adapter.ContainerConfig
47+
ECSParams *ECSParams
48+
}
49+
4050
// ConvertToTaskDefinition transforms the yaml configs to its ecs equivalent (task definition)
41-
func ConvertToTaskDefinition(taskDefinitionName string, volumes *adapter.Volumes,
42-
containerConfigs []adapter.ContainerConfig, taskRoleArn string, requiredCompatibilites string, ecsParams *ECSParams) (*ecs.TaskDefinition, error) {
43-
if len(containerConfigs) == 0 {
51+
func ConvertToTaskDefinition(params ConvertTaskDefParams) (*ecs.TaskDefinition, error) {
52+
if len(params.ContainerConfigs) == 0 {
4453
return nil, errors.New("cannot create a task definition with no containers; invalid service config")
4554
}
4655

4756
// Instantiates zero values for fields on task def specified by ecs-params
48-
taskDefParams, err := convertTaskDefParams(ecsParams)
57+
taskDefParams, err := convertTaskDefParams(params.ECSParams)
4958
if err != nil {
5059
return nil, err
5160
}
5261

5362
// The task-role-arn flag takes precedence over a taskRoleArn value specified in ecs-params file.
54-
if taskRoleArn == "" {
55-
taskRoleArn = taskDefParams.taskRoleArn
63+
if params.TaskRoleArn == "" {
64+
params.TaskRoleArn = taskDefParams.taskRoleArn
5665
}
5766

5867
// Create containerDefinitions
5968
containerDefinitions := []*ecs.ContainerDefinition{}
6069

61-
for _, containerConfig := range containerConfigs {
70+
for _, containerConfig := range params.ContainerConfigs {
6271
name := containerConfig.Name
6372
// Check if there are ecs-params specified for the container
6473
ecsContainerDef := &ContainerDef{Essential: true}
@@ -67,7 +76,7 @@ func ConvertToTaskDefinition(taskDefinitionName string, volumes *adapter.Volumes
6776
}
6877

6978
// Validate essential containers
70-
count := len(containerConfigs)
79+
count := len(params.ContainerConfigs)
7180
if !hasEssential(taskDefParams.containerDefs, count) {
7281
return nil, errors.New("Task definition does not have any essential containers")
7382
}
@@ -80,25 +89,25 @@ func ConvertToTaskDefinition(taskDefinitionName string, volumes *adapter.Volumes
8089
containerDefinitions = append(containerDefinitions, containerDef)
8190
}
8291

83-
ecsVolumes, err := convertToECSVolumes(volumes, ecsParams)
92+
ecsVolumes, err := convertToECSVolumes(params.Volumes, params.ECSParams)
8493
if err != nil {
8594
return nil, err
8695
}
8796

8897
taskDefinition := &ecs.TaskDefinition{
89-
Family: aws.String(taskDefinitionName),
98+
Family: aws.String(params.TaskDefName),
9099
ContainerDefinitions: containerDefinitions,
91100
Volumes: ecsVolumes,
92-
TaskRoleArn: aws.String(taskRoleArn),
101+
TaskRoleArn: aws.String(params.TaskRoleArn),
93102
NetworkMode: aws.String(taskDefParams.networkMode),
94103
Cpu: aws.String(taskDefParams.cpu),
95104
Memory: aws.String(taskDefParams.memory),
96105
ExecutionRoleArn: aws.String(taskDefParams.executionRoleArn),
97106
}
98107

99108
// Set launch type
100-
if requiredCompatibilites != "" {
101-
taskDefinition.RequiresCompatibilities = []*string{aws.String(requiredCompatibilites)}
109+
if params.RequiredCompatibilites != "" {
110+
taskDefinition.RequiresCompatibilities = []*string{aws.String(params.RequiredCompatibilites)}
102111
}
103112

104113
return taskDefinition, nil

‎ecs-cli/modules/utils/compose/convert_task_definition_test.go

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,7 +1168,16 @@ func TestMemReservationHigherThanMemLimit(t *testing.T) {
11681168
volumeConfigs := adapter.NewVolumes()
11691169
containerConfigs := []adapter.ContainerConfig{containerConfig}
11701170

1171-
_, err := ConvertToTaskDefinition(projectName, volumeConfigs, containerConfigs, "", "", nil)
1171+
testParams := ConvertTaskDefParams{
1172+
TaskDefName: projectName,
1173+
TaskRoleArn: "",
1174+
RequiredCompatibilites: "",
1175+
Volumes: volumeConfigs,
1176+
ContainerConfigs: containerConfigs,
1177+
ECSParams: nil,
1178+
}
1179+
1180+
_, err := ConvertToTaskDefinition(testParams)
11721181
assert.EqualError(t, err, "mem_limit must be greater than mem_reservation")
11731182
}
11741183

@@ -1209,7 +1218,16 @@ func TestConvertToTaskDefinitionWithVolumes(t *testing.T) {
12091218
},
12101219
}
12111220

1212-
taskDefinition, err := ConvertToTaskDefinition(projectName, volumeConfigs, containerConfigs, "", "", nil)
1221+
testParams := ConvertTaskDefParams{
1222+
TaskDefName: projectName,
1223+
TaskRoleArn: "",
1224+
RequiredCompatibilites: "",
1225+
Volumes: volumeConfigs,
1226+
ContainerConfigs: containerConfigs,
1227+
ECSParams: nil,
1228+
}
1229+
1230+
taskDefinition, err := ConvertToTaskDefinition(testParams)
12131231
assert.NoError(t, err, "Unexpected error converting Task Definition")
12141232

12151233
actualVolumes := taskDefinition.Volumes
@@ -1244,7 +1262,16 @@ func TestConvertToTaskDefinitionWithVolumesWithHostOnly(t *testing.T) {
12441262
},
12451263
}
12461264

1247-
taskDefinition, err := ConvertToTaskDefinition(projectName, volumeConfigs, containerConfigs, "", "", nil)
1265+
testParams := ConvertTaskDefParams{
1266+
TaskDefName: projectName,
1267+
TaskRoleArn: "",
1268+
RequiredCompatibilites: "",
1269+
Volumes: volumeConfigs,
1270+
ContainerConfigs: containerConfigs,
1271+
ECSParams: nil,
1272+
}
1273+
1274+
taskDefinition, err := ConvertToTaskDefinition(testParams)
12481275
assert.NoError(t, err, "Unexpected error converting Task Definition")
12491276

12501277
actualVolumes := taskDefinition.Volumes
@@ -1295,7 +1322,16 @@ func TestConvertToTaskDefinitionWithECSParamsVolumeWithoutNameError(t *testing.T
12951322
},
12961323
}
12971324

1298-
_, err := ConvertToTaskDefinition(projectName, volumeConfigs, containerConfigs, "", "", ecsParams)
1325+
testParams := ConvertTaskDefParams{
1326+
TaskDefName: projectName,
1327+
TaskRoleArn: "",
1328+
RequiredCompatibilites: "",
1329+
Volumes: volumeConfigs,
1330+
ContainerConfigs: containerConfigs,
1331+
ECSParams: ecsParams,
1332+
}
1333+
1334+
_, err := ConvertToTaskDefinition(testParams)
12991335
assert.Error(t, err, "Expected error converting Task Definition with ECS Params volume without name")
13001336
}
13011337

@@ -1474,7 +1510,16 @@ func convertToTaskDefinitionInTest(t *testing.T, containerConfig *adapter.Contai
14741510
containerConfigs := []adapter.ContainerConfig{}
14751511
containerConfigs = append(containerConfigs, *containerConfig)
14761512

1477-
taskDefinition, err := ConvertToTaskDefinition(projectName, volumeConfigs, containerConfigs, taskRoleArn, launchType, nil)
1513+
testParams := ConvertTaskDefParams{
1514+
TaskDefName: projectName,
1515+
TaskRoleArn: taskRoleArn,
1516+
RequiredCompatibilites: launchType,
1517+
Volumes: volumeConfigs,
1518+
ContainerConfigs: containerConfigs,
1519+
ECSParams: nil,
1520+
}
1521+
1522+
taskDefinition, err := ConvertToTaskDefinition(testParams)
14781523
if err != nil {
14791524
t.Errorf("Expected to convert [%v] containerConfigs without errors. But got [%v]", containerConfig, err)
14801525
}
@@ -1486,7 +1531,16 @@ func convertToTaskDefWithEcsParamsInTest(t *testing.T, containerConfigs []adapte
14861531
VolumeEmptyHost: []string{namedVolume},
14871532
}
14881533

1489-
taskDefinition, err := ConvertToTaskDefinition(projectName, volumeConfigs, containerConfigs, taskRoleArn, "", ecsParams)
1534+
testParams := ConvertTaskDefParams{
1535+
TaskDefName: projectName,
1536+
TaskRoleArn: taskRoleArn,
1537+
RequiredCompatibilites: "",
1538+
Volumes: volumeConfigs,
1539+
ContainerConfigs: containerConfigs,
1540+
ECSParams: ecsParams,
1541+
}
1542+
1543+
taskDefinition, err := ConvertToTaskDefinition(testParams)
14901544
if err != nil {
14911545
return nil, err
14921546
}

0 commit comments

Comments
 (0)