Refactor ACS attach instance ENI message handling

[danehlim]

Summary

Refactor ACS attach instance ENI message handling, clean up tests, and minor general refactor (mainly naming + file structure) of some items relevant to ACS responders dealing with ENI attachment.

Implementation details

This pull request has been split into two commits for easier readability of separation between attach instance ENI refactoring specific changes and general ENI minor refactoring changes.

Please note that, in the context of this PR, "ACS message responder"/"responder" is more or less synonymous with "ACS message handler"/"handler".

--- COMMIT 1 (click here to view associated changes) ---

1. Minor refactor (mainly naming + file structure) of things relevant to ENI attachment through ACS responders
   • Move/consolidate some testing constants used for ACS responders dealing with ENI attachment into centralized file ecs-agent/acs/session/testconst/test_const.go
   • Rename ValidateTaskENI function in ecs-agent/api/eni/eni.go to ValidateENI as its usage is not restricted/specific to task ENI
   • Rename some attach task ENI responder test functions to indicate they are specific to task ENI
   • Update NewAttachTaskENIResponder function in ecs-agent/acs/session/attach_task_eni_responder.go to return type wsclient.RequestResponder instead of *attachTaskENIResponder
   • Replace inline anonymous function for handling task ENI attachment in ecs-agent/acs/session/attach_task_eni_responder.go with its own separate function
   • Sort some unsorted imports
   • Remove sendAck function from agent/acs/handler/attach_eni_handler_common.go as this function is no longer used/needed after the changes from this pull request

--- COMMIT 2 (click here to view associated changes) ---

2. Refactor ACS attach instance ENI message handling and move to ecs-agent/
   • agent/acs/handler/attach_instance_eni_handler.go -> ecs-agent/acs/session/attach_instance_eni_responder.go
     • Refactor attachInstanceENIHandler struct and existing code into attachInstanceENIResponder struct, such that attachInstanceENIResponder struct implements the wsclient.RequestResponder interface
     • Handle ENI attachment in async (no reason for this to be a blocking operation)
     • Consider an attach instance ENI message with multiple network interface devices to be valid, in the event that ACS does send one
       • A similar change has already been previously done for payload messages (see here) and attach task ENI messages (see here), this is simply just a followup to those
     • Perform more validation of the ENI(s) in the attach instance ENI message than just validating MAC address is present
     • Log with level ERROR instead of WARN when attach instance ENI message is invalid
     • Use github.com/aws/amazon-ecs-agent/ecs-agent/logger instead of github.com/cihub/seelog for logging
   • Update tests accordingly based off of above changes
     • agent/acs/handler/attach_instance_eni_handler_test.go (partial) -> agent/acs/handler/attach_instance_eni_responder_test.go
       • Tests that need to interact with Agent state belong in this file
     • agent/acs/handler/attach_instance_eni_handler_test.go (partial) -> ecs-agent/acs/session/attach_instance_eni_responder_test.go
       • Tests that need to interact only with stateless components of the responder (e.g., message validation) belong in this file
   • General cleanup of tests, most notably:
     • Increase test coverage
     • Eliminate redundant initialization of test attachInstanceENIMessage variables for each attach instance ENI responder test that requires one
   • agent/acs/handler/acs_handler.go
     • Initialize attach instance ENI responder and add its HandlerFunc function as a request handler to the ACS client, such that attach instance ENI responder dictates what to do in response to receiving attach instance ENI messages from ACS
       • Eventually, each ACS message type will have its own responder (which implements the wsclient.RequestResponder interface) that dictates what to do in response to ACS messages of that specific type

Testing

Unit, integration, and functional tests.

New tests cover the changes: yes

Description for the changelog

Refactor ACS attach instance ENI message handling

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[Realmonia]

Is HandleENIAttachment Implemented differently by ec2 and fargate? How different are they? Can it be put into ecs-agent module too?

[danehlim]

@Realmonia:

Is HandleENIAttachment Implemented differently by ec2 and fargate? How different are they? Can it be put into ecs-agent module too?

HandleENIAttachment is implemented significantly differently between EC2 and Fargate. That is, each launch type makes use of significantly different implementations of the ENIHandler interface. Thus, each implementation should continue to live in their respective code base instead of in ecs-agent module.

[Realmonia]

Thanks!