win_updates - handle deleted temp profile (#422)

* win_updates - handle deleted temp profile

* Move code into separate function

changelogs/fragments/win_updates-tmp-profile.yml

@@ -0,0 +1,2 @@
+bugfixes:
+- win_updates - Handle running with a temp profile path that is deleted between reboots - https://github.com/ansible-collections/ansible.windows/issues/417

plugins/action/win_updates.py

@@ -682,6 +682,10 @@ def __init__(self, msg, **result):
         self.result = result
 
 
+class _RecreateTempPathException(Exception):
+    pass
+
+
 class ActionModule(ActionBase):
 
     _VALID_ARGS = [
@@ -750,12 +754,7 @@ def run(self, tmp=None, task_vars=None):
             )
 
         else:
-            if not self._connection._shell.tmpdir:
-                self._make_tmp_path()  # Stores the update scheduled task script/progress
-
             module_options['_wait'] = False
-            # In case we are running with become we need to make sure the module uses the correct dir
-            module_options['_output_path'] = self._connection._shell.tmpdir
 
             try:
                 result = self._run_sync(task_vars, module_options, reboot, reboot_timeout)
@@ -818,8 +817,8 @@ def run(self, tmp=None, task_vars=None):
 
     def _run_sync(self, task_vars, module_options, reboot, reboot_timeout):  # type: (Dict, Dict, bool, int) -> Dict
         """Installs the updates in a synchronous fashion with multiple update invocations if needed."""
-        poll_script_path = self._copy_script(_POLL_SCRIPT, 'poll.ps1')
-        cancel_script_path = self._copy_script(_CANCEL_SCRIPT, 'cancel.ps1')
+        # In case we are running with become we need to make sure the module uses the correct dir
+        module_options['_output_path'], poll_script_path, cancel_script_path = self._setup_updates_tmpdir()
 
         result = {
             'changed': False,
@@ -831,7 +830,14 @@ def _run_sync(self, task_vars, module_options, reboot, reboot_timeout):  # type:
             round += 1
             display.v("Running win_updates - round %s" % round, host=task_vars.get('inventory_hostname', None))
 
-            update_result = self._run_updates(task_vars, module_options, poll_script_path, cancel_script_path)
+            try:
+                update_result = self._run_updates(task_vars, module_options, poll_script_path, cancel_script_path)
+            except _RecreateTempPathException:
+                display.vv("Failure when running win_updates module with existing tempdir, retrying with new dir")
+                self._connection._shell.tmpdir = None
+                module_options['_output_path'], poll_script_path, cancel_script_path = self._setup_updates_tmpdir()
+
+                continue
 
             self._updates.update(update_result.updates)
             self._filtered_updates.update(update_result.filtered_updates)
@@ -898,6 +904,16 @@ def _run_sync(self, task_vars, module_options, reboot, reboot_timeout):  # type:
 
         return result
 
+    def _setup_updates_tmpdir(self):
+        """Sets up a remote tmpdir if needed and copies the files used by the action plugin."""
+        if not self._connection._shell.tmpdir:
+            self._make_tmp_path()  # Stores the update scheduled task script/progress
+
+        poll_script_path = self._copy_script(_POLL_SCRIPT, 'poll.ps1')
+        cancel_script_path = self._copy_script(_CANCEL_SCRIPT, 'cancel.ps1')
+
+        return self._connection._shell.tmpdir, poll_script_path, cancel_script_path
+
     def _run_updates(self, task_vars, module_options, poll_script_path, cancel_script_path):
         # type: (Dict, Dict, str, str) -> UpdateResult
         """Runs the win_updates module and returns the raw results from that task."""
@@ -937,12 +953,19 @@ def _start_updates(self, task_vars, module_options):  # type: (Dict, Dict) -> Tu
             module_args=module_options,
             task_vars=task_vars,
         )
+
         if 'invocation' in result and not self._invocation:
             # First run through we want to update the invocation value in the final results
             self._invocation = result['invocation']
 
+        failed = result.get('failed', False)
+        if failed and result.get('recreate_tmpdir', False):
+            # Might have been deleted across a reboot, try to recreate for the next run.
+            # https://github.com/ansible-collections/ansible.windows/issues/417
+            raise _RecreateTempPathException()
+
         if (
-            result.get('failed', False) or
+            failed or
             'output_path' not in result or
             'task_pid' not in result or
             'cancel_id' not in result

plugins/modules/win_updates.ps1

@@ -1530,6 +1530,15 @@ if (-not $outputPathDir) {
     # Running async means this won't be set, just use the module tmpdir.
     $outputPathDir = $module.Tmpdir
 }
+elseif (-not (Test-Path -LiteralPath $outputPathDir)) {
+    # If the _output_path is set but does not exist then it needs to be
+    # re-created by the action plugin and the module rerun. The recreate_tmpdir
+    # return value is used to flag this special failure from one to pass back
+    # to Ansible.
+    # https://github.com/ansible-collections/ansible.windows/issues/417
+    $module.Result.recreate_tmpdir = $true
+    $module.FailJson("Module tmpdir '$outputPathDir' does not exist")
+}
 
 # The scheduled task might need to fallback to run as SYSTEM so grant that SID rights to OutputDir
 $systemSid = (New-Object -TypeName Security.Principal.SecurityIdentifier -ArgumentList @(

tests/unit/plugins/action/test_win_updates.py

@@ -547,3 +547,70 @@ def test_fail_install(monkeypatch):
         else:
             assert u['installed']
             assert 'failure_hresult_code' not in u
+
+
+def test_reboot_with_tmpdir_cleanup(monkeypatch):
+    reboot_mock = MagicMock()
+    reboot_mock.return_value = {'failed': False}
+    monkeypatch.setattr(win_updates, 'reboot_host', reboot_mock)
+
+    mock_connection = mock_connection_init('reboot_with_tmpdir_cleanup.txt')
+
+    module_arg_return = {
+        'category_names': '*',
+        'state': 'installed',
+        'reboot': 'yes',
+    }
+    plugin = win_updates_init(module_arg_return, connection=mock_connection)
+    execute_module = MagicMock()
+    execute_module.side_effect = (
+        {
+            'invocation': {'module_args': module_arg_return},
+            'output_path': 'update_output_path',
+            'task_pid': 666,
+            'cancel_id': 'update_cancel_id',
+        },
+        {
+            'invocation': {'module_args': module_arg_return},
+            'failed': True,
+            'msg': 'Module tmpdir ''...'' does not exist"',
+            'recreate_tmpdir': True,
+        },
+        {
+            'invocation': {'module_args': module_arg_return},
+            'output_path': 'update_output_path',
+            'task_pid': 666,
+            'cancel_id': 'update_cancel_id',
+        },
+    )
+    monkeypatch.setattr(plugin, '_execute_module', execute_module)
+    monkeypatch.setattr(plugin, '_transfer_file', MagicMock())
+
+    def test_make_tmp_path():
+        mock_connection._shell.tmpdir = 'shell_tmpdir_2'
+
+    monkeypatch.setattr(plugin, '_make_tmp_path', test_make_tmp_path)
+
+    actual = plugin.run()
+
+    assert reboot_mock.call_count == 1
+    assert mock_connection._shell.tmpdir == 'shell_tmpdir_2'
+    assert execute_module.call_count == 3
+
+    assert actual['changed']
+    assert not actual['reboot_required']
+    assert actual['found_update_count'] == 6
+    assert actual['failed_update_count'] == 0
+    assert actual['installed_update_count'] == 6
+    assert actual['filtered_updates'] == {}
+    assert len(actual['updates']) == 6
+
+    for u_id, u in actual['updates'].items():
+        assert u['id'] == u_id
+        assert u['id'] in UPDATE_INFO
+        u_info = UPDATE_INFO[u['id']]
+        assert u['title'] == u_info['title']
+        assert u['kb'] == [u_info['kb']]
+        assert u['categories'] == u_info['categories']
+        assert u['downloaded']
+        assert u['installed']