Bugfix/8877/Fixes deletion of src on encoding error (#321)

* Fixed deletion of src on encoding error

The code failed while trying to copy files from a directory that needs to be converted, dirs_exist_ok=True should fix that. Additionally, I added two checks to always make sure that we don't accidentally delete source files when they're not temporary.

* Added functional test for copying dir to PDS

* Workaround for failing cleanup test in zos_copy

Quick workaround for a failing test in zos_copy. The test has been fixed more permanently in v1.3.3 of the collection.

* Updated directory encoding documentation

Co-authored-by: ddimatos <[email protected]>

changelogs/fragments/318-sdsf-dependency-removed.yml

@@ -3,5 +3,5 @@ major_changes:
     ibm_zos_core - Updates the entire collection in that the collection
     no longer depends on the managed node having
     installed System Display and Search Facility (SDSF).
-    "Remove SDSF dependency from ibm_zos_core collection.
+    Remove SDSF dependency from ibm_zos_core collection.
     (https://github.com/ansible-collections/ibm_zos_core/pull/303).

changelogs/fragments/321-fixes-deleting-src-on-encoding-error.yml

@@ -0,0 +1,7 @@
+bugfixes:
+  - >
+    ibm_zos_copy - Fixes a bug such that the module fails when copying
+    files from a directory needing also to be encoded. The failure would also
+    delete the `src` which was not desirable behavior.
+    Fixes deletion of src on encoding error.
+    (https://github.com/ansible-collections/ibm_zos_core/pull/303).

plugins/modules/zos_copy.py

@@ -106,6 +106,8 @@
         data and not binary data.
       - If C(encoding) is provided and C(src) is an MVS data set, task will fail.
       - Only valid if C(is_binary) is false.
+      - If C(encoding) is provided and C(src) is a directory, the encoding
+        conversion will be applied to all files.
     type: dict
     required: false
     suboptions:
@@ -842,13 +844,15 @@ def convert_encoding(self, src, temp_path, encoding):
             try:
                 if not temp_path:
                     temp_dir = tempfile.mkdtemp()
-                    shutil.copytree(new_src, temp_dir)
+                    shutil.copytree(new_src, temp_dir, dirs_exist_ok=True)
                     new_src = temp_dir
+
                 self._convert_encoding_dir(new_src, from_code_set, to_code_set)
                 self._tag_file_encoding(new_src, to_code_set, is_dir=True)
 
             except Exception as err:
-                shutil.rmtree(new_src)
+                if new_src != src:
+                    shutil.rmtree(new_src)
                 self.fail_json(msg=str(err))
         else:
             try:
@@ -873,7 +877,8 @@ def convert_encoding(self, src, temp_path, encoding):
                 self._tag_file_encoding(new_src, to_code_set)
 
             except Exception as err:
-                os.remove(new_src)
+                if new_src != src:
+                    os.remove(new_src)
                 self.fail_json(msg=str(err))
         return new_src
 

tests/functional/modules/test_zos_copy_func.py

@@ -1511,47 +1511,62 @@ def test_copy_local_symlink_to_uss_file(ansible_zos_module):
 
 def test_copy_local_file_to_uss_file_convert_encoding(ansible_zos_module):
     hosts = ansible_zos_module
+    src_path = "/etc/profile"
     dest_path = "/tmp/profile"
+
     try:
         hosts.all.file(path=dest_path, state="absent")
         copy_res = hosts.all.zos_copy(
-            src="/etc/profile",
+            src=src_path,
             dest=dest_path,
             encoding={"from": "ISO8859-1", "to": "IBM-1047"},
         )
-        stat_res = hosts.all.stat(path=dest_path)
+
+        dest_stat_res = hosts.all.stat(path=dest_path)
+        src_stat_res = hosts.all.stat(path=dest_path)
+
         for result in copy_res.contacted.values():
             assert result.get("msg") is None
-        for result in stat_res.contacted.values():
+        for result in dest_stat_res.contacted.values():
+            assert result.get("stat").get("exists") is True
+        for result in src_stat_res.contacted.values():
             assert result.get("stat").get("exists") is True
     finally:
         hosts.all.file(path=dest_path, state="absent")
 
 
 def test_copy_uss_file_to_uss_file_convert_encoding(ansible_zos_module):
     hosts = ansible_zos_module
+    src_path = "/etc/profile"
     dest_path = "/tmp/profile"
+
     try:
         hosts.all.file(path=dest_path, state="absent")
         copy_res = hosts.all.zos_copy(
-            src="/etc/profile",
+            src=src_path,
             dest=dest_path,
             encoding={"from": "IBM-1047", "to": "IBM-1047"},
             remote_src=True,
         )
-        stat_res = hosts.all.stat(path=dest_path)
+
+        dest_stat_res = hosts.all.stat(path=dest_path)
+        src_stat_res = hosts.all.stat(path=dest_path)
+
         for result in copy_res.contacted.values():
             assert result.get("msg") is None
-        for result in stat_res.contacted.values():
+        for result in dest_stat_res.contacted.values():
+            assert result.get("stat").get("exists") is True
+        for result in src_stat_res.contacted.values():
             assert result.get("stat").get("exists") is True
     finally:
         hosts.all.file(path=dest_path, state="absent")
 
 
 def test_copy_uss_file_to_pds_member_convert_encoding(ansible_zos_module):
     hosts = ansible_zos_module
-    src = "/etc/profile"
+    src_path = "/etc/profile"
     dest_path = "USER.TEST.PDS.FUNCTEST"
+
     try:
         hosts.all.zos_data_set(
             type="pds",
@@ -1561,7 +1576,7 @@ def test_copy_uss_file_to_pds_member_convert_encoding(ansible_zos_module):
             record_length=25,
         )
         copy_res = hosts.all.zos_copy(
-            src=src,
+            src=src_path,
             dest=dest_path,
             remote_src=True,
             encoding={"from": "IBM-1047", "to": "IBM-1047"},
@@ -1570,15 +1585,57 @@ def test_copy_uss_file_to_pds_member_convert_encoding(ansible_zos_module):
             cmd="head \"//'{0}'\"".format(dest_path + "(PROFILE)"),
             executable=SHELL_EXECUTABLE,
         )
+        stat_res = hosts.all.stat(path=src_path)
+
         for result in copy_res.contacted.values():
             assert result.get("msg") is None
         for result in verify_copy.contacted.values():
             assert result.get("rc") == 0
             assert result.get("stdout") != ""
+        for result in stat_res.contacted.values():
+            assert result.get("stat").get("exists") is True
     finally:
         hosts.all.zos_data_set(name=dest_path, state="absent")
 
 
+def test_copy_uss_dir_to_pds_convert_encoding(ansible_zos_module):
+    hosts = ansible_zos_module
+    profile_path = "/etc/profile"
+    src_path = "/tmp/zos_copy_test/"
+    dest_path = "USER.TEST.PDS.FUNCTEST"
+
+    try:
+        hosts.all.file(path=src_path, state="directory")
+        hosts.all.zos_copy(
+            src=profile_path,
+            dest=src_path,
+            remote_src=True
+        )
+
+        copy_res = hosts.all.zos_copy(
+            src=src_path,
+            dest=dest_path,
+            remote_src=True,
+            encoding={"from": "IBM-1047", "to": "IBM-1047"},
+        )
+        verify_copy = hosts.all.shell(
+            cmd="head \"//'{0}'\"".format(dest_path + "(PROFILE)"),
+            executable=SHELL_EXECUTABLE,
+        )
+        stat_res = hosts.all.stat(path=src_path)
+
+        for result in copy_res.contacted.values():
+            assert result.get("msg") is None
+        for result in verify_copy.contacted.values():
+            assert result.get("rc") == 0
+            assert result.get("stdout") != ""
+        for result in stat_res.contacted.values():
+            assert result.get("stat").get("exists") is True
+    finally:
+        hosts.all.file(path=src_path, state="absent")
+        hosts.all.zos_data_set(name=dest_path, state="absent")
+
+
 def test_ensure_tmp_cleanup(ansible_zos_module):
     hosts = ansible_zos_module
     file_name = "profile"
@@ -1598,6 +1655,7 @@ def test_ensure_tmp_cleanup(ansible_zos_module):
         stat_dir = hosts.all.shell(
             cmd="ls", executable=SHELL_EXECUTABLE, chdir=dest
         )
+
         stat_dir_list = list(stat_dir.contacted.values())[0].get("stdout_lines")
         file_count_post = len(stat_dir_list)