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.

Sign up for GitHub

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

Jump to bottom

Overcome "Connection Refused" on some operations #174

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Overcome "Connection Refused" on some operations #174

wants to merge 6 commits into from

Conversation

dagwieers
Copy link
Contributor

@dagwieers dagwieers commented Aug 20, 2017 •
edited
Loading

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-aroundfix 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

@dagwieers dagwieers changed the title Overcome Connection Refused on WinRM restart Overcome "Connection Refused" on WinRM restart Aug 20, 2017
@dagwieers dagwieers changed the title Overcome "Connection Refused" on WinRM restart Overcome "Connection Refused" on some operations Aug 20, 2017
@dagwieers dagwieers force-pushed the patch-2 branch 3 times, most recently from ee6a92d to 0941e35 Compare August 20, 2017 01:22
@coveralls
Copy link

coveralls commented Aug 20, 2017 •
edited
Loading

Coverage Status

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

@dagwieers dagwieers mentioned this pull request Aug 20, 2017
Closed
@coveralls
Copy link

coveralls commented Aug 20, 2017 •
edited
Loading

Coverage Status

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

@coveralls
Copy link

coveralls commented Aug 20, 2017 •
edited
Loading

Coverage Status

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

@dagwieers dagwieers force-pushed the patch-2 branch 2 times, most recently from ed137b3 to 8d21c4f Compare August 20, 2017 03:07
@coveralls
Copy link

coveralls commented Aug 20, 2017 •
edited
Loading

Coverage Status

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

@coveralls
Copy link

coveralls commented Aug 20, 2017 •
edited
Loading

Coverage Status

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

@coveralls
Copy link

coveralls commented Aug 20, 2017 •
edited
Loading

Coverage Status

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

3 similar comments
@coveralls
Copy link

coveralls commented Aug 20, 2017 •
edited
Loading

Coverage Status

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

@coveralls
Copy link

Coverage Status

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

@coveralls
Copy link

Coverage Status

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

@coveralls
Copy link

coveralls commented Aug 20, 2017 •
edited
Loading

Coverage Status

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

3 similar comments
@coveralls
Copy link

Coverage Status

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

@coveralls
Copy link

coveralls commented Aug 20, 2017 •
edited
Loading

Coverage Status

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

@coveralls
Copy link

Coverage Status

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

@coveralls
Copy link

coveralls commented Aug 20, 2017 •
edited
Loading

Coverage Status

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

@dagwieers dagwieers mentioned this pull request Aug 20, 2017
Closed
@dagwieers dagwieers mentioned this pull request Sep 25, 2017
Closed
@coveralls
Copy link

coveralls commented Sep 25, 2017 •
edited
Loading

Coverage Status

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

@coveralls
Copy link

coveralls commented Sep 25, 2017 •
edited
Loading

Coverage Status

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

@dagwieers dagwieers mentioned this pull request Oct 2, 2017
Closed
@nitzmahone nitzmahone mentioned this pull request Dec 8, 2017
Closed
@dagwieers dagwieers force-pushed the patch-2 branch 2 times, most recently from dd61198 to cbfbc4b Compare October 8, 2018 03:25
@dagwieers
Copy link
Contributor Author

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

@VladislavPershin
Copy link

@dagwieers , any news?

@dagwieers
Copy link
Contributor Author

No, I am waiting for feedback.

@dagwieers
Copy link
Contributor Author

dagwieers commented Dec 11, 2018 •
edited
Loading

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

@dagwieers dagwieers mentioned this pull request Dec 12, 2018
Closed
@dagwieers dagwieers mentioned this pull request Dec 28, 2018
Closed
@dagwieers
Copy link
Contributor Author

Please review !

@badcure badcure mentioned this pull request May 31, 2019
Closed
@jonathanmedd
Copy link

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

badcure
badcure requested changes Jun 11, 2019
View reviewed changes
Copy link
Collaborator

@badcure badcure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can remove this, and rebase/merge from master...I can merge this.

requirements-test.txt
@@ -4,3 +4,4 @@ pytest<=3.2.5
pytest-cov
pytest-pep8
mock
pycparser<=2.18; python_version<"2.7"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be removed since python 2.6 is no longer supported.

@badcure badcure added the needs a rebase/merge Awaiting a merge/rebase from master label Jun 11, 2019
@jborean93
Copy link
Collaborator

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 jborean93/pypsrp@95fb032 because it would fail on older requests versions.

@0x4c6565
Copy link

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
Copy link

gvcgael commented Feb 24, 2020

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
Copy link

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?

@Willsparker Willsparker mentioned this pull request Apr 1, 2021
Closed
@Akasurde
Copy link

Akasurde commented Jul 1, 2021

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

@suhancz
Copy link

suhancz commented May 17, 2023

Any update on this one? I'd love to have it implemented

@Yannik Yannik mentioned this pull request Apr 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@badcure badcure badcure requested changes

Assignees
No one assigned
Labels
needs a rebase/merge Awaiting a merge/rebase from master
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

WinRM: Bails out with "[Errno 111] Connection refused"