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

Stabilize and improve ec2_vpc_endpoint modules #473

Merged
merged 4 commits into from
Mar 24, 2021

Conversation

jillr
Copy link
Collaborator

@jillr jillr commented Mar 13, 2021

SUMMARY
  • Add tagging support
  • Make idempotent
  • Better exception handling
  • Better check_mode support
  • Use module_utils for common functions
  • Enable retries on common AWS failures
ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME

ec2_vpc_endpoint
ec2_vpc_endpoint_info

@ansibullbot
Copy link

@ansibullbot ansibullbot added WIP Work in progress bug This issue/PR relates to a bug integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels Mar 13, 2021
@jillr jillr force-pushed the cleanup_ec2_vpc_endpoint branch from c758f69 to 70cbdea Compare March 15, 2021 21:25
@jillr jillr changed the title [WIP] Stabilize and improve ec2_vpc_endpoint modules Stabilize and improve ec2_vpc_endpoint modules Mar 15, 2021
@ansibullbot ansibullbot added community_review needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed WIP Work in progress community_review labels Mar 15, 2021
time.sleep(polling_increment_secs)
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
module.fail_json_aws(e, msg='Failure while waiting for status')
def match_endpoints(module, endpoint):
Copy link
Member

Choose a reason for hiding this comment

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

You use module just to get access to the route_table_ids and service parameters. In order to reduce the inter-dependencies, I suggest to directly pass this two values to the function.
By doing so, it would be easier to write an unit-test.

Copy link
Member

Choose a reason for hiding this comment

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

To be clear, this is a nitpicking level suggestion as I know we use to pass module everywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a good point though, and easily changed - thanks

Copy link
Contributor

@alinabuzachis alinabuzachis left a comment

Choose a reason for hiding this comment

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

@jillr It looks good to me.

@ansibullbot ansibullbot added community_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Mar 19, 2021
jillr added 4 commits March 19, 2021 11:08
- Add tagging support
- Make idempotent
- Better exception handling
- Better check_mode support
- Use module_utils for common functions
- Enable retries on common AWS failures
@jillr jillr force-pushed the cleanup_ec2_vpc_endpoint branch from 2fa211b to b2317d3 Compare March 19, 2021 18:08
@jillr jillr requested a review from goneri March 19, 2021 18:10
@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR community_review and removed community_review needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Mar 19, 2021
Comment on lines +238 to +239
try:
result = client.describe_vpc_endpoints(aws_retry=True, **params)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're cleaning up this should probably be moved to a paginated query, they seem to be more consistent than simple describe lookups.

@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 24, 2021
@jillr jillr merged commit f1d338d into ansible-collections:main Mar 24, 2021
@jillr jillr deleted the cleanup_ec2_vpc_endpoint branch July 2, 2021 22:46
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 16, 2021
…lections#473)

* Stabilize and improve ec2_vpc_endpoint modules

- Add tagging support
- Make idempotent
- Better exception handling
- Better check_mode support
- Use module_utils for common functions
- Enable retries on common AWS failures

* Make endpoint deletion idempotent in check_mode

* Sanity fixes

* Address review feedback

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@f1d338d
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
* Stabilize and improve ec2_vpc_endpoint modules

- Add tagging support
- Make idempotent
- Better exception handling
- Better check_mode support
- Use module_utils for common functions
- Enable retries on common AWS failures

* Make endpoint deletion idempotent in check_mode

* Sanity fixes

* Address review feedback
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
* Stabilize and improve ec2_vpc_endpoint modules

- Add tagging support
- Make idempotent
- Better exception handling
- Better check_mode support
- Use module_utils for common functions
- Enable retries on common AWS failures

* Make endpoint deletion idempotent in check_mode

* Sanity fixes

* Address review feedback
danielcotton pushed a commit to danielcotton/community.aws that referenced this pull request Nov 23, 2021
* Stabilize and improve ec2_vpc_endpoint modules

- Add tagging support
- Make idempotent
- Better exception handling
- Better check_mode support
- Use module_utils for common functions
- Enable retries on common AWS failures

* Make endpoint deletion idempotent in check_mode

* Sanity fixes

* Address review feedback
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
* Stabilize and improve ec2_vpc_endpoint modules

- Add tagging support
- Make idempotent
- Better exception handling
- Better check_mode support
- Use module_utils for common functions
- Enable retries on common AWS failures

* Make endpoint deletion idempotent in check_mode

* Sanity fixes

* Address review feedback

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@f1d338d
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
…ions#473)

Add some integration tests for aws_service_ip_ranges

SUMMARY
Add some integration tests for aws_service_ip_ranges
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
aws_service_ip_ranges
ADDITIONAL INFORMATION
Initial tests to support ansible-collections#430

Reviewed-by: Alina Buzachis <None>
Reviewed-by: None <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug integration tests/integration module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR needs_triage plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants