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

Windows daemon manager #4239

Merged
merged 6 commits into from
Jul 23, 2024

Conversation

saurabhc123
Copy link
Contributor

Summary

This PR adds the DaemonManager and ManagedDaemon components to support EBS-TaskAttach for Windows.

Implementation details

We refactored the existing DaemonManager and ManagedDaemon modules for linux and repurposed the high-level workflow. The platform specific details were moved to the respective windows/linux modules.

Testing

We used the existing linux tests and refactored them so that all of them can be repurposed for Windows.

New tests cover the changes: Yes

Description for the changelog

Add support EBS-TaskAttach for Windows.

Licensing

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

@saurabhc123 saurabhc123 requested a review from a team as a code owner July 8, 2024 21:27
@saurabhc123 saurabhc123 force-pushed the windowsDaemonManager branch 5 times, most recently from d7ea047 to 6661a29 Compare July 10, 2024 20:32
@mye956 mye956 force-pushed the feature/ebs-windows branch 2 times, most recently from 4179cea to 225bb4b Compare July 11, 2024 21:10
@saurabhc123 saurabhc123 force-pushed the windowsDaemonManager branch 3 times, most recently from 0046c02 to 67b17ce Compare July 12, 2024 23:23
agent/api/task/task.go Outdated Show resolved Hide resolved
agent/ebs/watcher_linux.go Outdated Show resolved Hide resolved
defaultECSAgentLogPathContainer = "/log"
socketPathHostRoot = "/var/run/ecs"
logPathHostRoot = "/log/daemons"
daemonUID = 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit (non-blocking): I'm seeing a lot of different config files that's scattered in different files. Perhaps we can try to scope it down a bit to having fewer config files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have refactored as much as possible. For some constants that are specific to a module, I have kept it to the module itself. Most are limited to the module. For the Linux ones, I refactored as much as possible, but didn't want to perturb the code too much. Let me know if you see any config that can be made global.

@jiuchoe4 jiuchoe4 force-pushed the windowsDaemonManager branch 3 times, most recently from 82fc629 to 64718fc Compare July 16, 2024 21:56
mye956
mye956 previously approved these changes Jul 17, 2024
Copy link
Contributor

@xxx0624 xxx0624 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mainly took a look at the ecs-agent changes which look good to me.

@mye956 mye956 merged commit 790b77b into aws:feature/ebs-windows Jul 23, 2024
40 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.

None yet

5 participants