Add asg refresh and info modules #487

danquixote · 2021-03-19T21:20:37Z

SUMMARY

Resubmitting rebased-PR after initial review on #425 by @tremble .
Overview:
Adding the ec2_asg_instance_refresh and related *_info module. These modules are intended to be used together to start or cancel an EC2 AutoScaling Group (ASG) instance refresh, and then track the subsequent progress with the provided InstanceRefreshId. The *_info module can also be used to get multiple pages of refresh history using the NextToken.
Fixes #425

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

ec2_asg_instance_refresh
ec2_asg_instance_refreshes_info

ADDITIONAL INFORMATION


{
           'instance_refresh_id': 'string'
 }


 {
            'instance_refreshes': [
                    {
                        'instance_refresh_id': '6507a3e5-4950-4503-8978-e9f2636efc09',
                        'auto_scaling_group_name': 'ansible-test-hermes-63642726-asg',
                        'status': 'Cancelled',
                        'status_reason': 'Cancelled due to user request.',
                        'start_time': '2021-02-04T03:39:40+00:00',
                        'end_time': '2021-02-04T03:41:18+00:00',
                        'percentage_complete': 0,
                        'instances_to_update': 1
                    }
            ],
            'next_token': 'string'
        }

ansibullbot · 2021-03-19T21:25:55Z

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

danquixote · 2021-03-19T22:14:49Z

/rebuild_failed

goneri · 2021-04-30T19:04:58Z

Hi @danquixote,

Thank you for the PR. I re-assign @tremble to it since he was already reviewing the original PR.

tremble

Thanks for taking the time to submit this PR.

The code itself looks reasonable.

The docs need a little work, I've highlighted an example of each type of issue in the docs, rather than adding a suggestion for each time the same issue occurs.

It should be possible to merge the two test targets. This helps to ensure that changes to one module don't affect the other, and also generally reduces the time taken to run the tests since you don't have all of the setup/tear-down time.

tremble · 2021-05-03T07:22:56Z

plugins/modules/ec2_asg_instance_refresh.py

+---
+module: ec2_asg_instance_refresh
+version_added: 1.0.0
+short_description: Start or cancel an ec2 Auto Scaling Group (ASG) Instance Refresh in AWS


Suggested change

short_description: Start or cancel an ec2 Auto Scaling Group (ASG) Instance Refresh in AWS

short_description: Start or cancel an EC2 Auto Scaling Group (ASG) instance refresh in AWS

(nit) EC2 is a product name, the capitalization should follow the product owner's capitalization where possible.

Fixed in 1cacca4

tremble · 2021-05-03T07:31:05Z

plugins/modules/ec2_asg_instance_refresh.py

+version_added: 1.0.0
+short_description: Start or cancel an ec2 Auto Scaling Group (ASG) Instance Refresh in AWS
+description:
+  - Start or cancel an ec2 Auto Scaling Group Instance Refresh in AWS


Suggested change

- Start or cancel an ec2 Auto Scaling Group Instance Refresh in AWS

- Start or cancel an EC2 Auto Scaling Group instance refresh in AWS.

https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_documenting.html#documentation-fields

While short_description fields shouldn't have a full stop at the end, description fields should be complete sentences, including a full stop at the end. While there's plenty of bad examples around, we are slowly cleaning them up.

Fixed in f2ec8ae

tremble · 2021-05-03T07:35:10Z

plugins/modules/ec2_asg_instance_refresh.py

+  preferences:
+    description:
+      - Set of preferences associated with the instance refresh request.
+      - If not provided, the default values are used. For MinHealthyPercentage, the default value is 90.


Suggested change

- If not provided, the default values are used. For MinHealthyPercentage, the default value is 90.

- If not provided, the default values are used.

- For I(min_healthy_percentage), the default value is C(90).

When referring to other parameters you can should use I(<param_name>), when referring to values use C(<value>). This will then result in formatting which makes it a little easier to distinguish.

You can also do When I(my_param=SomeValue)... where referring to a specific parameter having a specific value.

Fixed in f2ec8ae

tremble · 2021-05-03T07:45:25Z

plugins/modules/ec2_asg_instance_refresh.py

+        state=dict(
+            type='str',
+            required=True,
+            choices=['started', 'canceled'],


The past tense of cancel is cancelled (with two 'l's)

Fixed in 47e6165

tremble · 2021-05-03T07:48:26Z

plugins/modules/ec2_asg_instance_refreshes_info.py

+  - You can determine the status of a request by looking at the Status parameter. The following are the possible statuses
+  -    Pending --  The request was created, but the operation has not started.
+  -    InProgress --  The operation is in progress.
+  -    Successful --  The operation completed successfully.
+  -    Failed --  The operation failed to complete. You can troubleshoot using the status reason and the scaling activities.
+  -    Cancelling --
+  -        An ongoing operation is being cancelled.
+  -        Cancellation does not roll back any replacements that have already been completed,
+  -        but it prevents new replacements from being started.
+  -    Cancelled --  The operation is cancelled.


It would be better to move most of this description into the return documentation.

Suggested change

- You can determine the status of a request by looking at the Status parameter. The following are the possible statuses

- Pending -- The request was created, but the operation has not started.

- InProgress -- The operation is in progress.

- Successful -- The operation completed successfully.

- Failed -- The operation failed to complete. You can troubleshoot using the status reason and the scaling activities.

- Cancelling --

- An ongoing operation is being cancelled.

- Cancellation does not roll back any replacements that have already been completed,

- but it prevents new replacements from being started.

- Cancelled -- The operation is cancelled.

- You can determine the status of a request by looking at the I(status) parameter.

Fixed in f2ec8ae

tremble · 2021-05-03T07:53:51Z

tests/integration/targets/ec2_asg_instance_refresh/defaults/main.yml

+---
+# defaults file for ec2_asg
+# Amazon Linux 2 AMI 2019.06.12 (HVM), GP2 Volume Type
+ec2_ami_name: 'amzn2-ami-hvm-2.0.20190612-x86_64-gp2'


We generally use a wildcard here and then filter it. See for example:
tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/find_ami.yml

This allows for pulling in fixes that might be related to things like hardware support.

I think I included this by mimicking the ec2_asg module...Fixed in 667e501

tremble · 2021-05-03T07:57:21Z

tests/integration/targets/ec2_asg_instance_refresh/tasks/main.yml

+    - name: Create VPC for use in testing
+      ec2_vpc_net:
+        name: "{{ resource_prefix }}-vpc"
+        cidr_block: 10.55.77.0/24


I prefer using a random subnet here rather than hard coding values. It reduces the risks of collisions when things are run in parallel.

Also an artifact of mimicking ec2_asg...Got a good example from ec2_instance. (Hopefully) fixed in 3392ef7

tremble · 2021-05-03T08:00:19Z

tests/integration/targets/ec2_asg_instance_refreshes_info/aliases

+cloud/aws
+ec2_asg_instance_refreshes_info


No need to duplicate the tests here. You can merge the ec2_asg_instance_refreshes_info and ec2_asg_instance_refresh targets and add ec2_asg_instance_refreshes_info to the aliases file

I think I understand what you're saying, and it would of course make sense to combine the two test-"suites" for these related modules into one. However, I'm not 100% sure I did it correctly. Calling each still only runs the tests only for that module. Attempted fix in da9fe1b

tremble · 2021-05-03T08:01:51Z

...integration/targets/ec2_asg_instance_refreshes_info/tasks/refresh_and_cancel_three_times.yml

+    aws_access_key: "{{ aws_access_key }}"
+    aws_secret_key: "{{ aws_secret_key }}"
+    region: "{{ aws_region }}"


Shouldn't be needed and won't work in CI because we use session tokens.

Fixed in 958a04a

danquixote · 2021-05-07T06:36:22Z

Hi @tremble. Thank you very much for your code-review. I haven't touched this code in a while, but will go over your comments this week and will hopefully also get my builds to pass.

---Also, it looks like I submitted this PR before the Shippable deprecation on May 3rd. Should I still continue work within this PR, or would it be better to open a 'fresh' one?

tremble · 2021-05-07T07:54:57Z

---Also, it looks like I submitted this PR before the Shippable deprecation on May 3rd. Should I still continue work within this PR, or would it be better to open a 'fresh' one?

When you push your new commits it'll automatically switch the shippable tests for the zuul tests we're now using. Mark

… bucket_ownership_controls

Wrap Kafka tests in custom virtualenv SUMMARY aws_msk_config and aws_msk_cluster need recent versions of botocore, explicitly install these so we can still test against our minimum supported boto3/botocore versions ISSUE TYPE Bugfix Pull Request COMPONENT NAME aws_msk_config aws_msk_cluster ADDITIONAL INFORMATION fixes: ansible-collections#678 Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Alina Buzachis <None> Reviewed-by: None <None>

…nce_refreshes_info.py

…stance_refreshes_info modules

…rgets

mandar242 · 2022-02-23T19:18:09Z

Hi @danquixote, closing this (#487) duplicate PR as you have already opened another with changes and rebase done. #795

…equests (ansible-collections#487) ec2_spot_instance_info: add new module for describing spot instance requests SUMMARY Added a new module that describes the specified Spot Instance requests. ISSUE TYPE New Module Pull Request COMPONENT NAME ec2_spot_instance_info ADDITIONAL INFORMATION Related Documentation: https://docs.aws.amazon.com/goto/WebAPI/ec2-2016-11-15/DescribeSpotInstanceRequests Reviewed-by: Jill R <None> Reviewed-by: None <None>

ansibullbot added community_review integration tests/integration module module needs_triage new_contributor Help guide this first time contributor new_module New module new_plugin New plugin plugins plugin (any type) tests tests labels Mar 19, 2021

ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Mar 19, 2021

goneri requested a review from tremble April 30, 2021 19:03

goneri removed the needs_triage label Apr 30, 2021

tremble requested changes May 3, 2021

View reviewed changes

danquixote force-pushed the add_asg_refresh_and_info_modules branch 2 times, most recently from 47e6165 to 52b5cbc Compare May 16, 2021 08:01

danquixote force-pushed the add_asg_refresh_and_info_modules branch from 812a396 to c55e32d Compare May 25, 2021 05:57

ansibullbot added community_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels May 25, 2021

danquixote requested a review from tremble May 25, 2021 06:06

danquixote force-pushed the add_asg_refresh_and_info_modules branch 2 times, most recently from c64eef5 to d2ea03a Compare June 1, 2021 04:54

tremble and others added 3 commits August 11, 2021 12:31

aws_secret - fix deletion idempotency when not using instant deletion

371222f

aws_s3_bucket_info - Add a check for botocore>='1.18.11' when pulling…

dcd2232

… bucket_ownership_controls

danquixote added 25 commits November 9, 2021 05:12

Add module and tests for ec2_asg_instance_refresh

c93c4c1

Fix documentation

f51b0e1

Update runtime.yml for new EC2 ASG refresh modules

d891f5a

Fixup documentation for ec2_asg_instance_refresh.py and ec2_asg_insta…

c1d1573

…nce_refreshes_info.py

Fix AMI selection and fact-setting

0d2a761

Make subnet selection more random

28e7711

Fix/combine aliases files for ec2_asg_instance_refresh and ec2_asg_in…

09fbd0b

…stance_refreshes_info modules

Remove unneeded AWS environment variables

63c16f4

Fix spelling of 'cancelled'

de0d562

Remove trailing whitespace in YAML

63158f7

Fix YaML bug in main testfile

eba7f72

Re-fix YAML

bfdcc78

Add vpc_seed variable to defaults

d4357c1

Fix 'subnet_a_cidr' var invocation

292fa62

Make test asertion more specific, due to spelling bug on AWS' side

83d1324

Merge ec2_asg_instance_refresh and ec2_asg_instance_refreshes_info ta…

8ba06bd

…rgets

Try to re-fix aliases

4009b73

Move mistakenly deleted YaML file to proper place.

9d8fa1e

Remove extra blank line in YaML

dd75861

Fix spelling

91af4e9

Improve loop and fixup assertion

18aff29

Fix another assertion

178a2b1

Fix asertion based on AWS' new token obfuscation

45cdf1a

Fix another assertion - after merging targets

37787be

topic branch.

ce9dbcd

danquixote mentioned this pull request Nov 9, 2021

Add asg refresh and asg refresh info modules #795

Closed

markuman mentioned this pull request Feb 1, 2022

4.0.0 release plan #895

Closed

1 task

mandar242 closed this Feb 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add asg refresh and info modules #487

Add asg refresh and info modules #487

danquixote commented Mar 19, 2021

ansibullbot commented Mar 19, 2021

danquixote commented Mar 19, 2021

goneri commented Apr 30, 2021

tremble left a comment

tremble May 3, 2021

danquixote May 16, 2021 •

edited

Loading

tremble May 3, 2021

danquixote May 16, 2021

tremble May 3, 2021

danquixote May 16, 2021

tremble May 3, 2021

danquixote May 16, 2021

tremble May 3, 2021

danquixote May 16, 2021

tremble May 3, 2021

danquixote May 16, 2021

tremble May 3, 2021

danquixote May 16, 2021

tremble May 3, 2021

danquixote May 16, 2021

tremble May 3, 2021

danquixote May 16, 2021

danquixote commented May 7, 2021

tremble commented May 7, 2021 via email

mandar242 commented Feb 23, 2022

	short_description: Start or cancel an ec2 Auto Scaling Group (ASG) Instance Refresh in AWS
	short_description: Start or cancel an EC2 Auto Scaling Group (ASG) instance refresh in AWS

	- Start or cancel an ec2 Auto Scaling Group Instance Refresh in AWS
	- Start or cancel an EC2 Auto Scaling Group instance refresh in AWS.

	- If not provided, the default values are used. For MinHealthyPercentage, the default value is 90.
	- If not provided, the default values are used.
	- For I(min_healthy_percentage), the default value is C(90).

		cloud/aws
		ec2_asg_instance_refreshes_info

Add asg refresh and info modules #487

Add asg refresh and info modules #487

Conversation

danquixote commented Mar 19, 2021

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibullbot commented Mar 19, 2021

danquixote commented Mar 19, 2021

goneri commented Apr 30, 2021

tremble left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danquixote May 16, 2021 • edited Loading

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

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

danquixote commented May 7, 2021

tremble commented May 7, 2021 via email

mandar242 commented Feb 23, 2022

danquixote May 16, 2021 •

edited

Loading