Skip to content

Commit

Permalink
Create Generic Attachment to decouple from ENI and save attachment st…
Browse files Browse the repository at this point in the history
…ate.
  • Loading branch information
fierlion committed Oct 20, 2023
1 parent 3500ac2 commit 0e3b384
Show file tree
Hide file tree
Showing 92 changed files with 701 additions and 901 deletions.
2 changes: 1 addition & 1 deletion agent/acs/session/attach_eni_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (eniHandler *eniHandler) removeENIAttachmentData(mac string) {
seelog.Errorf("Unable to retrieve ENI Attachment for mac address %s: ", mac)
return
}
attachmentId, err := utils.GetENIAttachmentId(attachmentToRemove.AttachmentARN)
attachmentId, err := utils.GetAttachmentId(attachmentToRemove.AttachmentARN)
if err != nil {
seelog.Errorf("Failed to get attachment id for %s: %v", attachmentToRemove.AttachmentARN, err)
} else {
Expand Down
10 changes: 5 additions & 5 deletions agent/acs/session/attach_eni_common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"github.com/aws/amazon-ecs-agent/agent/data"
"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/session/testconst"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment"
ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface"
)

Expand All @@ -53,7 +53,7 @@ func testENIAckTimeout(t *testing.T, attachmentType string) {

expiresAt := time.Now().Add(time.Millisecond * testconst.WaitTimeoutMillis)
eniAttachment := &ni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
AttachmentInfo: attachment.AttachmentInfo{
TaskARN: testconst.TaskARN,
AttachmentARN: attachmentArn,
ExpiresAt: expiresAt,
Expand Down Expand Up @@ -99,7 +99,7 @@ func testENIAckWithinTimeout(t *testing.T, attachmentType string) {
dataClient := data.NewNoopClient()
expiresAt := time.Now().Add(time.Millisecond * testconst.WaitTimeoutMillis)
eniAttachment := &ni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
AttachmentInfo: attachment.AttachmentInfo{
TaskARN: testconst.TaskARN,
AttachmentARN: attachmentArn,
ExpiresAt: expiresAt,
Expand Down Expand Up @@ -141,7 +141,7 @@ func testHandleENIAttachment(t *testing.T, attachmentType, taskArn string) {
taskEngineState := dockerstate.NewTaskEngineState()
expiresAt := time.Now().Add(time.Millisecond * testconst.WaitTimeoutMillis)
eniAttachment := &ni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
AttachmentInfo: attachment.AttachmentInfo{
TaskARN: taskArn,
AttachmentARN: attachmentArn,
ExpiresAt: expiresAt,
Expand Down Expand Up @@ -188,7 +188,7 @@ func testHandleExpiredENIAttachment(t *testing.T, attachmentType, taskArn string
dataClient := data.NewNoopClient()

eniAttachment := &ni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
AttachmentInfo: attachment.AttachmentInfo{
TaskARN: taskArn,
AttachmentARN: attachmentArn,
ExpiresAt: expiresAt,
Expand Down
4 changes: 2 additions & 2 deletions agent/acs/session/attach_instance_eni_responder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
acssession "github.com/aws/amazon-ecs-agent/ecs-agent/acs/session"
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/session/testconst"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment"
ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface"
)

Expand Down Expand Up @@ -113,7 +113,7 @@ func TestInstanceENIAckSingleMessageWithDuplicateENIAttachment(t *testing.T) {
mockState.EXPECT().
ENIByMac(testconst.RandomMAC).
Return(&ni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
AttachmentInfo: attachment.AttachmentInfo{
ExpiresAt: expiresAt,
},
}, true).
Expand Down
4 changes: 2 additions & 2 deletions agent/acs/session/attach_task_eni_responder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
acssession "github.com/aws/amazon-ecs-agent/ecs-agent/acs/session"
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/session/testconst"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment"
ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface"
)

Expand Down Expand Up @@ -113,7 +113,7 @@ func TestTaskENIAckSingleMessageWithDuplicateENIAttachment(t *testing.T) {
mockState.EXPECT().
ENIByMac(testconst.RandomMAC).
Return(&ni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
AttachmentInfo: attachment.AttachmentInfo{
ExpiresAt: expiresAt,
},
}, true).
Expand Down
8 changes: 4 additions & 4 deletions agent/acs/session/payload_responder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ import (
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
acssession "github.com/aws/amazon-ecs-agent/ecs-agent/acs/session"
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/session/testconst"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/eni"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment/resource"
apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status"
"github.com/aws/amazon-ecs-agent/ecs-agent/credentials"
ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface"
"github.com/aws/amazon-ecs-agent/ecs-agent/wsclient"
"github.com/aws/aws-sdk-go/aws"
"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -858,7 +858,7 @@ func TestHandlePayloadMessageAddedENITrunkToTask(t *testing.T) {
Arn: aws.String(testconst.TaskARN),
ElasticNetworkInterfaces: []*ecsacs.ElasticNetworkInterface{
{
InterfaceAssociationProtocol: aws.String(eni.VLANInterfaceAssociationProtocol),
InterfaceAssociationProtocol: aws.String(ni.VLANInterfaceAssociationProtocol),
AttachmentArn: aws.String(attachmentARN),
Ec2Id: aws.String(ec2ID),
Ipv4Addresses: []*ecsacs.IPv4AddressAssignment{
Expand Down Expand Up @@ -890,7 +890,7 @@ func TestHandlePayloadMessageAddedENITrunkToTask(t *testing.T) {

// Validate the added task has the ENI trunk information as expected.
taskeni := addedTask.GetPrimaryENI()
assert.Equal(t, eni.VLANInterfaceAssociationProtocol, taskeni.InterfaceAssociationProtocol)
assert.Equal(t, ni.VLANInterfaceAssociationProtocol, taskeni.InterfaceAssociationProtocol)
assert.Equal(t, testconst.RandomMAC, taskeni.InterfaceVlanProperties.TrunkInterfaceMacAddress)
assert.Equal(t, vlanID, taskeni.InterfaceVlanProperties.VlanID)
}
Expand Down
6 changes: 3 additions & 3 deletions agent/api/ecsclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -657,14 +657,14 @@ func (client *APIECSClient) SubmitContainerStateChange(change api.ContainerState
}

func (client *APIECSClient) SubmitAttachmentStateChange(change api.AttachmentStateChange) error {
attachmentStatus := change.Attachment.Status.String()
attachmentStatus := change.Attachment.GetAttachmentStatus()

req := ecs.SubmitAttachmentStateChangesInput{
Cluster: &client.config.Cluster,
Attachments: []*ecs.AttachmentStateChange{
{
AttachmentArn: aws.String(change.Attachment.AttachmentARN),
Status: aws.String(attachmentStatus),
AttachmentArn: aws.String(change.Attachment.GetAttachmentARN()),
Status: aws.String(attachmentStatus.String()),
},
},
}
Expand Down
12 changes: 5 additions & 7 deletions agent/api/ecsclient/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ import (
"testing"
"time"

"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/status"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -37,6 +34,7 @@ import (
"github.com/aws/amazon-ecs-agent/agent/config"
"github.com/aws/amazon-ecs-agent/agent/ec2"
mock_ec2 "github.com/aws/amazon-ecs-agent/agent/ec2/mocks"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment"
apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status"
apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status"
"github.com/aws/amazon-ecs-agent/ecs-agent/async"
Expand Down Expand Up @@ -1063,9 +1061,9 @@ func TestDiscoverTelemetryEndpointAfterPollEndpointCacheHit(t *testing.T) {
}
}

// TestSubmitTaskStateChangeWithAttachments tests the SubmitTaskStateChange API
// TestSubmitTaskStateChangeWithENIAttachments tests the SubmitTaskStateChange API
// also send the Attachment Status
func TestSubmitTaskStateChangeWithAttachments(t *testing.T) {
func TestSubmitTaskStateChangeWithENIAttachments(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

Expand All @@ -1086,9 +1084,9 @@ func TestSubmitTaskStateChangeWithAttachments(t *testing.T) {
err := client.SubmitTaskStateChange(api.TaskStateChange{
TaskARN: "task_arn",
Attachment: &ni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
AttachmentInfo: attachment.AttachmentInfo{
AttachmentARN: "eni_arn",
Status: status.AttachmentAttached,
Status: attachment.AttachmentAttached,
},
},
})
Expand Down
6 changes: 3 additions & 3 deletions agent/api/statechange.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container"
apitask "github.com/aws/amazon-ecs-agent/agent/api/task"
"github.com/aws/amazon-ecs-agent/agent/statechange"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment"
apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status"
apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status"
"github.com/aws/amazon-ecs-agent/ecs-agent/logger"
Expand Down Expand Up @@ -98,8 +99,7 @@ type TaskStateChange struct {
// AttachmentStateChange represents a state change that needs to be sent to the
// SubmitAttachmentStateChanges API
type AttachmentStateChange struct {
// Attachment is the eni attachment object to send
Attachment *ni.ENIAttachment
Attachment attachment.Attachment
}

type ErrShouldNotSendEvent struct {
Expand Down Expand Up @@ -352,7 +352,7 @@ func (change *TaskStateChange) String() string {
// String returns a human readable string representation of this object
func (change *AttachmentStateChange) String() string {
if change.Attachment != nil {
return fmt.Sprintf("%s -> %s, %s", change.Attachment.AttachmentARN, change.Attachment.Status.String(),
return fmt.Sprintf("%s -> %v, %s", change.Attachment.GetAttachmentARN(), change.Attachment.GetAttachmentStatus(),
change.Attachment.String())
}

Expand Down
2 changes: 1 addition & 1 deletion agent/api/task/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ import (
taskresourcevolume "github.com/aws/amazon-ecs-agent/agent/taskresource/volume"
"github.com/aws/amazon-ecs-agent/agent/utils"
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment/resource"
apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status"
apierrors "github.com/aws/amazon-ecs-agent/ecs-agent/api/errors"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource"
apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status"
"github.com/aws/amazon-ecs-agent/ecs-agent/credentials"
"github.com/aws/amazon-ecs-agent/ecs-agent/ecs_client/model/ecs"
Expand Down
4 changes: 2 additions & 2 deletions agent/api/task/task_attachment_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ import (
"github.com/aws/amazon-ecs-agent/agent/api/serviceconnect"
taskresourcevolume "github.com/aws/amazon-ecs-agent/agent/taskresource/volume"
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment/resource"
"github.com/aws/amazon-ecs-agent/ecs-agent/logger"
"github.com/aws/aws-sdk-go/aws"
)

// AttachmentHandler defines an interface to handel attachment received from ACS.
// AttachmentHandler defines an interface to handle attachment received from ACS.
type AttachmentHandler interface {
parseAttachment(acsAttachment *ecsacs.Attachment) error
validateAttachment(acsTask *ecsacs.Task, task *Task) error
Expand Down
2 changes: 1 addition & 1 deletion agent/api/task/task_attachment_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"github.com/aws/amazon-ecs-agent/agent/api/serviceconnect"
taskresourcevolume "github.com/aws/amazon-ecs-agent/agent/taskresource/volume"
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment/resource"
"github.com/aws/aws-sdk-go/aws"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down
2 changes: 1 addition & 1 deletion agent/api/task/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ import (
taskresourcevolume "github.com/aws/amazon-ecs-agent/agent/taskresource/volume"
"github.com/aws/amazon-ecs-agent/agent/utils"
"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment/resource"
apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource"
apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status"
"github.com/aws/amazon-ecs-agent/ecs-agent/credentials"
mock_credentials "github.com/aws/amazon-ecs-agent/ecs-agent/credentials/mocks"
Expand Down
2 changes: 1 addition & 1 deletion agent/api/task/taskvolume.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"github.com/aws/amazon-ecs-agent/agent/taskresource/fsxwindowsfileserver"
taskresourcetypes "github.com/aws/amazon-ecs-agent/agent/taskresource/types"
taskresourcevolume "github.com/aws/amazon-ecs-agent/agent/taskresource/volume"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment/resource"

"github.com/cihub/seelog"
"github.com/pkg/errors"
Expand Down
2 changes: 1 addition & 1 deletion agent/api/task/taskvolume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import (
mock_dockerapi "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi/mocks"
"github.com/aws/amazon-ecs-agent/agent/taskresource"
taskresourcevolume "github.com/aws/amazon-ecs-agent/agent/taskresource/volume"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment/resource"
apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status"
apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource"

"github.com/docker/docker/api/types/volume"
"github.com/golang/mock/gomock"
Expand Down
4 changes: 2 additions & 2 deletions agent/app/data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
"github.com/aws/amazon-ecs-agent/agent/engine/image"
"github.com/aws/amazon-ecs-agent/agent/statemanager"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment"
"github.com/aws/amazon-ecs-agent/ecs-agent/eventstream"
ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface"
"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -76,7 +76,7 @@ var (
}

testENIAttachment = &ni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
AttachmentInfo: attachment.AttachmentInfo{
AttachmentARN: testAttachmentArn,
AttachStatusSent: false,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,24 @@ package data
import (
"testing"

"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment"
"github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment/resource"
ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface"
"github.com/stretchr/testify/assert"
)

const (
testAttachmentArn = "arn:aws:ecs:us-west-2:167933679560:attachment/test-arn"
testAttachmentArn2 = "arn:aws:ecs:us-west-2:167933679560:attachment/test-arn2"
testAttachmentArn3 = "arn:aws:ecs:us-west-2:123456789012:volume/test-arn3"
testAttachmentArn4 = "arn:aws:ecs:us-west-2:123456789012:volume/test-arn4"
)

func TestManageENIAttachments(t *testing.T) {
testClient := newTestClient(t)

testEniAttachment := &ni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
AttachmentInfo: attachment.AttachmentInfo{
AttachmentARN: testAttachmentArn,
AttachStatusSent: false,
},
Expand All @@ -49,7 +52,7 @@ func TestManageENIAttachments(t *testing.T) {
assert.Equal(t, testAttachmentArn, res[0].AttachmentARN)

testEniAttachment2 := &ni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
AttachmentInfo: attachment.AttachmentInfo{
AttachmentARN: testAttachmentArn2,
AttachStatusSent: true,
},
Expand All @@ -67,15 +70,66 @@ func TestManageENIAttachments(t *testing.T) {
assert.Len(t, res, 0)
}

func TestManageResourceAttachments(t *testing.T) {
testClient := newTestClient(t)

testResAttachment := &resource.ResourceAttachment{
AttachmentInfo: attachment.AttachmentInfo{
AttachmentARN: testAttachmentArn3,
AttachStatusSent: false,
},
}

assert.NoError(t, testClient.SaveResourceAttachment(testResAttachment))
testResAttachment.SetSentStatus()
assert.NoError(t, testClient.SaveResourceAttachment(testResAttachment))
res, err := testClient.GetResourceAttachments()
assert.NoError(t, err)
assert.Len(t, res, 1)
assert.Equal(t, true, res[0].AttachStatusSent)
assert.Equal(t, testAttachmentArn3, res[0].AttachmentARN)

testResAttachment2 := &resource.ResourceAttachment{
AttachmentInfo: attachment.AttachmentInfo{
AttachmentARN: testAttachmentArn4,
AttachStatusSent: true,
},
}

assert.NoError(t, testClient.SaveResourceAttachment(testResAttachment2))
res, err = testClient.GetResourceAttachments()
assert.NoError(t, err)
assert.Len(t, res, 2)

assert.NoError(t, testClient.DeleteResourceAttachment("test-arn3"))
assert.NoError(t, testClient.DeleteResourceAttachment("test-arn4"))
res, err = testClient.GetResourceAttachments()
assert.NoError(t, err)
assert.Len(t, res, 0)
}

func TestSaveENIAttachmentInvalidID(t *testing.T) {
testClient := newTestClient(t)

testEniAttachment := &ni.ENIAttachment{
AttachmentInfo: attachmentinfo.AttachmentInfo{
AttachmentInfo: attachment.AttachmentInfo{
AttachmentARN: "invalid-arn",
AttachStatusSent: false,
},
}

assert.Error(t, testClient.SaveENIAttachment(testEniAttachment))
}

func TestSaveResAttachmentInvalidID(t *testing.T) {
testClient := newTestClient(t)

testResAttachment := &resource.ResourceAttachment{
AttachmentInfo: attachment.AttachmentInfo{
AttachmentARN: "invalid-arn",
AttachStatusSent: false,
},
}

assert.Error(t, testClient.SaveResourceAttachment(testResAttachment))
}
Loading

0 comments on commit 0e3b384

Please sign in to comment.