From 66a4352581edea3c5e211b88224defd74148f00e Mon Sep 17 00:00:00 2001 From: Tim Harrold Date: Tue, 24 Oct 2023 18:14:26 +0000 Subject: [PATCH] Move utils function Uint16SliceToStringSlice to shared lib --- agent/api/ecsclient/client.go | 5 ++-- agent/api/task/task.go | 5 ++-- agent/api/task/task_test.go | 3 +- agent/engine/host_resource_manager_test.go | 7 +++-- agent/utils/utils.go | 11 ------- .../amazon-ecs-agent/ecs-agent/utils/utils.go | 16 +++++++++- ecs-agent/utils/utils.go | 16 +++++++++- ecs-agent/utils/utils_test.go | 30 +++++++++++++++++++ 8 files changed, 72 insertions(+), 21 deletions(-) diff --git a/agent/api/ecsclient/client.go b/agent/api/ecsclient/client.go index 97f321aab0e..19b02d504af 100644 --- a/agent/api/ecsclient/client.go +++ b/agent/api/ecsclient/client.go @@ -33,6 +33,7 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/ecs_client/model/ecs" "github.com/aws/amazon-ecs-agent/ecs-agent/httpclient" "github.com/aws/amazon-ecs-agent/ecs-agent/logger" + commonutils "github.com/aws/amazon-ecs-agent/ecs-agent/utils" "github.com/aws/amazon-ecs-agent/ecs-agent/utils/retry" "github.com/aws/aws-sdk-go/aws" @@ -322,12 +323,12 @@ func (client *APIECSClient) getResources() ([]*ecs.Resource, error) { portResource := ecs.Resource{ Name: utils.Strptr("PORTS"), Type: utils.Strptr("STRINGSET"), - StringSetValue: utils.Uint16SliceToStringSlice(client.config.ReservedPorts), + StringSetValue: commonutils.Uint16SliceToStringSlice(client.config.ReservedPorts), } udpPortResource := ecs.Resource{ Name: utils.Strptr("PORTS_UDP"), Type: utils.Strptr("STRINGSET"), - StringSetValue: utils.Uint16SliceToStringSlice(client.config.ReservedPortsUDP), + StringSetValue: commonutils.Uint16SliceToStringSlice(client.config.ReservedPortsUDP), } return []*ecs.Resource{&cpuResource, &memResource, &portResource, &udpPortResource}, nil diff --git a/agent/api/task/task.go b/agent/api/task/task.go index 17cfb13a10d..baa5eb51028 100644 --- a/agent/api/task/task.go +++ b/agent/api/task/task.go @@ -50,6 +50,7 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/logger/field" nlappmesh "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/appmesh" ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface" + commonutils "github.com/aws/amazon-ecs-agent/ecs-agent/utils" "github.com/aws/amazon-ecs-agent/ecs-agent/utils/arn" "github.com/aws/amazon-ecs-agent/ecs-agent/utils/ttime" @@ -3649,12 +3650,12 @@ func (task *Task) ToHostResources() map[string]*ecs.Resource { resources["PORTS_TCP"] = &ecs.Resource{ Name: utils.Strptr("PORTS_TCP"), Type: utils.Strptr("STRINGSET"), - StringSetValue: utils.Uint16SliceToStringSlice(tcpPortSet), + StringSetValue: commonutils.Uint16SliceToStringSlice(tcpPortSet), } resources["PORTS_UDP"] = &ecs.Resource{ Name: utils.Strptr("PORTS_UDP"), Type: utils.Strptr("STRINGSET"), - StringSetValue: utils.Uint16SliceToStringSlice(udpPortSet), + StringSetValue: commonutils.Uint16SliceToStringSlice(udpPortSet), } // GPU diff --git a/agent/api/task/task_test.go b/agent/api/task/task_test.go index 5747f88af82..b5959656b81 100644 --- a/agent/api/task/task_test.go +++ b/agent/api/task/task_test.go @@ -55,6 +55,7 @@ import ( mock_credentials "github.com/aws/amazon-ecs-agent/ecs-agent/credentials/mocks" "github.com/aws/amazon-ecs-agent/ecs-agent/ecs_client/model/ecs" ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface" + commonutils "github.com/aws/amazon-ecs-agent/ecs-agent/utils" "github.com/aws/aws-sdk-go/service/secretsmanager" "github.com/aws/amazon-ecs-agent/agent/taskresource/asmsecret" @@ -5178,7 +5179,7 @@ func TestToHostResources(t *testing.T) { }, { task: testTask4, - expectedResources: getTestTaskResourceMap(int64(1024), int64(512), utils.Uint16SliceToStringSlice(portsTCP), utils.Uint16SliceToStringSlice(portsUDP), []*string{}), + expectedResources: getTestTaskResourceMap(int64(1024), int64(512), commonutils.Uint16SliceToStringSlice(portsTCP), commonutils.Uint16SliceToStringSlice(portsUDP), []*string{}), }, { task: testTask5, diff --git a/agent/engine/host_resource_manager_test.go b/agent/engine/host_resource_manager_test.go index 447eb6d8af3..e2d331496f1 100644 --- a/agent/engine/host_resource_manager_test.go +++ b/agent/engine/host_resource_manager_test.go @@ -21,6 +21,7 @@ import ( "github.com/aws/amazon-ecs-agent/agent/utils" "github.com/aws/amazon-ecs-agent/ecs-agent/ecs_client/model/ecs" + commonutils "github.com/aws/amazon-ecs-agent/ecs-agent/utils" "github.com/aws/aws-sdk-go/aws" "github.com/stretchr/testify/assert" ) @@ -231,7 +232,7 @@ func TestConsumable(t *testing.T) { } for _, tc := range testCases { - resources := getTestTaskResourceMap(tc.cpu, tc.mem, utils.Uint16SliceToStringSlice(tc.ports), utils.Uint16SliceToStringSlice(tc.portsUdp), aws.StringSlice(tc.gpus)) + resources := getTestTaskResourceMap(tc.cpu, tc.mem, commonutils.Uint16SliceToStringSlice(tc.ports), commonutils.Uint16SliceToStringSlice(tc.portsUdp), aws.StringSlice(tc.gpus)) canBeConsumed, err := h.consumable(resources) assert.Equal(t, canBeConsumed, tc.canBeConsumed, "Error in checking if resources can be successfully consumed") assert.Equal(t, err, nil, "Error in checking if resources can be successfully consumed, error returned from consumable") @@ -244,7 +245,7 @@ func TestResourceHealthTrue(t *testing.T) { gpuIDs := []string{"gpu1", "gpu2", "gpu3", "gpu4"} h := getTestHostResourceManager(int64(2048), int64(2048), []*string{&hostResourcePort1}, []*string{&hostResourcePort2}, aws.StringSlice(gpuIDs)) - resources := getTestTaskResourceMap(1024, 1024, utils.Uint16SliceToStringSlice([]uint16{22}), utils.Uint16SliceToStringSlice([]uint16{1000}), aws.StringSlice([]string{"gpu1", "gpu2"})) + resources := getTestTaskResourceMap(1024, 1024, commonutils.Uint16SliceToStringSlice([]uint16{22}), commonutils.Uint16SliceToStringSlice([]uint16{1000}), aws.StringSlice([]string{"gpu1", "gpu2"})) err := h.checkResourcesHealth(resources) assert.NoError(t, err, "Error in checking healthy resource map status") } @@ -256,7 +257,7 @@ func TestResourceHealthGPUFalse(t *testing.T) { gpuIDs := []string{"gpu1", "gpu2", "gpu3", "gpu4"} h := getTestHostResourceManager(int64(2048), int64(2048), []*string{&hostResourcePort1}, []*string{&hostResourcePort2}, aws.StringSlice(gpuIDs)) - resources := getTestTaskResourceMap(1024, 1024, utils.Uint16SliceToStringSlice([]uint16{22}), utils.Uint16SliceToStringSlice([]uint16{1000}), aws.StringSlice([]string{"gpu1", "gpu5"})) + resources := getTestTaskResourceMap(1024, 1024, commonutils.Uint16SliceToStringSlice([]uint16{22}), commonutils.Uint16SliceToStringSlice([]uint16{1000}), aws.StringSlice([]string{"gpu1", "gpu5"})) err := h.checkResourcesHealth(resources) assert.Error(t, err, "Error in checking unhealthy resource map status") } diff --git a/agent/utils/utils.go b/agent/utils/utils.go index e2a5b19565e..3b7ae0e6f4e 100644 --- a/agent/utils/utils.go +++ b/agent/utils/utils.go @@ -98,17 +98,6 @@ func BoolPtr(b bool) *bool { return &b } -// Uint16SliceToStringSlice converts a slice of type uint16 to a slice of type -// *string. It uses strconv.Itoa on each element -func Uint16SliceToStringSlice(slice []uint16) []*string { - stringSlice := make([]*string, len(slice)) - for i, el := range slice { - str := strconv.Itoa(int(el)) - stringSlice[i] = &str - } - return stringSlice -} - func StrSliceEqual(s1, s2 []string) bool { if len(s1) != len(s2) { return false diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/utils/utils.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/utils/utils.go index 7ac89d29750..fa4581a5d30 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/utils/utils.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/utils/utils.go @@ -12,7 +12,10 @@ // permissions and limitations under the License. package utils -import "reflect" +import ( + "reflect" + "strconv" +) func ZeroOrNil(obj interface{}) bool { value := reflect.ValueOf(obj) @@ -35,3 +38,14 @@ func ZeroOrNil(obj interface{}) bool { } return false } + +// Uint16SliceToStringSlice converts a slice of type uint16 to a slice of type +// *string. It uses strconv.Itoa on each element +func Uint16SliceToStringSlice(slice []uint16) []*string { + stringSlice := make([]*string, len(slice)) + for i, el := range slice { + str := strconv.Itoa(int(el)) + stringSlice[i] = &str + } + return stringSlice +} diff --git a/ecs-agent/utils/utils.go b/ecs-agent/utils/utils.go index 7ac89d29750..fa4581a5d30 100644 --- a/ecs-agent/utils/utils.go +++ b/ecs-agent/utils/utils.go @@ -12,7 +12,10 @@ // permissions and limitations under the License. package utils -import "reflect" +import ( + "reflect" + "strconv" +) func ZeroOrNil(obj interface{}) bool { value := reflect.ValueOf(obj) @@ -35,3 +38,14 @@ func ZeroOrNil(obj interface{}) bool { } return false } + +// Uint16SliceToStringSlice converts a slice of type uint16 to a slice of type +// *string. It uses strconv.Itoa on each element +func Uint16SliceToStringSlice(slice []uint16) []*string { + stringSlice := make([]*string, len(slice)) + for i, el := range slice { + str := strconv.Itoa(int(el)) + stringSlice[i] = &str + } + return stringSlice +} diff --git a/ecs-agent/utils/utils_test.go b/ecs-agent/utils/utils_test.go index 2ffecf4fae0..cc6a7697401 100644 --- a/ecs-agent/utils/utils_test.go +++ b/ecs-agent/utils/utils_test.go @@ -13,6 +13,7 @@ package utils import ( + "strconv" "testing" "github.com/stretchr/testify/assert" @@ -59,3 +60,32 @@ func TestZeroOrNil(t *testing.T) { } } + +// TestUint16SliceToStringSlice tests the utils method Uint16SliceToStringSlice +// By taking in a slice of untyped 16 bit ints, asserting the util function +// returns the correct size of array, and asserts their equality. +// This is done by re-converting the string into a uint16. +func TestUint16SliceToStringSlice(t *testing.T) { + testCases := []struct { + param []uint16 + expected int + name string + }{ + {nil, 0, "Nil argument"}, + {[]uint16{0, 1, 2, 3}, 4, "Basic set"}, + {[]uint16{65535, 65535}, 2, "Max Value"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + output := Uint16SliceToStringSlice(tc.param) + assert.Equal(t, tc.expected, len(output), tc.name) + for idx, num := range tc.param { + reconverted, err := strconv.Atoi(*output[idx]) + assert.NoError(t, err) + assert.Equal(t, num, uint16(reconverted)) + } + + }) + } +}