Sanity test fixups for AWS ec2 modules #64230

tremble · 2019-11-01T07:53:55Z

SUMMARY

Sanity test fixups for AWS ec2 modules

standard python3 related boilierplate
move type checking into arg parser so we don't document "type: str" when it's going to be cast to int
deprecate a couple of unused options
remove duplicate definition of region (ec2 doc/spec adds it)
missing documentation of type / suboptions

ISSUE TYPE

Bugfix Pull Request
Docs Pull Request

COMPONENT NAME

   lib/ansible/modules/cloud/amazon/ec2.py
   lib/ansible/modules/cloud/amazon/ec2_ami.py
   lib/ansible/modules/cloud/amazon/ec2_ami_copy.py
   lib/ansible/modules/cloud/amazon/ec2_ami_info.py
   lib/ansible/modules/cloud/amazon/ec2_asg.py
   lib/ansible/modules/cloud/amazon/ec2_asg_info.py
   lib/ansible/modules/cloud/amazon/ec2_asg_lifecycle_hook.py
   lib/ansible/modules/cloud/amazon/ec2_customer_gateway.py
   lib/ansible/modules/cloud/amazon/ec2_customer_gateway_info.py
   lib/ansible/modules/cloud/amazon/ec2_eip.py
   lib/ansible/modules/cloud/amazon/ec2_eip_info.py
   lib/ansible/modules/cloud/amazon/ec2_eni.py
   lib/ansible/modules/cloud/amazon/ec2_eni_info.py
   lib/ansible/modules/cloud/amazon/ec2_group.py
   lib/ansible/modules/cloud/amazon/ec2_group_info.py
   lib/ansible/modules/cloud/amazon/ec2_key.py
   lib/ansible/modules/cloud/amazon/ec2_launch_template.py
   lib/ansible/modules/cloud/amazon/ec2_lc.py
   lib/ansible/modules/cloud/amazon/ec2_lc_find.py
   lib/ansible/modules/cloud/amazon/ec2_lc_info.py
   lib/ansible/modules/cloud/amazon/ec2_metadata_facts.py
   lib/ansible/modules/cloud/amazon/ec2_metric_alarm.py
   lib/ansible/modules/cloud/amazon/ec2_placement_group.py
   lib/ansible/modules/cloud/amazon/ec2_placement_group_info.py
   lib/ansible/modules/cloud/amazon/ec2_scaling_policy.py
   lib/ansible/modules/cloud/amazon/ec2_snapshot.py
   lib/ansible/modules/cloud/amazon/ec2_snapshot_copy.py
   lib/ansible/modules/cloud/amazon/ec2_snapshot_info.py
   lib/ansible/modules/cloud/amazon/ec2_tag.py
   lib/ansible/modules/cloud/amazon/ec2_transit_gateway.py
   lib/ansible/modules/cloud/amazon/ec2_vol.py
   lib/ansible/modules/cloud/amazon/ec2_vol_info.py
   lib/ansible/modules/cloud/amazon/ec2_win_password.py

ADDITIONAL INFORMATION

ansibot · 2019-11-01T07:55:38Z

ansibot · 2019-11-01T07:55:39Z

@tremble, just so you are aware we have a dedicated Working Group for aws.
You can find other people interested in this in #ansible-aws on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

ansibot · 2019-11-01T09:13:59Z

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

lib/ansible/modules/cloud/amazon/ec2_eip.py:0:0: option-incorrect-version-added: version_added for new option (wait_timeout) should be '2.10'. Currently StrictVersion ('0.0')
lib/ansible/modules/cloud/amazon/ec2_lc.py:0:0: option-incorrect-version-added: version_added for new option (associate_public_ip_address) should be '2.10'. Currently StrictVersion ('0.0')

click here for bot help

felixfontein

Looks good to me.

…parsing the args Boto would through a type exception if the string couldn't be converted to an int.

It's defined in ec2_argument_spec and ec2 doc fragment

…_start and sort_end into arg parser

ansibot · 2019-11-04T17:52:58Z

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

lib/ansible/modules/cloud/amazon/ec2_eip.py:0:0: option-incorrect-version-added: version_added for new option (wait_timeout) should be '2.10'. Currently StrictVersion ('0.0')
lib/ansible/modules/cloud/amazon/ec2_lc.py:0:0: option-incorrect-version-added: version_added for new option (associate_public_ip_address) should be '2.10'. Currently StrictVersion ('0.0')

click here for bot help

jillr

Missed the merged, adding these comments so I don't lose them in an unsaved review while I'm working on it and will open a new PR for them.

jillr · 2019-11-01T22:24:22Z

lib/ansible/modules/cloud/amazon/ec2_launch_template.py

      ebs:
-        description: Parameters used to automatically set up EBS volumes when the instance is launched.
+        description: Parameters used to automatically set up EBS volumes when the instance is launchedself


Suggested change

description: Parameters used to automatically set up EBS volumes when the instance is launchedself

description: Parameters used to automatically set up EBS volumes when the instance is launched

jillr · 2019-11-01T22:26:03Z

lib/ansible/modules/cloud/amazon/ec2_eip.py

-        matches the tag_value
-    default: 'no'
+      - Supplements I(tag_name) but also checks that the value of the tag provided in I(tag_name) matches I(tag_value).
+        matches the I(tag_value)


Suggested change

matches the I(tag_value)

You didn't add this, but this sentence fragment doesn't seem to belong here. Would you mind cleaning it up?

jillr · 2019-11-01T22:32:01Z

lib/ansible/modules/cloud/amazon/ec2_launch_template.py

  credit_specification:
-    description: The credit option for CPU usage of the instance. Valid for T2 or T3 instances only.
+    description: The credit option for CPU usage of the instance. Valid for T2 or T3 instances onlyself.


Suggested change

description: The credit option for CPU usage of the instance. Valid for T2 or T3 instances onlyself.

description: The credit option for CPU usage of the instance. Valid for T2 or T3 instances only

jillr · 2019-11-01T22:34:50Z

lib/ansible/modules/cloud/amazon/ec2_launch_template.py

  instance_type:
    description: >
-      The instance type, such as I(c5.2xlarge). For a full list of instance types, see
+      The instance type, such as C(c5.2xlarge). For a full list of instance types, see
      http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html


Suggested change

http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html

U(http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html)

jillr · 2019-11-01T22:36:46Z

lib/ansible/modules/cloud/amazon/ec2_launch_template.py

      ipv6_addresses:
        description: >
          A list of one or more specific IPv6 addresses from the IPv6 CIDR
          block range of your subnet. You can't use this option if you're
          specifying the I(ipv6_address_count) option.
+        type: list
+        elements: int


Suggested change

elements: int

elements: str

jillr · 2019-11-01T22:42:26Z

lib/ansible/modules/cloud/amazon/ec2_lc.py

+    choices: ['default', 'dedicated']
+  associate_public_ip_address:
+    description:
+      - The I(wait_timeout) option does nothing and will be removed in Ansible 2.14.


Suggested change

- The I(wait_timeout) option does nothing and will be removed in Ansible 2.14.

- The I(associate_public_ip_address) option does nothing and will be removed in Ansible 2.14.

Cleanup some typos that were missed in ansible#64230

* ec2_* Sanity corrections Cleanup some typos that were missed in #64230 * Update lib/ansible/modules/cloud/amazon/ec2_launch_template.py Co-Authored-By: Felix Fontein <felix@fontein.de>

tremble force-pushed the sanity_ec2 branch from f352390 to 70a57b5 Compare November 1, 2019 08:11

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Nov 1, 2019

felixfontein approved these changes Nov 1, 2019

View reviewed changes

ansibot removed the needs_triage Needs a first human triage before being processed. label Nov 1, 2019

jillr self-assigned this Nov 1, 2019

tremble added 8 commits November 4, 2019 17:58

ec2_ boilerplate

d04ac41

Deprecate unused options.

7a2f04f

ec2_vol: Perform explicit type checking on volume_size and iops when …

e61bec0

…parsing the args Boto would through a type exception if the string couldn't be converted to an int.

ec2_lc_find: remove duplicate definition of region

9c285ad

It's defined in ec2_argument_spec and ec2 doc fragment

ec2_lc_info: Move responsibility for type checking/conversion of sort…

15fe041

…_start and sort_end into arg parser

General sanity test related doc fixups

a3b9757

Remove EC2 related sanity/ignore.txt entries

bfd15e1

Add changelog fragment

a2f8d2e

tremble force-pushed the sanity_ec2 branch from fcaa50d to a2f8d2e Compare November 4, 2019 17:01

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Nov 4, 2019

gundalow approved these changes Nov 4, 2019

View reviewed changes

gundalow merged commit 819ba22 into ansible:devel Nov 4, 2019

jillr reviewed Nov 4, 2019

View reviewed changes

jillr added a commit to jillr/ansible that referenced this pull request Nov 4, 2019

ec2_* Sanity corrections

22efab8

Cleanup some typos that were missed in ansible#64230

jillr mentioned this pull request Nov 4, 2019

ec2_* Sanity corrections #64409

Merged

ansible locked and limited conversation to collaborators Dec 2, 2019

tremble deleted the sanity_ec2 branch December 21, 2019 06:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sanity test fixups for AWS ec2 modules #64230

Sanity test fixups for AWS ec2 modules #64230

tremble commented Nov 1, 2019

ansibot commented Nov 1, 2019

ansibot commented Nov 1, 2019

ansibot commented Nov 1, 2019

felixfontein left a comment

ansibot commented Nov 4, 2019

jillr left a comment

jillr Nov 1, 2019

jillr Nov 1, 2019

jillr Nov 1, 2019

jillr Nov 1, 2019

jillr Nov 1, 2019

jillr Nov 1, 2019

	description: Parameters used to automatically set up EBS volumes when the instance is launchedself
	description: Parameters used to automatically set up EBS volumes when the instance is launched

	description: The credit option for CPU usage of the instance. Valid for T2 or T3 instances onlyself.
	description: The credit option for CPU usage of the instance. Valid for T2 or T3 instances only

	http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html
	U(http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html)

	- The I(wait_timeout) option does nothing and will be removed in Ansible 2.14.
	- The I(associate_public_ip_address) option does nothing and will be removed in Ansible 2.14.

Sanity test fixups for AWS ec2 modules #64230

Sanity test fixups for AWS ec2 modules #64230

Conversation

tremble commented Nov 1, 2019

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibot commented Nov 1, 2019

ansibot commented Nov 1, 2019

ansibot commented Nov 1, 2019

felixfontein left a comment

Choose a reason for hiding this comment

ansibot commented Nov 4, 2019

jillr left a comment

Choose a reason for hiding this comment

jillr Nov 1, 2019

Choose a reason for hiding this comment

jillr Nov 1, 2019

Choose a reason for hiding this comment

jillr Nov 1, 2019

Choose a reason for hiding this comment

jillr Nov 1, 2019

Choose a reason for hiding this comment

jillr Nov 1, 2019

Choose a reason for hiding this comment

jillr Nov 1, 2019

Choose a reason for hiding this comment