From 24c50e79f7f4d1ec0d9d5b51d73049dccf41aca5 Mon Sep 17 00:00:00 2001 From: Ivan Moreno Date: Sun, 21 Aug 2022 22:50:08 -0700 Subject: [PATCH] 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 --- .../fragments/318-sdsf-dependency-removed.yml | 2 +- ...1-fixes-deleting-src-on-encoding-error.yml | 7 ++ plugins/modules/zos_copy.py | 11 ++- .../functional/modules/test_zos_copy_func.py | 74 +++++++++++++++++-- 4 files changed, 82 insertions(+), 12 deletions(-) create mode 100644 changelogs/fragments/321-fixes-deleting-src-on-encoding-error.yml diff --git a/changelogs/fragments/318-sdsf-dependency-removed.yml b/changelogs/fragments/318-sdsf-dependency-removed.yml index 2c4802154..8d06b3bac 100644 --- a/changelogs/fragments/318-sdsf-dependency-removed.yml +++ b/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). \ No newline at end of file diff --git a/changelogs/fragments/321-fixes-deleting-src-on-encoding-error.yml b/changelogs/fragments/321-fixes-deleting-src-on-encoding-error.yml new file mode 100644 index 000000000..65171adf8 --- /dev/null +++ b/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). \ No newline at end of file diff --git a/plugins/modules/zos_copy.py b/plugins/modules/zos_copy.py index a2af4c4d7..2b20813e9 100644 --- a/plugins/modules/zos_copy.py +++ b/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 diff --git a/tests/functional/modules/test_zos_copy_func.py b/tests/functional/modules/test_zos_copy_func.py index cb095491d..ff97d3bb7 100644 --- a/tests/functional/modules/test_zos_copy_func.py +++ b/tests/functional/modules/test_zos_copy_func.py @@ -1511,18 +1511,25 @@ 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") @@ -1530,19 +1537,26 @@ def test_copy_local_file_to_uss_file_convert_encoding(ansible_zos_module): 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") @@ -1550,8 +1564,9 @@ def test_copy_uss_file_to_uss_file_convert_encoding(ansible_zos_module): 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)