Overcome "Connection Refused" on some operations

[dagwieers]

So some operations on Windows actually cause the WinRM service to become unavailable for some time and WinRM currently cannot overcome this problem. Since this is a real problem to us when e.g. installing SCVMM on servers and we cannot influence the installer, here is the ~work-around~fix we implemented.

~The current implementation simply tries 5 times with a 5 seconds delay, but YMMV. Ideally this is configurable by the user, and future defaults should be tested.~

This fixes ansible/ansible#25532

[coveralls]

Coverage decreased (-2.5%) to 65.341% when pulling 8ab78e458574a2aa36064b3fbbcf1e21a318dca8 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.6%) to 66.282% when pulling ee6a92de52111a4f461767b3d6640276d4c53f04 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.6%) to 66.282% when pulling 272716af4163f76fcc79c21d9f55ba479203a0a6 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-2.3%) to 65.527% when pulling ed137b3fba67179a5797fcfb474d642b77fc5e0c on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.2%) to 66.667% when pulling 8d21c4fb66b38c7e1d5a227562b030b0b02025e5 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.6%) to 66.282% when pulling 272716af4163f76fcc79c21d9f55ba479203a0a6 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.6%) to 66.282% when pulling 272716af4163f76fcc79c21d9f55ba479203a0a6 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.6%) to 66.282% when pulling 272716af4163f76fcc79c21d9f55ba479203a0a6 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.6%) to 66.282% when pulling 272716af4163f76fcc79c21d9f55ba479203a0a6 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.6%) to 66.282% when pulling 272716af4163f76fcc79c21d9f55ba479203a0a6 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.6%) to 66.282% when pulling 272716af4163f76fcc79c21d9f55ba479203a0a6 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.6%) to 66.282% when pulling 272716af4163f76fcc79c21d9f55ba479203a0a6 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.6%) to 66.282% when pulling 272716af4163f76fcc79c21d9f55ba479203a0a6 on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-1.2%) to 66.667% when pulling db3c89e63d219358322590353f8d27a07524838f on dagwieers:patch-2 into b1a54bc95ec16677c6619b52625e0b5b5bf8a29e on diyan:master.

[coveralls]

Coverage decreased (-2.8%) to 69.061% when pulling cc050057ed686763272666c53bbc3fee5d1cf6b5 on dagwieers:patch-2 into f2fae367d3ac2abc3ed6fe9c7c17e6ffbac69e67 on diyan:master.

[coveralls]

Coverage increased (+0.5%) to 69.912% when pulling 22f296684a80ce420bdcd1911ab6373eb0dc8bcd on dagwieers:patch-2 into ffec9542d2063305efab9242ffb3623ed618756f on diyan:master.

[nitzmahone]

I wouldn't mind getting some version of this in, but I'd like it to be configurable... Unfortunately with the current API pattern that means yet more constructor kwargs... :(

[avishefi]

Can we merge this? WinRM connection retries are critical for Ansible working against Windows machines.

Edit: with a configurable retries amount so it can be controlled.

[dagwieers]

This has now been implemented.

[dagwieers]

cc @jborean93 @nitzmahone I think this is ready to be merged. Please review.

[dagwieers]

Now retries is off by default, please re-review.

[VladislavPershin]

@dagwieers , any news?

[dagwieers]

No, I am waiting for feedback.

[dagwieers]

BTW The pypsrp code has this code added recently: https://github.com/jborean93/pypsrp/pull/10 ~But we still need to add the supporting code in the Ansible psrp connection plugin. https://github.com/ansible/ansible/pull/49772~

[dagwieers]

Please review !

[jonathanmedd]

We have this exact same issue when installing SCVMM. Would be good to see this reviewed and implemented.

[jborean93]

If you are moving forward with this request you will need to optionally set status as that was only added in requests 2.14.0 and urllib3 1.21 whereas we have a minimum required of 2.9.1 (mostly for EL7 compat). I had to do something similar with https://github.com/jborean93/pypsrp/commit/95fb032bab438590d88b0a571a9f84cbb12fe9f0 because it would fail on older requests versions.

[0x4c6565]

Running into problems which this PR would fix too - not sure if you're able to progress with this @dagwieers or anything I can do?

[gvcgael]

We are testing this patch because we also have connection issues with WinRM. It seems that this patch helps a lot (but it does not cover all errors) on ReadTimeout. It would be great to see it merged.

[Angelicvorian]

Hi there, We are also running into issues with WinRM along these lines. Being able to specify a retry would be awesome. I've looked at PSRP but for some of the things we are doing, it's not suitable. Is there anything we can do to help this along or has this been covered with a different PR somewhere else and not documented?

[Akasurde]

@dagwieers Can you please rebase this with the required review comments? Thanks