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

Bug Fix: Route 53 Health Check behaves incorrectly with > 100 health checks #58539

Merged
merged 1 commit into from
Jul 11, 2019
Merged

Bug Fix: Route 53 Health Check behaves incorrectly with > 100 health checks #58539

merged 1 commit into from
Jul 11, 2019

Conversation

mdierolf
Copy link
Contributor

This PR fixes a bug where the Route 53 Health Check would add a new health check instead of updating an existing one when the user has > 100 health checks on their AWS account

SUMMARY

When you attempt to update a health check with route53_health_check, the module first loads a list of all your health checks, looks for a matching health check, and then updates an existing check if found, or creates a new one.

However, when you have more than 100 health checks, AWS requires you to list the health checks 100 at a time and walk through the paginated results, which this module was not doing, causing massive numbers of duplicate health checks to be created, as the module thought that their were no existing health checks which matched

This PR implements pagination for the find_health_check call to fix this bug

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

route53_health_check

…eck instead of updating an existing one when the user has > 100 health checks
@ansibot
Copy link
Contributor

ansibot commented Jun 29, 2019

@ansibot
Copy link
Contributor

ansibot commented Jun 29, 2019

@mdierolf, just so you are aware we have a dedicated Working Group for aws.
You can find other people interested in this in #ansible-aws on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 aws bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:community This issue/PR relates to code supported by the Ansible community. labels Jun 29, 2019
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 8, 2019
@jillr jillr self-assigned this Jul 11, 2019
@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Jul 11, 2019
Copy link
Contributor

@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.

As we discussed in irc there's some limitations around testing for this use case, this module ultimately needs to be updated to boto3. But testing locally I can cleanly create/delete healthchecks with this branch and the code lgtm. Thanks for this fix @mdierolf !

@jillr jillr merged commit 9fc710f into ansible:devel Jul 11, 2019
@ansible ansible locked and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 aws bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. new_contributor This PR is the first contribution by a new community member. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants