in hashing task definitions also sort containers by Name

dicksbu · PettitWesley · commit bfea559ba18a · 2018-06-19T16:54:08.000-07:00
diff --git a/ecs-cli/modules/cli/compose/adapter/convert.go b/ecs-cli/modules/cli/compose/adapter/convert.go
@@ -18,6 +18,7 @@ import (
 	"fmt"
 	"reflect"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -504,6 +505,17 @@ func SortedGoString(v interface{}) (string, error) {
 	return string(b), nil
 }
 
+// Necessary in conjunction with SortedGoString to avoid spurious tdcache misses
+func SortedContainerDefinitionsByName(request *ecs.RegisterTaskDefinitionInput) ecs.RegisterTaskDefinitionInput {
+	cdefs := request.ContainerDefinitions
+	sort.Slice(cdefs, func(i, j int) bool {
+		return *cdefs[i].Name < *cdefs[j].Name
+	})
+	sorted := *request
+	sorted.ContainerDefinitions = cdefs
+	return sorted
+}
+
 // ConvertCamelCaseToUnderScore returns an underscore-separated name for a given camelcased string
 // e.g., "NetworkMode" -> "network_mode"
 func ConvertCamelCaseToUnderScore(s string) string {
diff --git a/ecs-cli/modules/cli/compose/adapter/convert_test.go b/ecs-cli/modules/cli/compose/adapter/convert_test.go
@@ -15,7 +15,9 @@ package adapter
 
 import (
 	"fmt"
+	"math/rand"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -563,6 +565,48 @@ func TestConvertToVolumes_ErrorsWithExternalSubfield(t *testing.T) {
 	assert.Error(t, err, "Expected error converting libcompose volume configs when external is specified")
 }
 
+func TestRegisterTaskDefinitionInputEquivalence(t *testing.T) {
+	family := aws.String("family1")
+	dockerLabels := map[string]string{
+		"label1":         "",
+		"com.foo.label2": "value",
+	}
+	cdefs := []*ecs.ContainerDefinition{}
+	N := 10
+	for i := 0; i < N; i++ {
+		command := make([]string, i+1)
+		for j := 0; j < i+1; j++ {
+			command[j] = strings.Repeat(string(rune(65+j)), i+1)
+		}
+		cdefs = append(cdefs, &ecs.ContainerDefinition{
+			Name:         aws.String(strings.Repeat(string(rune(65+i)), i+1)),
+			Command:      aws.StringSlice(command),
+			DockerLabels: aws.StringMap(dockerLabels),
+		})
+	}
+	inputA := ecs.RegisterTaskDefinitionInput{
+		Family:               family,
+		ContainerDefinitions: cdefs,
+	}
+
+	shuffle_cdefs := make([]*ecs.ContainerDefinition, len(cdefs))
+	for i, v := range rand.Perm(len(cdefs)) {
+		shuffle_cdefs[v] = cdefs[i]
+	}
+
+	inputB := ecs.RegisterTaskDefinitionInput{
+		ContainerDefinitions: shuffle_cdefs,
+		Family:               family,
+	}
+
+	strA, err := SortedGoString(SortedContainerDefinitionsByName(&inputA))
+	assert.NoError(t, err, "Unexpected error generating sorted map string")
+	strB, err := SortedGoString(SortedContainerDefinitionsByName(&inputB))
+	assert.NoError(t, err, "Unexpected error generating sorted map string")
+
+	assert.Equal(t, strA, strB, "Sorted inputs should match")
+}
+
 func TestSortedGoString(t *testing.T) {
 	family := aws.String("family1")
 	name := aws.String("foo")
diff --git a/ecs-cli/modules/clients/aws/ecs/client.go b/ecs-cli/modules/clients/aws/ecs/client.go
@@ -310,7 +310,7 @@ func (c *ecsClient) constructTaskDefinitionCacheHash(taskDefinition *ecs.TaskDef
 	// Get the region from the ecsClient configuration
 	region := aws.StringValue(c.config.Session.Config.Region)
 	awsUserAccountId := utils.GetAwsAccountIdFromArn(aws.StringValue(taskDefinition.TaskDefinitionArn))
-	sortedRequestString, err := adapter.SortedGoString(request)
+	sortedRequestString, err := adapter.SortedGoString(adapter.SortedContainerDefinitionsByName(request))
 	if err != nil {
 		log.WithFields(log.Fields{
 			"error": err,
