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

Add ability to remove detached internet gateway #1786

Merged

Conversation

branic
Copy link
Contributor

@branic branic commented Oct 3, 2023

SUMMARY

Introduces the ability to remove an internet gateway that is not attached to a VPC. To remove an internet gateway either the ID of the internet gateway or the ID of the attached VPC can be supplied. It is also possible to supply both IDs. If both IDs are supplied then a failure will be generated if the attached VPC ID does not match the user supplied VPC.

Fixes #1669

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ec2_vpc_igw

ADDITIONAL INFORMATION

I wasn't able to add a test for removing an internet gateway that is detached as the module currently doesn't have a way to create a detached internet gateway. I plan on opening a follow-up PR for that use case and will add an appropriate removal test in that PR.

@github-actions
Copy link

github-actions bot commented Oct 3, 2023

Docs Build 📝

Thank you for contribution!✨

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

@branic branic force-pushed the branic/issue1669 branch 3 times, most recently from 8b23f42 to 412b2e5 Compare October 3, 2023 21:00
@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 50s
✔️ build-ansible-collection SUCCESS in 12m 45s
✔️ ansible-test-splitter SUCCESS in 4m 50s
✔️ integration-amazon.aws-1 SUCCESS in 10m 25s
Skipped 43 jobs

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 taking the time to submit this PR.

I've had a quick look. The way the documentation's now worded it looks like the module should cope with attaching an existing (unattached) IGW to a VPC, which isn't the case. I think it's worth either adding this feature or rewording the documentation. (This might also help with #1787 since you'd at least be able to use the created IGW)

@branic branic force-pushed the branic/issue1669 branch 2 times, most recently from 72304c9 to 0366776 Compare October 16, 2023 23:34
@branic
Copy link
Contributor Author

branic commented Oct 16, 2023

@tremble Please review this PR again when you have a chance. I've added the ability to create an IGW without attaching a VPC to it. I also fixed #1787 and added functionality to attach/detach and change VPC attached to the internet gateway.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/65d70cd2f2c0464894129b3bf0a2f1d9

✔️ ansible-galaxy-importer SUCCESS in 5m 32s
✔️ build-ansible-collection SUCCESS in 13m 28s
✔️ ansible-test-splitter SUCCESS in 5m 05s
✔️ integration-amazon.aws-1 SUCCESS in 8m 36s
integration-community.aws-1 TIMED_OUT in 1h 00m 57s
✔️ integration-community.aws-2 SUCCESS in 25m 16s
✔️ integration-community.aws-3 SUCCESS in 15m 40s
✔️ integration-community.aws-4 SUCCESS in 8m 23s
✔️ integration-community.aws-5 SUCCESS in 8m 37s
Skipped 38 jobs

abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
…-collections#1786)

aws_config_delivery_channel - Add support for KMS encryption

SUMMARY
Add support for KMS keys when creating an AWS  Config delivery channel
ISSUE TYPE


Feature Pull Request

COMPONENT NAME
aws_config_delivery_channel
ADDITIONAL INFORMATION
AWS added support for KMS encryption of objects stored in S3. This adds that option via a new kms_key_arn module option.

Reviewed-by: Mark Chappell
@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 5m 15s
✔️ build-ansible-collection SUCCESS in 21m 35s
✔️ ansible-test-splitter SUCCESS in 4m 57s
✔️ integration-amazon.aws-1 SUCCESS in 7m 35s
Skipped 43 jobs

@branic
Copy link
Contributor Author

branic commented Oct 30, 2023

@tremble I just wanted to check in and see if you could review this PR again.

Thank you.

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.

Apologies for the delay, life got in the way.

Two issues:

  • add version_added to docs for new paramters
  • Setting force_attach and detach_vpc to True should throw an error (as early as possible) their behaviours conflict.

plugins/modules/ec2_vpc_igw.py Show resolved Hide resolved
plugins/modules/ec2_vpc_igw.py Show resolved Hide resolved
plugins/modules/ec2_vpc_igw.py Outdated Show resolved Hide resolved
plugins/modules/ec2_vpc_igw.py Show resolved Hide resolved
@branic
Copy link
Contributor Author

branic commented Oct 30, 2023

Thanks @tremble. I totally understand life getting in the way and I appreciate you taking a look at this so quickly today. I've made the requested changes so hopefully all looks good now. If you find anything else that needs to be addressed I'm happy to do so.

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 54s
✔️ build-ansible-collection SUCCESS in 14m 37s
✔️ ansible-test-splitter SUCCESS in 5m 22s
✔️ integration-amazon.aws-1 SUCCESS in 7m 07s
Skipped 43 jobs

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

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

✔️ ansible-galaxy-importer SUCCESS in 5m 05s
✔️ build-ansible-collection SUCCESS in 14m 01s
✔️ ansible-test-splitter SUCCESS in 5m 22s
✔️ integration-amazon.aws-1 SUCCESS in 10m 22s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

Pull request merge failed: Required status check "ansible/gate" is expected.

@tremble
Copy link
Contributor

tremble commented Oct 31, 2023

regate

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 14m 07s
✔️ build-ansible-collection SUCCESS in 17m 51s
✔️ ansible-test-splitter SUCCESS in 5m 44s
✔️ integration-amazon.aws-1 SUCCESS in 6m 40s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

Pull request merge failed: Required status check "ansible/gate" is expected.

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 46s
✔️ build-ansible-collection SUCCESS in 14m 13s
✔️ ansible-test-splitter SUCCESS in 5m 20s
✔️ integration-amazon.aws-1 SUCCESS in 8m 23s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 4m 04s
✔️ build-ansible-collection SUCCESS in 13m 56s
✔️ ansible-test-splitter SUCCESS in 5m 14s
✔️ integration-amazon.aws-1 SUCCESS in 7m 54s
Skipped 43 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 35ea344 into ansible-collections:main Oct 31, 2023
36 checks passed
@branic branic deleted the branic/issue1669 branch October 31, 2023 15:33
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.

ec2_vpc_igw should be able to remove detached internet gateway
3 participants