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

Minor Unit test and PEP8 sanity fixes (Ansible 2.16) #1846

Merged
merged 2 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 0 additions & 64 deletions .github/workflows/sanity.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,67 +6,3 @@ on: [workflow_call] # allow this workflow to be called from other workflows
jobs:
sanity:
uses: ansible-network/github_actions/.github/workflows/sanity.yml@main
with:
matrix_include: "[]"
matrix_exclude: >-
[
{
"ansible-version": "stable-2.9"
},
{
"ansible-version": "stable-2.12",
"python-version": "3.7"
},
{
"ansible-version": "stable-2.12",
"python-version": "3.11"
},
{
"ansible-version": "stable-2.13",
"python-version": "3.7"
},
{
"ansible-version": "stable-2.13",
"python-version": "3.11"
},
{
"ansible-version": "stable-2.14",
"python-version": "3.7"
},
{
"ansible-version": "stable-2.14",
"python-version": "3.8"
},
{
"ansible-version": "stable-2.15",
"python-version": "3.7"
},
{
"ansible-version": "stable-2.15",
"python-version": "3.8"
},
{
"ansible-version": "milestone",
"python-version": "3.7"
},
{
"ansible-version": "milestone",
"python-version": "3.8"
},
{
"ansible-version": "milestone",
"python-version": "3.9"
},
{
"ansible-version": "devel",
"python-version": "3.7"
},
{
"ansible-version": "devel",
"python-version": "3.8"
},
{
"ansible-version": "devel",
"python-version": "3.9"
}
]
4 changes: 4 additions & 0 deletions changelogs/fragments/20231110-sanity.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
minor_changes:
- autoscaling_group - minor PEP8 whitespace sanity fixes (https://github.com/ansible-collections/amazon.aws/pull/1846).
- rds_instance_snapshot - minor PEP8 whitespace sanity fixes (https://github.com/ansible-collections/amazon.aws/pull/1846).
- ec2_ami_info - simplify parameters to ``get_image_attribute`` to only pass ID of image (https://github.com/ansible-collections/amazon.aws/pull/1846).
18 changes: 9 additions & 9 deletions plugins/modules/autoscaling_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -1675,15 +1675,15 @@ def get_instances_by_launch_config(props, lc_check, initial_instances):
old_instances.append(i)

else:
module.debug(f"Comparing initial instances with current: {*initial_instances,}")
module.debug(f"Comparing initial instances with current: {*initial_instances, }")
for i in props["instances"]:
if i not in initial_instances:
new_instances.append(i)
else:
old_instances.append(i)

module.debug(f"New instances: {len(new_instances)}, {*new_instances,}")
module.debug(f"Old instances: {len(old_instances)}, {*old_instances,}")
module.debug(f"New instances: {len(new_instances)}, {*new_instances, }")
module.debug(f"Old instances: {len(old_instances)}, {*old_instances, }")

return new_instances, old_instances

Expand All @@ -1702,15 +1702,15 @@ def get_instances_by_launch_template(props, lt_check, initial_instances):
else:
old_instances.append(i)
else:
module.debug(f"Comparing initial instances with current: {*initial_instances,}")
module.debug(f"Comparing initial instances with current: {*initial_instances, }")
for i in props["instances"]:
if i not in initial_instances:
new_instances.append(i)
else:
old_instances.append(i)

module.debug(f"New instances: {len(new_instances)}, {*new_instances,}")
module.debug(f"Old instances: {len(old_instances)}, {*old_instances,}")
module.debug(f"New instances: {len(new_instances)}, {*new_instances, }")
module.debug(f"Old instances: {len(old_instances)}, {*old_instances, }")

return new_instances, old_instances

Expand Down Expand Up @@ -1775,9 +1775,9 @@ def terminate_batch(connection, replace_instances, initial_instances, leftovers=
instances_to_terminate = list_purgeable_instances(props, lc_check, lt_check, replace_instances, initial_instances)

module.debug(f"new instances needed: {num_new_inst_needed}")
module.debug(f"new instances: {*new_instances,}")
module.debug(f"old instances: {*old_instances,}")
module.debug(f"batch instances: {*instances_to_terminate,}")
module.debug(f"new instances: {*new_instances, }")
module.debug(f"old instances: {*old_instances, }")
module.debug(f"batch instances: {*instances_to_terminate, }")

if num_new_inst_needed == 0:
decrement_capacity = True
Expand Down
7 changes: 4 additions & 3 deletions plugins/modules/ec2_ami_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,10 @@ def get_images(ec2_client, request_args):
return images


def get_image_attribute(ec2_client, image):
def get_image_attribute(ec2_client, image_id):
try:
launch_permissions = ec2_client.describe_image_attribute(
aws_retry=True, Attribute="launchPermission", ImageId=image["image_id"]
aws_retry=True, Attribute="launchPermission", ImageId=image_id
)
except (ClientError, BotoCoreError) as err:
raise AmiInfoFailure(err, "error describing image attribute")
Expand All @@ -276,9 +276,10 @@ def list_ec2_images(ec2_client, module, request_args):

for image in images:
try:
image_id = image["image_id"]
image["tags"] = boto3_tag_list_to_ansible_dict(image.get("tags", []))
if module.params.get("describe_image_attributes"):
launch_permissions = get_image_attribute(ec2_client, image).get("LaunchPermissions", [])
launch_permissions = get_image_attribute(ec2_client, image_id).get("LaunchPermissions", [])
image["launch_permissions"] = [camel_dict_to_snake_dict(perm) for perm in launch_permissions]
except is_boto3_error_code("AuthFailure"):
# describing launch permissions of images owned by others is not permitted, but shouldn't cause failures
Expand Down
2 changes: 1 addition & 1 deletion plugins/modules/rds_instance_snapshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ def get_parameters(parameters, method_name):
required_options = get_boto3_client_method_parameters(client, method_name, required=True)
if any(parameters.get(k) is None for k in required_options):
method_description = get_rds_method_attribute(method_name, module).operation_description
module.fail_json(msg=f"To {method_description} requires the parameters: {*required_options,}")
module.fail_json(msg=f"To {method_description} requires the parameters: {*required_options, }")
options = get_boto3_client_method_parameters(client, method_name)
parameters = dict((k, v) for k, v in parameters.items() if k in options and v is not None)

Expand Down
1 change: 1 addition & 0 deletions tests/unit/module_utils/test_acm.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
# Handled by HAS_BOTO3
pass


from ansible_collections.amazon.aws.plugins.module_utils.acm import ACMServiceManager
from ansible_collections.amazon.aws.plugins.module_utils.acm import acm_catch_boto_exception

Expand Down
35 changes: 9 additions & 26 deletions tests/unit/plugins/modules/test_ec2_ami_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,33 +94,13 @@ def test_get_image_attribute():
"LaunchPermissions": [{"UserId": "1234567890"}, {"UserId": "0987654321"}],
}

image = {
"architecture": "x86_64",
"blockDeviceMappings": [
{
"device_name": "/dev/sda1",
"ebs": {
"delete_on_termination": "True",
"encrypted": "False",
"snapshot_id": "snap-0f00cba784af62428",
"volume_size": 10,
"volume_Type": "gp2",
},
}
],
"image_id": "ami-1234567890",
"image_location": "1234567890/test-ami-uefi-boot",
"image_type": "machine",
"name": "test-ami-uefi-boot",
"owner_id": "1234567890",
"platform_details": "Linux/UNIX",
}
image_id = "ami-1234567890"

get_image_attribute_result = ec2_ami_info.get_image_attribute(ec2_client, image)
get_image_attribute_result = ec2_ami_info.get_image_attribute(ec2_client, image_id)

ec2_client.describe_image_attribute.call_count == 1
ec2_client.describe_image_attribute.assert_called_with(
aws_retry=True, Attribute="launchPermission", ImageId=image["image_id"]
aws_retry=True, Attribute="launchPermission", ImageId=image_id
)
assert len(get_image_attribute_result["LaunchPermissions"]) == 2

Expand Down Expand Up @@ -203,7 +183,10 @@ def test_list_ec2_images(m_get_images, m_get_image_attribute):
m_get_images.assert_called_with(ec2_client, request_args)

assert m_get_image_attribute.call_count == 2
assert m_get_image_attribute.has_calls([call(ec2_client, images[0])], [call(ec2_client, images[1])])
m_get_image_attribute.assert_has_calls(
[call(ec2_client, images[0]["image_id"])],
[call(ec2_client, images[1]["image_id"])],
)

assert len(list_ec2_images_result) == 2
assert list_ec2_images_result[0]["image_id"] == "ami-1234567890"
Expand Down Expand Up @@ -234,8 +217,8 @@ def test_api_failure_get_images(ec2_client):


def test_api_failure_get_image_attribute(ec2_client):
image = {"image_id": "ami-1234567890"}
image_id = "ami-1234567890"
ec2_client.describe_image_attribute.side_effect = a_boto_exception()

with pytest.raises(ec2_ami_info.AmiInfoFailure):
ec2_ami_info.get_image_attribute(ec2_client, image)
ec2_ami_info.get_image_attribute(ec2_client, image_id)
Loading