Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

"fetch" action stalls on local I/O problems #11612

Closed
youam opened this issue Jul 16, 2015 · 1 comment
Closed

"fetch" action stalls on local I/O problems #11612

youam opened this issue Jul 16, 2015 · 1 comment
Labels
bug This issue/PR relates to a bug.

Comments

@youam
Copy link
Contributor

youam commented Jul 16, 2015

Issue Type:

Bug Report

Ansible Version:
ansible 2.0.0 (devel ba7243c5f9) last updated 2015/07/16 13:07:32 (GMT +200)
  lib/ansible/modules/core: (detached HEAD 291fef3b34) last updated 2015/07/16 15:52:49 (GMT +200)
  lib/ansible/modules/extras: (detached HEAD ff2386faf4) last updated 2015/07/16 15:52:49 (GMT +200)
  v1/ansible/modules/core: (detached HEAD f8d8af17cd) last updated 2015/07/16 13:39:35 (GMT +200)
  v1/ansible/modules/extras: (detached HEAD 495ad450e5) last updated 2015/07/16 13:39:43 (GMT +200)
  configured module search path = None
Ansible Configuration:

nothing important changed from the upstream example, only the defaults.inventory and defaults.remote_user settings.

Environment

N/A (Debian Jessie on amd64)

Summary:

When fetching a file from the remote host which can not be written locally, the write fails silently.

Steps To Reproduce:

Below, I just execute the same fetch command multiple times:

youam@aurnia:...e-staging (git)-[master]-$ ansible majella.youam.de -m fetch -a 'dest=test src=/etc/passwd flat=yes' -v
Using /home/youam/work/projects/ansible-staging/ansible.cfg as config file
majella.youam.de | SUCCESS => {
    "changed": true, 
    "checksum": "0f000bb0b80828f38a5376968c6dc6782f5352a3", 
    "dest": "/home/youam/work/projects/ansible-staging/test", 
    "md5sum": "bd653c27a04f04512f7b90c734de04e5", 
    "remote_checksum": "0f000bb0b80828f38a5376968c6dc6782f5352a3", 
    "remote_md5sum": null
}

the first call fetches the file, and has the status "changed"

youam@aurnia:...e-staging (git)-[master]-$ ansible majella.youam.de -m fetch -a 'dest=test src=/etc/passwd flat=yes'
majella.youam.de | SUCCESS => {
    "changed": false, 
    "checksum": "0f000bb0b80828f38a5376968c6dc6782f5352a3", 
    "dest": "/home/youam/work/projects/ansible-staging/test", 
    "file": "/etc/passwd", 
    "md5sum": "bd653c27a04f04512f7b90c734de04e5"
}

the second call doesn't change anything, everything is well. now reproduce the problem: change the file locally and make it unwritable

youam@aurnia:...e-staging (git)-[master]-$ echo foo > test
youam@aurnia:...e-staging (git)-[master]-$ chmod -w test
youam@aurnia:...e-staging (git)-[master]-$ ansible majella.youam.de -m fetch -a 'dest=test src=/etc/passwd flat=yes'
majella.youam.de | SUCCESS => {
    "changed": true, 
    "checksum": "f1d2d2f924e986ac86fdf7b36c94bcdf32beec15", 
    "dest": "/home/youam/work/projects/ansible-staging/test", 
    "md5sum": "d3b07384d113edec49eaa6238ad5ff00", 
    "remote_checksum": "0f000bb0b80828f38a5376968c6dc6782f5352a3", 
    "remote_md5sum": null
}
youam@aurnia:...e-staging (git)-[master]-$ ansible majella.youam.de -m fetch -a 'dest=test src=/etc/passwd flat=yes'
majella.youam.de | SUCCESS => {
    "changed": true, 
    "checksum": "f1d2d2f924e986ac86fdf7b36c94bcdf32beec15", 
    "dest": "/home/youam/work/projects/ansible-staging/test", 
    "md5sum": "d3b07384d113edec49eaa6238ad5ff00", 
    "remote_checksum": "0f000bb0b80828f38a5376968c6dc6782f5352a3", 
    "remote_md5sum": null
}

ansible notices that the file doesn't have the correct output, tries to change it but does not recognise that updating the file fails. because of this, subsequent calls of the same action still return "changed" status - allthough nothing happened.

Expected Results:

On I/O errors, fail.

Actual Results:

ignores I/O error silently.

one could argue that using the "validate_checksum" flag makes ansible fail this action instead of silently skipping it. In my opinion, the validate_checksum flag should be used to ensure that the source file didn't change in the meantime and that a full, consistent file was fetched, but should not guard against programming errors in ansible itself.

Quick and Dirty Patch

The following patch makes ansible fail this action instead of silently skipping the problem. The status message could be improved...

diff --git a/lib/ansible/plugins/action/fetch.py b/lib/ansible/plugins/action/fetch.py
index fb2d545..35294a2 100644
--- a/lib/ansible/plugins/action/fetch.py
+++ b/lib/ansible/plugins/action/fetch.py
@@ -133,9 +133,12 @@ class ActionModule(ActionBase):
             if remote_data is None:
                 self._connection.fetch_file(source, dest)
             else:
-                f = open(dest, 'w')
-                f.write(remote_data)
-                f.close()
+                try:
+                    f = open(dest, 'w')
+                    f.write(remote_data)
+                    f.close()
+                except:
+                    return dict(failed=True, msg="file write error")
             new_checksum = secure_hash(dest)
             # For backwards compatibility.  We'll return None on FIPS enabled
             # systems
@jimi-c jimi-c closed this as completed in 7a99164 Jul 22, 2015
@jimi-c
Copy link
Member

jimi-c commented Jul 22, 2015

Closing This Ticket

Hi!

We believe the above commit should resolve this problem for you. This will also be included in the next major release.

If you continue seeing any problems related to this issue, or if you have any further questions, please let us know by stopping by one of the two mailing lists, as appropriate:

Because this project is very active, we're unlikely to see comments made on closed tickets, but the mailing list is a great way to ask questions, or post if you don't think this particular issue is resolved.

Thank you!

@jctanner jctanner mentioned this issue May 12, 2016
Merged
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bug_report labels Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug This issue/PR relates to a bug.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jimi-c @bcoca @youam @ansibot