Skip to content

Commit 18850dd

Browse files
danehlimdanehlim
committed
Minor refactor of existing items related to ACS ENI attachment
1 parent ea47747 commit 18850dd

File tree

13 files changed

+189
-189
lines changed

13 files changed

+189
-189
lines changed

‎agent/acs/handler/attach_eni_handler_common.go

+2-18
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,13 @@ package handler
1616
import (
1717
"fmt"
1818

19-
"github.com/aws/amazon-ecs-agent/ecs-agent/logger"
20-
"github.com/aws/amazon-ecs-agent/ecs-agent/logger/field"
21-
2219
"github.com/aws/amazon-ecs-agent/agent/data"
2320
"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
2421
"github.com/aws/amazon-ecs-agent/agent/utils"
25-
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
2622
apieni "github.com/aws/amazon-ecs-agent/ecs-agent/api/eni"
23+
"github.com/aws/amazon-ecs-agent/ecs-agent/logger"
24+
"github.com/aws/amazon-ecs-agent/ecs-agent/logger/field"
2725
"github.com/aws/amazon-ecs-agent/ecs-agent/utils/arn"
28-
"github.com/aws/amazon-ecs-agent/ecs-agent/wsclient"
29-
"github.com/aws/aws-sdk-go/aws"
30-
3126
"github.com/cihub/seelog"
3227
"github.com/pkg/errors"
3328
)
@@ -133,14 +128,3 @@ func (eniHandler *eniHandler) removeENIAttachmentData(mac string) {
133128
}
134129
}
135130
}
136-
137-
// sendAck sends ack for a certain ACS message
138-
func sendAck(acsClient wsclient.ClientServer, clusterArn *string, containerInstanceArn *string, messageId *string) {
139-
if err := acsClient.MakeRequest(&ecsacs.AckRequest{
140-
Cluster: clusterArn,
141-
ContainerInstance: containerInstanceArn,
142-
MessageId: messageId,
143-
}); err != nil {
144-
seelog.Warnf("Failed to ack request with messageId: %s, error: %v", aws.StringValue(messageId), err)
145-
}
146-
}

‎agent/acs/handler/attach_eni_handler_common_test.go

+16-14
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,14 @@ import (
2020
"testing"
2121
"time"
2222

23+
"github.com/golang/mock/gomock"
24+
"github.com/stretchr/testify/assert"
25+
2326
"github.com/aws/amazon-ecs-agent/agent/data"
2427
"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
28+
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/session/testconst"
2529
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo"
2630
apieni "github.com/aws/amazon-ecs-agent/ecs-agent/api/eni"
27-
"github.com/golang/mock/gomock"
28-
"github.com/stretchr/testify/assert"
2931
)
3032

3133
const (
@@ -49,7 +51,7 @@ func testENIAckTimeout(t *testing.T, attachmentType string) {
4951
taskEngineState := dockerstate.NewTaskEngineState()
5052
dataClient := newTestDataClient(t)
5153

52-
expiresAt := time.Now().Add(time.Millisecond * waitTimeoutMillis)
54+
expiresAt := time.Now().Add(time.Millisecond * testconst.WaitTimeoutMillis)
5355
eniAttachment := &apieni.ENIAttachment{
5456
AttachmentInfo: attachmentinfo.AttachmentInfo{
5557
TaskARN: taskArn,
@@ -58,7 +60,7 @@ func testENIAckTimeout(t *testing.T, attachmentType string) {
5860
AttachStatusSent: false,
5961
},
6062
AttachmentType: attachmentType,
61-
MACAddress: randomMAC,
63+
MACAddress: testconst.RandomMAC,
6264
}
6365
eniHandler := &eniHandler{
6466
state: taskEngineState,
@@ -72,7 +74,7 @@ func testENIAckTimeout(t *testing.T, attachmentType string) {
7274
assert.NoError(t, err)
7375
assert.Len(t, res, 1)
7476
for {
75-
time.Sleep(time.Millisecond * waitTimeoutMillis)
77+
time.Sleep(time.Millisecond * testconst.WaitTimeoutMillis)
7678
if len(taskEngineState.(*dockerstate.DockerTaskEngineState).AllENIAttachments()) == 0 {
7779
res, err := dataClient.GetENIAttachments()
7880
assert.NoError(t, err)
@@ -98,7 +100,7 @@ func testENIAckWithinTimeout(t *testing.T, attachmentType string) {
98100

99101
taskEngineState := dockerstate.NewTaskEngineState()
100102
dataClient := data.NewNoopClient()
101-
expiresAt := time.Now().Add(time.Millisecond * waitTimeoutMillis)
103+
expiresAt := time.Now().Add(time.Millisecond * testconst.WaitTimeoutMillis)
102104
eniAttachment := &apieni.ENIAttachment{
103105
AttachmentInfo: attachmentinfo.AttachmentInfo{
104106
TaskARN: taskArn,
@@ -107,7 +109,7 @@ func testENIAckWithinTimeout(t *testing.T, attachmentType string) {
107109
AttachStatusSent: false,
108110
},
109111
AttachmentType: attachmentType,
110-
MACAddress: randomMAC,
112+
MACAddress: testconst.RandomMAC,
111113
}
112114
eniHandler := &eniHandler{
113115
state: taskEngineState,
@@ -117,11 +119,11 @@ func testENIAckWithinTimeout(t *testing.T, attachmentType string) {
117119
err := eniHandler.addENIAttachmentToState(eniAttachment)
118120
assert.NoError(t, err)
119121
assert.Len(t, taskEngineState.(*dockerstate.DockerTaskEngineState).AllENIAttachments(), 1)
120-
eniAttachment, ok := taskEngineState.(*dockerstate.DockerTaskEngineState).ENIByMac(randomMAC)
122+
eniAttachment, ok := taskEngineState.(*dockerstate.DockerTaskEngineState).ENIByMac(testconst.RandomMAC)
121123
assert.True(t, ok)
122124
eniAttachment.SetSentStatus()
123125

124-
time.Sleep(time.Millisecond * waitTimeoutMillis)
126+
time.Sleep(time.Millisecond * testconst.WaitTimeoutMillis)
125127

126128
assert.Len(t, taskEngineState.(*dockerstate.DockerTaskEngineState).AllENIAttachments(), 1)
127129
}
@@ -143,7 +145,7 @@ func testHandleENIAttachment(t *testing.T, attachmentType, taskArn string) {
143145
dataClient := newTestDataClient(t)
144146

145147
taskEngineState := dockerstate.NewTaskEngineState()
146-
expiresAt := time.Now().Add(time.Millisecond * waitTimeoutMillis)
148+
expiresAt := time.Now().Add(time.Millisecond * testconst.WaitTimeoutMillis)
147149
eniAttachment := &apieni.ENIAttachment{
148150
AttachmentInfo: attachmentinfo.AttachmentInfo{
149151
TaskARN: taskArn,
@@ -152,7 +154,7 @@ func testHandleENIAttachment(t *testing.T, attachmentType, taskArn string) {
152154
AttachStatusSent: false,
153155
},
154156
AttachmentType: attachmentType,
155-
MACAddress: randomMAC,
157+
MACAddress: testconst.RandomMAC,
156158
}
157159
eniHandler := &eniHandler{
158160
state: taskEngineState,
@@ -162,11 +164,11 @@ func testHandleENIAttachment(t *testing.T, attachmentType, taskArn string) {
162164
err := eniHandler.HandleENIAttachment(eniAttachment)
163165
assert.NoError(t, err)
164166
assert.Len(t, taskEngineState.(*dockerstate.DockerTaskEngineState).AllENIAttachments(), 1)
165-
eniAttachment, ok := taskEngineState.(*dockerstate.DockerTaskEngineState).ENIByMac(randomMAC)
167+
eniAttachment, ok := taskEngineState.(*dockerstate.DockerTaskEngineState).ENIByMac(testconst.RandomMAC)
166168
assert.True(t, ok)
167169
eniAttachment.SetSentStatus()
168170

169-
time.Sleep(time.Millisecond * waitTimeoutMillis)
171+
time.Sleep(time.Millisecond * testconst.WaitTimeoutMillis)
170172

171173
assert.Len(t, taskEngineState.(*dockerstate.DockerTaskEngineState).AllENIAttachments(), 1)
172174
res, err := dataClient.GetENIAttachments()
@@ -201,7 +203,7 @@ func testHandleExpiredENIAttachment(t *testing.T, attachmentType, taskArn string
201203
ExpiresAt: expiresAt,
202204
},
203205
AttachmentType: attachmentType,
204-
MACAddress: randomMAC,
206+
MACAddress: testconst.RandomMAC,
205207
}
206208
eniHandler := &eniHandler{
207209
state: taskEngineState,

‎agent/acs/handler/attach_instance_eni_handler_test.go

+34-33
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ import (
2222
"testing"
2323
"time"
2424

25+
"github.com/aws/aws-sdk-go/aws"
26+
"github.com/golang/mock/gomock"
27+
"github.com/stretchr/testify/assert"
28+
2529
"github.com/aws/amazon-ecs-agent/agent/data"
2630
"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
2731
mock_dockerstate "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate/mocks"
@@ -30,9 +34,6 @@ import (
3034
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo"
3135
apieni "github.com/aws/amazon-ecs-agent/ecs-agent/api/eni"
3236
mock_wsclient "github.com/aws/amazon-ecs-agent/ecs-agent/wsclient/mock"
33-
"github.com/aws/aws-sdk-go/aws"
34-
"github.com/golang/mock/gomock"
35-
"github.com/stretchr/testify/assert"
3637
)
3738

3839
// TestInvalidAttachInstanceENIMessage tests various invalid formats of AttachInstanceNetworkInterfacesMessage
@@ -46,89 +47,89 @@ func TestInvalidAttachInstanceENIMessage(t *testing.T) {
4647
ClusterArn: aws.String(testconst.ClusterName),
4748
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
4849
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{},
49-
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
50+
WaitTimeoutMs: aws.Int64(testconst.WaitTimeoutMillis),
5051
},
5152
description: "Message without message id should be invalid",
5253
},
5354
{
5455
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
55-
MessageId: aws.String(eniMessageId),
56+
MessageId: aws.String(testconst.MessageID),
5657
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
5758
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{},
58-
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
59+
WaitTimeoutMs: aws.Int64(testconst.WaitTimeoutMillis),
5960
},
6061
description: "Message without cluster arn should be invalid",
6162
},
6263
{
6364
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
64-
MessageId: aws.String(eniMessageId),
65+
MessageId: aws.String(testconst.MessageID),
6566
ClusterArn: aws.String(testconst.ClusterName),
6667
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{},
67-
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
68+
WaitTimeoutMs: aws.Int64(testconst.WaitTimeoutMillis),
6869
},
6970
description: "Message without container instance arn should be invalid",
7071
},
7172
{
7273
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
73-
MessageId: aws.String(eniMessageId),
74+
MessageId: aws.String(testconst.MessageID),
7475
ClusterArn: aws.String(testconst.ClusterName),
75-
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
76+
WaitTimeoutMs: aws.Int64(testconst.WaitTimeoutMillis),
7677
},
7778
description: "Message without network interfaces should be invalid",
7879
},
7980
{
8081
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
81-
MessageId: aws.String(eniMessageId),
82+
MessageId: aws.String(testconst.MessageID),
8283
ClusterArn: aws.String(testconst.ClusterName),
8384
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
8485
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
8586
{
86-
MacAddress: aws.String(randomMAC),
87+
MacAddress: aws.String(testconst.RandomMAC),
8788
Ec2Id: aws.String("1"),
8889
},
8990
{
90-
MacAddress: aws.String(randomMAC),
91+
MacAddress: aws.String(testconst.RandomMAC),
9192
Ec2Id: aws.String("2"),
9293
},
9394
},
94-
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
95+
WaitTimeoutMs: aws.Int64(testconst.WaitTimeoutMillis),
9596
},
9697
description: "Message with multiple network interfaces should be invalid",
9798
},
9899
{
99100
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
100-
MessageId: aws.String(eniMessageId),
101+
MessageId: aws.String(testconst.MessageID),
101102
ClusterArn: aws.String(testconst.ClusterName),
102103
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
103104
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
104105
{},
105106
},
106-
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
107+
WaitTimeoutMs: aws.Int64(testconst.WaitTimeoutMillis),
107108
},
108109
description: "Message without network details should be invalid",
109110
},
110111
{
111112
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
112-
MessageId: aws.String(eniMessageId),
113+
MessageId: aws.String(testconst.MessageID),
113114
ClusterArn: aws.String(testconst.ClusterName),
114115
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
115116
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
116117
{
117118
Ec2Id: aws.String("1"),
118119
},
119120
},
120-
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
121+
WaitTimeoutMs: aws.Int64(testconst.WaitTimeoutMillis),
121122
},
122123
description: "Message with a network interface without macAddress should be invalid",
123124
},
124125
{
125126
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
126-
MessageId: aws.String(eniMessageId),
127+
MessageId: aws.String(testconst.MessageID),
127128
ClusterArn: aws.String(testconst.ClusterName),
128129
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
129130
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
130131
{
131-
MacAddress: aws.String(randomMAC),
132+
MacAddress: aws.String(testconst.RandomMAC),
132133
Ec2Id: aws.String("1"),
133134
},
134135
},
@@ -164,25 +165,25 @@ func TestInstanceENIAckSingleMessage(t *testing.T) {
164165
var ackSent sync.WaitGroup
165166
ackSent.Add(1)
166167
mockWSClient.EXPECT().MakeRequest(gomock.Any()).Do(func(ackRequest *ecsacs.AckRequest) {
167-
assert.Equal(t, aws.StringValue(ackRequest.MessageId), eniMessageId)
168+
assert.Equal(t, aws.StringValue(ackRequest.MessageId), testconst.MessageID)
168169
ackSent.Done()
169170
})
170171

171172
go handler.start()
172173

173174
mockNetInterface1 := ecsacs.ElasticNetworkInterface{
174175
Ec2Id: aws.String("1"),
175-
MacAddress: aws.String(randomMAC),
176+
MacAddress: aws.String(testconst.RandomMAC),
176177
AttachmentArn: aws.String(attachmentArn),
177178
}
178179
message := &ecsacs.AttachInstanceNetworkInterfacesMessage{
179-
MessageId: aws.String(eniMessageId),
180+
MessageId: aws.String(testconst.MessageID),
180181
ClusterArn: aws.String(testconst.ClusterName),
181182
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
182183
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
183184
&mockNetInterface1,
184185
},
185-
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
186+
WaitTimeoutMs: aws.Int64(testconst.WaitTimeoutMillis),
186187
}
187188

188189
handler.messageBuffer <- message
@@ -215,7 +216,7 @@ func TestInstanceENIAckSingleMessageDuplicateENIAttachmentMessageStartsTimer(t *
215216
var ackSent sync.WaitGroup
216217
ackSent.Add(1)
217218
mockWSClient.EXPECT().MakeRequest(gomock.Any()).Do(func(ackRequest *ecsacs.AckRequest) {
218-
assert.Equal(t, aws.StringValue(ackRequest.MessageId), eniMessageId)
219+
assert.Equal(t, aws.StringValue(ackRequest.MessageId), testconst.MessageID)
219220
ackSent.Done()
220221
})
221222
gomock.InOrder(
@@ -224,7 +225,7 @@ func TestInstanceENIAckSingleMessageDuplicateENIAttachmentMessageStartsTimer(t *
224225
// Ensuring that statemanager.Save() is not invoked should be a strong
225226
// enough check to ensure that the timer was started (since StartTimer would be
226227
// the only place to return error)
227-
mockState.EXPECT().ENIByMac(randomMAC).Return(&apieni.ENIAttachment{
228+
mockState.EXPECT().ENIByMac(testconst.RandomMAC).Return(&apieni.ENIAttachment{
228229
AttachmentInfo: attachmentinfo.AttachmentInfo{
229230
ExpiresAt: expiresAt,
230231
},
@@ -233,17 +234,17 @@ func TestInstanceENIAckSingleMessageDuplicateENIAttachmentMessageStartsTimer(t *
233234

234235
mockNetInterface1 := ecsacs.ElasticNetworkInterface{
235236
Ec2Id: aws.String("1"),
236-
MacAddress: aws.String(randomMAC),
237+
MacAddress: aws.String(testconst.RandomMAC),
237238
AttachmentArn: aws.String("attachmentarn"),
238239
}
239240
message := &ecsacs.AttachInstanceNetworkInterfacesMessage{
240-
MessageId: aws.String(eniMessageId),
241+
MessageId: aws.String(testconst.MessageID),
241242
ClusterArn: aws.String(testconst.ClusterName),
242243
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
243244
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
244245
&mockNetInterface1,
245246
},
246-
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
247+
WaitTimeoutMs: aws.Int64(testconst.WaitTimeoutMillis),
247248
}
248249

249250
// Expect an error starting the timer because of <=0 duration
@@ -272,7 +273,7 @@ func TestInstanceENIAckHappyPath(t *testing.T) {
272273
var ackSent sync.WaitGroup
273274
ackSent.Add(1)
274275
mockWSClient.EXPECT().MakeRequest(gomock.Any()).Do(func(ackRequest *ecsacs.AckRequest) {
275-
assert.Equal(t, aws.StringValue(ackRequest.MessageId), eniMessageId)
276+
assert.Equal(t, aws.StringValue(ackRequest.MessageId), testconst.MessageID)
276277
ackSent.Done()
277278
handler.stop()
278279
})
@@ -281,16 +282,16 @@ func TestInstanceENIAckHappyPath(t *testing.T) {
281282

282283
mockNetInterface1 := ecsacs.ElasticNetworkInterface{
283284
Ec2Id: aws.String("1"),
284-
MacAddress: aws.String(randomMAC),
285+
MacAddress: aws.String(testconst.RandomMAC),
285286
}
286287
message := &ecsacs.AttachInstanceNetworkInterfacesMessage{
287-
MessageId: aws.String(eniMessageId),
288+
MessageId: aws.String(testconst.MessageID),
288289
ClusterArn: aws.String(testconst.ClusterName),
289290
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
290291
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
291292
&mockNetInterface1,
292293
},
293-
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
294+
WaitTimeoutMs: aws.Int64(testconst.WaitTimeoutMillis),
294295
}
295296

296297
handler.messageBuffer <- message

0 commit comments

Comments
 (0)