Skip to content

Commit 3876b7c

Browse files
SoManyHsSoManyHs
committed
Fix log message for resource overrides
Also some cleanup/reordering in tests
1 parent a97def5 commit 3876b7c

File tree

2 files changed

+51
-54
lines changed

2 files changed

+51
-54
lines changed

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

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ const (
2525
defaultMemLimit = 512
2626
)
2727

28-
// TaskDefParams contains basic fields to build an
29-
// ECS task definition
28+
// TaskDefParams contains basic fields to build an ECS task definition
3029
type TaskDefParams struct {
3130
networkMode string
3231
taskRoleArn string
@@ -58,15 +57,14 @@ func ConvertToTaskDefinition(taskDefinitionName string, volumes *adapter.Volumes
5857
containerDefinitions := []*ecs.ContainerDefinition{}
5958

6059
for _, containerConfig := range containerConfigs {
61-
// logUnsupportedServiceConfigFields(name, serviceConfig) // TODO switch this to use ContainerConfig
6260
name := containerConfig.Name
6361
// Check if there are ecs-params specified for the container
6462
ecsContainerDef := &ContainerDef{Essential: true}
6563
if cd, ok := taskDefParams.containerDefs[name]; ok {
6664
ecsContainerDef = &cd
6765
}
6866

69-
// Validate essential containers TODO: merge other ecs params fields
67+
// Validate essential containers
7068
count := len(containerConfigs)
7169
if !hasEssential(taskDefParams.containerDefs, count) {
7270
return nil, errors.New("Task definition does not have any essential containers")
@@ -168,13 +166,13 @@ func convertToContainerDef(inputCfg *adapter.ContainerConfig, ecsContainerDef *C
168166
if ecsContainerDef != nil {
169167
outputContDef.Essential = aws.Bool(ecsContainerDef.Essential)
170168

171-
cpu = getResourceValue(inputCfg.Name, cpu, ecsContainerDef.Cpu)
169+
cpu = getResourceValue(inputCfg.Name, cpu, ecsContainerDef.Cpu, "CPU")
172170

173171
ecsMemInMB := adapter.ConvertToMemoryInMB(int64(ecsContainerDef.Memory))
174-
mem = getResourceValue(inputCfg.Name, mem, ecsMemInMB)
172+
mem = getResourceValue(inputCfg.Name, mem, ecsMemInMB, "MemoryLimit")
175173

176174
ecsMemResInMB := adapter.ConvertToMemoryInMB(int64(ecsContainerDef.MemoryReservation))
177-
memRes = getResourceValue(inputCfg.Name, memRes, ecsMemResInMB)
175+
memRes = getResourceValue(inputCfg.Name, memRes, ecsMemResInMB, "MemoryReservation")
178176
}
179177

180178
if mem < memRes {
@@ -199,21 +197,21 @@ func convertToContainerDef(inputCfg *adapter.ContainerConfig, ecsContainerDef *C
199197
return outputContDef, nil
200198
}
201199

202-
func getResourceValue(serviceName string, inputVal, ecsVal int64) int64 {
200+
func getResourceValue(serviceName string, inputVal, ecsVal int64, option string) int64 {
203201
if ecsVal == 0 {
204202
return inputVal
205203
}
206204
if inputVal > 0 {
207-
showResourceOverrideMsg(serviceName, inputVal, ecsVal)
205+
showResourceOverrideMsg(serviceName, inputVal, ecsVal, option)
208206
}
209207
return ecsVal
210208
}
211209

212-
func showResourceOverrideMsg(serviceName string, oldValue int64, newValue int64) {
210+
func showResourceOverrideMsg(serviceName string, oldValue int64, newValue int64, option string) {
213211
overrideMsg := "Using ecs-params value as override (was %v but is now %v)"
214212

215213
log.WithFields(log.Fields{
216-
"option name": "MemoryReservation",
214+
"option name": option,
217215
"service name": serviceName,
218216
}).Infof(overrideMsg, oldValue, newValue)
219217
}

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

Lines changed: 42 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -266,45 +266,6 @@ func TestConvertToTaskDefinition(t *testing.T) {
266266
}
267267
}
268268

269-
// ConvertToContainerDefinition tests
270-
271-
func TestConvertToTaskDefinitionWithECSParams_DefaultMemoryLessThanMemoryRes(t *testing.T) {
272-
// set up containerConfig w/o value for Memory
273-
containerConfig := &adapter.ContainerConfig{
274-
Name: "web",
275-
Image: "httpd",
276-
CPU: int64(5),
277-
}
278-
279-
// define ecs-params value we expect to be present in final containerDefinition
280-
ecsParamsString := `version: 1
281-
task_definition:
282-
services:
283-
web:
284-
mem_reservation: 1g`
285-
286-
content := []byte(ecsParamsString)
287-
288-
tmpfile, err := ioutil.TempFile("", "ecs-params")
289-
assert.NoError(t, err, "Could not create ecs params tempfile")
290-
291-
defer os.Remove(tmpfile.Name())
292-
293-
_, err = tmpfile.Write(content)
294-
assert.NoError(t, err, "Could not write data to ecs params tempfile")
295-
296-
err = tmpfile.Close()
297-
assert.NoError(t, err, "Could not close tempfile")
298-
299-
ecsParamsFileName := tmpfile.Name()
300-
ecsParams, err := ReadECSParams(ecsParamsFileName)
301-
assert.NoError(t, err, "Could not read ECS Params file")
302-
303-
containerConfigs := []adapter.ContainerConfig{*containerConfig}
304-
_, err = convertToTaskDefWithEcsParamsInTest(t, containerConfigs, "", ecsParams)
305-
306-
assert.Error(t, err, "Expected error because memory reservation is greater than memory limit")
307-
}
308269
func TestConvertToTaskDefinitionWithNoSharedMemorySize(t *testing.T) {
309270
containerConfig := &adapter.ContainerConfig{
310271
ShmSize: int64(0),
@@ -370,7 +331,45 @@ func TestConvertToTaskDefinitionLaunchTypeFargate(t *testing.T) {
370331
assert.Equal(t, "FARGATE", aws.StringValue(taskDefinition.RequiresCompatibilities[0]))
371332
}
372333

373-
// Test Conversion with ECS Params
334+
// Tests for ConvertToContainerDefinition with ECS Params
335+
func TestConvertToTaskDefinitionWithECSParams_DefaultMemoryLessThanMemoryRes(t *testing.T) {
336+
// set up containerConfig w/o value for Memory
337+
containerConfig := &adapter.ContainerConfig{
338+
Name: "web",
339+
Image: "httpd",
340+
CPU: int64(5),
341+
}
342+
343+
// define ecs-params value we expect to be present in final containerDefinition
344+
ecsParamsString := `version: 1
345+
task_definition:
346+
services:
347+
web:
348+
mem_reservation: 1g`
349+
350+
content := []byte(ecsParamsString)
351+
352+
tmpfile, err := ioutil.TempFile("", "ecs-params")
353+
assert.NoError(t, err, "Could not create ecs params tempfile")
354+
355+
defer os.Remove(tmpfile.Name())
356+
357+
_, err = tmpfile.Write(content)
358+
assert.NoError(t, err, "Could not write data to ecs params tempfile")
359+
360+
err = tmpfile.Close()
361+
assert.NoError(t, err, "Could not close tempfile")
362+
363+
ecsParamsFileName := tmpfile.Name()
364+
ecsParams, err := ReadECSParams(ecsParamsFileName)
365+
assert.NoError(t, err, "Could not read ECS Params file")
366+
367+
containerConfigs := []adapter.ContainerConfig{*containerConfig}
368+
_, err = convertToTaskDefWithEcsParamsInTest(t, containerConfigs, "", ecsParams)
369+
370+
assert.Error(t, err, "Expected error because memory reservation is greater than memory limit")
371+
}
372+
374373
func testContainerConfigs(names []string) []adapter.ContainerConfig {
375374
containerConfigs := []adapter.ContainerConfig{}
376375
for _, name := range names {
@@ -419,7 +418,7 @@ task_definition:
419418
}
420419
}
421420

422-
func TestConvertToTaskDefinition_WithECSParamsAllFields(t *testing.T) {
421+
func TestConvertToTaskDefinitionWithECSParams_AllFields(t *testing.T) {
423422
containerConfigs := testContainerConfigs([]string{"mysql", "wordpress"})
424423
ecsParamsString := `version: 1
425424
task_definition:
@@ -803,7 +802,7 @@ func TestConvertToTaskDefinitionWithECSParams_ContainerResourcesPresent(t *testi
803802
ecsParamsString := `version: 1
804803
task_definition:
805804
services:
806-
mysql:
805+
mysql:
807806
cpu_shares: 100
808807
mem_limit: 15m
809808
mem_reservation: 10m
@@ -1026,7 +1025,7 @@ task_definition:
10261025
assert.Error(t, err, "Expected error if mem_reservation was more than mem_limit")
10271026
}
10281027

1029-
func TestConvertToTaskDefinition_WithTaskSize(t *testing.T) {
1028+
func TestConvertToTaskDefinitionWithECSParams_WithTaskSize(t *testing.T) {
10301029
containerConfigs := testContainerConfigs([]string{"mysql", "wordpress"})
10311030
ecsParamsString := `version: 1
10321031
task_definition:

0 commit comments

Comments
 (0)