Add initial work on new spot instance module #407

srirachanaachyuthuni · 2021-07-19T16:18:55Z

SUMMARY

Working on a new EC2 spot instance module

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

ec2_spot_instance

ADDITIONAL INFORMATION

ansibullbot · 2021-07-19T16:27:00Z

cc @jillr @s-hertel @tremble @wimnat
click here for bot help

plugins/modules/ec2_spot_instance.py

jillr

The module currently has a number of options specified as parameters that need to be subkeys of LaunchSpecification. We could list them all out individually and then build a LaunchSpecification from the params, but if we do that we shouldn't also have a separate launch_specification option.

Please make sure your API calls are wrapped in try/except blocks and that both botocore.exceptions.BotoCoreError and botocore.exceptions.ClientError are handled in the except. Please also return information about the created spot request (and document the return data). Good work so far!

plugins/modules/ec2_spot_instance.py

* Remove ec2_vpc_nat_gateway unit tests * update comments in ec2_vpc_nat_gateway integration tests - they're not based on the hard coded check_mode results any more This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@701153a

plugins/modules/ec2_spot_instance.py

jillr · 2021-07-21T22:53:22Z

plugins/modules/ec2_spot_instance.py

+    choices: [ "one-time", "persistent" ]
+    type: str
+  valid_from:
+    description:


Ansible doesn't support datetime parameters so the best thing we can probably do is describe what the data needs to look like, take it as a string, and convert it. I don't love this option though. :( I can try to find an example of a module that does something similar but I don't know of one offhand.

jillr · 2021-07-21T22:55:56Z

plugins/modules/ec2_spot_instance.py

+    default: []
+    type: list


Suggested change

default: []

type: list

default: {}

type: dict

Tags should be a dictionary, there's a module util for doing most of the work. https://github.com/ansible-collections/amazon.aws/blob/main/plugins/module_utils/ec2.py#L908

plugins/modules/ec2_spot_instance.py

jillr · 2021-07-21T23:04:37Z

plugins/modules/ec2_spot_instance.py

+        count=dict(type='int', default=1),
+        interruption=dict(type='str', default="terminate"),
+        launch_group=dict(type='str', default='  '),
+        launch_specification=dict(type='dict', default=dict()),


When you document this key, it can be helpful for readability to put the suboptions in a separate var like this https://github.com/ansible-collections/amazon.aws/blob/main/plugins/modules/ec2_ami.py#L703

Please make sure the suboptions of these arguments:
launch_specification
block_device_mappings
network_interfaces
placement
monitoring

Are properly defined as such. You need an options key in the arg spec for each parameter that contains the spec for the suboptions.
https://docs.ansible.com/ansible/devel/dev_guide/developing_program_flow_modules.html#argument-spec

plugins/modules/ec2_spot_instance.py

…ement

plugins/modules/ec2_spot_instance.py

jillr · 2021-07-27T23:59:35Z

plugins/modules/ec2_spot_instance.py

+        count=dict(type='int', default=1),
+        interruption=dict(type='str', default="terminate"),
+        launch_group=dict(type='str', default='  '),
+        launch_specification=dict(type='dict', default=dict()),


Please make sure the suboptions of these arguments:
launch_specification
block_device_mappings
network_interfaces
placement
monitoring

Are properly defined as such. You need an options key in the arg spec for each parameter that contains the spec for the suboptions.
https://docs.ansible.com/ansible/devel/dev_guide/developing_program_flow_modules.html#argument-spec

jillr · 2021-07-28T00:02:40Z

plugins/modules/ec2_spot_instance.py

+
+    if state == 'absent':
+        response = cancel_spot_instance_requests(module, connection)
+        changed = True


This will report True even if the spot instance is already cancelled or does not exist. Please describe_spot_instance_requests() first to determine if there is work to do before setting changed status/running the cancellation.

plugins/modules/ec2_spot_instance.py

jillr · 2021-07-28T00:11:42Z

plugins/modules/ec2_spot_instance.py

+
+    if state == 'present':
+        response = request_spot_instances(module, connection)
+        changed = True


What does the API do if a request that matches all the parameters already exists? Does it recreate it or is there no change?

I have been running the same playbook for testing. It creates a new request each time since even if the parameters are the same, Boto3 considers it as a new request for a spot instance with the same configuration.

In this case we should handle idempotency inside the module by checking for existing requests and comparing them to the specification then. Let's get the current functionality sorted out and then we can work on this.

plugins/modules/ec2_spot_instance.py

tremble · 2021-07-30T08:12:29Z

plugins/modules/ec2_spot_instance.py

+    - >
+      Note: This module uses the boto3 Python module to interact with the EC2 API.
+      M(amazon.aws.ec2) will still support the older boto Python module to interact with spot instances.


We can lose this note. The old boto SDK is no longer supported upstream and by adding the document fragment we automatically add a note about which Python modules we require.

* Remove ec2_vpc_nat_gateway unit tests * update comments in ec2_vpc_nat_gateway integration tests - they're not based on the hard coded check_mode results any more This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@701153a

…n up, more tests

* Remove ec2_vpc_nat_gateway unit tests * update comments in ec2_vpc_nat_gateway integration tests - they're not based on the hard coded check_mode results any more This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@701153a

plugins/modules/ec2_spot_instance.py

* Remove ec2_vpc_nat_gateway unit tests * update comments in ec2_vpc_nat_gateway integration tests - they're not based on the hard coded check_mode results any more This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@701153a

alinabuzachis

LGTM!

zuul doesn't like -1 reviews

ansible-zuul

LGTM!

jillr · 2021-08-31T16:52:27Z

Thanks for all your hard work on this @srirachanaachyuthuni, it's merging into the collection and will be released soon!

* Remove ec2_vpc_nat_gateway unit tests * update comments in ec2_vpc_nat_gateway integration tests - they're not based on the hard coded check_mode results any more

Add initial work on new spot instance module

9ff70ef

ansibullbot added community_review module module needs_triage new_module New module new_plugin New plugin plugins plugin (any type) labels Jul 19, 2021

Changed some boto2 references to boto3

92401a7

jillr reviewed Jul 19, 2021

View reviewed changes

plugins/modules/ec2_spot_instance.py Outdated Show resolved Hide resolved

Add cancel spot instance request module

3bd0d33

jillr reviewed Jul 20, 2021

View reviewed changes

alinabuzachis removed the needs_triage label Jul 20, 2021

Add suboptions, catch Boto exceptions and remove block_duration

aead9cb

Add return, RETURN block and examples

e814fdc

jillr reviewed Jul 21, 2021

View reviewed changes

goneri reviewed Jul 22, 2021

View reviewed changes

plugins/modules/ec2_spot_instance.py Show resolved Hide resolved

Add suboptions for block_device_mappings, network_interfaces and plac…

c62ce42

…ement

jillr reviewed Jul 28, 2021

View reviewed changes

Add seperate options dictionary and modify delete functionality

10fed95

tremble previously requested changes Jul 30, 2021

View reviewed changes

Docs updates, get the building of the launch spec working, minor clea…

6fef7d4

…n up, more tests

jillr changed the title ~~(WIP) Add initial work on new spot instance module~~ Add initial work on new spot instance module Aug 24, 2021

jillr requested a review from tremble August 24, 2021 23:34

alinabuzachis requested changes Aug 26, 2021

View reviewed changes

Address review comments

bed46f9

jillr requested review from alinabuzachis and goneri August 30, 2021 16:54

sanity fixups

eb2df32

jillr force-pushed the ec2_spot_instance branch from b94e34c to eb2df32 Compare August 31, 2021 00:35

alinabuzachis approved these changes Aug 31, 2021

View reviewed changes

jillr added the gate label Aug 31, 2021

ansible-zuul bot approved these changes Aug 31, 2021

View reviewed changes

jillr added gate and removed gate labels Aug 31, 2021

ansible-zuul bot merged commit 62e1387 into ansible-collections:main Aug 31, 2021

tremble mentioned this pull request Sep 6, 2021

Spot instance support ansible-collections/community.aws#704

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial work on new spot instance module #407

Add initial work on new spot instance module #407

srirachanaachyuthuni commented Jul 19, 2021

ansibullbot commented Jul 19, 2021

jillr left a comment

jillr Jul 21, 2021

jillr Jul 21, 2021

jillr Jul 21, 2021

jillr Jul 27, 2021

jillr Jul 27, 2021

jillr Jul 28, 2021

jillr Jul 28, 2021

srirachanaachyuthuni Jul 28, 2021

jillr Jul 28, 2021

srirachanaachyuthuni Jul 29, 2021

tremble Jul 30, 2021 •

edited

Loading

alinabuzachis left a comment

ansible-zuul bot left a comment

jillr commented Aug 31, 2021

Add initial work on new spot instance module #407

Add initial work on new spot instance module #407

Conversation

srirachanaachyuthuni commented Jul 19, 2021

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibullbot commented Jul 19, 2021

jillr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tremble Jul 30, 2021 • edited Loading

Choose a reason for hiding this comment

alinabuzachis left a comment

Choose a reason for hiding this comment

ansible-zuul bot left a comment

Choose a reason for hiding this comment

jillr commented Aug 31, 2021

tremble Jul 30, 2021 •

edited

Loading