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.

Sign up for GitHub

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

Jump to bottom

DTPK-209 feat: add ecs features #23

Merged
merged 16 commits into from
Aug 15, 2022
Merged

DTPK-209 feat: add ecs features #23

merged 16 commits into from
Aug 15, 2022

Conversation

bemillenium
Copy link
Contributor

@bemillenium bemillenium commented Aug 3, 2022 •
edited
Loading

Submit a pull request 🚀

Thank you for help us contribute! Please give us more information about this PR.

What 😗

Adds

  • Options in aws_ecs_service for EC2 launch type with Auto Scaling Group

    • capacity_provider_strategy
    • ordered_placement_strategy
    • unix_max_connection

  • Options for All ECS type

    • entry_point
    • command
    • volumes efs
    • deployment circuit breaker

Why 🥺

  • Add options for support capacity provider cluster for scale the node up when the container is pending.
  • add support unix_max_connections by increasing max socket in EC2 launch type to make the task the best performance in networking.
  • Add options for ECS to support override things
  • Add circuit breaker to stop deploying the failed task and roll back to the previous one.

@bemillenium bemillenium requested a review from a team August 3, 2022 09:48
@bemillenium bemillenium self-assigned this Aug 3, 2022
@bemillenium bemillenium added the enhancement New feature or request label Aug 3, 2022
@bemillenium bemillenium marked this pull request as ready for review August 3, 2022 09:49
@xshot9011
Copy link
Contributor

Will check tmr

@xshot9011
Copy link
Contributor

In .gitignore plz update

*.tfvars
!terraform.*example*.tfvars

@xshot9011
Copy link
Contributor

For example in root level; README.md
I think you can move it to "examples/complete," since that's almost configurable for this module.

@xshot9011
Copy link
Contributor

Last thing I saw in variables.tf
variable name -> envvars -> this one will always create envvars for service
Can we make it like empty by default

@xshot9011
Copy link
Contributor

LGTM for me

@bemillenium
Copy link
Contributor Author

@xshot9011 Please review again

xshot9011
xshot9011 reviewed Aug 15, 2022
View reviewed changes
examples/ecs_ec2_asg/variables.example Outdated Show resolved Hide resolved
examples/simple/variables.example Outdated Show resolved Hide resolved
examples/ecs_ec2_asg/variables.tf Outdated Show resolved Hide resolved
examples/ecs_ec2_asg/main.tf Outdated Show resolved Hide resolved
examples/simple/main.tf Outdated Show resolved Hide resolved
locals.tf Show resolved Hide resolved
examples/ecs_ec2_asg/main.tf Outdated Show resolved Hide resolved
locals.tf Show resolved Hide resolved
examples/simple/variables.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
xshot9011
xshot9011 approved these changes Aug 15, 2022
View reviewed changes
Copy link
Contributor

@xshot9011 xshot9011 left a comment

Choose a reason for hiding this comment

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

LGTM

@bemillenium bemillenium merged commit 4a3b553 into main Aug 15, 2022
@bemillenium bemillenium deleted the feat/add-cp branch August 15, 2022 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xshot9011 xshot9011 xshot9011 approved these changes

Assignees

@bemillenium bemillenium

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bemillenium @xshot9011