Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get the 'tests' task working again #1975

Merged
merged 11 commits into from
Jul 23, 2024
25 changes: 11 additions & 14 deletions teuthology/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@
from sys import stdin
import pprint
import datetime
from types import MappingProxyType

from tarfile import ReadError

from typing import Optional
from typing import Optional, TypeVar

from teuthology.util.compat import urljoin, urlopen, HTTPError

Expand Down Expand Up @@ -110,7 +109,7 @@ def config_file(string):
return config_dict


def merge_configs(config_paths):
def merge_configs(config_paths) -> dict:
""" Takes one or many paths to yaml config files and merges them
together, returning the result.
"""
Expand All @@ -123,7 +122,7 @@ def merge_configs(config_paths):
continue
else:
with open(conf_path) as partial_file:
partial_dict = yaml.safe_load(partial_file)
partial_dict: dict = yaml.safe_load(partial_file)
try:
conf_dict = deep_merge(conf_dict, partial_dict)
except Exception:
Expand Down Expand Up @@ -986,7 +985,8 @@ def replace_all_with_clients(cluster, config):
return norm_config


def deep_merge(a, b):
DeepMerge = TypeVar('DeepMerge')
def deep_merge(a: DeepMerge, b: DeepMerge) -> DeepMerge:
zmc marked this conversation as resolved.
Show resolved Hide resolved
"""
Deep Merge. If a and b are both lists, all elements in b are
added into a. If a and b are both dictionaries, elements in b are
Expand All @@ -996,21 +996,18 @@ def deep_merge(a, b):
"""
if b is None:
return a
elif isinstance(a, list):
if a is None:
return deep_merge(b.__class__(), b)
if isinstance(a, list):
assert isinstance(b, list)
a.extend(b)
return a
elif isinstance(a, dict):
assert isinstance(b, dict) or isinstance(b, MappingProxyType)
if isinstance(a, dict):
assert isinstance(b, dict)
for (k, v) in b.items():
a[k] = deep_merge(a.get(k), v)
return a
elif isinstance(b, dict) or isinstance(b, list):
return deep_merge(b.__class__(), b)
elif isinstance(b, MappingProxyType):
return deep_merge(dict(), b)
else:
return b
return b


def get_valgrind_args(testdir, name, preamble, v, exit_on_first_error=True):
Expand Down
15 changes: 9 additions & 6 deletions teuthology/provision/test/test_fog.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@
from teuthology.provision import fog


test_config = dict(fog=dict(
endpoint='http://fog.example.com/fog',
api_token='API_TOKEN',
user_token='USER_TOKEN',
machine_types='type1,type2',
))
test_config = dict(
fog=dict(
endpoint='http://fog.example.com/fog',
api_token='API_TOKEN',
user_token='USER_TOKEN',
machine_types='type1,type2',
),
fog_reimage_timeout=1800,
)


class TestFOG(object):
Expand Down
14 changes: 8 additions & 6 deletions teuthology/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def fetch_tasks_if_needed(job_config):
return suite_path


def setup_config(config_paths):
def setup_config(config_paths) -> dict:
"""
Takes a list of config yaml files and combines them
into a single dictionary. Processes / validates the dictionary and then
Expand Down Expand Up @@ -260,7 +260,7 @@ def get_initial_tasks(lock, config, machine_type):
return init_tasks


def report_outcome(config, archive, summary, fake_ctx):
def report_outcome(config, archive, summary):
""" Reports on the final outcome of the command. """
status = get_status(summary)
passed = status == 'pass'
Expand Down Expand Up @@ -326,8 +326,6 @@ def main(args):
os_version = args["--os-version"]
interactive_on_error = args["--interactive-on-error"]

set_up_logging(verbose, archive)

# print the command being ran
log.debug("Teuthology command: {0}".format(get_teuthology_command(args)))

Expand All @@ -338,6 +336,10 @@ def main(args):

if archive is not None and 'archive_path' not in config:
config['archive_path'] = archive
elif archive is None and 'archive_path' in config:
archive = args['--archive'] = config['archive_path']

set_up_logging(verbose, archive)

write_initial_metadata(archive, config, name, description, owner)
report.try_push_job_info(config, dict(status='running'))
Expand Down Expand Up @@ -400,10 +402,10 @@ def main(args):
# FIXME this should become more generic, and the keys should use
# '_' uniformly
if fake_ctx.config.get('interactive-on-error'):
teuthology.config.config.ctx = fake_ctx
teuth_config.config.ctx = fake_ctx

try:
run_tasks(tasks=config['tasks'], ctx=fake_ctx)
finally:
# print to stdout the results and possibly send an email on any errors
report_outcome(config, archive, fake_ctx.summary, fake_ctx)
report_outcome(config, archive, fake_ctx.summary)
5 changes: 2 additions & 3 deletions teuthology/suite/merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
TEUTHOLOGY_TEMPLATE = MappingProxyType({
zmc marked this conversation as resolved.
Show resolved Hide resolved
"teuthology": {
"fragments_dropped": [],
"meta": MappingProxyType({}),
"meta": {},
"postmerge": [],
}
})
Expand Down Expand Up @@ -114,7 +114,6 @@ def config_merge(configs, suite_name=None, **kwargs):
postmerge scripts. Logically, if a filter matches then reject will drop
the entire job (config) from the list.
"""

seed = kwargs.setdefault('seed', 1)
if not isinstance(seed, int):
log.debug("no valid seed input: using 1")
Expand All @@ -130,7 +129,7 @@ def config_merge(configs, suite_name=None, **kwargs):
desc = combine_path(suite_name, desc)

yaml_complete_obj = {}
deep_merge(yaml_complete_obj, TEUTHOLOGY_TEMPLATE)
deep_merge(yaml_complete_obj, dict(TEUTHOLOGY_TEMPLATE))
for path in paths:
if path not in yaml_cache:
with open(path) as f:
Expand Down
2 changes: 1 addition & 1 deletion teuthology/task/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class MySubtask(MyTask):
name = 'mytask.mysubtask'
"""

def __init__(self, ctx=None, config=None):
def __init__(self, ctx, config=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious - why did we remove the default param value of "None" here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! This was to fix the pyright error reportOptionalMemberAccess resulting from things like references to self.ctx.archive in teuthology.task.Ansible. The Task class was never really intended to be used without the ctx.

Copy link
Member Author

@zmc zmc Jul 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error messages like 'foo' is not a known member of 'None'

Copy link
Member

@VallariAg VallariAg Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh I see, thank you for the explanation! :)

if not hasattr(self, 'name'):
self.name = self.__class__.__name__.lower()
self.log = log
Expand Down
20 changes: 1 addition & 19 deletions teuthology/task/ansible.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,12 @@
from teuthology.config import config as teuth_config
from teuthology.exceptions import CommandFailedError, AnsibleFailedError
from teuthology.job_status import set_status

from teuthology.task import Task
from teuthology.util.loggerfile import LoggerFile

log = logging.getLogger(__name__)


class LoggerFile(object):
"""
A thin wrapper around a logging.Logger instance that provides a file-like
interface.

Used by Ansible.execute_playbook() when it calls pexpect.run()
"""
def __init__(self, logger, level):
self.logger = logger
self.level = level

def write(self, string):
self.logger.log(self.level, string.decode('utf-8', 'ignore'))

def flush(self):
pass


class FailureAnalyzer:
def analyze(self, failure_log):
failure_obj = yaml.safe_load(failure_log)
Expand Down
3 changes: 2 additions & 1 deletion teuthology/task/cephmetrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@

from teuthology.config import config as teuth_config
from teuthology.exceptions import CommandFailedError
from teuthology.task.ansible import Ansible
from teuthology.util.loggerfile import LoggerFile

from teuthology.task.ansible import Ansible, LoggerFile

log = logging.getLogger(__name__)

Expand Down
Loading
Loading