[1LP][RFR] Prepare connect_ssh

cfme/fixtures/utility_vm.py

@@ -16,77 +16,54 @@
 ping, but the required service may not be up and ready.
 """
 import os.path
-import time
-from contextlib import contextmanager
 
 import pytest
 
 from cfme.base.credential import SSHCredential
 from cfme.utils.conf import cfme_data
 from cfme.utils.generators import random_vm_name
 from cfme.utils.log import logger
-from cfme.utils.net import net_check
+from cfme.utils.net import pick_responding_ip
 from cfme.utils.ssh import SSHClient
 from cfme.utils.virtual_machines import deploy_template
-from cfme.utils.wait import TimedOutError
 
 
-def _trying_ips(vm, attempts=60, interval=10):
-    for attempt in range(attempts):
-        for ip in getattr(vm, 'all_ips', []):
-            yield ip
-            return
-        time.sleep(interval)
-
-
-@contextmanager
-def connect_ssh(vm, creds):
-    for ip in _trying_ips(vm):
-        try:
-            with SSHClient(hostname=ip, username=creds.principal, password=creds.secret) as client:
-                logger.info("SSH connected to IP %s", ip)
-                result = client.run_command("true")
-                if not result.success:
-                    raise Exception(f"Command `true` failed on ip {ip}.")
-                yield client
-                return
-        except Exception as ex:
-            logger.info("Failed to connect with SSH to %s: %s", ip, ex)
-    else:
-        raise TimedOutError(f"Coudln't find an IP responding to ssh for vm {vm}")
-
-
-def _pick_responding_ip(vm, port):
-    for ip in _trying_ips(vm):
-        if net_check(port, ip):
-            return ip
-    else:
-        raise TimedOutError(f"Coudln't find an IP of vm {vm} with port {port} responding")
+ATTEMPT_TIMEOUT = 10
+""" How long to wait until connection is determined as not successful. """
+IP_PICK_TIMEOUT = 600
+""" How many rounds to connect all the vm IPs. """
+ROUNDS_DELAY = 10
+""" How long to delay after unsucessful connection round. """
 
 
 @pytest.fixture
 def utility_vm_nfs_ip(utility_vm):
     vm, _, _ = utility_vm
     one_of_the_nfs_ports = 111
-    yield _pick_responding_ip(vm, one_of_the_nfs_ports)
+    yield pick_responding_ip(lambda: vm.all_ips, one_of_the_nfs_ports,
+        IP_PICK_TIMEOUT, ROUNDS_DELAY, ATTEMPT_TIMEOUT)
 
 
 @pytest.fixture
 def utility_vm_samba_ip(utility_vm):
     vm, _, _ = utility_vm
-    yield _pick_responding_ip(vm, 445)
+    yield pick_responding_ip(lambda: vm.all_ips, 445,
+        IP_PICK_TIMEOUT, ROUNDS_DELAY, ATTEMPT_TIMEOUT)
 
 
 @pytest.fixture(scope='module')
 def utility_vm_proxy_data(utility_vm):
     vm, __, data = utility_vm
-    yield _pick_responding_ip(vm, data.proxy.port), data.proxy.port
+    ip = pick_responding_ip(lambda: vm.all_ips, data.proxy.port,
+        IP_PICK_TIMEOUT, ROUNDS_DELAY, ATTEMPT_TIMEOUT)
+    yield ip, data.proxy.port
 
 
 @pytest.fixture(scope='module')
 def utility_vm_ssh(utility_vm):
     vm, injected_user_cred, __ = utility_vm
-    ip = _pick_responding_ip(vm, 22)
+    ip = pick_responding_ip(lambda: vm.all_ips, 22,
+        IP_PICK_TIMEOUT, ROUNDS_DELAY, ATTEMPT_TIMEOUT)
 
     with SSHClient(
             hostname=ip,

cfme/utils/net.py

@@ -58,6 +58,51 @@ def ip_echo_socket(port=32123):
             conn.close()
 
 
+def pick_responding_ip(vm, port, num_sec, rounds_delay_second, attempt_timeout):
+    """
+    Given a vm and port, pick one of the vm's addresses that is connectible
+    on the given port
+
+    Args:
+        vm: mgmt vm
+        port: port number to attempt connecting to
+        num_sec: Minimal ammount of time how long to keep checking (slight
+                 variation may happen -- approximately the attempt_timeout).
+        rounds_delay_second: The delay to wait after checking each IP round in
+                             immediate succession.
+        attempt_timeout: A connection timeout for every connection attempt.
+
+    Raise TimedOutError if no such IP is found.
+    """
+
+    def connection_factory(ip):
+        if net_check(port, ip, attempt_timeout):
+            return ip
+
+    return retry_connect(vm, connection_factory, num_sec, rounds_delay_second)
+
+
+def retry_connect(ips_getter, connection_factory, num_sec, delay):
+    def _try_batch_of_ips():
+        for ip in ips_getter():
+            try:
+                connection = connection_factory(ip)
+            except Exception as ex:
+                logger.warning(f"Failed to connect {ip}: {ex}")
+                continue
+            else:
+                logger.info(f"Connected to IP {ip}")
+                # No other IPs should be attempted, so return.
+                return connection
+        return False
+    connection, _ = wait_for(_try_batch_of_ips, num_sec=num_sec, delay=delay)
+    return connection
+
+
+def retry_connect_vm(vm, connection_factory, num_sec, delay):
+    return retry_connect(lambda: vm.all_ips, connection_factory, num_sec, delay)
+
+
 def net_check(port, addr=None, force=False, timeout=10):
     """Checks the availablility of a port"""
     port = int(port)

cfme/utils/ssh.py

@@ -18,6 +18,7 @@
 from cfme.utils import ports
 from cfme.utils.log import logger
 from cfme.utils.net import net_check
+from cfme.utils.net import retry_connect_vm
 from cfme.utils.path import project_path
 from cfme.utils.quote import quote
 from cfme.utils.timeutil import parsetime
@@ -28,6 +29,9 @@
 # Default blocking time before giving up on an ssh command execution,
 # in seconds (float)
 RUNCMD_TIMEOUT = 1200.0
+CONNECT_RETRIES_TIMEOUT = 2 * 60
+CONNECT_TIMEOUT = 10
+CONNECT_SSH_DELAY = 1
 
 
 @attr.s(frozen=True, eq=False)
@@ -121,7 +125,7 @@ class SSHClient(paramiko.SSHClient):
         stdout: If specified, overrides the system stdout file for streaming output.
         stderr: If specified, overrides the system stderr file for streaming output.
     """
-    def __init__(self, stream_output=False, **connect_kwargs):
+    def __init__(self, *, stream_output=False, **connect_kwargs):
         super().__init__()
         self._streaming = stream_output
         # deprecated/useless karg, included for backward-compat
@@ -135,11 +139,12 @@ def __init__(self, stream_output=False, **connect_kwargs):
         self.oc_password = connect_kwargs.pop('oc_password', False)
         self.f_stdout = connect_kwargs.pop('stdout', sys.stdout)
         self.f_stderr = connect_kwargs.pop('stderr', sys.stderr)
+        self._use_check_port = connect_kwargs.pop('use_check_port', True)
         self.strict_host_key_checking = connect_kwargs.pop('strict_host_key_checking', True)
 
         # load the defaults for ssh, including current_appliance and default credentials keys
         compiled_kwargs = dict(
-            timeout=10,
+            timeout=CONNECT_TIMEOUT,
             allow_agent=False,
             look_for_keys=False,
             gss_auth=False,
@@ -231,7 +236,9 @@ def connect(self, hostname=None, **kwargs):
 
         if not self.connected:
             self._connect_kwargs.update(kwargs)
-            wait_for(self._check_port, handle_exception=True, timeout='2m', delay=5)
+            if self._use_check_port:
+                wait_for(self._check_port, handle_exception=True,
+                         timeout=CONNECT_TIMEOUT, delay=5)
             try:
                 conn = super().connect(**self._connect_kwargs)
             except paramiko.ssh_exception.BadHostKeyException:
@@ -842,3 +849,42 @@ def keygen():
     with _ssh_pubkey_file.open('w') as f:
         f.write("{} {} {}\n".format(pub.get_name(), pub.get_base64(),
             'autogenerated cfme_tests key'))
+
+
+def unquirked_ssh_client(**kwargs):
+    """ A factory for the SSHClient dealing with some of its quirks:
+
+    * When hostname is None, the SSHClient will connect to the default
+      appliance.
+    * Set use_check_port to False to disable waiting for port being
+      available in the SSHClient.
+    * The SSHClient does not connect in the object creation time.
+    """
+    if kwargs['hostname'] is None:
+        raise ValueError(f'The hostname argument cannot be None!')
+    client = SSHClient(use_check_port=False, **kwargs)
+    client.connect()
+    return client
+
+
+def connect_ssh(vm, creds,
+                num_sec=CONNECT_RETRIES_TIMEOUT,
+                connect_timeout=CONNECT_TIMEOUT,
+                delay=CONNECT_SSH_DELAY):
+    """
+    This function attempts to connect all the IPs of the vm in several
+    round. After each round it will delay an ammount of seconds specified as
+    `delay`. Once connective IP is found, it will return connected SSHClient.
+    The `connect_timeout` specifies a timeout for each connection attempt.
+    """
+
+    def _connection_factory(ip):
+        return unquirked_ssh_client(
+            hostname=ip,
+            username=creds.principal, password=creds.secret,
+            timeout=connect_timeout)
+
+    msg = "The num_sec is smaller then connect_timeout. This looks like a bug."
+    assert num_sec > connect_timeout, msg
+
+    return retry_connect_vm(vm, _connection_factory, num_sec=num_sec, delay=delay)

cfme/utils/tests/test_connect_ssh.py

@@ -0,0 +1,68 @@
+from types import SimpleNamespace
+from unittest.mock import Mock
+from unittest.mock import PropertyMock
+
+import pytest
+
+from cfme.utils.ssh import connect_ssh
+
+
+creds = SimpleNamespace(principal='mockuser', secret='mockpass')
+
+
+@pytest.fixture(autouse=True)
+def nosleep(mocker):
+    mocker.patch('time.sleep')
+
+
+@pytest.fixture
+def vm_mock():
+    vm = Mock()
+    all_ips_mock = PropertyMock()
+    type(vm).all_ips = all_ips_mock
+    all_ips_mock.side_effect = [
+        [None],
+        ['NOT_WORKING_IP_MOCK'],
+        ['NOT_WORKING_IP_MOCK', 'OTHER_NOT_WORKING_IP_MOCK',
+            'WORKING_IP_MOCK', 'SHOULD_NOT_REACH_THIS_MOCK'],
+        ['SHOULD_NOT_REACH_THIS_MOCK', 'SHOULD_NOT_REACH_THIS_MOCK']
+    ]
+    return vm
+
+
+@pytest.fixture
+def vm_mock2():
+    vm = Mock()
+    all_ips_mock = PropertyMock()
+    type(vm).all_ips = all_ips_mock
+    all_ips_mock.side_effect = [
+        ['WORKING_IP_MOCK', 'SHOULD_NOT_REACH_THIS_MOCK']
+    ]
+    return vm
+
+
+class SSHClientMock:
+    def __init__(self, **kwargs):
+        self.kwargs = kwargs
+        self.close = Mock()
+        self.run_command = Mock()
+        self.run_command.success.return_value = True
+
+        # disabling check_port is required for not making repeated connection
+        # attempts in the SSHClient
+        assert kwargs['use_check_port'] is False
+
+    def connect(self):
+        hostname = self.kwargs['hostname']
+        if hostname == 'WORKING_IP_MOCK':
+            return self
+        elif hostname == 'SHOULD_NOT_REACH_THIS_MOCK':
+            pytest.fail('We should not have reached checking this IP!')
+        else:
+            raise Exception(f'This was raised for IP {hostname}.')
+
+
+def test_connect_ssh(mocker, vm_mock, vm_mock2):
+    mocker.patch('cfme.utils.ssh.SSHClient', SSHClientMock)
+    assert connect_ssh(vm_mock, creds, num_sec=3, connect_timeout=1, delay=1).run_command()
+    assert connect_ssh(vm_mock2, creds, num_sec=3, connect_timeout=1, delay=1).run_command()

requirements/frozen.txt

@@ -259,6 +259,7 @@ pyperclip==1.7.0
 pytesseract==0.3.3
 pytest==5.4.1
 pytest-polarion-collect==0.23.1
+pytest-mock==2.0.0
 python-box==3.2.4
 python-bugzilla==2.3.0
 python-cinderclient==4.1.0

requirements/template_non_imported.txt

@@ -9,5 +9,6 @@ pdfminer.six
 polarion-docstrings
 polarion-tools-common
 pre-commit
+pytest-mock
 pytest-polarion-collect
 pytest-report-parameters