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

Refactor ec2_vpc_peer* modules #2153

Conversation

alinabuzachis
Copy link
Contributor

@alinabuzachis alinabuzachis commented Sep 24, 2024

SUMMARY

Depends-On: ansible-collections/amazon.aws#2303

Refactor ec2_vpc_peer* modules
Once the module is migrated to amazon.aws I will deprecate result returned by the info module and rename the ec2_vpc_peer module (see #2154).

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

ec2_vpc_peer
ec2_vpc_peering_info

ADDITIONAL INFORMATION

Copy link

github-actions bot commented Sep 24, 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

This change depends on a change that failed to merge.

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

Copy link
Contributor

This change depends on a change that failed to merge.

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

Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 23s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 05s
✔️ ansible-test-splitter SUCCESS in 4m 34s
✔️ integration-community.aws-1 SUCCESS in 15m 52s
✔️ integration-community.aws-2 SUCCESS in 7m 50s
✔️ integration-community.aws-3 SUCCESS in 5m 17s
✔️ integration-community.aws-4 SUCCESS in 5m 03s
✔️ integration-community.aws-5 SUCCESS in 8m 42s
✔️ integration-community.aws-6 SUCCESS in 10m 39s
✔️ integration-community.aws-7 SUCCESS in 5m 18s
Skipped 15 jobs

Signed-off-by: Alina Buzachis <[email protected]>
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 29s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 49s
✔️ ansible-test-splitter SUCCESS in 4m 25s
✔️ integration-community.aws-1 SUCCESS in 17m 05s
✔️ integration-community.aws-2 SUCCESS in 8m 36s
✔️ integration-community.aws-3 SUCCESS in 6m 35s
✔️ integration-community.aws-4 SUCCESS in 5m 29s
✔️ integration-community.aws-5 SUCCESS in 6m 21s
✔️ integration-community.aws-6 SUCCESS in 8m 44s
✔️ integration-community.aws-7 SUCCESS in 5m 23s
Skipped 15 jobs

Copy link
Contributor

@abikouo abikouo left a comment

Choose a reason for hiding this comment

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

Just minor remarks

changelogs/fragments/20240924-ec2_vpc_peer-refactor.yml Outdated Show resolved Hide resolved
plugins/modules/ec2_vpc_peering_info.py Outdated Show resolved Hide resolved
plugins/modules/ec2_vpc_peer.py Outdated Show resolved Hide resolved
plugins/modules/ec2_vpc_peer.py Outdated Show resolved Hide resolved
plugins/modules/ec2_vpc_peer.py Outdated Show resolved Hide resolved
plugins/modules/ec2_vpc_peer.py Outdated Show resolved Hide resolved
Signed-off-by: Alina Buzachis <[email protected]>
Copy link
Contributor

This change depends on a change that failed to merge.

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

Copy link
Contributor

@abikouo abikouo left a comment

Choose a reason for hiding this comment

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

We should add support for check_mode for the ec2_vpc_peer module (the module is using the ensure_ec2_tags API which natively support the check mode).

Copy link
Contributor

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

ansible-galaxy-importer FAILURE in 4m 43s (non-voting)
✔️ build-ansible-collection SUCCESS in 13m 01s
✔️ ansible-test-splitter SUCCESS in 4m 25s
✔️ integration-community.aws-1 SUCCESS in 16m 30s
✔️ integration-community.aws-2 SUCCESS in 6m 51s
✔️ integration-community.aws-3 SUCCESS in 6m 54s
✔️ integration-community.aws-4 SUCCESS in 5m 24s
✔️ integration-community.aws-5 SUCCESS in 7m 20s
✔️ integration-community.aws-6 SUCCESS in 8m 02s
✔️ integration-community.aws-7 SUCCESS in 7m 37s
✔️ integration-community.aws-8 SUCCESS in 6m 20s
Skipped 14 jobs

@alinabuzachis
Copy link
Contributor Author

recheck

Copy link
Contributor

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

ansible-galaxy-importer FAILURE in 4m 55s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 53s
✔️ ansible-test-splitter SUCCESS in 4m 40s
✔️ integration-community.aws-1 SUCCESS in 18m 06s
✔️ integration-community.aws-2 SUCCESS in 5m 28s
✔️ integration-community.aws-3 SUCCESS in 9m 13s
✔️ integration-community.aws-4 SUCCESS in 6m 58s
✔️ integration-community.aws-5 SUCCESS in 8m 07s
✔️ integration-community.aws-6 SUCCESS in 8m 40s
✔️ integration-community.aws-7 SUCCESS in 11m 58s
✔️ integration-community.aws-8 SUCCESS in 6m 24s
Skipped 14 jobs

@alinabuzachis alinabuzachis requested a review from abikouo October 4, 2024 09:31
@alinabuzachis
Copy link
Contributor Author

recheck

1 similar comment
@alinabuzachis
Copy link
Contributor Author

recheck

Signed-off-by: Alina Buzachis <[email protected]>
Copy link
Contributor

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

ansible-galaxy-importer FAILURE in 4m 50s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 19s
✔️ ansible-test-splitter SUCCESS in 4m 36s
✔️ integration-community.aws-1 SUCCESS in 6m 45s
Skipped 21 jobs

Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 22s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 00s
✔️ ansible-test-splitter SUCCESS in 4m 27s
✔️ integration-community.aws-1 SUCCESS in 5m 15s
Skipped 21 jobs

"status-code": state,
}
try:
waiter.wait(Filters=ansible_dict_to_boto3_filter_list(peer_filter))
waiter.wait(Filters=ansible_dict_to_boto3_filter_list(filters))
except botocore.exceptions.WaiterError as e:
module.fail_json_aws(e, "Failed to wait for state change")
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
module.fail_json_aws(e, "Enable to describe Peerig Connection while waiting for state to change")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should the message be "Unable to describe Peerig Connection while waiting for state to change" ?

register: peers_info
check_mode: True

- name: Assert success
assert:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assert:
ansible.builtin.assert:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add FQCN for all assert tasks ?

peering_connections = describe_vpc_peering_connections(
client, Filters=ansible_dict_to_boto3_filter_list(filters)
)
except AnsibleEC2Error as e:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Won't a.aws' ec2 module_utils manage this error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought the errors need to be handled here.

Signed-off-by: Alina Buzachis <[email protected]>
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 46s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 36s
✔️ ansible-test-splitter SUCCESS in 4m 23s
✔️ integration-community.aws-1 SUCCESS in 6m 35s
Skipped 21 jobs

Copy link
Contributor

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

ansible-galaxy-importer FAILURE in 5m 24s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 47s
✔️ ansible-test-splitter SUCCESS in 4m 26s
✔️ integration-community.aws-1 SUCCESS in 5m 30s
Skipped 21 jobs

Signed-off-by: Alina Buzachis <[email protected]>
Copy link
Contributor

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

ansible-galaxy-importer FAILURE in 5m 15s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 06s
✔️ ansible-test-splitter SUCCESS in 4m 32s
✔️ integration-community.aws-1 SUCCESS in 5m 26s
Skipped 21 jobs

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

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

✔️ ansible-galaxy-importer SUCCESS in 3m 44s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 38s
✔️ ansible-test-splitter SUCCESS in 4m 20s
✔️ integration-community.aws-1 SUCCESS in 6m 30s
Skipped 21 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit fcd780e into ansible-collections:main Oct 10, 2024
65 checks passed
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