Add timeout to all open_session calls

The default paramiko timeout value when a session/channel is going to be
opened is 3600 seconds:
https://github.com/paramiko/paramiko/blob/2.10.4/paramiko/transport.py#L1009

Sometimes tobiko tests get stuck when they call the paramiko open_session
method and pytest times out (its test timeout value is 3600 too)
This happens especially when tobiko tries to connect to a machine that
has just been rebooted

This patch adds a timeout of 30 seconds to any open_session calls that
did not include any value before

After removing the open session timeout of 3600 seconds, some ubuntu
tests failed on upstream CI because it takes more than the default
ubuntu connection timeout (600 seconds) to start an ubuntu instance
sometimes - that value has been changed to 1500 seconds

Additionally, an incoherent behavior has been fixed from
SSHShellProcessFixture.create_process method:
- this method includes a retry loop, and it calls ssh_client.connect
  within it
- the ssh_client.connect method includes another retry loop
- it may happen that the timeout and reattempts of the inner loop are
  higher than the timeout and reattempts of the outer loop, which should not
  happen
This has been fixed in this patch as well

Change-Id: I4133c559c3299fe7965cafae108146d2661052fb
This commit is contained in:
Eduardo Olivares 2022-09-16 11:01:24 +02:00
parent cd0710e2c8
commit fe3e5cce56
4 changed files with 37 additions and 5 deletions

View File

@ -44,7 +44,7 @@ class UbuntuMinimalImageFixture(glance.URLGlanceImageFixture):
container_format = CONF.tobiko.ubuntu.container_format or "bare" container_format = CONF.tobiko.ubuntu.container_format or "bare"
username = CONF.tobiko.ubuntu.username or 'ubuntu' username = CONF.tobiko.ubuntu.username or 'ubuntu'
password = CONF.tobiko.ubuntu.password or 'ununtu' password = CONF.tobiko.ubuntu.password or 'ununtu'
connection_timeout = CONF.tobiko.ubuntu.connection_timeout or 600. connection_timeout = CONF.tobiko.ubuntu.connection_timeout or 1500.
disabled_algorithms = CONF.tobiko.ubuntu.disabled_algorithms disabled_algorithms = CONF.tobiko.ubuntu.disabled_algorithms

View File

@ -73,6 +73,7 @@ def ssh_process(command, environment=None, current_dir=None,
class SSHShellProcessParameters(_process.ShellProcessParameters): class SSHShellProcessParameters(_process.ShellProcessParameters):
ssh_client = None ssh_client = None
open_session_timeout = 30.0
class SSHShellProcessFixture(_process.ShellProcessFixture): class SSHShellProcessFixture(_process.ShellProcessFixture):
@ -91,9 +92,36 @@ class SSHShellProcessFixture(_process.ShellProcessFixture):
environment = parameters.environment environment = parameters.environment
current_dir = parameters.current_dir current_dir = parameters.current_dir
if (hasattr(ssh_client, 'connect_parameters') and
ssh_client.connect_parameters is not None):
ssh_client_timeout = ssh_client.connect_parameters.get(
'connection_timeout')
ssh_client_attempts = ssh_client.connect_parameters.get(
'connection_attempts')
else:
ssh_client_timeout = None
ssh_client_attempts = None
# use ssh_client timeout and attempts values if they are higher than
# self (SSHShellProcessFixture) values because the values from the
# outer retry loop should be equal or greater than those from the inner
# retry loop
process_retry_timeout = (
ssh_client_timeout if (
ssh_client_timeout and (
self.parameters.timeout is None or
ssh_client_timeout > self.parameters.timeout))
else self.parameters.timeout)
process_retry_attempts = (
ssh_client_attempts if (
ssh_client_attempts and (
self.parameters.retry_count is None or
ssh_client_attempts > self.parameters.retry_count))
else self.parameters.retry_count)
for attempt in tobiko.retry( for attempt in tobiko.retry(
timeout=self.parameters.timeout, timeout=process_retry_timeout,
default_count=self.parameters.retry_count, default_count=process_retry_attempts,
default_interval=self.parameters.retry_interval, default_interval=self.parameters.retry_interval,
default_timeout=self.parameters.retry_timeout): default_timeout=self.parameters.retry_timeout):
@ -107,7 +135,8 @@ class SSHShellProcessFixture(_process.ShellProcessFixture):
LOG.debug(f"Create remote process... ({details})") LOG.debug(f"Create remote process... ({details})")
try: try:
client = ssh_client.connect() client = ssh_client.connect()
process = client.get_transport().open_session() process = client.get_transport().open_session(
timeout=self.open_session_timeout)
if environment: if environment:
variables = " ".join( variables = " ".join(
f"{name}={shlex.quote(value)}" f"{name}={shlex.quote(value)}"

View File

@ -156,7 +156,8 @@ class SSHUnixForwardHandler(sshtunnel._ForwardHandler):
assert isinstance(remote_address, str) assert isinstance(remote_address, str)
command = 'sudo nc -U "{}"'.format(remote_address) command = 'sudo nc -U "{}"'.format(remote_address)
chan = self.transport.open_session() # reusing the same timeout value that is used by the parent class
chan = self.transport.open_session(timeout=sshtunnel.TUNNEL_TIMEOUT)
chan.exec_command(command) chan.exec_command(command)
self.logger.log(sshtunnel.TRACE_LEVEL, self.logger.log(sshtunnel.TRACE_LEVEL,

View File

@ -44,6 +44,8 @@ class HostnameTest(unit.TobikoUnitTest):
client_mock = mock.MagicMock(spec=ssh.SSHClientFixture) client_mock = mock.MagicMock(spec=ssh.SSHClientFixture)
client_mock.connect().get_transport().open_session.return_value = \ client_mock.connect().get_transport().open_session.return_value = \
channel_mock channel_mock
client_mock.connect_parameters = {'retry_count': 200,
'connection_timeout': 1000}
return client_mock return client_mock
def test_get_hostname_with_no_ssh_client(self): def test_get_hostname_with_no_ssh_client(self):