Repository: ansible/ansible
Pull request: reboot - Fix connection timeout reset
##### SUMMARY
The current logic to reset the connection timeout is not correct as it won't run if the original connection timeout (connection plugin) was never set. This PR changes the logic to compare the previous value with the current value instead to detect if there were any changes.

Fixes https://github.com/ansible/ansible/issues/48665.

##### ISSUE TYPE
- Bugfix Pull Request

##### COMPONENT NAME
reboot

Problem statement:
win_update: FATAL ERROR DURING FILE TRANSFER if Windows boot takes a long time
##### SUMMARY
win_update: FATAL ERROR DURING FILE TRANSFER if Windows boot takes a long time after update. This message appears when Windows rebooted and apply latest changes, WinRM accessible, but not working.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
win_updates

##### ANSIBLE VERSION
```paste below
ansible 2.7.1
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, Jul 13 2018, 13:06:57) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]
```

##### OS / ENVIRONMENT
Windows Server 2016

##### STEPS TO REPRODUCE
Run update install like this play
```yaml
- name: search and install updates
  hosts: windows
  gather_facts: false
  tasks:
  - name: Install updates
    win_updates:
      state: installed
      reboot: true
      reboot_timeout: 1800
      use_scheduled_task: yes
    register: updates_were_installed
    ignore_errors: true
```
When Windows reboots and showing again "Configuring Windows updates, Do not Turn off" WinRM service started but not working correctly yet. I think that this leads to error.

##### EXPECTED RESULTS
Play returns that task completed with Changed status

##### ACTUAL RESULTS
```paste below
 [WARNING]: FATAL ERROR DURING FILE TRANSFER:

fatal: [server1.contoso.net]: FAILED! => {"msg": "winrm send_input failed; \nstdout: {\"changed\":false,\"msg\":null,\"failed\":true}\r\n\nstderr "}
```

I will try to get this error with -vvvv logging.

Hints from issue discussion:
Hi @specator, thank you for submitting this issue!

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: thank_submitter --->
Files identified in the description:
None

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
Files identified in the description:
* [lib/ansible/modules/windows/win_updates.ps1](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/windows/win_updates.ps1)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @dagwieers @jborean93 @jhawkesworth @nitzmahone
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
Not necessarily the solution you were looking for, but it may help to change the 'WinRM' service to become delayed-started.

```yaml
- name: Ensure that WinRM is started when the system has settled
  win_service:
    name: WinRM
    start_mode: delayed
```
PS This is now one of the examples in the  [win_service documentation](https://docs.ansible.com/ansible/devel/modules/win_service_module.html#examples) and the [win_reboot documentation](https://docs.ansible.com/ansible/devel/modules/win_reboot_module.html#examples), as it can prevent some other issues caused by the system not being ready yet, but WinRM is available.
Another thing you could try is to increase the parameter `reboot_timeout`, which defaults to `1200` seconds (20 minutes) and you already turned up to `1800`. In cases where updates may take longer for a system to become available (pre and post reboot), it is possible that **win_updates** bails out because the WinRM service is in fact not started within the expected 20 minutes.

The problem is explained in issue [#47229 win_reboot: Default reboot_timeout (600 sec) may not suffice](https://github.com/ansible/ansible/issues/47229) but extends to the **win_updates** module as well.

I expect you have tried this already, but since I wasn't sure...
Hi @dagwieers,

Thank you for a fast answers!

If you look closer I had increase timeout to 1800 already (to 30 mins). Any way I checked that error appears before timeout exceeds (e.g. server was go to reboot and install updates at 00:00, rebooted at 00:16, booted to "Configuring Windows updates, Do not Turn off" at 00:17, error appeared at 00:18, windows booted and asked for CTRL+ALT+DEL at 00:23)

I will try your solution with delayed service start option, but want to pay attention that Windows defaults is Autostart.
Hi @dagwieers,

I had tested today with workaround "set WinRM to delayed start", "update and restart", "set WinRM to auto start" and seems that issue has gone. I will do some test next week.

May be in module documentation put some strings about that behavior?
@specator If you're 100% sure this fixes the issue you have been seeing, I will add a note to **win_updates** with some information.
@specator I have experienced similar issue with 2.7.1, i can confirm that set WinRm to delayed start does help with this specific issue.

Set reboot_timeout or wait_for_connection does not help.

Now that with winrm quickconfig, it will chanage WinRM to delayed start up by default. But with  [ConfigureRemotingForAnsible.ps1](https://github.com/ansible/ansible/blob/devel/examples/scripts/ConfigureRemotingForAnsible.ps1), WinRM is set to auto.

Can we consider change ConfigureRemotingForAnsible.ps1, so it will set WinRM to delayed start up instead?

Commit message: reboot - Fix connection timeout reset

Patch to review:
diff --git a/changelogs/fragments/reboot-conn-timeout-reset.yaml b/changelogs/fragments/reboot-conn-timeout-reset.yaml
new file mode 100644
index 00000000000000..c6427111debfb5
--- /dev/null
+++ b/changelogs/fragments/reboot-conn-timeout-reset.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+- reboot - Fix bug where the connection timeout was not reset in the same task after rebooting
diff --git a/lib/ansible/plugins/action/reboot.py b/lib/ansible/plugins/action/reboot.py
index 6fa2700629822b..ebee0b044217d8 100644
--- a/lib/ansible/plugins/action/reboot.py
+++ b/lib/ansible/plugins/action/reboot.py
@@ -296,7 +296,6 @@ def validate_reboot(self, distribution, original_connection_timeout=None, action
         try:
             # keep on checking system boot_time with short connection responses
             reboot_timeout = int(self._task.args.get('reboot_timeout', self._task.args.get('reboot_timeout_sec', self.DEFAULT_REBOOT_TIMEOUT)))
-            connect_timeout = self._task.args.get('connect_timeout', self._task.args.get('connect_timeout_sec', self.DEFAULT_CONNECT_TIMEOUT))
 
             self.do_until_success_or_timeout(
                 action=self.check_boot_time,
@@ -305,7 +304,13 @@ def validate_reboot(self, distribution, original_connection_timeout=None, action
                 distribution=distribution,
                 action_kwargs=action_kwargs)
 
-            if connect_timeout and original_connection_timeout:
+            # Get the connect_timeout set on the connection to compare to the original
+            try:
+                connect_timeout = self._connection.get_option('connection_timeout')
+            except AnsibleError:
+                pass
+
+            if original_connection_timeout != connect_timeout:
                 try:
                     display.debug("{action}: setting connect_timeout back to original value of {value}".format(
                         action=self._task.action,


Code review comments:
diff_hunk: @@ -305,7 +304,13 @@ def validate_reboot(self, distribution, original_connection_timeout=None, action
                 distribution=distribution,
                 action_kwargs=action_kwargs)
 
-            if connect_timeout and original_connection_timeout:
+            # Get the connect_timeout set on the connection to compare to the original
+            try:
+                connect_timeout = self._connection.get_option('connection_timeout')
+            except AnsibleError:
+                pass
+
+            if original_connection_timeout != connect_timeout:
path: lib/ansible/plugins/action/reboot.py
text: @user1:
Isn't it possible for connect_timeout to be undefined now?

@author:
That's true, will fix that up