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

Fix target group remove / add logic #493

Merged
merged 1 commit into from
Apr 10, 2021

Conversation

msven
Copy link
Contributor

@msven msven commented Mar 23, 2021

SUMMARY

Fixes target group update logic to support updating target groups when the final number
of target groups attached to the asg remains the same.

Fixes #492

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_asg

ADDITIONAL INFORMATION

Integration tests were added to validate the change. The following test would've previously failed the final assertion.

- name: update autoscaling group tg2
  ec2_asg:
    name: "{{ resource_prefix }}-asg"
    launch_template: 
      launch_template_name: "{{ resource_prefix }}-lt"
    target_group_arns:
      - "{{ out_tg2.target_group_arn }}"
    desired_capacity: 1
    min_size: 1
    max_size: 1
    state: present
    wait_for_instances: yes
  register: output

- assert:
    that:
      - "output.target_group_arns | length == 1"
      - "output.target_group_arns[0] == out_tg2.target_group_arn"

- name: update autoscaling group remove tg2 and add tg1
  ec2_asg:
    name: "{{ resource_prefix }}-asg"
    launch_template: 
      launch_template_name: "{{ resource_prefix }}-lt"
    target_group_arns:
      - "{{ out_tg1.target_group_arn }}"
    desired_capacity: 1
    min_size: 1
    max_size: 1
    state: present
    wait_for_instances: yes
  register: output

- assert:
    that:
      - "output.target_group_arns | length == 1"
      - "output.target_group_arns[0] == out_tg1.target_group_arn"

@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review integration tests/integration module module needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) tests tests labels Mar 23, 2021
@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 23, 2021
@msven
Copy link
Contributor Author

msven commented Mar 23, 2021

/rebuild

@msven
Copy link
Contributor Author

msven commented Mar 24, 2021

Not sure how to get the tests to re-run - the test failures seem to be issues pulling a docker image.

@tremble
Copy link
Contributor

tremble commented Mar 24, 2021

Test was failing with a linting issue:

06:05 ERROR: tests/integration/targets/ec2_asg/tasks/main.yml:822:7: key-duplicates: duplication of key "register" in mapping (100%)

@tremble
Copy link
Contributor

tremble commented Mar 24, 2021

@jillr any chance you could try the ASG tests locally my connection's currently too flakey for the slow ASG tests.

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.

Changes generally look good, unfortunately I'm not able to run the ASG tests for the next week or two (they're disabled in CI due to their very long duration).

@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 24, 2021
@tremble tremble requested a review from jillr March 27, 2021 12:46
@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed new_contributor Help guide this first time contributor community_review labels Apr 1, 2021
Add additional integration tests to test linking target groups
to autoscaling groups.

Add changelog
@msven
Copy link
Contributor Author

msven commented Apr 1, 2021

ready_for_review

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

tremble commented Apr 10, 2021

Successfully run the local tests

@ansible-zuul ansible-zuul bot merged commit 63769d5 into ansible-collections:main Apr 10, 2021
@tremble
Copy link
Contributor

tremble commented Apr 10, 2021

@msven, many thanks for your submission.

This fix should be available in the next release of this collection (1.5.0)

alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 16, 2021
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
Fix target group remove / add logic

Reviewed-by: Mark Chappell
             https://github.com/tremble
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
Fix target group remove / add logic

Reviewed-by: Mark Chappell
             https://github.com/tremble
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 community_review integration tests/integration module module plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ec2_asg not updating target group
3 participants