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

Prepare ec2_placement_group* module for promotion #2167

Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 2 additions & 0 deletions changelogs/fragments/refactor_ec2_placement_group.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- ec2_placement_group - Refactor module to use shared code from ``amazon.aws.plugins.module_utils.ec2`` and update ``RETURN`` block (https://github.com/ansible-collections/community.aws/pull/2167).
70 changes: 25 additions & 45 deletions plugins/modules/ec2_placement_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
partition_count:
description:
- The number of partitions.
- Valid only when I(Strategy) is set to C(partition).
- Must be a value between C(1) and C(7).
- Valid only when O(strategy) is set to V(partition).
- Must be a value between V(1) and V(7).
type: int
version_added: 3.1.0
state:
Expand Down Expand Up @@ -86,7 +86,7 @@
placement_group:
description: Placement group attributes
returned: when state != absent
type: complex
type: dict
contains:
name:
description: PG name
Expand All @@ -110,34 +110,28 @@
other: value2
"""

try:
import botocore
except ImportError:
pass # caught by AnsibleAWSModule
from typing import Any
from typing import Dict

from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code
from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry
from ansible_collections.amazon.aws.plugins.module_utils.ec2 import create_ec2_placement_group
from ansible_collections.amazon.aws.plugins.module_utils.ec2 import delete_ec2_placement_group
from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_ec2_placement_groups
from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict
from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_specifications

from ansible_collections.community.aws.plugins.module_utils.modules import AnsibleCommunityAWSModule as AnsibleAWSModule


@AWSRetry.exponential_backoff()
def search_placement_group(connection, module):
def search_placement_group(connection, name: str) -> Dict[str, Any]:
"""
Check if a placement group exists.
"""
name = module.params.get("name")
try:
response = connection.describe_placement_groups(Filters=[{"Name": "group-name", "Values": [name]}])
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
module.fail_json_aws(e, msg=f"Couldn't find placement group named [{name}]")
response = describe_ec2_placement_groups(connection, Filters=[{"Name": "group-name", "Values": [name]}])

if len(response["PlacementGroups"]) != 1:
if len(response) != 1:
return None
else:
placement_group = response["PlacementGroups"][0]
placement_group = response[0]
return {
"name": placement_group["GroupName"],
"state": placement_group["State"],
Expand All @@ -146,13 +140,12 @@ def search_placement_group(connection, module):
}


@AWSRetry.exponential_backoff(catch_extra_error_codes=["InvalidPlacementGroup.Unknown"])
def get_placement_group_information(connection, name):
def get_placement_group_information(connection, name: str) -> Dict[str, Any]:
"""
Retrieve information about a placement group.
"""
response = connection.describe_placement_groups(GroupNames=[name])
placement_group = response["PlacementGroups"][0]
response = describe_ec2_placement_groups(connection, GroupNames=[name])
placement_group = response[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should test that the response is not empty

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The search for an existing placement group is handled in the main logic. Therefore, this function is called only after successfully creating the placement group. As a result, the response will never be empty whenever this function is invoked.

return {
"name": placement_group["GroupName"],
"state": placement_group["State"],
Expand All @@ -161,8 +154,7 @@ def get_placement_group_information(connection, name):
}


@AWSRetry.exponential_backoff()
def create_placement_group(connection, module):
def create_placement_group(connection, module: AnsibleAWSModule) -> None:
name = module.params.get("name")
strategy = module.params.get("strategy")
tags = module.params.get("tags")
Expand All @@ -178,38 +170,26 @@ def create_placement_group(connection, module):
params["TagSpecifications"] = boto3_tag_specifications(tags, types=["placement-group"])
if partition_count:
params["PartitionCount"] = partition_count
params["DryRun"] = module.check_mode

try:
connection.create_placement_group(**params)
except is_boto3_error_code("DryRunOperation"):
if module.check_mode:
module.exit_json(
changed=True,
placement_group={
"name": name,
"state": "DryRun",
"strategy": strategy,
"tags": tags,
},
msg="EC2 placement group would be created if not in check mode",
)
except (
botocore.exceptions.ClientError,
botocore.exceptions.BotoCoreError,
) as e: # pylint: disable=duplicate-except
module.fail_json_aws(e, msg=f"Couldn't create placement group [{name}]")

response = create_ec2_placement_group(connection, **params)
module.exit_json(changed=True, placement_group=get_placement_group_information(connection, name))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
response = create_ec2_placement_group(connection, **params)
module.exit_json(changed=True, placement_group=get_placement_group_information(connection, name))
response = create_ec2_placement_group(connection, **params)
response = camel_dict_to_snake_dict(response, ignore_list=["Tags"])
if "Tags" in response:
response["tags"] = boto3_tag_list_to_ansible_dict(response.get("Tags", []))
del response["Tags"]
module.exit_json(changed=True, placement_group=response)

The call to get_placement_group_information is not required it adds more delay in the process.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GomathiselviS this change was not adressed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abikouo I have modified get_placement_group to format_placement_group here. It does not call the describe api.



@AWSRetry.exponential_backoff()
def delete_placement_group(connection, module):
def delete_placement_group(connection, module: AnsibleAWSModule) -> None:
if module.check_mode:
module.exit_json(changed=True, msg="VPC would be deleted if not in check mode")
name = module.params.get("name")

try:
connection.delete_placement_group(GroupName=name, DryRun=module.check_mode)
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
module.fail_json_aws(e, msg=f"Couldn't delete placement group [{name}]")

delete_ec2_placement_group(connection, name)
abikouo marked this conversation as resolved.
Show resolved Hide resolved
module.exit_json(changed=True)


Expand All @@ -227,9 +207,10 @@ def main():
connection = module.client("ec2")

state = module.params.get("state")
name = module.params.get("name")
placement_group = search_placement_group(connection, name)

if state == "present":
placement_group = search_placement_group(connection, module)
if placement_group is None:
create_placement_group(connection, module)
else:
Expand All @@ -243,7 +224,6 @@ def main():
)

elif state == "absent":
placement_group = search_placement_group(connection, module)
if placement_group is None:
module.exit_json(changed=False)
else:
Expand Down
45 changes: 19 additions & 26 deletions plugins/modules/ec2_placement_group_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
name:
description: PG name
type: str
sample: my-cluster
sample: "my-cluster"
state:
description: PG state
type: str
Expand All @@ -77,36 +77,28 @@
other: value2
"""

try:
from botocore.exceptions import BotoCoreError
from botocore.exceptions import ClientError
except ImportError:
pass # caught by AnsibleAWSModule
from typing import Any
from typing import Dict
from typing import List

from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_ec2_placement_groups
from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule
from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict

from ansible_collections.community.aws.plugins.module_utils.modules import AnsibleCommunityAWSModule as AnsibleAWSModule


def get_placement_groups_details(connection, module):
names = module.params.get("names")
try:
if len(names) > 0:
response = connection.describe_placement_groups(
Filters=[
{
"Name": "group-name",
"Values": names,
}
]
)
else:
response = connection.describe_placement_groups()
except (BotoCoreError, ClientError) as e:
module.fail_json_aws(e, msg=f"Couldn't find placement groups named [{names}]")
def get_placement_groups_details(connection, names: List) -> Dict[str, Any]:
params = {}
if len(names) > 0:
params["Filters"] = [
{
"Name": "group-name",
"Values": names,
}
]
response = describe_ec2_placement_groups(connection, **params)

results = []
for placement_group in response["PlacementGroups"]:
for placement_group in response:
results.append(
{
"name": placement_group["GroupName"],
Expand All @@ -129,8 +121,9 @@ def main():
)

connection = module.client("ec2")
names = module.params.get("names")

placement_groups = get_placement_groups_details(connection, module)
placement_groups = get_placement_groups_details(connection, names)
module.exit_json(changed=False, placement_groups=placement_groups)


Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
- name: remove any instances in the test VPC
abikouo marked this conversation as resolved.
Show resolved Hide resolved
ec2_instance:
amazon.aws.ec2_instance:
filters:
vpc_id: "{{ testing_vpc.vpc.id }}"
state: absent
Expand All @@ -9,13 +9,13 @@
retries: 10

- name: Get ENIs
ec2_eni_info:
amazon.aws.ec2_eni_info:
filters:
vpc-id: "{{ testing_vpc.vpc.id }}"
register: enis

- name: delete all ENIs
ec2_eni:
amazon.aws.ec2_eni:
eni_id: "{{ item.id }}"
state: absent
until: removed is not failed
Expand All @@ -24,7 +24,7 @@
retries: 10

- name: remove the security group
ec2_security_group:
amazon.aws.ec2_security_group:
name: "{{ resource_prefix }}-sg"
description: a security group for ansible tests
vpc_id: "{{ testing_vpc.vpc.id }}"
Expand All @@ -35,7 +35,7 @@
retries: 10

- name: remove routing rules
ec2_vpc_route_table:
amazon.aws.ec2_vpc_route_table:
state: absent
vpc_id: "{{ testing_vpc.vpc.id }}"
tags:
Expand All @@ -52,7 +52,7 @@
retries: 10

- name: remove internet gateway
ec2_vpc_igw:
amazon.aws.ec2_vpc_igw:
vpc_id: "{{ testing_vpc.vpc.id }}"
state: absent
register: removed
Expand All @@ -61,7 +61,7 @@
retries: 10

- name: remove subnet A
ec2_vpc_subnet:
amazon.aws.ec2_vpc_subnet:
state: absent
vpc_id: "{{ testing_vpc.vpc.id }}"
cidr: 10.22.32.0/24
Expand All @@ -71,7 +71,7 @@
retries: 10

- name: remove subnet B
ec2_vpc_subnet:
amazon.aws.ec2_vpc_subnet:
state: absent
vpc_id: "{{ testing_vpc.vpc.id }}"
cidr: 10.22.33.0/24
Expand All @@ -81,7 +81,7 @@
retries: 10

- name: remove the VPC
ec2_vpc_net:
amazon.aws.ec2_vpc_net:
name: "{{ resource_prefix }}-vpc"
cidr_block: 10.22.32.0/23
state: absent
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
- name: Create VPC for use in testing
ec2_vpc_net:
amazon.aws.ec2_vpc_net:
name: "{{ resource_prefix }}-vpc"
cidr_block: 10.22.32.0/23
tags:
Expand All @@ -8,15 +8,15 @@
register: testing_vpc

- name: Create internet gateway for use in testing
ec2_vpc_igw:
amazon.aws.ec2_vpc_igw:
vpc_id: "{{ testing_vpc.vpc.id }}"
state: present
tags:
Name: Ansible ec2_lc Testing gateway
register: igw

- name: Create default subnet in zone A
ec2_vpc_subnet:
amazon.aws.ec2_vpc_subnet:
state: present
vpc_id: "{{ testing_vpc.vpc.id }}"
cidr: 10.22.32.0/24
Expand All @@ -26,7 +26,7 @@
register: testing_subnet_a

- name: Create secondary subnet in zone B
ec2_vpc_subnet:
amazon.aws.ec2_vpc_subnet:
state: present
vpc_id: "{{ testing_vpc.vpc.id }}"
cidr: 10.22.33.0/24
Expand All @@ -36,7 +36,7 @@
register: testing_subnet_b

- name: create routing rules
ec2_vpc_route_table:
amazon.aws.ec2_vpc_route_table:
vpc_id: "{{ testing_vpc.vpc.id }}"
tags:
created: "{{ resource_prefix }}-route"
Expand All @@ -48,7 +48,7 @@
- "{{ testing_subnet_b.subnet.id }}"

- name: create a security group with the vpc
ec2_security_group:
amazon.aws.ec2_security_group:
name: "{{ resource_prefix }}-sg"
description: a security group for ansible tests
vpc_id: "{{ testing_vpc.vpc.id }}"
Expand Down
Loading
Loading