From 2731eea16b41d4620e67de2e3d94e163336ba957 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Tue, 14 Jul 2026 11:23:50 -0600 Subject: [PATCH 1/2] Add type annotations Signed-off-by: Zack Cerza # Conflicts: # teuthology/task/install/__init__.py --- tests/orchestra/test_console.py | 6 +- tests/orchestra/test_remote.py | 4 +- tests/provision/cloud/test_openstack.py | 2 +- tests/test_timer.py | 4 +- teuthology/__init__.py | 10 +- teuthology/beanstalk.py | 41 ++++---- teuthology/config.py | 69 ++++++------- teuthology/contextutil.py | 18 ++-- teuthology/describe_tests.py | 2 +- teuthology/dispatcher/__init__.py | 8 +- teuthology/dispatcher/supervisor.py | 2 +- teuthology/exceptions.py | 61 ++++++------ teuthology/exit.py | 14 +-- teuthology/job_status.py | 7 +- teuthology/lock/cli.py | 6 +- teuthology/lock/ops.py | 72 +++++++------- teuthology/lock/query.py | 42 ++++---- teuthology/lock/util.py | 13 ++- teuthology/misc.py | 6 +- teuthology/nuke/__init__.py | 2 +- teuthology/orchestra/cluster.py | 24 ++--- teuthology/orchestra/connection.py | 16 ++- teuthology/orchestra/console.py | 89 ++++++++++------- teuthology/orchestra/daemon/cephadmunit.py | 1 + teuthology/orchestra/daemon/group.py | 14 +-- teuthology/orchestra/daemon/state.py | 11 ++- teuthology/orchestra/monkey.py | 21 +--- teuthology/orchestra/opsys.py | 50 +++++++--- teuthology/orchestra/remote.py | 48 ++++++--- teuthology/orchestra/run.py | 21 ++-- teuthology/packaging.py | 14 ++- teuthology/parallel.py | 20 ++-- teuthology/provision/__init__.py | 16 +-- teuthology/provision/cloud/__init__.py | 27 ++++-- teuthology/provision/cloud/base.py | 24 +++-- teuthology/provision/cloud/openstack.py | 75 ++++++++------- teuthology/provision/cloud/util.py | 19 ++-- teuthology/provision/downburst.py | 89 ++++++++++------- teuthology/provision/fog.py | 54 ++++++----- teuthology/provision/maas.py | 35 ++++--- teuthology/provision/openstack.py | 30 +++--- teuthology/provision/pelagos.py | 38 +++++--- teuthology/repo_utils.py | 8 +- teuthology/safepath.py | 5 +- teuthology/suite/__init__.py | 19 ++-- teuthology/suite/build_matrix.py | 23 +++-- teuthology/suite/matrix.py | 107 +++++++++++---------- teuthology/suite/merge.py | 7 +- teuthology/suite/placeholder.py | 4 +- teuthology/suite/run.py | 52 +++++----- teuthology/suite/util.py | 43 +++++---- teuthology/task/__init__.py | 21 ++-- teuthology/task/ansible.py | 70 +++++++------- teuthology/task/args.py | 12 ++- teuthology/task/background_exec.py | 3 +- teuthology/task/buildpackages.py | 24 ++--- teuthology/task/ceph_ansible.py | 40 ++++---- teuthology/task/cephmetrics.py | 17 ++-- teuthology/task/clock.py | 10 +- teuthology/task/common_fs_utils.py | 10 +- teuthology/task/console_log.py | 19 ++-- teuthology/task/dump_ctx.py | 4 +- teuthology/task/exec.py | 3 +- teuthology/task/full_sequential.py | 4 +- teuthology/task/full_sequential_finally.py | 7 +- teuthology/task/hadoop.py | 26 ++--- teuthology/task/install/__init__.py | 39 ++++---- teuthology/task/install/deb.py | 18 ++-- teuthology/task/install/redhat.py | 18 ++-- teuthology/task/install/rpm.py | 36 +++---- teuthology/task/install/util.py | 13 +-- teuthology/task/interactive.py | 5 +- teuthology/task/internal/__init__.py | 48 ++++----- teuthology/task/internal/check_lock.py | 3 +- teuthology/task/internal/git_ignore_ssl.py | 3 +- teuthology/task/internal/lock_machines.py | 3 +- teuthology/task/internal/redhat.py | 33 ++++--- teuthology/task/internal/syslog.py | 5 +- teuthology/task/internal/vm_setup.py | 4 +- teuthology/task/iscsi.py | 30 +++--- teuthology/task/kernel.py | 3 +- teuthology/task/knfsd.py | 5 +- teuthology/task/localdir.py | 3 +- teuthology/task/lockfile.py | 10 +- teuthology/task/loop.py | 4 +- teuthology/task/mpi.py | 4 +- teuthology/task/nfs.py | 8 +- teuthology/task/nop.py | 3 +- teuthology/task/parallel.py | 6 +- teuthology/task/parallel_example.py | 9 +- teuthology/task/pcp.py | 53 +++++----- teuthology/task/pexec.py | 19 ++-- teuthology/task/print.py | 2 +- teuthology/task/proc_thrasher.py | 13 +-- teuthology/task/selinux.py | 24 ++--- teuthology/task/sequential.py | 4 +- teuthology/task/sleep.py | 3 +- teuthology/task/ssh_keys.py | 21 ++-- teuthology/task/tasktest.py | 5 +- teuthology/task/timer.py | 5 +- teuthology/timer.py | 20 ++-- teuthology/util/flock.py | 8 +- teuthology/util/loggerfile.py | 4 +- teuthology/util/scanner.py | 4 +- teuthology/util/sentry.py | 3 +- teuthology/util/strtobool.py | 2 +- 106 files changed, 1192 insertions(+), 974 deletions(-) diff --git a/tests/orchestra/test_console.py b/tests/orchestra/test_console.py index fe0399b489..f7a9e7738a 100644 --- a/tests/orchestra/test_console.py +++ b/tests/orchestra/test_console.py @@ -155,7 +155,7 @@ def test_get_console_conserver(self): cons = self.klass(self.hostname) assert cons.has_conserver is True with patch( - 'teuthology.orchestra.console.pexpect.spawn', + 'teuthology.orchestra.console.LoggedPexpect', autospec=True, ) as m_spawn: cons._get_console() @@ -174,7 +174,7 @@ def test_get_console_ipmitool(self): cons = self.klass(self.hostname) assert cons.has_conserver is True with patch( - 'teuthology.orchestra.console.pexpect.spawn', + 'teuthology.orchestra.console.LoggedPexpect', autospec=True, ) as m_spawn: cons.has_conserver = False @@ -193,7 +193,7 @@ def test_get_console_fallback(self): cons = self.klass(self.hostname) assert cons.has_conserver is True with patch( - 'teuthology.orchestra.console.pexpect.spawn', + 'teuthology.orchestra.console.LoggedPexpect', autospec=True, ) as m_spawn: cons.has_conserver = True diff --git a/tests/orchestra/test_remote.py b/tests/orchestra/test_remote.py index 1328bc83e1..5d44b8bf72 100644 --- a/tests/orchestra/test_remote.py +++ b/tests/orchestra/test_remote.py @@ -151,7 +151,7 @@ def test_host_key(self): m_transport.get_remote_server_key.assert_called_once_with() def test_inventory_info(self): - r = remote.Remote('user@host', host_key='host_key') + r = remote.Remote('user@host', host_key='host_key', ssh=self.m_ssh) r._arch = 'arch' r._os = opsys.OS(name='os_name', version='1.2.3', codename='code') inv_info = r.inventory_info @@ -233,7 +233,7 @@ def test_resolve_ip(self, m_sh): except Exception as e: assert 'Cannot get IPv4 address' in str(e) try: - ip4 = remote.Remote.resolve_ip(r, 'smithi001', 5) + ip4 = remote.Remote.resolve_ip(r, 'smithi001', "5") except Exception as e: assert 'Unknown IP version' in str(e) diff --git a/tests/provision/cloud/test_openstack.py b/tests/provision/cloud/test_openstack.py index c1521054ae..7d4a9008f4 100644 --- a/tests/provision/cloud/test_openstack.py +++ b/tests/provision/cloud/test_openstack.py @@ -2,7 +2,7 @@ import yaml import os -from teuthology.util.compat import parse_qs +from urllib.parse import parse_qs from copy import deepcopy from libcloud.compute.providers import get_driver diff --git a/tests/test_timer.py b/tests/test_timer.py index 312a9c8b86..4a3c600bd9 100644 --- a/tests/test_timer.py +++ b/tests/test_timer.py @@ -56,7 +56,7 @@ def test_write(self): self.timer = timer.Timer(path=_path) assert self.timer.path == _path self.timer.write() - _open.assert_called_once_with(_path, 'w') + _open.assert_called_once_with(_path, mode='w') _safe_dump.assert_called_once_with( dict(), _open.return_value.__enter__.return_value, @@ -72,7 +72,7 @@ def test_sync(self): assert self.timer.path == _path assert self.timer.sync is True self.timer.mark() - _open.assert_called_once_with(_path, 'w') + _open.assert_called_once_with(_path, mode='w') _safe_dump.assert_called_once_with( self.timer.data, _open.return_value.__enter__.return_value, diff --git a/teuthology/__init__.py b/teuthology/__init__.py index 47baed8393..78f6c671ac 100644 --- a/teuthology/__init__.py +++ b/teuthology/__init__.py @@ -75,7 +75,7 @@ log.debug('teuthology version: %s', __version__) -def setup_log_file(log_path): +def setup_log_file(log_path: str) -> None: root_logger = logging.getLogger() handlers = root_logger.handlers for handler in handlers: @@ -93,12 +93,12 @@ def setup_log_file(log_path): root_logger.info('teuthology version: %s', __version__) -def install_except_hook(): +def install_except_hook() -> None: """ Install an exception hook that first logs any uncaught exception, then raises it. """ - def log_exception(exc_type, exc_value, exc_traceback): + def log_exception(exc_type, exc_value, exc_traceback) -> None: if not issubclass(exc_type, KeyboardInterrupt): log.critical("Uncaught exception", exc_info=(exc_type, exc_value, exc_traceback)) @@ -106,10 +106,10 @@ def log_exception(exc_type, exc_value, exc_traceback): sys.excepthook = log_exception -def patch_gevent_hub_error_handler(): +def patch_gevent_hub_error_handler() -> None: Hub._origin_handle_error = Hub.handle_error - def custom_handle_error(self, context, type, value, tb): + def custom_handle_error(self, context, type, value, tb) -> None: if context is None or issubclass(type, Hub.SYSTEM_ERROR): self.handle_system_error(type, value) elif issubclass(type, Hub.NOT_ERROR): diff --git a/teuthology/beanstalk.py b/teuthology/beanstalk.py index 76bc2c97ae..d7f07767e1 100644 --- a/teuthology/beanstalk.py +++ b/teuthology/beanstalk.py @@ -5,6 +5,7 @@ import pprint import sys from collections import OrderedDict +from typing import Optional from teuthology import report from teuthology.config import config @@ -12,7 +13,7 @@ log = logging.getLogger(__name__) -def connect(): +def connect() -> beanstalkc.Connection: host = config.queue_host port = config.queue_port if host is None or port is None: @@ -22,7 +23,7 @@ def connect(): return beanstalkc.Connection(host=host, port=port, parse_yaml=yaml.safe_load) -def watch_tube(connection, tube_name): +def watch_tube(connection: beanstalkc.Connection, tube_name: str) -> str: """ Watch a given tube, potentially correcting to 'multi' if necessary. Returns the tube_name that was actually used. @@ -35,7 +36,7 @@ def watch_tube(connection, tube_name): return tube_name -def walk_jobs(connection, tube_name, processor, pattern=None): +def walk_jobs(connection: beanstalkc.Connection, tube_name: str, processor: 'JobProcessor', pattern: Optional[str] = None) -> None: """ def callback(jobs_dict) """ @@ -62,26 +63,26 @@ def callback(jobs_dict) processor.complete() -def print_progress(index, total, message=None): +def print_progress(index: int, total: int, message: Optional[str] = None) -> None: msg = "{m} ".format(m=message) if message else '' sys.stderr.write("{msg}{i}/{total}\r".format( msg=msg, i=index, total=total)) sys.stderr.flush() -def end_progress(): +def end_progress() -> None: sys.stderr.write('\n') sys.stderr.flush() class JobProcessor(object): - def __init__(self): + def __init__(self) -> None: self.jobs = OrderedDict() - def add_job(self, job_id, job_config, job_obj=None): + def add_job(self, job_id: int | str, job_config: dict, job_obj: Optional[beanstalkc.Job] = None) -> None: job_id = str(job_id) - job_dict = dict( + job_dict: dict = dict( index=(len(self.jobs) + 1), job_config=job_config, ) @@ -91,20 +92,20 @@ def add_job(self, job_id, job_config, job_obj=None): self.process_job(job_id) - def process_job(self, job_id): + def process_job(self, job_id: str) -> None: pass - def complete(self): + def complete(self) -> None: pass class JobPrinter(JobProcessor): - def __init__(self, show_desc=False, full=False): + def __init__(self, show_desc: bool = False, full: bool = False) -> None: super(JobPrinter, self).__init__() self.show_desc = show_desc self.full = full - def process_job(self, job_id): + def process_job(self, job_id: str) -> None: job_config = self.jobs[job_id]['job_config'] job_index = self.jobs[job_id]['index'] job_priority = job_config['priority'] @@ -124,11 +125,11 @@ def process_job(self, job_id): class RunPrinter(JobProcessor): - def __init__(self): + def __init__(self) -> None: super(RunPrinter, self).__init__() self.runs = list() - def process_job(self, job_id): + def process_job(self, job_id: str) -> None: run = self.jobs[job_id]['job_config']['name'] if run not in self.runs: self.runs.append(run) @@ -136,16 +137,16 @@ def process_job(self, job_id): class JobDeleter(JobProcessor): - def __init__(self, pattern): + def __init__(self, pattern: str) -> None: self.pattern = pattern super(JobDeleter, self).__init__() - def add_job(self, job_id, job_config, job_obj=None): + def add_job(self, job_id: int | str, job_config: dict, job_obj: Optional[beanstalkc.Job] = None) -> None: job_name = job_config['name'] if self.pattern in job_name: super(JobDeleter, self).add_job(job_id, job_config, job_obj) - def process_job(self, job_id): + def process_job(self, job_id: str) -> None: job_config = self.jobs[job_id]['job_config'] job_name = job_config['name'] print('Deleting {job_name}/{job_id}'.format( @@ -158,7 +159,7 @@ def process_job(self, job_id): report.try_delete_jobs(job_name, job_id) -def pause_tube(connection, tube, duration): +def pause_tube(connection: beanstalkc.Connection, tube: Optional[str], duration: int | str) -> None: duration = int(duration) if not tube: tubes = sorted(connection.tubes()) @@ -172,7 +173,7 @@ def pause_tube(connection, tube, duration): connection.pause_tube(tube, duration) -def stats_tube(connection, tube): +def stats_tube(connection: beanstalkc.Connection, tube: str) -> dict: stats = connection.stats_tube(tube) result = dict( name=tube, @@ -182,7 +183,7 @@ def stats_tube(connection, tube): return result -def main(args): +def main(args: dict) -> None: machine_type = args['--machine_type'] status = args['--status'] delete = args['--delete'] diff --git a/teuthology/config.py b/teuthology/config.py index 7e51b3dde9..2ebff09d51 100644 --- a/teuthology/config.py +++ b/teuthology/config.py @@ -1,6 +1,7 @@ import os import yaml import logging +from typing import Optional, Iterator try: from collections.abc import MutableMapping except ImportError: @@ -13,7 +14,7 @@ CONFIG_PATH_VAR_NAME = 'TEUTHOLOGY_CONFIG' # name of env var to check -def init_logging(): +def init_logging() -> logging.Logger: log = logging.getLogger(__name__) return log @@ -31,14 +32,14 @@ class YamlConfig(MutableMapping): """ _defaults = dict() - def __init__(self, yaml_path=None): + def __init__(self, yaml_path: Optional[str] = None) -> None: self.yaml_path = yaml_path if self.yaml_path: self.load() else: self._conf = dict() - def load(self, conf=None): + def load(self, conf: Optional[dict|str] = None) -> None: if conf is not None: if isinstance(conf, dict): self._conf = conf @@ -46,14 +47,14 @@ def load(self, conf=None): elif conf: self._conf = yaml.safe_load(conf) return - if os.path.exists(self.yaml_path): + if self.yaml_path and os.path.exists(self.yaml_path): with open(self.yaml_path) as f: self._conf = yaml.safe_load(f) else: log.debug("%s not found", self.yaml_path) self._conf = dict() - def update(self, in_dict): + def update(self, in_dict: dict) -> None: # ty: ignore[invalid-method-override] """ Update an existing configuration using dict.update() @@ -62,7 +63,7 @@ def update(self, in_dict): self._conf.update(in_dict) @classmethod - def from_dict(cls, in_dict): + def from_dict(cls, in_dict: dict) -> 'YamlConfig': """ Build a config object from a dict. @@ -73,14 +74,14 @@ def from_dict(cls, in_dict): conf_obj._conf = in_dict return conf_obj - def to_dict(self): + def to_dict(self) -> dict: """ :returns: A shallow copy of the configuration as a dict """ return dict(self._conf) @classmethod - def from_str(cls, in_str): + def from_str(cls, in_str: str) -> 'YamlConfig': """ Build a config object from a string or yaml stream. @@ -91,49 +92,49 @@ def from_str(cls, in_str): conf_obj._conf = yaml.safe_load(in_str) return conf_obj - def to_str(self): + def to_str(self) -> str: """ :returns: str(self) """ return str(self) - def get(self, key, default=None): + def get(self, key: str, default = None): return self._conf.get(key, default) - def __str__(self): + def __str__(self) -> str: return yaml.safe_dump(self._conf, default_flow_style=False).strip() - def __repr__(self): + def __repr__(self) -> str: return self.__str__() - def __getitem__(self, name): + def __getitem__(self, name: str): return self.__getattr__(name) - def __getattr__(self, name): + def __getattr__(self, name: str): return self._conf.get(name, self._defaults.get(name)) - def __contains__(self, name): + def __contains__(self, name: object) -> bool: return self._conf.__contains__(name) - def __setattr__(self, name, value): + def __setattr__(self, name: str, value) -> None: if name.endswith('_conf') or name in ('yaml_path'): object.__setattr__(self, name, value) else: self._conf[name] = value - def __delattr__(self, name): + def __delattr__(self, name: str) -> None: del self._conf[name] - def __len__(self): + def __len__(self) -> int: return self._conf.__len__() - def __iter__(self): + def __iter__(self) -> Iterator[str]: return self._conf.__iter__() - def __setitem__(self, name, value): + def __setitem__(self, name: str, value) -> None: self._conf.__setitem__(name, value) - def __delitem__(self, name): + def __delitem__(self, name: str) -> None: self._conf.__delitem__(name) @@ -209,22 +210,22 @@ class TeuthologyConfig(YamlConfig): 'active_machine_types': [], } - def __init__(self, yaml_path=None): + def __init__(self, yaml_path: Optional[str] = None) -> None: super(TeuthologyConfig, self).__init__(yaml_path or self.yaml_path) - def get_ceph_cm_ansible_git_url(self): + def get_ceph_cm_ansible_git_url(self) -> str: return (self.ceph_cm_ansible_git_url or self.ceph_git_base_url + 'ceph-cm-ansible.git') - def get_ceph_qa_suite_git_url(self): + def get_ceph_qa_suite_git_url(self) -> str: return (self.ceph_qa_suite_git_url or self.get_ceph_git_url()) - def get_ceph_git_url(self): + def get_ceph_git_url(self) -> str: return (self.ceph_git_url or self.ceph_git_base_url + 'ceph-ci.git') - def get_teuthology_git_url(self): + def get_teuthology_git_url(self) -> str: return (self.teuthology_git_url or self.ceph_git_base_url + 'teuthology.git') @@ -240,13 +241,13 @@ class FakeNamespace(YamlConfig): We'll use this as a stop-gap as we refactor commands but allow the tasks to still be passed a single namespace object for the time being. """ - def __init__(self, config_dict=None): + def __init__(self, config_dict: Optional[dict] = None) -> None: if not config_dict: config_dict = dict() self._conf = self._clean_config(config_dict) set_config_attr(self) - def _clean_config(self, config_dict): + def _clean_config(self, config_dict: dict) -> dict: """ Makes sure that the keys of config_dict are able to be used. For example the "--" prefix of a docopt dict isn't valid and won't populate @@ -267,7 +268,7 @@ def _clean_config(self, config_dict): return result - def __getattr__(self, name): + def __getattr__(self, name: str): """ We need to modify this for FakeNamespace so that getattr() will work correctly on a FakeNamespace instance. @@ -278,27 +279,27 @@ def __getattr__(self, name): return self._defaults[name] raise AttributeError(name) - def __setattr__(self, name, value): + def __setattr__(self, name: str, value) -> None: if name == 'teuthology_config': object.__setattr__(self, name, value) else: super(FakeNamespace, self).__setattr__(name, value) - def __repr__(self): + def __repr__(self) -> str: return repr(self._conf) - def __str__(self): + def __str__(self) -> str: return str(self._conf) -def set_config_attr(obj): +def set_config_attr(obj) -> None: """ Set obj.teuthology_config, mimicking the old behavior of misc.read_config """ obj.teuthology_config = config -def _get_config_path(): +def _get_config_path() -> Optional[str]: """Look for a teuthology config yaml and return it's path. Raises ValueError if no config yaml can be found. """ diff --git a/teuthology/contextutil.py b/teuthology/contextutil.py index 8e53e5439b..d6a76a9481 100644 --- a/teuthology/contextutil.py +++ b/teuthology/contextutil.py @@ -2,6 +2,8 @@ import sys import logging import time +from typing import Callable, Generator, Optional +from types import TracebackType from teuthology.config import config from teuthology.exceptions import MaxWhileTries @@ -10,7 +12,7 @@ log = logging.getLogger(__name__) @contextlib.contextmanager -def nested(*managers): +def nested(*managers: Callable[[], contextlib.AbstractContextManager]) -> Generator[list, None, None]: """ Like contextlib.nested but takes callables returning context managers, to avoid the major reason why contextlib.nested was @@ -90,8 +92,8 @@ class safe_while(object): Default time.sleep """ - def __init__(self, sleep=6, increment=0, tries=10, timeout=0, action=None, - _raise=True, _sleeper=None): + def __init__(self, sleep: int = 6, increment: float = 0, tries: int = 10, timeout: int = 0, action: Optional[str] = None, + _raise: bool = True, _sleeper: Optional[Callable[[float], None]] = None) -> None: self.sleep = sleep self.increment = increment self.tries = tries @@ -103,7 +105,7 @@ def __init__(self, sleep=6, increment=0, tries=10, timeout=0, action=None, self.sleeper = _sleeper or time.sleep self.total_seconds = sleep - def _make_error_msg(self): + def _make_error_msg(self) -> str: """ Sum the total number of seconds we waited while providing the number of tries we attempted @@ -120,11 +122,11 @@ def _make_error_msg(self): ) return msg - def __call__(self): + def __call__(self) -> bool: self.counter += 1 if self.counter == 1: return True - def must_stop(): + def must_stop() -> bool: return self.tries > 0 and self.counter > self.tries if ((self.timeout > 0 and self.total_seconds >= self.timeout) or @@ -142,8 +144,8 @@ def must_stop(): self.sleeper(self.sleep_current) return True - def __enter__(self): + def __enter__(self) -> 'safe_while': return self - def __exit__(self, exc_type, exc_val, exc_tb): + def __exit__(self, exc_type: Optional[type[BaseException]], exc_val: Optional[BaseException], exc_tb: Optional[TracebackType]) -> bool: return False diff --git a/teuthology/describe_tests.py b/teuthology/describe_tests.py index 80c6f528db..087090a1e4 100644 --- a/teuthology/describe_tests.py +++ b/teuthology/describe_tests.py @@ -110,7 +110,7 @@ def output_results(headers, rows, output_format, hrule): def output_summary(path, limit=0, seed=None, subset=None, - no_nested_subset=None, + no_nested_subset=False, show_desc=True, show_frag=False, show_matrix=False, diff --git a/teuthology/dispatcher/__init__.py b/teuthology/dispatcher/__init__.py index d4d36dde4f..0703b7f44f 100644 --- a/teuthology/dispatcher/__init__.py +++ b/teuthology/dispatcher/__init__.py @@ -7,13 +7,13 @@ import sys import yaml -from typing import Dict, List +from typing import Dict, List, Optional try: from gevent.exceptions import LoopExit except ImportError: # gevent might not be available in some environments - LoopExit = Exception + LoopExit: type[Exception] = Exception from teuthology import ( # non-modules @@ -39,7 +39,7 @@ def stop(_sig, _frame) -> None: sys.exit(0) -def load_config(archive_dir=None): +def load_config(archive_dir: Optional[str] = None) -> None: teuth_config.load() if archive_dir is not None: if not os.path.isdir(archive_dir): @@ -51,7 +51,7 @@ def load_config(archive_dir=None): teuth_config.archive_base = archive_dir -def main(args): +def main(args) -> int: signal.signal(signal.SIGTERM, stop) archive_dir = args.archive_dir or teuth_config.archive_base diff --git a/teuthology/dispatcher/supervisor.py b/teuthology/dispatcher/supervisor.py index 7f5b33eeee..ecabcd6b4d 100644 --- a/teuthology/dispatcher/supervisor.py +++ b/teuthology/dispatcher/supervisor.py @@ -123,7 +123,7 @@ def run_job(job_config: dict, for f in os.listdir(job_config['archive_path']): os.remove(os.path.join(job_config['archive_path'], f)) os.rmdir(job_config['archive_path']) - return + return 0 log.info('Running job %s', job_config['job_id']) diff --git a/teuthology/exceptions.py b/teuthology/exceptions.py index da38343545..b67890a0aa 100644 --- a/teuthology/exceptions.py +++ b/teuthology/exceptions.py @@ -1,9 +1,12 @@ +from typing import Optional + + class BranchNotFoundError(ValueError): - def __init__(self, branch, repo=None): + def __init__(self, branch: str, repo: Optional[str] = None) -> None: self.branch = branch self.repo = repo - def __str__(self): + def __str__(self) -> str: if self.repo: repo_str = " in repo: %s" % self.repo else: @@ -13,23 +16,23 @@ def __str__(self): class BranchMismatchError(ValueError): - def __init__(self, branch, repo, reason=None): + def __init__(self, branch: str, repo: str, reason: Optional[str] = None) -> None: self.branch = branch self.repo = repo self.reason = reason - def __str__(self): + def __str__(self) -> str: msg = f"Cannot use branch {self.branch} with repo {self.repo}" if self.reason: msg = f"{msg} because {self.reason}" return msg class CommitNotFoundError(ValueError): - def __init__(self, commit, repo=None): + def __init__(self, commit: str, repo: Optional[str] = None) -> None: self.commit = commit self.repo = repo - def __str__(self): + def __str__(self) -> str: if self.repo: repo_str = " in repo: %s" % self.repo else: @@ -62,13 +65,13 @@ class CommandFailedError(Exception): """ Exception thrown on command failure """ - def __init__(self, command, exitstatus, node=None, label=None): + def __init__(self, command: str, exitstatus: int, node: Optional[str] = None, label: Optional[str] = None) -> None: self.command = command self.exitstatus = exitstatus self.node = node self.label = label - def __str__(self): + def __str__(self) -> str: prefix = "Command failed" if self.label: prefix += " ({label})".format(label=self.label) @@ -80,7 +83,7 @@ def __str__(self): prefix=prefix, ) - def fingerprint(self): + def fingerprint(self) -> list[str]: """ Returns a list of strings to group failures with. Used by sentry instead of grouping by backtrace. @@ -97,15 +100,15 @@ class AnsibleFailedError(Exception): """ Exception thrown when an ansible playbook fails """ - def __init__(self, failures): + def __init__(self, failures: list[str]) -> None: self.failures = failures - def __str__(self): + def __str__(self) -> str: return "{failures}".format( failures=self.failures, ) - def fingerprint(self): + def fingerprint(self) -> list[str]: """ Sentry will use this to group events by their failure reasons, rather than lumping all AnsibleFailedErrors together @@ -118,10 +121,10 @@ class CommandCrashedError(Exception): """ Exception thrown on crash """ - def __init__(self, command): + def __init__(self, command: str) -> None: self.command = command - def __str__(self): + def __str__(self) -> str: return "Command crashed: {command!r}".format( command=self.command, ) @@ -132,11 +135,11 @@ class ConnectionLostError(Exception): """ Exception thrown when the connection is lost """ - def __init__(self, command, node=None): + def __init__(self, command: str, node: Optional[str] = None) -> None: self.command = command self.node = node - def __str__(self): + def __str__(self) -> str: node_str = 'to %s ' % self.node if self.node else '' return "SSH connection {node_str}was lost: {command!r}".format( node_str=node_str, @@ -145,11 +148,11 @@ def __str__(self): class ScheduleFailError(RuntimeError): - def __init__(self, message, name=None): + def __init__(self, message: str, name: Optional[str] = None) -> None: self.message = message self.name = name - def __str__(self): + def __str__(self) -> str: return "Scheduling {name} failed: {msg}".format( name=self.name, msg=self.message, @@ -157,37 +160,37 @@ def __str__(self): class VersionNotFoundError(Exception): - def __init__(self, url): + def __init__(self, url: str) -> None: self.url = url - def __str__(self): + def __str__(self) -> str: return "Failed to fetch package version from %s" % self.url class UnsupportedPackageTypeError(Exception): - def __init__(self, node): + def __init__(self, node) -> None: self.node = node - def __str__(self): + def __str__(self) -> str: return "os.package_type {pkg_type!r} on {node}".format( node=self.node, pkg_type=self.node.os.package_type) class SELinuxError(Exception): - def __init__(self, node, denials): + def __init__(self, node, denials: list[str]) -> None: self.node = node self.denials = denials - def __str__(self): + def __str__(self) -> str: return "SELinux denials found on {node}: {denials}".format( node=self.node, denials=self.denials) class QuotaExceededError(Exception): - def __init__(self, message): + def __init__(self, message: str) -> None: self.message = message - def __str__(self): + def __str__(self) -> str: return self.message @@ -210,7 +213,7 @@ class ConsoleError(Exception): class NoRemoteError(Exception): message = "This operation requires a remote" - def __str__(self): + def __str__(self) -> str: return self.message @@ -218,13 +221,13 @@ class UnitTestError(Exception): """ Exception thrown on unit test failure """ - def __init__(self, exitstatus=None, node=None, label=None, message=None): + def __init__(self, exitstatus: Optional[int] = None, node: Optional[str] = None, label: Optional[str] = None, message: Optional[str] = None) -> None: self.exitstatus = exitstatus self.node = node self.label = label self.message = message - def __str__(self): + def __str__(self) -> str: prefix = "Unit test failed" if self.label: prefix += " ({label})".format(label=self.label) diff --git a/teuthology/exit.py b/teuthology/exit.py index 266e988eba..a07985efd1 100644 --- a/teuthology/exit.py +++ b/teuthology/exit.py @@ -1,6 +1,8 @@ import logging import os import signal +from typing import Callable, Optional +from types import FrameType log = logging.getLogger(__name__) @@ -11,10 +13,10 @@ class Exiter(object): A helper to manage any signal handlers we need to call upon receiving a given signal """ - def __init__(self): + def __init__(self) -> None: self.handlers = list() - def add_handler(self, signals, func): + def add_handler(self, signals: int | list[int], func: Callable[[int, Optional[FrameType]], None]) -> 'Handler': """ Adds a handler function to be called when any of the given signals are received. @@ -37,7 +39,7 @@ def add_handler(self, signals, func): self.handlers.append(handler) return handler - def default_handler(self, signal_, frame): + def default_handler(self, signal_: int, frame: Optional[FrameType]) -> None: log.debug( "Got signal %s; running %s handler%s...", signal_, @@ -54,19 +56,19 @@ def default_handler(self, signal_, frame): class Handler(object): - def __init__(self, exiter, func, signals): + def __init__(self, exiter: Exiter, func: Callable[[int, Optional[FrameType]], None], signals: list[int]) -> None: self.exiter = exiter self.func = func self.signals = signals - def remove(self): + def remove(self) -> None: try: log.debug("Removing handler: %s", self) self.exiter.handlers.remove(self) except ValueError: pass - def __repr__(self): + def __repr__(self) -> str: return "{c}(exiter={e}, func={f}, signals={s})".format( c=self.__class__.__name__, e=self.exiter, diff --git a/teuthology/job_status.py b/teuthology/job_status.py index 05ff80d715..d6091b16a6 100644 --- a/teuthology/job_status.py +++ b/teuthology/job_status.py @@ -1,4 +1,7 @@ -def get_status(summary): +from typing import Optional + + +def get_status(summary: dict) -> Optional[str]: """ :param summary: The job summary dict. Normally ctx.summary :returns: A status string like 'pass', 'fail', or 'dead' @@ -17,7 +20,7 @@ def get_status(summary): return status -def set_status(summary, status): +def set_status(summary: dict, status: Optional[str]) -> None: """ Sets summary['status'] to status, and summary['success'] to True if status is 'pass'. If status is not 'pass', then 'success' is False. diff --git a/teuthology/lock/cli.py b/teuthology/lock/cli.py index ed4f28bd5a..95e87305d1 100644 --- a/teuthology/lock/cli.py +++ b/teuthology/lock/cli.py @@ -22,7 +22,7 @@ log = logging.getLogger(__name__) -def main(ctx): +def main(ctx) -> int: if ctx.verbose: teuthology.log.setLevel(logging.DEBUG) @@ -251,7 +251,7 @@ def main(ctx): return ret -def do_summary(ctx): +def do_summary(ctx) -> None: lockd = collections.defaultdict(lambda: [0, 0, 'unknown']) if ctx.machine_type: locks = query.list_locks(machine_type=ctx.machine_type) @@ -282,7 +282,7 @@ def do_summary(ctx): print("{cnt:12d} {up:3d}".format(cnt=total_count, up=total_up)) -def updatekeys(args): +def updatekeys(args: dict) -> int: loglevel = logging.DEBUG if args['--verbose'] else logging.INFO logging.basicConfig( level=loglevel, diff --git a/teuthology/lock/ops.py b/teuthology/lock/ops.py index 9cccd758c9..c3e8927768 100644 --- a/teuthology/lock/ops.py +++ b/teuthology/lock/ops.py @@ -6,7 +6,7 @@ import yaml import requests -from typing import List, Union +from typing import Dict, List, Optional, Union import teuthology.orchestra.remote import teuthology.parallel @@ -25,7 +25,7 @@ log = logging.getLogger(__name__) -def update_nodes(nodes, reset_os=False): +def update_nodes(nodes: List[str], reset_os: bool = False) -> None: for node in nodes: remote = teuthology.orchestra.remote.Remote( canonicalize_hostname(node)) @@ -41,8 +41,8 @@ def update_nodes(nodes, reset_os=False): update_inventory(inventory_info) -def lock_many_openstack(ctx, num, machine_type, user=None, description=None, - arch=None): +def lock_many_openstack(ctx, num: int, user: Optional[str] = None, + description: Optional[str] = None, arch: Optional[str] = None) -> Dict[str, None]: os_type = teuthology.provision.get_distro(ctx) os_version = teuthology.provision.get_distro_version(ctx) if hasattr(ctx, 'config'): @@ -50,7 +50,7 @@ def lock_many_openstack(ctx, num, machine_type, user=None, description=None, else: resources_hint = None machines = teuthology.provision.openstack.ProvisionOpenStack().create( - num, os_type, os_version, arch, resources_hint) + num, os_type, os_version, arch, resources_hint or {}) result = {} for machine in machines: lock_one(machine, user, description) @@ -58,8 +58,10 @@ def lock_many_openstack(ctx, num, machine_type, user=None, description=None, return result -def lock_many(ctx, num, machine_type, user=None, description=None, - os_type=None, os_version=None, arch=None, reimage=True): +def lock_many(ctx, num: int, machine_type: str, user: Optional[str] = None, + description: Optional[str] = None, os_type: Optional[str] = None, + os_version: Optional[str] = None, arch: Optional[str] = None, + reimage: bool = True) -> Union[Dict[str, str], Dict[str, None], list, None]: if user is None: user = misc.get_user() @@ -80,10 +82,7 @@ def lock_many(ctx, num, machine_type, user=None, description=None, if all(t in downburst_types for t in machine_types_list): machine_types = machine_types_list elif machine_types_list == ['openstack']: - return lock_many_openstack(ctx, num, machine_type, - user=user, - description=description, - arch=arch) + return lock_many_openstack(ctx, num, user=user, description=description, arch=arch) elif any(t in downburst_types for t in machine_types_list): the_vps = list(t for t in machine_types_list if t in downburst_types) @@ -131,7 +130,7 @@ def lock_many(ctx, num, machine_type, user=None, description=None, machines=', '.join(machines.keys()))) if machine_type in vm_types: ok_machs = {} - update_nodes(machines, True) + update_nodes(list(machines.keys()), True) for machine in machines: if teuthology.provision.create_if_vm(ctx, machine): ok_machs[machine] = machines[machine] @@ -140,14 +139,14 @@ def lock_many(ctx, num, machine_type, user=None, description=None, machine) unlock_one(machine, user) ok_machs = do_update_keys(list(ok_machs.keys()))[1] - update_nodes(ok_machs) + update_nodes(list(ok_machs.keys())) return ok_machs elif reimage and machine_type in reimage_types: try: return reimage_machines(ctx, machines, machine_type) except Exception: log.exception('Reimaging error. Unlocking machines...') - unlock_many(machines, user) + unlock_many(list(machines.keys()), user) continue return machines elif response.status_code == 503: @@ -160,7 +159,7 @@ def lock_many(ctx, num, machine_type, user=None, description=None, return [] -def lock_one(name, user=None, description=None): +def lock_one(name: str, user: Optional[str] = None, description: Optional[str] = None) -> requests.Response: name = misc.canonicalize_hostname(name, user=None) if user is None: user = misc.get_user() @@ -210,7 +209,7 @@ def unlock_one_safe(name: str, owner: str, run_name: str = "", job_id: str = "") return False -def unlock_many(names, user): +def unlock_many(names: list[str], user: str) -> bool: fixed_names = [misc.canonicalize_hostname(name, user=None) for name in names] names = fixed_names @@ -242,9 +241,9 @@ def unlock_one(name, user, description=None, status: Union[dict, None] = None) - log.error('destroy failed for %s', name) return False # we're trying to stop node before actual unlocking - status_info = teuthology.lock.query.get_status(name) + status_info = query.get_status(name) try: - if not teuthology.lock.query.is_vm(status=status_info): + if not query.is_vm(status=status_info): stop_node(name, status) except Exception: log.exception(f"Failed to stop {name}!") @@ -273,7 +272,7 @@ def unlock_one(name, user, description=None, status: Union[dict, None] = None) - return False -def update_lock(name, description=None, status=None, ssh_pub_key=None): +def update_lock(name: str, description: Optional[str] = None, status: Optional[str] = None, ssh_pub_key: Optional[str] = None) -> bool: name = misc.canonicalize_hostname(name, user=None) updated = {} if description is not None: @@ -298,7 +297,7 @@ def update_lock(name, description=None, status=None, ssh_pub_key=None): return True -def update_inventory(node_dict): +def update_inventory(node_dict: dict) -> None: """ Like update_lock(), but takes a dict and doesn't try to do anything smart by itself @@ -330,15 +329,18 @@ def update_inventory(node_dict): if response.ok: return -def do_update_keys(machines, all_=False, _raise=True): - reference = query.list_locks(keyed_by_name=True) +def do_update_keys(machines: list[str], all_: bool = False, _raise: bool = True) -> tuple[int, dict[str, str]]: + reference = {node['name']: node for node in query.list_locks()} if all_: - machines = reference.keys() + if isinstance(reference, dict): + machines = list(reference.keys()) + else: + machines = [] keys_dict = misc.ssh_keyscan(machines, _raise=_raise) return push_new_keys(keys_dict, reference), keys_dict -def push_new_keys(keys_dict, reference): +def push_new_keys(keys_dict: dict[str, str], reference: dict[str, dict]) -> int: ret = 0 for hostname, pubkey in keys_dict.items(): log.info('Checking %s', hostname) @@ -350,7 +352,7 @@ def push_new_keys(keys_dict, reference): return ret -def reimage_machines(ctx, machines, machine_type): +def reimage_machines(ctx, machines: dict, machine_type: str) -> dict[str, str]: reimage_types = teuthology.provision.get_reimage_types() if machine_type not in reimage_types: log.info(f"Skipping reimage of {machines.keys()} because {machine_type} is not in {reimage_types}") @@ -371,11 +373,11 @@ def reimage_machines(ctx, machines, machine_type): machine, machine_type) reimaged[machine] = machines[machine] reimaged = do_update_keys(list(reimaged.keys()))[1] - update_nodes(reimaged) + update_nodes(list(reimaged.keys())) return reimaged -def block_and_lock_machines(ctx, total_requested, machine_type, reimage=True, tries=10): +def block_and_lock_machines(ctx, total_requested: int, machine_type: str, reimage: bool = True, tries: int = 10) -> None: # It's OK for os_type and os_version to be None here. If we're trying # to lock a bare metal machine, we'll take whatever is available. If # we want a vps, defaults will be provided by misc.get_distro and @@ -436,11 +438,12 @@ def block_and_lock_machines(ctx, total_requested, machine_type, reimage=True, tr if 'summary' in ctx: set_status(ctx.summary, 'dead') raise - all_locked.update(newly_locked) + if newly_locked: + all_locked.update(newly_locked) log.info( '{newly_locked} {mtype} machines locked this try, ' '{total_locked}/{total_requested} locked so far'.format( - newly_locked=len(newly_locked), + newly_locked=len(newly_locked) if newly_locked else 0, mtype=machine_type, total_locked=len(all_locked), total_requested=total_requested, @@ -469,7 +472,7 @@ def block_and_lock_machines(ctx, total_requested, machine_type, reimage=True, tr full_name = misc.canonicalize_hostname(guest) teuthology.provision.destroy_if_vm(full_name) teuthology.provision.create_if_vm(ctx, full_name) - if do_update_keys(keys_dict)[0]: + if do_update_keys(list(keys_dict.keys()))[0]: log.info("Error in virtual machine keys") newscandict = {} for dkey in all_locked.keys(): @@ -489,23 +492,24 @@ def block_and_lock_machines(ctx, total_requested, machine_type, reimage=True, tr elif not ctx.block: assert 0, 'not enough machines are available' else: - requested = requested - len(newly_locked) + requested = requested - (len(newly_locked) if newly_locked else 0) assert requested > 0, "lock_machines: requested counter went" \ "negative, this shouldn't happen" log.info( "{total} machines locked ({new} new); need {more} more".format( - total=len(all_locked), new=len(newly_locked), more=requested) + total=len(all_locked), new=len(newly_locked) if newly_locked else 0, more=requested) ) log.warning('Could not lock enough machines, waiting...') time.sleep(10) -def stop_node(name: str, status: Union[dict, None]): +def stop_node(name: str, status: Union[dict, None]) -> None: status = status or query.get_status(name) remote_ = remote.Remote(misc.canonicalize_hostname(name)) if status['machine_type'] in provision.fog.get_types(): - remote_.console.power_off() + if remote_.console: + remote_.console.power_off() return elif status['machine_type'] in provision.pelagos.get_types(): provision.pelagos.park_node(name) diff --git a/teuthology/lock/query.py b/teuthology/lock/query.py index 506ad56f9c..8af7493d01 100644 --- a/teuthology/lock/query.py +++ b/teuthology/lock/query.py @@ -3,7 +3,7 @@ import os import requests -from typing import Dict, List, Union +from typing import Dict, List, Optional, Union from teuthology import misc from teuthology.config import config @@ -31,23 +31,20 @@ def get_status(name) -> dict: return dict() -def get_statuses(machines): - if machines: - statuses = [] - for machine in machines: - machine = misc.canonicalize_hostname(machine) - status = get_status(machine) - if status: - statuses.append(status) - else: - log.error("Lockserver doesn't know about machine: %s" % - machine) - else: - statuses = list_locks() +def get_statuses(machines: list[str]) -> list[dict]: + statuses: list[dict] = [] + for machine in machines: + machine = misc.canonicalize_hostname(machine) + status = get_status(machine) + if status: + statuses.append(status) + else: + log.error("Lockserver doesn't know about machine: %s" % + machine) return statuses -def is_vm(name=None, status=None): +def is_vm(name: Optional[str] = None, status: Optional[dict] = None) -> bool: if status is None: if name is None: raise ValueError("Must provide either name or status, or both") @@ -56,12 +53,12 @@ def is_vm(name=None, status=None): return status.get('is_vm', False) -def list_locks(keyed_by_name=False, tries=10, **kwargs): +def list_locks(tries: int = 10, **kwargs) -> list[dict]: uri = os.path.join(config.lock_server, 'nodes', '') for key, value in kwargs.items(): - if kwargs[key] is False: + if value is False: kwargs[key] = '0' - if kwargs[key] is True: + if value is True: kwargs[key] = '1' if kwargs: if machine_type := kwargs.get("machine_type"): @@ -83,12 +80,8 @@ def list_locks(keyed_by_name=False, tries=10, **kwargs): except requests.ConnectionError: log.exception("Could not contact lock server: %s, retrying...", config.lock_server) if response.ok: - if not keyed_by_name: - return response.json() - else: - return {node['name']: node - for node in response.json()} - return dict() + return response.json() + return [] def find_stale_locks(owner: str | None = None, machine_type: str | None = None) -> List[Dict]: @@ -159,7 +152,6 @@ def node_active_job(name: str, status: Union[dict, None] = None, grace_time: int # We thought this node might have a stale job, but no. return "node description does not contained scheduled job info" url = f"{config.results_server}/runs/{run_name}/jobs/{job_id}/" - job_status = "" active = True with safe_while( sleep=1, increment=0.5, action='node_is_active') as proceed: diff --git a/teuthology/lock/util.py b/teuthology/lock/util.py index 955b97e43d..7e9e18428e 100644 --- a/teuthology/lock/util.py +++ b/teuthology/lock/util.py @@ -1,6 +1,7 @@ import datetime import json import logging +from typing import Callable, Optional from teuthology import misc import teuthology.provision.downburst @@ -8,7 +9,7 @@ log = logging.getLogger(__name__) -def vps_version_or_type_valid(machine_type, os_type, os_version): +def vps_version_or_type_valid(machine_type: str, os_type: Optional[str], os_version: Optional[str]) -> bool: """ Check os-type and os-version parameters when locking a vps. Os-type will always be set (defaults to ubuntu). @@ -42,7 +43,7 @@ def vps_version_or_type_valid(machine_type, os_type, os_version): return True -def validate_distro_version(version, supported_versions): +def validate_distro_version(version: str, supported_versions: list[str]) -> Optional[bool]: """ Return True if the version is valid. For Ubuntu, possible supported version values are of the form '12.04 (precise)' where @@ -59,7 +60,7 @@ def validate_distro_version(version, supported_versions): return True -def json_matching_statuses(json_file_or_str, statuses): +def json_matching_statuses(json_file_or_str: str, statuses: list[dict]) -> list[dict]: """ Filter statuses by json dict in file or fragment; return list of matching statuses. json_file_or_str must be a file containing @@ -69,8 +70,6 @@ def json_matching_statuses(json_file_or_str, statuses): open(json_file_or_str, 'r') except IOError: query = json.loads(json_file_or_str) - else: - query = json.load(json_file_or_str) if not isinstance(query, dict): raise RuntimeError('--json-query must be a dict') @@ -86,7 +85,7 @@ def json_matching_statuses(json_file_or_str, statuses): return return_statuses -def winnow(statuses, arg, status_key, func=None): +def winnow(statuses: list[dict], arg, status_key: str, func: Optional[Callable[[dict], bool]] = None) -> list[dict]: """ Call with a list of statuses, and the ctx. 'arg' that you may want to filter by. @@ -112,7 +111,7 @@ def winnow(statuses, arg, status_key, func=None): return statuses -def locked_since_seconds(node): +def locked_since_seconds(node: dict) -> float: now = datetime.datetime.now() since = datetime.datetime.strptime( node['locked_since'], '%Y-%m-%d %H:%M:%S.%f') diff --git a/teuthology/misc.py b/teuthology/misc.py index 1cd00d10ea..e3ef5e5532 100644 --- a/teuthology/misc.py +++ b/teuthology/misc.py @@ -45,7 +45,7 @@ hostname_expr_templ = '(?P.*@)?(?P.*){lab_domain}' -def host_shortname(hostname): +def host_shortname(hostname: str) -> str: if _is_ipv4(hostname) or _is_ipv6(hostname): return hostname else: @@ -80,7 +80,7 @@ def canonicalize_hostname(hostname, user: Optional[str] ='ubuntu'): return ret -def decanonicalize_hostname(hostname): +def decanonicalize_hostname(hostname: str) -> str: lab_domain = '' if config.lab_domain: lab_domain=r'\.' + config.lab_domain.replace('.', r'\.') @@ -91,7 +91,7 @@ def decanonicalize_hostname(hostname): return hostname -def config_file(string): +def config_file(string: str) -> dict: """ Create a config file diff --git a/teuthology/nuke/__init__.py b/teuthology/nuke/__init__.py index 9c6eefe18f..535ccf34d6 100644 --- a/teuthology/nuke/__init__.py +++ b/teuthology/nuke/__init__.py @@ -4,7 +4,7 @@ # This is being kept because ceph.git/qa/tasks/cephfs/filesystem.py references it. -def clear_firewall(ctx): +def clear_firewall(ctx) -> None: """ Remove any iptables rules created by teuthology. These rules are identified by containing a comment with 'teuthology' in it. Non-teuthology diff --git a/teuthology/orchestra/cluster.py b/teuthology/orchestra/cluster.py index 654ef0c3de..40104abadd 100644 --- a/teuthology/orchestra/cluster.py +++ b/teuthology/orchestra/cluster.py @@ -2,6 +2,7 @@ Cluster definition part of context, Cluster is used to save connection information. """ +from typing import Callable, Iterable, List, Optional, Tuple, Union from teuthology.orchestra import run class Cluster(object): @@ -9,17 +10,17 @@ class Cluster(object): Manage SSH connections to a cluster of machines. """ - def __init__(self, remotes=None): + def __init__(self, remotes: Optional[Iterable[Tuple[object, list[str]]]] = None) -> None: """ :param remotes: A sequence of 2-tuples of this format: (Remote, [role_1, role_2 ...]) """ - self.remotes = {} + self.remotes: dict = {} if remotes is not None: for remote, roles in remotes: self.add(remote, roles) - def __repr__(self): + def __repr__(self) -> str: remotes = [(k, v) for k, v in self.remotes.items()] remotes.sort(key=lambda tup: tup[0].name) remotes = '[' + ', '.join('[{remote!r}, {roles!r}]'.format( @@ -29,14 +30,14 @@ def __repr__(self): remotes=remotes, ) - def __str__(self): + def __str__(self) -> str: remotes = list(self.remotes.items()) remotes.sort(key=lambda tup: tup[0].name) remotes = ((k, ','.join(v)) for k, v in remotes) remotes = ('{k}[{v}]'.format(k=k, v=v) for k, v in remotes) return ' '.join(remotes) - def add(self, remote, roles): + def add(self, remote, roles: List[str]) -> None: """ Add roles to the list of remotes. """ @@ -49,7 +50,7 @@ def add(self, remote, roles): ) self.remotes[remote] = list(roles) - def run(self, wait=True, parallel=False, **kwargs): + def run(self, wait: bool = True, parallel: bool = False, **kwargs) -> list: """ Run a command on all the nodes in this cluster. @@ -91,7 +92,7 @@ def run(self, wait=True, parallel=False, **kwargs): run.wait(procs) return procs - def sh(self, script, **kwargs): + def sh(self, script: str, **kwargs) -> List[str]: """ Run a command on all the nodes in this cluster. @@ -102,7 +103,8 @@ def sh(self, script, **kwargs): remotes = sorted(self.remotes.keys(), key=lambda rem: rem.name) return [remote.sh(script, **kwargs) for remote in remotes] - def write_file(self, file_name, content, sudo=False, perms=None, owner=None): + def write_file(self, file_name: str, content: str, sudo: bool = False, + perms: Optional[str] = None, owner: Optional[str] = None) -> None: """ Write text to a file on each node. @@ -121,7 +123,7 @@ def write_file(self, file_name, content, sudo=False, perms=None, owner=None): raise ValueError("To specify perms or owner, sudo must be True") remote.write_file(file_name, content) - def only(self, *roles): + def only(self, *roles: Union[str, Callable[[str], bool]]) -> 'Cluster': """ Return a cluster with only the remotes that have all of given roles. @@ -161,7 +163,7 @@ def only(self, *roles): return c - def exclude(self, *roles): + def exclude(self, *roles: Union[str, Callable[[str], bool]]) -> 'Cluster': """ Return a cluster *without* remotes that have all of given roles. @@ -174,7 +176,7 @@ def exclude(self, *roles): c.add(remote, has_roles) return c - def filter(self, func): + def filter(self, func: Callable) -> 'Cluster': """ Return a cluster whose remotes are filtered by `func`. diff --git a/teuthology/orchestra/connection.py b/teuthology/orchestra/connection.py index 1772d37b50..ece8b4202a 100644 --- a/teuthology/orchestra/connection.py +++ b/teuthology/orchestra/connection.py @@ -4,15 +4,17 @@ import paramiko import os import logging +from typing import Callable, Optional, Tuple, Union, List from teuthology.config import config from teuthology.contextutil import safe_while from paramiko.hostkeys import HostKeyEntry +from paramiko.pkey import PKey log = logging.getLogger(__name__) -def split_user(user_at_host): +def split_user(user_at_host: str) -> Tuple[Optional[str], str]: """ break apart user@host fields into user and host. """ @@ -25,7 +27,7 @@ def split_user(user_at_host): return user, host -def create_key(keytype, key): +def create_key(keytype: str, key: str) -> PKey: """ Create an ssh-rsa, ssh-dss or ssh-ed25519 key. """ @@ -36,8 +38,10 @@ def create_key(keytype, key): return ke.key -def connect(user_at_host, host_key=None, keep_alive=False, timeout=60, - _SSHClient=None, _create_key=None, retry=True, key_filename=None): +def connect(user_at_host: str, host_key: Optional[str] = None, keep_alive: bool = False, + timeout: int = 60, _SSHClient: Optional = None, + _create_key: Optional[Callable[[str, str], PKey]] = None, + retry: bool = True, key_filename: Optional[Union[str, List[str]]] = None) -> paramiko.SSHClient: """ ssh connection routine. @@ -52,6 +56,8 @@ def connect(user_at_host, host_key=None, keep_alive=False, timeout=60, :param key_filename: Optionally override which private key to use. :return: ssh connection. """ + if timeout is None: + timeout = 60 user, host = split_user(user_at_host) if _SSHClient is None: _SSHClient = paramiko.SSHClient @@ -73,7 +79,7 @@ def connect(user_at_host, host_key=None, keep_alive=False, timeout=60, key=_create_key(keytype, key) ) - connect_args = dict( + connect_args: dict[str, Union[str, None, int, list[str]]] = dict( hostname=host, username=user, timeout=timeout diff --git a/teuthology/orchestra/console.py b/teuthology/orchestra/console.py index b3ec963d7e..fbdbd57fb0 100644 --- a/teuthology/orchestra/console.py +++ b/teuthology/orchestra/console.py @@ -17,7 +17,7 @@ from teuthology.misc import host_shortname try: - import libvirt + import libvirt # ty: ignore [unresolved-import] except ImportError: libvirt = None @@ -25,8 +25,27 @@ PowerOnOffState = Union[Literal["on"], Literal["off"]] +class LoggedPexpect(pexpect.spawn): + logfile_read: io.StringIO + + def __init__(self, + command: str, + encoding='utf-8', + codec_errors="backslashreplace", + ): + pexpect.spawn.__init__( + self, + command=command, + encoding=encoding, + codec_errors=codec_errors, + ) + self.logfile_read = io.StringIO() + + class RemoteConsole(): - def getShortName(self, name=None): + name: str + + def getShortName(self, name: Optional[str] = None) -> str: """ Extract the name portion from remote name strings. """ @@ -38,8 +57,9 @@ class PhysicalConsole(RemoteConsole): """ Physical Console (set from getRemoteConsole) """ - def __init__(self, name, ipmiuser=None, ipmipass=None, ipmidomain=None, - timeout=120): + def __init__(self, name: str, ipmiuser: Optional[str] = None, + ipmipass: Optional[str] = None, ipmidomain: Optional[str] = None, + timeout: int = 120) -> None: self.name = name self.shortname = self.getShortName(name) self.log = log.getChild(self.shortname) @@ -64,27 +84,26 @@ def __init__(self, name, ipmiuser=None, ipmipass=None, ipmidomain=None, conserver_client_found, ]) - def _pexpect_spawn_ipmi(self, ipmi_cmd): + def _pexpect_spawn_ipmi(self, ipmi_cmd: str) -> LoggedPexpect: """ Run the cmd specified using ipmitool. """ full_command = self._ipmi_command(ipmi_cmd) return self._pexpect_spawn(full_command) - def _pexpect_spawn(self, cmd): + def _pexpect_spawn(self, cmd: str) -> LoggedPexpect: """ Run a command using pexpect.spawn(). Return the child object. """ self.log.debug('pexpect command: %s', cmd) - p = pexpect.spawn( + p = LoggedPexpect( cmd, encoding='utf-8', codec_errors="backslashreplace", ) - p.logfile_read = io.StringIO() return p - def _get_console(self, readonly=True): + def _get_console(self, readonly: bool = True) -> LoggedPexpect: def start(): cmd = self._console_command(readonly=readonly) return self._pexpect_spawn(cmd) @@ -96,7 +115,7 @@ def start(): child = start() return child - def _console_command(self, readonly=True): + def _console_command(self, readonly: bool = True) -> str: if self.has_conserver: return 'console -M {master} -p {port} {mode} {host}'.format( master=self.conserver_master, @@ -107,7 +126,7 @@ def _console_command(self, readonly=True): else: return self._ipmi_command('sol activate') - def _ipmi_command(self, subcommand): + def _ipmi_command(self, subcommand: str) -> str: self._check_ipmi_credentials() template = \ 'ipmitool -H {s}.{dn} -I lanplus -U {ipmiuser} -P {ipmipass} {cmd}' @@ -119,14 +138,14 @@ def _ipmi_command(self, subcommand): ipmipass=self.ipmipass, ) - def _check_ipmi_credentials(self): + def _check_ipmi_credentials(self) -> None: if not self.has_ipmi_credentials: self.log.error( "Must set ipmi_user, ipmi_password, and ipmi_domain in " ".teuthology.yaml" ) - def _exit_session(self, child, timeout=None): + def _exit_session(self, child: LoggedPexpect, timeout: Optional[int] = None) -> None: t = timeout or self.timeout if self.has_conserver: child.sendcontrol('e') @@ -147,14 +166,14 @@ def _exit_session(self, child, timeout=None): self._pexpect_spawn_ipmi('sol deactivate') self.log.debug('sol deactivate output: %s', child.logfile_read.getvalue().strip()) - def _wait_for_login(self, timeout=None, attempts=2): + def _wait_for_login(self, timeout: Optional[int] = None, attempts: int = 2) -> None: """ Wait for login. Retry if timeouts occur on commands. """ t = timeout or self.timeout self.log.debug('Waiting for login prompt') # wait for login prompt to indicate boot completed - for i in range(0, attempts): + for _ in range(0, attempts): start = time.time() while time.time() - start < t: child = self._get_console(readonly=False) @@ -172,14 +191,14 @@ def _wait_for_login(self, timeout=None, attempts=2): return raise ConsoleError("Did not get a login prompt from %s!" % self.name) - def check_power(self, state: Literal["on","off"]): + def check_power(self, state: Literal["on","off"]) -> bool: c = self._pexpect_spawn_ipmi('power status') r = c.expect(['Chassis Power is {s}'.format( s=state), pexpect.EOF, pexpect.TIMEOUT], timeout=1) self.log.debug('check power output: %s', c.logfile_read.getvalue().strip()) return r == 0 - def set_power(self, state: PowerOnOffState, timeout: Optional[int]): + def set_power(self, state: PowerOnOffState, timeout: Optional[int]) -> bool: self.log.info(f"Power {state}") timeout = timeout or self.timeout sleep_time = 4 @@ -237,12 +256,12 @@ def set_power(self, state: PowerOnOffState, timeout: Optional[int]): ) return False - def check_power_retries(self, state, timeout=None): + def check_power_retries(self, state: str, timeout: Optional[int] = None) -> bool: """ Check power. Retry if EOF encountered on power check read. """ timeout = timeout or self.timeout - sleep_time = 4.0 + sleep_time = 4 with safe_while( sleep=sleep_time, tries=int(timeout / sleep_time), @@ -257,7 +276,7 @@ def check_power_retries(self, state, timeout=None): return True return False - def check_status(self, timeout=None): + def check_status(self, timeout: Optional[int] = None) -> bool: """ Check status. Returns True if console is at login prompt """ @@ -269,7 +288,7 @@ def check_status(self, timeout=None): self.log.exception('Failed to get ipmi console status') return False - def power_cycle(self, timeout=300): + def power_cycle(self, timeout: int = 300) -> None: """ Power cycle and wait for login. @@ -282,7 +301,7 @@ def power_cycle(self, timeout=300): self._wait_for_login(timeout=timeout) self.log.info('Power cycle completed') - def hard_reset(self, wait_for_login=True): + def hard_reset(self, wait_for_login: bool = True) -> None: """ Perform physical hard reset. Retry if EOF returned from read and wait for login when complete. @@ -300,13 +319,13 @@ def hard_reset(self, wait_for_login=True): self._wait_for_login() self.log.info('Hard reset completed') - def power_on(self): + def power_on(self) -> bool: """ Physical power on. Loop checking cmd return. """ return self.set_power("on", timeout=None) - def power_off(self): + def power_off(self) -> Optional[bool]: """ Physical power off. Loop checking cmd return. """ @@ -315,7 +334,7 @@ def power_off(self): except Exception: pass - def power_off_for_interval(self, interval=30): + def power_off_for_interval(self, interval: int = 30) -> None: """ Physical power off for an interval. Wait for login when complete. @@ -337,7 +356,7 @@ def power_off_for_interval(self, interval=30): self._wait_for_login() self.log.info('Power off for {i} seconds completed'.format(i=interval)) - def spawn_sol_log(self, dest_path): + def spawn_sol_log(self, dest_path: str) -> psutil.Popen: """ Using the subprocess module, spawn an ipmitool process using 'sol activate' and redirect its output to a file. @@ -378,7 +397,7 @@ class VirtualConsole(RemoteConsole): """ Virtual Console (set from getRemoteConsole) """ - def __init__(self, name): + def __init__(self, name: str) -> None: if libvirt is None: raise RuntimeError("libvirt not found") @@ -398,46 +417,48 @@ def __init__(self, name): break return - def check_power(self, state, timeout=None): + def check_power(self, state: str, timeout: Optional[int] = None) -> bool: """ Return true if vm domain state indicates power is on. """ + assert libvirt return self.vm_domain.info[0] in [libvirt.VIR_DOMAIN_RUNNING, libvirt.VIR_DOMAIN_BLOCKED, libvirt.VIR_DOMAIN_PAUSED] - def check_status(self, timeout=None): + def check_status(self, timeout: Optional[int] = None) -> bool: """ Return true if running. """ + assert libvirt return self.vm_domain.info()[0] == libvirt.VIR_DOMAIN_RUNNING - def power_cycle(self): + def power_cycle(self) -> None: """ Simiulate virtual machine power cycle """ self.vm_domain.info().destroy() self.vm_domain.info().create() - def hard_reset(self): + def hard_reset(self) -> None: """ Simiulate hard reset """ self.vm_domain.info().destroy() - def power_on(self): + def power_on(self) -> None: """ Simiulate power on """ self.vm_domain.info().create() - def power_off(self): + def power_off(self) -> None: """ Simiulate power off """ self.vm_domain.info().destroy() - def power_off_for_interval(self, interval=30): + def power_off_for_interval(self, interval: int = 30) -> None: """ Simiulate power off for an interval. """ diff --git a/teuthology/orchestra/daemon/cephadmunit.py b/teuthology/orchestra/daemon/cephadmunit.py index e21e0449bb..17b007787f 100644 --- a/teuthology/orchestra/daemon/cephadmunit.py +++ b/teuthology/orchestra/daemon/cephadmunit.py @@ -88,6 +88,7 @@ def _stop_logger(self): check_status=False, ) #self.log.info('_stop_logger %s waiting') + assert self.remote_logger self.remote_logger.wait() self.remote_logger = None #self.log.info('_stop_logger done') diff --git a/teuthology/orchestra/daemon/group.py b/teuthology/orchestra/daemon/group.py index 656f5a0ba1..b67dd48e25 100644 --- a/teuthology/orchestra/daemon/group.py +++ b/teuthology/orchestra/daemon/group.py @@ -1,3 +1,4 @@ +from typing import List, Optional, Union from teuthology import misc from teuthology.orchestra.daemon.state import DaemonState from teuthology.orchestra.daemon.systemd import SystemDState @@ -8,7 +9,7 @@ class DaemonGroup(object): """ Collection of daemon state instances """ - def __init__(self, use_systemd=False, use_cephadm=None): + def __init__(self, use_systemd: bool = False, use_cephadm: Optional[bool] = None) -> None: """ self.daemons is a dictionary indexed by role. Each entry is a dictionary of DaemonState values indexed by an id parameter. @@ -21,7 +22,7 @@ def __init__(self, use_systemd=False, use_cephadm=None): self.use_systemd = use_systemd self.use_cephadm = use_cephadm - def add_daemon(self, remote, type_, id_, *args, **kwargs): + def add_daemon(self, remote, type_: str, id_: str, *args, **kwargs) -> None: """ Add a daemon. If there already is a daemon for this id_ and role, stop that daemon. (Re)start the daemon once the new value is set. @@ -39,7 +40,7 @@ def add_daemon(self, remote, type_, id_, *args, **kwargs): role = cluster + '.' + type_ self.daemons[role][id_].restart() - def register_daemon(self, remote, type_, id_, *args, **kwargs): + def register_daemon(self, remote, type_: str, id_: str, *args, **kwargs) -> None: """ Add a daemon. If there already is a daemon for this id_ and role, stop that daemon. @@ -73,7 +74,7 @@ def register_daemon(self, remote, type_, id_, *args, **kwargs): self.daemons[role][id_] = klass( remote, role, id_, *args, **kwargs) - def get_daemon(self, type_, id_, cluster='ceph'): + def get_daemon(self, type_: str, id_: Union[str, int], cluster: str = 'ceph') -> Optional[DaemonState]: """ get the daemon associated with this id_ for this role. @@ -85,7 +86,7 @@ def get_daemon(self, type_, id_, cluster='ceph'): return None return self.daemons[role].get(str(id_), None) - def iter_daemons_of_role(self, type_, cluster='ceph'): + def iter_daemons_of_role(self, type_: str, cluster: str = 'ceph'): """ Iterate through all daemon instances for this role. Return dictionary of daemon values. @@ -95,7 +96,8 @@ def iter_daemons_of_role(self, type_, cluster='ceph'): role = cluster + '.' + type_ return self.daemons.get(role, {}).values() - def resolve_role_list(self, roles, types, cluster_aware=False): + def resolve_role_list(self, roles: Optional[List[str]], types: List[str], + cluster_aware: bool = False) -> List[str]: """ Resolve a configuration setting that may be None or contain wildcards into a list of roles (where a role is e.g. 'mds.a' or 'osd.0'). This diff --git a/teuthology/orchestra/daemon/state.py b/teuthology/orchestra/daemon/state.py index 03387c9991..293617055c 100644 --- a/teuthology/orchestra/daemon/state.py +++ b/teuthology/orchestra/daemon/state.py @@ -1,5 +1,6 @@ import logging import struct +from typing import Optional from teuthology.exceptions import CommandFailedError from teuthology.orchestra import run @@ -11,7 +12,7 @@ class DaemonState(object): """ Daemon State. A daemon exists for each instance of each role. """ - def __init__(self, remote, role, id_, *command_args, **command_kwargs): + def __init__(self, remote, role: str, id_: str, *command_args, **command_kwargs) -> None: """ Pass remote command information as parameters to remote site @@ -31,7 +32,7 @@ def __init__(self, remote, role, id_, *command_args, **command_kwargs): self.proc = None self.command_kwargs = command_kwargs - def check_status(self): + def check_status(self) -> Optional[int]: """ Check to see if the process has exited. @@ -43,10 +44,10 @@ def check_status(self): return self.proc.poll() @property - def pid(self): + def pid(self) -> Optional[int]: raise NotImplementedError - def reset(self): + def reset(self) -> None: """ clear remote run command value. """ @@ -111,6 +112,7 @@ def signal(self, sig, silent=False): :param sig: signal to send """ if self.running(): + assert self.proc try: self.proc.stdin.write(struct.pack('!b', sig)) except IOError as e: @@ -140,6 +142,7 @@ def stop(self, timeout=300): if not self.running(): self.log.error('tried to stop a non-running daemon') return + assert self.proc self.proc.stdin.close() self.log.debug('waiting for process to exit') try: diff --git a/teuthology/orchestra/monkey.py b/teuthology/orchestra/monkey.py index e13e77305e..6b1996c87d 100644 --- a/teuthology/orchestra/monkey.py +++ b/teuthology/orchestra/monkey.py @@ -5,7 +5,7 @@ log = logging.getLogger(__name__) -def patch_001_paramiko_deprecation(): +def patch_001_paramiko_deprecation() -> None: """ Silence an an unhelpful Deprecation Warning triggered by Paramiko. @@ -19,7 +19,7 @@ def patch_001_paramiko_deprecation(): ) -def patch_100_paramiko_log(): +def patch_100_paramiko_log() -> None: """ Silence some noise paramiko likes to log. @@ -28,24 +28,13 @@ def patch_100_paramiko_log(): logging.getLogger('paramiko.transport').setLevel(logging.WARNING) -def patch_100_logger_getChild(): - """ - Imitate Python 2.7 feature Logger.getChild. - """ - import logging - if not hasattr(logging.Logger, 'getChild'): - def getChild(self, name): - return logging.getLogger('.'.join([self.name, name])) - logging.Logger.getChild = getChild - - -def patch_100_trigger_rekey(): +def patch_100_trigger_rekey() -> None: # Fixes http://tracker.ceph.com/issues/15236 from paramiko.packet import Packetizer - Packetizer._trigger_rekey = lambda self: True + Packetizer._trigger_rekey = lambda _: True # ty: ignore[invalid-assignment] -def patch_all(): +def patch_all() -> None: """ Run all the patch_* functions in this module. """ diff --git a/teuthology/orchestra/opsys.py b/teuthology/orchestra/opsys.py index 562db06891..0140f25594 100644 --- a/teuthology/orchestra/opsys.py +++ b/teuthology/orchestra/opsys.py @@ -1,4 +1,5 @@ import re +from typing import Dict, Optional, Tuple from packaging.version import parse as parse_version, Version @@ -120,23 +121,38 @@ class OS(object): __slots__ = ['name', 'version', 'codename', 'package_type'] + name: str + version: str + codename: Optional[str] + package_type: str + _deb_distros = ('debian', 'ubuntu') _rpm_distros = ('alma', 'rocky', 'fedora', 'rhel', 'centos', 'opensuse', 'sle') - def __init__(self, name=None, version=None, codename=None): + def __init__(self, name: str, version: Optional[str] = None, + codename: Optional[str] = None) -> None: self.name = name - self.version = version or self._codename_to_version(name, codename) - self.codename = codename or self._version_to_codename(name, version) + if version and codename: + self.version = version + self.codename = codename + if version is None: + assert codename is not None + self.codename = codename + self.version = self._codename_to_version(name, codename) + if codename is None: + assert version is not None + self.version = version + self.codename = self._version_to_codename(name, version) self._set_package_type() @staticmethod - def _version_to_codename(name, version): - for (_version, codename) in DISTRO_CODENAME_MAP[name].items(): + def _version_to_codename(name: str, version: str) -> Optional[str]: + for (_version, codename) in DISTRO_CODENAME_MAP.get(name, {}).items(): if str(version) == _version or str(version).split('.')[0] == _version: return codename @staticmethod - def _codename_to_version(name, codename): + def _codename_to_version(name: str, codename: str) -> str: for (version, _codename) in DISTRO_CODENAME_MAP[name].items(): if codename == _codename: return version @@ -146,7 +162,7 @@ def _codename_to_version(name, codename): )) @classmethod - def from_lsb_release(cls, lsb_release_str): + def from_lsb_release(cls, lsb_release_str: str) -> 'OS': """ Parse output from lsb_release -a and populate attributes @@ -180,7 +196,7 @@ def from_lsb_release(cls, lsb_release_str): return obj @classmethod - def from_os_release(cls, os_release_str): + def from_os_release(cls, os_release_str: str) -> 'OS': """ Parse /etc/os-release and populate attributes @@ -218,7 +234,7 @@ def from_os_release(cls, os_release_str): @classmethod - def version_codename(cls, name, version_or_codename): + def version_codename(cls, name: str, version_or_codename: str) -> Tuple[str, str]: """ Return (version, codename) based on one input, trying to infer which we're given @@ -247,36 +263,38 @@ def version_codename(cls, name, version_or_codename): @staticmethod - def _get_value(str_, name): + def _get_value(str_: str, name: str) -> str: regex = '^%s[:=](.+)' % name match = re.search(regex, str_, flags=re.M) if match: return match.groups()[0].strip(' \t"\'') return '' - def _set_package_type(self): + def _set_package_type(self) -> None: if self.name in self._deb_distros: self.package_type = "deb" elif self.name in self._rpm_distros: self.package_type = "rpm" - def to_dict(self): + def to_dict(self) -> Dict[str, Optional[str]]: return dict( name=self.name, version=self.version, codename=self.codename, ) - def __str__(self): - return " ".join([self.name, self.version]).strip() + def __str__(self) -> str: + return " ".join([str(self.name), str(self.version)]).strip() - def __repr__(self): + def __repr__(self) -> str: return "OS(name={name}, version={version}, codename={codename})"\ .format(name=repr(self.name), version=repr(self.version), codename=repr(self.codename)) - def __eq__(self, other): + def __eq__(self, other: object) -> bool: + if not isinstance(other, self.__class__): + return NotImplemented if self.name.lower() != other.name.lower(): return False normalize = lambda s: s.lower().removesuffix(".stream") diff --git a/teuthology/orchestra/remote.py b/teuthology/orchestra/remote.py index 0ad29d5bac..b3cfa3e694 100644 --- a/teuthology/orchestra/remote.py +++ b/teuthology/orchestra/remote.py @@ -23,6 +23,7 @@ import pwd import tempfile import netaddr +from typing import List, Optional, Union log = logging.getLogger(__name__) @@ -43,10 +44,18 @@ class RemoteShell(object): the subclass. """ - def remove(self, path): + # Attributes that subclasses should provide + name: str + shortname: str + + def run(self, **_): + """Subclasses must implement this method""" + raise NotImplementedError("Subclasses must implement run()") + + def remove(self, path: str): self.run(args=['rm', '-fr', path]) - def mkdtemp(self, suffix=None, parentdir=None): + def mkdtemp(self, suffix: Optional[str] = None, parentdir: Optional[str] = None) -> str: """ Create a temporary directory on remote machine and return it's path. """ @@ -59,7 +68,8 @@ def mkdtemp(self, suffix=None, parentdir=None): return self.sh(args).strip() - def mktemp(self, suffix=None, parentdir=None, data=None): + def mktemp(self, suffix: Optional[str] = None, parentdir: Optional[str] = None, + data: Optional[Union[str, bytes]] = None) -> str: """ Make a remote temporary file. @@ -82,7 +92,7 @@ def mktemp(self, suffix=None, parentdir=None, data=None): return path - def sh(self, script, **kwargs): + def sh(self, script: Union[str, List[str]], **kwargs) -> str: """ Shortcut for run method. @@ -101,7 +111,8 @@ def sh(self, script, **kwargs): else: return out - def sh_file(self, script, label="script", sudo=False, **kwargs): + def sh_file(self, script: Union[str], label: str = "script", + sudo: Union[bool, str] = False, **kwargs) -> str: """ Run shell script after copying its contents to a remote file @@ -126,7 +137,7 @@ def sh_file(self, script, label="script", sudo=False, **kwargs): return self.sh(command, **kwargs) - def chmod(self, file_path, permissions): + def chmod(self, file_path: str, permissions: str) -> None: """ As super-user, set permissions on the remote file specified. """ @@ -140,7 +151,7 @@ def chmod(self, file_path, permissions): args=args, ) - def chcon(self, file_path, context): + def chcon(self, file_path: str, context: str) -> None: """ Set the SELinux context of a given file. @@ -158,8 +169,8 @@ def chcon(self, file_path, context): self.run(args="sudo chcon {con} {path}".format( con=context, path=file_path)) - def copy_file(self, src, dst, sudo=False, mode=None, owner=None, - mkdir=False, append=False): + def copy_file(self, src: str, dst: str, sudo: bool = False, mode: Optional[str] = None, + owner: Optional[str] = None, mkdir: bool = False, append: bool = False) -> None: """ Copy data to remote file @@ -190,8 +201,8 @@ def copy_file(self, src, dst, sudo=False, mode=None, owner=None, args = 'set -ex' + '\n' + args self.run(args=args) - def move_file(self, src, dst, sudo=False, mode=None, owner=None, - mkdir=False): + def move_file(self, src: str, dst: str, sudo: bool = False, mode: Optional[str] = None, + owner: Optional[str] = None, mkdir: bool = False): """ Move data to remote file @@ -358,7 +369,6 @@ class Remote(RemoteShell): # for unit tests to hook into _runner = staticmethod(run.run) - _reimage_types = None def __init__(self, name, ssh=None, shortname=None, console=None, host_key=None, keep_alive=True): @@ -379,12 +389,13 @@ def __init__(self, name, ssh=None, shortname=None, console=None, self._console = console self.ssh = ssh - if self._reimage_types is None: - Remote._reimage_types = teuthology.provision.get_reimage_types() + @property + def reimage_types(self): + return teuthology.provision.get_reimage_types() def connect(self, timeout=None, create_key=None, context='connect'): args = dict(user_at_host=self.name, host_key=self._host_key, - keep_alive=self.keep_alive, _create_key=create_key) + keep_alive=self.keep_alive) if context == 'reconnect': # The reason for the 'context' workaround is not very # clear from the technical side. @@ -450,6 +461,7 @@ def safe_hard_reboot(self): @property def ip_address(self): + assert self.ssh return self.ssh.get_transport().getpeername()[0] @property @@ -539,7 +551,7 @@ def machine_type(self): @property def is_reimageable(self): - return self.machine_type in self._reimage_types + return self.machine_type in self.reimage_types @property def shortname(self): @@ -615,6 +627,7 @@ def _sftp_put_file(self, local_path, remote_path): """ Use the paramiko.SFTPClient to put a file. Returns the remote filename. """ + assert self.ssh sftp = self.ssh.open_sftp() sftp.put(local_path, remote_path) return @@ -623,6 +636,7 @@ def _sftp_get_file(self, remote_path, local_path): """ Use the paramiko.SFTPClient to get a file. Returns the local filename. """ + assert self.ssh file_size = self._format_size( self._sftp_get_size(remote_path) ).strip() @@ -636,6 +650,7 @@ def _sftp_open_file(self, remote_path, mode=None): Use the paramiko.SFTPClient to open a file. Returns a paramiko.SFTPFile object. """ + assert self.ssh sftp = self.ssh.open_sftp() if mode: return sftp.open(remote_path, mode) @@ -752,6 +767,7 @@ def get_tar_stream(self, path, sudo=False, compress=True): @property def host_key(self): + assert self.ssh if not self._host_key: trans = self.ssh.get_transport() key = trans.get_remote_server_key() diff --git a/teuthology/orchestra/run.py b/teuthology/orchestra/run.py index 7f6fdb2407..3b18db765d 100644 --- a/teuthology/orchestra/run.py +++ b/teuthology/orchestra/run.py @@ -3,6 +3,7 @@ """ import io +from typing import List, Optional, Union from paramiko import ChannelFile @@ -38,8 +39,10 @@ class RemoteProcess(object): deadlock_warning = "Using PIPE for %s without wait=False would deadlock" - def __init__(self, client, args, check_status=True, hostname=None, - label=None, timeout=None, wait=True, logger=None, cwd=None): + def __init__(self, client, args: Union[str, List[str]], check_status: bool = True, + hostname: Optional[str] = None, label: Optional[str] = None, + timeout: Optional[int] = None, wait: bool = True, + logger: Optional[logging.Logger] = None, cwd: Optional[str] = None) -> None: """ Create the object. Does not initiate command execution. @@ -85,7 +88,7 @@ def __init__(self, client, args, check_status=True, hostname=None, self._wait = wait self.logger = logger or log - def execute(self): + def execute(self) -> None: """ Execute remote command """ @@ -101,10 +104,10 @@ def execute(self): (self.stdin, self.stdout, self.stderr) = \ (self._stdin_buf, self._stdout_buf, self._stderr_buf) - def add_greenlet(self, greenlet): + def add_greenlet(self, greenlet) -> None: self.greenlets.append(greenlet) - def setup_stdin(self, stream_obj): + def setup_stdin(self, stream_obj) -> None: self.stdin = KludgeFile(wrapped=self.stdin) if stream_obj is not PIPE: greenlet = gevent.spawn(copy_and_close, stream_obj, self.stdin) @@ -114,7 +117,7 @@ def setup_stdin(self, stream_obj): # FIXME: Is this actually true? raise RuntimeError(self.deadlock_warning % 'stdin') - def setup_output_stream(self, stream_obj, stream_name, quiet=False): + def setup_output_stream(self, stream_obj, stream_name: str, quiet: bool = False) -> None: if stream_obj is not PIPE: # Log the stream host_log = self.logger.getChild(self.hostname) @@ -133,7 +136,7 @@ def setup_output_stream(self, stream_obj, stream_name, quiet=False): # FIXME: Is this actually true? raise RuntimeError(self.deadlock_warning % stream_name) - def wait(self): + def wait(self) -> Optional[int]: """ Block until remote process finishes. @@ -161,7 +164,7 @@ def wait(self): self._raise_for_status() return status - def _raise_for_status(self): + def _raise_for_status(self) -> None: if self.returncode is None: self._get_exitstatus() if self.check_status: @@ -183,7 +186,7 @@ def _raise_for_status(self): node=self.hostname, label=self.label ) - def _get_exitstatus(self): + def _get_exitstatus(self) -> Optional[int]: """ :returns: the remote command's exit status (return code). Note that if the connection is lost, or if the process was killed by a diff --git a/teuthology/packaging.py b/teuthology/packaging.py index cbeb6d13d5..a21730cbcf 100644 --- a/teuthology/packaging.py +++ b/teuthology/packaging.py @@ -498,6 +498,7 @@ def _init_from_remote(self): """ Initializes the class from a teuthology.orchestra.remote.Remote object """ + assert self.remote self.arch = self.remote.arch self.os_type = self.remote.os.name self.os_version = self.remote.os.version @@ -621,7 +622,7 @@ def _parse_version(version): return version.split(".")[0] @classmethod - def _get_distro(cls, distro=None, version=None, codename=None): + def _get_distro(cls, distro: str|None=None, version: str|None=None, codename: str|None=None): """ Given a distro and a version, returned the combined string to use in a gitbuilder url. @@ -633,7 +634,7 @@ def _get_distro(cls, distro=None, version=None, codename=None): """ if distro in ('centos', 'rhel'): distro = "centos" - elif distro in ("fedora"): + elif distro == "fedora": pass elif distro in ("opensuse", "sle"): pass @@ -642,6 +643,8 @@ def _get_distro(cls, distro=None, version=None, codename=None): else: # deb based systems use codename instead of a distro/version combo if not codename: + assert distro + assert version # lookup codename based on distro string codename = OS._version_to_codename(distro, version) if not codename: @@ -812,6 +815,7 @@ def install_repo(self): self._install_deb_repo() def _install_rpm_repo(self): + assert self.remote dist_release = self.dist_release project = self.project proj_release = \ @@ -833,6 +837,7 @@ def _install_rpm_repo(self): self.remote.run(args=['sudo', 'yum', '-y', 'install', url]) def _install_deb_repo(self): + assert self.remote self.remote.run( args=[ 'echo', 'deb', self.base_url, self.codename, 'main', @@ -857,6 +862,7 @@ def remove_repo(self): self._remove_deb_repo() def _remove_rpm_repo(self): + assert self.remote if self.dist_release in ['opensuse', 'sle']: self.remote.run(args=[ 'sudo', 'zypper', '-n', 'removerepo', 'ceph-rpm-under-test' @@ -865,6 +871,7 @@ def _remove_rpm_repo(self): remove_package('%s-release' % self.project, self.remote) def _remove_deb_repo(self): + assert self.remote self.remote.run( args=[ 'sudo', @@ -1038,7 +1045,7 @@ def build_complete(self): try: resp = requests.get(build_url) resp.raise_for_status() - except requests.HttpError: + except requests.exceptions.HTTPError: return False log.debug(f'looking for {self.distro} {self.arch} {self.flavor}') for build in resp.json(): @@ -1090,6 +1097,7 @@ def _install_deb_repo(self): def _remove_rpm_repo(self): # FIXME: zypper + assert self.remote self.remote.run( args=[ 'sudo', diff --git a/teuthology/parallel.py b/teuthology/parallel.py index 0a7d3ab35a..9d6a96ba9f 100644 --- a/teuthology/parallel.py +++ b/teuthology/parallel.py @@ -1,5 +1,7 @@ import logging import sys +from typing import Callable, Iterator, Optional +from types import TracebackType import gevent import gevent.pool @@ -10,11 +12,11 @@ class ExceptionHolder(object): - def __init__(self, exc_info): + def __init__(self, exc_info: tuple[type[BaseException], BaseException, TracebackType]) -> None: self.exc_info = exc_info -def capture_traceback(func, *args, **kwargs): +def capture_traceback(func: Callable, *args, **kwargs): """ Utility function to capture tracebacks of any exception func raises. @@ -25,7 +27,7 @@ def capture_traceback(func, *args, **kwargs): return ExceptionHolder(sys.exc_info()) -def resurrect_traceback(exc): +def resurrect_traceback(exc) -> None: if isinstance(exc, ExceptionHolder): raise exc.exc_info[1] elif isinstance(exc, BaseException): @@ -60,23 +62,23 @@ class parallel(object): kills the rest and raises the exception. """ - def __init__(self): + def __init__(self) -> None: self.group = gevent.pool.Group() self.results = gevent.queue.Queue() self.count = 0 self.any_spawned = False self.iteration_stopped = False - def spawn(self, func, *args, **kwargs): + def spawn(self, func: Callable, *args, **kwargs) -> None: self.count += 1 self.any_spawned = True greenlet = self.group.spawn(capture_traceback, func, *args, **kwargs) greenlet.link(self._finish) - def __enter__(self): + def __enter__(self) -> 'parallel': return self - def __exit__(self, type_, value, traceback): + def __exit__(self, type_: Optional[type[BaseException]], value: Optional[BaseException], traceback: Optional[TracebackType]) -> bool: if value is not None: return False @@ -86,7 +88,7 @@ def __exit__(self, type_, value, traceback): return True - def __iter__(self): + def __iter__(self) -> Iterator: return self def __next__(self): @@ -104,7 +106,7 @@ def __next__(self): next = __next__ - def _finish(self, greenlet): + def _finish(self, greenlet: gevent.Greenlet) -> None: if greenlet.successful(): self.results.put(greenlet.value) else: diff --git a/teuthology/provision/__init__.py b/teuthology/provision/__init__.py index 8fe9bf0812..4b3ad97e6f 100644 --- a/teuthology/provision/__init__.py +++ b/teuthology/provision/__init__.py @@ -1,5 +1,6 @@ import logging import os +from typing import List, Optional import teuthology.exporter import teuthology.lock.query @@ -15,16 +16,16 @@ log = logging.getLogger(__name__) -def _logfile(shortname: str, archive_path: str = ""): +def _logfile(shortname: str, archive_path: str = "") -> Optional[str]: if os.path.isfile(archive_path): return f"{archive_path}/{shortname}.downburst.log" -def get_reimage_types(): +def get_reimage_types() -> List[str]: return pelagos.get_types() + fog.get_types() + maas.get_types() -def reimage(ctx, machine_name, machine_type): +def reimage(ctx, machine_name: str, machine_type: str): os_type = get_distro(ctx) os_version = get_distro_version(ctx) @@ -61,7 +62,7 @@ def reimage(ctx, machine_name, machine_type): return result -def create_if_vm(ctx, machine_name, _downburst=None): +def create_if_vm(ctx, machine_name: str, _downburst: Optional = None) -> bool: """ Use downburst to create a virtual machine @@ -97,15 +98,16 @@ def create_if_vm(ctx, machine_name, _downburst=None): downburst.Downburst(name=machine_name, os_type=os_type, os_version=os_version, status=status_info, logfile=_logfile(ctx, shortname)) - return dbrst.create() + result = dbrst.create() + return bool(result) def destroy_if_vm( machine_name: str, user: str = "", description: str = "", - _downburst=None -): + _downburst: Optional = None +) -> bool: """ Use downburst to destroy a virtual machine diff --git a/teuthology/provision/cloud/__init__.py b/teuthology/provision/cloud/__init__.py index d30ad3338c..ad4279bfcf 100644 --- a/teuthology/provision/cloud/__init__.py +++ b/teuthology/provision/cloud/__init__.py @@ -1,13 +1,17 @@ import logging +from typing import List, Optional, Type, TypedDict from teuthology.config import config -from teuthology.provision.cloud import openstack +from teuthology.provision.cloud import base, openstack log = logging.getLogger(__name__) +class SupportedDrivers(TypedDict): + provider: Type[base.Provider] + provisioner: Type[base.Provisioner] -supported_drivers = dict( +supported_drivers: dict[str, SupportedDrivers] = dict( openstack=dict( provider=openstack.OpenStackProvider, provisioner=openstack.OpenStackProvisioner, @@ -15,35 +19,42 @@ ) -def get_types(): +def get_types() -> List[str]: types = list() if 'libcloud' in config and 'providers' in config.libcloud: types = list(config.libcloud['providers'].keys()) return types -def get_provider_conf(node_type): +def get_provider_conf(node_type: str) -> dict: all_providers = config.libcloud['providers'] provider_conf = all_providers[node_type] return provider_conf -def get_provider(node_type): +def get_provider(node_type: str): provider_conf = get_provider_conf(node_type) driver = provider_conf['driver'] - provider_cls = supported_drivers[driver]['provider'] + provider_cls: Type[base.Provider] = supported_drivers[driver]['provider'] return provider_cls(name=node_type, conf=provider_conf) -def get_provisioner(node_type, name, os_type, os_version, conf=None): +def get_provisioner( + node_type: str, + name: str, + os_type: Optional[str], + os_version: Optional[str], + conf: Optional[dict] = None +): provider = get_provider(node_type) provider_conf = get_provider_conf(node_type) driver = provider_conf['driver'] provisioner_cls = supported_drivers[driver]['provisioner'] + # Dynamic class lookup - type checker can't verify the correct signature return provisioner_cls( provider=provider, name=name, os_type=os_type, os_version=os_version, - conf=conf, + conf=conf or {}, ) diff --git a/teuthology/provision/cloud/base.py b/teuthology/provision/cloud/base.py index 1700fa9ed4..9a2e7ee5ce 100644 --- a/teuthology/provision/cloud/base.py +++ b/teuthology/provision/cloud/base.py @@ -1,5 +1,6 @@ import logging from copy import deepcopy +from typing import List, Optional from libcloud.compute.providers import get_driver from libcloud.compute.types import Provider as lc_Provider @@ -12,9 +13,9 @@ class Provider(object): - _driver_posargs = list() + _driver_posargs: List[str] = [] - def __init__(self, name, conf): + def __init__(self, name: str, conf: dict) -> None: self.name = name self.conf = conf self.driver_name = self.conf['driver'] @@ -31,15 +32,20 @@ def _get_driver(self): return driver driver = property(fget=_get_driver) - def _get_driver_args(self): + def _get_driver_args(self) -> dict: return deepcopy(self.conf['driver_args']) class Provisioner(object): def __init__( - self, provider, name, os_type=None, os_version=None, - conf=None, user='ubuntu', - ): + self, + provider: Provider | str, + name: str, + os_type: Optional[str] = None, + os_version: Optional[str] = None, + conf: Optional[dict] = None, + user: str = 'ubuntu', + ) -> None: if isinstance(provider, str): provider = teuthology.provision.cloud.get_provider(provider) self.provider = provider @@ -49,7 +55,7 @@ def __init__( self.os_version = os_version self.user = user - def create(self): + def create(self) -> bool: try: return self._create() except Exception: @@ -59,7 +65,7 @@ def create(self): def _create(self): pass - def destroy(self): + def destroy(self) -> bool: try: return self._destroy() except Exception: @@ -77,7 +83,7 @@ def remote(self): ) return self._remote - def __repr__(self): + def __repr__(self) -> str: template = "%s(provider='%s', name='%s', os_type='%s', " \ "os_version='%s')" return template % ( diff --git a/teuthology/provision/cloud/openstack.py b/teuthology/provision/cloud/openstack.py index d8b838b13e..7529ef1476 100644 --- a/teuthology/provision/cloud/openstack.py +++ b/teuthology/provision/cloud/openstack.py @@ -4,11 +4,13 @@ import socket import time import yaml +from typing import Callable, Optional from teuthology.util.compat import urlencode from copy import deepcopy from libcloud.common.exceptions import RateLimitReachedError, BaseHTTPError +from libcloud.compute.base import Node from paramiko import AuthenticationException from paramiko.ssh_exception import NoValidConnectionsError @@ -27,7 +29,7 @@ RETRY_EXCEPTIONS = (RateLimitReachedError, BaseHTTPError) -def retry(function, *args, **kwargs): +def retry(function: Callable, *args, **kwargs): """ Call a function (returning its results), retrying if any of the exceptions in RETRY_EXCEPTIONS are raised @@ -39,9 +41,10 @@ def retry(function, *args, **kwargs): try: result = function(*args, **kwargs) if tries > 1: + func_name = getattr(function, '__name__', str(function)) log.debug( "'%s' succeeded after %s tries", - function.__name__, + func_name, tries, ) return result @@ -68,7 +71,7 @@ def _get_driver(self): return driver driver = property(fget=_get_driver) - def _get_driver_args(self): + def _get_driver_args(self) -> dict: driver_args = super(OpenStackProvider, self)._get_driver_args() if self._auth_token.value: driver_args['ex_force_auth_token'] = self._auth_token.value @@ -76,13 +79,13 @@ def _get_driver_args(self): return driver_args @property - def ssh_interface(self): + def ssh_interface(self) -> str: if not hasattr(self, '_ssh_interface'): self._ssh_interface = self.conf.get('ssh_interface', 'public_ips') return self._ssh_interface @property - def images(self): + def images(self) -> list: if not hasattr(self, '_images'): exclude_image = self.conf.get('exclude_image', []) if exclude_image and not isinstance(exclude_image, list): @@ -94,7 +97,7 @@ def images(self): return self._images @property - def sizes(self): + def sizes(self) -> list: if not hasattr(self, '_sizes'): allow_sizes = self.conf.get('allow_sizes', '.*') if not isinstance(allow_sizes, list): @@ -116,7 +119,7 @@ def sizes(self): return self._sizes @property - def networks(self): + def networks(self) -> list: if not hasattr(self, '_networks'): allow_networks = self.conf.get('allow_networks', '.*') if not isinstance(allow_networks, list): @@ -125,25 +128,25 @@ def networks(self): try: networks = retry(self.driver.ex_list_networks) if networks: - self._networks = filter( + self._networks = list(filter( lambda s: any(x.match(s.name) for x in networks_re), networks - ) + )) else: - self._networks = list() + self._networks = [] except AttributeError: log.warning("Unable to list networks for %s", self.driver) - self._networks = list() + self._networks = [] return self._networks @property - def default_userdata(self): + def default_userdata(self) -> dict: if not hasattr(self, '_default_userdata'): self._default_userdata = self.conf.get('userdata', dict()) return self._default_userdata @property - def security_groups(self): + def security_groups(self) -> list: if not hasattr(self, '_security_groups'): try: self._security_groups = retry( @@ -157,6 +160,8 @@ def security_groups(self): class OpenStackProvisioner(base.Provisioner): _sentinel_path = '/.teuth_provisioned' + provider: OpenStackProvider + user: str defaults = dict( openstack=dict( @@ -174,16 +179,19 @@ class OpenStackProvisioner(base.Provisioner): def __init__( self, - provider, name, os_type=None, os_version=None, - conf=None, - user='ubuntu', - ): + provider: OpenStackProvider | str, + name: str, + os_type: Optional[str] = None, + os_version: Optional[str] = None, + conf: Optional[dict] = None, + user: str = 'ubuntu', + ) -> None: super(OpenStackProvisioner, self).__init__( provider, name, os_type, os_version, conf=conf, user=user, ) self._read_conf(conf) - def _read_conf(self, conf=None): + def _read_conf(self, conf: Optional[dict] = None) -> None: """ Looks through the following in order: @@ -256,7 +264,7 @@ def _create(self): self._wait_for_ready() return self.node - def _create_volumes(self): + def _create_volumes(self) -> bool: vol_count = self.conf['volumes']['count'] vol_size = self.conf['volumes']['size'] name_templ = "%s_%0{0}d".format(len(str(vol_count - 1))) @@ -281,7 +289,7 @@ def _create_volumes(self): return False return True - def _destroy_volumes(self): + def _destroy_volumes(self) -> None: all_volumes = retry(self.provider.driver.list_volumes) our_volumes = [vol for vol in all_volumes if vol.name.startswith("%s_" % self.name)] @@ -295,7 +303,7 @@ def _destroy_volumes(self): except Exception: log.exception("Could not destroy volume %s", vol) - def _update_dns(self): + def _update_dns(self) -> None: query = urlencode(dict( name=self.name, ip=self.ips[0], @@ -307,7 +315,7 @@ def _update_dns(self): resp = requests.get(nsupdate_url) resp.raise_for_status() - def _wait_for_ready(self): + def _wait_for_ready(self) -> None: with safe_while(sleep=6, tries=20) as proceed: while proceed(): try: @@ -363,7 +371,7 @@ def good_size(size): return smallest_match @property - def security_groups(self): + def security_groups(self) -> Optional[list]: group_names = self.provider.conf.get('security_groups') if group_names is None: return @@ -382,10 +390,10 @@ def security_groups(self): return result @property - def userdata(self): + def userdata(self) -> str: spec="{t}-{v}".format(t=self.os_type, v=self.os_version) - base_config = dict( + base_config: dict = dict( packages=[ 'git', 'wget', @@ -402,13 +410,12 @@ def userdata(self): if spec in self.provider.default_userdata: base_config = deepcopy( self.provider.default_userdata.get(spec, dict())) - base_config.update(user=self.user) + base_config['user'] = self.user if 'manage_etc_hosts' not in base_config: - base_config.update( - manage_etc_hosts=True, - hostname=self.hostname, - ) - base_config['runcmd'] = base_config.get('runcmd', list()) + base_config['manage_etc_hosts'] = True + base_config['hostname'] = self.hostname + if 'runcmd' not in base_config: + base_config['runcmd'] = [] base_config['runcmd'].extend(runcmd) ssh_pubkey = util.get_user_ssh_pubkey() if ssh_pubkey: @@ -419,7 +426,7 @@ def userdata(self): return user_str @property - def node(self): + def node(self) -> Optional[Node]: if hasattr(self, '_node'): return self._node matches = self._find_nodes() @@ -435,12 +442,12 @@ def node(self): return self._node raise RuntimeError(msg % self.name) - def _find_nodes(self): + def _find_nodes(self) -> list[Node]: nodes = retry(self.provider.driver.list_nodes) matches = [node for node in nodes if node.name == self.name] return matches - def _destroy(self): + def _destroy(self) -> bool: self._destroy_volumes() nodes = self._find_nodes() if not nodes: diff --git a/teuthology/provision/cloud/util.py b/teuthology/provision/cloud/util.py index 03ea7796f2..fe15fcf477 100644 --- a/teuthology/provision/cloud/util.py +++ b/teuthology/provision/cloud/util.py @@ -2,10 +2,11 @@ import dateutil.parser import json import os +from typing import Callable, List, Optional from teuthology.util.flock import FileLock -def get_user_ssh_pubkey(path='~/.ssh/id_rsa.pub'): +def get_user_ssh_pubkey(path: str = '~/.ssh/id_rsa.pub') -> Optional[str]: full_path = os.path.expanduser(path) if not os.path.exists(full_path): return @@ -13,7 +14,7 @@ def get_user_ssh_pubkey(path='~/.ssh/id_rsa.pub'): return f.read().strip() -def combine_dicts(list_of_dicts, func): +def combine_dicts(list_of_dicts: List[dict], func: Callable) -> dict: """ A useful function to merge a list of dicts. Most of the work is done by selective_update(). @@ -30,7 +31,7 @@ def combine_dicts(list_of_dicts, func): return new_dict -def selective_update(a, b, func): +def selective_update(a: dict, b: dict, func: Callable) -> None: """ Given two dicts and a comparison function, recursively inspects key-value pairs in the second dict and merges them into the first dict if func() @@ -62,7 +63,7 @@ def selective_update(a, b, func): class AuthToken(object): time_format = '%Y-%m-%d %H:%M:%S%z' - def __init__(self, name, directory=os.path.expanduser('~/.cache/')): + def __init__(self, name: str, directory: str = os.path.expanduser('~/.cache/')) -> None: self.name = name self.directory = directory self.path = os.path.join(directory, name) @@ -71,7 +72,7 @@ def __init__(self, name, directory=os.path.expanduser('~/.cache/')): self.value = None self.endpoint = None - def read(self): + def read(self) -> None: if not os.path.exists(self.path): self.value = None self.expires = None @@ -88,7 +89,7 @@ def read(self): self.value = obj['value'] self.endpoint = obj['endpoint'] - def write(self, value, expires, endpoint): + def write(self, value: str, expires: datetime.datetime, endpoint: str) -> None: obj = dict( value=value, expires=datetime.datetime.strftime(expires, self.time_format), @@ -99,17 +100,17 @@ def write(self, value, expires, endpoint): obj.write(string) @property - def expired(self): + def expired(self) -> bool: if self.expires is None: return True utcnow = datetime.datetime.now(datetime.timezone.utc) offset = datetime.timedelta(minutes=30) return self.expires < (utcnow + offset) - def __enter__(self): + def __enter__(self) -> 'AuthToken': with FileLock(self.lock_path): self.read() return self - def __exit__(self, exc_type, exc_val, exc_tb): + def __exit__(self, exc_type, exc_val, exc_tb) -> None: pass diff --git a/teuthology/provision/downburst.py b/teuthology/provision/downburst.py index 2a7dcdcc8e..a6512c221e 100644 --- a/teuthology/provision/downburst.py +++ b/teuthology/provision/downburst.py @@ -3,6 +3,7 @@ import os import subprocess import tempfile +from typing import Dict, List, Optional, Tuple import yaml from teuthology.config import config @@ -14,7 +15,7 @@ log = logging.getLogger(__name__) -def get_types(): +def get_types() -> List[str]: types = ['vps'] if 'downburst' in config and 'machine' in config.downburst: machine = config.downburst.get('machine') @@ -23,7 +24,7 @@ def get_types(): return types -def downburst_executable(): +def downburst_executable() -> str: """ First check for downburst in the user's path. Then check in ~/src, ~ubuntu/src, and ~teuthology/src. @@ -51,9 +52,11 @@ def downburst_executable(): return '' -def downburst_environment(): - env = dict() - env['PATH'] = os.environ.get('PATH') +def downburst_environment() -> Dict[str, str]: + env: Dict[str, str] = dict() + path = os.environ.get('PATH') + if path: + env['PATH'] = path discover_url = os.environ.get('DOWNBURST_DISCOVER_URL') if config.downburst and not discover_url: if isinstance(config.downburst, dict): @@ -68,8 +71,15 @@ class Downburst(object): A class that provides methods for creating and destroying virtual machine instances using downburst: https://github.com/ceph/downburst """ - def __init__(self, name, os_type, os_version, status=None, user='ubuntu', - logfile=None): + def __init__( + self, + name: str, + os_type: Optional[str], + os_version: Optional[str], + status: Optional[dict] = None, + user: str = 'ubuntu', + logfile: Optional[str] = None + ) -> None: self.name = name self.shortname = decanonicalize_hostname(self.name) self.os_type = os_type @@ -83,7 +93,7 @@ def __init__(self, name, os_type, os_version, status=None, user='ubuntu', self.executable = downburst_executable() self.environment = downburst_environment() - def create(self): + def create(self) -> Optional[bool]: """ Launch a virtual machine instance. @@ -122,7 +132,7 @@ def create(self): break return success - def _run_create(self): + def _run_create(self) -> Tuple[int, str, str]: """ Used by create(), this method is what actually calls downburst when creating a virtual machine instance. @@ -154,7 +164,7 @@ def _run_create(self): out, err = proc.communicate() return (proc.returncode, out, err) - def destroy(self): + def destroy(self) -> bool: """ Destroy (shutdown and delete) a virtual machine instance. """ @@ -185,25 +195,27 @@ def destroy(self): log.info("Destroyed %s%s" % (self.name, out_str)) return True - def build_config(self): + def build_config(self) -> bool: """ Assemble a configuration to pass to downburst, and write it to a file. """ config_fd = tempfile.NamedTemporaryFile(delete=False, mode='wt') + if not self.os_type or not self.os_version: + raise ValueError("os_type and os_version must be set") os_type = self.os_type.lower() os_version = self.os_version.lower() mac_address = self.status['mac_address'] - machine = dict( - disk=os.environ.get('DOWNBURST_DISK_SIZE', '100G'), - ram=os.environ.get('DOWNBURST_RAM_SIZE', '3.8G'), - cpus=int(os.environ.get('DOWNBURST_CPUS', 1)), - volumes=dict( - count=int(os.environ.get('DOWNBURST_EXTRA_DISK_NUMBER', 4)), - size=os.environ.get('DOWNBURST_EXTRA_DISK_SIZE', '100G'), - ), - ) + machine: dict = { + 'disk': os.environ.get('DOWNBURST_DISK_SIZE', '100G'), + 'ram': os.environ.get('DOWNBURST_RAM_SIZE', '3.8G'), + 'cpus': int(os.environ.get('DOWNBURST_CPUS', 1)), + 'volumes': { + 'count': int(os.environ.get('DOWNBURST_EXTRA_DISK_NUMBER', 4)), + 'size': os.environ.get('DOWNBURST_EXTRA_DISK_SIZE', '100G'), + }, + } def belongs_machine_type(machine_config: dict, machine_type: str) -> bool: if isinstance(machine_config, dict): t = machine_config.get('type', None) @@ -212,15 +224,19 @@ def belongs_machine_type(machine_config: dict, machine_type: str) -> bool: elif isinstance(t, list): return machine_type in t return False + machine_config: Optional[dict] = None if isinstance(config.downburst, dict) and isinstance(config.downburst.get('machine'), list): machine_type = self.status['machine_type'] - machine_config = next((m for m in config.downburst.get('machine') - if belongs_machine_type(m, machine_type)), None) - if machine_config is None: - raise RuntimeError(f"Cannot find config for machine type {machine_type}.") + machine_list = config.downburst.get('machine') + if machine_list: + machine_config = next((m for m in machine_list + if belongs_machine_type(m, machine_type)), None) + if machine_config is None: + raise RuntimeError(f"Cannot find config for machine type {machine_type}.") elif isinstance(config.downburst, dict) and isinstance(config.downburst.get('machine'), dict): machine_config = config.downburst.get('machine') - deep_merge(machine, machine_config) + if machine_config: + deep_merge(machine, machine_config) log.debug('Using machine config: %s', machine) file_info = { 'disk-size': machine['disk'], @@ -247,7 +263,7 @@ def belongs_machine_type(machine_config: dict, machine_type: str) -> bool: # Remove the user's password so console logins are possible 'runcmd': [ ['passwd', '-d', self.user], - ] + ], } # for opensuse-15.2 we need to replace systemd-logger with rsyslog for teuthology if os_type == 'opensuse' and os_version == '15.2': @@ -257,18 +273,16 @@ def belongs_machine_type(machine_config: dict, machine_type: str) -> bool: ]) # Install git on downbursted VMs to clone upstream linux-firmware. # Issue #17154 - if 'packages' not in user_info: - user_info['packages'] = list() - user_info['packages'].extend([ + packages: List[str] = [ 'git', 'wget', - ]) + ] if os_type in ('centos', 'opensuse'): - user_info['packages'].extend([ + packages.extend([ 'chrony', ]) if os_type in ('ubuntu', 'debian'): - user_info['packages'].extend([ + packages.extend([ 'ntp', ]) @@ -281,19 +295,20 @@ def belongs_machine_type(machine_config: dict, machine_type: str) -> bool: # On Ubuntu, starting with 16.04, and Fedora, starting with 24, we need # to install 'python' to get python2.7, which ansible needs if os_type in ('ubuntu', 'fedora'): - user_info['packages'].append('python') + packages.append('python') if os_type in ('centos', 'rocky'): - user_info['packages'].extend([ + packages.extend([ 'python3-pip', 'bind-utils', ]) user_fd = tempfile.NamedTemporaryFile(delete=False, mode='wt') + user_info['packages'] = packages user_str = "#cloud-config\n" + yaml.safe_dump(user_info) user_fd.write(user_str) self.user_path = user_fd.name return True - def remove_config(self): + def remove_config(self) -> bool: """ Remove the downburst configuration file created by build_config() """ @@ -307,7 +322,7 @@ def remove_config(self): return True return False - def __del__(self): + def __del__(self) -> None: self.remove_config() @@ -333,7 +348,7 @@ def __del__(self): ], } -def get_distro_from_downburst(): +def get_distro_from_downburst() -> Dict[str, List[str]]: """ Return a table of valid distros. diff --git a/teuthology/provision/fog.py b/teuthology/provision/fog.py index bd7256b7ed..965d633384 100644 --- a/teuthology/provision/fog.py +++ b/teuthology/provision/fog.py @@ -4,11 +4,12 @@ import requests import socket import re +from typing import List, Optional from paramiko import SSHException from paramiko.ssh_exception import NoValidConnectionsError -import teuthology.orchestra +import teuthology.orchestra.remote from teuthology.config import config from teuthology.contextutil import safe_while @@ -19,7 +20,7 @@ log = logging.getLogger(__name__) -def enabled(warn=False): +def enabled(warn: bool = False) -> bool: """ Check for required FOG settings @@ -37,7 +38,7 @@ def enabled(warn=False): return (unset == []) -def get_types(): +def get_types() -> List[str]: """ Fetch and parse config.fog['machine_types'] @@ -59,7 +60,7 @@ class FOG(object): """ timestamp_format = '%Y-%m-%d %H:%M:%S' - def __init__(self, name, os_type, os_version): + def __init__(self, name: str, os_type: str, os_version: str) -> None: self.remote = teuthology.orchestra.remote.Remote( misc.canonicalize_hostname(name)) self.name = self.remote.hostname @@ -68,7 +69,7 @@ def __init__(self, name, os_type, os_version): self.os_version = os_version self.log = log.getChild(self.shortname) - def create(self): + def create(self) -> None: """ Initiate deployment and wait until completion """ @@ -93,7 +94,13 @@ def create(self): self._verify_installed_os() self.log.info("Deploy complete!") - def do_request(self, url_suffix, data=None, method='GET', verify=True): + def do_request( + self, + url_suffix: str, + data: Optional[str] = None, + method: str = 'GET', + verify: bool = True + ) -> requests.Response: """ A convenience method to submit a request to the FOG server :param url_suffix: The portion of the URL to append to the endpoint, @@ -104,12 +111,12 @@ def do_request(self, url_suffix, data=None, method='GET', verify=True): unsuccessful (default: True) :returns: A requests.models.Response object """ - req_kwargs = dict( - headers={ + req_kwargs: dict = { + 'headers': { 'fog-api-token': config.fog['api_token'], 'fog-user-token': config.fog['user_token'], }, - ) + } if data is not None: req_kwargs['data'] = data req = requests.Request( @@ -125,7 +132,7 @@ def do_request(self, url_suffix, data=None, method='GET', verify=True): resp.raise_for_status() return resp - def get_host_data(self): + def get_host_data(self) -> dict: """ Locate the host we want to use, and return the FOG object which represents it @@ -143,13 +150,13 @@ def get_host_data(self): "More than one host found for %s" % self.shortname) return obj['hosts'][0] - def get_image_data(self): + def get_image_data(self) -> dict: """ Locate the image we want to use, and return the FOG object which represents it :returns: A dict describing the image """ - def do_get(name): + def do_get(name: str) -> Optional[dict]: resp = self.do_request( '/image', data=json.dumps(dict(name=name)), @@ -172,7 +179,7 @@ def do_get(name): "Fog has no %s image. Available %s images: %s" % (name, self.remote.machine_type, self.suggest_image_names())) - def suggest_image_names(self): + def suggest_image_names(self) -> List[str]: """ Suggest available image names for this machine type. @@ -183,7 +190,7 @@ def suggest_image_names(self): images = obj['images'] return [image['name'] for image in images] - def set_image(self, host_id): + def set_image(self, host_id: int) -> None: """ Tell FOG to use the proper image on the next deploy :param host_id: The id of the host to deploy @@ -198,7 +205,7 @@ def set_image(self, host_id): data=json.dumps(dict(imageID=image_id)), ) - def schedule_deploy_task(self, host_id): + def schedule_deploy_task(self, host_id: int) -> str: """ :param host_id: The id of the host to deploy :returns: The id of the scheduled task @@ -234,8 +241,9 @@ def schedule_deploy_task(self, host_id): # case there are multiple, select a very recent one. if time_delta < 5: return task['id'] + raise RuntimeError(f"Could not find deploy task for host {self.shortname}") - def get_deploy_tasks(self): + def get_deploy_tasks(self) -> List[dict]: """ :returns: A list of deploy tasks which are active on our host """ @@ -249,7 +257,7 @@ def get_deploy_tasks(self): if obj['host']['name'] == self.shortname] return host_tasks - def deploy_task_active(self, task_id): + def deploy_task_active(self, task_id: str) -> bool: """ :param task_id: The id of the task to query :returns: True if the task is active @@ -259,7 +267,7 @@ def deploy_task_active(self, task_id): [task['id'] == task_id for task in host_tasks] ) - def wait_for_deploy_task(self, task_id): + def wait_for_deploy_task(self, task_id: str) -> None: """ Wait until the specified task is no longer active (i.e., it has completed) @@ -270,7 +278,7 @@ def wait_for_deploy_task(self, task_id): if not self.deploy_task_active(task_id): break - def cancel_deploy_task(self, task_id): + def cancel_deploy_task(self, task_id: str) -> None: """ Cancel an active deploy task """ self.log.debug(f"Canceling deploy task with ID {task_id}") resp = self.do_request( @@ -280,7 +288,7 @@ def cancel_deploy_task(self, task_id): ) resp.raise_for_status() - def _wait_for_ready(self): + def _wait_for_ready(self) -> None: """ Attempt to connect to the machine via SSH (power cycle once at 50% of timeout). """ @@ -342,7 +350,7 @@ def _wait_for_ready(self): log.error(f"{e} on {self.shortname}") self.log.info("Node is ready") - def _fix_hostname(self): + def _fix_hostname(self) -> None: """ After a reimage, the host will still have the hostname of the machine used to create the image initially. Fix that by making a call to @@ -374,7 +382,7 @@ def _fix_hostname(self): check_status=False, ) - def _verify_installed_os(self): + def _verify_installed_os(self) -> None: wanted_os = OS(name=self.os_type, version=self.os_version) if self.remote.os != wanted_os: raise RuntimeError( @@ -382,6 +390,6 @@ def _verify_installed_os(self): f"found {self.remote.os}" ) - def destroy(self): + def destroy(self) -> None: """A no-op; we just leave idle nodes as-is""" pass diff --git a/teuthology/provision/maas.py b/teuthology/provision/maas.py index 5d30510421..b29d0ed752 100644 --- a/teuthology/provision/maas.py +++ b/teuthology/provision/maas.py @@ -5,9 +5,10 @@ from oauthlib.oauth1 import SIGNATURE_PLAINTEXT from requests import Response from requests_oauthlib import OAuth1Session -from typing import Any, Dict, List, Optional, Union +from typing import List, Optional import teuthology.orchestra +import teuthology.orchestra.remote from teuthology.config import config from teuthology.contextutil import safe_while @@ -110,15 +111,19 @@ def __init__( _info = self.get_machines_data() self.system_id = _info.get("system_id") - self.cpu_arch, arch_variant = _info.get("architecture").split("/") + architecture = _info.get("architecture", "") + if architecture: + self.cpu_arch, arch_variant = architecture.split("/") + else: + self.cpu_arch = "" def do_request( self, path: str, method: str = "GET", - params: Optional[Dict[str, Any]] = None, - data: Optional[Union[Dict[str, Any], list]] = None, - files: Optional[Dict[str, Any]] = None, + params: Optional[dict] = None, + data: Optional[dict[str, bool]] = None, + files: Optional[dict] = None, raise_on_error: bool = True ) -> Response: """Submit a request to the MAAS server @@ -133,7 +138,7 @@ def do_request( :returns: A Response object from the requests library. """ - args: Dict[str, Any] = {"url": f"{config.maas['api_url'].strip('/')}/{path}"} + args: dict = {"url": f"{config.maas['api_url'].strip('/')}/{path}"} args["data"] = json.dumps(data) if data else None args["params"] = params if params else None args["files"] = files if files else None @@ -160,7 +165,7 @@ def do_request( return resp - def get_machines_data(self, interval: int = 3, timeout: int = 30) -> Dict[str, Any]: + def get_machines_data(self, interval: int = 3, timeout: int = 30) -> dict: """Locate the machine we want to use :param interval: Time to wait between retries, in seconds (default: 3) @@ -203,7 +208,7 @@ def get_image_name(self) -> str: return f"{self.os_type}/{self.os_type}{os_version}" return f"{self.os_type}/{self.os_version}" - def get_image_data(self) -> Dict[str, Any]: + def get_image_data(self) -> dict: """Locate the image we want to use :returns: The image data as a dictionary @@ -252,7 +257,7 @@ def unlock_machine(self) -> None: def deploy_machine(self) -> None: """Deploy the machine""" - image_data: Dict[str, Any] = self.get_image_data() + image_data: dict = self.get_image_data() if image_data.get("type", "").lower() not in ["synced", "uploaded"]: raise RuntimeError( f"MaaS image {image_data.get('name')} is not synced, " @@ -267,7 +272,7 @@ def deploy_machine(self) -> None: "distro_series": (None, image_data.get("name")), "user_data": (None, self._get_user_data()), } - data: Dict[str, Any] = self.do_request( + data: dict = self.do_request( f"/machines/{self.system_id}/op-deploy", method="POST", files=files ).json() if data.get("status_name", "").lower() != "deploying": @@ -282,8 +287,8 @@ def release_machine(self, erase: bool = False) -> None: :param erase: Optional parameter to indicate whether to erase disks (default: False) """ - data: Dict[str, bool] = {"erase": erase} - resp: Dict[str, Any] = self.do_request( + data: dict[str, bool] = {"erase": erase} + resp: dict = self.do_request( f"/machines/{self.system_id}/op-release", method="POST", data=data ).json() if resp.get("status_name", "").lower() not in ["disk erasing", "releasing", "ready"]: @@ -347,7 +352,7 @@ def create(self) -> None: def release(self) -> None: """Release the machine""" - machine_data: Dict[str, Any] = self.get_machines_data() + machine_data: dict = self.get_machines_data() status_name = machine_data.get("status_name", "").lower() if status_name in ["new", "allocated", "ready"]: @@ -373,7 +378,7 @@ def release(self) -> None: self.log.info(f"Releasing machine '{self.shortname}'") self.release_machine() - def _get_user_data(self) -> Optional[io.BytesIO]: + def _get_user_data(self) -> io.BufferedReader|None: """Get user data for cloud-init :returns: BytesIO object containing formatted user data, or None if no template @@ -404,7 +409,7 @@ def _wait_for_status( sleep=interval, timeout=int(config.maas.get("timeout", timeout)) ) as proceed: while proceed(): - maas_machine: Dict[str, Any] = self.get_machines_data() + maas_machine: dict = self.get_machines_data() status_name = maas_machine["status_name"] if status_name.lower() == status.lower(): log.info( diff --git a/teuthology/provision/openstack.py b/teuthology/provision/openstack.py index d829b4ee55..e52bede05a 100644 --- a/teuthology/provision/openstack.py +++ b/teuthology/provision/openstack.py @@ -6,6 +6,7 @@ import subprocess import time import tempfile +from typing import List, Optional from subprocess import CalledProcessError @@ -25,7 +26,7 @@ class ProvisionOpenStack(OpenStack): A class that provides methods for creating and destroying virtual machine instances using OpenStack """ - def __init__(self): + def __init__(self) -> None: super(ProvisionOpenStack, self).__init__() fd, self.user_data = tempfile.mkstemp() os.close(fd) @@ -34,11 +35,11 @@ def __init__(self): self.up_string = 'The system is finally up' self.property = "%16x" % random.getrandbits(128) - def __del__(self): + def __del__(self) -> None: if os.path.exists(self.user_data): os.unlink(self.user_data) - def init_user_data(self, os_type, os_version): + def init_user_data(self, os_type: str, os_version: str) -> None: """ Get the user-data file that is fit for os_type and os_version. It is responsible for setting up enough for ansible to take @@ -56,7 +57,7 @@ def init_user_data(self, os_type, os_version): lab_domain=config.lab_domain) open(self.user_data, 'w').write(user_data) - def _openstack(self, subcommand, get=None): + def _openstack(self, subcommand: str, get: Optional[str] = None): # do not use OpenStack().run because its # bugous for volume create as of openstackclient 3.2.0 # https://bugs.launchpad.net/python-openstackclient/+bug/1619726 @@ -67,7 +68,7 @@ def _openstack(self, subcommand, get=None): return self.get_value(r, get) return r - def _create_volume(self, volume_name, size): + def _create_volume(self, volume_name: str, size: int) -> str: """ Create a volume and return valume id """ @@ -91,7 +92,7 @@ def _create_volume(self, volume_name, size): else: raise Exception("Failed to create volume %s" % volume_name) - def _await_volume_status(self, volume_id, status='available'): + def _await_volume_status(self, volume_id: str, status: str = 'available') -> None: """ Wait for volume to have status, like 'available' or 'in-use' """ @@ -110,7 +111,7 @@ def _await_volume_status(self, volume_id, status='available'): log.warning("volume " + volume_id + " not information available yet") - def _attach_volume(self, volume_id, name): + def _attach_volume(self, volume_id: str, name: str) -> None: """ Attach volume to OpenStack instance. @@ -126,7 +127,7 @@ def _attach_volume(self, volume_id, name): log.warning("openstack add volume failed unexpectedly; retrying") self._await_volume_status(volume_id, 'in-use') - def attach_volumes(self, server_name, volumes): + def attach_volumes(self, server_name: str, volumes: dict) -> None: """ Create and attach volumes to the named OpenStack instance. If attachment is failed, make another try. @@ -148,14 +149,21 @@ def attach_volumes(self, server_name, volumes): OpenStack().volume_delete(volume_id) @staticmethod - def ip2name(prefix, ip): + def ip2name(prefix: str, ip: str) -> str: """ return the instance name suffixed with the IP address. """ digits = map(int, re.findall(r'(\d+)\.(\d+)\.(\d+)\.(\d+)', ip)[0]) return prefix + "%03d%03d%03d%03d" % tuple(digits) - def create(self, num, os_type, os_version, arch, resources_hint): + def create( + self, + num: int, + os_type: str, + os_version: str, + arch: Optional[str], + resources_hint: dict + ) -> List[str]: """ Create num OpenStack instances running os_type os_version and return their names. Each instance has at least the resources @@ -230,6 +238,6 @@ def create(self, num, os_type, os_version, arch, resources_hint): raise e return fqdns - def destroy(self, name_or_id): + def destroy(self, name_or_id: str) -> bool: log.debug('ProvisionOpenStack:destroy ' + name_or_id) return OpenStackInstance(name_or_id).destroy() diff --git a/teuthology/provision/pelagos.py b/teuthology/provision/pelagos.py index 5dd04a4fae..90fe92289c 100644 --- a/teuthology/provision/pelagos.py +++ b/teuthology/provision/pelagos.py @@ -3,11 +3,12 @@ import requests import re import time +from typing import Dict, List, Optional from teuthology.config import config from teuthology.contextutil import safe_while from teuthology.misc import canonicalize_hostname -from teuthology.util.compat import HTTPError +from teuthology.util.compat import HTTPError log = logging.getLogger(__name__) config_section = 'pelagos' @@ -15,7 +16,7 @@ # Provisioner configuration section description see in # docs/siteconfig.rst -def enabled(warn=False): +def enabled(warn: bool = False) -> bool: """ Check for required Pelagos settings @@ -33,7 +34,7 @@ def enabled(warn=False): return (unset == []) -def get_types(): +def get_types() -> List[str]: """ Fetch and parse config.pelagos['machine_types'] @@ -48,14 +49,14 @@ def get_types(): types = [_ for _ in types.split(',') if _] return [_ for _ in types if _] -def park_node(name): +def park_node(name: str) -> None: p = Pelagos(name, "maintenance_image") p.create(wait=False) class Pelagos(object): - def __init__(self, name, os_type, os_version=""): + def __init__(self, name: str, os_type: str, os_version: str = "") -> None: #for service should be a hostname, not a user@host split_uri = re.search(r'(\w*)@(.+)', canonicalize_hostname(name)) if split_uri is not None: @@ -71,7 +72,7 @@ def __init__(self, name, os_type, os_version=""): self.os_name = os_type self.log = log.getChild(self.name) - def create(self, wait=True): + def create(self, wait: bool = True): """ Initiate deployment via REST requests and wait until completion :param wait: optional, by default is True, if set to False, function @@ -84,7 +85,7 @@ def create(self, wait=True): """ if not enabled(): raise RuntimeError("Pelagos is not configured!") - location = None + location: Optional[str] = None try: params = dict(os=self.os_name, node=self.name) response = self.do_request('node/provision', @@ -92,6 +93,8 @@ def create(self, wait=True): if not wait: return response location = response.headers.get('Location') + if not location: + raise RuntimeError("No Location header in response") self.log.debug("provision task: '%s'", location) # gracefully wait till provision task gets created on pelagos time.sleep(2) @@ -109,25 +112,25 @@ def create(self, wait=True): self.log.error("Failed to start deploy tasks!") raise e self.log.info("Deploy complete!") - if self.task_status_response.status_code != 200: + if hasattr(self.task_status_response, 'status_code') and self.task_status_response.status_code != 200: raise Exception("Deploy failed") return self.task_status_response - def cancel_deploy_task(self, task_id): + def cancel_deploy_task(self, task_id: str) -> None: # TODO implement it return - def is_task_active(self, task_url): + def is_task_active(self, task_url: str) -> bool: try: status_response = self.do_request('', url=task_url, verify=False) except HTTPError as err: self.log.error("Task fail reason: '%s'", err.reason) - if err.status_code == 404: + if hasattr(err, 'status_code') and err.status_code == 404: self.log.error(err.reason) self.task_status_response = 'failed' return False else: - raise HTTPError(err.code, err.reason) + raise self.log.debug("Response code '%s'", str(status_response.status_code)) self.task_status_response = status_response @@ -138,7 +141,14 @@ def is_task_active(self, task_url): return True return False - def do_request(self, url_suffix, url="" , data=None, method='GET', verify=True): + def do_request( + self, + url_suffix: str, + url: str = "", + data: Optional[Dict[str, str]] = None, + method: str = 'GET', + verify: bool = True + ) -> requests.Response: """ A convenience method to submit a request to the Pelagos server :param url_suffix: The portion of the URL to append to the endpoint, @@ -167,7 +177,7 @@ def do_request(self, url_suffix, url="" , data=None, method='GET', verify=True): resp.raise_for_status() return resp - def destroy(self): + def destroy(self) -> None: """A no-op; we just leave idle nodes as-is""" pass diff --git a/teuthology/repo_utils.py b/teuthology/repo_utils.py index eab140767e..3a8cebc09c 100644 --- a/teuthology/repo_utils.py +++ b/teuthology/repo_utils.py @@ -280,6 +280,7 @@ def clone_repo(repo_url, dest_path, branch, shallow=True): stderr=subprocess.STDOUT) not_found_str = "Remote branch %s not found" % branch + assert proc.stdout out = proc.stdout.read().decode() result = proc.wait() # Newer git versions will bail if the branch is not found, but older ones @@ -358,6 +359,7 @@ def set_remote(repo_path, repo_url): cwd=repo_path, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + assert proc.stdout if proc.wait() != 0: out = proc.stdout.read() log.error(out) @@ -377,6 +379,7 @@ def fetch(repo_path): cwd=repo_path, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + assert proc.stdout if proc.wait() != 0: out = proc.stdout.read().decode() log.error(out) @@ -404,6 +407,7 @@ def fetch_branch(repo_path, branch, shallow=True): cwd=repo_path, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + assert proc.stdout if proc.wait() != 0: not_found_str = "fatal: couldn't find remote ref %s" % branch out = proc.stdout.read().decode() @@ -523,7 +527,9 @@ def url_to_dirname(url): file:///my/dir/has/ceph.git -> my_dir_has_ceph """ # Strip protocol from left-hand side - string = re.match('(?:.*://|.*@)(.*)', url).group(1) + match = re.match('(?:.*://|.*@)(.*)', url) + assert match, "URL is invalid" + string = match.group(1) # Strip '.git' from the right-hand side if string.endswith('.git'): string = string[:-4] diff --git a/teuthology/safepath.py b/teuthology/safepath.py index b8115a25ed..0e239d57d8 100644 --- a/teuthology/safepath.py +++ b/teuthology/safepath.py @@ -1,7 +1,8 @@ import errno import os -def munge(path): + +def munge(path: str) -> str: """ Munge a potentially hostile path name to be safe to use. @@ -24,7 +25,7 @@ def munge(path): return '/'.join(segments) -def makedirs(root, path): +def makedirs(root: str, path: str) -> None: """ os.makedirs gets confused if the path contains '..', and root might. diff --git a/teuthology/suite/__init__.py b/teuthology/suite/__init__.py index 5c87197eee..ce24a0b783 100644 --- a/teuthology/suite/__init__.py +++ b/teuthology/suite/__init__.py @@ -7,6 +7,7 @@ import random import sys import time +from typing import Optional import teuthology from teuthology.config import config, YamlConfig @@ -20,7 +21,7 @@ log = logging.getLogger(__name__) -def override_arg_defaults(name, default, env=os.environ): +def override_arg_defaults(name: str, default: Optional[str], env=os.environ) -> Optional[str]: env_arg = { '--ceph-repo' : 'TEUTH_CEPH_REPO', '--suite-repo' : 'TEUTH_SUITE_REPO', @@ -38,7 +39,7 @@ def override_arg_defaults(name, default, env=os.environ): return default -def process_args(args): +def process_args(args: dict) -> YamlConfig: conf = YamlConfig() rename_args = { 'ceph': 'ceph_branch', @@ -85,10 +86,10 @@ def process_args(args): return conf -def normalize_suite_name(name): +def normalize_suite_name(name: str) -> str: return name.replace('/', ':') -def expand_short_repo_name(name, orig): +def expand_short_repo_name(name: str, orig: str) -> str: # Allow shortname repo name 'foo' or 'foo/bar'. This works with # github URLs, e.g. # @@ -108,7 +109,7 @@ def expand_short_repo_name(name, orig): # otherwise, assume a full URL return name -def main(args): +def main(args: dict) -> Optional[int]: conf = process_args(args) if conf.verbose: teuthology.log.setLevel(logging.DEBUG) @@ -143,7 +144,7 @@ def main(args): conf.archive_upload_url) -def get_rerun_conf_overrides(conf): +def get_rerun_conf_overrides(conf: YamlConfig) -> None: reporter = ResultsReporter() run = reporter.get_run(conf.rerun) @@ -203,8 +204,8 @@ def get_rerun_conf_overrides(conf): conf.filter_in.extend(rerun_filters['descriptions']) -def get_rerun_filters(run, statuses): - filters = dict() +def get_rerun_filters(run: dict, statuses: list[str]) -> dict[str, list[str]]: + filters: dict = dict() jobs = [] for job in run['jobs']: if job['status'] in statuses: @@ -217,7 +218,7 @@ class WaitException(Exception): pass -def wait(name, max_job_time, upload_url): +def wait(name: str, max_job_time: int, upload_url: Optional[str]) -> int: stale_job = max_job_time + Run.WAIT_MAX_JOB_TIME reporter = ResultsReporter() past_unfinished_jobs = [] diff --git a/teuthology/suite/build_matrix.py b/teuthology/suite/build_matrix.py index e9ee9e60cb..a96a73d8a8 100644 --- a/teuthology/suite/build_matrix.py +++ b/teuthology/suite/build_matrix.py @@ -1,13 +1,14 @@ import logging import os import random +from typing import Optional, cast from teuthology.suite import matrix log = logging.getLogger(__name__) -def build_matrix(path, subset=None, no_nested_subset=False, seed=None): +def build_matrix(path: str, subset: Optional[tuple[int, int]] = None, no_nested_subset: bool = False, seed: Optional[int] = None) -> list[tuple[str, list[str]]]: """ Return a list of items descibed by path such that if the list of items is chunked into mincyclicity pieces, each piece is still a @@ -63,17 +64,21 @@ def build_matrix(path, subset=None, no_nested_subset=False, seed=None): return generate_combinations(path, mat, first, matlimit) -def _get_matrix(path, subset=None, no_nested_subset=False): +def _get_matrix(path: str, subset: Optional[tuple[int, int]] = None, no_nested_subset: bool = False) -> tuple[matrix.Matrix, int, int]: (which, divisions) = (0,1) if subset is None else subset + mat: matrix.Matrix if divisions > 1: - mat = _build_matrix(path, mincyclicity=divisions, no_nested_subset=no_nested_subset) - mat = matrix.Subset(mat, divisions, which=which) + built_mat = _build_matrix(path, mincyclicity=divisions, no_nested_subset=no_nested_subset) + assert built_mat is not None + mat = matrix.Subset(built_mat, divisions, which=which) else: - mat = _build_matrix(path, no_nested_subset=no_nested_subset) - return mat, 0, mat.size() + built_mat = _build_matrix(path, no_nested_subset=no_nested_subset) + assert built_mat is not None + mat = built_mat + return mat, 0, cast(int, mat.size()) -def _build_matrix(path, mincyclicity=0, no_nested_subset=False, item=''): +def _build_matrix(path: str, mincyclicity: int = 0, no_nested_subset: bool = False, item: str = '') -> Optional[matrix.Matrix]: if os.path.basename(path)[0] == '.': return None if not os.path.exists(path): @@ -167,7 +172,7 @@ def _build_matrix(path, mincyclicity=0, no_nested_subset=False, item=''): return None -def generate_combinations(path, mat, generate_from, generate_to): +def generate_combinations(path: str, mat: matrix.Matrix, generate_from: int, generate_to: int) -> list[tuple[str, list[str]]]: """ Return a list of items describe by path @@ -200,7 +205,7 @@ def generate_combinations(path, mat, generate_from, generate_to): return ret -def combine_path(left, right): +def combine_path(left: str, right: Optional[str]) -> str: """ os.path.join(a, b) doesn't like it when b is None """ diff --git a/teuthology/suite/matrix.py b/teuthology/suite/matrix.py index e713bc4433..b39addecf8 100644 --- a/teuthology/suite/matrix.py +++ b/teuthology/suite/matrix.py @@ -3,20 +3,21 @@ import heapq from math import gcd from functools import reduce +from typing import Callable, Optional, Iterator -def lcm(a, b): +def lcm(a: int, b: int) -> int: return a*b // gcd(a, b) -def lcml(l): +def lcml(l: Iterator[int]) -> int: return reduce(lcm, l) class Matrix: """ Interface for sets """ - def size(self): - pass + def size(self) -> int: + ... - def index(self, i): + def index(self, i: int): """ index() should return a recursive structure represending the paths to concatenate for index i: @@ -32,23 +33,23 @@ def index(self, i): """ pass - def minscanlen(self): + def minscanlen(self) -> int: """ min run require to get a good sample """ - pass + ... - def cyclicity(self): + def cyclicity(self) -> int: """ A cyclicity of N means that the set represented by the Matrix can be chopped into N good subsets of sequential indices. """ return self.size() // self.minscanlen() - def tostr(self, depth): - pass + def tostr(self, depth: int) -> str: + ... - def __str__(self): + def __str__(self) -> str: """ str method """ @@ -59,20 +60,20 @@ class Cycle(Matrix): """ Run a matrix multiple times """ - def __init__(self, num, mat): + def __init__(self, num: int, mat: 'Matrix') -> None: self.mat = mat self.num = num - def size(self): + def size(self) -> int: return self.mat.size() * self.num - def index(self, i): + def index(self, i: int): return self.mat.index(i % self.mat.size()) - def minscanlen(self): + def minscanlen(self) -> int: return self.mat.minscanlen() - def tostr(self, depth): + def tostr(self, depth: int) -> str: return '\t'*depth + "Cycle({num}):\n".format(num=self.num) + self.mat.tostr(depth + 1) # Logically, inverse of Cycle @@ -80,7 +81,7 @@ class Subset(Matrix): """ Run a matrix subset. """ - def __init__(self, mat, divisions, which=None): + def __init__(self, mat: 'Matrix', divisions: int, which: Optional[int] = None) -> None: self.mat = mat self.divisions = divisions if which is None: @@ -89,38 +90,38 @@ def __init__(self, mat, divisions, which=None): assert which < divisions self.which = which - def size(self): + def size(self) -> int: return self.mat.size() // self.divisions - def index(self, i): + def index(self, i: int): i += self.which * self.size() assert i < self.mat.size() return self.mat.index(i) - def minscanlen(self): + def minscanlen(self) -> int: return self.mat.minscanlen() - def tostr(self, depth): - return '\t'*depth + "Subset({num}, {index}):\n".format(num=self.num, index=self.index) + self.mat.tostr(depth + 1) + def tostr(self, depth: int) -> str: + return '\t'*depth + "Subset({divisions}, {which}):\n".format(divisions=self.divisions, which=self.which) + self.mat.tostr(depth + 1) class Base(Matrix): """ Just a single item. """ - def __init__(self, item): + def __init__(self, item: str) -> None: self.item = item - def size(self): + def size(self) -> int: return 1 - def index(self, i): + def index(self, i: int) -> str: return self.item - def minscanlen(self): + def minscanlen(self) -> int: return 1 - def tostr(self, depth): + def tostr(self, depth: int) -> str: return '\t'*depth + "Base({item})\n".format(item=self.item) @@ -129,7 +130,7 @@ class Product(Matrix): Builds items by taking one item from each submatrix. Contiguous subsequences should move through all dimensions. """ - def __init__(self, item, _submats): + def __init__(self, item: str, _submats: list['Matrix']) -> None: assert len(_submats) > 0, \ "Product requires child submats to be passed in" self.item = item @@ -150,17 +151,17 @@ def __init__(self, item, _submats): else: self._minscanlen += 1 - def tostr(self, depth): + def tostr(self, depth: int) -> str: ret = '\t'*depth + "Product({item}):\n".format(item=self.item) return ret + ''.join([i[1].tostr(depth+1) for i in self.submats]) - def minscanlen(self): + def minscanlen(self) -> int: return self._minscanlen - def size(self): + def size(self) -> int: return self._size - def _index(self, i, submats): + def _index(self, i: int, submats: list[tuple[int, 'Matrix']]) -> frozenset: """ We recursively reduce the N dimension problem to a two dimension problem. @@ -199,7 +200,7 @@ def combine(r, s=frozenset()): ritems = self._index(i, submats[1:]) return combine(litems, combine(ritems)) - def index(self, i): + def index(self, i: int) -> tuple[str, frozenset]: items = self._index(i, self.submats) return (self.item, items) @@ -207,24 +208,24 @@ class Concat(Matrix): """ Concatenates all items in child matrices """ - def __init__(self, item, submats): + def __init__(self, item: str, submats: list['Matrix']) -> None: self.submats = submats self.item = item - def size(self): + def size(self) -> int: return 1 - def minscanlen(self): + def minscanlen(self) -> int: return 1 - def index(self, i): + def index(self, i: int) -> tuple[str, frozenset]: out = frozenset() for submat in self.submats: for i in range(submat.size()): out = out | frozenset([submat.index(i)]) return (self.item, out) - def tostr(self, depth): + def tostr(self, depth: int) -> str: ret = '\t'*depth + "Concat({item}):\n".format(item=self.item) return ret + ''.join([i.tostr(depth+1) for i in self.submats]) @@ -232,23 +233,23 @@ class PickRandom(Matrix): """ Select a random item from the child matrices. """ - def __init__(self, item, submats): + def __init__(self, item: str, submats: list['Matrix']) -> None: self.submats = submats self.item = item - def size(self): + def size(self) -> int: return 1 - def minscanlen(self): + def minscanlen(self) -> int: return 1 - def index(self, i): + def index(self, i: int) -> tuple[str, frozenset]: indx = random.randint(0, len(self.submats) - 1) submat = self.submats[indx] out = frozenset([submat.index(indx)]) return (self.item, out) - def tostr(self, depth): + def tostr(self, depth: int) -> str: ret = '\t'*depth + "PickRandom({item}):\n".format(item=self.item) return ret + ''.join([i.tostr(depth+1) for i in self.submats]) @@ -275,7 +276,7 @@ class Sum(Matrix): precompute a mapping in the constructor (self._i_so_sis) from index to (subset_index, subset). """ - def __init__(self, item, _submats): + def __init__(self, item: str, _submats: list['Matrix']) -> None: assert len(_submats) > 0, \ f"Sum requires non-empty _submats: {item}" self.item = item @@ -314,7 +315,7 @@ def index_to_pindex_generator(submats): self._minscanlen = self.pseudo_index_to_index( max(map(sm_to_pmsl, self._submats))) - def pi_to_sis(self, pi, offset_multiple): + def pi_to_sis(self, pi: int, offset_multiple: tuple[int, int]) -> int: """ offset_multiple tuple of offset and multiple @@ -325,27 +326,27 @@ def pi_to_sis(self, pi, offset_multiple): return -1 return (pi - offset) // multiple - def pseudo_index_to_index(self, pi): + def pseudo_index_to_index(self, pi: int) -> int: """ Count all pseudoindex values <= pi with corresponding subset indices """ return sum((self.pi_to_sis(pi, i) + 1 for i, _ in self._submats)) - 1 - def tostr(self, depth): + def tostr(self, depth: int) -> str: ret = '\t'*depth + "Sum({item}):\n".format(item=self.item) return ret + ''.join([i[1].tostr(depth+1) for i in self._submats]) - def minscanlen(self): + def minscanlen(self) -> int: return self._minscanlen - def size(self): + def size(self) -> int: return self._size - def index(self, i): + def index(self, i: int) -> tuple: si, submat = self._i_to_sis[i % self._size] return (self.item, submat.index(si)) -def generate_lists(result): +def generate_lists(result) -> frozenset[tuple]: """ Generates a set of tuples representing paths to concatenate """ @@ -366,14 +367,14 @@ def generate_lists(result): return frozenset([(result,)]) -def generate_paths(path, result, joinf=os.path.join): +def generate_paths(path: str, result, joinf: Callable[[str, str], str] = os.path.join) -> list[str]: """ Generates from the result set a list of sorted paths to concatenate """ return [reduce(joinf, i, path) for i in sorted(generate_lists(result))] -def generate_desc(joinf, result): +def generate_desc(joinf: Callable[[str, str], str], result) -> str: """ Generates the text description of the test represented by result """ diff --git a/teuthology/suite/merge.py b/teuthology/suite/merge.py index 730534d165..b4a42e0cfd 100644 --- a/teuthology/suite/merge.py +++ b/teuthology/suite/merge.py @@ -1,8 +1,9 @@ import copy import logging -import lupa.lua54 as lupa +import lupa.lua54 as lupa # ty: ignore[unresolved-import] import os from types import MappingProxyType +from typing import Iterator, Optional import yaml from teuthology.config import JobConfig @@ -25,7 +26,7 @@ with open(FRAGMENT_MERGE) as f: L.execute(f.read()) -def config_merge(configs, suite_name=None, **kwargs): +def config_merge(configs: list[tuple[str, list[str]]], suite_name: Optional[str] = None, **kwargs) -> Iterator[tuple[str, list[str], dict]]: """ This procedure selects and merges YAML fragments for each job in the configs array generated for the matrix of jobs. @@ -138,7 +139,7 @@ def config_merge(configs, suite_name=None, **kwargs): txt = f.read() yaml_cache[path] = (txt, yaml.safe_load(txt)) - yaml_fragment_txt, yaml_fragment_obj = yaml_cache[path] + _, yaml_fragment_obj = yaml_cache[path] if yaml_fragment_obj is None: continue yaml_fragment_obj = copy.deepcopy(yaml_fragment_obj) diff --git a/teuthology/suite/placeholder.py b/teuthology/suite/placeholder.py index f36bcf593f..5aed625c07 100644 --- a/teuthology/suite/placeholder.py +++ b/teuthology/suite/placeholder.py @@ -6,12 +6,12 @@ class Placeholder(object): A placeholder for use with substitute_placeholders. Simply has a 'name' attribute. """ - def __init__(self, name, required=True): + def __init__(self, name: str, required: bool = True) -> None: self.name = name self.required = required -def substitute_placeholders(input_dict, values_dict): +def substitute_placeholders(input_dict: dict, values_dict: dict) -> dict: """ Replace any Placeholder instances with values named in values_dict. In the case of None values, the key is omitted from the result. diff --git a/teuthology/suite/run.py b/teuthology/suite/run.py index 7b7923367e..48130c9300 100644 --- a/teuthology/suite/run.py +++ b/teuthology/suite/run.py @@ -6,6 +6,7 @@ import yaml import re import time +from typing import Optional from pathlib import Path @@ -13,7 +14,7 @@ from teuthology import repo_utils -from teuthology.config import config, JobConfig +from teuthology.config import config, JobConfig, YamlConfig from teuthology.exceptions import ( BranchMismatchError, BranchNotFoundError, CommitNotFoundError, ) @@ -38,7 +39,7 @@ class Run(object): 'base_args', 'kernel_dict', 'config_input', 'timestamp', 'user', 'os', ) - def __init__(self, args): + def __init__(self, args: YamlConfig) -> None: """ args must be a config.YamlConfig object """ @@ -60,7 +61,7 @@ def __init__(self, args): self.base_yaml_paths = [os.path.join(self.suite_repo_path, b) for b in self.args.base_yaml_paths] - def make_run_name(self): + def make_run_name(self) -> str: """ Generate a run name. A run name looks like: teuthology-2014-06-23_19:00:37-rados-dumpling-testing-basic-plana @@ -77,7 +78,7 @@ def make_run_name(self): ] ).replace('/', ':') - def create_initial_config(self): + def create_initial_config(self) -> JobConfig: """ Put together the config file used as the basis for each job in the run. Grabs hashes for the latest ceph, kernel and teuthology versions in the @@ -136,7 +137,7 @@ def create_initial_config(self): self.config_input['os_version'] = self.args.distro_version.lower() return self.build_base_config() - def get_expiration(self, _base_time: datetime.datetime | None = None) -> datetime.datetime | None: + def get_expiration(self, _base_time: Optional[datetime.datetime] = None) -> Optional[datetime.datetime]: """ _base_time: For testing, calculate relative offsets from this base time @@ -155,7 +156,7 @@ def get_expiration(self, _base_time: datetime.datetime | None = None) -> datetim expires = _base_time + parse_offset(expires_str) return expires - def choose_os(self): + def choose_os(self) -> OS: os_type = self.args.distro os_version = self.args.distro_version if not (os_type and os_version): @@ -165,7 +166,7 @@ def choose_os(self): os_ = OS(os_type, os_version) return os_ - def choose_kernel(self): + def choose_kernel(self) -> dict: # Put together a stanza specifying the kernel hash if self.args.kernel_branch == 'distro': kernel_hash = 'distro' @@ -201,7 +202,7 @@ def choose_kernel(self): kernel_dict = dict() return kernel_dict - def choose_ceph_hash(self): + def choose_ceph_hash(self) -> str: """ Get the ceph hash: if --sha1/-S is supplied, use it if it is valid, and just keep the ceph_branch around. Otherwise use the current git branch @@ -232,10 +233,11 @@ def choose_ceph_hash(self): ) util.schedule_fail(message=str(exc), name=self.name, dry_run=self.args.dry_run) + assert ceph_hash is not None log.info("ceph sha1: {hash}".format(hash=ceph_hash)) return ceph_hash - def choose_teuthology_branch(self): + def choose_teuthology_branch(self) -> tuple[str, str]: """Select teuthology branch, check if it is present in repo and return tuple (branch, hash) where hash is commit sha1 corresponding to the HEAD of the branch. @@ -304,28 +306,29 @@ def choose_teuthology_branch(self): if not teuthology_sha1: exc = BranchNotFoundError(teuthology_branch, build_git_url('teuthology')) util.schedule_fail(message=str(exc), name=self.name, dry_run=self.args.dry_run) + assert teuthology_sha1 is not None log.info("teuthology branch: %s %s", teuthology_branch, teuthology_sha1) return teuthology_branch, teuthology_sha1 @property - def ceph_repo_name(self): + def ceph_repo_name(self) -> str: if self.args.ceph_repo: return self._repo_name(self.args.ceph_repo) else: return 'ceph' @property - def suite_repo_name(self): + def suite_repo_name(self) -> str: if self.args.suite_repo: return self._repo_name(self.args.suite_repo) else: return 'ceph-qa-suite' @staticmethod - def _repo_name(url): + def _repo_name(url: str) -> str: return re.sub(r'\.git$', '', url.split('/')[-1]) - def choose_suite_branch(self): + def choose_suite_branch(self) -> str: suite_repo_name = self.suite_repo_name suite_repo_project_or_url = self.args.suite_repo or 'ceph-qa-suite' suite_branch = self.args.suite_branch @@ -351,7 +354,7 @@ def choose_suite_branch(self): suite_branch = 'main' return suite_branch - def choose_suite_hash(self, suite_branch): + def choose_suite_hash(self, suite_branch: str) -> str: suite_repo_name = self.suite_repo_name suite_hash = None if self.args.suite_sha1: @@ -374,10 +377,11 @@ def choose_suite_hash(self, suite_branch): if not suite_hash: exc = BranchNotFoundError(suite_branch, suite_repo_name) util.schedule_fail(message=str(exc), name=self.name, dry_run=self.args.dry_run) + assert suite_hash is not None log.info("%s branch: %s %s", suite_repo_name, suite_branch, suite_hash) return suite_hash - def build_base_config(self): + def build_base_config(self) -> JobConfig: conf_dict = substitute_placeholders(dict_templ, self.config_input) conf_dict.update(self.kernel_dict) job_config = JobConfig.from_dict(conf_dict) @@ -398,7 +402,7 @@ def build_base_config(self): job_config.rocketchat = self.args.rocketchat return job_config - def build_base_args(self): + def build_base_args(self) -> list[str]: base_args = [ '--name', self.name, '--worker', util.get_worker(self.args.machine_type), @@ -416,7 +420,7 @@ def build_base_args(self): return base_args - def write_rerun_memo(self): + def write_rerun_memo(self) -> None: args = copy.deepcopy(self.base_args) args.append('--first-in-suite') if self.args.subset: @@ -432,7 +436,7 @@ def write_rerun_memo(self): log_prefix="Memo: ") - def write_result(self): + def write_result(self) -> None: arg = copy.deepcopy(self.base_args) arg.append('--last-in-suite') if self.base_config.email: @@ -449,7 +453,7 @@ def write_result(self): log.info("Test results viewable at %s", results_url) - def prepare_and_schedule(self): + def prepare_and_schedule(self) -> None: """ Puts together some "base arguments" with which to execute teuthology-schedule for each job, then passes them and other parameters @@ -469,7 +473,7 @@ def prepare_and_schedule(self): if num_jobs: self.write_result() - def collect_jobs(self, arch, configs, newest=False, limit=0): + def collect_jobs(self, arch: str|None, configs: list[tuple[str, list[str], dict]], newest: bool = False, limit: int = 0) -> tuple[list[dict], list[dict]]: jobs_to_schedule = [] jobs_missing_packages = [] for description, fragment_paths, parsed_yaml in configs: @@ -538,7 +542,7 @@ def collect_jobs(self, arch, configs, newest=False, limit=0): jobs_to_schedule.append(job) return jobs_missing_packages, jobs_to_schedule - def schedule_jobs(self, jobs_missing_packages, jobs_to_schedule, name): + def schedule_jobs(self, jobs_missing_packages: list[dict], jobs_to_schedule: list[dict], name: str) -> None: for job in jobs_to_schedule: log.info( 'Scheduling %s', job['desc'] @@ -566,7 +570,7 @@ def schedule_jobs(self, jobs_missing_packages, jobs_to_schedule, name): log.info("pause between jobs : --throttle " + str(throttle)) time.sleep(int(throttle)) - def check_priority(self, jobs_to_schedule): + def check_priority(self, jobs_to_schedule: int) -> None: priority = self.args.priority msg=f'''Unable to schedule {jobs_to_schedule} jobs with priority {priority}. @@ -585,7 +589,7 @@ def check_priority(self, jobs_to_schedule): elif priority < 150 and jobs_to_schedule > 100: util.schedule_fail(msg, dry_run=self.args.dry_run) - def check_num_jobs(self, jobs_to_schedule): + def check_num_jobs(self, jobs_to_schedule: int) -> None: """ Fail schedule if number of jobs exceeds job threshold. """ @@ -596,7 +600,7 @@ def check_num_jobs(self, jobs_to_schedule): if threshold and jobs_to_schedule > threshold: util.schedule_fail(msg, dry_run=self.args.dry_run) - def schedule_suite(self): + def schedule_suite(self) -> int: """ Schedule the suite-run. Returns the number of jobs scheduled. """ diff --git a/teuthology/suite/util.py b/teuthology/suite/util.py index cc884ebf90..3f66bcde8c 100644 --- a/teuthology/suite/util.py +++ b/teuthology/suite/util.py @@ -7,6 +7,7 @@ import socket from subprocess import Popen, PIPE, DEVNULL import sys +from typing import Optional from email.mime.text import MIMEText @@ -29,7 +30,7 @@ CONTAINER_FLAVOR = 'default' -def fetch_repos(branch, test_name, dry_run, commit=None): +def fetch_repos(branch: str, test_name: str, dry_run: bool, commit: Optional[str] = None) -> str: """ Fetch the suite repo (and also the teuthology repo) so that we can use it to build jobs. Repos are stored in ~/src/. @@ -42,6 +43,7 @@ def fetch_repos(branch, test_name, dry_run, commit=None): :returns: The path to the suite repo on disk """ + suite_repo_path: str try: # When a user is scheduling a test run from their own copy of # teuthology, let's not wreak havoc on it. @@ -52,10 +54,11 @@ def fetch_repos(branch, test_name, dry_run, commit=None): suite_repo_path = fetch_qa_suite(branch, commit) except BranchNotFoundError as exc: schedule_fail(message=str(exc), name=test_name, dry_run=dry_run) + raise # schedule_fail raises, but type checker doesn't know return suite_repo_path -def schedule_fail(message, name='', dry_run=None): +def schedule_fail(message: str, name: str = '', dry_run: Optional[bool] = None) -> None: """ If an email address has been specified anywhere, send an alert there. Then raise a ScheduleFailError. @@ -77,7 +80,7 @@ def schedule_fail(message, name='', dry_run=None): raise ScheduleFailError(message, name) -def get_worker(machine_type): +def get_worker(machine_type: str) -> str: """ Map a given machine_type to a beanstalkd worker. If machine_type mentions multiple machine types - e.g. 'plana,mira', then this returns 'multi'. @@ -89,9 +92,9 @@ def get_worker(machine_type): return machine_type -def get_gitbuilder_hash(project=None, branch=None, flavor=None, - machine_type=None, distro=None, - distro_version=None): +def get_gitbuilder_hash(project: Optional[str] = None, branch: Optional[str] = None, flavor: Optional[str] = None, + machine_type: Optional[str] = None, distro: Optional[str] = None, + distro_version: Optional[str] = None) -> Optional[str]: """ Find the hash representing the head of the project's repository via querying a gitbuilder repo. @@ -119,7 +122,7 @@ def get_gitbuilder_hash(project=None, branch=None, flavor=None, return bp.sha1 -def get_distro_defaults(distro, machine_type): +def get_distro_defaults(distro: Optional[str], machine_type: str) -> tuple[str, str, OS]: """ Given a distro (e.g. 'ubuntu') and machine type, return: (arch, release, pkg_type) @@ -146,7 +149,7 @@ def get_distro_defaults(distro, machine_type): ) -def git_ls_remote(project_or_url, branch, project_owner='ceph'): +def git_ls_remote(project_or_url: str, branch: str, project_owner: str = 'ceph') -> Optional[str]: """ Find the latest sha1 for a given project's branch. @@ -163,7 +166,7 @@ def git_ls_remote(project_or_url, branch, project_owner='ceph'): return repo_utils.ls_remote(url, branch) -def git_validate_sha1(project, sha1, project_owner='ceph'): +def git_validate_sha1(project: str, sha1: str, project_owner: str = 'ceph') -> Optional[str]: ''' Use http to validate that project contains sha1 I can't find a way to do this with git, period, so @@ -189,7 +192,7 @@ def git_validate_sha1(project, sha1, project_owner='ceph'): return None -def git_branch_exists(project_or_url, branch, project_owner='ceph'): +def git_branch_exists(project_or_url: str, branch: str, project_owner: str = 'ceph') -> bool: """ Query the git repository to check the existence of a project's branch @@ -201,7 +204,7 @@ def git_branch_exists(project_or_url, branch, project_owner='ceph'): return git_ls_remote(project_or_url, branch, project_owner) is not None -def get_branch_info(project, branch, project_owner='ceph'): +def get_branch_info(project: str, branch: str, project_owner: str = 'ceph') -> Optional[dict]: """ NOTE: This is currently not being used because of GitHub's API rate limiting. We use github_branch_exists() instead. @@ -224,8 +227,8 @@ def get_branch_info(project, branch, project_owner='ceph'): @functools.lru_cache() -def package_version_for_hash(hash, flavor='default', distro='rhel', - distro_version='8.0', machine_type='smithi'): +def package_version_for_hash(hash: str, flavor: str = 'default', distro: str = 'rhel', + distro_version: str = '8.0', machine_type: str = 'smithi') -> Optional[str]: """ Does what it says on the tin. Uses gitbuilder repos. @@ -245,8 +248,8 @@ def package_version_for_hash(hash, flavor='default', distro='rhel', ), ) - if (bp.distro == CONTAINER_DISTRO and bp.flavor == CONTAINER_FLAVOR and - not bp.build_complete): + if (bp.distro == CONTAINER_DISTRO and bp.flavor == CONTAINER_FLAVOR and + not getattr(bp, 'build_complete', True)): log.info("Container build incomplete") return None @@ -256,7 +259,7 @@ def package_version_for_hash(hash, flavor='default', distro='rhel', return None -def get_arch(machine_type): +def get_arch(machine_type: str) -> Optional[str]: """ Based on a given machine_type, return its architecture by querying the lock server. @@ -270,7 +273,7 @@ def get_arch(machine_type): return result[0]['arch'] -def strip_fragment_path(original_path): +def strip_fragment_path(original_path: str) -> str: """ Given a path, remove the text before '/suites/'. Part of the fix for http://tracker.ceph.com/issues/15470 @@ -282,7 +285,7 @@ def strip_fragment_path(original_path): return original_path -def get_install_task_flavor(job_config): +def get_install_task_flavor(job_config: dict) -> str: """ Pokes through the install task's configuration (including its overrides) to figure out which flavor it will want to install. @@ -305,7 +308,7 @@ def get_install_task_flavor(job_config): return get_flavor(first_install_config) -def teuthology_schedule(args, verbose, dry_run, log_prefix='', stdin=None): +def teuthology_schedule(args: list[str], verbose: int, dry_run: bool, log_prefix: str = '', stdin: Optional[str] = None) -> None: """ Run teuthology-schedule to schedule individual jobs. @@ -340,7 +343,7 @@ def teuthology_schedule(args, verbose, dry_run, log_prefix='', stdin=None): else: p.communicate() -def find_git_parents(project: str, sha1: str, count=1): +def find_git_parents(project: str, sha1: str, count: int = 1) -> list[str]: base_url = config.githelper_base_url if not base_url: diff --git a/teuthology/task/__init__.py b/teuthology/task/__init__.py index 98330a7bf6..c8fc97740d 100644 --- a/teuthology/task/__init__.py +++ b/teuthology/task/__init__.py @@ -1,4 +1,5 @@ import logging +from typing import Optional from teuthology.misc import deep_merge from teuthology.orchestra.cluster import Cluster @@ -24,7 +25,7 @@ class MySubtask(MyTask): name = 'mytask.mysubtask' """ - def __init__(self, ctx, config=None): + def __init__(self, ctx, config: Optional[dict] = None) -> None: if not hasattr(self, 'name'): self.name = self.__class__.__name__.lower() self.log = log @@ -35,7 +36,7 @@ def __init__(self, ctx, config=None): self.apply_overrides() self.filter_hosts() - def apply_overrides(self): + def apply_overrides(self) -> None: """ Look for an 'overrides' dict in self.ctx.config; look inside that for a dict with the same name as this task. Override any settings in @@ -54,7 +55,7 @@ def apply_overrides(self): ) deep_merge(self.config, task_overrides) - def filter_hosts(self): + def filter_hosts(self) -> Optional[Cluster]: """ Look for a 'hosts' list in self.config. Each item in the list may either be a role or a hostname. Builds a new Cluster object containing @@ -63,7 +64,7 @@ def filter_hosts(self): that the task may only run against those hosts. """ if not hasattr(self.ctx, 'cluster'): - return + return None elif 'hosts' not in self.config: self.cluster = self.ctx.cluster return self.cluster @@ -88,25 +89,25 @@ def filter_hosts(self): ) return self.cluster - def setup(self): + def setup(self) -> None: """ Perform any setup that is needed by the task before it executes """ pass - def begin(self): + def begin(self) -> None: """ Execute the main functionality of the task """ pass - def end(self): + def end(self) -> None: """ Perform any work needed to stop processes started in begin() """ pass - def teardown(self): + def teardown(self) -> None: """ Perform any work needed to restore configuration to a previous state. @@ -114,7 +115,7 @@ def teardown(self): """ pass - def __enter__(self): + def __enter__(self) -> "Task": """ When using an instance of the class as a context manager, this method calls self.setup(), then calls self.begin() and returns self. @@ -123,7 +124,7 @@ def __enter__(self): self.begin() return self - def __exit__(self, type_, value, traceback): + def __exit__(self, type_, value, traceback) -> None: """ When using an instance of the class as a context manager, this method calls self.end() and self.teardown() - unless diff --git a/teuthology/task/ansible.py b/teuthology/task/ansible.py index 29d1170d1a..936e8c8ae4 100644 --- a/teuthology/task/ansible.py +++ b/teuthology/task/ansible.py @@ -1,14 +1,14 @@ import json import logging -import re -import requests import os import pathlib import pexpect -import yaml +import re +import requests import shutil - +import yaml from tempfile import mkdtemp, NamedTemporaryFile +from typing import List, Optional, Set from teuthology import repo_utils from teuthology.config import config as teuth_config @@ -21,18 +21,18 @@ class FailureAnalyzer: - def analyze(self, failure_log): + def analyze(self, failure_log: str) -> List[str]: failure_obj = yaml.safe_load(failure_log) - lines = set() + lines: Set[str] = set() if failure_obj is None: - return lines + return [] for host_obj in failure_obj.values(): if not isinstance(host_obj, dict): continue lines = lines.union(self.analyze_host_record(host_obj)) return sorted(lines) - def analyze_host_record(self, record): + def analyze_host_record(self, record: dict) -> List[str]: lines = set() for result in record.get("results", [record]): cmd = result.get("cmd", "") @@ -54,7 +54,7 @@ def analyze_host_record(self, record): lines.add(line) return list(lines) - def analyze_line(self, line): + def analyze_line(self, line: str) -> str: if line.startswith("W: ") or line.endswith("?"): return "" drop_phrases = [ @@ -167,7 +167,7 @@ class Ansible(Task): # assign hosts to for dynamic inventory creation inventory_group = None - def __init__(self, ctx, config): + def __init__(self, ctx, config: dict) -> None: super(Ansible, self).__init__(ctx, config) self.generated_inventory = False self.generated_playbook = False @@ -176,7 +176,7 @@ def __init__(self, ctx, config): self.log.addHandler(logging.FileHandler( os.path.join(ctx.archive, "ansible.log"))) - def setup(self): + def setup(self) -> None: super(Ansible, self).setup() self.find_repo() self.get_playbook() @@ -193,7 +193,7 @@ def failure_log(self): ) return self._failure_log - def find_repo(self): + def find_repo(self) -> None: """ Locate the repo we're using; cloning it from a remote repo if necessary """ @@ -207,7 +207,7 @@ def find_repo(self): repo_path = os.path.abspath(os.path.expanduser(repo)) self.repo_path = repo_path - def get_playbook(self): + def get_playbook(self) -> None: """ If necessary, fetch and read the playbook file """ @@ -241,7 +241,7 @@ def get_playbook(self): "playbook value must either be a list, URL or a filename") log.info("Playbook: %s", self.playbook) - def get_inventory(self): + def get_inventory(self) -> Optional[str]: """ Determine whether or not we're using an existing inventory file """ @@ -253,7 +253,7 @@ def get_inventory(self): self.inventory = etc_ansible_hosts return self.inventory - def generate_inventory(self): + def generate_inventory(self) -> None: """ Generate a hosts (inventory) file to use. This should not be called if we're using an existing file. @@ -269,7 +269,7 @@ def generate_inventory(self): self.inventory = self._write_inventory_files(hosts_str) self.generated_inventory = True - def _write_inventory_files(self, inventory, inv_suffix=''): + def _write_inventory_files(self, inventory: str, inv_suffix: str = '') -> str: """ Actually write the inventory files. Writes out group_vars files as necessary based on configuration. @@ -284,7 +284,7 @@ def _write_inventory_files(self, inventory, inv_suffix=''): ) inv_fn = os.path.join(inventory_dir, 'inventory') if inv_suffix: - inv_fn = '.'.join(inv_fn, inv_suffix) + inv_fn = '.'.join([inv_fn, inv_suffix]) # Write out the inventory file inv_file = open(inv_fn, 'w') inv_file.write(inventory) @@ -303,7 +303,7 @@ def _write_inventory_files(self, inventory, inv_suffix=''): return inventory_dir - def generate_playbook(self): + def generate_playbook(self) -> None: """ Generate a playbook file to use. This should not be called if we're using an existing file. @@ -318,14 +318,14 @@ def generate_playbook(self): self.playbook_file = playbook_file self.generated_playbook = True - def begin(self): + def begin(self) -> None: super(Ansible, self).begin() if len(self.cluster.remotes) > 0: self.execute_playbook() else: log.info("There are no remotes; skipping playbook execution") - def execute_playbook(self, _logfile=None): + def execute_playbook(self, _logfile: Optional = None) -> None: """ Execute ansible-playbook @@ -361,9 +361,9 @@ def execute_playbook(self, _logfile=None): for remote in remotes: remote.reconnect() - def _handle_failure(self, command, status): + def _handle_failure(self, command: str, status: int) -> None: self._set_status('dead') - failures = None + failures: List[str] = [] with open(self.failure_log.name, 'r') as fail_log_file: fail_log = fail_log_file.read() try: @@ -377,20 +377,20 @@ def _handle_failure(self, command, status): log.exception(f"Failed to analyze ansible failure log: {self.failure_log.name}") # If we hit an exception, or if analyze() returned nothing, use the log as-is if not failures: - failures = fail_log.replace('\n', '') + failures = [fail_log.replace('\n', '')] - if failures: + if any(failures): self._archive_failures() raise AnsibleFailedError(failures) raise CommandFailedError(command, status) - def _set_status(self, status): + def _set_status(self, status: str) -> None: """ Not implemented in the base class """ pass - def _archive_failures(self): + def _archive_failures(self) -> None: if self.ctx.archive: archive_path = "{0}/ansible_failures.yaml".format(self.ctx.archive) log.info("Archiving ansible failure log at: {0}".format( @@ -402,7 +402,7 @@ def _archive_failures(self): ) os.chmod(archive_path, 0o664) - def _build_args(self): + def _build_args(self) -> List[str]: """ Assemble the list of args to be executed """ @@ -426,15 +426,15 @@ def _build_args(self): args.extend(['--skip-tags', skip_tags]) return args - def teardown(self): + def teardown(self) -> None: self._cleanup() - if self.generated_inventory: + if self.generated_inventory and self.inventory: shutil.rmtree(self.inventory) if self.generated_playbook: os.remove(self.playbook_file.name) super(Ansible, self).teardown() - def _cleanup(self): + def _cleanup(self) -> None: """ If the ``cleanup`` key exists in config the same playbook will be run again during the teardown step with the var ``cleanup`` given with @@ -444,8 +444,8 @@ def _cleanup(self): if self.config.get("cleanup"): log.info("Running ansible cleanup...") extra = dict(cleanup=True) - if self.config.get('vars'): - self.config.get('vars').update(extra) + if vars_dict := self.config.get('vars'): + vars_dict.update(extra) else: self.config['vars'] = extra self.execute_playbook() @@ -471,7 +471,7 @@ class CephLab(Ansible): name = 'ansible.cephlab' inventory_group = 'testnodes' - def __init__(self, ctx, config): + def __init__(self, ctx, config: Optional[dict]) -> None: config = config or dict() if 'playbook' not in config: config['playbook'] = 'cephlab.yml' @@ -479,7 +479,7 @@ def __init__(self, ctx, config): config['repo'] = teuth_config.get_ceph_cm_ansible_git_url() super(CephLab, self).__init__(ctx, config) - def begin(self): + def begin(self) -> None: # Write foo to ~/.vault_pass.txt if it's missing. # In almost all cases we don't need the actual vault password. # Touching an empty file broke as of Ansible 2.4 @@ -489,7 +489,7 @@ def begin(self): f.write('foo') super(CephLab, self).begin() - def _set_status(self, status): + def _set_status(self, status: str) -> None: set_status(self.ctx.summary, status) diff --git a/teuthology/task/args.py b/teuthology/task/args.py index 17e9e9dc00..cda43daad1 100644 --- a/teuthology/task/args.py +++ b/teuthology/task/args.py @@ -1,7 +1,9 @@ """ These routines only appear to be used by the peering_speed tests. """ -def gen_args(name, args): +from typing import Callable, List, Tuple + +def gen_args(name: str, args: List[Tuple[str, str, object, Callable]]) -> Tuple[str, Callable]: """ Called from argify to generate arguments. """ @@ -36,25 +38,25 @@ class Object(object): return obj return usage, ret -def argify(name, args): +def argify(name: str, args: List[Tuple[str, str, object, Callable]]) -> Callable[[Callable], Callable]: """ Object used as a decorator for the peering speed tests. See peering_spee_test.py """ (usage, config_func) = gen_args(name, args) - def ret1(f): + def ret1(f: Callable) -> Callable: """ Wrapper to handle doc and usage information """ def ret2(**kwargs): """ - Call f (the parameter passed to ret1) + Call f (the parameter passed to ret1) """ config = kwargs.get('config', {}) if config is None: config = {} kwargs['config'] = config_func(config) return f(**kwargs) - ret2.__doc__ = f.__doc__ + usage + ret2.__doc__ = (f.__doc__ or '') + usage return ret2 return ret1 diff --git a/teuthology/task/background_exec.py b/teuthology/task/background_exec.py index 897b525312..dc3fee04df 100644 --- a/teuthology/task/background_exec.py +++ b/teuthology/task/background_exec.py @@ -4,6 +4,7 @@ import contextlib import logging +from typing import Dict, Generator, List, Union from teuthology import misc from teuthology.orchestra import run @@ -12,7 +13,7 @@ @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config: Dict[str, Union[str, List[str]]]) -> Generator[None, None, None]: """ Run a background task. diff --git a/teuthology/task/buildpackages.py b/teuthology/task/buildpackages.py index ae56af01fa..e6a2ece4b7 100644 --- a/teuthology/task/buildpackages.py +++ b/teuthology/task/buildpackages.py @@ -13,7 +13,8 @@ import copy import logging import os -import types +from typing import Optional, Union + from teuthology import packaging from teuthology import misc from teuthology.config import config as teuth_config @@ -27,13 +28,13 @@ def __init__(self): pass -def get_pkg_type(os_type): +def get_pkg_type(os_type: str) -> str: if os_type in ('centos', 'fedora', 'opensuse', 'rhel', 'sle'): return 'rpm' else: return 'deb' -def apply_overrides(ctx, config): +def apply_overrides(ctx, config: Optional[dict]) -> dict: if config is None: config = {} else: @@ -50,7 +51,7 @@ def apply_overrides(ctx, config): misc.deep_merge(config, install_overrides.get(project, {})) return config -def get_config_install(ctx, config): +def get_config_install(ctx, config: Optional[dict]) -> list: config = apply_overrides(ctx, config) log.debug('install config %s' % config) return [(config.get('flavor', 'default'), @@ -58,7 +59,7 @@ def get_config_install(ctx, config): config.get('branch', ''), config.get('sha1'))] -def get_config_install_upgrade(ctx, config): +def get_config_install_upgrade(ctx, config: dict) -> list: log.debug('install.upgrade config before override %s' % config) configs = [] for (role, role_config) in config.items(): @@ -80,12 +81,12 @@ def get_config_install_upgrade(ctx, config): 'install.upgrade': get_config_install_upgrade, } -def lookup_configs(ctx, node): +def lookup_configs(ctx, node: Union[list, dict]) -> list: configs = [] - if type(node) is types.ListType: + if isinstance(node, list): for leaf in node: configs.extend(lookup_configs(ctx, leaf)) - elif type(node) is types.DictType: + elif isinstance(node, dict): for (key, value) in node.items(): if key in ('install', 'install.upgrade'): configs.extend(GET_CONFIG_FUNCTIONS[key](ctx, value)) @@ -95,12 +96,12 @@ def lookup_configs(ctx, node): configs.extend(lookup_configs(ctx, value)) return configs -def get_sha1(ref): +def get_sha1(ref: str) -> str: url = teuth_config.get_ceph_git_url() ls_remote = misc.sh("git ls-remote " + url + " " + ref) return ls_remote.split()[0] -def task(ctx, config): +def task(ctx, config: Optional[dict]) -> None: """ Build Ceph packages. This task will automagically be run before the task that need to install packages (this is taken @@ -241,5 +242,6 @@ def task(ctx, config): "instance and start again from scratch.".format(pkg_type)) log.info("buildpackages make command: " + cmd) misc.sh(cmd) - teuth_config.gitbuilder_host = openstack.get_ip(pkg_repo, '') + # Set gitbuilder_host after the loop completes + teuth_config.gitbuilder_host = openstack.get_ip(pkg_repo, '') log.info('Finished buildpackages') diff --git a/teuthology/task/ceph_ansible.py b/teuthology/task/ceph_ansible.py index 0e7d483c30..962ee666ae 100644 --- a/teuthology/task/ceph_ansible.py +++ b/teuthology/task/ceph_ansible.py @@ -1,11 +1,11 @@ import json +import logging import os import re -import logging import yaml +from tempfile import NamedTemporaryFile from teuthology.task import Task -from tempfile import NamedTemporaryFile from teuthology.config import config as teuth_config from teuthology.misc import get_scratch_devices, get_file from teuthology import contextutil @@ -54,7 +54,7 @@ class CephAnsible(Task): nfss='nfs', ) - def __init__(self, ctx, config): + def __init__(self, ctx, config: dict) -> None: super(CephAnsible, self).__init__(ctx, config) config = self.config or dict() self.playbook = None @@ -76,7 +76,7 @@ def __init__(self, ctx, config): vars['ceph_dev_branch'] = ctx.config.get('branch', 'main') self.cluster_name = vars.get('cluster', 'ceph') - def setup(self): + def setup(self) -> None: super(CephAnsible, self).setup() # generate hosts file based on test config self.generate_hosts_file() @@ -97,7 +97,7 @@ def setup(self): self.extra_vars_file = self._write_hosts_file(prefix='teuth_ansible_gvar', content=gvar) - def execute_playbook(self): + def execute_playbook(self) -> None: """ Execute ansible-playbook @@ -129,7 +129,7 @@ def execute_playbook(self): else: self.run_playbook() - def generate_hosts_file(self): + def generate_hosts_file(self) -> None: hosts_dict = dict() for group in sorted(self.groups_to_roles.keys()): role_prefix = self.groups_to_roles[group] @@ -165,11 +165,11 @@ def generate_hosts_file(self): content=hosts_content.strip()) self.generated_inventory = True - def begin(self): + def begin(self) -> None: super(CephAnsible, self).begin() self.execute_playbook() - def _write_hosts_file(self, prefix, content): + def _write_hosts_file(self, prefix: str, content: str) -> str: """ Actually write the hosts file """ @@ -179,10 +179,10 @@ def _write_hosts_file(self, prefix, content): hosts_file.flush() return hosts_file.name - def teardown(self): + def teardown(self) -> None: log.info("Cleaning up temporary files") os.remove(self.inventory) - if self.playbook is not None: + if self.playbook is not None and self.playbook_file is not None: os.remove(self.playbook_file) os.remove(self.extra_vars_file) # collect logs @@ -249,7 +249,7 @@ def teardown(self): 'python-dev' ]) - def collect_logs(self): + def collect_logs(self) -> None: ctx = self.ctx if ctx.archive is not None and \ not (ctx.config.get('archive-on-error') and ctx.summary['success']): @@ -270,7 +270,7 @@ def wanted(role): misc.pull_directory(remote, '/var/log/ceph', os.path.join(sub, 'log')) - def wait_for_ceph_health(self): + def wait_for_ceph_health(self) -> None: with contextutil.safe_while(sleep=15, tries=6, action='check health') as proceed: (remote,) = self.ctx.cluster.only('mon.a').remotes @@ -289,7 +289,7 @@ def wait_for_ceph_health(self): if state in ('HEALTH_OK', 'HEALTH_WARN'): break - def get_host_vars(self, remote): + def get_host_vars(self, remote) -> dict: extra_vars = self.config.get('vars', dict()) host_vars = dict() if not extra_vars.get('osd_auto_discovery', False): @@ -323,7 +323,7 @@ def get_host_vars(self, remote): host_vars['public_network'] = remote.cidr return host_vars - def run_rh_playbook(self): + def run_rh_playbook(self) -> None: ceph_installer = self.ceph_installer args = self.args ceph_installer.run(args=[ @@ -350,7 +350,7 @@ def run_rh_playbook(self): raise CephAnsibleError("Failed during ceph-ansible execution") self._create_rbd_pool() - def run_playbook(self): + def run_playbook(self) -> None: # setup ansible on first mon node ceph_installer = self.ceph_installer args = self.args @@ -382,8 +382,8 @@ def run_playbook(self): if self.config.get('branch'): branch = self.config.get('branch') ansible_ver = 'ansible==2.5' - if self.config.get('ansible-version'): - ansible_ver = 'ansible==' + self.config.get('ansible-version') + if ansible_version := self.config.get('ansible-version'): + ansible_ver = 'ansible==' + str(ansible_version) ceph_installer.run( args=[ 'rm', @@ -444,7 +444,7 @@ def run_playbook(self): self._create_rbd_pool() self.fix_keyring_permission() - def _copy_and_print_config(self): + def _copy_and_print_config(self) -> None: ceph_installer = self.ceph_installer # copy the inventory file to installer node ceph_installer.put_file(self.inventory, 'ceph-ansible/inven.yml') @@ -467,7 +467,7 @@ def _copy_and_print_config(self): ceph_installer.run(args=['cat', 'ceph-ansible/site.yml']) ceph_installer.run(args=['cat', 'ceph-ansible/group_vars/all']) - def _create_rbd_pool(self): + def _create_rbd_pool(self) -> None: mon_node = self.ceph_first_mon log.info('Creating RBD pool') mon_node.run( @@ -483,7 +483,7 @@ def _create_rbd_pool(self): ], check_status=False) - def fix_keyring_permission(self): + def fix_keyring_permission(self) -> None: clients_only = lambda role: role.startswith('client') for client in self.cluster.only(clients_only).remotes.keys(): client.run(args=[ diff --git a/teuthology/task/cephmetrics.py b/teuthology/task/cephmetrics.py index 813d266add..0328a2c3cb 100644 --- a/teuthology/task/cephmetrics.py +++ b/teuthology/task/cephmetrics.py @@ -2,6 +2,7 @@ import os import pexpect import time +from typing import Optional from teuthology.config import config as teuth_config from teuthology.exceptions import CommandFailedError @@ -13,7 +14,7 @@ class CephMetrics(Ansible): - def __init__(self, ctx, config): + def __init__(self, ctx, config: dict) -> None: super(CephMetrics, self).__init__(ctx, config) if 'repo' not in self.config: self.config['repo'] = os.path.join( @@ -21,10 +22,10 @@ def __init__(self, ctx, config): if 'playbook' not in self.config: self.config['playbook'] = './ansible/playbook.yml' - def get_inventory(self): - return False + def get_inventory(self) -> Optional[str]: + return None - def generate_inventory(self): + def generate_inventory(self) -> None: groups_to_roles = { 'mons': 'mon', 'mgrs': 'mgr', @@ -58,10 +59,10 @@ def generate_inventory(self): hosts_lines.append('[%s]' % group) for host, vars_ in hosts_dict[group]['hosts'].items(): host_line = ' '.join( - [host] + map( + [host] + list(map( lambda tuple_: '='.join(tuple_), vars_.items(), - ) + )) ) hosts_lines.append(host_line) hosts_lines.append('') @@ -69,7 +70,7 @@ def generate_inventory(self): self.inventory = self._write_inventory_files(hosts_str) self.generated_inventory = True - def begin(self): + def begin(self) -> None: super(CephMetrics, self).begin() wait_time = 5 * 60 self.log.info( @@ -79,7 +80,7 @@ def begin(self): time.sleep(wait_time) self.run_tests() - def run_tests(self): + def run_tests(self) -> None: self.log.info("Running tests...") command = "tox -e integration %s" % self.inventory out, status = pexpect.run( diff --git a/teuthology/task/clock.py b/teuthology/task/clock.py index 982eb8e1bd..0fbb3654ea 100644 --- a/teuthology/task/clock.py +++ b/teuthology/task/clock.py @@ -1,14 +1,16 @@ """ Clock synchronizer """ -import logging import contextlib +import logging +from typing import Generator from teuthology.orchestra import run +from teuthology.orchestra.cluster import Cluster log = logging.getLogger(__name__) -def filter_out_containers(cluster): +def filter_out_containers(cluster: Cluster) -> Cluster: """ Returns a cluster that excludes remotes which should skip this task. Currently, only skips containerized remotes. @@ -16,7 +18,7 @@ def filter_out_containers(cluster): return cluster.filter(lambda r: not r.is_container) @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config) -> Generator[None, None, None]: """ Sync or skew clock @@ -82,7 +84,7 @@ def task(ctx, config): @contextlib.contextmanager -def check(ctx, config): +def check(ctx, config) -> Generator[None, None, None]: """ Run ntpq at the start and the end of the task. diff --git a/teuthology/task/common_fs_utils.py b/teuthology/task/common_fs_utils.py index 584897968a..e0ae83d5ff 100644 --- a/teuthology/task/common_fs_utils.py +++ b/teuthology/task/common_fs_utils.py @@ -3,14 +3,16 @@ code was part of rbd.py. It was broken out so that it could be used by other modules (tgt.py and iscsi.py for instance). """ -import logging import contextlib +import logging +from typing import Callable, Dict, Generator, List, Optional, Union + from teuthology import misc as teuthology log = logging.getLogger(__name__) -def default_image_name(role): +def default_image_name(role: str) -> str: """ Image name used by rbd and iscsi """ @@ -18,7 +20,7 @@ def default_image_name(role): @contextlib.contextmanager -def generic_mkfs(ctx, config, devname_rtn): +def generic_mkfs(ctx, config: Union[List[str], dict], devname_rtn: Callable) -> Generator[None, None, None]: """ Create a filesystem (either rbd or tgt, depending on devname_rtn) @@ -55,7 +57,7 @@ def generic_mkfs(ctx, config, devname_rtn): @contextlib.contextmanager -def generic_mount(ctx, config, devname_rtn): +def generic_mount(ctx, config: Union[List[str], Dict[str, Optional[str]]], devname_rtn: Callable) -> Generator[None, None, None]: """ Generic Mount an rbd or tgt image. diff --git a/teuthology/task/console_log.py b/teuthology/task/console_log.py index 01b89351f5..26e6f37983 100644 --- a/teuthology/task/console_log.py +++ b/teuthology/task/console_log.py @@ -1,5 +1,6 @@ import logging import os +from typing import Optional from teuthology.orchestra.cluster import Cluster from teuthology.exit import exiter @@ -13,7 +14,7 @@ class ConsoleLog(Task): name = 'console_log' logfile_name = '{shortname}.log' - def __init__(self, ctx=None, config=None): + def __init__(self, ctx: Optional = None, config: Optional[dict] = None) -> None: super(ConsoleLog, self).__init__(ctx, config) if self.config.get('enabled') is False: self.enabled = False @@ -24,7 +25,7 @@ def __init__(self, ctx=None, config=None): if 'remotes' in self.config: self.remotes = self.config['remotes'] - def filter_hosts(self): + def filter_hosts(self) -> Optional[Cluster]: super(ConsoleLog, self).filter_hosts() if not hasattr(self.ctx, 'cluster'): return @@ -44,7 +45,7 @@ def filter_hosts(self): self.remotes = self.cluster.remotes.keys() return self.cluster - def setup(self): + def setup(self) -> None: if not self.enabled: return super(ConsoleLog, self).setup() @@ -52,7 +53,7 @@ def setup(self): self.signal_handlers = list() self.setup_archive() - def setup_archive(self): + def setup_archive(self) -> None: self.archive_dir = os.path.join( self.ctx.archive, 'console_logs', @@ -60,13 +61,13 @@ def setup_archive(self): if not os.path.isdir(self.archive_dir): os.makedirs(self.archive_dir) - def begin(self): + def begin(self) -> None: if not self.enabled: return super(ConsoleLog, self).begin() self.start_logging() - def start_logging(self): + def start_logging(self) -> None: for remote in self.remotes: log_path = os.path.join( self.archive_dir, @@ -83,13 +84,13 @@ def kill_console_loggers(signal_, frame): proc.terminate() exiter.add_handler(15, kill_console_loggers) - def end(self): + def end(self) -> None: if not self.enabled: return super(ConsoleLog, self).end() self.stop_logging() - def stop_logging(self, force=False): + def stop_logging(self, force: bool = False) -> None: for proc in self.processes.values(): if proc.poll() is not None: continue @@ -102,7 +103,7 @@ def stop_logging(self, force=False): for handler in self.signal_handlers: handler.remove() - def teardown(self): + def teardown(self) -> None: if not self.enabled: return self.stop_logging(force=True) diff --git a/teuthology/task/dump_ctx.py b/teuthology/task/dump_ctx.py index f2da22e121..2d0373e880 100644 --- a/teuthology/task/dump_ctx.py +++ b/teuthology/task/dump_ctx.py @@ -4,10 +4,10 @@ log = logging.getLogger(__name__) pp = pprint.PrettyPrinter(indent=4) -def _pprint_me(thing, prefix): +def _pprint_me(thing, prefix: str) -> str: return prefix + "\n" + pp.pformat(thing) -def task(ctx, config): +def task(ctx, config) -> None: """ Dump task context and config in teuthology log/output diff --git a/teuthology/task/exec.py b/teuthology/task/exec.py index b3548c332d..2da482aaeb 100644 --- a/teuthology/task/exec.py +++ b/teuthology/task/exec.py @@ -2,12 +2,13 @@ Exececute custom commands """ import logging +from typing import Dict, List from teuthology import misc as teuthology log = logging.getLogger(__name__) -def task(ctx, config): +def task(ctx, config: Dict[str, List[str]]) -> None: """ Execute commands on a given role diff --git a/teuthology/task/full_sequential.py b/teuthology/task/full_sequential.py index a9990f2aa3..c6b65c5259 100644 --- a/teuthology/task/full_sequential.py +++ b/teuthology/task/full_sequential.py @@ -1,15 +1,15 @@ """ Task sequencer - full """ -import sys import logging +import sys from teuthology import run_tasks log = logging.getLogger(__name__) -def task(ctx, config): +def task(ctx, config: list) -> None: """ Run a set of tasks to completion in order. __exit__ is called on a task before __enter__ on the next diff --git a/teuthology/task/full_sequential_finally.py b/teuthology/task/full_sequential_finally.py index 76e3bbbdeb..f0fc711e75 100644 --- a/teuthology/task/full_sequential_finally.py +++ b/teuthology/task/full_sequential_finally.py @@ -1,9 +1,10 @@ """ Task sequencer finally """ -import sys -import logging import contextlib +import logging +import sys +from typing import Generator from teuthology import run_tasks @@ -11,7 +12,7 @@ @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config: list) -> Generator[None, None, None]: """ Sequentialize a group of tasks into one executable block, run on cleanup diff --git a/teuthology/task/hadoop.py b/teuthology/task/hadoop.py index 7754a76959..fe1bd705b0 100644 --- a/teuthology/task/hadoop.py +++ b/teuthology/task/hadoop.py @@ -1,6 +1,8 @@ from io import StringIO import contextlib import logging +from typing import Callable, Dict, Generator, Tuple + from teuthology import misc as teuthology from teuthology import contextutil from teuthology.orchestra import run @@ -10,7 +12,7 @@ HADOOP_2x_URL = "https://archive.apache.org/dist/hadoop/core/hadoop-2.5.2/hadoop-2.5.2.tar.gz" -def dict_to_hadoop_conf(items): +def dict_to_hadoop_conf(items: Dict[str, str]) -> str: out = "\n" for key, value in items.items(): out += " \n" @@ -20,10 +22,10 @@ def dict_to_hadoop_conf(items): out += "\n" return out -def is_hadoop_type(type_): +def is_hadoop_type(type_: str) -> Callable[[str], bool]: return lambda role: role.startswith('hadoop.' + type_) -def get_slaves_data(ctx): +def get_slaves_data(ctx) -> Tuple[str, str]: tempdir = teuthology.get_testdir(ctx) path = "{tdir}/hadoop/etc/hadoop/slaves".format(tdir=tempdir) nodes = ctx.cluster.only(is_hadoop_type('slave')) @@ -31,7 +33,7 @@ def get_slaves_data(ctx): data = '\n'.join(hosts) return path, data -def get_masters_data(ctx): +def get_masters_data(ctx) -> Tuple[str, str]: tempdir = teuthology.get_testdir(ctx) path = "{tdir}/hadoop/etc/hadoop/masters".format(tdir=tempdir) nodes = ctx.cluster.only(is_hadoop_type('master')) @@ -39,7 +41,7 @@ def get_masters_data(ctx): data = '\n'.join(hosts) return path, data -def get_core_site_data(ctx, config): +def get_core_site_data(ctx, config: dict) -> Tuple[str, str]: tempdir = teuthology.get_testdir(ctx) path = "{tdir}/hadoop/etc/hadoop/core-site.xml".format(tdir=tempdir) nodes = ctx.cluster.only(is_hadoop_type('master')) @@ -66,7 +68,7 @@ def get_core_site_data(ctx, config): data_tmpl = dict_to_hadoop_conf(conf) return path, data_tmpl.format(tdir=tempdir, namenode=host) -def get_mapred_site_data(ctx): +def get_mapred_site_data(ctx) -> Tuple[str, str]: data_tmpl = """ @@ -87,7 +89,7 @@ def get_mapred_site_data(ctx): host = hosts[0] return path, data_tmpl.format(namenode=host) -def get_yarn_site_data(ctx): +def get_yarn_site_data(ctx) -> Tuple[str, str]: conf = {} conf.update({ 'yarn.resourcemanager.resourcetracker.address': '{namenode}:8025', @@ -108,7 +110,7 @@ def get_yarn_site_data(ctx): host = hosts[0] return path, data_tmpl.format(namenode=host) -def get_hdfs_site_data(ctx): +def get_hdfs_site_data(ctx) -> Tuple[str, str]: data = """ @@ -121,7 +123,7 @@ def get_hdfs_site_data(ctx): path = "{tdir}/hadoop/etc/hadoop/hdfs-site.xml".format(tdir=tempdir) return path, data -def configure(ctx, config, hadoops): +def configure(ctx, config: dict, hadoops) -> None: tempdir = teuthology.get_testdir(ctx) log.info("Writing Hadoop slaves file...") @@ -182,7 +184,7 @@ def configure(ctx, config, hadoops): ) @contextlib.contextmanager -def install_hadoop(ctx, config): +def install_hadoop(ctx, config: dict) -> Generator[None, None, None]: testdir = teuthology.get_testdir(ctx) log.info("Downloading Hadoop...") @@ -335,7 +337,7 @@ def install_hadoop(ctx, config): ) @contextlib.contextmanager -def start_hadoop(ctx, config): +def start_hadoop(ctx, config: dict) -> Generator[None, None, None]: testdir = teuthology.get_testdir(ctx) hadoop_dir = "{tdir}/hadoop/".format(tdir=testdir) masters = ctx.cluster.only(is_hadoop_type('master')) @@ -407,7 +409,7 @@ def start_hadoop(ctx, config): ) @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config: dict) -> Generator[None, None, None]: if config is None: config = {} assert isinstance(config, dict), "task hadoop config must be dictionary" diff --git a/teuthology/task/install/__init__.py b/teuthology/task/install/__init__.py index d6d82d5b81..a9e76710d9 100644 --- a/teuthology/task/install/__init__.py +++ b/teuthology/task/install/__init__.py @@ -2,28 +2,27 @@ import copy import logging import os +from typing import Dict, Generator, List, Optional + import yaml -from teuthology import misc as teuthology -from teuthology import contextutil, packaging +from teuthology import contextutil, misc as teuthology, packaging from teuthology.parallel import parallel from teuthology.task import ansible - +from teuthology.task.install import deb, redhat, rpm from teuthology.task.install.util import ( _get_builder_project, get_flavor, ship_utilities, ) - -from teuthology.task.install import rpm, deb, redhat from teuthology.util.version import LooseVersion log = logging.getLogger(__name__) -def get_upgrade_version(ctx, config, remote): +def get_upgrade_version(ctx, config: dict, remote) -> str: builder = _get_builder_project(ctx, remote, config) version = builder.version return version -def verify_package_version(ctx, config, remote): +def verify_package_version(ctx, config: dict, remote) -> None: """ Ensures that the version of package installed is what was asked for in the config. @@ -33,7 +32,7 @@ def verify_package_version(ctx, config, remote): """ if 'repos' in config and config.get('repos'): log.info("Skipping version verification because we have custom repos...") - return True + return builder = _get_builder_project(ctx, remote, config) version = builder.version pkg_to_check = builder.project @@ -54,7 +53,7 @@ def verify_package_version(ctx, config, remote): ) -def install_packages(ctx, pkgs, config): +def install_packages(ctx, pkgs: Dict[str, List[str]], config: dict) -> None: """ Installs packages on each remote in ctx. @@ -78,7 +77,7 @@ def install_packages(ctx, pkgs, config): verify_package_version(ctx, config, remote) -def remove_packages(ctx, config, pkgs): +def remove_packages(ctx, config: dict, pkgs: Dict[str, List[str]]) -> None: """ Removes packages from each remote in ctx. @@ -99,7 +98,7 @@ def remove_packages(ctx, config, pkgs): system_type], ctx, config, remote, pkgs[system_type]) -def remove_sources(ctx, config): +def remove_sources(ctx, config: dict) -> None: """ Removes repo source files from each remote in ctx. @@ -121,7 +120,7 @@ def remove_sources(ctx, config): p.spawn(remove_fn, ctx, config, remote) -def get_package_list(ctx, config): +def get_package_list(ctx, config: dict) -> Dict[str, List[str]]: debug = config.get('debuginfo', False) project = config.get('project', 'ceph') yaml_path = None @@ -171,7 +170,7 @@ def exclude(pkgs, exclude_list): @contextlib.contextmanager -def install(ctx, config): +def install(ctx, config: dict) -> Generator[None, None, None]: """ The install task. Installs packages for a given project on all hosts in ctx. May work for projects besides ceph, but may not. Patches welcomed! @@ -203,7 +202,7 @@ def install(ctx, config): remove_sources(ctx, config) -def upgrade_old_style(ctx, node, remote, pkgs, system_type): +def upgrade_old_style(ctx, node: dict, remote, pkgs: List[str], system_type: str) -> None: """ Handle the upgrade using methods in use prior to deploy. """ @@ -213,7 +212,7 @@ def upgrade_old_style(ctx, node, remote, pkgs, system_type): rpm._upgrade_packages(ctx, node, remote, pkgs) -def upgrade_remote_to_config(ctx, config): +def upgrade_remote_to_config(ctx, config: dict) -> dict: assert config is None or isinstance(config, dict), \ "install.upgrade only supports a dictionary for configuration" @@ -264,12 +263,12 @@ def upgrade_remote_to_config(ctx, config): return result -def _upgrade_is_downgrade(installed_version, upgrade_version): +def _upgrade_is_downgrade(installed_version: str, upgrade_version: str) -> bool: assert installed_version, "installed_version is empty" assert upgrade_version, "upgrade_version is empty" return LooseVersion(installed_version) > LooseVersion(upgrade_version) -def upgrade_common(ctx, config, deploy_style): +def upgrade_common(ctx, config: dict, deploy_style) -> int: """ Common code for upgrading """ @@ -356,14 +355,14 @@ def upgrade_common(ctx, config, deploy_style): @contextlib.contextmanager -def upgrade(ctx, config): +def upgrade(ctx, config: dict) -> Generator[None, None, None]: upgrade_common(ctx, config, upgrade_old_style) yield upgrade.__doc__ = docstring_for_upgrade.format(cmd_parameter='upgrade') -def _override_extra_system_packages(config, project, install_overrides): +def _override_extra_system_packages(config: dict, project: str, install_overrides: dict) -> None: teuthology.deep_merge(config, install_overrides.get(project, {})) extra_overrides = install_overrides.get('extra_system_packages') if extra_overrides: @@ -381,7 +380,7 @@ def _override_extra_system_packages(config, project, install_overrides): config['extra_system_packages'] = teuthology.deep_merge(e, extra_overrides) @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config: Optional[dict]) -> Generator[None, None, None]: """ Install packages for a given project. diff --git a/teuthology/task/install/deb.py b/teuthology/task/install/deb.py index 1270af83a8..a3304217c9 100644 --- a/teuthology/task/install/deb.py +++ b/teuthology/task/install/deb.py @@ -1,17 +1,15 @@ import logging import os - from io import StringIO +from typing import List -from teuthology.orchestra import run from teuthology.contextutil import safe_while - +from teuthology.orchestra import run from teuthology.task.install.util import _get_builder_project, _get_local_dir - log = logging.getLogger(__name__) -def _retry_if_eagain_in_output(remote, args): +def _retry_if_eagain_in_output(remote, args: List[str]): # wait at most 5 minutes with safe_while(sleep=10, tries=30) as proceed: while proceed(): @@ -44,10 +42,10 @@ def _retry_if_eagain_in_output(remote, args): else: raise -def install_dep_packages(remote, args): +def install_dep_packages(remote, args: List[str]) -> None: _retry_if_eagain_in_output(remote, args) -def _update_package_list_and_install(ctx, remote, debs, config): +def _update_package_list_and_install(ctx, remote, debs: List[str], config: dict) -> None: """ Runs ``apt-get update`` first, then runs ``apt-get install``, installing the requested packages on the remote system. @@ -121,7 +119,7 @@ def _update_package_list_and_install(ctx, remote, debs, config): remote.run(args=['sudo', 'dpkg', '-i', fname],) -def _remove(ctx, config, remote, debs): +def _remove(ctx, config: dict, remote, debs: List[str]) -> None: """ Removes Debian packages from remote, rudely @@ -178,7 +176,7 @@ def _remove(ctx, config, remote, debs): ) -def _remove_sources_list(ctx, config, remote): +def _remove_sources_list(ctx, config: dict, remote) -> None: builder = _get_builder_project(ctx, remote, config) builder.remove_repo() remote.run( @@ -189,7 +187,7 @@ def _remove_sources_list(ctx, config, remote): ) -def _upgrade_packages(ctx, config, remote, debs): +def _upgrade_packages(ctx, config: dict, remote, debs: List[str]) -> None: """ Upgrade project's packages on remote Debian host Before doing so, installs the project's GPG key, writes a sources.list diff --git a/teuthology/task/install/redhat.py b/teuthology/task/install/redhat.py index 721772ec5f..9db839b508 100644 --- a/teuthology/task/install/redhat.py +++ b/teuthology/task/install/redhat.py @@ -1,18 +1,20 @@ import contextlib import logging -import yaml import os +from typing import Generator, Optional + +import yaml from teuthology import packaging +from teuthology.config import config as teuth_config from teuthology.orchestra import run from teuthology.parallel import parallel -from teuthology.config import config as teuth_config log = logging.getLogger(__name__) @contextlib.contextmanager -def install(ctx, config): +def install(ctx, config: dict) -> Generator[None, None, None]: """ Installs rh ceph on all hosts in ctx. @@ -80,7 +82,7 @@ def install(ctx, config): p.spawn(uninstall_pkgs, ctx, remote, downstream_config) -def install_pkgs(ctx, remote, version, downstream_config): +def install_pkgs(ctx, remote, version: str, downstream_config: dict) -> None: """ Installs RH build using ceph. @@ -126,7 +128,7 @@ def install_pkgs(ctx, remote, version, downstream_config): raise RuntimeError("Version check failed on node %s", remote.shortname) -def set_deb_repo(remote, deb_repo, deb_gpg_key=None): +def set_deb_repo(remote, deb_repo: str, deb_gpg_key: Optional[str] = None) -> None: """ Sets up debian repo and gpg key for package verification :param remote - remote node object @@ -162,8 +164,8 @@ def set_deb_repo(remote, deb_repo, deb_gpg_key=None): def install_deb_pkgs( ctx, remote, - version, - downstream_config): + version: str, + downstream_config: dict) -> None: """ Setup debian repo, Install gpg key and Install on debian packages @@ -197,7 +199,7 @@ def install_deb_pkgs( raise RuntimeError("Version check failed on node %s", remote.shortname) -def uninstall_pkgs(ctx, remote, downstream_config): +def uninstall_pkgs(ctx, remote, downstream_config: dict) -> None: """ Removes Ceph from all RH hosts diff --git a/teuthology/task/install/rpm.py b/teuthology/task/install/rpm.py index 0e526757d5..9c70fe3185 100644 --- a/teuthology/task/install/rpm.py +++ b/teuthology/task/install/rpm.py @@ -1,19 +1,19 @@ import logging import os.path from io import StringIO +from typing import List, Optional +from teuthology import packaging from teuthology.config import config as teuth_config from teuthology.contextutil import safe_while from teuthology.orchestra import run -from teuthology import packaging - from teuthology.task.install.util import _get_builder_project, _get_local_dir from teuthology.util.version import LooseVersion log = logging.getLogger(__name__) -def _remove(ctx, config, remote, rpm): +def _remove(ctx, config: dict, remote, rpm: List[str]) -> None: """ Removes RPM packages from remote @@ -66,7 +66,7 @@ def _remove(ctx, config, remote, rpm): remote.run(args='sudo yum clean expire-cache') -def _zypper_addrepo(remote, repo_list): +def _zypper_addrepo(remote, repo_list: List[dict]) -> None: """ Add zypper repos to the remote system. @@ -90,7 +90,7 @@ def _zypper_addrepo(remote, repo_list): # is invalid remote.run(args='sudo zypper ref ' + repo['name']) -def _zypper_removerepo(remote, repo_list): +def _zypper_removerepo(remote, repo_list: List[dict]) -> None: """ Remove zypper repos on the remote system. @@ -103,7 +103,7 @@ def _zypper_removerepo(remote, repo_list): 'sudo', 'zypper', '-n', 'removerepo', repo['name'], ]) -def _zypper_wipe_all_repos(remote): +def _zypper_wipe_all_repos(remote) -> None: """ Completely "wipe" (remove) all zypper repos @@ -116,7 +116,7 @@ def _zypper_wipe_all_repos(remote): 'true') -def _yum_addrepo(remote, repo_list): +def _yum_addrepo(remote, repo_list: List[dict]) -> None: """ Add dnf repos to the remote system. @@ -138,7 +138,7 @@ def _yum_addrepo(remote, repo_list): remote.sudo_write_file(repo_path, "\n".join(repo_lines)) -def _yum_removerepo(remote, repo_list): +def _yum_removerepo(remote, repo_list: List[dict]) -> None: """ Remove yum repos on the remote system. @@ -151,7 +151,7 @@ def _yum_removerepo(remote, repo_list): remote.run(args=['sudo', 'rm', repo_path]) -def _yum_wipe_all_repos(remote): +def _yum_wipe_all_repos(remote) -> None: """ Completely "wipe" (remove) all yum repos @@ -163,7 +163,7 @@ def _yum_wipe_all_repos(remote): 'true') -def _downgrade_packages(ctx, remote, pkgs, pkg_version, config): +def _downgrade_packages(ctx, remote, pkgs: List[str], pkg_version: str, config: dict) -> List[str]: """ Downgrade packages listed by 'downgrade_packages' @@ -194,7 +194,7 @@ def _downgrade_packages(ctx, remote, pkgs, pkg_version, config): remote.run(args='sudo yum -y downgrade {}'.format(' '.join(pkgs_opt))) return [pkg for pkg in pkgs if pkg not in downgrade_pkgs] -def _retry_if_failures_are_recoverable(remote, args): +def _retry_if_failures_are_recoverable(remote, args: str): # wait at most 5 minutes with safe_while(sleep=10, tries=30) as proceed: while proceed(): @@ -210,7 +210,7 @@ def _retry_if_failures_are_recoverable(remote, args): else: raise -def _update_package_list_and_install(ctx, remote, rpm, config): +def _update_package_list_and_install(ctx, remote, rpm: List[str], config: dict) -> Optional[None]: """ Installs the repository for the relevant branch, then installs the requested packages on the remote system. @@ -330,7 +330,7 @@ def _update_package_list_and_install(ctx, remote, rpm, config): .format(install_cmd=install_cmd, cpack=cpack) ) -def _yum_fix_repo_priority(remote, project, uri): +def _yum_fix_repo_priority(remote, project: str, uri: str) -> None: """ On the remote, add 'priority=1' lines to each enabled repo in: @@ -350,7 +350,7 @@ def _yum_fix_repo_priority(remote, project, uri): ) -def _yum_fix_repo_host(remote, project): +def _yum_fix_repo_host(remote, project: str) -> None: """ Update the hostname to reflect the gitbuilder_host setting. """ @@ -372,7 +372,7 @@ def _yum_fix_repo_host(remote, project): ) -def _yum_set_check_obsoletes(remote): +def _yum_set_check_obsoletes(remote) -> None: """ Set check_obsoletes = 1 in /etc/yum/pluginconf.d/priorities.conf @@ -396,7 +396,7 @@ def _yum_set_check_obsoletes(remote): remote.run(args=cmd) -def _yum_unset_check_obsoletes(remote): +def _yum_unset_check_obsoletes(remote) -> None: """ Restore the /etc/yum/pluginconf.d/priorities.conf backup """ @@ -406,7 +406,7 @@ def _yum_unset_check_obsoletes(remote): check_status=False) -def _remove_sources_list(ctx, config, remote): +def _remove_sources_list(ctx, config: dict, remote) -> None: """ Removes /etc/yum.repos.d/{proj}.repo @@ -421,7 +421,7 @@ def _remove_sources_list(ctx, config, remote): for copr in config.get('enable_coprs', []): remote.run(args=['sudo', 'dnf', '-y', 'copr', 'disable', copr]) -def _upgrade_packages(ctx, config, remote, pkgs): +def _upgrade_packages(ctx, config: dict, remote, pkgs: List[str]) -> None: """ Upgrade project's packages on remote RPM-based host Before doing so, it makes sure the project's repository is installed - diff --git a/teuthology/task/install/util.py b/teuthology/task/install/util.py index 46fbde9c9c..2519567911 100644 --- a/teuthology/task/install/util.py +++ b/teuthology/task/install/util.py @@ -1,6 +1,7 @@ import contextlib import logging import os +from typing import Generator, List, Optional from teuthology import misc as teuthology from teuthology import packaging @@ -9,7 +10,7 @@ log = logging.getLogger(__name__) -def _get_builder_project(ctx, remote, config): +def _get_builder_project(ctx, remote, config: dict): return packaging.get_builder_project()( config.get('project', 'ceph'), config, @@ -18,7 +19,7 @@ def _get_builder_project(ctx, remote, config): ) -def _get_local_dir(config, remote): +def _get_local_dir(config: dict, remote) -> Optional[str]: """ Extract local directory name from the task lists. Copy files over to the remote site. @@ -33,7 +34,7 @@ def _get_local_dir(config, remote): return ldir -def get_flavor(config): +def get_flavor(config: Optional[dict]) -> str: """ Determine the flavor to use. """ @@ -51,7 +52,7 @@ def get_flavor(config): flavor = 'gcov' return flavor -def _ship_utilities(ctx): +def _ship_utilities(ctx) -> List[str]: """ Write a copy of valgrind.supp to each of the remote sites. Set executables used by Ceph in /usr/local/bin. When finished (upon exit of the teuthology @@ -107,7 +108,7 @@ def _ship_utilities(ctx): ) return filenames -def _remove_utilities(ctx, filenames): +def _remove_utilities(ctx, filenames: List[str]) -> None: """ Remove the shipped utilities. @@ -130,7 +131,7 @@ def _remove_utilities(ctx, filenames): ) @contextlib.contextmanager -def ship_utilities(ctx, config): +def ship_utilities(ctx, config: None) -> Generator[None, None, None]: """ Ship utilities during the first call, and skip it in the following ones. See also `_ship_utilities`. diff --git a/teuthology/task/interactive.py b/teuthology/task/interactive.py index dd1676e49a..2052d93f14 100644 --- a/teuthology/task/interactive.py +++ b/teuthology/task/interactive.py @@ -2,14 +2,15 @@ Drop into a python shell """ import code +import pprint import readline import rlcompleter + rlcompleter.__name__ # silence pyflakes -import pprint readline.parse_and_bind('tab: complete') -def task(ctx, config): +def task(ctx, config) -> None: """ Run an interactive Python shell, with the cluster accessible via the ``ctx`` variable. diff --git a/teuthology/task/internal/__init__.py b/teuthology/task/internal/__init__.py index 15b8f81f54..222fec4ba2 100644 --- a/teuthology/task/internal/__init__.py +++ b/teuthology/task/internal/__init__.py @@ -6,19 +6,19 @@ import contextlib import functools import gzip +import humanfriendly import logging import os +import re import shutil -import time -import yaml import subprocess import tempfile -import re -import humanfriendly +import time +from typing import Generator +import yaml import teuthology.lock.ops -from teuthology import misc, packaging -from teuthology import report +from teuthology import misc, packaging, report from teuthology.config import config as teuth_config from teuthology.exceptions import ConfigError, VersionNotFoundError from teuthology.job_status import get_status, set_status @@ -32,7 +32,7 @@ @contextlib.contextmanager -def base(ctx, config): +def base(ctx, config) -> Generator[None, None, None]: """ Create the test directory that we will be using on the remote system """ @@ -60,7 +60,7 @@ def base(ctx, config): ) -def save_config(ctx, config): +def save_config(ctx, config) -> None: """ Store the config in a yaml file """ @@ -70,7 +70,7 @@ def save_config(ctx, config): yaml.safe_dump(ctx.config, f, default_flow_style=False) -def check_packages(ctx, config): +def check_packages(ctx, config) -> None: """ Checks gitbuilder to determine if there are missing packages for this job. @@ -124,7 +124,7 @@ def check_packages(ctx, config): @contextlib.contextmanager -def timer(ctx, config): +def timer(ctx, config) -> Generator[None, None, None]: """ Start the timer used by teuthology """ @@ -138,7 +138,7 @@ def timer(ctx, config): ctx.summary['duration'] = duration -def add_remotes(ctx, config): +def add_remotes(ctx, config) -> None: """ Create a ctx.cluster object populated with remotes mapped to roles """ @@ -170,7 +170,7 @@ def add_remotes(ctx, config): ctx.cluster.add(rem, rem.name) -def connect(ctx, config): +def connect(ctx, config) -> None: """ Connect to all remotes in ctx.cluster """ @@ -180,7 +180,7 @@ def connect(ctx, config): rem.connect() -def push_inventory(ctx, config): +def push_inventory(ctx, config) -> None: if not teuth_config.lock_server: return @@ -198,7 +198,7 @@ def push(): BUILDPACKAGES_REMOVED = 2 BUILDPACKAGES_NOTHING = 3 -def buildpackages_prep(ctx, config): +def buildpackages_prep(ctx, config) -> int: """ Make sure the 'buildpackages' task happens before the 'install' task. @@ -240,12 +240,12 @@ def buildpackages_prep(ctx, config): log.info('buildpackages removed because no install task found in ' + str(all_tasks)) return BUILDPACKAGES_REMOVED - elif buildpackages_index is None: + else: log.info('no buildpackages task found') return BUILDPACKAGES_NOTHING -def serialize_remote_roles(ctx, config): +def serialize_remote_roles(ctx, config) -> None: """ Provides an explicit mapping for which remotes have been assigned what roles So that other software can be loosely coupled to teuthology @@ -258,7 +258,7 @@ def serialize_remote_roles(ctx, config): yaml.safe_dump(info_yaml, info_file, default_flow_style=False) -def check_ceph_data(ctx, config): +def check_ceph_data(ctx, config) -> None: """ Check for old /var/lib/ceph subdirectories and detect staleness. """ @@ -278,7 +278,7 @@ def check_ceph_data(ctx, config): raise RuntimeError('Stale /var/lib/ceph detected, aborting.') -def check_conflict(ctx, config): +def check_conflict(ctx, config) -> None: """ Note directory use conflicts and stale directories. """ @@ -299,7 +299,7 @@ def check_conflict(ctx, config): raise RuntimeError('Stale jobs detected, aborting.') -def fetch_binaries_for_coredumps(path, remote): +def fetch_binaries_for_coredumps(path: str, remote) -> None: """ Pul ELFs (debug and stripped) for each coredump found """ @@ -364,7 +364,7 @@ def fetch_binaries_for_coredumps(path, remote): remote.get_file(debug_path, coredump_path) -def gzip_if_too_large(compress_min_size, src, tarinfo, local_path): +def gzip_if_too_large(compress_min_size: int, src, tarinfo, local_path: str) -> None: if tarinfo.size >= compress_min_size: with gzip.open(local_path + '.gz', 'wb') as dest: shutil.copyfileobj(src, dest) @@ -373,7 +373,7 @@ def gzip_if_too_large(compress_min_size, src, tarinfo, local_path): @contextlib.contextmanager -def archive(ctx, config): +def archive(ctx, config) -> Generator[None, None, None]: """ Handle the creation and deletion of the archive directory. """ @@ -430,7 +430,7 @@ def archive(ctx, config): @contextlib.contextmanager -def sudo(ctx, config): +def sudo(ctx, config) -> Generator[None, None, None]: """ Enable use of sudo """ @@ -461,7 +461,7 @@ def sudo(ctx, config): @contextlib.contextmanager -def coredump(ctx, config): +def coredump(ctx, config) -> Generator[None, None, None]: """ Stash a coredump of this system if an error occurs. """ @@ -524,7 +524,7 @@ def coredump(ctx, config): @contextlib.contextmanager -def archive_upload(ctx, config): +def archive_upload(ctx, config) -> Generator[None, None, None]: """ Upload the archive directory to a designated location """ diff --git a/teuthology/task/internal/check_lock.py b/teuthology/task/internal/check_lock.py index 152e41c2d9..01fb91ccb0 100644 --- a/teuthology/task/internal/check_lock.py +++ b/teuthology/task/internal/check_lock.py @@ -2,13 +2,12 @@ import teuthology.lock.query import teuthology.lock.util - from teuthology.config import config as teuth_config log = logging.getLogger(__name__) -def check_lock(ctx, config, check_up=True): +def check_lock(ctx, config, check_up: bool = True) -> None: """ Check lock status of remote machines. """ diff --git a/teuthology/task/internal/git_ignore_ssl.py b/teuthology/task/internal/git_ignore_ssl.py index b7bfa3ade2..6d44345dbe 100644 --- a/teuthology/task/internal/git_ignore_ssl.py +++ b/teuthology/task/internal/git_ignore_ssl.py @@ -1,5 +1,6 @@ import contextlib import logging +from typing import Generator from teuthology.orchestra import run @@ -7,7 +8,7 @@ @contextlib.contextmanager -def git_ignore_ssl(ctx, config): +def git_ignore_ssl(ctx, config) -> Generator[None, None, None]: """ Ignore ssl error's while cloning from untrusted http """ diff --git a/teuthology/task/internal/lock_machines.py b/teuthology/task/internal/lock_machines.py index fdbfcc2251..150d6041de 100644 --- a/teuthology/task/internal/lock_machines.py +++ b/teuthology/task/internal/lock_machines.py @@ -1,5 +1,6 @@ import contextlib import logging +from typing import Generator, List, Union import teuthology.lock.ops import teuthology.lock.query @@ -9,7 +10,7 @@ @contextlib.contextmanager -def lock_machines(ctx, config): +def lock_machines(ctx, config: List[Union[int, str]]) -> Generator[None, None, None]: """ Lock machines. Called when the teuthology run finds and locks new machines. This is not called if the one has teuthology-locked diff --git a/teuthology/task/internal/redhat.py b/teuthology/task/internal/redhat.py index 3f2030bd67..f2e1ef6651 100644 --- a/teuthology/task/internal/redhat.py +++ b/teuthology/task/internal/redhat.py @@ -3,21 +3,24 @@ """ import contextlib import logging -import requests -import yaml import os from tempfile import NamedTemporaryFile +from typing import Dict, Generator, List + +import requests +import yaml + from teuthology.config import config as teuthconfig -from teuthology.parallel import parallel +from teuthology.exceptions import CommandFailedError, ConfigError from teuthology.orchestra import run +from teuthology.parallel import parallel from teuthology.task.install.redhat import set_deb_repo -from teuthology.exceptions import CommandFailedError, ConfigError log = logging.getLogger(__name__) @contextlib.contextmanager -def setup_stage_cdn(ctx, config): +def setup_stage_cdn(ctx, config) -> Generator[None, None, None]: """ Configure internal stage cdn """ @@ -44,7 +47,7 @@ def setup_stage_cdn(ctx, config): p.spawn(_unsubscribe_stage_cdn, remote) -def _subscribe_stage_cdn(remote): +def _subscribe_stage_cdn(remote) -> None: _unsubscribe_stage_cdn(remote) cdn_config = teuthconfig.get('cdn-config', dict()) server_url = cdn_config.get('server-url', 'subscription.rhsm.stage.redhat.com:443/subscription') @@ -64,7 +67,7 @@ def _subscribe_stage_cdn(remote): _enable_rhel_repos(remote) -def _unsubscribe_stage_cdn(remote): +def _unsubscribe_stage_cdn(remote) -> None: try: remote.run(args=['sudo', 'subscription-manager', 'unregister'], check_status=False) @@ -74,7 +77,7 @@ def _unsubscribe_stage_cdn(remote): @contextlib.contextmanager -def setup_cdn_repo(ctx, config): +def setup_cdn_repo(ctx, config) -> Generator[None, None, None]: """ setup repo if set_cdn_repo exists in config redhat: @@ -89,7 +92,7 @@ def setup_cdn_repo(ctx, config): yield @contextlib.contextmanager -def setup_container_registry(ctx, config): +def setup_container_registry(ctx, config) -> Generator[None, None, None]: """ setup container registry if setup_container_registry in config @@ -120,7 +123,7 @@ def setup_container_registry(ctx, config): yield @contextlib.contextmanager -def setup_additional_repo(ctx, config): +def setup_additional_repo(ctx, config) -> Generator[None, None, None]: """ set additional repo's for testing redhat: @@ -138,7 +141,7 @@ def setup_additional_repo(ctx, config): yield -def _enable_rhel_repos(remote): +def _enable_rhel_repos(remote) -> None: # Look for rh specific repos ds_yaml = os.path.join( @@ -155,7 +158,7 @@ def _enable_rhel_repos(remote): @contextlib.contextmanager -def setup_base_repo(ctx, config): +def setup_base_repo(ctx, config) -> Generator[None, None, None]: """ Setup repo based on redhat nodes redhat: @@ -184,7 +187,7 @@ def setup_base_repo(ctx, config): ], check_status=False) -def _setup_latest_repo(ctx, config): +def _setup_latest_repo(ctx, config: dict) -> None: """ Setup repo based on redhat nodes """ @@ -240,7 +243,7 @@ def _setup_latest_repo(ctx, config): set_deb_repo(remote, deb_repo, deb_gpg_key) -def _get_repos_to_use(base_url, repos): +def _get_repos_to_use(base_url: str, repos: List[str]) -> Dict[str, str]: repod = dict() for repo in repos: repo_to_use = base_url + "compose/" + repo + "/x86_64/os/" @@ -252,7 +255,7 @@ def _get_repos_to_use(base_url, repos): return repod -def _create_temp_repo_file(repos, repo_file): +def _create_temp_repo_file(repos: Dict[str, str], repo_file) -> None: for repo in repos.keys(): header = "[ceph-" + repo + "]" + "\n" name = "name=ceph-" + repo + "\n" diff --git a/teuthology/task/internal/syslog.py b/teuthology/task/internal/syslog.py index b103c8b7c9..5fc6d9e967 100644 --- a/teuthology/task/internal/syslog.py +++ b/teuthology/task/internal/syslog.py @@ -1,18 +1,17 @@ import contextlib import logging - from io import BytesIO +from typing import Generator from teuthology import misc from teuthology.job_status import set_status from teuthology.orchestra import run - log = logging.getLogger(__name__) @contextlib.contextmanager -def syslog(ctx, config): +def syslog(ctx, config) -> Generator[None, None, None]: """ start syslog / stop syslog on exit. """ diff --git a/teuthology/task/internal/vm_setup.py b/teuthology/task/internal/vm_setup.py index f210bc7f41..b98fbf7de7 100644 --- a/teuthology/task/internal/vm_setup.py +++ b/teuthology/task/internal/vm_setup.py @@ -2,14 +2,14 @@ import os import subprocess +from teuthology.exceptions import CommandFailedError from teuthology.parallel import parallel from teuthology.task import ansible -from teuthology.exceptions import CommandFailedError log = logging.getLogger(__name__) -def vm_setup(ctx, config): +def vm_setup(ctx, config) -> None: """ Look for virtual machines and handle their initialization """ diff --git a/teuthology/task/iscsi.py b/teuthology/task/iscsi.py index 80d01cb8f1..059aeed3f9 100644 --- a/teuthology/task/iscsi.py +++ b/teuthology/task/iscsi.py @@ -1,38 +1,42 @@ """ Handle iscsi adm commands for tgt connections. """ -import logging import contextlib +import logging import socket +from typing import Generator, List, Optional, Tuple -from teuthology import misc as teuthology from teuthology import contextutil -from teuthology.task.common_fs_utils import generic_mkfs -from teuthology.task.common_fs_utils import generic_mount +from teuthology import misc as teuthology from teuthology.orchestra import run +from teuthology.task.common_fs_utils import generic_mkfs, generic_mount log = logging.getLogger(__name__) -def _get_remote(remotes, client): +def _get_remote(remotes: dict, client: str) -> Optional: """ Get remote object that is associated with the client specified. """ for rem in remotes: if client in remotes[rem]: return rem + return None -def _get_remote_name(remotes, client): +def _get_remote_name(remotes: dict, client: str) -> str: """ Get remote name that is associated with the client specified. """ - rem_name = _get_remote(remotes, client).name + rem = _get_remote(remotes, client) + if rem is None: + raise ValueError(f"No remote found for client {client}") + rem_name = rem.name rem_name = rem_name[rem_name.find('@') + 1:] return rem_name -def tgt_devname_get(ctx, test_image): +def tgt_devname_get(ctx, test_image: str) -> str: """ Get the name of the newly created device by following the by-path link (which is symbolically linked to the appropriate /dev/sd* file). @@ -44,7 +48,7 @@ def tgt_devname_get(ctx, test_image): return lnkpath -def tgt_devname_rtn(ctx, test_image): +def tgt_devname_rtn(ctx, test_image: str) -> str: """ Wrapper passed to common_fs_util functions. """ @@ -52,7 +56,7 @@ def tgt_devname_rtn(ctx, test_image): return tgt_devname_get(ctx, image) -def file_io_test(rem, file_from, lnkpath): +def file_io_test(rem, file_from: str, lnkpath: str) -> None: """ dd to the iscsi inteface, read it, and compare with original """ @@ -96,7 +100,7 @@ def file_io_test(rem, file_from, lnkpath): rem.run(args=['rm', tfile2]) -def general_io_test(ctx, rem, image_name): +def general_io_test(ctx, rem, image_name: str) -> None: """ Do simple I/O tests to the iscsi interface before putting a filesystem on it. @@ -122,7 +126,7 @@ def general_io_test(ctx, rem, image_name): @contextlib.contextmanager -def start_iscsi_initiators(ctx, tgt_link): +def start_iscsi_initiators(ctx, tgt_link: List[Tuple[str, str]]) -> Generator[None, None, None]: """ This is the sub-task that assigns an rbd to an iscsiadm control and performs a login (thereby creating a /dev/sd device). It performs @@ -177,7 +181,7 @@ def start_iscsi_initiators(ctx, tgt_link): ]) @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config) -> Generator[None, None, None]: """ handle iscsi admin login after a tgt connection has been established. diff --git a/teuthology/task/kernel.py b/teuthology/task/kernel.py index 1a0dff2109..62c84d1500 100644 --- a/teuthology/task/kernel.py +++ b/teuthology/task/kernel.py @@ -8,6 +8,7 @@ import re import shlex from io import StringIO +from typing import Generator, Optional from teuthology.util.compat import urljoin @@ -1278,7 +1279,7 @@ def get_sha1_from_pkg_name(path): @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config: Optional[dict]) -> Generator[None, None, None]: """ Make sure the specified kernel is installed. This can be a branch, tag, or sha1 of ceph-client.git or a local diff --git a/teuthology/task/knfsd.py b/teuthology/task/knfsd.py index 100671d822..588b869e41 100644 --- a/teuthology/task/knfsd.py +++ b/teuthology/task/knfsd.py @@ -4,12 +4,13 @@ import contextlib import logging import os +from typing import Generator, List, Optional, Union from teuthology import misc as teuthology log = logging.getLogger(__name__) -def get_nfsd_args(remote, cmd): +def get_nfsd_args(remote, cmd: str) -> List[str]: args=[ 'sudo', 'service', @@ -21,7 +22,7 @@ def get_nfsd_args(remote, cmd): return args @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config: Optional[Union[List[str], dict]]) -> Generator[None, None, None]: """ Export/Unexport a ``nfs server`` client. diff --git a/teuthology/task/localdir.py b/teuthology/task/localdir.py index 8a84514651..505b9eaec3 100644 --- a/teuthology/task/localdir.py +++ b/teuthology/task/localdir.py @@ -4,6 +4,7 @@ import contextlib import logging import os +from typing import Generator, List, Optional from teuthology import misc as teuthology @@ -11,7 +12,7 @@ @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config: Optional[List[str]]) -> Generator[None, None, None]: """ Create a mount dir 'client' that is just the local disk: diff --git a/teuthology/task/lockfile.py b/teuthology/task/lockfile.py index 63ff9f3b12..f865384280 100644 --- a/teuthology/task/lockfile.py +++ b/teuthology/task/lockfile.py @@ -3,16 +3,16 @@ """ import logging import os - -from teuthology.orchestra import run -from teuthology import misc as teuthology import time + import gevent +from teuthology import misc as teuthology +from teuthology.orchestra import run log = logging.getLogger(__name__) -def task(ctx, config): +def task(ctx, config: dict) -> None: """ This task is designed to test locking. It runs an executable for each lock attempt you specify, at 0.01 second intervals (to @@ -184,7 +184,7 @@ def task(ctx, config): #done! # task -def lock_one(op, ctx): +def lock_one(op: dict, ctx) -> bool: """ Perform the individual lock """ diff --git a/teuthology/task/loop.py b/teuthology/task/loop.py index cd48df1cca..668b2e61ea 100644 --- a/teuthology/task/loop.py +++ b/teuthology/task/loop.py @@ -1,14 +1,14 @@ """ Task to loop a list of items """ -import sys import logging +import sys from teuthology import run_tasks log = logging.getLogger(__name__) -def task(ctx, config): +def task(ctx, config: dict) -> None: """ Loop a sequential group of tasks diff --git a/teuthology/task/mpi.py b/teuthology/task/mpi.py index 0e1b504dd5..4e334c1982 100644 --- a/teuthology/task/mpi.py +++ b/teuthology/task/mpi.py @@ -9,7 +9,7 @@ log = logging.getLogger(__name__) -def _check_mpi_version(remotes): +def _check_mpi_version(remotes: list) -> None: """ Retrieve the MPI version from each of `remotes` and raise an exception if they are not all the same version. @@ -30,7 +30,7 @@ def _check_mpi_version(remotes): log.info("MPI version {0}".format(list(versions)[0])) -def task(ctx, config): +def task(ctx, config: dict) -> None: """ Setup MPI and execute commands diff --git a/teuthology/task/nfs.py b/teuthology/task/nfs.py index 5cd3aac81f..8808e7ea62 100644 --- a/teuthology/task/nfs.py +++ b/teuthology/task/nfs.py @@ -4,13 +4,14 @@ import contextlib import logging import os +from typing import Dict, Generator from teuthology import misc as teuthology log = logging.getLogger(__name__) @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config: Dict[str, dict]) -> Generator[None, None, None]: """ Mount nfs client (requires nfs server export like knfsd or ganesh) @@ -63,6 +64,7 @@ def task(ctx, config): assert client_config.get('server') is not None server = client_config.get('server'); + assert server svr_id = server[len('client.'):] svr_mnt = os.path.join(testdir, 'mnt.{id}'.format(id=svr_id)) @@ -79,8 +81,8 @@ def task(ctx, config): assert svr_remote is not None svr_remote = svr_remote.name.split('@', 2)[1] - if client_config.get('options') is not None: - opts = ','.join(client_config.get('options')) + if (opts := client_config.get('options')) is not None: + opts = ','.join(opts) else: opts = 'rw' diff --git a/teuthology/task/nop.py b/teuthology/task/nop.py index c7b181403f..4b6ad11300 100644 --- a/teuthology/task/nop.py +++ b/teuthology/task/nop.py @@ -1,7 +1,8 @@ """ Null task """ -def task(ctx, config): + +def task(ctx, config) -> None: """ This task does nothing. diff --git a/teuthology/task/parallel.py b/teuthology/task/parallel.py index 6999c0aae3..3b3129e8aa 100644 --- a/teuthology/task/parallel.py +++ b/teuthology/task/parallel.py @@ -1,8 +1,8 @@ """ Task to group parallel running tasks """ -import sys import logging +import sys from teuthology import run_tasks from teuthology import parallel @@ -10,7 +10,7 @@ log = logging.getLogger(__name__) -def task(ctx, config): +def task(ctx, config: list) -> None: """ Run a group of tasks in parallel. @@ -56,7 +56,7 @@ def task(ctx, config): p.spawn(_run_spawned, ctx, confg, taskname) -def _run_spawned(ctx, config, taskname): +def _run_spawned(ctx, config, taskname: str) -> None: """Run one of the tasks (this runs in parallel with others)""" mgr = {} try: diff --git a/teuthology/task/parallel_example.py b/teuthology/task/parallel_example.py index eb9659a81d..7d2ae1b38a 100644 --- a/teuthology/task/parallel_example.py +++ b/teuthology/task/parallel_example.py @@ -3,15 +3,16 @@ """ import contextlib import logging +from typing import Generator, List, Optional, Union -from teuthology import misc as teuthology from teuthology import contextutil +from teuthology import misc as teuthology from teuthology.orchestra import run log = logging.getLogger(__name__) @contextlib.contextmanager -def sequential_test(ctx, config): +def sequential_test(ctx, config: List[str]) -> Generator[None, None, None]: """Example contextmanager that executes a command on remote hosts sequentially.""" for role in config: """Create a cluster composed of all hosts with the given role, and run the command on them sequentially.""" @@ -20,7 +21,7 @@ def sequential_test(ctx, config): yield @contextlib.contextmanager -def parallel_test(ctx, config): +def parallel_test(ctx, config: List[str]) -> Generator[None, None, None]: """Example contextmanager that executes a command on remote hosts in parallel.""" for role in config: """Create a cluster composed of all hosts with the given role, and run the command on them concurrently.""" @@ -37,7 +38,7 @@ def parallel_test(ctx, config): yield @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config: Optional[Union[List[str], dict]]) -> Generator[None, None, None]: """This is the main body of the task that gets run.""" """Take car of some yaml parsing here""" diff --git a/teuthology/task/pcp.py b/teuthology/task/pcp.py index 80458a1317..24af85bcd4 100644 --- a/teuthology/task/pcp.py +++ b/teuthology/task/pcp.py @@ -6,6 +6,7 @@ import os import requests import time +from typing import List, Optional, Union from teuthology.util.compat import urljoin, urlencode @@ -24,7 +25,7 @@ class PCPDataSource(object): - def __init__(self, hosts, time_from, time_until='now'): + def __init__(self, hosts: List[str], time_from: Union[int, str], time_until: Union[int, str] = 'now') -> None: self.hosts = hosts self.time_from = time_from self.time_until = time_until @@ -34,13 +35,13 @@ class PCPArchive(PCPDataSource): archive_base_path = '/var/log/pcp/pmlogger' archive_file_extensions = ('0', 'index', 'meta') - def get_archive_input_dir(self, host): + def get_archive_input_dir(self, host: str) -> str: return os.path.join( self.archive_base_path, host, ) - def get_pmlogextract_cmd(self, host): + def get_pmlogextract_cmd(self, host: str) -> list: cmd = [ 'pmlogextract', '-S', self._format_time(self.time_from), @@ -52,7 +53,7 @@ def get_pmlogextract_cmd(self, host): return cmd @staticmethod - def _format_time(seconds): + def _format_time(seconds: Union[int, str]) -> str: if isinstance(seconds, str): return seconds return "@ %s" % time.asctime(time.gmtime(seconds)) @@ -61,7 +62,7 @@ def _format_time(seconds): class PCPGrapher(PCPDataSource): _endpoint = '/' - def __init__(self, hosts, time_from, time_until='now'): + def __init__(self, hosts: List[str], time_from: Union[int, str], time_until: Union[int, str] = 'now') -> None: super(PCPGrapher, self).__init__(hosts, time_from, time_until) self.base_url = urljoin( teuth_config.pcp_host, @@ -71,11 +72,11 @@ def __init__(self, hosts, time_from, time_until='now'): class GrafanaGrapher(PCPGrapher): _endpoint = '/grafana/index.html#/dashboard/script/index.js' - def __init__(self, hosts, time_from, time_until='now', job_id=None): + def __init__(self, hosts: List[str], time_from: Union[int, str], time_until: Union[int, str] = 'now', job_id: Optional[str] = None) -> None: super(GrafanaGrapher, self).__init__(hosts, time_from, time_until) self.job_id = job_id - def build_graph_url(self): + def build_graph_url(self) -> str: config = dict( hosts=','.join(self.hosts), time_from=self._format_time(self.time_from), @@ -87,7 +88,7 @@ def build_graph_url(self): return template.format(base_url=self.base_url, args=args) @staticmethod - def _format_time(seconds): + def _format_time(seconds: Union[int, str]) -> str: if isinstance(seconds, str): return seconds seconds = int(seconds) @@ -113,13 +114,13 @@ class GraphiteGrapher(PCPGrapher): ) _endpoint = '/graphite/render' - def __init__(self, hosts, time_from, time_until='now', dest_dir=None, - job_id=None): + def __init__(self, hosts: List[str], time_from: Union[int, str], time_until: Union[int, str] = 'now', dest_dir: Optional[str] = None, + job_id: Optional[str] = None) -> None: super(GraphiteGrapher, self).__init__(hosts, time_from, time_until) self.dest_dir = dest_dir self.job_id = job_id - def build_graph_urls(self): + def build_graph_urls(self) -> None: if not hasattr(self, 'graphs'): self.graphs = dict() for metric in self.metrics: @@ -127,18 +128,18 @@ def build_graph_urls(self): metric_dict['url'] = self.get_graph_url(metric) self.graphs[metric] = metric_dict - def _check_dest_dir(self): + def _check_dest_dir(self) -> None: if not self.dest_dir: raise RuntimeError("Must provide a dest_dir!") - def write_html(self, mode='dynamic'): + def write_html(self, mode: str = 'dynamic') -> None: self._check_dest_dir() generated_html = self.generate_html(mode=mode) html_path = os.path.join(self.dest_dir, 'pcp.html') with open(html_path, 'w') as f: f.write(generated_html) - def generate_html(self, mode='dynamic'): + def generate_html(self, mode: str = 'dynamic') -> str: self.build_graph_urls() cwd = os.path.dirname(__file__) loader = jinja2.loaders.FileSystemLoader(cwd) @@ -151,7 +152,7 @@ def generate_html(self, mode='dynamic'): ) return data - def download_graphs(self): + def download_graphs(self) -> None: self._check_dest_dir() self.build_graph_urls() for metric in self.graphs.keys(): @@ -173,7 +174,7 @@ def download_graphs(self): with open(graph_path, 'wb') as f: f.write(resp.content) - def get_graph_url(self, metric): + def get_graph_url(self, metric: str) -> str: config = dict(self.graph_defaults) config.update({ 'from': self.time_from, @@ -186,14 +187,14 @@ def get_graph_url(self, metric): template = "{base_url}?{args}" return template.format(base_url=self.base_url, args=args) - def get_target_globs(self, metric=''): + def get_target_globs(self, metric: str = '') -> List[str]: globs = ['*{}*'.format(host) for host in self.hosts] if metric: globs = ['{}.{}'.format(glob, metric) for glob in globs] return globs @staticmethod - def _sanitize_metric_name(metric): + def _sanitize_metric_name(metric: str) -> str: result = metric replacements = [ (' ', '_'), @@ -220,7 +221,7 @@ class PCP(Task): """ enabled = True - def __init__(self, ctx, config): + def __init__(self, ctx, config: dict) -> None: super(PCP, self).__init__(ctx, config) if teuth_config.get('pcp_host') is None: self.enabled = False @@ -235,7 +236,7 @@ def __init__(self, ctx, config): # pmlogextract self.fetch_archives = self.config.get('fetch_archives', False) - def setup(self): + def setup(self) -> None: if not self.enabled: return super(PCP, self).setup() @@ -243,14 +244,14 @@ def setup(self): log.debug("start_time: %s", self.start_time) self.setup_collectors() - def setup_collectors(self): + def setup_collectors(self) -> None: log.debug("cluster: %s", self.cluster) hosts = [rem.shortname for rem in self.cluster.remotes.keys()] self.setup_grafana(hosts) self.setup_graphite(hosts) self.setup_archive(hosts) - def setup_grafana(self, hosts): + def setup_grafana(self, hosts: List[str]) -> None: if self.use_grafana: self.grafana = GrafanaGrapher( hosts=hosts, @@ -259,7 +260,7 @@ def setup_grafana(self, hosts): job_id=self.job_id, ) - def setup_graphite(self, hosts): + def setup_graphite(self, hosts: List[str]) -> None: if not getattr(self.ctx, 'archive', None): self.use_graphite = False if self.use_graphite: @@ -278,7 +279,7 @@ def setup_graphite(self, hosts): job_id=self.job_id, ) - def setup_archive(self, hosts): + def setup_archive(self, hosts: List[str]) -> None: if not getattr(self.ctx, 'archive', None): self.fetch_archives = False if self.fetch_archives: @@ -288,7 +289,7 @@ def setup_archive(self, hosts): time_until=self.stop_time, ) - def begin(self): + def begin(self) -> None: if not self.enabled: return if self.use_grafana: @@ -299,7 +300,7 @@ def begin(self): if self.use_graphite: self.graphite.write_html() - def end(self): + def end(self) -> None: if not self.enabled: return self.stop_time = int(time.time()) diff --git a/teuthology/task/pexec.py b/teuthology/task/pexec.py index d61979e9c8..713db23000 100644 --- a/teuthology/task/pexec.py +++ b/teuthology/task/pexec.py @@ -2,21 +2,22 @@ Handle parallel execution on remote hosts """ import logging +from typing import Generator, List, Tuple + +from gevent import event as event +from gevent import queue as queue from teuthology import misc as teuthology -from teuthology.parallel import parallel from teuthology.orchestra import run as tor +from teuthology.parallel import parallel log = logging.getLogger(__name__) -from gevent import queue as queue -from gevent import event as event - -def _init_barrier(barrier_queue, remote): +def _init_barrier(barrier_queue: queue.Queue, remote) -> None: """current just queues a remote host""" barrier_queue.put(remote) -def _do_barrier(barrier, barrier_queue, remote): +def _do_barrier(barrier: event.Event, barrier_queue: queue.Queue, remote) -> None: """special case for barrier""" barrier_queue.get() if barrier_queue.empty(): @@ -32,7 +33,7 @@ def _do_barrier(barrier, barrier_queue, remote): else: barrier.wait() -def _exec_host(barrier, barrier_queue, remote, sudo, testdir, ls): +def _exec_host(barrier: event.Event, barrier_queue: queue.Queue, remote, sudo: bool, testdir: str, ls: List[str]) -> None: """Execute command remotely""" args = [ 'TESTDIR={tdir}'.format(tdir=testdir), @@ -61,7 +62,7 @@ def _exec_host(barrier, barrier_queue, remote, sudo, testdir, ls): log.info('%s', l) tor.wait([r]) -def _generate_remotes(ctx, config): +def _generate_remotes(ctx, config: dict) -> Generator[Tuple[object, list[str]], None, None]: """Return remote roles and the type of role specified in config""" if 'all' in config and len(config) == 1: ls = config['all'] @@ -81,7 +82,7 @@ def _generate_remotes(ctx, config): (remote,) = ctx.cluster.only(role).remotes.keys() yield (remote, ls) -def task(ctx, config): +def task(ctx, config: dict) -> None: """ Execute commands on multiple hosts in parallel diff --git a/teuthology/task/print.py b/teuthology/task/print.py index 6594c16819..4f4ec6a136 100644 --- a/teuthology/task/print.py +++ b/teuthology/task/print.py @@ -18,7 +18,7 @@ log = logging.getLogger(__name__) -def task(ctx, config): +def task(ctx, config) -> None: """ Print out config argument in teuthology log/output """ diff --git a/teuthology/task/proc_thrasher.py b/teuthology/task/proc_thrasher.py index c01911c5a8..b287a74e8c 100644 --- a/teuthology/task/proc_thrasher.py +++ b/teuthology/task/proc_thrasher.py @@ -2,10 +2,11 @@ Process thrasher """ import logging -import gevent import random import time +import gevent + from teuthology.orchestra import run log = logging.getLogger(__name__) @@ -14,7 +15,7 @@ class ProcThrasher: """ Kills and restarts some number of the specified process on the specified remote """ - def __init__(self, config, remote, *proc_args, **proc_kwargs): + def __init__(self, config: dict, remote, *proc_args, **proc_kwargs) -> None: self.proc_kwargs = proc_kwargs self.proc_args = proc_args self.config = config @@ -27,13 +28,13 @@ def __init__(self, config, remote, *proc_args, **proc_kwargs): self.rest_period = self.config.get("rest_period", 100) # seconds self.run_time = self.config.get("run_time", 1000) # seconds - def log(self, msg): + def log(self, msg: str) -> None: """ Local log wrapper """ self.logger.info(msg) - def start(self): + def start(self) -> None: """ Start thrasher. This also makes sure that the greenlet interface is used. @@ -43,13 +44,13 @@ def start(self): self.greenlet = gevent.Greenlet(self.loop) self.greenlet.start() - def join(self): + def join(self) -> None: """ Local join """ self.greenlet.join() - def loop(self): + def loop(self) -> None: """ Thrashing loop -- loops at time intervals. Inside that loop, the code loops through the individual procs, creating new procs. diff --git a/teuthology/task/selinux.py b/teuthology/task/selinux.py index baf7a9003c..06e1b2df8b 100644 --- a/teuthology/task/selinux.py +++ b/teuthology/task/selinux.py @@ -1,7 +1,7 @@ import logging import os - from io import StringIO +from typing import Dict, Optional from teuthology.exceptions import SELinuxError from teuthology.misc import get_archive_dir @@ -38,12 +38,12 @@ class SELinux(Task): Automatically skips hosts running non-RPM-based OSes. """ - def __init__(self, ctx, config): + def __init__(self, ctx, config: dict) -> None: super(SELinux, self).__init__(ctx, config) self.log = log self.mode = self.config.get('mode', 'permissive') - def filter_hosts(self): + def filter_hosts(self) -> Optional[Cluster]: """ Exclude any non-RPM-based hosts, and any downburst VMs """ @@ -68,17 +68,17 @@ def filter_hosts(self): self.cluster = new_cluster return self.cluster - def setup(self): + def setup(self) -> None: super(SELinux, self).setup() self.rotate_log() self.old_modes = self.get_modes() self.old_denials = self.get_denials() self.set_mode() - def rotate_log(self): + def rotate_log(self) -> None: self.cluster.run(args="sudo service auditd rotate") - def get_modes(self): + def get_modes(self) -> Dict[str, str]: """ Get the current SELinux mode from each host so that we can restore during teardown @@ -95,7 +95,7 @@ def get_modes(self): log.debug("Existing SELinux modes: %s", modes) return modes - def set_mode(self): + def set_mode(self) -> None: """ Set the requested SELinux mode """ @@ -108,7 +108,7 @@ def set_mode(self): args=['sudo', '/usr/sbin/setenforce', self.mode], ) - def get_denials(self): + def get_denials(self) -> Dict[str, list]: """ Look for denials in the audit log """ @@ -163,12 +163,12 @@ def get_denials(self): all_denials[remote.name] = denials return all_denials - def teardown(self): + def teardown(self) -> None: self.restore_modes() self.archive_log() self.get_new_denials() - def restore_modes(self): + def restore_modes(self) -> None: """ If necessary, restore previous SELinux modes """ @@ -185,7 +185,7 @@ def restore_modes(self): args=['sudo', '/usr/sbin/setenforce', mode], ) - def archive_log(self): + def archive_log(self) -> None: if not hasattr(self.ctx, 'archive') or not self.ctx.archive: return archive_dir = get_archive_dir(self.ctx) @@ -199,7 +199,7 @@ def archive_log(self): args=full_cmd.format(audit_archive=audit_archive) ) - def get_new_denials(self): + def get_new_denials(self) -> None: """ Determine if there are any new denials in the audit log """ diff --git a/teuthology/task/sequential.py b/teuthology/task/sequential.py index 2414336fe2..e45e4a9882 100644 --- a/teuthology/task/sequential.py +++ b/teuthology/task/sequential.py @@ -1,15 +1,15 @@ """ Task sequencer """ -import sys import logging +import sys from teuthology import run_tasks log = logging.getLogger(__name__) -def task(ctx, config): +def task(ctx, config: list) -> None: """ Sequentialize a group of tasks into one executable block diff --git a/teuthology/task/sleep.py b/teuthology/task/sleep.py index bd6d445446..6fb7c2131d 100644 --- a/teuthology/task/sleep.py +++ b/teuthology/task/sleep.py @@ -3,11 +3,12 @@ """ import logging import time +from typing import Optional log = logging.getLogger(__name__) -def task(ctx, config): +def task(ctx, config: Optional[dict]) -> None: """ Sleep for some number of seconds. diff --git a/teuthology/task/ssh_keys.py b/teuthology/task/ssh_keys.py index 0f497ebe90..99e64b3b52 100644 --- a/teuthology/task/ssh_keys.py +++ b/teuthology/task/ssh_keys.py @@ -7,8 +7,9 @@ import logging import paramiko import re - from io import StringIO +from typing import Generator, Optional, Tuple + from teuthology import contextutil import teuthology.misc as misc from teuthology.orchestra import run @@ -17,14 +18,14 @@ ssh_keys_user = 'ssh-keys-user' -def timestamp(format_='%Y-%m-%d_%H:%M:%S:%f'): +def timestamp(format_: str = '%Y-%m-%d_%H:%M:%S:%f') -> str: """ Return a UTC timestamp suitable for use in filenames """ return datetime.datetime.now(datetime.timezone.utc).strftime(format_) -def backup_file(remote, path, sudo=False): +def backup_file(remote, path: str, sudo: bool = False) -> str: """ Creates a backup of a file on the remote, simply by copying it and adding a timestamp to the name. @@ -41,7 +42,7 @@ def backup_file(remote, path, sudo=False): return backup_path -def generate_keys(): +def generate_keys() -> Tuple[str, str]: """ Generatees a public and private key """ @@ -50,7 +51,7 @@ def generate_keys(): key.write_private_key(privateString) return key.get_base64(), privateString.getvalue() -def particular_ssh_key_test(line_to_test, ssh_key): +def particular_ssh_key_test(line_to_test: str, ssh_key: str) -> bool: """ Check the validity of the ssh_key """ @@ -61,7 +62,7 @@ def particular_ssh_key_test(line_to_test, ssh_key): else: return True -def ssh_keys_user_line_test(line_to_test, username ): +def ssh_keys_user_line_test(line_to_test: str, username: str) -> bool: """ Check the validity of the username """ @@ -72,7 +73,7 @@ def ssh_keys_user_line_test(line_to_test, username ): else: return True -def cleanup_added_key(ctx, key_backup_files, path): +def cleanup_added_key(ctx, key_backup_files: dict, path: str) -> None: """ Delete the keys and removes ~/.ssh/authorized_keys entries we added """ @@ -89,7 +90,7 @@ def cleanup_added_key(ctx, key_backup_files, path): misc.move_file(remote, key_backup_files[remote], path) @contextlib.contextmanager -def tweak_ssh_config(ctx, config): +def tweak_ssh_config(ctx, config: dict) -> Generator[None, None, None]: """ Turn off StrictHostKeyChecking """ @@ -125,7 +126,7 @@ def tweak_ssh_config(ctx, config): ) @contextlib.contextmanager -def push_keys_to_host(ctx, config, public_key, private_key): +def push_keys_to_host(ctx, config: dict, public_key: str, private_key: str) -> Generator[None, None, None]: """ Push keys to all hosts """ @@ -178,7 +179,7 @@ def push_keys_to_host(ctx, config, public_key, private_key): @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config: Optional[dict]) -> Generator[None, None, None]: """ Creates a set of RSA keys, distributes the same key pair to all hosts listed in ctx.cluster, and adds all hosts diff --git a/teuthology/task/tasktest.py b/teuthology/task/tasktest.py index 40926c569c..a5255b2dc6 100644 --- a/teuthology/task/tasktest.py +++ b/teuthology/task/tasktest.py @@ -2,14 +2,15 @@ Parallel and sequential task tester. Not used by any ceph tests, but used to unit test the parallel and sequential tasks """ -import logging import contextlib +import logging import time +from typing import Generator log = logging.getLogger(__name__) @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config) -> Generator[None, None, None]: """ Task that just displays information when it is create and when it is destroyed/cleaned up. This task was used to test parallel and diff --git a/teuthology/task/timer.py b/teuthology/task/timer.py index 2abf188275..295b306ae8 100644 --- a/teuthology/task/timer.py +++ b/teuthology/task/timer.py @@ -1,14 +1,15 @@ """ Timer task """ -import logging import contextlib import datetime +import logging +from typing import Generator log = logging.getLogger(__name__) @contextlib.contextmanager -def task(ctx, config): +def task(ctx, config: str) -> Generator[None, None, None]: """ Timer diff --git a/teuthology/timer.py b/teuthology/timer.py index 9ca5d76a71..fbae219ca0 100644 --- a/teuthology/timer.py +++ b/teuthology/timer.py @@ -1,6 +1,7 @@ import logging import time import yaml +from typing import Optional import datetime @@ -19,7 +20,7 @@ class Timer(object): # The format to use for date-time strings datetime_format = '%Y-%m-%d_%H:%M:%S' - def __init__(self, path=None, sync=False): + def __init__(self, path: Optional[str] = None, sync: bool = False) -> None: """ :param path: A path to a file to be written when self.write() is called. The file will contain self.data in yaml @@ -36,7 +37,7 @@ def __init__(self, path=None, sync=False): self.start_time = None self.start_string = None - def mark(self, message=''): + def mark(self, message: str = '') -> None: """ Create a time mark @@ -45,7 +46,8 @@ def mark(self, message=''): the time elapsed in seconds since time-keeping began. """ if self.start_time is None: - self._mark_start(message) + self._mark_start() + assert self.start_time interval = round(time.time() - self.start_time, self.precision) mark = dict( interval=interval, @@ -55,14 +57,14 @@ def mark(self, message=''): if self.sync: self.write() - def _mark_start(self, message): + def _mark_start(self) -> None: """ Create the initial time mark """ self.start_time = time.time() self.start_string = self.get_datetime_string(self.start_time) - def get_datetime_string(self, time): + def get_datetime_string(self, time: float) -> str: """ Return a human-readable timestamp in UTC @@ -75,7 +77,7 @@ def get_datetime_string(self, time): ) @property - def data(self): + def data(self) -> dict: """ Return an object similar to:: @@ -97,6 +99,7 @@ def data(self): end_interval = 0 else: end_interval = self.marks[-1]['interval'] + assert self.start_time end_time = self.start_time + end_interval result = dict( start=self.start_string, @@ -106,9 +109,10 @@ def data(self): ) return result - def write(self): + def write(self) -> None: try: - with open(self.path, 'w') as f: + assert self.path + with open(self.path, mode='w') as f: yaml.safe_dump(self.data, f, default_flow_style=False) except Exception: log.exception("Failed to write timing.yaml !") diff --git a/teuthology/util/flock.py b/teuthology/util/flock.py index f381d8b51b..1e761ef865 100644 --- a/teuthology/util/flock.py +++ b/teuthology/util/flock.py @@ -1,20 +1,22 @@ import fcntl +from typing import Optional +from types import TracebackType class FileLock(object): - def __init__(self, filename, noop=False): + def __init__(self, filename: str, noop: bool = False) -> None: self.filename = filename self.file = None self.noop = noop - def __enter__(self): + def __enter__(self) -> 'FileLock': if not self.noop: assert self.file is None self.file = open(self.filename, 'w') fcntl.lockf(self.file, fcntl.LOCK_EX) return self - def __exit__(self, exc_type, exc_val, exc_tb): + def __exit__(self, exc_type: Optional[type[BaseException]], exc_val: Optional[BaseException], exc_tb: Optional[TracebackType]) -> None: if not self.noop: assert self.file is not None fcntl.lockf(self.file, fcntl.LOCK_UN) diff --git a/teuthology/util/loggerfile.py b/teuthology/util/loggerfile.py index 3dd786258d..6a86ed5683 100644 --- a/teuthology/util/loggerfile.py +++ b/teuthology/util/loggerfile.py @@ -11,9 +11,9 @@ def __init__(self, logger: logging.Logger, level: int): self.logger = logger self.level = level - def write(self, string): + def write(self, string: bytes) -> None: self.logger.log(self.level, string.decode('utf-8', 'ignore')) - def flush(self): + def flush(self) -> None: pass diff --git a/teuthology/util/scanner.py b/teuthology/util/scanner.py index 421d5a028b..f269988cdc 100644 --- a/teuthology/util/scanner.py +++ b/teuthology/util/scanner.py @@ -38,7 +38,7 @@ def scan_file(self, path: str) -> Optional[str]: except Exception as exc: log.error(str(exc)) - def scan_all_files(self, path_regex: str) -> [str]: + def scan_all_files(self, path_regex: str) -> list[str]: """ Scans all files matching path_regex and collect additional data in self.summary_data @@ -101,7 +101,7 @@ def _parse(self, file_content: str) -> Tuple[Optional[str], Optional[dict]]: return exception_txt, { "failed_testsuites": dict(error_data), "num_of_failures": len(failed_testcases) } @property - def num_of_total_failures(self): + def num_of_total_failures(self) -> int: total_failed_testcases = 0 if self.summary_data: for file_data in self.summary_data: diff --git a/teuthology/util/sentry.py b/teuthology/util/sentry.py index ed767745b8..ede8e51c13 100644 --- a/teuthology/util/sentry.py +++ b/teuthology/util/sentry.py @@ -1,5 +1,6 @@ import logging import sentry_sdk +from typing import Optional from copy import deepcopy @@ -9,7 +10,7 @@ log = logging.getLogger(__name__) -def report_error(job_config, exception, task_name=None): +def report_error(job_config: dict, exception: Exception, task_name: Optional[str] = None) -> Optional[str]: if not teuth_config.sentry_dsn: return None sentry_sdk.init(teuth_config.sentry_dsn) diff --git a/teuthology/util/strtobool.py b/teuthology/util/strtobool.py index b6737e86f9..c178d0f413 100644 --- a/teuthology/util/strtobool.py +++ b/teuthology/util/strtobool.py @@ -1,4 +1,4 @@ -def strtobool(val): +def strtobool(val: str) -> int: """Convert a string representation of truth to true (1) or false (0). True values are 'y', 'yes', 't', 'true', 'on', and '1'; false values From 853502a830ccc8bd18cdd80bd2cae83bd0d36608 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Fri, 5 Jun 2026 12:25:49 -0600 Subject: [PATCH 2/2] Remove test dep. injection; use patches Signed-off-by: Zack Cerza --- tests/orchestra/test_connection.py | 8 ++--- tests/provision/test_downburst.py | 54 +++++++++++++----------------- tests/test_results.py | 17 ++++------ teuthology/orchestra/connection.py | 16 +++------ teuthology/orchestra/remote.py | 5 ++- teuthology/provision/__init__.py | 31 +++++------------ teuthology/results.py | 4 +-- 7 files changed, 49 insertions(+), 86 deletions(-) diff --git a/tests/orchestra/test_connection.py b/tests/orchestra/test_connection.py index 8051d572d4..b88d57dd8f 100644 --- a/tests/orchestra/test_connection.py +++ b/tests/orchestra/test_connection.py @@ -54,7 +54,6 @@ def test_connect(self): m_ssh_instance.get_transport.return_value = m_transport got = connection.connect( 'jdoe@orchestra.test.newdream.net.invalid', - _SSHClient=self.m_ssh, ) self.m_ssh.assert_called_once() m_ssh_instance.set_missing_host_key_policy.assert_called_once() @@ -75,7 +74,6 @@ def test_connect_no_verify_host_keys(self): m_ssh_instance.get_transport.return_value = m_transport got = connection.connect( 'jdoe@orchestra.test.newdream.net.invalid', - _SSHClient=self.m_ssh, ) self.m_ssh.assert_called_once() m_ssh_instance.set_missing_host_key_policy.assert_called_once() @@ -88,20 +86,18 @@ def test_connect_no_verify_host_keys(self): m_transport.set_keepalive.assert_called_once_with(False) assert got is m_ssh_instance - def test_connect_override_hostkeys(self): + @patch('teuthology.orchestra.connection.create_key') + def test_connect_override_hostkeys(self, m_create_key): self.clear_config() m_ssh_instance = self.m_ssh.return_value = Mock(); m_transport = Mock() m_ssh_instance.get_transport.return_value = m_transport m_host_keys = Mock() m_ssh_instance.get_host_keys.return_value = m_host_keys - m_create_key = Mock() m_create_key.return_value = "frobnitz" got = connection.connect( 'jdoe@orchestra.test.newdream.net.invalid', host_key='ssh-rsa testkey', - _SSHClient=self.m_ssh, - _create_key=m_create_key, ) self.m_ssh.assert_called_once() m_ssh_instance.get_host_keys.assert_called_once() diff --git a/tests/provision/test_downburst.py b/tests/provision/test_downburst.py index 4ba7d9a8ae..b7415784ff 100644 --- a/tests/provision/test_downburst.py +++ b/tests/provision/test_downburst.py @@ -18,68 +18,62 @@ def setup_method(self): description="desc", ) - def test_create_if_vm_success(self): + @patch('teuthology.lock.query.get_status') + @patch('teuthology.provision.downburst.Downburst') + def test_create_if_vm_success(self, m_downburst, m_get_status): name = self.name ctx = self.ctx status = self.status + m_get_status.return_value = status - dbrst = provision.downburst.Downburst( - name, ctx.os_type, ctx.os_version, status) + dbrst = MagicMock() dbrst.executable = '/fake/path' dbrst.build_config = MagicMock(name='build_config') dbrst._run_create = MagicMock(name='_run_create') dbrst._run_create.return_value = (0, '', '') + dbrst.create.return_value = True remove_config = MagicMock(name='remove_config') dbrst.remove_config = remove_config + m_downburst.return_value = dbrst - result = provision.create_if_vm(ctx, name, dbrst) + result = provision.create_if_vm(ctx, name) assert result is True - dbrst._run_create.assert_called_with() - dbrst.build_config.assert_called_with() - del dbrst - remove_config.assert_called_with() + dbrst.create.assert_called_with() - def test_destroy_if_vm_success(self): + @patch('teuthology.lock.query.get_status') + @patch('teuthology.provision.downburst.Downburst') + def test_destroy_if_vm_success(self, m_downburst, m_get_status): name = self.name - ctx = self.ctx status = self.status + m_get_status.return_value = status - dbrst = provision.downburst.Downburst( - name, ctx.os_type, ctx.os_version, status) + dbrst = MagicMock() dbrst.destroy = MagicMock(name='destroy') dbrst.destroy.return_value = True + m_downburst.return_value = dbrst - result = provision.destroy_if_vm(name, user="user@a", _downburst=dbrst) + result = provision.destroy_if_vm(name, user="user@a") assert result is True dbrst.destroy.assert_called_with() - def test_destroy_if_vm_wrong_owner(self): + @patch('teuthology.lock.query.get_status') + def test_destroy_if_vm_wrong_owner(self, m_get_status): name = self.name - ctx = self.ctx status = self.status + m_get_status.return_value = status - dbrst = provision.downburst.Downburst( - name, ctx.os_type, ctx.os_version, status) - dbrst.destroy = MagicMock(name='destroy', side_effect=RuntimeError) - - result = provision.destroy_if_vm(name, user='user@b', - _downburst=dbrst) + result = provision.destroy_if_vm(name, user='user@b') assert result is False - def test_destroy_if_vm_wrong_description(self): + @patch('teuthology.lock.query.get_status') + def test_destroy_if_vm_wrong_description(self, m_get_status): name = self.name - ctx = self.ctx status = self.status + m_get_status.return_value = status - dbrst = provision.downburst.Downburst( - name, ctx.os_type, ctx.os_version, status) - dbrst.destroy = MagicMock(name='destroy') - dbrst.destroy = MagicMock(name='destroy', side_effect=RuntimeError) - - result = provision.destroy_if_vm(name, description='desc_b', - _downburst=dbrst) + result = provision.destroy_if_vm(name, description='desc_b') assert result is False @patch('teuthology.provision.downburst.downburst_executable') diff --git a/tests/test_results.py b/tests/test_results.py index 3d7ba93bdc..7fee5c4f51 100644 --- a/tests/test_results.py +++ b/tests/test_results.py @@ -1,9 +1,8 @@ import textwrap from teuthology.config import config from teuthology import results -from teuthology import report -from unittest.mock import patch, DEFAULT +from unittest.mock import patch class TestResultsEmail(object): @@ -141,15 +140,11 @@ def setup_method(self): config.results_ui_server = "http://example.com/" config.archive_server = "http://qa-proxy.ceph.com/teuthology/" - def test_build_email_body(self): + @patch('teuthology.results.ResultsReporter') + def test_build_email_body(self, m_reporter): run_name = self.reference['run_name'] - with patch.multiple( - report, - ResultsReporter=DEFAULT, - ): - reporter = report.ResultsReporter() - reporter.get_jobs.return_value = self.reference['jobs'] - (subject, body) = results.build_email_body( - run_name, _reporter=reporter) + reporter = m_reporter.return_value + reporter.get_jobs.return_value = self.reference['jobs'] + (subject, body) = results.build_email_body(run_name) assert subject == self.reference['subject'] assert body == self.reference['body'] diff --git a/teuthology/orchestra/connection.py b/teuthology/orchestra/connection.py index ece8b4202a..28d04eae81 100644 --- a/teuthology/orchestra/connection.py +++ b/teuthology/orchestra/connection.py @@ -4,7 +4,7 @@ import paramiko import os import logging -from typing import Callable, Optional, Tuple, Union, List +from typing import Optional, Tuple, Union, List from teuthology.config import config from teuthology.contextutil import safe_while @@ -39,8 +39,7 @@ def create_key(keytype: str, key: str) -> PKey: def connect(user_at_host: str, host_key: Optional[str] = None, keep_alive: bool = False, - timeout: int = 60, _SSHClient: Optional = None, - _create_key: Optional[Callable[[str, str], PKey]] = None, + timeout: int = 60, retry: bool = True, key_filename: Optional[Union[str, List[str]]] = None) -> paramiko.SSHClient: """ ssh connection routine. @@ -49,8 +48,6 @@ def connect(user_at_host: str, host_key: Optional[str] = None, keep_alive: bool :param host_key: ssh key :param keep_alive: keep_alive indicator :param timeout: timeout in seconds - :param _SSHClient: client, default is paramiko ssh client - :param _create_key: routine to create a key (defaults to local reate_key) :param retry: Whether or not to retry failed connection attempts (eventually giving up if none succeed). Default is True :param key_filename: Optionally override which private key to use. @@ -59,12 +56,7 @@ def connect(user_at_host: str, host_key: Optional[str] = None, keep_alive: bool if timeout is None: timeout = 60 user, host = split_user(user_at_host) - if _SSHClient is None: - _SSHClient = paramiko.SSHClient - ssh = _SSHClient() - - if _create_key is None: - _create_key = create_key + ssh = paramiko.SSHClient() if host_key is None: ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) @@ -76,7 +68,7 @@ def connect(user_at_host: str, host_key: Optional[str] = None, keep_alive: bool ssh.get_host_keys().add( hostname=host, keytype=keytype, - key=_create_key(keytype, key) + key=create_key(keytype, key) ) connect_args: dict[str, Union[str, None, int, list[str]]] = dict( diff --git a/teuthology/orchestra/remote.py b/teuthology/orchestra/remote.py index b3cfa3e694..db88314341 100644 --- a/teuthology/orchestra/remote.py +++ b/teuthology/orchestra/remote.py @@ -401,9 +401,8 @@ def connect(self, timeout=None, create_key=None, context='connect'): # clear from the technical side. # I'll get "[Errno 98] Address already in use" altough # there are no open tcp(ssh) connections. - # When connecting without keepalive, host_key and _create_key - # set, it will proceed. - args = dict(user_at_host=self.name, _create_key=False, host_key=None) + # When connecting without keepalive and host_key set, it will proceed. + args = dict(user_at_host=self.name, host_key=None) if timeout: args['timeout'] = timeout diff --git a/teuthology/provision/__init__.py b/teuthology/provision/__init__.py index 4b3ad97e6f..9f6a977ff2 100644 --- a/teuthology/provision/__init__.py +++ b/teuthology/provision/__init__.py @@ -62,16 +62,11 @@ def reimage(ctx, machine_name: str, machine_type: str): return result -def create_if_vm(ctx, machine_name: str, _downburst: Optional = None) -> bool: +def create_if_vm(ctx, machine_name: str) -> bool: """ Use downburst to create a virtual machine - - :param _downburst: Only used for unit testing. """ - if _downburst: - status_info = _downburst.status - else: - status_info = teuthology.lock.query.get_status(machine_name) + status_info = teuthology.lock.query.get_status(machine_name) shortname = decanonicalize_hostname(machine_name) machine_type = status_info['machine_type'] os_type = get_distro(ctx) @@ -94,10 +89,9 @@ def create_if_vm(ctx, machine_name: str, _downburst: Optional = None) -> bool: 'Usage of a custom downburst config has been deprecated.' ) - dbrst = _downburst or \ - downburst.Downburst(name=machine_name, os_type=os_type, - os_version=os_version, status=status_info, - logfile=_logfile(ctx, shortname)) + dbrst = downburst.Downburst(name=machine_name, os_type=os_type, + os_version=os_version, status=status_info, + logfile=_logfile(ctx, shortname)) result = dbrst.create() return bool(result) @@ -106,19 +100,13 @@ def destroy_if_vm( machine_name: str, user: str = "", description: str = "", - _downburst: Optional = None ) -> bool: """ Use downburst to destroy a virtual machine Return False only on vm downburst failures. - - :param _downburst: Only used for unit testing. """ - if _downburst: - status_info = _downburst.status - else: - status_info = teuthology.lock.query.get_status(machine_name) + status_info = teuthology.lock.query.get_status(machine_name) if not status_info or not teuthology.lock.query.is_vm(status=status_info): return True if user is not None and user != status_info['locked_by']: @@ -142,8 +130,7 @@ def destroy_if_vm( return cloud.get_provisioner( machine_type, shortname, None, None).destroy() - dbrst = _downburst or \ - downburst.Downburst(name=machine_name, os_type=None, - os_version=None, status=status_info, - logfile=_logfile(description, shortname)) + dbrst = downburst.Downburst(name=machine_name, os_type=None, + os_version=None, status=status_info, + logfile=_logfile(description, shortname)) return dbrst.destroy() diff --git a/teuthology/results.py b/teuthology/results.py index aae991eaf1..7852229b1e 100644 --- a/teuthology/results.py +++ b/teuthology/results.py @@ -97,7 +97,7 @@ def email_results(subject, from_, to, body): smtp.quit() -def build_email_body(name, _reporter=None): +def build_email_body(name): stanzas = OrderedDict([ ('fail', dict()), ('dead', dict()), @@ -106,7 +106,7 @@ def build_email_body(name, _reporter=None): ('queued', dict()), ('pass', dict()), ]) - reporter = _reporter or ResultsReporter() + reporter = ResultsReporter() fields = ('job_id', 'status', 'description', 'duration', 'failure_reason', 'sentry_event', 'log_href') jobs = reporter.get_jobs(name, fields=fields)