Skip to content

Commit

Permalink
make consumable() return a list of non-consumable resource keys
Browse files Browse the repository at this point in the history
  • Loading branch information
@singholt
singholt committed Sep 11, 2024
1 parent edd5f2e commit a1d9a39
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 66 deletions.
26 changes: 16 additions & 10 deletions agent/engine/host_resource_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (h *HostResourceManager) consume(taskArn string, resources map[string]*ecs.
return true, nil
}

ok, failedResourceKey, err := h.consumable(resources)
ok, failedResourceKeys, err := h.consumable(resources)
if err != nil {
logger.Error("Resources failing to consume, error in task resources", logger.Fields{
"taskArn": taskArn,
Expand All @@ -140,8 +140,8 @@ func (h *HostResourceManager) consume(taskArn string, resources map[string]*ecs.
return true, nil
}
logger.Info("Resources not consumed, enough resources not available", logger.Fields{
"taskArn": taskArn,
"resource": failedResourceKey,
"taskArn": taskArn,
"resources": failedResourceKeys,
})
return false, nil
}
Expand Down Expand Up @@ -254,31 +254,37 @@ func (h *HostResourceManager) checkResourcesHealth(resources map[string]*ecs.Res
}

// Helper function for consume to check if resources are consumable with the current account
// we have for the host resources. Should not call host resource manager lock in this func
// return values
func (h *HostResourceManager) consumable(resources map[string]*ecs.Resource) (bool, string, error) {
// we have for the host resources. Should not call host resource manager lock in this func return values
// This function returns a bool (indicating whether ALL requested resources are consumable), a list of non-consumable
// resource keys, and error, if any.
func (h *HostResourceManager) consumable(resources map[string]*ecs.Resource) (bool, []string, error) {
err := h.checkResourcesHealth(resources)
if err != nil {
return false, "", err
return false, nil, err
}

var resourcesNotConsumable []string
for resourceKey := range resources {
if *resources[resourceKey].Type == "INTEGER" {
consumable := h.checkConsumableIntType(resourceKey, resources)
if !consumable {
return false, resourceKey, nil
resourcesNotConsumable = append(resourcesNotConsumable, resourceKey)
}
}

if *resources[resourceKey].Type == "STRINGSET" {
consumable := h.checkConsumableStringSetType(resourceKey, resources)
if !consumable {
return false, resourceKey, nil
resourcesNotConsumable = append(resourcesNotConsumable, resourceKey)
}
}
}

return true, "", nil
if resourcesNotConsumable != nil {
return false, resourcesNotConsumable, nil
} else {
return true, nil, nil
}
}

// Utility function to manage release of ports
Expand Down
125 changes: 69 additions & 56 deletions agent/engine/host_resource_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package engine

import (
"reflect"
"testing"

"github.com/aws/amazon-ecs-agent/agent/utils"
Expand Down Expand Up @@ -184,76 +185,88 @@ func TestConsumable(t *testing.T) {
[]*string{&hostResourcePort2}, aws.StringSlice(gpuIDs))

testCases := []struct {
name string
cpu int64
mem int64
ports []uint16
portsUdp []uint16
gpus []string
canBeConsumed bool
expectedFailedResourceKey string
name string
cpu int64
mem int64
ports []uint16
portsUdp []uint16
gpus []string
canBeConsumed bool
expectedFailedResourceKeys []string
}{
{
name: "consumable",
cpu: int64(1024),
mem: int64(1024),
ports: []uint16{25},
portsUdp: []uint16{1003},
gpus: []string{"gpu1", "gpu2"},
canBeConsumed: true,
expectedFailedResourceKey: "",
name: "consumable",
cpu: int64(1024),
mem: int64(1024),
ports: []uint16{25},
portsUdp: []uint16{1003},
gpus: []string{"gpu1", "gpu2"},
canBeConsumed: true,
expectedFailedResourceKeys: nil,
},
{
name: "cpu not consumable",
cpu: int64(2500),
mem: int64(1024),
ports: []uint16{},
portsUdp: []uint16{},
gpus: []string{},
canBeConsumed: false,
expectedFailedResourceKey: "CPU",
name: "cpu not consumable",
cpu: int64(2500),
mem: int64(1024),
ports: []uint16{},
portsUdp: []uint16{},
gpus: []string{},
canBeConsumed: false,
expectedFailedResourceKeys: []string{"CPU"},
},
{
name: "memory not consumable",
cpu: int64(1024),
mem: int64(2500),
ports: []uint16{},
portsUdp: []uint16{},
gpus: []string{},
canBeConsumed: false,
expectedFailedResourceKey: "MEMORY",
name: "memory not consumable",
cpu: int64(1024),
mem: int64(2500),
ports: []uint16{},
portsUdp: []uint16{},
gpus: []string{},
canBeConsumed: false,
expectedFailedResourceKeys: []string{"MEMORY"},
},
{
name: "tcp ports not consumable",
cpu: int64(1024),
mem: int64(1024),
ports: []uint16{22},
portsUdp: []uint16{},
gpus: []string{},
canBeConsumed: false,
expectedFailedResourceKey: "PORTS_TCP",
name: "tcp ports not consumable",
cpu: int64(1024),
mem: int64(1024),
ports: []uint16{22},
portsUdp: []uint16{},
gpus: []string{},
canBeConsumed: false,
expectedFailedResourceKeys: []string{"PORTS_TCP"},
},
{
name: "udp ports not consumable",
cpu: int64(1024),
mem: int64(1024),
ports: []uint16{},
portsUdp: []uint16{1000},
gpus: []string{},
canBeConsumed: false,
expectedFailedResourceKey: "PORTS_UDP",
name: "udp ports not consumable",
cpu: int64(1024),
mem: int64(1024),
ports: []uint16{},
portsUdp: []uint16{1000},
gpus: []string{},
canBeConsumed: false,
expectedFailedResourceKeys: []string{"PORTS_UDP"},
},
{
name: "multiple resources not consumable - cpu and udp ports",
cpu: int64(2500),
mem: int64(1024),
ports: []uint16{},
portsUdp: []uint16{1000},
gpus: []string{},
canBeConsumed: false,
expectedFailedResourceKeys: []string{"CPU", "PORTS_UDP"},
},
}

for _, tc := range testCases {
resources := getTestTaskResourceMap(tc.cpu, tc.mem, commonutils.Uint16SliceToStringSlice(tc.ports),
commonutils.Uint16SliceToStringSlice(tc.portsUdp), aws.StringSlice(tc.gpus))
canBeConsumed, failedResourceKey, err := h.consumable(resources)
assert.Equal(t, tc.canBeConsumed, canBeConsumed,
"Error in checking if resources can be successfully consumed")
assert.Equal(t, nil, err,
"Error in checking if resources can be successfully consumed, error returned from consumable")
assert.Equal(t, tc.expectedFailedResourceKey, failedResourceKey)
t.Run(tc.name, func(t *testing.T) {
resources := getTestTaskResourceMap(tc.cpu, tc.mem, commonutils.Uint16SliceToStringSlice(tc.ports),
commonutils.Uint16SliceToStringSlice(tc.portsUdp), aws.StringSlice(tc.gpus))
canBeConsumed, failedResourceKeys, err := h.consumable(resources)
assert.Equal(t, tc.canBeConsumed, canBeConsumed,
"Error in checking if resources can be successfully consumed")
assert.Equal(t, nil, err,
"Error in checking if resources can be successfully consumed, error returned from consumable")
assert.True(t, reflect.DeepEqual(tc.expectedFailedResourceKeys, failedResourceKeys))
})
}
}

Expand Down

0 comments on commit a1d9a39

Please sign in to comment.