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.

Sign up for GitHub

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

Jump to bottom

[ec2_vpc_nat_gateway] Fix broken check_mode #436

Merged
merged 2 commits into from
Mar 10, 2021
Merged

[ec2_vpc_nat_gateway] Fix broken check_mode #436

merged 2 commits into from
Mar 10, 2021

Conversation

alinabuzachis
Copy link
Contributor

@alinabuzachis alinabuzachis commented Feb 22, 2021 •
edited
Loading

SUMMARY

Fix broken check_mode

  • fix broken check_mode
  • comment out integration tests run in check_mode
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_vpc_nat_gateway

@ansibullbot
Copy link

cc @Etherdaemon @jillr @jonhadfield @linuxdynasty @s-hertel @tremble @wimnat
click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels Feb 22, 2021
@ansibullbot
Copy link

cc @BobBoldin @mikedlr @slapula
click here for bot help

@alinabuzachis alinabuzachis mentioned this pull request Feb 25, 2021
Merged
@alinabuzachis alinabuzachis force-pushed the check_mode_nat_gateway branch from 08264b6 to e27680a Compare March 1, 2021 11:19
@alinabuzachis
ec2_vpc_nat_gateway: fix broken check_mode
75b7abf 
     * fix broken check_mode (remove hard coded values)
     * comment out integration tests run in check_mode

Signed-off-by: Alina Buzachis <[email protected]>
@alinabuzachis alinabuzachis force-pushed the check_mode_nat_gateway branch from e27680a to 75b7abf Compare March 1, 2021 11:20
@alinabuzachis alinabuzachis requested review from tremble and jillr March 1, 2021 15:01
jillr
jillr reviewed Mar 1, 2021
View reviewed changes
Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

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

Just a couple small things, thanks very much @alinabuzachis!

plugins/modules/ec2_vpc_nat_gateway.py Outdated
@@ -990,6 +950,10 @@ def ensure_tags(client, module, nat_gw_id, tags, purge_tags, check_mode):
final_tags = []
changed = False

if check_mode and nat_gw_id is None:
# We can't describe tags without an option id, we might get here when creating a new option set in check_mode
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
# We can't describe tags without an option id, we might get here when creating a new option set in check_mode
# We can't describe tags without an EIP id, we might get here when creating a new EIP in check_mode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jillr thank you for reviewing. I applied this change.

tests/integration/targets/ec2_vpc_nat_gateway/tasks/main.yml Show resolved Hide resolved
@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Mar 2, 2021
@alinabuzachis alinabuzachis force-pushed the check_mode_nat_gateway branch 2 times, most recently from ac19245 to 7bef5a3 Compare March 3, 2021 12:03
@jillr
Copy link
Collaborator

jillr commented Mar 3, 2021

recheck

@alinabuzachis alinabuzachis force-pushed the check_mode_nat_gateway branch 2 times, most recently from d41016a to 34c473f Compare March 9, 2021 12:21
@alinabuzachis
* Address reviewer comments
851b6c9 
Signed-off-by: Alina Buzachis <[email protected]>
@alinabuzachis alinabuzachis force-pushed the check_mode_nat_gateway branch from 34c473f to 851b6c9 Compare March 9, 2021 12:24
tremble
tremble approved these changes Mar 9, 2021
View reviewed changes
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.

LGTM

@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Mar 9, 2021
@tremble
Copy link
Contributor

tremble commented Mar 10, 2021

Tests passed. (initial failures were due to exceeding the AWS quota for EIPs)
Merging.

@tremble tremble merged commit 044b30c into ansible-collections:main Mar 10, 2021
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request Mar 10, 2021
@alinabuzachis
[ec2_vpc_nat_gateway] Fix broken check_mode (ansible-collections#436)
362ea50 
* ec2_vpc_nat_gateway: fix broken check_mode
     * fix broken check_mode (remove hard coded values)
Signed-off-by: Alina Buzachis <[email protected]>
danquixote pushed a commit to danquixote/community.aws that referenced this pull request May 16, 2021
@alinabuzachis @danquixote
[ec2_vpc_nat_gateway] Fix broken check_mode (ansible-collections#436)
6e787f7 
* ec2_vpc_nat_gateway: fix broken check_mode
     * fix broken check_mode (remove hard coded values)
Signed-off-by: Alina Buzachis <[email protected]>
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request Jul 16, 2021
@alinabuzachis
[promoted][ec2_vpc_nat_gateway] Fix broken check_mode (ansible-collec…
1df891d 
…tions#436)

* ec2_vpc_nat_gateway: fix broken check_mode
     * fix broken check_mode (remove hard coded values)
Signed-off-by: Alina Buzachis <[email protected]>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@044b30c
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@alinabuzachis
[ec2_vpc_nat_gateway] Fix broken check_mode (ansible-collections#436)
eff918e 
* ec2_vpc_nat_gateway: fix broken check_mode
     * fix broken check_mode (remove hard coded values)
Signed-off-by: Alina Buzachis <[email protected]>
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@alinabuzachis
[ec2_vpc_nat_gateway] Fix broken check_mode (ansible-collections#436)
f5c4c45 
* ec2_vpc_nat_gateway: fix broken check_mode
     * fix broken check_mode (remove hard coded values)
Signed-off-by: Alina Buzachis <[email protected]>
danielcotton pushed a commit to danielcotton/community.aws that referenced this pull request Nov 23, 2021
@alinabuzachis @danielcotton
[ec2_vpc_nat_gateway] Fix broken check_mode (ansible-collections#436)
3b46933 
* ec2_vpc_nat_gateway: fix broken check_mode
     * fix broken check_mode (remove hard coded values)
Signed-off-by: Alina Buzachis <[email protected]>
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
@mandar242
ec2_group: update documentation for specifying port ranges (ansible-c…
c060264 
…ollections#436)

ec2_group: update documentation for specifying port ranges

Depends-on: ansible-collections#429
SUMMARY

Added missing part of documentation for specifying port ranges as -1 value in parameter from_port and to_port is only supported for proto:icmp.
Fixes ansible-collections#397

ISSUE TYPE


Docs Pull Request

COMPONENT NAME

ec2_group

Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Alina Buzachis <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: None <None>
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
@alinabuzachis @jillr
Fix broken check_mode (ansible-collections#436)
1c5950f 
* ec2_vpc_nat_gateway: fix broken check_mode
     * fix broken check_mode (remove hard coded values)
Signed-off-by: Alina Buzachis <[email protected]>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@044b30c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jillr jillr jillr left review comments

@tremble tremble tremble approved these changes

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug community_review integration tests/integration module module plugins plugin (any type) tests tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alinabuzachis @ansibullbot @jillr @tremble