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.

Sign up for GitHub

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

Jump to bottom

Refactor ACS attach task ENI message handling #3744

Merged
merged 1 commit into from
Jun 19, 2023
Merged

Refactor ACS attach task ENI message handling #3744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 7 additions & 16 deletions agent/acs/handler/acs_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,19 +263,6 @@ func (acsSession *session) startACSSession(client wsclient.ClientServer) error {
dataClient: acsSession.dataClient,
}

// Add handler to ack task ENI attach message
eniAttachHandler := newAttachTaskENIHandler(
acsSession.ctx,
cfg.Cluster,
acsSession.containerInstanceARN,
client,
eniHandler,
)
eniAttachHandler.start()
defer eniAttachHandler.stop()

client.AddRequestHandler(eniAttachHandler.handlerFunc())

// Add handler to ack instance ENI attach message
instanceENIAttachHandler := newAttachInstanceENIHandler(
acsSession.ctx,
Expand Down Expand Up @@ -325,9 +312,13 @@ func (acsSession *session) startACSSession(client wsclient.ClientServer) error {
responseSender := func(response interface{}) error {
return client.MakeRequest(response)
}

heartbeatResponder := acssession.NewHeartbeatResponder(acsSession.doctor, responseSender)
client.AddRequestHandler(heartbeatResponder.HandlerFunc())
responders := []wsclient.RequestResponder{
acssession.NewAttachTaskENIResponder(eniHandler, responseSender),
Realmonia marked this conversation as resolved.
Show resolved Hide resolved
acssession.NewHeartbeatResponder(acsSession.doctor, responseSender),
}
for _, r := range responders {
client.AddRequestHandler(r.HandlerFunc())
}

updater.AddAgentUpdateHandlers(client, cfg, acsSession.state, acsSession.dataClient, acsSession.taskEngine)

Expand Down
40 changes: 40 additions & 0 deletions agent/acs/handler/attach_eni_handler_common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,43 @@ func testHandleENIAttachment(t *testing.T, attachmentType, taskArn string) {
assert.NoError(t, err)
assert.Len(t, res, 1)
}

// TestHandleExpiredENIAttachmentTaskENI tests handling an expired task eni
func TestHandleExpiredENIAttachmentTaskENI(t *testing.T) {
testHandleExpiredENIAttachment(t, apieni.ENIAttachmentTypeTaskENI, taskArn)
}

// TestHandleExpiredENIAttachmentInstanceENI tests handling an expired instance eni
func TestHandleExpiredENIAttachmentInstanceENI(t *testing.T) {
testHandleExpiredENIAttachment(t, apieni.ENIAttachmentTypeInstanceENI, "")
}

func testHandleExpiredENIAttachment(t *testing.T, attachmentType, taskArn string) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

// Set expiresAt to a value in the past.
expiresAt := time.Unix(time.Now().Unix()-1, 0)

taskEngineState := dockerstate.NewTaskEngineState()
dataClient := data.NewNoopClient()

eniAttachment := &apieni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
TaskARN: taskArn,
AttachmentARN: attachmentArn,
ExpiresAt: expiresAt,
},
AttachmentType: attachmentType,
MACAddress: randomMAC,
}
eniHandler := &eniHandler{
state: taskEngineState,
dataClient: dataClient,
}

// Expect an error starting the timer because of <=0 duration.
err := eniHandler.HandleENIAttachment(eniAttachment)
assert.Error(t, err)
assert.Equal(t, true, eniAttachment.HasExpired())
}
45 changes: 23 additions & 22 deletions agent/acs/handler/attach_instance_eni_handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
mock_dockerstate "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate/mocks"
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/session/testconst"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo"
apieni "github.com/aws/amazon-ecs-agent/ecs-agent/api/eni"
mock_wsclient "github.com/aws/amazon-ecs-agent/ecs-agent/wsclient/mock"
Expand All @@ -42,8 +43,8 @@ func TestInvalidAttachInstanceENIMessage(t *testing.T) {
}{
{
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
ClusterArn: aws.String(clusterName),
ContainerInstanceArn: aws.String(containerInstanceArn),
ClusterArn: aws.String(testconst.ClusterName),
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{},
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
},
Expand All @@ -52,7 +53,7 @@ func TestInvalidAttachInstanceENIMessage(t *testing.T) {
{
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
MessageId: aws.String(eniMessageId),
ContainerInstanceArn: aws.String(containerInstanceArn),
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{},
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
},
Expand All @@ -61,7 +62,7 @@ func TestInvalidAttachInstanceENIMessage(t *testing.T) {
{
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
MessageId: aws.String(eniMessageId),
ClusterArn: aws.String(clusterName),
ClusterArn: aws.String(testconst.ClusterName),
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{},
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
},
Expand All @@ -70,16 +71,16 @@ func TestInvalidAttachInstanceENIMessage(t *testing.T) {
{
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
MessageId: aws.String(eniMessageId),
ClusterArn: aws.String(clusterName),
ClusterArn: aws.String(testconst.ClusterName),
WaitTimeoutMs: aws.Int64(waitTimeoutMillis),
},
description: "Message without network interfaces should be invalid",
},
{
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
MessageId: aws.String(eniMessageId),
ClusterArn: aws.String(clusterName),
ContainerInstanceArn: aws.String(containerInstanceArn),
ClusterArn: aws.String(testconst.ClusterName),
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
{
MacAddress: aws.String(randomMAC),
Expand All @@ -97,8 +98,8 @@ func TestInvalidAttachInstanceENIMessage(t *testing.T) {
{
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
MessageId: aws.String(eniMessageId),
ClusterArn: aws.String(clusterName),
ContainerInstanceArn: aws.String(containerInstanceArn),
ClusterArn: aws.String(testconst.ClusterName),
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
{},
},
Expand All @@ -109,8 +110,8 @@ func TestInvalidAttachInstanceENIMessage(t *testing.T) {
{
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
MessageId: aws.String(eniMessageId),
ClusterArn: aws.String(clusterName),
ContainerInstanceArn: aws.String(containerInstanceArn),
ClusterArn: aws.String(testconst.ClusterName),
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
{
Ec2Id: aws.String("1"),
Expand All @@ -123,8 +124,8 @@ func TestInvalidAttachInstanceENIMessage(t *testing.T) {
{
message: &ecsacs.AttachInstanceNetworkInterfacesMessage{
MessageId: aws.String(eniMessageId),
ClusterArn: aws.String(clusterName),
ContainerInstanceArn: aws.String(containerInstanceArn),
ClusterArn: aws.String(testconst.ClusterName),
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
{
MacAddress: aws.String(randomMAC),
Expand Down Expand Up @@ -153,7 +154,7 @@ func TestInstanceENIAckSingleMessage(t *testing.T) {

ctx := context.TODO()
mockWSClient := mock_wsclient.NewMockClientServer(ctrl)
handler := newAttachInstanceENIHandler(ctx, clusterName, containerInstanceArn, mockWSClient,
handler := newAttachInstanceENIHandler(ctx, testconst.ClusterName, testconst.ContainerInstanceARN, mockWSClient,
&eniHandler{
state: taskEngineState,
dataClient: dataClient,
Expand All @@ -176,8 +177,8 @@ func TestInstanceENIAckSingleMessage(t *testing.T) {
}
message := &ecsacs.AttachInstanceNetworkInterfacesMessage{
MessageId: aws.String(eniMessageId),
ClusterArn: aws.String(clusterName),
ContainerInstanceArn: aws.String(containerInstanceArn),
ClusterArn: aws.String(testconst.ClusterName),
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
&mockNetInterface1,
},
Expand All @@ -201,7 +202,7 @@ func TestInstanceENIAckSingleMessageDuplicateENIAttachmentMessageStartsTimer(t *

ctx := context.TODO()
mockWSClient := mock_wsclient.NewMockClientServer(ctrl)
handler := newAttachInstanceENIHandler(ctx, clusterName, containerInstanceArn, mockWSClient,
handler := newAttachInstanceENIHandler(ctx, testconst.ClusterName, testconst.ContainerInstanceARN, mockWSClient,
&eniHandler{
state: mockState,
dataClient: dataClient,
Expand Down Expand Up @@ -237,8 +238,8 @@ func TestInstanceENIAckSingleMessageDuplicateENIAttachmentMessageStartsTimer(t *
}
message := &ecsacs.AttachInstanceNetworkInterfacesMessage{
MessageId: aws.String(eniMessageId),
ClusterArn: aws.String(clusterName),
ContainerInstanceArn: aws.String(containerInstanceArn),
ClusterArn: aws.String(testconst.ClusterName),
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
&mockNetInterface1,
},
Expand All @@ -261,7 +262,7 @@ func TestInstanceENIAckHappyPath(t *testing.T) {
dataClient := data.NewNoopClient()

mockWSClient := mock_wsclient.NewMockClientServer(ctrl)
handler := newAttachInstanceENIHandler(ctx, clusterName, containerInstanceArn, mockWSClient,
handler := newAttachInstanceENIHandler(ctx, testconst.ClusterName, testconst.ContainerInstanceARN, mockWSClient,
&eniHandler{
state: taskEngineState,
dataClient: dataClient,
Expand All @@ -284,8 +285,8 @@ func TestInstanceENIAckHappyPath(t *testing.T) {
}
message := &ecsacs.AttachInstanceNetworkInterfacesMessage{
MessageId: aws.String(eniMessageId),
ClusterArn: aws.String(clusterName),
ContainerInstanceArn: aws.String(containerInstanceArn),
ClusterArn: aws.String(testconst.ClusterName),
ContainerInstanceArn: aws.String(testconst.ContainerInstanceARN),
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
&mockNetInterface1,
},
Expand Down
Loading