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

Load Managed Daemon images in background #3984

Merged
merged 6 commits into from Oct 25, 2023
Merged

Conversation

amogh09
Copy link
Contributor

@amogh09 amogh09 commented Oct 25, 2023

Summary

Currently Agent loads Managed Daemon images synchronously on startup which causes a delay in startup. This PR addresses the startup delay by making Managed Daemon images load in background.

Behavior changes -

  • Managed Daemon image loading is still started on Agent startup but main thread no longer blocks on the image loading.
  • Capability for EBS Task Attach now relies on the image file's existence on the host rather than image load success. This is a trade-off we are making to speed up startup time.
  • EBS Watcher is updated to ensure that image load for EBS Managed Daemon has finished before it starts a Managed Daemon task.
  • EBS Managed Daemon task start is moved from EBS Watcher's EBS attachment payload handler to its tick handler. Note that an EBS Managed Daemon task can only be started after its image has been loaded. It is preferred to move the task start to tick handler for the following reasons.
    1. Tick handler is called repeatedly so there is no need to block it on image load and a tick can simply be skipped if the image is not loaded yet.
    2. Attachment payload handler is triggered by Agent receiving a new attachment payload from ECS backend, so its invocation is non-deterministic and concurrent. If we make the handler block on image load completion then we would need to acquire a lock to prevent multiple EBS Managed Daemon tasks from getting created. It is worth nothing that this issue exists in current design also but would be exacerbated by asynchronous image loading that's introduced by this PR.

Implementation details

  • A new function loadManagedDaemonImagesAsync is added to app package. This function loops through all registered Managed Daemons and starts loading the image for each. It adds the image to image cleanup exclusion list on successful image load.
  • imageManager.AddImageToCleanUpExclusionList method is updated to acquire the struct's RW lock to make the method thread-safe now that it's called concurrently.
  • appendEBSTaskAttachCapabilities function in app package is updated to add EBS Task attach capability based on EBS Managed Daemon's image file being found on the host instead of image load success.
  • EBS Managed Daemon task creation code is removed from EBS Watcher's HandleEBSResourceAttachment method. A new tick method is added for handling watcher ticks. In addition to how the tick is handled currently, tick method also calls a new daemonRunning method to check if EBS Managed Daemon task is running and does nothing if the Daemon task is not running.
  • daemonRunning method checks if a Daemon task exists and its state. If the task has not reached RUNNING state then the method returns true if task is already running and false if not running yet. Otherwise (either the task has not initialized or has stopped or is stopping) the method starts a new EBS Managed Daemon task.
  • A new ImageExists method is added to DaemonManager and an implementation is added for Linux. The implementation checks if the managed daemon's image tar file exists on the host.

Testing

Unit tests are added for all the changes.

Manual tests performed -

  • Checked that EBS backed tasks run as expected. Ran multiple tasks.
  • Checked that Agent is able to start a new EBS Managed Daemon if an existing one is stopped.
  • Restarted the Agent and checked that Agent is able to pick up an existing and running EBS Managed Daemon's state upon restart.

New tests cover the changes: yes

Description for the changelog

Load managed daemon images in background to speed up startup time.

Does this PR include breaking model changes? If so, Have you added transformation functions?

no

Licensing

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

@amogh09 amogh09 requested a review from a team as a code owner October 25, 2023 17:27
agent/ebs/watcher.go Outdated Show resolved Hide resolved
agent/app/agent.go Outdated Show resolved Hide resolved
agent/ebs/watcher.go Outdated Show resolved Hide resolved
fierlion
fierlion previously approved these changes Oct 25, 2023
mye956
mye956 previously approved these changes Oct 25, 2023
@amogh09 amogh09 merged commit 75db5cb into aws:dev Oct 25, 2023
38 checks passed
singholt added a commit to singholt/amazon-ecs-agent that referenced this pull request Oct 26, 2023
This reverts commit 75db5cb.

2. Revert "update daemon log path host root in container (aws#3982)"

This reverts commit 6166b6e.

3. Revert "Fix loading CSI driver container from state if it exists"

This reverts commit 1391502.

4. Revert "Latest FE model files (aws#3974)"

This reverts commit b589a08.

5. Revert "Move utils function Uint16SliceToStringSlice to shared lib"

This reverts commit 882bcb2.

6. Revert "Move remaining credentials module to shared library"

This reverts commit f58f228.

7. Revert "fix merge import failure (aws#3976)"

This reverts commit 46e21e6.

8. Revert "Fixing task volumes from payload for EBS-backed tasks"

This reverts commit fc97633.

9. Revert "Create Generic Attachment to decouple from ENI and save attachment state. (aws#3969)"

This reverts commit 12e23ed.

10. Revert "Add support for finding EBS devices on Xen instances (aws#3971)"

This reverts commit 3500ac2.

11. Revert "Add network builder and platform APIs (aws#3939)"

This reverts commit b8d2a7f.

12. Revert "Minor refactor of TTL cache (allow nil TTL + get TTL)"

This reverts commit 4b33664.

13. Revert "Move httpclient to ecs-agent/ and minor refactor"

This reverts commit 041ca3f.

14. Revert "Add missing EBSTaskAttach changes from feature branch"

This reverts commit 7a0d116.
singholt added a commit to singholt/amazon-ecs-agent that referenced this pull request Oct 26, 2023
1. Revert "Load Managed Daemon images in background (aws#3984)"

This reverts commit 75db5cb.

2. Revert "update daemon log path host root in container (aws#3982)"

This reverts commit 6166b6e.

3. Revert "Fix loading CSI driver container from state if it exists"

This reverts commit 1391502.

4. Revert "Latest FE model files (aws#3974)"

This reverts commit b589a08.

5. Revert "Move utils function Uint16SliceToStringSlice to shared lib"

This reverts commit 882bcb2.

6. Revert "Move remaining credentials module to shared library"

This reverts commit f58f228.

7. Revert "fix merge import failure (aws#3976)"

This reverts commit 46e21e6.

8. Revert "Fixing task volumes from payload for EBS-backed tasks"

This reverts commit fc97633.

9. Revert "Create Generic Attachment to decouple from ENI and save attachment state. (aws#3969)"

This reverts commit 12e23ed.

10. Revert "Add support for finding EBS devices on Xen instances (aws#3971)"

This reverts commit 3500ac2.

11. Revert "Add network builder and platform APIs (aws#3939)"

This reverts commit b8d2a7f.

12. Revert "Minor refactor of TTL cache (allow nil TTL + get TTL)"

This reverts commit 4b33664.

13. Revert "Move httpclient to ecs-agent/ and minor refactor"

This reverts commit 041ca3f.

14. Revert "Add missing EBSTaskAttach changes from feature branch"

This reverts commit 7a0d116.
singholt added a commit that referenced this pull request Oct 26, 2023
1. Revert "Load Managed Daemon images in background (#3984)"

This reverts commit 75db5cb.

2. Revert "update daemon log path host root in container (#3982)"

This reverts commit 6166b6e.

3. Revert "Fix loading CSI driver container from state if it exists"

This reverts commit 1391502.

4. Revert "Latest FE model files (#3974)"

This reverts commit b589a08.

5. Revert "Move utils function Uint16SliceToStringSlice to shared lib"

This reverts commit 882bcb2.

6. Revert "Move remaining credentials module to shared library"

This reverts commit f58f228.

7. Revert "fix merge import failure (#3976)"

This reverts commit 46e21e6.

8. Revert "Fixing task volumes from payload for EBS-backed tasks"

This reverts commit fc97633.

9. Revert "Create Generic Attachment to decouple from ENI and save attachment state. (#3969)"

This reverts commit 12e23ed.

10. Revert "Add support for finding EBS devices on Xen instances (#3971)"

This reverts commit 3500ac2.

11. Revert "Add network builder and platform APIs (#3939)"

This reverts commit b8d2a7f.

12. Revert "Minor refactor of TTL cache (allow nil TTL + get TTL)"

This reverts commit 4b33664.

13. Revert "Move httpclient to ecs-agent/ and minor refactor"

This reverts commit 041ca3f.

14. Revert "Add missing EBSTaskAttach changes from feature branch"

This reverts commit 7a0d116.
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.

None yet

5 participants