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] Add tag support #372

Merged
merged 1 commit into from
Feb 1, 2021
Merged

[ec2_vpc_nat_gateway] Add tag support #372

merged 1 commit into from
Feb 1, 2021

Conversation

alinabuzachis
Copy link
Contributor

SUMMARY

Tags feature: support tags specification for NAT gateway

ISSUE TYPE
  • implements tags feature for NAT gateway
  • adds integration test tasks for tags feature
ADDITIONAL INFORMATION
  • refactor integration tests (overall) removing hard-coded parameters
  • add missing integration test tasks without CHECK_MODE

@alinabuzachis alinabuzachis requested a review from jillr January 26, 2021 13:37
@ansibullbot
Copy link

@alinabuzachis this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request merge_commit This PR contains at least one merge commit. Please resolve! module module needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_triage needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 26, 2021
@ansibullbot
Copy link

@alinabuzachis this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot
Copy link

@alinabuzachis this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

tremble
tremble requested changes Jan 26, 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.

Some nits while you're still cleaning up.

plugins/modules/ec2_vpc_nat_gateway.py Outdated Show resolved Hide resolved
plugins/modules/ec2_vpc_nat_gateway.py Outdated Show resolved Hide resolved
plugins/modules/ec2_vpc_nat_gateway.py Show resolved Hide resolved
tests/integration/targets/ec2_vpc_nat_gateway/tasks/main.yml Outdated Show resolved Hide resolved
plugins/modules/ec2_vpc_nat_gateway.py Outdated Show resolved Hide resolved
plugins/modules/ec2_vpc_nat_gateway.py Outdated Show resolved Hide resolved
@jillr jillr changed the title I345 [ec2_vpc_nat_gateway] Add tag support Jan 27, 2021
@ansibullbot
Copy link

@alinabuzachis this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@jillr
Copy link
Collaborator

jillr commented Jan 29, 2021

@tremble We're thinking about refactoring the vpc_nat_gateway modules sooner than later (to support migrating them to amazon.aws), how do you feel about just adding an until: result_var_foo is not failed to any info tasks where we're seeing RequestLimitExceeded errors in shippable and we can look at putting waiters in the module or else requesting an increase from AWS when we sit down and clean up the modules?

@tremble
Copy link
Contributor

tremble commented Jan 29, 2021

@tremble ... how do you feel about just adding an until: result_var_foo is not failed to any info tasks where we're seeing RequestLimitExceeded errors in shippable ...?

I'm fine with that. I certainly don't want to hold up this PR based on rate limit flakes. That said, given we've identified an issue I'd like to see it tracked.

@ansibullbot
Copy link

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

@ansibullbot ansibullbot added integration tests/integration plugins plugin (any type) tests tests and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jan 29, 2021
@alinabuzachis alinabuzachis force-pushed the i345 branch 2 times, most recently from 7f02154 to 8ca0e79 Compare January 29, 2021 20:09
@alinabuzachis
Tags feature: support tags specification for NAT gateway (ansible-col…
0fec3b1 
…lections#345)

 * implement tags feature for NAT gateway
 * add integration test tasks for tags feature
 * refactor integration tests (overall) removing hard-coded parameters
 * add missing integration test tasks without CHECK_MODE
 * include until loop for some tasks as they failed during the integration

 * added code to support tags in ec2_vap_nat_gateway - return error 'NoneType' object has no attribute 'get' because of curr_tags seems to remain None
 * removed tests in check_mode because not working due to DRY_RUN_GATEWAY
 * Addressed reviewers comments

Signed-off-by: Alina Buzachis <[email protected]>
tremble
tremble approved these changes Feb 1, 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.

existing comments fixed and flake tracked in #379

@tremble tremble merged commit b7afd18 into ansible-collections:main Feb 1, 2021
@tremble
Copy link
Contributor

tremble commented Feb 1, 2021

Thanks for your work on this @alinabuzachis

@ikhan2010
Copy link
Member

@alinabuzachis great work!

@tremble tremble mentioned this pull request Feb 11, 2021
Merged
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request Jul 16, 2021
@alinabuzachis
[promoted]Tags feature: support tags specification for NAT gateway (a…
0985f93 
…nsible-collections#345) (ansible-collections#372)

* implement tags feature for NAT gateway
 * add integration test tasks for tags feature
 * refactor integration tests (overall) removing hard-coded parameters
 * add missing integration test tasks without CHECK_MODE
 * include until loop for some tasks as they failed during the integration

 * added code to support tags in ec2_vap_nat_gateway - return error 'NoneType' object has no attribute 'get' because of curr_tags seems to remain None
 * removed tests in check_mode because not working due to DRY_RUN_GATEWAY
 * Addressed reviewers comments

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

Co-authored-by: Alina Buzachis <[email protected]>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@b7afd18
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@alinabuzachis
Tags feature: support tags specification for NAT gateway (ansible-col…
304ecac 
…lections#345) (ansible-collections#372)

* implement tags feature for NAT gateway
 * add integration test tasks for tags feature
 * refactor integration tests (overall) removing hard-coded parameters
 * add missing integration test tasks without CHECK_MODE
 * include until loop for some tasks as they failed during the integration

 * added code to support tags in ec2_vap_nat_gateway - return error 'NoneType' object has no attribute 'get' because of curr_tags seems to remain None
 * removed tests in check_mode because not working due to DRY_RUN_GATEWAY
 * Addressed reviewers comments

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

Co-authored-by: Alina Buzachis <[email protected]>
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@alinabuzachis
Tags feature: support tags specification for NAT gateway (ansible-col…
de7b209 
…lections#345) (ansible-collections#372)

* implement tags feature for NAT gateway
 * add integration test tasks for tags feature
 * refactor integration tests (overall) removing hard-coded parameters
 * add missing integration test tasks without CHECK_MODE
 * include until loop for some tasks as they failed during the integration

 * added code to support tags in ec2_vap_nat_gateway - return error 'NoneType' object has no attribute 'get' because of curr_tags seems to remain None
 * removed tests in check_mode because not working due to DRY_RUN_GATEWAY
 * Addressed reviewers comments

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

Co-authored-by: Alina Buzachis <[email protected]>
danielcotton pushed a commit to danielcotton/community.aws that referenced this pull request Nov 23, 2021
@alinabuzachis @danielcotton
Tags feature: support tags specification for NAT gateway (ansible-col…
9986885 
…lections#345) (ansible-collections#372)

* implement tags feature for NAT gateway
 * add integration test tasks for tags feature
 * refactor integration tests (overall) removing hard-coded parameters
 * add missing integration test tasks without CHECK_MODE
 * include until loop for some tasks as they failed during the integration

 * added code to support tags in ec2_vap_nat_gateway - return error 'NoneType' object has no attribute 'get' because of curr_tags seems to remain None
 * removed tests in check_mode because not working due to DRY_RUN_GATEWAY
 * Addressed reviewers comments

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

Co-authored-by: Alina Buzachis <[email protected]>
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
@goneri
aws_s3/test: keep length of name below 63 char (ansible-collections#372)
8c39dc0 
aws_s3/test: keep length of name below 63 char

Reviewed-by: https://github.com/apps/ansible-zuul
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
@alinabuzachis @jillr
Tags feature: support tags specification for NAT gateway (ansible-col…
3b7728e 
…lections#345) (ansible-collections#372)

* implement tags feature for NAT gateway
 * add integration test tasks for tags feature
 * refactor integration tests (overall) removing hard-coded parameters
 * add missing integration test tasks without CHECK_MODE
 * include until loop for some tasks as they failed during the integration

 * added code to support tags in ec2_vap_nat_gateway - return error 'NoneType' object has no attribute 'get' because of curr_tags seems to remain None
 * removed tests in check_mode because not working due to DRY_RUN_GATEWAY
 * Addressed reviewers comments

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

Co-authored-by: Alina Buzachis <[email protected]>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@b7afd18
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

@Akasurde Akasurde Akasurde left review comments

@tremble tremble tremble approved these changes

Assignees
No one assigned
Labels
feature This issue/PR relates to a feature request integration tests/integration module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR plugins plugin (any type) tests tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ec2_vpc_nat_gateway should support tags
6 participants
@alinabuzachis @ansibullbot @jillr @tremble @ikhan2010 @Akasurde