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

ec2_instance: Allow auto selection of ebs optimized #61222

Open
wants to merge 10 commits into
base: devel
from
@@ -0,0 +1,2 @@
minor_changes:
- ec2_instance - ``auto`` mode for ebs optimization, allowing to automatically enable ebs optimization when instance supports it
@@ -167,8 +167,9 @@
type: bool
ebs_optimized:
description:
- Whether instance is should use optimized EBS volumes, see U(https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSOptimized.html).
type: bool
- Whether instance should be EBS optimized, see U(https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSOptimized.html).
- The "auto" value will enable EBS optimization if the instance supports it.
- Valid values are "true, false, auto"
filters:
description:
- A dict of filters to apply when deciding whether existing instances match and should be altered. Each dict item
@@ -717,6 +718,7 @@
from ansible.module_utils.six import text_type, string_types
from ansible.module_utils.six.moves.urllib import parse as urlparse
from ansible.module_utils._text import to_bytes, to_native
from ansible.module_utils.parsing.convert_bool import boolean
import ansible.module_utils.ec2 as ec2_utils
from ansible.module_utils.ec2 import (boto3_conn,
ec2_argument_spec,
@@ -1084,6 +1086,13 @@ def discover_security_groups(group, groups, parent_vpc_id=None, subnet_id=None,
return list(dict((g['GroupId'], g) for g in found_groups).values())


def _supports_ebs_optimized(instance_type):
ebs_optimized_types = ['a1', 'c4', 'c5', 'd2', 'f1', 'g3', 'h1', 'i3', 'm4', 'm5', 'p2', 'p3', 'r4', 'r5', 't3', 'u-', 'x1', 'z1']

This comment has been minimized.

Copy link
@willthames

willthames Sep 2, 2019

Contributor

This doesn't seem hugely future proof, but the alternative might be a bit harder.

I think the pricing api might be able to help but with significantly more work - ec2instances.info seems to use the API to reasonable effect: https://github.com/powdahound/ec2instances.info/blob/master/ec2.py

Not a blocker though

This comment has been minimized.

Copy link
@willthames

willthames Sep 2, 2019

Contributor

Looking at this again, I can see no simple way of getting this info from the pricing info.

It seems to be: (set of instance types with dedicatedEbsThroughput set) - (set of instance types with ebsOptimized set) - {'r2','t3'}

aws pricing get-products --service-code=AmazonEC2 --region us-east-1  --filters \
Type=TERM_MATCH,Field=operatingSystem,Value=RHEL \
Type=TERM_MATCH,Field=capacitystatus,Value=AllocatedCapacityReservation \
Type=TERM_MATCH,Field=location,Value="US East (N. Virginia)" \
--query PriceList[]  --output text | sed 's/  /^M/g' | grep dedicatedEbsThroughput | \
while read line; do echo $line | jq -r .product.attributes.instanceType ; done | \
awk -F. '{print $1}' | sort -u
aws pricing get-products --service-code=AmazonEC2 --region us-east-1 --filters \
Type=TERM_MATCH,Field=ebsOptimized,Value=Yes \
--query PriceList[] --output text | sed 's/  /^M/g' | \
while read line; do echo $line | jq -r .product.attributes.instanceType ; done | \
awk -F. '{print $1}' | sort -u

This comment has been minimized.

Copy link
@jillr

jillr Sep 3, 2019

Contributor

I'd also prefer not to hardcode this list, but if there's no reasonable alternative I'd be ok with it.

This comment has been minimized.

Copy link
@Shaps

Shaps Sep 4, 2019

Author Contributor

One thing to consider with using the pricing API ( or any other API, really ), is that we'd be adding additional API calls to each instance, potentially impacting the performance of the module.
( Thanks @s-hertel for pointing it out )

if instance_type[:2] in ebs_optimized_types:
return True
return False


def build_top_level_options(params):
spec = {}
if params.get('image_id'):
@@ -1130,7 +1139,11 @@ def build_top_level_options(params):
if params.get('placement_group'):
spec.setdefault('Placement', {'GroupName': str(params.get('placement_group'))})
if params.get('ebs_optimized') is not None:
spec['EbsOptimized'] = params.get('ebs_optimized')
if params.get('ebs_optimized') == 'auto':
ebs_optimized = True if _supports_ebs_optimized(params.get('instance_type')) else False
This conversation was marked as resolved by s-hertel

This comment has been minimized.

Copy link
@jillr

jillr Sep 3, 2019

Contributor
Suggested change
ebs_optimized = True if _supports_ebs_optimized(params.get('instance_type')) else False
ebs_optimized = _supports_ebs_optimized(params.get('instance_type'))

This comment has been minimized.

Copy link
@Shaps

Shaps Sep 4, 2019

Author Contributor

erm.. Not sure why I put that there. Fixed, thanks! :)

else:
ebs_optimized = boolean(params.get('ebs_optimized'))
spec['EbsOptimized'] = ebs_optimized
if params.get('instance_initiated_shutdown_behavior'):
spec['InstanceInitiatedShutdownBehavior'] = params.get('instance_initiated_shutdown_behavior')
if params.get('termination_protection') is not None:
@@ -1587,7 +1600,7 @@ def main():
instance_type=dict(default='t2.micro', type='str'),
user_data=dict(type='str'),
tower_callback=dict(type='dict'),
ebs_optimized=dict(type='bool'),
ebs_optimized=dict(type='raw'),

This comment has been minimized.

Copy link
@s-hertel

s-hertel Sep 9, 2019

Contributor

If someone passes something other than a boolean or 'auto', boolean() will cause a TypeError. Using choices with a list containing 'auto' and all the possible booleans for choices is one solution, or you could use https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/common/validation.py#L436 and if the value isn't 'auto' or a valid boolean fail at the beginning of the module for invalid type.

LGTM besides that. The hardcoding is unfortunate but seems like the best choice for now (once this module can spin up multiple instances at once we could reconsider the try/except approach).

vpc_subnet_id=dict(type='str', aliases=['subnet_id']),
availability_zone=dict(type='str'),
security_groups=dict(default=[], type='list'),
@@ -32,3 +32,56 @@
assert:
that:
- "{{ ebs_opt_instance_info.instances.0.ebs_optimized }}"


- name: Use 'auto' to create EBS optimized instance
ec2_instance:
name: "{{ resource_prefix }}-test-auto-ebs-optimized-instance-in-vpc"
image_id: "{{ ec2_ebs_optimized_ami_image[aws_region] }}"
tags:
TestId: "{{ resource_prefix }}"
security_groups: "{{ sg.group_id }}"
vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}"
ebs_optimized: auto
instance_type: t3.nano
wait: false
<<: *aws_connection_info
register: auto_ebs_opt

- name: Get ec2 instance info
ec2_instance_info:
filters:
"tag:Name": "{{ resource_prefix }}-test-auto-ebs-optimized-instance-in-vpc"
<<: *aws_connection_info
register: ebs_auto_opt_instance_info

- name: Assert instance is ebs optimized
assert:
that:
- "{{ ebs_auto_opt_instance_info.instances.0.ebs_optimized }}"

- name: Use 'auto' to create non EBS optimized instance
ec2_instance:
name: "{{ resource_prefix }}-test-auto-non-ebs-optimized-instance-in-vpc"
image_id: "{{ ec2_ebs_optimized_ami_image[aws_region] }}"
tags:
TestId: "{{ resource_prefix }}"
security_groups: "{{ sg.group_id }}"
vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}"
ebs_optimized: auto
instance_type: t2.nano
wait: false
<<: *aws_connection_info
register: auto_non_ebs_opt

- name: Get ec2 instance info
ec2_instance_info:
filters:
"tag:Name": "{{ resource_prefix }}-test-auto-non-ebs-optimized-instance-in-vpc"
<<: *aws_connection_info
register: non_ebs_opt_auto_instance_info

- name: Assert instance is not ebs optimized
assert:
that:
- "{{ non_ebs_opt_auto_instance_info.instances.0.ebs_optimized }} == False"
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.