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

ec2_vpc_route_table: Add support for Route entry for Carrier Gateway #926

Conversation

mtulio
Copy link
Contributor

@mtulio mtulio commented Jul 13, 2022

SUMMARY

Add support for VPC Carrear Gateways entry on route table.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • module/ec2_vpc_route_table
ADDITIONAL INFORMATION

Support Carrier Gateway route on Route Table module using the same strategy of Nat GW, discovering the prefix of resource name cagw-.

Not directly related, but testing in the same solution the cagw modules: ansible-collections/community.aws#1353

  • ec2_carrier_gateway
  • ec2_carrier_gateway_info

@ansibullbot
Copy link

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request module module needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) labels Jul 13, 2022
@github-actions
Copy link

github-actions bot commented Jul 13, 2022

Docs Build 📝

Thank you for contribution!✨

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

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 30s
✔️ build-ansible-collection SUCCESS in 5m 02s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 9m 23s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 8m 23s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 9m 25s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 11s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 42s
✔️ ansible-test-splitter SUCCESS in 2m 29s
✔️ integration-amazon.aws-1 SUCCESS in 16m 59s
⚠️ integration-amazon.aws-2 SKIPPED
⚠️ integration-amazon.aws-3 SKIPPED
⚠️ integration-amazon.aws-4 SKIPPED
⚠️ integration-amazon.aws-5 SKIPPED
⚠️ integration-amazon.aws-6 SKIPPED
⚠️ integration-amazon.aws-7 SKIPPED
⚠️ integration-amazon.aws-8 SKIPPED
⚠️ integration-amazon.aws-9 SKIPPED
⚠️ integration-amazon.aws-10 SKIPPED
⚠️ integration-amazon.aws-11 SKIPPED
⚠️ integration-amazon.aws-12 SKIPPED
⚠️ integration-amazon.aws-13 SKIPPED
⚠️ integration-community.aws-1 SKIPPED
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

Thanks for this, once we've got ansible-collections/community.aws#1353 merged we should probably update the integration tests for this module.

While the code looks good, I've not got Wavelength set up in my test environments yet, so I can't verify that this change actually works.

Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

While the new modules over in community.aws don't need a changelog fragment (it's automatic when a module's added), we will need a changelog fragment for this PR: https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to

@mtulio
Copy link
Contributor Author

mtulio commented Mar 30, 2023

@tremble do you think we are fine to move on this PR as ansible-collections/community.aws#1353 have been merged?

@mtulio mtulio force-pushed the fix-ec2-vpc-rtb-cagw branch from 5f23199 to 4a5671c Compare March 30, 2023 19:18
@mtulio
Copy link
Contributor Author

mtulio commented Mar 30, 2023

/assign @tremble

@tremble tremble self-assigned this Mar 30, 2023
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

2 minor things,

  1. version_added
  2. changelog fragment - https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to

Have you been able to test this? I'm about 90% sure this is right, but don't have an environment with Carrier Gateways

plugins/modules/ec2_vpc_route_table.py Show resolved Hide resolved
@mtulio
Copy link
Contributor Author

mtulio commented Mar 30, 2023

2 minor things,

  1. version_added
  2. changelog fragment - https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to

Have you been able to test this? I'm about 90% sure this is right, but don't have an environment with Carrier Gateways

Yep, I tested when I submitted. I need to set the environment back to test both new modules again and provide logs if you consider we need it here.

@tremble
Copy link
Contributor

tremble commented Mar 30, 2023

I'm happy enough with the change as long as we get the changelog fragment in there.

@mtulio mtulio changed the title feat(ec2-vpc-cagw): Add support for Route entry to Carrier GW ec2_vpc_route_table: Add support for Route entry for Carrier Gateway ID Mar 30, 2023
@mtulio mtulio changed the title ec2_vpc_route_table: Add support for Route entry for Carrier Gateway ID ec2_vpc_route_table: Add support for Route entry for Carrier Gateway Mar 30, 2023
@mtulio
Copy link
Contributor Author

mtulio commented Mar 30, 2023

I'm happy enough with the change as long as we get the changelog fragment in there.

Done, lmk wdyt.

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 4m 00s
✔️ build-ansible-collection SUCCESS in 12m 33s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 8m 28s (non-voting)
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 8m 42s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 9m 44s
✔️ ansible-test-sanity-aws-ansible-2.14 SUCCESS in 8m 31s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 8m 41s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 10m 15s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 11m 50s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 8m 57s
✔️ cloud-tox-py3 SUCCESS in 8m 00s
✔️ ansible-test-changelog SUCCESS in 4m 32s
✔️ ansible-test-splitter SUCCESS in 4m 46s
✔️ integration-amazon.aws-1 SUCCESS in 20m 42s
Skipped 43 jobs

@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Mar 31, 2023
@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 5m 25s
✔️ build-ansible-collection SUCCESS in 12m 50s
✔️ ansible-test-splitter SUCCESS in 4m 59s
✔️ integration-amazon.aws-1 SUCCESS in 22m 17s
Skipped 43 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit e1b2dc7 into ansible-collections:main Mar 31, 2023
@mtulio mtulio deleted the fix-ec2-vpc-rtb-cagw branch March 31, 2023 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review feature This issue/PR relates to a feature request mergeit Merge the PR (SoftwareFactory) module module new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants