elb_target_facts Fixes #39780 #43565

mostfunkyduck · 2018-08-01T19:21:57Z

SUMMARY

Adds an 'elb_target_facts' module which will gather a list of all target groups to which a given instance is registered, covering both IP and instance target groups. This is added to a fact called 'ec2_tgs', which can be used by a playbook to change deregister/reregister nodes to the target groups.

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

lib/ansible/modules/cloud/amazon/elb_target_facts.py

ANSIBLE VERSION

ansible 2.7.0.dev0 (elb_target_facts f4d8b185f4) last updated 2018/08/01 14:26:26 (GMT -400)
  config file = None
  configured module search path = [u'/Users/ykuperman/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/ykuperman/code/ansible_devel/lib/ansible
  executable location = /Users/ykuperman/code/ansible_devel/bin/ansible
  python version = 2.7.10 (default, Jul 15 2017, 17:16:57) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.31)]

ADDITIONAL INFORMATION


localhost | SUCCESS => {
    "ansible_facts": {
        "ec2_tgs": [
            {
                "target_group_arn": "<arn>", 
                "target_group_type": "instance", 
                "targets": [
                    {
                        "target_az": null, 
                        "target_health": {
                            "description": "Health checks failed", 
                            "reason": "Target.FailedHealthChecks", 
                            "state": "unhealthy"
                        }, 
                        "target_id": "i-0289cbd5cc4f5f458", 
                        "target_port": 8081
                    }, 
                    {
                        "target_az": null, 
                        "target_health": {
                            "description": "Health checks failed", 
                            "reason": "Target.FailedHealthChecks", 
                            "state": "unhealthy"
                        }, 
                        "target_id": "i-0289cbd5cc4f5f458", 
                        "target_port": 8080
                    }
                ]
            }, 
            {
                "target_group_arn": "<arn>", 
                "target_group_type": "ip", 
                "targets": [
                    {
                        "target_az": "us-west-2a", 
                        "target_health": {
                            "state": "healthy"
                        }, 
                        "target_id": "172.31.22.138", 
                        "target_port": 22
                    }, 
                    {
                        "target_az": "us-west-2a", 
                        "target_health": {
                            "state": "healthy"
                        }, 
                        "target_id": "172.31.24.19", 
                        "target_port": 22
                    }
                ]
            }
        ]
    }, 
    "changed": false
}

ansibot · 2018-08-01T19:23:39Z

@yaakov-github this PR contains the following merge commits:

f01c001

Please rebase your branch to remove these commits.

click here for bot help

ansibot · 2018-08-01T19:23:39Z

Hi @yaakov-github,

Thank you for the pullrequest, 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

registered ELBv2 target groups for a given instance id

ansibot · 2018-08-02T12:20:03Z

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

test/integration/targets/elb_target_facts/playbooks/roles/elb_target_facts/tasks/main.yml:251:13: hyphens too many spaces after hyphen

click here for bot help

ansibot · 2018-08-02T12:37:11Z

@Constantin007 @Constantin07 @Deepakkothandan @Etherdaemon @Java1Guy @Lujeni @Madhura-CSI @MichaelBaydoun @Sodki @adq @akazakov @alachaum @amir343 @anryko @bekelchik @bpennypacker @brandond @carsongee @chenl87 @defunctio @dennisconrad @dkhenry @fiunchinho @fivethreeo @flowerysong @garethr @gunzy83 @gurumaia @hsingh @hyperized @iiibrad @infectsoldier @j-carl @jarv @Java1Guy @jimbydamonk @jmenga @joelthompson @jonhadfield @jonmer85 @joshsouza @jsdalton @jsmartin @kaczynskid @leedm777 @linuxdynasty @loia @lwade @MichaelBaydoun @michaeljs1990 @minichate @mjschultz @mmochan @nadirollo @nand0p @naslanidis @nathanwebsterdotme @nerzhul @nickball @orthanc @piontas @pjodouin @prasadkatti @psykotox @pwnall @raags @rickmendes @roadmapper @ryansydnor @scicoin-project @scottanderson42 @shepdelacreme @silviud @slapula @steynovich @tastychutney @tedder @tgerla @timmahoney @tombamford @tsiganenok @viper233 @whiter @willricardo @wilvk @wimnat @zacblazic @zbal @zeekin @zimbatm

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

willthames · 2018-08-22T23:32:35Z

Let's not use ansible_facts. Just return a dict containing instance_target_groups or something

willthames · 2018-08-22T23:33:37Z

lib/ansible/modules/cloud/amazon/elb_target_facts.py

+        supports_check_mode=True,
+    )
+
+    if not HAS_BOTO3:


Not needed with AnsibleAWSModule

willthames · 2018-08-22T23:35:55Z

lib/ansible/modules/cloud/amazon/elb_target_facts.py

+        """Fetch all IPs associated with this instance so that we can determine
+           whether or not an instance is in an IP-based target group"""
+        try:
+            ec2 = self.module.client(


I really don't see the point of using a class if you're just going to create the client inside methods!

I'd create the ec2 and elbv2 clients at __init__ time and just use self.ec2 in the methods

ryansb

Minor style change needed, looks good otherwise.

ryansb · 2018-08-22T18:54:55Z

lib/ansible/modules/cloud/amazon/elb_target_facts.py

+"""
+
+RETURN = """
+ec2_tgs:


ryansb · 2018-08-23T15:54:47Z

lib/ansible/modules/cloud/amazon/elb_target_facts.py

+                                      AWSRetry)
+
+
+class Target:


These declarations need to be new-style classes like Target(object):

… return a fact, and use instance variables for the AWS connections. updating tests to go along with that

arbabnazar · 2018-08-30T15:19:28Z

@ryansb will it be available for 2.6 future release or 2.7.0?

ryansb · 2018-08-30T15:21:21Z

2.7 is the targeted release for this.

willthames · 2018-08-30T22:15:54Z

You can use it with 2.6 though: http://willthames.github.io/2017/12/12/using-updated-modules-with-stable-ansible.html

jborean93 removed the needs_triage Needs a first human triage before being processed. label Aug 2, 2018

= added 7 commits August 2, 2018 08:09

initial source of the elb_target_facts module, which gathers all

334198d

registered ELBv2 target groups for a given instance id

updating

fec36da

initial dump of tests

89d5466

updating docs, adding AWSRetry decorators

ce433d2

updating tests

84e7683

More brush up, some refactoring

0c4a2fd

updating for sanity tests

fb80ffd

ansibot added ci_verified Changes made in this PR are causing tests to fail. 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 Aug 2, 2018

removing extra space

f356912

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Aug 2, 2018

ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 2, 2018

ansibot added the community_review In order to be merged, this PR must follow the community review workflow. label Aug 2, 2018

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 Aug 10, 2018

willthames reviewed Aug 22, 2018

View reviewed changes

ryansb suggested changes Aug 23, 2018

View reviewed changes

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Aug 23, 2018

updating elb_target_facts to use a new name for the return value, not…

0a4fd7e

… return a fact, and use instance variables for the AWS connections. updating tests to go along with that

ansibot removed 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 Aug 27, 2018

updating classes to be 'new-style' classes

75f4971

ryansb approved these changes Aug 30, 2018

View reviewed changes

ryansb merged commit 6b7ea50 into ansible:devel Aug 30, 2018

ansible locked and limited conversation to collaborators Jul 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

elb_target_facts Fixes #39780 #43565

elb_target_facts Fixes #39780 #43565

mostfunkyduck commented Aug 1, 2018

ansibot commented Aug 1, 2018

ansibot commented Aug 1, 2018

ansibot commented Aug 2, 2018

ansibot commented Aug 2, 2018

willthames commented Aug 22, 2018

willthames Aug 22, 2018

willthames Aug 22, 2018

ryansb left a comment

ryansb Aug 22, 2018

ryansb Aug 23, 2018

arbabnazar commented Aug 30, 2018

ryansb commented Aug 30, 2018

willthames commented Aug 30, 2018

elb_target_facts Fixes #39780 #43565

elb_target_facts Fixes #39780 #43565

Conversation

mostfunkyduck commented Aug 1, 2018

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ADDITIONAL INFORMATION

ansibot commented Aug 1, 2018

ansibot commented Aug 1, 2018

ansibot commented Aug 2, 2018

ansibot commented Aug 2, 2018

willthames commented Aug 22, 2018

willthames Aug 22, 2018

Choose a reason for hiding this comment

willthames Aug 22, 2018

Choose a reason for hiding this comment

ryansb left a comment

Choose a reason for hiding this comment

ryansb Aug 22, 2018

Choose a reason for hiding this comment

ryansb Aug 23, 2018

Choose a reason for hiding this comment

arbabnazar commented Aug 30, 2018

ryansb commented Aug 30, 2018

willthames commented Aug 30, 2018