Fix container change not reported when task status is NONE

agent/api/ecsclient/client.go

@@ -312,12 +312,6 @@ func (client *APIECSClient) SubmitTaskStateChange(change api.TaskStateChange) er
 		return nil
 	}
 
-	// Submit task state change
-	if change.Status == api.TaskStatusNone {
-		seelog.Warnf("SubmitTaskStateChange called with an invalid change: %s", change.String())
-		return errors.New("ecs api client: SubmitTaskStateChange called with an invalid change")
-	}
-
 	if change.Status != api.TaskRunning && change.Status != api.TaskStopped && len(change.Containers) == 0 {
 		seelog.Debugf("Not submitting unsupported upstream task state: %s", change.Status.String())
 		// Not really an error

agent/eventhandler/task_handler_types.go

@@ -62,6 +62,8 @@ func (event *sendableEvent) taskArn() string {
 	return event.taskChange.TaskARN
 }
 
+// taskShouldBeSent checks whether the event should be sent, this includes
+// both task state change and container state change events
 func (event *sendableEvent) taskShouldBeSent() bool {
 	event.lock.RLock()
 	defer event.lock.RUnlock()
@@ -70,27 +72,31 @@ func (event *sendableEvent) taskShouldBeSent() bool {
 		return false
 	}
 	tevent := event.taskChange
-	if tevent.Status == api.TaskStatusNone {
-		return false // defensive programming :)
-	}
 	if event.taskSent {
 		return false // redundant event
 	}
-	if tevent.Task != nil && tevent.Task.GetSentStatus() >= tevent.Status {
-		// If the task status has already been sent, check if there are
-		// any container states that need to be sent
-		for _, containerStateChange := range tevent.Containers {
-			container := containerStateChange.Container
-			if container.GetSentStatus() < container.GetKnownStatus() {
-				// We found a container that needs its state
-				// change to be sent to ECS.
-				return true
-			}
-		}
 
+	// task and container change event should have task != nil
+	if tevent.Task == nil {
 		return false
 	}
-	return true
+
+	// Task event should be sent
+	if tevent.Task.GetSentStatus() < tevent.Status {
+		return true
+	}
+
+	// Container event should be sent
+	for _, containerStateChange := range tevent.Containers {
+		container := containerStateChange.Container
+		if container.GetSentStatus() < container.GetKnownStatus() {
+			// We found a container that needs its state
+			// change to be sent to ECS.
+			return true
+		}
+	}
+
+	return false
 }
 
 func (event *sendableEvent) taskAttachmentShouldBeSent() bool {

agent/eventhandler/task_handler_types_test.go

@@ -39,13 +39,17 @@ func TestShouldTaskEventBeSent(t *testing.T) {
 			// We don't send a task event to backend if task status == NONE
 			event: newSendableTaskEvent(api.TaskStateChange{
 				Status: api.TaskStatusNone,
+				Task: &api.Task{
+					SentStatusUnsafe: api.TaskStatusNone,
+				},
 			}),
 			shouldBeSent: false,
 		},
 		{
 			// task status == RUNNING should be sent to backend
 			event: newSendableTaskEvent(api.TaskStateChange{
 				Status: api.TaskRunning,
+				Task:   &api.Task{},
 			}),
 			shouldBeSent: true,
 		},
@@ -94,6 +98,25 @@ func TestShouldTaskEventBeSent(t *testing.T) {
 			}),
 			shouldBeSent: true,
 		},
+		{
+			// Container state change should be sent regardless of task
+			// status.
+			event: newSendableTaskEvent(api.TaskStateChange{
+				Status: api.TaskStatusNone,
+				Task: &api.Task{
+					SentStatusUnsafe: api.TaskStatusNone,
+				},
+				Containers: []api.ContainerStateChange{
+					{
+						Container: &api.Container{
+							SentStatusUnsafe:  api.ContainerStatusNone,
+							KnownStatusUnsafe: api.ContainerRunning,
+						},
+					},
+				},
+			}),
+			shouldBeSent: true,
+		},
 		{
 			// All states sent, nothing to send
 			event: newSendableTaskEvent(api.TaskStateChange{
@@ -137,6 +160,7 @@ func TestShouldTaskAttachmentEventBeSent(t *testing.T) {
 			// ENI Attachment is only sent if task status == NONE
 			event: newSendableTaskEvent(api.TaskStateChange{
 				Status: api.TaskStopped,
+				Task:   &api.Task{},
 			}),
 			attachmentShouldBeSent: false,
 			taskShouldBeSent:       true,