Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

wait_for: Fix false positives with state=drained #1822

Closed
wants to merge 1 commit into from
Closed

wait_for: Fix false positives with state=drained #1822

wants to merge 1 commit into from

Conversation

ahill00
Copy link
Contributor

@ahill00 ahill00 commented Jul 28, 2015

While manually testing wait_for checking for drained connections, I found the
module was wrongly returning drained.

To reproduce the current state:

source #] nc -l -p 9797
destination #] nc destination 9797

Then, invoke the module:

ansible hosts -m "wait_for" -a "port=9797 delay=1 state=drained"

The module instantly returns 'drained' even though the connection is open.

This patch has two fixes:

  • _get_exclude_ips for Linux returned a list of tuples, not a list of IP strings
  • A logic issue detecting the active connection count.

While manually testing wait_for checking for drained connections, I found the
module was wrongly returning drained.

To reproduce the current state:

    source #] nc -l -p 9797
    destination #] nc destination 9797

Then, invoke the module:

    ansible hosts -m "wait_for" -a "port=9797 delay=1 state=drained"

The module instantly returns 'drained' even though the connection is open.

This patch has two fixes:
- _get_exclude_ips for Linux returned a list of tuples, not a list of IP strings
- A logic issue detecting the active connection count.
@vaupelt
Copy link
Contributor

vaupelt commented Jul 28, 2015

Eventually you have only forgotten to specify a host. This attribute defaults to 127.0.0.1. I think this fix (the logic issue) will work for you if you want to wait for a connection on any ip different to 127.0.0.1 and haven't specified it properly. Otherwise I encourage you to have a look at PR1301.

@ahill00
Copy link
Contributor Author

ahill00 commented Jul 29, 2015

@vaupelt - Shouldn't using state='drained' and specifying a host be impossible? There would be no way for a remote host's TCP connections to be inspected.

@vaupelt
Copy link
Contributor

vaupelt commented Jul 29, 2015

@andyhky - the host attribute is compared with local addresses.

ansible-doc wait_for lists some examples (item 2 and 3) with state=drained.

@ahill00
Copy link
Contributor Author

ahill00 commented Jul 29, 2015

@vaupelt - Thanks for the clarification. I tested #1301 with my scenario and it is more complete since it offers IPv6 support. Closing this PR.

@ahill00 ahill00 closed this Jul 29, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants