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

update elastic_whenever for FARGATE launch type #34

Merged
merged 4 commits into from Dec 16, 2018
Merged

update elastic_whenever for FARGATE launch type #34

merged 4 commits into from Dec 16, 2018

Conversation

avinson
Copy link
Contributor

@avinson avinson commented Oct 1, 2018

Made a quick attempt to get this gem working with FARGATE launch type. example usage:

elastic_whenever --region us-west-2 --launch_type FARGATE --security_groups sg-2c503655,sg-72f0cb0a --subnets subnet-4973d63f,subnet-45827d1d --update <MY_SERVICE>

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/

Copy link
Owner

@wata727 wata727 left a 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.

security_groups: [ security_groups ],
assign_public_ip: assign_public_ip,
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

end
opts.on('--subnets subnets', "Example: --subnets 'subnet-4973d63f,subnet-45827d1d'") do |subnets|
@subnets = subnets
end
Copy link
Owner

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?

Copy link
Contributor Author

@avinson avinson Oct 18, 2018

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.

Copy link
Owner

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.

Choose a reason for hiding this comment

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

@avinson @wata727 For now it would be best to add a TODO comment to keep it top of mind.

@wata727
Copy link
Owner

wata727 commented Oct 2, 2018

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 schedule.rb, so it may be better to separate them into separate files.

@avinson
Copy link
Contributor Author

avinson commented Oct 10, 2018

@wata727 thanks for the suggestions. I'll update my PR within the next week.

@wata727
Copy link
Owner

wata727 commented Nov 17, 2018

@avinson Any updates? If you are not working on this, I will take over.

@avinson
Copy link
Contributor Author

avinson commented Nov 21, 2018

@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:

  • add option validation (should throw error if launch type is Fargate but subnets and seucrity_groups is nil)
  • fix tests
  • separate config file for complex network/fargate settings. I'm happy to help with this but was wondering what you were thinking for the format

@swordfish444
Copy link

@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!

@wata727
Copy link
Owner

wata727 commented Dec 7, 2018

@swordfish444 Sorry I'm a little busy..., but I'm planning to work on this by the end of next week.

@wata727 wata727 merged commit e7fb6fa into wata727:master Dec 16, 2018
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

3 participants