Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip Task resource accounting for Fargate 1.3.0 launch type #3896

Merged
merged 4 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions agent/api/task/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -3509,6 +3509,10 @@ func (task *Task) IsServiceConnectConnectionDraining() bool {
return task.ServiceConnectConnectionDrainingUnsafe
}

func (task *Task) IsLaunchTypeFargate() bool {
return strings.ToUpper(task.LaunchType) == "FARGATE"
}

// ToHostResources will convert a task to a map of resources which ECS takes into account when scheduling tasks on instances
// * CPU
// - If task level CPU is set, use that
Expand Down
97 changes: 97 additions & 0 deletions agent/engine/engine_unix_integ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1381,3 +1381,100 @@ func TestHostResourceManagerStopTaskNotBlockWaitingTasks(t *testing.T) {

waitFinished(t, finished, testTimeout)
}

// Test Host Resource Manager does not account Fargate tasks when started
func TestHostResourceManagerLaunchTypeBehavior(t *testing.T) {
testCases := []struct {
Name string
LaunchType string
}{
{
Name: "TestHostResourceManagerFargateLaunchTypeBehavior",
LaunchType: "FARGATE",
},
{
Name: "TestHostResourceManagerEC2LaunchTypeBehavior",
LaunchType: "EC2",
},
{
Name: "TestHostResourceManagerExternalLaunchTypeBehavior",
LaunchType: "EXTERNAL",
},
{
Name: "TestHostResourceManagerRandomLaunchTypeBehavior",
LaunchType: "RaNdOmStrInG",
},
{
Name: "TestHostResourceManagerEmptyLaunchTypeBehavior",
LaunchType: "",
},
}

for _, tc := range testCases {
prateekchaudhry marked this conversation as resolved.
Show resolved Hide resolved
t.Run(tc.Name, func(t *testing.T) {
testTimeout := 1 * time.Minute
taskEngine, done, _ := setupWithDefaultConfig(t)
defer done()

stateChangeEvents := taskEngine.StateChangeEvents()

taskArn := "IntegTaskArn"
testTask := createTestTask(taskArn)
testTask.Memory = int64(768)
testTask.LaunchType = tc.LaunchType

// create container
taskContainer := createTestContainerWithImageAndName(baseImageForOS, "SleepWithTrap")
taskContainer.EntryPoint = &entryPointForOS
taskContainer.Command = []string{"trap shortsleep SIGTERM; shortsleep() { sleep 6; exit 1; }; sleep 10"}
taskContainer.Essential = true
taskContainer.StopTimeout = uint(6)
testTask.Containers = []*apicontainer.Container{
taskContainer,
}

// Stop task payloads from ACS for the tasks
stopTask := createTestTask("IntegTaskArn")
stopTask.DesiredStatusUnsafe = apitaskstatus.TaskStopped
stopTask.Containers = []*apicontainer.Container{}

// goroutine to schedule tasks
go func() {
taskEngine.AddTask(testTask)
time.Sleep(2 * time.Second)

// single managedTask which should have started
assert.Equal(t, 1, len(taskEngine.(*DockerTaskEngine).managedTasks), "exactly one task should be running")

// stopTask - stop running task, this task will go to STOPPING due to trap handler defined and STOPPED after 6s
taskEngine.AddTask(stopTask)
}()

finished := make(chan interface{})

// goroutine to verify task running order and verify assertions
go func() {
// Task goes to RUNNING
verifyContainerRunningStateChange(t, taskEngine)
verifyTaskIsRunning(stateChangeEvents, testTask)

time.Sleep(2500 * time.Millisecond)

// At this time, stopTask is received, and SIGTERM sent to task
// but the task is still RUNNING due to trap handler
assert.Equal(t, apitaskstatus.TaskRunning, testTask.GetKnownStatus(), "task known status should be RUNNING")
assert.Equal(t, apitaskstatus.TaskStopped, testTask.GetDesiredStatus(), "task desired status should be STOPPED")
// Verify resources are properly consumed in host resource manager, and not consumed for Fargate
if tc.LaunchType == "FARGATE" {
assert.False(t, taskEngine.(*DockerTaskEngine).hostResourceManager.checkTaskConsumed(testTask.Arn), "fargate task resources should not be consumed")
} else {
assert.True(t, taskEngine.(*DockerTaskEngine).hostResourceManager.checkTaskConsumed(testTask.Arn), "non fargate task resources should be consumed")
}
time.Sleep(2 * time.Second)
close(finished)
}()

waitFinished(t, finished, testTimeout)
})
}
}
3 changes: 2 additions & 1 deletion agent/engine/task_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,9 @@ func (mtask *managedTask) waitForHostResources() {
return
}

if !mtask.IsInternal && !mtask.engine.hostResourceManager.checkTaskConsumed(mtask.Arn) {
if !mtask.IsLaunchTypeFargate() && !mtask.IsInternal && !mtask.engine.hostResourceManager.checkTaskConsumed(mtask.Arn) {
amogh09 marked this conversation as resolved.
Show resolved Hide resolved
// Internal tasks are started right away as their resources are not accounted for
// Fargate (1.3.0) - rely on backend instance placement and skip resource accounting
mtask.engine.enqueueTask(mtask)
for !mtask.waitEvent(mtask.consumedHostResourceEvent) {
if mtask.GetDesiredStatus().Terminal() {
Expand Down