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

Conversation

GomathiselviS
Copy link
Collaborator

@GomathiselviS GomathiselviS commented Oct 7, 2024

SUMMARY

This PR refactors ec2_placement_group*.

Depends-On: ansible-collections/amazon.aws#2322
Refer: https://issues.redhat.com/browse/ACA-1886

ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
  • Feature Pull Request
  • New Module Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION

Copy link
Contributor

This change depends on a change that failed to merge.

Change ansible-collections/amazon.aws#2322 is needed.

Copy link

github-actions bot commented Oct 7, 2024

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/174018af6e6e44dbb7c44820b2ec3b7d

✔️ ansible-galaxy-importer SUCCESS in 3m 29s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 57s
✔️ ansible-test-splitter SUCCESS in 4m 24s
✔️ integration-community.aws-1 SUCCESS in 17m 43s
✔️ integration-community.aws-2 SUCCESS in 4m 39s
✔️ integration-community.aws-3 SUCCESS in 8m 21s
✔️ integration-community.aws-4 SUCCESS in 6m 55s
integration-community.aws-5 FAILURE in 8m 47s
✔️ integration-community.aws-6 SUCCESS in 5m 56s
✔️ integration-community.aws-7 SUCCESS in 8m 19s
✔️ integration-community.aws-8 SUCCESS in 8m 29s
✔️ integration-community.aws-9 SUCCESS in 6m 27s
Skipped 13 jobs

plugins/modules/ec2_placement_group.py Outdated Show resolved Hide resolved
plugins/modules/ec2_placement_group.py Outdated Show resolved Hide resolved
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.

plugins/modules/ec2_placement_group.py Outdated Show resolved Hide resolved
plugins/modules/ec2_placement_group.py Outdated Show resolved Hide resolved
plugins/modules/ec2_placement_group_info.py Outdated Show resolved Hide resolved
@GomathiselviS GomathiselviS requested a review from abikouo October 9, 2024 17:20
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/7806731455dc49dea81d2a5794cd7763

✔️ ansible-galaxy-importer SUCCESS in 3m 18s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 32s
✔️ ansible-test-splitter SUCCESS in 4m 43s
✔️ integration-community.aws-1 SUCCESS in 17m 22s
✔️ integration-community.aws-2 SUCCESS in 6m 23s
✔️ integration-community.aws-3 SUCCESS in 9m 22s
✔️ integration-community.aws-4 SUCCESS in 6m 51s
✔️ integration-community.aws-5 SUCCESS in 6m 47s
✔️ integration-community.aws-6 SUCCESS in 6m 59s
✔️ integration-community.aws-7 SUCCESS in 14m 30s
✔️ integration-community.aws-8 SUCCESS in 9m 35s
✔️ integration-community.aws-9 SUCCESS in 7m 41s
Skipped 13 jobs

@GomathiselviS
Copy link
Collaborator Author

recheck

Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/9c15dfe2c8d2415da4555dcb07804468

⚠️ ansible-galaxy-importer SKIPPED Skipped due to failed job build-ansible-collection (non-voting)
build-ansible-collection NODE_FAILURE Node request 200-0007622328 failed in 0s
ansible-test-splitter NODE_FAILURE Node request 200-0007622329 failed in 0s
⚠️ integration-community.aws-1 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-2 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-3 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-4 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-5 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-6 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-7 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-8 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-9 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-10 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-11 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-12 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-13 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-14 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-15 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-16 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-17 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-18 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-19 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-20 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-21 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-22 SKIPPED Skipped due to failed job build-ansible-collection

Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/2f5bed0fe12e4d4398a9f4b6a4ce8d90

⚠️ ansible-galaxy-importer SKIPPED Skipped due to failed job build-ansible-collection (non-voting)
build-ansible-collection NODE_FAILURE Node request 200-0007622619 failed in 0s
ansible-test-splitter NODE_FAILURE Node request 200-0007622620 failed in 0s
⚠️ integration-community.aws-1 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-2 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-3 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-4 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-5 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-6 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-7 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-8 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-9 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-10 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-11 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-12 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-13 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-14 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-15 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-16 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-17 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-18 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-19 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-20 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-21 SKIPPED Skipped due to failed job build-ansible-collection
⚠️ integration-community.aws-22 SKIPPED Skipped due to failed job build-ansible-collection

@GomathiselviS
Copy link
Collaborator Author

recheck

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/86990d04e90f43f6be4d177609c2d5cd

✔️ ansible-galaxy-importer SUCCESS in 3m 52s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 23s
✔️ ansible-test-splitter SUCCESS in 3m 48s
✔️ integration-community.aws-1 SUCCESS in 7m 31s
Skipped 21 jobs

Comment on lines 184 to 185
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.

plugins/modules/ec2_placement_group.py Show resolved Hide resolved
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/20eb7759e193454ca7643a3a8afc51a3

ansible-galaxy-importer FAILURE in 4m 43s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 00s
✔️ ansible-test-splitter SUCCESS in 3m 58s
✔️ integration-community.aws-1 SUCCESS in 8m 03s
Skipped 21 jobs

@GomathiselviS
Copy link
Collaborator Author

recheck

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/eaf945828bc146f1b9e6862d63a6fcb2

ansible-galaxy-importer FAILURE in 5m 51s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 09s
✔️ ansible-test-splitter SUCCESS in 4m 10s
✔️ integration-community.aws-1 SUCCESS in 6m 13s
Skipped 21 jobs

@GomathiselviS GomathiselviS added the mergeit Merge the PR (SoftwareFactory) label Oct 16, 2024
Copy link
Contributor

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/8fad89bcdd59435b8356321a72a22645

✔️ ansible-galaxy-importer SUCCESS in 3m 18s (non-voting)
✔️ build-ansible-collection SUCCESS in 9m 58s
✔️ ansible-test-splitter SUCCESS in 3m 53s
✔️ integration-community.aws-1 SUCCESS in 6m 10s
Skipped 21 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 290e89a into ansible-collections:main Oct 16, 2024
67 checks passed
alinabuzachis pushed a commit to GomathiselviS/community.aws that referenced this pull request Oct 17, 2024
…s#2167)

SUMMARY
This PR refactors ec2_placement_group*.
Depends-On: ansible-collections/amazon.aws#2322
Refer: https://issues.redhat.com/browse/ACA-1886


ISSUE TYPE


Bugfix Pull Request
Docs Pull Request
Feature Pull Request
New Module Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION

Reviewed-by: Bikouo Aubin
Reviewed-by: GomathiselviS <[email protected]>
Reviewed-by: Alina Buzachis
alinabuzachis pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2024
…s#2167)

SUMMARY
This PR refactors ec2_placement_group*.
Depends-On: ansible-collections/amazon.aws#2322
Refer: https://issues.redhat.com/browse/ACA-1886


ISSUE TYPE


Bugfix Pull Request
Docs Pull Request
Feature Pull Request
New Module Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION

Reviewed-by: Bikouo Aubin
Reviewed-by: GomathiselviS <[email protected]>
Reviewed-by: Alina Buzachis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit Merge the PR (SoftwareFactory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants