Deprecate protected methods

1. Deprecate some of protected methods in favor of ending underscore name
2. Use try: except: for sftp connection

blueprint fuel-qa-python3-compatibility

Change-Id: Ib512e4c92281ad7c48e6fd5a44d7dfecae2a3cc4
(cherry picked from commit df2c25d)

devops/driver/libvirt/libvirt_driver.py

@@ -23,7 +23,7 @@ from netaddr import IPNetwork
 
 from devops.driver.libvirt.libvirt_xml_builder import LibvirtXMLBuilder
 from devops.error import DevopsError
-from devops.helpers.helpers import _get_file_size
+from devops.helpers.helpers import get_file_size
 from devops.helpers.retry import retry
 from devops.helpers import scancodes
 from devops import logger
@@ -921,7 +921,7 @@ class DevopsDriver(object):
 
     @retry(count=2)
     def volume_upload(self, volume, path):
-        size = _get_file_size(path)
+        size = get_file_size(path)
         with open(path, 'rb') as fd:
             stream = self.conn.newStream(0)
             self.conn.storageVolLookupByKey(volume.uuid).upload(

devops/helpers/helpers.py

@@ -20,6 +20,7 @@ import posixpath
 import socket
 import stat
 import time
+from warnings import warn
 
 from keystoneauth1.identity import V2Password
 from keystoneauth1.session import Session as KeystoneSession
@@ -60,7 +61,7 @@ def icmp_ping(host, timeout=1):
             'host': host, 'timeout': timeout}) == 0
 
 
-def _tcp_ping(host, port, timeout=None):
+def tcp_ping_(host, port, timeout=None):
     s = socket.socket()
     if timeout:
         s.settimeout(timeout)
@@ -68,6 +69,12 @@ def _tcp_ping(host, port, timeout=None):
     s.close()
 
 
+def _tcp_ping(*args, **kwargs):
+    logger.warning('_tcp_ping is deprecated in favor of tcp_ping')
+    warn('_tcp_ping is deprecated in favor of tcp_ping', DeprecationWarning)
+    return tcp_ping_(*args, **kwargs)
+
+
 def tcp_ping(host, port, timeout=None):
     """Run TCP ping
 
@@ -76,7 +83,7 @@ def tcp_ping(host, port, timeout=None):
     False - otherwise.
     """
     try:
-        _tcp_ping(host, port, timeout)
+        tcp_ping_(host, port, timeout)
     except socket.error:
         return False
     return True
@@ -112,7 +119,8 @@ def wait(predicate, interval=5, timeout=60, timeout_msg="Waiting timed out"):
     return timeout + start_time - time.time()
 
 
-def _wait(raising_predicate, expected=Exception, interval=5, timeout=None):
+def wait_pass(raising_predicate, expected=Exception, interval=5, timeout=None):
+    """Wait for successful return from predicate or expected exception"""
     start_time = time.time()
     while True:
         try:
@@ -123,6 +131,12 @@ def _wait(raising_predicate, expected=Exception, interval=5, timeout=None):
             time.sleep(interval)
 
 
+def _wait(*args, **kwargs):
+    logger.warning('_wait has been deprecated in favor of wait_pass')
+    warn('_wait has been deprecated in favor of wait_pass', DeprecationWarning)
+    return wait_pass(*args, **kwargs)
+
+
 def http(host='localhost', port=80, method='GET', url='/', waited_code=200):
     try:
         conn = http_client.HTTPConnection(str(host), int(port))
@@ -259,14 +273,27 @@ class SSHClient(object):
 
         self.sudo_mode = False
         self.sudo = self.get_sudo(self)
+        self._ssh = None
+        self.__sftp = None
 
         self.reconnect()
 
+    @property
+    def _sftp(self):
+        if self.__sftp is not None:
+            return self.__sftp
+        logger.warning('SFTP is not connected, try to reconnect')
+        self._connect_sftp()
+        if self.__sftp is not None:
+            return self.__sftp
+        raise paramiko.SSHException('SFTP connection failed')
+
     def clear(self):
-        try:
-            self._sftp.close()
-        except Exception:
-            logger.exception("Could not close sftp connection")
+        if self.__sftp is not None:
+            try:
+                self.__sftp.close()
+            except Exception:
+                logger.exception("Could not close sftp connection")
         try:
             self._ssh.close()
         except Exception:
@@ -301,11 +328,17 @@ class SSHClient(object):
             self.host, port=self.port, username=self.username,
             password=self.password)
 
+    def _connect_sftp(self):
+        try:
+            self.__sftp = self._ssh.open_sftp()
+        except paramiko.SSHException:
+            logger.warning('SFTP enable failed! SSH only is accessible.')
+
     def reconnect(self):
         self._ssh = paramiko.SSHClient()
         self._ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
         self.connect()
-        self._sftp = self._ssh.open_sftp()
+        self._connect_sftp()
 
     def check_call(self, command, verbose=False):
         ret = self.execute(command, verbose)
@@ -328,7 +361,7 @@ class SSHClient(object):
         for remote in remotes:
             cmd = "%s\n" % command
             if remote.sudo_mode:
-                cmd = 'sudo -S bash -c "{}"'.format(cmd.replace('"', '\\"'))
+                cmd = 'sudo -S bash -c "%s"' % cmd.replace('"', '\\"')
             chan = remote._ssh.get_transport().open_session()
             chan.exec_command(cmd)
             futures[remote] = chan
@@ -483,7 +516,7 @@ def generate_mac():
         *bytearray(os.urandom(5)))
 
 
-def _get_file_size(path):
+def get_file_size(path):
     """Get size of file-like object
 
     :type path: str
@@ -493,6 +526,15 @@ def _get_file_size(path):
     return os.stat(path).st_size
 
 
+def _get_file_size(*args, **kwargs):
+    logger.warning(
+        '_get_file_size has been deprecated in favor of get_file_size')
+    warn(
+        '_get_file_size has been deprecated in favor of get_file_size',
+        DeprecationWarning)
+    return get_file_size(*args, **kwargs)
+
+
 def get_nodes(admin_ip):
     keystone_auth = V2Password(
         auth_url="http://{}:5000/v2.0".format(admin_ip),

devops/helpers/templates.py

@@ -15,7 +15,7 @@
 import os
 import yaml
 
-from devops.helpers.helpers import _get_file_size
+from devops.helpers.helpers import get_file_size
 
 
 def yaml_template_load(config_file):
@@ -127,7 +127,7 @@ def create_admin_config(admin_vcpu, admin_memory, admin_sysvolume_capacity,
                 {
                     'name': 'iso',
                     'source_image': admin_iso_path,
-                    'capacity': _get_file_size(admin_iso_path),
+                    'capacity': get_file_size(admin_iso_path),
                     'format': 'raw',
                     'device': iso_device,
                     'bus': iso_bus,

devops/models/environment.py

@@ -20,7 +20,7 @@ from netaddr import IPNetwork
 from paramiko import Agent
 from paramiko import RSAKey
 
-from devops.helpers.helpers import _get_file_size
+from devops.helpers.helpers import get_file_size
 from devops.helpers.helpers import SSHClient
 from devops.helpers.templates import create_devops_config
 from devops.helpers.templates import get_devops_config
@@ -364,7 +364,7 @@ class Environment(DriverModel):
                 new_vol = self.add_empty_volume(
                     node,
                     volume_name,
-                    capacity=_get_file_size(volume['source_image']),
+                    capacity=get_file_size(volume['source_image']),
                     format=volume.get('format', 'qcow2'),
                     device=volume.get('device', 'disk'),
                     bus=volume.get('bus', 'virtio'),

devops/models/node.py

@@ -20,9 +20,9 @@ from django.conf import settings
 from django.db import models
 
 from devops.error import DevopsError
-from devops.helpers.helpers import _tcp_ping
-from devops.helpers.helpers import _wait
 from devops.helpers.helpers import SSHClient
+from devops.helpers.helpers import tcp_ping
+from devops.helpers.helpers import wait_pass
 from devops import logger
 from devops.models.base import choices
 from devops.models.base import DriverModel
@@ -176,8 +176,8 @@ class Node(DriverModel):
         self.driver.node_send_keys(self, keys)
 
     def await(self, network_name, timeout=120, by_port=22):
-        _wait(
-            lambda: _tcp_ping(
+        wait_pass(
+            lambda: tcp_ping(
                 self.get_ip_address_by_network_name(network_name), by_port),
             timeout=timeout)
 

devops/shell.py

@@ -26,7 +26,7 @@ from six.moves import xrange
 import tabulate
 
 import devops
-from devops.helpers.helpers import _get_file_size
+from devops.helpers.helpers import get_file_size
 from devops.helpers import node_manager
 from devops.helpers.ntp import sync_time
 from devops.helpers.templates import create_admin_config
@@ -285,7 +285,7 @@ class Shell(object):
         vcpu = self.params.admin_vcpu_count
         ram = self.params.admin_ram_size
         iso_path = self.params.iso_path
-        iso_size = _get_file_size(iso_path)
+        iso_size = get_file_size(iso_path)
 
         if iso_size <= 0:
             print("Please, set correct ISO file")

devops/tests/use_cases.py

@@ -17,9 +17,9 @@ import unittest
 
 os.environ.setdefault("DJANGO_SETTINGS_MODULE", "devops.settings")
 
-from devops.driver.libvirt import libvirt_driver
-from devops.helpers.helpers import _get_file_size
-from devops.models import Volume
+from devops.driver.libvirt import libvirt_driver  # noqa
+from devops.helpers.helpers import get_file_size  # noqa
+from devops.models import Volume  # noqa
 
 
 class UseCases(unittest.TestCase):
@@ -33,7 +33,7 @@ class UseCases(unittest.TestCase):
         }
 
         for name, vol in images_for_upload.items():
-            v = Volume.volume_create(name, _get_file_size(vol))
+            v = Volume.volume_create(name, get_file_size(vol))
             if not self.driver.volume_exists(v):
                 self.driver.volume_define(v)
                 self.driver.volume_upload(v, vol)