search

pyinfra-dev / pyinfra

pyinfra turns Python code into shell commands and runs them on your servers. Execute ad-hoc commands and write declarative operations. Target SSH servers, local machine and Docker containers. Fast and scales from one server to thousands.
https://pyinfra.com
MIT License
3.91k stars 382 forks source link

files.put to a readonly filesystem does not fail in a clear way #878

Closed drewp closed 1 year ago

drewp commented 2 years ago

The error for trying to files.put to a readonly filesystem is confusing, and actions continue afterwards in a surprising way:

(on host slash, / is mounted ro for some reason)

--> Beginning operation run...
--> Starting operation: Files/Put (dest=/usr/local/bin/k3s-killall.sh, src=files/kube/k3s-killall.sh, mode=a+rx)
    [prime] Skipped
    [dot] Skipped
    [garage] Skipped
    [bang] Skipped
    [pipe] Skipped
[dash] file uploaded: /usr/local/bin/k3s-killall.sh
[dash] >>> sh -c 'chmod a+rx /usr/local/bin/k3s-killall.sh'
    Failed to upload file, retrying: Failure
[slash] file uploaded: /usr/local/bin/k3s-killall.sh
[slash] >>> sh -c 'chmod a+rx /usr/local/bin/k3s-killall.sh'
    [dash] Success
[slash] chmod: cannot access '/usr/local/bin/k3s-killall.sh': No such file or directory
    [slash] Error

I did not expect things to continue after 'failed to upload file'.

The ideal message here would be like "[slash] /usr/local/bin is on a readonly filesystem".

Fizzadar commented 2 years ago

Interesting - looks like the result of the upload function was a success despite the error! Thank you for raising this I’ll get a repro working and fix.

Fizzadar commented 2 years ago

Hm, finally testing this but it seems to be working correctly, firstly created a container with a read only dir:

docker run -it -v `pwd`:/opt/pyinfra:ro ubuntu

Then tried to use files.put on it:

pyinfra @docker/64008a0b1685 files.put setup.py /opt/pyinfra/file

--> Loading config...

--> Loading inventory...

--> Connecting to hosts...
    [@docker/64008a0b1685] Connected

--> Preparing operation...
    [@docker/64008a0b1685] Ready: files.put

--> Proposed changes:
    Groups: @docker
    [@docker/64008a0b1685]   Operations: 1   Change: 1   No change: 0

--> Beginning operation run...
--> Starting operation: Files/Put (setup.py, /opt/pyinfra/file)
    [@docker/64008a0b1685] Command socket/SSH error: OSError('Error response from daemon: mounted volume is marked read-only',)
    [@docker/64008a0b1685] Error: executed 0/1 commands
    [@docker/64008a0b1685] docker build complete, container left running: 64008a0b1685
--> pyinfra error: No hosts remaining!

The upload code does re-attempt uploads 3 times, in the above it looks like it failed once and subsequently claimed to work, even though the file clearly wasn't there:

https://github.com/Fizzadar/pyinfra/blob/7cc0e02149106d561df3f6acba390e20b7f7c1c5/pyinfra/connectors/ssh.py#L472-L483

Even more concerning is paramiko appears to do a stat check on the file after uploading by default (https://docs.paramiko.org/en/stable/api/sftp.html#paramiko.sftp_client.SFTPClient.putfo).

I wonder if this is limited to situations where / is read-only... I need to use a test VM/server to do that which I currently cannot access unfortunately but when I can I will try it out.

Fizzadar commented 1 year ago

v2.6.1 should fix the false positive success here, my bad for not spotting this before! More @ https://github.com/Fizzadar/pyinfra/issues/936#issuecomment-1368192388