Skip to content

Commit fb239e3

Browse files
danehlimdanehlim
committed
UpdateDesiredStatus for task stop verification ACK
1 parent 10afa9c commit fb239e3

4 files changed

+58
-29
lines changed

‎agent/acs/session/task_manifest_responder_test.go

+8-5
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,14 @@ import (
3434
)
3535

3636
const (
37-
initialSeqNum = 11
38-
nextSeqNum = 12
39-
taskARN1 = "arn1"
40-
taskARN2 = "arn2"
41-
taskARN3 = "arn3"
37+
initialSeqNum = 11
38+
nextSeqNum = 12
39+
taskARN1 = "arn1"
40+
taskARN2 = "arn2"
41+
taskARN3 = "arn3"
42+
containerName1 = "name1"
43+
containerName2 = "name2"
44+
containerName3 = "name3"
4245
)
4346

4447
var expectedTaskManifestAck = &ecsacs.AckRequest{

‎agent/acs/session/task_stop_verification_ack_responder.go

+11-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package session
1515

1616
import (
17+
"github.com/aws/amazon-ecs-agent/agent/data"
1718
"github.com/aws/amazon-ecs-agent/agent/engine"
1819
apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status"
1920
"github.com/aws/amazon-ecs-agent/ecs-agent/logger"
@@ -23,12 +24,14 @@ import (
2324
// taskStopper implements the TaskStopper interface defined in ecs-agent module.
2425
type taskStopper struct {
2526
taskEngine engine.TaskEngine
27+
dataClient data.Client
2628
}
2729

2830
// NewTaskStopper creates a new taskStopper.
29-
func NewTaskStopper(taskEngine engine.TaskEngine) *taskStopper {
31+
func NewTaskStopper(taskEngine engine.TaskEngine, dataClient data.Client) *taskStopper {
3032
return &taskStopper{
3133
taskEngine: taskEngine,
34+
dataClient: dataClient,
3235
}
3336
}
3437

@@ -39,7 +42,13 @@ func (ts *taskStopper) StopTask(taskARN string) {
3942
loggerfield.TaskARN: task.Arn,
4043
})
4144
task.SetDesiredStatus(apitaskstatus.TaskStopped)
42-
ts.taskEngine.AddTask(task)
45+
task.UpdateDesiredStatus()
46+
if err := ts.dataClient.SaveTask(task); err != nil {
47+
logger.Error("Failed to save data for task", logger.Fields{
48+
loggerfield.TaskARN: task.Arn,
49+
loggerfield.Error: err,
50+
})
51+
}
4352
} else {
4453
logger.Debug("Task from task stop verification ACK not found on the instance", logger.Fields{
4554
loggerfield.TaskARN: taskARN,

‎agent/acs/session/task_stop_verification_ack_responder_test.go

+38-21
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@ package session
1818
import (
1919
"testing"
2020

21+
"github.com/aws/amazon-ecs-agent/agent/api/container"
2122
"github.com/aws/amazon-ecs-agent/agent/api/task"
23+
"github.com/aws/amazon-ecs-agent/agent/data"
2224
mock_engine "github.com/aws/amazon-ecs-agent/agent/engine/mocks"
2325
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
2426
acssession "github.com/aws/amazon-ecs-agent/ecs-agent/acs/session"
2527
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/session/testconst"
28+
apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status"
2629
apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status"
2730
"github.com/aws/amazon-ecs-agent/ecs-agent/metrics"
2831
"github.com/aws/amazon-ecs-agent/ecs-agent/wsclient"
@@ -44,7 +47,7 @@ func setupTaskStopVerificationAckTest(t *testing.T) *taskStopVerificationAckTest
4447
manifestMessageIDAccessor := NewManifestMessageIDAccessor()
4548
manifestMessageIDAccessor.SetMessageID(testconst.MessageID)
4649
taskStopVerificationAckResponder := acssession.NewTaskStopVerificationACKResponder(
47-
NewTaskStopper(taskEngine),
50+
NewTaskStopper(taskEngine, data.NewNoopClient()),
4851
manifestMessageIDAccessor,
4952
metrics.NewNopEntryFactory())
5053

@@ -58,9 +61,27 @@ func setupTaskStopVerificationAckTest(t *testing.T) *taskStopVerificationAckTest
5861
// defaultTasksOnInstance returns a baseline map of tasks that simulates/tracks the tasks on an instance.
5962
func defaultTasksOnInstance() map[string]*task.Task {
6063
return map[string]*task.Task{
61-
taskARN1: {Arn: taskARN1, DesiredStatusUnsafe: apitaskstatus.TaskRunning},
62-
taskARN2: {Arn: taskARN2, DesiredStatusUnsafe: apitaskstatus.TaskRunning},
63-
taskARN3: {Arn: taskARN3, DesiredStatusUnsafe: apitaskstatus.TaskRunning},
64+
taskARN1: {Arn: taskARN1, DesiredStatusUnsafe: apitaskstatus.TaskRunning,
65+
Containers: []*container.Container{
66+
{
67+
Name: containerName1,
68+
DesiredStatusUnsafe: apicontainerstatus.ContainerRunning,
69+
},
70+
}},
71+
taskARN2: {Arn: taskARN2, DesiredStatusUnsafe: apitaskstatus.TaskRunning,
72+
Containers: []*container.Container{
73+
{
74+
Name: containerName2,
75+
DesiredStatusUnsafe: apicontainerstatus.ContainerRunning,
76+
},
77+
}},
78+
taskARN3: {Arn: taskARN3, DesiredStatusUnsafe: apitaskstatus.TaskRunning,
79+
Containers: []*container.Container{
80+
{
81+
Name: containerName3,
82+
DesiredStatusUnsafe: apicontainerstatus.ContainerRunning,
83+
},
84+
}},
6485
}
6586
}
6687

@@ -99,21 +120,24 @@ func TestTaskStopVerificationAckResponderStopsMultipleTasks(t *testing.T) {
99120

100121
tester.taskEngine.EXPECT().GetTaskByArn(taskARN2).Return(tasksOnInstance[taskARN2], true)
101122
tester.taskEngine.EXPECT().GetTaskByArn(taskARN3).Return(tasksOnInstance[taskARN3], true)
102-
tester.taskEngine.EXPECT().AddTask(tasksOnInstance[taskARN2]).Do(func(task *task.Task) {
103-
task.SetDesiredStatus(apitaskstatus.TaskStopped)
104-
})
105-
tester.taskEngine.EXPECT().AddTask(tasksOnInstance[taskARN3]).Do(func(task *task.Task) {
106-
task.SetDesiredStatus(apitaskstatus.TaskStopped)
107-
})
108123

109124
handleTaskStopVerificationAck :=
110125
tester.taskStopVerificationAckResponder.HandlerFunc().(func(message *ecsacs.TaskStopVerificationAck))
111126
handleTaskStopVerificationAck(taskStopVerificationAck)
112127

113-
// Only task2 and task3 should be stopped.
128+
// Only task2 and task3 and their containers should be stopped.
114129
assert.Equal(t, apitaskstatus.TaskRunning, tasksOnInstance[taskARN1].GetDesiredStatus())
130+
container1, ok := tasksOnInstance[taskARN1].ContainerByName(containerName1)
131+
assert.True(t, ok)
132+
assert.Equal(t, apicontainerstatus.ContainerRunning, container1.GetDesiredStatus())
115133
assert.Equal(t, apitaskstatus.TaskStopped, tasksOnInstance[taskARN2].GetDesiredStatus())
134+
container2, ok := tasksOnInstance[taskARN2].ContainerByName(containerName2)
135+
assert.True(t, ok)
136+
assert.Equal(t, apicontainerstatus.ContainerStopped, container2.GetDesiredStatus())
116137
assert.Equal(t, apitaskstatus.TaskStopped, tasksOnInstance[taskARN3].GetDesiredStatus())
138+
container3, ok := tasksOnInstance[taskARN3].ContainerByName(containerName3)
139+
assert.True(t, ok)
140+
assert.Equal(t, apicontainerstatus.ContainerStopped, container3.GetDesiredStatus())
117141

118142
}
119143

@@ -149,22 +173,15 @@ func TestTaskStopVerificationAckResponderStopsAllTasks(t *testing.T) {
149173
tester.taskEngine.EXPECT().GetTaskByArn(taskARN1).Return(tasksOnInstance[taskARN1], true)
150174
tester.taskEngine.EXPECT().GetTaskByArn(taskARN2).Return(tasksOnInstance[taskARN2], true)
151175
tester.taskEngine.EXPECT().GetTaskByArn(taskARN3).Return(tasksOnInstance[taskARN3], true)
152-
tester.taskEngine.EXPECT().AddTask(tasksOnInstance[taskARN1]).Do(func(task *task.Task) {
153-
task.SetDesiredStatus(apitaskstatus.TaskStopped)
154-
})
155-
tester.taskEngine.EXPECT().AddTask(tasksOnInstance[taskARN2]).Do(func(task *task.Task) {
156-
task.SetDesiredStatus(apitaskstatus.TaskStopped)
157-
})
158-
tester.taskEngine.EXPECT().AddTask(tasksOnInstance[taskARN3]).Do(func(task *task.Task) {
159-
task.SetDesiredStatus(apitaskstatus.TaskStopped)
160-
})
161176

162177
handleTaskStopVerificationAck :=
163178
tester.taskStopVerificationAckResponder.HandlerFunc().(func(message *ecsacs.TaskStopVerificationAck))
164179
handleTaskStopVerificationAck(taskStopVerificationAck)
165180

166-
// All tasks on instance should be stopped.
181+
// All tasks and containers on instance should be stopped.
167182
for _, task := range tasksOnInstance {
168183
assert.Equal(t, apitaskstatus.TaskStopped, task.GetDesiredStatus())
184+
assert.Equal(t, 1, len(task.Containers))
185+
assert.Equal(t, apicontainerstatus.ContainerStopped, task.Containers[0].GetDesiredStatus())
169186
}
170187
}

‎agent/app/agent.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1044,7 +1044,7 @@ func (agent *ecsAgent) startACSSession(
10441044
manifestMessageIDAccessor := agentacs.NewManifestMessageIDAccessor()
10451045
sequenceNumberAccessor := agentacs.NewSequenceNumberAccessor(agent.latestSeqNumberTaskManifest, agent.dataClient)
10461046
taskComparer := agentacs.NewTaskComparer(taskEngine)
1047-
taskStopper := agentacs.NewTaskStopper(taskEngine)
1047+
taskStopper := agentacs.NewTaskStopper(taskEngine, agent.dataClient)
10481048

10491049
acsSession := session.NewSession(agent.containerInstanceARN,
10501050
agent.cfg.Cluster,

0 commit comments

Comments
 (0)