wait_for_connection timeout/retries fixes

[lilatomic]

SUMMARY

fixes #74237

Sets the timeout and disables retries on the connection.
Also improves the timeout behaviour so it won't exceed the maximum timeout specified.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

wait_for_connection

ADDITIONAL INFORMATION

Before:

time ansible -i 10.99.99.99, -m wait_for_connection -a '{{ {"timeout": 3, "connect_timeout": 1} }}' all
10.99.99.99 | FAILED! => {
    "changed": false,
    "elapsed": 11,
    "msg": "timed out waiting for ping module test: Failed to connect to the host via ssh: ssh: connect to host 10.99.99.99 port 22: Connection timed out"
}
Command exited with non-zero status 2
2.42user 0.20system 0:12.24elapsed 21%CPU (0avgtext+0avgdata 46592maxresident)k
0inputs+16outputs (0major+30820minor)pagefaults 0swaps

After:

time ansible -i 10.99.99.99, -m wait_for_connection -a '{{ {"timeout": 3, "connect_timeout": 1} }}' all
10.99.99.99 | FAILED! => {
    "changed": false,
    "elapsed": 2,
    "msg": "timed out waiting for ping module test: Failed to connect to the host via ssh: ssh: connect to host 10.99.99.99 port 22: Connection timed out"
}
Command exited with non-zero status 2
1.30user 0.14system 0:03.35elapsed 43%CPU (0avgtext+0avgdata 46864maxresident)k
41912inputs+16outputs (117major+30787minor)pagefaults 0swaps

We can see that the elapsed time no longer exceeds the timeout

[s-hertel]

The added tests are awesome.

[s-hertel]

I think you could simplify this. The _total_seconds shim doesn't seem necessary since those Python versions aren't supported anyway.

Suggested change

	while datetime.utcnow() < max_end_time:
	while datetime.utcnow() < max_end_time:
	if (datetime.utcnow() + timedelta(seconds=connect_timeout)) > max_end_time:
	# connect_timeout is longer than the max time left, so use the remaining time instead
	connect_timeout = int((max_end_time - datetime.utcnow()).total_seconds())
	if connect_timeout < 1:
	# invalid timeout or only a fraction of a second is left
	break

[lilatomic]

oh that's much simpler. I modified the suggestion slightly so it doesn't reassign to the connect_timeout argument and so it uses the same value for 'now' throughout an iteration of the while loop (good practice for guarding against timing edge cases).

[ansibot]

@lilatomic This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

[webknjaz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[lilatomic]

looks like a flaky CI? I don't think this would affect the galaxy setup
looks like this hung:

03:53 TASK [ansible-galaxy-collection : setup test collections for install and download test] ***
50:02 ERROR: Tests aborted after exceeding the 50 minute time limit.

[webknjaz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[webknjaz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).