Set default for essential to be true on ContainerDef

SoManyHs · allisaurus · commit dff67f2cd8c7 · 2018-06-04T10:55:48.000-07:00
Previously, the only field on a service in ecs-params.yml was essential.
The desired behavior is that if essential is not specified, the default
is true. However, the yaml unmarshaller by default sets any unspecified
field in the yaml to the zero value, which in the case of a boolean is
false. Since we did not expect customers to specify the 'services' field
at all in the past unless they wished to specify essential to be false,
this worked. However, with the addition of other fields (CPU, Memory) on
container definitions, we need to ensure that the default for essential
is still true even when not specified.
diff --git a/ecs-cli/modules/utils/compose/convert_task_definition_test.go b/ecs-cli/modules/utils/compose/convert_task_definition_test.go
@@ -14,7 +14,6 @@
 package utils
 
 import (
-	"fmt"
 	"io/ioutil"
 	"os"
 	"reflect"
@@ -205,7 +204,6 @@ func TestConvertToTaskDefinition(t *testing.T) {
 
 	// convert
 	taskDefinition := convertToTaskDefinitionInTest(t, nil, testContainerConfig, taskRoleArn, "")
-	fmt.Printf("TASK DEF: %+v\n\n", taskDefinition)
 	containerDef := *taskDefinition.ContainerDefinitions[0]
 
 	// verify task def fields
@@ -629,6 +627,77 @@ task_definition:
 	assert.Error(t, err, "Expected error if no containers are marked essential")
 }
 
+func TestConvertToTaskDefinitionWithECSParams_EssentialDefaultsToTrueWhenNoServicesSpecified(t *testing.T) {
+	// We expect essential to be set to be true in the converter
+	containerConfigs := testContainerConfigs([]string{"mysql", "wordpress"})
+	ecsParamsString := `version: 1
+task_definition:
+  ecs_network_mode: host`
+
+	content := []byte(ecsParamsString)
+
+	tmpfile, err := ioutil.TempFile("", "ecs-params")
+	assert.NoError(t, err, "Could not create ecs fields tempfile")
+
+	defer os.Remove(tmpfile.Name())
+
+	_, err = tmpfile.Write(content)
+	assert.NoError(t, err, "Could not write data to ecs fields tempfile")
+
+	err = tmpfile.Close()
+	assert.NoError(t, err, "Could not close tempfile")
+
+	ecsParamsFileName := tmpfile.Name()
+	ecsParams, err := ReadECSParams(ecsParamsFileName)
+	assert.NoError(t, err, "Could not read ECS Params file")
+
+	taskDefinition, err := convertToTaskDefWithEcsParamsInTest(t, nil, containerConfigs, "", ecsParams)
+	assert.NoError(t, err, "Unexpected error when no containers are marked essential")
+
+	mysql := findContainerByName("mysql", taskDefinition.ContainerDefinitions)
+	assert.True(t, *mysql.Essential, "Expected mysql to be essential")
+	wordpress := findContainerByName("wordpress", taskDefinition.ContainerDefinitions)
+	assert.True(t, *wordpress.Essential, "Expected wordpressto be essential")
+}
+
+func TestConvertToTaskDefinitionWithECSParams_EssentialDefaultsToTrueWhenNotSpecified(t *testing.T) {
+	// We expect essential to be set to be true in the unmarshaller
+	containerConfigs := testContainerConfigs([]string{"mysql", "wordpress"})
+	ecsParamsString := `version: 1
+task_definition:
+  ecs_network_mode: host
+  services:
+    wordpress:
+      mem_limit: 1000000
+    mysql:
+      mem_limit: 1000000`
+
+	content := []byte(ecsParamsString)
+
+	tmpfile, err := ioutil.TempFile("", "ecs-params")
+	assert.NoError(t, err, "Could not create ecs fields tempfile")
+
+	defer os.Remove(tmpfile.Name())
+
+	_, err = tmpfile.Write(content)
+	assert.NoError(t, err, "Could not write data to ecs fields tempfile")
+
+	err = tmpfile.Close()
+	assert.NoError(t, err, "Could not close tempfile")
+
+	ecsParamsFileName := tmpfile.Name()
+	ecsParams, err := ReadECSParams(ecsParamsFileName)
+	assert.NoError(t, err, "Could not read ECS Params file")
+
+	taskDefinition, err := convertToTaskDefWithEcsParamsInTest(t, nil, containerConfigs, "", ecsParams)
+
+	assert.NoError(t, err, "Unexpected error when no containers are marked essential")
+	mysql := findContainerByName("mysql", taskDefinition.ContainerDefinitions)
+	assert.True(t, *mysql.Essential, "Expected mysql to be essential")
+	wordpress := findContainerByName("wordpress", taskDefinition.ContainerDefinitions)
+	assert.True(t, *wordpress.Essential, "Expected wordpressto be essential")
+}
+
 func TestConvertToTaskDefinitionWithECSParamsAndTaskRoleArnFlag(t *testing.T) {
 	containerConfigs := testContainerConfigs([]string{"mysql", "wordpress"})
 	ecsParamsString := `version: 1
diff --git a/ecs-cli/modules/utils/compose/ecs_params_reader.go b/ecs-cli/modules/utils/compose/ecs_params_reader.go
@@ -87,6 +87,17 @@ const (
 	Disabled AssignPublicIp = "DISABLED"
 )
 
+func (cd *ContainerDef) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	type rawContainerDef ContainerDef
+	raw := rawContainerDef{Essential: true} //  If essential is not specified, we want it to be true
+	if err := unmarshal(&raw); err != nil {
+		return err
+	}
+
+	*cd = ContainerDef(raw)
+	return nil
+}
+
 // ReadECSParams parses the ecs-params.yml file and puts it into an ECSParams struct.
 func ReadECSParams(filename string) (*ECSParams, error) {
 	if filename == "" {
