-
Notifications
You must be signed in to change notification settings - Fork 22
update elastic_whenever for FARGATE launch type #34
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
Conversation
wata727
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@avinson Thanks for your contribution! There are several suggestions.
lib/elastic_whenever/task/target.rb
Outdated
| security_groups: [ security_groups ], | ||
| assign_public_ip: assign_public_ip, | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be able to specify group or platform_version?
https://docs.aws.amazon.com/AmazonCloudWatchEvents/latest/APIReference/API_EcsParameters.html
| end | ||
| opts.on('--subnets subnets', "Example: --subnets 'subnet-4973d63f,subnet-45827d1d'") do |subnets| | ||
| @subnets = subnets | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this option needs validation about launch type. For example, If security_groups or subnets are unspecified when launch_type is FARGATE, will they get an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wata727 I'm still thinking about this/looking into it. fyi, I did discover something kinda terrible about this AWS API in that it does not appear to validate either the subnets or the security groups. If you specify either a non-existent subnet or security group it will accept the event rule and then you'll get "invocation failures" in CloudWatch with no error logs or any indication of what's wrong.
Maybe it makes sense to do this validation in elastic_whenever? I presume that AWS will eventually add this on their end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@avinson Sorry for the late reply.
That's terrible... Perhaps I believe that validation is necessary for a better experience. Considering the behavior of the current AWS API, I think that it makes sense even if without the validation, but I'd like to add it as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
In addition, I think that it makes it should be able to choose the launch type from a configuration file. However, I think it is a problem to describe complex network settings in |
|
@wata727 thanks for the suggestions. I'll update my PR within the next week. |
|
@avinson Any updates? If you are not working on this, I will take over. |
feel free to pick up the PR.. i did a couple other improvements:
still needs:
|
|
@wata727 I'm happy to help contribute as well. This gem is standing in the way of us migrating to Fargate and would love to see this get merged! |
|
@swordfish444 Sorry I'm a little busy..., but I'm planning to work on this by the end of next week. |
Made a quick attempt to get this gem working with FARGATE launch type. example usage:
Happy to polish this up further if you have any suggestions.
See: https://aws.amazon.com/about-aws/whats-new/2018/08/aws-fargate-now-supports-time-and-event-based-task-scheduling/