Handle SCP and SFTP writes correctly in native client by resuming from last written byte on EAGAIN - resolves #148

Updated documentation, readme. Added docstrings for parallel client scp functions.
Updated ssh2-python minimum version.
Updated C code with cython 0.29.
Updated travis cfg.

Author: Pan

.travis.yml

@@ -42,6 +42,33 @@ after_success:
 jobs:
   include:
 
+    - stage: build packages
+      env:
+        - WHEELS=1
+      os: linux
+      python: 3.6
+      before_install: skip
+      install:
+        - pip install twine
+      script:
+        - docker login -u="$DOCKER_USERNAME" -p="$DOCKER_PASSWORD" &&
+          ./ci/travis/build-manylinux.sh;
+      after_success:
+        - if [[ ! -z "$TRAVIS_TAG" ]]; then
+            twine upload --skip-existing -u $PYPI_U -p $PYPI_P wheelhouse/*.whl;
+          fi
+      deploy:
+        - provider: pypi
+          user: pkittenis
+          password:
+            secure: ZQJ41Nguc7Y5XHvtN8lITIiW1S1jvy0p50rssMUJpa9wVZIh0HcW0K/Xv2v17fDNuOvQlVbsF0sY/BmcRfH7c7nzwt7fRXGOjXbZk5djqwusKXL6zlVN7OKjAY6j2EByOjD9UpDDkB5tDqb4lRBCX87wknii/t+7/8P0ddoBojM=
+          on:
+            repo: ParallelSSH/parallel-ssh
+            tags: true
+          distributions: sdist
+          skip_upload_docs: true
+          skip_cleanup: true
+
     - &osx-wheels
       stage: build packages
       os: osx
@@ -50,10 +77,8 @@ jobs:
         - PYENV: 3.6.4
       before_install:
         - brew update
-        - brew outdated openssl || brew upgrade openssl || echo "y"
         - brew link --overwrite python@2 || brew install python@2 || brew link --overwrite python@2
         - which python2
-        - python2 -c "from __future__ import print_function; import ssl; from platform import python_version; print(ssl.OPENSSL_VERSION); print(python_version())"
         - sudo -H pip2 install twine
         - which twine
         - mkdir -p wheels
@@ -93,10 +118,10 @@ jobs:
         - ./ci/travis/pyenv-wheel.sh
 
     - <<: *osx-wheels
-      osx_image: xcode8.3
+      osx_image: xcode9.2
 
     - <<: *osx-wheels
-      osx_image: xcode9.3
+      osx_image: xcode9.4
 
     - <<: *osx-wheels
       osx_image: xcode8
@@ -107,15 +132,15 @@ jobs:
         - ./ci/travis/pyenv-wheel.sh
 
     - <<: *osx-wheels
-      osx_image: xcode8.3
+      osx_image: xcode9.2
       env:
         - PYENV: 3.7.0
       install: skip
       script:
         - ./ci/travis/pyenv-wheel.sh
 
     - <<: *osx-wheels
-      osx_image: xcode9.3
+      osx_image: xcode9.4
       env:
         - PYENV: 3.7.0
       install: skip
@@ -142,31 +167,3 @@ jobs:
           on:
             repo: ParallelSSH/parallel-ssh
             tags: true
-
-    - stage: build packages
-      env:
-        - WHEELS=1
-        - OPENSSL=openssl-1.0.2o
-      os: linux
-      python: 3.6
-      before_install: skip
-      install:
-        - pip install twine
-      script:
-        - docker login -u="$DOCKER_USERNAME" -p="$DOCKER_PASSWORD" &&
-          ./ci/travis/build-manylinux.sh;
-      after_success:
-        - if [[ ! -z "$TRAVIS_TAG" ]]; then
-            twine upload --skip-existing -u $PYPI_U -p $PYPI_P wheelhouse/*.whl;
-          fi
-      deploy:
-        - provider: pypi
-          user: pkittenis
-          password:
-            secure: ZQJ41Nguc7Y5XHvtN8lITIiW1S1jvy0p50rssMUJpa9wVZIh0HcW0K/Xv2v17fDNuOvQlVbsF0sY/BmcRfH7c7nzwt7fRXGOjXbZk5djqwusKXL6zlVN7OKjAY6j2EByOjD9UpDDkB5tDqb4lRBCX87wknii/t+7/8P0ddoBojM=
-          on:
-            repo: ParallelSSH/parallel-ssh
-            tags: true
-          distributions: sdist
-          skip_upload_docs: true
-          skip_cleanup: true

Changelog.rst

@@ -1,6 +1,19 @@
 Change Log
 ============
 
+
+1.9.1
+++++++
+
+Fixes
+-----
+
+* Native client SCP and SFTP uploads would not handle partial writes from waiting on socket correctly.
+* Native client ``copy_file`` SFTP upload would get stuck repeating same writes until killed when copying multi-MB files from Windows clients - #148
+* Native client ``scp_send`` would not correctly preserve file mask of local file on the remote.
+* Native client tunnel, used for proxy implementation, would not handle partial writes from waiting on socket correctly.
+
+
 1.9.0
 ++++++
 
@@ -9,7 +22,7 @@ Changes
 
 * Removed libssh2 native library dependency in favour of bundled ``ssh2-python`` libssh2 library.
 * Changed native client forward agent default behaviour to off due to incompatibility with certain SSH server implementations.
-* Added keep-alive functionality to native client - defaults to ``60`` seconds. ``ParallelSSHClient(<..>, keepalive_seconds=<interval>)`` to configure interval, Set to ``0`` to disable.
+* Added keep-alive functionality to native client - defaults to ``60`` seconds. ``ParallelSSHClient(<..>, keepalive_seconds=<interval>)`` to configure interval. Set to ``0`` to disable.
 * Added ``~/.ssh/id_ecdsa`` default identity location to native client.
 
 

README.rst

@@ -116,7 +116,7 @@ Native Code Client Features
 Exit codes
 ***********
 
-Once either standard output is iterated on *to completion*, or ``client.join(output)`` is called, exit codes become available in host output. Iteration ends *only when remote command has completed*, though it may be interrupted and resumed at any point.
+Once *either* standard output is iterated on *to completion*, or ``client.join(output)`` is called, exit codes become available in host output. Iteration ends *only when remote command has completed*, though it may be interrupted and resumed at any point.
 
 .. code-block:: python
 
@@ -178,10 +178,35 @@ To log output without having to iterate over output generators, the ``consume_ou
       [localhost]	Linux
 
 
+SCP
+****
+
+SCP is supported - native clients only - and provides the best performance for file copying.
+
+Unlike with the SFTP functionality, remote files that already exist are *not* overwritten and an exception is raised instead.
+
+Note that enabling recursion with SCP requires server SFTP support for creating remote directories.
+
+To copy a local file to remote hosts in parallel with SCP:
+
+.. code-block:: python
+
+  from pssh.clients import ParallelSSHClient
+  from gevent import joinall
+
+  hosts = ['myhost1', 'myhost2']
+  client = ParallelSSHClient(hosts)
+  cmds = client.scp_send('../test', 'test_dir/test')
+  joinall(cmds, raise_error=True)
+
+
+See also documentation for SCP recv.
+
+
 SFTP
-******
+*****
 
-SFTP is supported natively.
+SFTP is supported natively. Performance is much slower than SCP due to underlying library limitations and SCP should be preferred where possible. In the case of the deprecated paramiko clients, several bugs exist with SFTP performance and behaviour - avoid if at all possible.
 
 To copy a local file to remote hosts in parallel:
 

pssh/clients/miko/single.py

@@ -223,7 +223,7 @@ def _connect(self, client, host, port, sock=None, retries=1,
                 "Error connecting to host '%s:%s' - %s - retry %s/%s",
                 host, self.port, str(error_type), retries,
                 self.num_retries,)
-        except paramiko.AuthenticationException as ex:
+        except paramiko.AuthenticationException:
             msg = "Authentication error while connecting to %s:%s."
             raise AuthenticationException(msg, host, port)
         # SSHException is more general so should be below other types

pssh/clients/native/parallel.py

@@ -387,7 +387,7 @@ def _make_ssh_client(self, host):
                     keepalive_seconds=self.keepalive_seconds)
 
     def copy_file(self, local_file, remote_file, recurse=False, copy_args=None):
-        """Copy local file to remote file in parallel
+        """Copy local file to remote file in parallel via SFTP.
 
         This function returns a list of greenlets which can be
         `join`-ed on to wait for completion.
@@ -440,7 +440,7 @@ def copy_file(self, local_file, remote_file, recurse=False, copy_args=None):
     def copy_remote_file(self, remote_file, local_file, recurse=False,
                          suffix_separator='_', copy_args=None,
                          encoding='utf-8'):
-        """Copy remote file(s) in parallel as
+        """Copy remote file(s) in parallel via SFTP as
         <local_file><suffix_separator><host>
 
         With a ``local_file`` value of ``myfile`` and default separator ``_``
@@ -522,15 +522,109 @@ def _scp_recv(self, host, remote_file, local_file, recurse=False):
             remote_file, local_file, recurse=recurse)
 
     def scp_send(self, local_file, remote_file, recurse=False):
+        """Copy local file to remote file in parallel via SCP.
+
+        This function returns a list of greenlets which can be
+        `join`-ed on to wait for completion.
+
+        :py:func:`gevent.joinall` function may be used to join on all greenlets
+        and will also raise exceptions from them if called with
+        ``raise_error=True`` - default is `False`.
+
+        Alternatively call `.get()` on each greenlet to raise any exceptions
+        from it.
+
+        .. note::
+          Creating remote directories when either ``remote_file`` contains
+          directory paths or ``recurse`` is enabled requires SFTP support on
+          the server as libssh2 SCP implementation lacks directory creation
+          support.
+
+        :param local_file: Local filepath to copy to remote host
+        :type local_file: str
+        :param remote_file: Remote filepath on remote host to copy file to
+        :type remote_file: str
+        :param recurse: Whether or not to descend into directories recursively.
+        :type recurse: bool
+
+        :rtype: list(:py:class:`gevent.Greenlet`) of greenlets for remote copy
+          commands.
+
+        :raises: :py:class:`pss.exceptions.SCPError` on errors copying file.
+        :raises: :py:class:`OSError` on local OS errors like permission denied.
+        """
         return [self.pool.spawn(self._scp_send, host, local_file,
                                 remote_file, recurse=recurse)
                 for host in self.hosts]
 
-    def scp_recv(self, remote_file, local_file, recurse=False, copy_args=None):
-        copy_args = [{'local_file': '_'.join([local_file, host]),
+    def scp_recv(self, remote_file, local_file, recurse=False, copy_args=None,
+                 suffix_separator='_'):
+        """Copy remote file(s) in parallel via SCP as
+        <local_file><suffix_separator><host> or as per ``copy_args`` argument.
+
+        With a ``local_file`` value of ``myfile`` and default separator ``_``
+        the resulting filename will be ``myfile_myhost`` for the file from host
+        ``myhost``.
+
+        De-duplication behaviour is configurable by providing ``copy_args``
+        argument, see below.
+
+        This function, like :py:func:`ParallelSSHClient.scp_send`, returns a
+        list of greenlets which can be `join`-ed on to wait for completion.
+
+        :py:func:`gevent.joinall` function may be used to join on all greenlets
+        and will also raise exceptions if called with ``raise_error=True`` -
+        default is `False`.
+
+        Alternatively call `.get` on each greenlet to raise any exceptions from
+        it.
+
+        Exceptions listed here are raised when
+        either ``gevent.joinall(<greenlets>, raise_error=True)`` is called
+        or ``.get`` is called on each greenlet, not this function itself.
+
+        :param remote_file: remote filepath to copy to local host
+        :type remote_file: str
+        :param local_file: local filepath on local host to copy file to
+        :type local_file: str
+        :param recurse: whether or not to recurse
+        :type recurse: bool
+        :param suffix_separator: (Optional) Separator string between
+          filename and host, defaults to ``_``. For example, for a
+          ``local_file`` value of ``myfile`` and default separator the
+          resulting filename will be ``myfile_myhost`` for the file from
+          host ``myhost``. ``suffix_separator`` has no meaning if
+          ``copy_args`` is provided
+        :type suffix_separator: str
+        :param copy_args: (Optional) format remote_file and local_file strings
+          with per-host arguments in ``copy_args``. ``copy_args`` length *must*
+          equal length of host list -
+          :py:class:`pssh.exceptions.HostArgumentException` is raised otherwise
+        :type copy_args: tuple or list
+
+        :rtype: list(:py:class:`gevent.Greenlet`) of greenlets for remote copy
+          commands.
+
+        :raises: :py:class:`ValueError` when a directory is supplied to
+          local_file and recurse is not set.
+        :raises: :py:class:`pssh.exceptions.HostArgumentException` on number of
+          per-host copy arguments not equal to number of hosts.
+        :raises: :py:class:`pss.exceptions.SCPError` on errors copying file.
+        :raises: :py:class:`OSError` on local OS errors like permission denied.
+
+        .. note ::
+          Local directories in ``local_file`` that do not exist will be
+          created as long as permissions allow.
+
+        .. note ::
+          File names will be de-duplicated by appending the hostname to the
+          filepath separated by ``suffix_separator`` or as per ``copy_args``
+          argument if provided.
+        """
+        copy_args = [{'local_file': suffix_separator.join([local_file, host]),
                       'remote_file': remote_file}
                      for i, host in enumerate(self.hosts)] \
-                         if copy_args is None else copy_args
+            if copy_args is None else copy_args
         local_file = "%(local_file)s"
         remote_file = "%(remote_file)s"
         try:

pssh/clients/native/single.py

@@ -41,7 +41,7 @@
      ConnectionErrorException, SessionError, SFTPError, SFTPIOError, Timeout, \
      SCPError
 from ...constants import DEFAULT_RETRIES, RETRY_DELAY
-from ...native._ssh2 import wait_select, _read_output
+from ...native._ssh2 import wait_select, eagain_write, _read_output
 from .common import _validate_pkey_path
 
 
@@ -524,9 +524,9 @@ def copy_file(self, local_file, remote_file, recurse=False,
                     local_file, self.host, remote_file)
 
     def _sftp_put(self, remote_fh, local_file):
-        with open(local_file, 'rb') as local_fh:
+        with open(local_file, 'rb', 2097152) as local_fh:
             for data in local_fh:
-                self._eagain(remote_fh.write, data)
+                eagain_write(remote_fh.write, data, self.session)
 
     def sftp_put(self, sftp, local_file, remote_file):
         mode = LIBSSH2_SFTP_S_IRUSR | \
@@ -787,16 +787,16 @@ def _scp_send(self, local_file, remote_file):
         try:
             chan = self._eagain(
                 self.session.scp_send64,
-                remote_file, fileinfo.st_mode & 777, fileinfo.st_size,
+                remote_file, fileinfo.st_mode & 0o777, fileinfo.st_size,
                 fileinfo.st_mtime, fileinfo.st_atime)
         except Exception as ex:
             msg = "Error opening remote file %s for writing on host %s - %s"
             logger.error(msg, remote_file, self.host, ex)
             raise SCPError(msg, remote_file, self.host, ex)
         try:
-            with open(local_file, 'rb') as local_fh:
+            with open(local_file, 'rb', 2097152) as local_fh:
                 for data in local_fh:
-                    chan.write(data)
+                    eagain_write(chan.write, data, self.session)
         except Exception as ex:
             msg = "Error writing to remote file %s on host %s - %s"
             logger.error(msg, remote_file, self.host, ex)