ec2_asg: add support for Spot allocation #418
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tremble
requested changes
Apr 12, 2021
There was a problem hiding this comment.
Thanks for taking the time to open this PR. I'm sorry it's taken a while to get this reviewed
A couple of minor comments inline.
Additionally:
- Please add a changelog fragment (https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to)
- Please update the integration tests (tests/integration/targets/ec2_asg) this will help ensure that the change works and that future changes don't break it. (https://www.ansible.com/blog/getting-started-with-aws-ansible-module-development)
- ec2_asg has been changed since your PR was initially created, please rebase against the latest main branch (https://git-scm.com/book/en/v2/Git-Branching-Rebasing)
| @@ -1661,6 +1677,8 @@ def main(): | |||
| type='list', | |||
| elements='str' | |||
| ), | |||
| ondemand_percentage_above_base=dict(type='int'), | |||
| ondemand_base=dict(type='int') | |||
There was a problem hiding this comment.
Suggested change
| ondemand_base=dict(type='int') | |
| ondemand_base=dict(type='int'), |
By leaving the comma at the end it simplifies patches for future features (they don't need to change this line, they just add the next one). Which then helps if you ever need to use git blame
| @@ -93,6 +93,14 @@ | |||
| - A list of instance_types. | |||
| type: list | |||
| elements: str | |||
| ondemand_base: | |||
| description: | |||
| - the minimum amount of capacity that must be fulfilled by On-Demand instances | |||
There was a problem hiding this comment.
Suggested change
| - the minimum amount of capacity that must be fulfilled by On-Demand instances | |
| - "A baseline of the ASG's capacity which will be fulfilled using only on-demand instances." |
| type: int | ||
| ondemand_percentage_above_base: | ||
| description: | ||
| - percentages of on-demand instances beyond OnDemandBaseCapacity |
There was a problem hiding this comment.
Suggested change
| - percentages of on-demand instances beyond OnDemandBaseCapacity | |
| - Defines the percentage of Instances above I(ondemand_base) which will be provisioned using on-demand Instances. | |
| - The first I(ondemand_base) instances will be provisioned using on-demand instances. | |
| - Capacity above I(ondemand_base) will be split between on-demand- and spot- instances based upon I(ondemand_percentage_above_base). |
|
This functionality seem to be already added and supported in the current |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
This should allow to create an AutoScalingGroup with 100% spot allocation.
ISSUE TYPE
COMPONENT NAME
ec2_asg
ADDITIONAL INFORMATION
Very quick and dirty.