[stable-2.9] file - change _diff_peek type in argument_spec (#60428) (#66587)

* Add integration tests
* Handle error in _get_diff_data()
* Change to warning rather than error
* Also change failure to warning in assemble action plugin
(cherry picked from commit 9b7198d25e)

Co-authored-by: Sam Doran <sdoran@redhat.com>

changelogs/fragments/file-fix-diff-peek-arg-spec.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+  - file - change ``_diff_peek`` in argument spec to be the correct type, which is ``bool`` (https://github.com/ansible/ansible/issues/59433)

lib/ansible/modules/files/file.py

@@ -886,7 +886,7 @@ def main():
             recurse=dict(type='bool', default=False),
             force=dict(type='bool', default=False),  # Note: Should not be in file_common_args in future
             follow=dict(type='bool', default=True),  # Note: Different default than file_common_args
-            _diff_peek=dict(type='str'),  # Internal use only, for internal checks in the action plugins
+            _diff_peek=dict(type='bool'),  # Internal use only, for internal checks in the action plugins
             src=dict(type='path'),  # Note: Should not be in file_common_args in future
             modification_time=dict(type='str'),
             modification_time_format=dict(type='str', default='%Y%m%d%H%M.%S'),

lib/ansible/plugins/action/__init__.py

@@ -1110,7 +1110,11 @@ class ActionBase(with_metaclass(ABCMeta, object)):
         display.debug("Going to peek to see if file has changed permissions")
         peek_result = self._execute_module(module_name='file', module_args=dict(path=destination, _diff_peek=True), task_vars=task_vars, persist_files=True)
 
-        if not peek_result.get('failed', False) or peek_result.get('rc', 0) == 0:
+        if peek_result.get('failed', False):
+            display.warning(u"Failed to get diff between '%s' and '%s': %s" % (os.path.basename(source), destination, to_text(peek_result.get(u'msg', u''))))
+            return diff
+
+        if peek_result.get('rc', 0) == 0:
 
             if peek_result.get('state') in (None, 'absent'):
                 diff['before'] = u''

test/integration/targets/file/tasks/diff_peek.yml

@@ -0,0 +1,10 @@
+- name: Run task with _diff_peek
+  file:
+    path: "{{ output_file }}"
+    _diff_peek: yes
+  register: diff_peek_result
+
+- name: Ensure warning was not issued when using _diff_peek parameter
+  assert:
+    that:
+      - diff_peek_result['warnings'] is not defined

test/integration/targets/file/tasks/main.yml

@@ -25,25 +25,29 @@
 - set_fact:
     remote_file_expanded: '{{ echo.stdout }}'
 
-# Include the tests
+# Import the test tasks
 - name: Run tests for state=link
-  include: state_link.yml
+  import_tasks: state_link.yml
 
 - name: Run tests for directory as dest
-  include: directory_as_dest.yml
+  import_tasks: directory_as_dest.yml
 
 - name: Run tests for unicode
-  include: unicode_path.yml
+  import_tasks: unicode_path.yml
   environment:
     LC_ALL: C
     LANG: C
 
 - name: decide to include or not include selinux tests
-  include: selinux_tests.yml
+  include_tasks: selinux_tests.yml
   when: selinux_installed is defined and selinux_installed.stdout != "" and selinux_enabled.stdout != "Disabled"
 
 - name: Initialize the test output dir
-  include: initialize.yml
+  import_tasks: initialize.yml
+
+- name: Test _diff_peek
+  import_tasks: diff_peek.yml
+
 
 # These tests need to be organized by state parameter into separate files later