Inhibit chmod

[manschwetusCF]

SUMMARY

Added the inhibit_copystat flag to copy to allow copy to succeed on platforms where stat values cannot be copied from src to dest.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

copy

ADDITIONAL INFORMATION

When running playbooks on HPE NonStop platforms, the shutil.copystat operation does not succeed
when copying files between legacy (GUARDIAN) and current file systems (OSS/POSIX). The stat values are incompatible and does not work of the dest file is in the always-three-level GUARDIAN space
(e.g., /G/disk/dir/file). This option will allow the copy operation to succeed. The author considered adding
to the except clause, but felt that the decision to enable this behaviour in general is not desirable, and should by by deliberate choice of the user of the copy module on platforms where this situation applies.

This is originally work of @rsbeckerca, see #51103

before

$ ansible-playbook testCopy.yml
[WARNING]: You are running the development version of Ansible. You should only run Ansible from "devel" if you are modifying the Ansible engine, or trying out features under development. This is a rapidly changing source of code and can become unstable at any point.

PLAY [cs5] ****************************************************************************************************************************

TASK [Gathering Facts] ***************************************************************************************************************
ok: [cs5]

TASK [copyTest : Copy file oss to oss] ************************************************************************************************
changed: [cs5]

TASK [copyTest : Copy file oss to guardian] ******************************************************************************************
fatal: [cs5]: FAILED! => {"changed": false, "msg": "failed to copy: /home/test/test0cat to /G/data01/anscpyt/test0cat", "traceback": "Traceback (most recent call last):\n  File \"/tmp/ansible_ansible.legacy.copy_payload_gxz2_3q3/ansible_ansible.legacy.copy_payload.zip/ansible/modules/copy.py\", line 685, in main\n  File \"/usr/lib/python3.6/shutil.py\", line 203, in copystat\n    lookup(\"chmod\")(dst, mode, follow_symlinks=follow)\nOSError: [Errno 4022] Invalid function argument: b'/G/data01/anscpyt/tmpstorzhk2'\n"}

PLAY RECAP **************************************************************************************************************************
cs5                        : ok=2    changed=1    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0 

after

$ ansible-playbook testCopy.yml
[WARNING]: You are running the development version of Ansible. You should only run Ansible from "devel" if you are modifying the Ansible engine, or trying out features under development. This is a rapidly changing source of code and can become unstable at any point.

PLAY [cs5] ****************************************************************************************************************************

TASK [Gathering Facts] ***************************************************************************************************************
ok: [cs5]

TASK [copyTest : Copy file oss to oss] ************************************************************************************************
changed: [cs5]

TASK [copyTest : Copy file oss to guardian] ******************************************************************************************
changed: [cs5]

TASK [copyTest : Copy file guardian to guardian] ************************************************************************************
changed: [cs5]

TASK [copyTest : Copy file guardian to oss] ******************************************************************************************
changed: [cs5]

TASK [copyTest : Remove file (delete file) oss 1/2] ************************************************************************************
changed: [cs5]

TASK [copyTest : Remove file (delete file) oss 2/2] ************************************************************************************
changed: [cs5]

TASK [copyTest : Remove file (delete file) guardian 1/2] ******************************************************************************
changed: [cs5]

TASK [copyTest : Remove file (delete file) guardian 2/2] ******************************************************************************
changed: [cs5]

PLAY RECAP **************************************************************************************************************************
cs5                        : ok=9    changed=8    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

[rsbeckerca]

This has my support

[samdoran]

Looking at this a bit more, it seems like adding a new option is probably not the best fix. We should detect and handle the failure of copystat by setting the appropriate mode by other means.

[rsbeckerca]

Looking at this a bit more, it seems like adding a new option is probably not the best fix. We should detect and handle the failure of copystat by setting the appropriate mode by other means.

As you say, but I'm not sure how one can detect this. There is no standardization of error codes or messages, other than an unhelpful position of approximating it may have worked from the OS. One could check the idempotency of the operation by checking that the target file exists, has an expected mode, and timestamp, but there is no true indication of whether the copy actually worked without comparing the contents for equality, which itself is not always going to be possible if read access is not permitted. The only relative certainty would come from a SHA-256 signature comparison of source/destination files.

[manschwetusCF]

Hm if read access is not permitted then the hash comparison won’t work, too. But regarding the present use case, I’m not sure if there is any reliable way to check this files the unix way.

…

-- Mit freundlichen Grüßen / With kind regards Florian Manschwetus E-Mail: ***@***.******@***.***> Tel.: +49-(0)611-8908534 CS Software Concepts and Solutions GmbH Geschäftsführer / Managing Director: Falk Dreßler, Manfred Brandner Amtsgericht Wiesbaden HRB 28619 (Commercial registry) Schiersteiner Straße 31 D-65187 Wiesbaden Germany Tel.: 0611/8908555 Von: Randall S. Becker ***@***.***> Gesendet: Dienstag, 20. April 2021 17:57 An: ansible/ansible ***@***.***> Cc: Florian Manschwetus ***@***.***>; Author ***@***.***> Betreff: Re: [ansible/ansible] Inhibit chmod (#74342) Looking at this a bit more, it seems like adding a new option is probably not the best fix. We should detect and handle the failure of copystat by setting the appropriate mode by other means. As you say, but I'm not sure how you can detect this. There is no standardization of error codes or messages, other than an unhelpful position of approximating it may have worked from the OS. You could check the idempotency of the operation by checking that the target file exists and has an expected mode, but you have no true indication of whether the copy actually worked without comparing the contents for equality, which itself is not always going to be possible if read access is not permitted. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#74342 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHKNGURJKW4QUMKQRQFPL7DTJWP33ANCNFSM43ICHZOA>.

[manschwetusCF]

Lets approach on this from a use case perspective, there are various corner cases, where this generic copy operation may not fit perfectly, especially the copystat facet. As the copy module is a generic approach, I won't expect it to automatically detect and handle such more or less exotic cases.
So I would prefer it to fail clearly and to allow to intentionally disable things like copystat, so it is clear what you get and what not, so you can bring more specialized tooling into play if needed.

Also I stumbled over an additional misfit with the copy operation in the Guardian scope, which works by accident.
It uses tempfile.mkstemp(dir=os.path.dirname(b_dest)) to generate a temporary file name in target location, but the generated filename is illegal, as it is longer than eight characters. It works beside this, as the OS API truncates filenames to eight characters, what may end in unexpected effects. Additionally some interfaces have a magic syntax, to control aspects like file code and afaik mode, but not sure if this would work here, but due to this syntax being very exotic for posix filenames, I don't expect that such magic names are ever created by mkstemp(...).

[rsbeckerca]

The other related PR was closed because it went stale, not because it was not valuable. This PR will allow operations to work and I do not see another useful suggestion of an alternative implementation.

[rsbeckerca]

@manschwetusCS were you able to get any traction, or is Ansible just not wanting this?

[bcoca]

i would still not make this an option, but try to capture the copystat error on hp/ux and issue the warning as we do for enosys case

[rsbeckerca]

i would still not make this an option, but try to capture the copystat error on hp/ux and issue the warning as we do for enosys case

This is not HP/UX, but HPE NonStop. There is no specific error code for this situation and the error message varies depending on circumstance - I think LANG might be involved too, so parsing the stderr output may be impractical. The general error - not specific to this situation - is Invalid function argument, which can result from many causes but is part of the message here of significance at least with the cp command, attempting to copy k from one directory to another in the edge condition described in this PR:

cp: preserving permissions for '/G/data01/randall/k': Invalid function argument

[bcoca]

LANG should not be involved as we are making a system call, so answers should always be in POSIX/C locale, even if we were using the command line tools we do have the ability to force the locale/lang setting to enable parsing the output.

[jborean93]

This is still something we don't want to have hidden behind an extra option but rather implicit fallback behaviour in the copy module. It should try its best to capture the error and expose a warning if the copystat fails on the filesystem.

As for the question around how to handle the errors as the message is not consistent, wouldn't catching OSError and using errno.SOMETHING be able to catch the specific error that is at fault here.

[rsbeckerca]

This is still something we don't want to have hidden behind an extra option but rather implicit fallback behaviour in the copy module. It should try its best to capture the error and expose a warning if the copystat fails on the filesystem.

As for the question around how to handle the errors as the message is not consistent, wouldn't catching OSError and using errno.SOMETHING be able to catch the specific error that is at fault here.

The problem is that the copystat is report different errors in different situations. ENOENT and EINVAL are both reported depending on the file being copied. This is burying the underlying issue. If there was an errno that made any sense here, I would have contributed that fix.

[jborean93]

ENOENT and EINVAL are both reported

In what scenarios are these different values, is it on different platforms, different paths, etc?