Check the sentstatus before updating

CHANGELOG.md

@@ -4,6 +4,7 @@
 * Feature - Configurable container image pull behavior [#1348](https://github.com/aws/amazon-ecs-agent/pull/1348)
 * Bug - Fixed a bug where Docker Version() API never returns by adding a timeout [#1363](https://github.com/aws/amazon-ecs-agent/pull/1363)
 * Bug - Fixed a bug where tasks could get stuck waiting for execution of CNI plugin [#1358](https://github.com/aws/amazon-ecs-agent/pull/1358)
+* Bug - Fixed a bug where task cleanup could be blocked due to incorrect sentstatus [#1383](https://github.com/aws/amazon-ecs-agent/pull/1383)
 
 ## 1.17.3
 * Enhancement - Distinct startContainerTimeouts for windows/linux, introduce a new environment variable `ECS_CONTAINER_START_TIMEOUT` to make it configurable [#1321](https://github.com/aws/amazon-ecs-agent/pull/1321)

agent/eventhandler/task_handler_types.go

@@ -186,19 +186,26 @@ type setStatusSent func(event *sendableEvent)
 
 // setContainerChangeSent sets the event's container change object as sent
 func setContainerChangeSent(event *sendableEvent) {
-	if event.containerChange.Container != nil {
-		event.containerChange.Container.SetSentStatus(event.containerChange.Status)
+	containerChangeStatus := event.containerChange.Status
+	container := event.containerChange.Container
+	if container != nil && container.GetSentStatus() < containerChangeStatus {
+		container.SetSentStatus(containerChangeStatus)
 	}
 }
 
 // setTaskChangeSent sets the event's task change object as sent
 func setTaskChangeSent(event *sendableEvent) {
-	if event.taskChange.Task != nil {
-		event.taskChange.Task.SetSentStatus(event.taskChange.Status)
+	taskChangeStatus := event.taskChange.Status
+	task := event.taskChange.Task
+	if task != nil && task.GetSentStatus() < taskChangeStatus {
+		task.SetSentStatus(taskChangeStatus)
 	}
 	for _, containerStateChange := range event.taskChange.Containers {
 		container := containerStateChange.Container
-		container.SetSentStatus(containerStateChange.Status)
+		containerChangeStatus := containerStateChange.Status
+		if container.GetSentStatus() < containerChangeStatus {
+			container.SetSentStatus(containerStateChange.Status)
+		}
 	}
 }
 

agent/eventhandler/task_handler_types_test.go

@@ -224,3 +224,54 @@ func TestShouldTaskAttachmentEventBeSent(t *testing.T) {
 		})
 	}
 }
+
+func TestSetTaskSentStatus(t *testing.T) {
+	testContainer := &api.Container{}
+	testTask := &api.Task{}
+
+	taskRunningStateChange := newSendableTaskEvent(api.TaskStateChange{
+		Status: api.TaskRunning,
+		Task:   testTask,
+		Containers: []api.ContainerStateChange{
+			{
+				Status:    api.ContainerRunning,
+				Container: testContainer,
+			},
+		},
+	})
+	taskStoppedStateChange := newSendableTaskEvent(api.TaskStateChange{
+		Status: api.TaskStopped,
+		Task:   testTask,
+		Containers: []api.ContainerStateChange{
+			{
+				Status:    api.ContainerStopped,
+				Container: testContainer,
+			},
+		},
+	})
+
+	setTaskChangeSent(taskStoppedStateChange)
+	assert.Equal(t, testTask.GetSentStatus(), api.TaskStopped)
+	assert.Equal(t, testContainer.GetSentStatus(), api.ContainerStopped)
+	setTaskChangeSent(taskRunningStateChange)
+	assert.Equal(t, testTask.GetSentStatus(), api.TaskStopped)
+	assert.Equal(t, testContainer.GetSentStatus(), api.ContainerStopped)
+}
+
+func TestSetContainerSentStatus(t *testing.T) {
+	testContainer := &api.Container{}
+
+	containerRunningStateChange := newSendableContainerEvent(api.ContainerStateChange{
+		Status:    api.ContainerRunning,
+		Container: testContainer,
+	})
+	containerStoppedStateChange := newSendableContainerEvent(api.ContainerStateChange{
+		Status:    api.ContainerStopped,
+		Container: testContainer,
+	})
+
+	setContainerChangeSent(containerStoppedStateChange)
+	assert.Equal(t, testContainer.GetSentStatus(), api.ContainerStopped)
+	setContainerChangeSent(containerRunningStateChange)
+	assert.Equal(t, testContainer.GetSentStatus(), api.ContainerStopped)
+}