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.

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

Move ACS session to ecs-agent module and refactor #3887

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

danehlim
Copy link
Contributor

@danehlim danehlim commented Sep 5, 2023

Summary

Move ACS session to ecs-agent module and refactor associated code.

Implementation details

  • agent/acs/handler/acs_handler.go -> ecs-agent/acs/session/session.go
    • Various code enhancement changes and making the ACS session more general such that it makes less assumptions about the underlying Agent making use of the session. Among the most notable are:
      • Session's Start method and other related methods it calls now take in a context.Context in favor of storing a context.Context as a struct field in the whole session itself
      • Session's Start method also now returns the error code returned by the context, which provides meaningful information about why the Session ended (as opposed to current behavior, where the method always returns nil)
      • Pass in the following pre-initialized as session struct fields instead of determining what they are from within the session itself: agentVersion, agentHash, dockerVersion, inactiveInstanceCB() function, PayloadMessageHandler, CredentialsMetadataSetter, ENIHandler, ResourceHandler, ManifestMessageIDAccessor, SequenceNumberAccessor, TaskComparer, TaskStopper, metrics.EntryFactory, and *wsclient.WSClientMinAgentConfig
  • agent/acs/handler/acs_handler_test.go -> ecs-agent/acs/session/session_test.go
    • Update ACS session tests accordingly based off of above changes to ACS session
    • General cleanup/quality of life updates to tests (e.g., add additional tests, use assert package for test assertions instead of if statement blocks, use defined test constants instead of hard coding test values at the test level, remove initialization and usage of some unnecessary variables, update some code comments to be more clear, etc.)
  • agent/app/agent.go
    • Make corresponding changes based off of above changes to ACS session (e.g., update what is passed into the session, how the return value of its Start method is handled)

Testing

Unit, integration, and functional tests.

New tests cover the changes: yes

Description for the changelog

Move ACS session to ecs-agent module and refactor

Licensing

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

@danehlim danehlim marked this pull request as ready for review September 5, 2023 23:59
@danehlim danehlim requested a review from a team as a code owner September 5, 2023 23:59
agent/app/agent.go Outdated Show resolved Hide resolved
agent/app/agent.go Outdated Show resolved Hide resolved
ecs-agent/acs/session/session.go Outdated Show resolved Hide resolved
agent/app/agent.go Outdated Show resolved Hide resolved
ecs-agent/acs/session/session.go Outdated Show resolved Hide resolved
ecs-agent/acs/session/session.go Show resolved Hide resolved
ecs-agent/acs/session/session.go Show resolved Hide resolved
ecs-agent/acs/session/session.go Show resolved Hide resolved
ecs-agent/acs/session/session.go Outdated Show resolved Hide resolved
ecs-agent/acs/session/session.go Outdated Show resolved Hide resolved
ecs-agent/acs/session/session_test.go Show resolved Hide resolved
ecs-agent/acs/session/session_test.go Show resolved Hide resolved
ecs-agent/acs/session/session_test.go Show resolved Hide resolved
ecs-agent/acs/session/session_test.go Show resolved Hide resolved
@danehlim danehlim merged commit 06b7c01 into aws:dev Sep 7, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants