From f4028dd8620f06c1526f792cde6b5095cc176119 Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Thu, 20 Mar 2025 18:00:08 +0100 Subject: [PATCH 01/13] wip: add some behavior tests --- ceph_devstack/resources/ceph/__init__.py | 5 + ceph_devstack/resources/test/test_devstack.py | 138 ++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 ceph_devstack/resources/test/test_devstack.py diff --git a/ceph_devstack/resources/ceph/__init__.py b/ceph_devstack/resources/ceph/__init__.py index 344cc16a6..1260e3b3b 100644 --- a/ceph_devstack/resources/ceph/__init__.py +++ b/ceph_devstack/resources/ceph/__init__.py @@ -226,3 +226,8 @@ async def wait(self, container_name: str): return await object.wait() logger.error(f"Could not find container {container_name}") return 1 + + async def logs( + self, run_name: str = None, job_id: str = None, locate: bool = False + ): + pass diff --git a/ceph_devstack/resources/test/test_devstack.py b/ceph_devstack/resources/test/test_devstack.py new file mode 100644 index 000000000..ce3055deb --- /dev/null +++ b/ceph_devstack/resources/test/test_devstack.py @@ -0,0 +1,138 @@ +import os +import io +import contextlib +import tempfile +import random as rd +from datetime import datetime, timedelta + +from ceph_devstack import config +from ceph_devstack.resources.ceph import CephDevStack + + +class TestDevStack: + # ceph-devstack logs : returns teuthology.log of latest run + # ceph-devstack logs : display message to pick job id if there is more than one job + # ceph-devstack logs -j : display content of given job + # ceph-devstack logs -r : display content on provided run name + # cepth-devstack logs --locate : show filepath instead one its content + async def test_logs_command_display_log_file_of_latest_run(self): + with tempfile.TemporaryDirectory() as data_dir: + config["data_dir"] = data_dir + f = io.StringIO() + content = "custom log content" + now = datetime.now().strftime("%Y-%m-%dT%H-%M-%S") + forty_days_ago = (datetime.now() - timedelta(days=40)).strftime( + "%Y-%m-%dT%H-%M-%S" + ) + + self.create_logfile(data_dir, timestamp=now, content=content) + self.create_logfile(data_dir, timestamp=forty_days_ago) + + with contextlib.redirect_stdout(f): + devstack = CephDevStack() + await devstack.logs() + assert content in f.getvalue() + + async def test_logs_command_display_message_to_pick_job_id_if_more_than_one_job( + self, + ): + with tempfile.TemporaryDirectory() as data_dir: + config["data_dir"] = data_dir + f = io.StringIO() + content = "log message" + now = datetime.now().strftime("%Y-%m-%dT%H-%M-%S") + + self.create_logfile( + data_dir, timestamp=now, test_type="ceph", job_id="1", content=content + ) + self.create_logfile( + data_dir, timestamp=now, test_type="ceph", job_id="2", content=content + ) + + with contextlib.redirect_stdout(f): + devstack = CephDevStack() + await devstack.logs() + assert "please pick a job id" in f.getvalue() + + async def test_logs_command_display_log_file_of_given_job_id(self): + with tempfile.TemporaryDirectory() as data_dir: + config["data_dir"] = data_dir + f = io.StringIO() + content = "custom log message" + now = datetime.now().strftime("%Y-%m-%dT%H-%M-%S") + + self.create_logfile( + data_dir, + timestamp=now, + test_type="ceph", + job_id="1", + content="another log", + ) + self.create_logfile( + data_dir, timestamp=now, test_type="ceph", job_id="2", content=content + ) + + with contextlib.redirect_stdout(f): + devstack = CephDevStack() + await devstack.logs(job_id="2") + assert content in f.getvalue() + + async def test_logs_display_content_of_provided_run_name(self): + with tempfile.TemporaryDirectory() as data_dir: + config["data_dir"] = data_dir + f = io.StringIO() + content = "custom content" + now = datetime.now().strftime("%Y-%m-%dT%H-%M-%S") + three_days_ago = (datetime.now() - timedelta(days=3)).strftime( + "%Y-%m-%dT%H-%M-%S" + ) + + self.create_logfile( + data_dir, + timestamp=now, + ) + run_name = self.create_logfile( + data_dir, + timestamp=three_days_ago, + content=content, + ) + + with contextlib.redirect_stdout(f): + devstack = CephDevStack() + await devstack.logs(run_name=run_name) + assert content in f.getvalue() + + async def test_logs_locate_display_file_path_instead_of_config(self): + with tempfile.TemporaryDirectory() as data_dir: + config["data_dir"] = data_dir + f = io.StringIO() + + logfile = self.create_logfile(data_dir) + with contextlib.redirect_stdout(f): + devstack = CephDevStack() + await devstack.logs(locate=True) + assert logfile in f.getvalue() + + def create_logfile(self, data_dir: str, **kwargs): + parts = { + "timestamp": (datetime.now() - timedelta(days=rd.randint(1, 100))).strftime( + "%Y-%m-%dT%H-%M-%S" + ), + "test_type": rd.choice(["ceph", "rgw", "rbd", "mds"]), + "job_id": rd.randint(1, 100), + "content": "some log data", + **kwargs, + } + timestamp = parts["timestamp"] + test_type = parts["test_type"] + job_id = parts["job_id"] + content = parts["content"] + + run_name = f"{timestamp}_{test_type}_master_{job_id}" + log_dir = f"{data_dir}/{run_name}/{job_id}" + + os.makedirs(log_dir, exist_ok=True) + log_file = f"{log_dir}/teuthology.log" + with open(log_file, "w") as f: + f.write(content) + return log_file From 00570f06739f5aeb2b01a6b645033eac546f8403 Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Fri, 21 Mar 2025 00:04:17 +0100 Subject: [PATCH 02/13] separate concerns to add more tests --- ceph_devstack/resources/ceph/__init__.py | 30 ++++++- ceph_devstack/resources/ceph/exceptions.py | 2 + ceph_devstack/resources/ceph/utils.py | 44 ++++++++++ ceph_devstack/resources/test/test_devstack.py | 84 +++++++++++-------- 4 files changed, 123 insertions(+), 37 deletions(-) create mode 100644 ceph_devstack/resources/ceph/exceptions.py create mode 100644 ceph_devstack/resources/ceph/utils.py diff --git a/ceph_devstack/resources/ceph/__init__.py b/ceph_devstack/resources/ceph/__init__.py index 1260e3b3b..2e81a2ec4 100644 --- a/ceph_devstack/resources/ceph/__init__.py +++ b/ceph_devstack/resources/ceph/__init__.py @@ -23,6 +23,8 @@ LoopControlDeviceWriteable, SELinuxModule, ) +from ceph_devstack.resources.ceph.utils import get_most_recent_run, get_job_id +from ceph_devstack.resources.ceph.exceptions import TooManyJobsFound class SSHKeyPair(Secret): @@ -230,4 +232,30 @@ async def wait(self, container_name: str): async def logs( self, run_name: str = None, job_id: str = None, locate: bool = False ): - pass + try: + log_file = self.get_log_file(run_name, job_id) + except FileNotFoundError: + logger.error("No log file found") + except TooManyJobsFound: + logger.error("Found too many jobs for provided. Please pick a job id") + else: + if locate: + print(log_file) + else: + with open(log_file) as f: + print(f.read()) + + def get_log_file(self, run_name: str = None, job_id: str = None): + archive_dir = Teuthology().archive_dir.expanduser() + + if not run_name: + run_name = get_most_recent_run(os.listdir(archive_dir)) + run_dir = os.path.join(archive_dir, run_name) + + if not job_id: + job_id = get_job_id(os.listdir(run_dir)) + + log_file = os.path.join(run_dir, job_id, "teuthology.log") + if not os.path.exists(log_file): + raise FileNotFoundError + return log_file diff --git a/ceph_devstack/resources/ceph/exceptions.py b/ceph_devstack/resources/ceph/exceptions.py new file mode 100644 index 000000000..35fccb76f --- /dev/null +++ b/ceph_devstack/resources/ceph/exceptions.py @@ -0,0 +1,2 @@ +class TooManyJobsFound(Exception): + pass diff --git a/ceph_devstack/resources/ceph/utils.py b/ceph_devstack/resources/ceph/utils.py new file mode 100644 index 000000000..7ddcb5b66 --- /dev/null +++ b/ceph_devstack/resources/ceph/utils.py @@ -0,0 +1,44 @@ +import re +from datetime import datetime + +from ceph_devstack.resources.ceph.exceptions import TooManyJobsFound + +RUN_DIRNAME_PATTERN = re.compile( + r"^(?P^[a-z_]([a-z0-9_-]{0,31}|[a-z0-9_-]{0,30}))-(?P\d{4}-\d{2}-\d{2}_\d{2}:\d{2}:\d{2})" +) + + +def get_logtimestamp(dirname: str) -> datetime: + match_ = RUN_DIRNAME_PATTERN.search(dirname) + return datetime.strptime(match_.group("timestamp"), "%Y-%m-%d_%H:%M:%S") + + +def get_most_recent_run(runs: list[str]) -> str: + try: + run_name = next( + iter( + sorted( + ( + dirname + for dirname in runs + if RUN_DIRNAME_PATTERN.search(dirname) + ), + key=lambda dirname: get_logtimestamp(dirname), + reverse=True, + ) + ) + ) + return run_name + except StopIteration: + raise FileNotFoundError + + +def get_job_id(jobs: list[str]): + job_dir_pattern = re.compile(r"^\d+$") + dirs = [d for d in jobs if job_dir_pattern.match(d)] + + if len(dirs) == 0: + raise FileNotFoundError + elif len(dirs) > 1: + raise TooManyJobsFound + return dirs[0] diff --git a/ceph_devstack/resources/test/test_devstack.py b/ceph_devstack/resources/test/test_devstack.py index ce3055deb..82428443b 100644 --- a/ceph_devstack/resources/test/test_devstack.py +++ b/ceph_devstack/resources/test/test_devstack.py @@ -4,25 +4,57 @@ import tempfile import random as rd from datetime import datetime, timedelta +import unittest from ceph_devstack import config +from ceph_devstack.resources.ceph.utils import ( + get_logtimestamp, + get_most_recent_run, + get_job_id, +) +from ceph_devstack.resources.ceph.exceptions import TooManyJobsFound from ceph_devstack.resources.ceph import CephDevStack -class TestDevStack: - # ceph-devstack logs : returns teuthology.log of latest run - # ceph-devstack logs : display message to pick job id if there is more than one job - # ceph-devstack logs -j : display content of given job - # ceph-devstack logs -r : display content on provided run name - # cepth-devstack logs --locate : show filepath instead one its content +class TestDevStack(unittest.IsolatedAsyncioTestCase): + def test_get_logtimestamp(self): + dirname = "root-2025-03-20_18:34:43-orch:cephadm:smoke-small-main-distro-default-testnode" + assert get_logtimestamp(dirname) == datetime(2025, 3, 20, 18, 34, 43) + + def test_get_most_recent_run_returns_most_recent_run(self): + runs = [ + "root-2024-02-07_12:23:43-orch:cephadm:smoke-small-devlop-distro-smithi-testnode", + "root-2025-02-20_11:23:43-orch:cephadm:smoke-small-devlop-distro-smithi-testnode", + "root-2025-03-20_18:34:43-orch:cephadm:smoke-small-main-distro-default-testnode", + "root-2025-01-18_18:34:43-orch:cephadm:smoke-small-main-distro-default-testnode", + ] + assert ( + get_most_recent_run(runs) + == "root-2025-03-20_18:34:43-orch:cephadm:smoke-small-main-distro-default-testnode" + ) + + def test_get_job_id_returns_job_on_unique_job(self): + jobs = ["97"] + assert get_job_id(jobs) == "97" + + def test_get_job_id_throws_filenotfound_on_missing_job(self): + jobs = [] + with self.assertRaises(FileNotFoundError): + get_job_id(jobs) + + def test_get_job_id_throws_toomanyjobsfound_on_more_than_one_job(self): + jobs = ["1", "2"] + with self.assertRaises(TooManyJobsFound): + get_job_id(jobs) + async def test_logs_command_display_log_file_of_latest_run(self): with tempfile.TemporaryDirectory() as data_dir: config["data_dir"] = data_dir f = io.StringIO() content = "custom log content" - now = datetime.now().strftime("%Y-%m-%dT%H-%M-%S") + now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") forty_days_ago = (datetime.now() - timedelta(days=40)).strftime( - "%Y-%m-%dT%H-%M-%S" + "%Y-%m-%d_%H:%M:%S" ) self.create_logfile(data_dir, timestamp=now, content=content) @@ -33,33 +65,12 @@ async def test_logs_command_display_log_file_of_latest_run(self): await devstack.logs() assert content in f.getvalue() - async def test_logs_command_display_message_to_pick_job_id_if_more_than_one_job( - self, - ): - with tempfile.TemporaryDirectory() as data_dir: - config["data_dir"] = data_dir - f = io.StringIO() - content = "log message" - now = datetime.now().strftime("%Y-%m-%dT%H-%M-%S") - - self.create_logfile( - data_dir, timestamp=now, test_type="ceph", job_id="1", content=content - ) - self.create_logfile( - data_dir, timestamp=now, test_type="ceph", job_id="2", content=content - ) - - with contextlib.redirect_stdout(f): - devstack = CephDevStack() - await devstack.logs() - assert "please pick a job id" in f.getvalue() - async def test_logs_command_display_log_file_of_given_job_id(self): with tempfile.TemporaryDirectory() as data_dir: config["data_dir"] = data_dir f = io.StringIO() content = "custom log message" - now = datetime.now().strftime("%Y-%m-%dT%H-%M-%S") + now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") self.create_logfile( data_dir, @@ -82,9 +93,9 @@ async def test_logs_display_content_of_provided_run_name(self): config["data_dir"] = data_dir f = io.StringIO() content = "custom content" - now = datetime.now().strftime("%Y-%m-%dT%H-%M-%S") + now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") three_days_ago = (datetime.now() - timedelta(days=3)).strftime( - "%Y-%m-%dT%H-%M-%S" + "%Y-%m-%d_%H:%M:%S" ) self.create_logfile( @@ -95,7 +106,7 @@ async def test_logs_display_content_of_provided_run_name(self): data_dir, timestamp=three_days_ago, content=content, - ) + ).split("/")[-3] with contextlib.redirect_stdout(f): devstack = CephDevStack() @@ -116,7 +127,7 @@ async def test_logs_locate_display_file_path_instead_of_config(self): def create_logfile(self, data_dir: str, **kwargs): parts = { "timestamp": (datetime.now() - timedelta(days=rd.randint(1, 100))).strftime( - "%Y-%m-%dT%H-%M-%S" + "%Y-%m-%d_%H:%M:%S" ), "test_type": rd.choice(["ceph", "rgw", "rbd", "mds"]), "job_id": rd.randint(1, 100), @@ -128,11 +139,12 @@ def create_logfile(self, data_dir: str, **kwargs): job_id = parts["job_id"] content = parts["content"] - run_name = f"{timestamp}_{test_type}_master_{job_id}" - log_dir = f"{data_dir}/{run_name}/{job_id}" + run_name = f"root-{timestamp}-orch:cephadm:{test_type}-small-main-distro-default-testnode" + log_dir = f"{data_dir}/archive/{run_name}/{job_id}" os.makedirs(log_dir, exist_ok=True) log_file = f"{log_dir}/teuthology.log" with open(log_file, "w") as f: f.write(content) + print(log_file) return log_file From 2726844d7f6a878b9e91c65a7be010b2d5ffedf5 Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Fri, 21 Mar 2025 00:16:35 +0100 Subject: [PATCH 03/13] fix conflicts with rebase C Add 'logs' command to CLI --- ceph_devstack/__init__.py | 4 ++++ ceph_devstack/cli.py | 4 ++++ ceph_devstack/resources/test/test_devstack.py | 9 +++++---- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/ceph_devstack/__init__.py b/ceph_devstack/__init__.py index fc0cc8d94..2ed3ffba5 100644 --- a/ceph_devstack/__init__.py +++ b/ceph_devstack/__init__.py @@ -104,6 +104,10 @@ def parse_args(args: List[str]) -> argparse.Namespace: "container", help="The container to wait for", ) + parser_log = subparsers.add_parser("logs", help="Dump teuthology logs") + parser_log.add_argument("--run-name", type=str, default=None) + parser_log.add_argument("--job-id", type=str, default=None) + parser_log.add_argument("--locate", action=argparse.BooleanOptionalAction) return parser.parse_args(args) diff --git a/ceph_devstack/cli.py b/ceph_devstack/cli.py index ccb02ef83..46c0ef58f 100644 --- a/ceph_devstack/cli.py +++ b/ceph_devstack/cli.py @@ -40,6 +40,10 @@ async def run(): return elif args.command == "wait": return await obj.wait(container_name=args.container) + elif args.command == "logs": + return await obj.logs( + run_name=args.run_name, job_id=args.job_id, locate=args.locate + ) else: await obj.apply(args.command) return 0 diff --git a/ceph_devstack/resources/test/test_devstack.py b/ceph_devstack/resources/test/test_devstack.py index 82428443b..280781b41 100644 --- a/ceph_devstack/resources/test/test_devstack.py +++ b/ceph_devstack/resources/test/test_devstack.py @@ -4,7 +4,8 @@ import tempfile import random as rd from datetime import datetime, timedelta -import unittest + +import pytest from ceph_devstack import config from ceph_devstack.resources.ceph.utils import ( @@ -16,7 +17,7 @@ from ceph_devstack.resources.ceph import CephDevStack -class TestDevStack(unittest.IsolatedAsyncioTestCase): +class TestDevStack: def test_get_logtimestamp(self): dirname = "root-2025-03-20_18:34:43-orch:cephadm:smoke-small-main-distro-default-testnode" assert get_logtimestamp(dirname) == datetime(2025, 3, 20, 18, 34, 43) @@ -39,12 +40,12 @@ def test_get_job_id_returns_job_on_unique_job(self): def test_get_job_id_throws_filenotfound_on_missing_job(self): jobs = [] - with self.assertRaises(FileNotFoundError): + with pytest.raises(FileNotFoundError): get_job_id(jobs) def test_get_job_id_throws_toomanyjobsfound_on_more_than_one_job(self): jobs = ["1", "2"] - with self.assertRaises(TooManyJobsFound): + with pytest.raises(TooManyJobsFound): get_job_id(jobs) async def test_logs_command_display_log_file_of_latest_run(self): From 182ece32f32af4ec4a370a0c173c843123dfb869 Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Fri, 21 Mar 2025 00:18:51 +0100 Subject: [PATCH 04/13] add short flag for parser args --- ceph_devstack/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ceph_devstack/__init__.py b/ceph_devstack/__init__.py index 2ed3ffba5..9667cacc4 100644 --- a/ceph_devstack/__init__.py +++ b/ceph_devstack/__init__.py @@ -105,8 +105,8 @@ def parse_args(args: List[str]) -> argparse.Namespace: help="The container to wait for", ) parser_log = subparsers.add_parser("logs", help="Dump teuthology logs") - parser_log.add_argument("--run-name", type=str, default=None) - parser_log.add_argument("--job-id", type=str, default=None) + parser_log.add_argument("-r", "--run-name", type=str, default=None) + parser_log.add_argument("-j", "--job-id", type=str, default=None) parser_log.add_argument("--locate", action=argparse.BooleanOptionalAction) return parser.parse_args(args) From 4c44e5fba7b012a65a672d9ef49e78286357ec6e Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Fri, 21 Mar 2025 00:30:51 +0100 Subject: [PATCH 05/13] add small improvements --- ceph_devstack/resources/ceph/__init__.py | 8 ++++++-- ceph_devstack/resources/test/test_devstack.py | 1 - 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ceph_devstack/resources/ceph/__init__.py b/ceph_devstack/resources/ceph/__init__.py index 2e81a2ec4..6adce9265 100644 --- a/ceph_devstack/resources/ceph/__init__.py +++ b/ceph_devstack/resources/ceph/__init__.py @@ -237,13 +237,17 @@ async def logs( except FileNotFoundError: logger.error("No log file found") except TooManyJobsFound: - logger.error("Found too many jobs for provided. Please pick a job id") + logger.error( + "Found too many jobs for target run. Please pick a job id with -j flag." + ) else: if locate: print(log_file) else: + buffer_size = 8 * 1024 with open(log_file) as f: - print(f.read()) + while chunk := f.read(buffer_size): + print(chunk) def get_log_file(self, run_name: str = None, job_id: str = None): archive_dir = Teuthology().archive_dir.expanduser() diff --git a/ceph_devstack/resources/test/test_devstack.py b/ceph_devstack/resources/test/test_devstack.py index 280781b41..281c997b4 100644 --- a/ceph_devstack/resources/test/test_devstack.py +++ b/ceph_devstack/resources/test/test_devstack.py @@ -147,5 +147,4 @@ def create_logfile(self, data_dir: str, **kwargs): log_file = f"{log_dir}/teuthology.log" with open(log_file, "w") as f: f.write(content) - print(log_file) return log_file From 4a7b2a38c1fe6fe87f994f3b82b5a861b61d0078 Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Wed, 16 Apr 2025 20:15:56 +0100 Subject: [PATCH 06/13] wip: fix break line display in logs dump --- ceph_devstack/__init__.py | 6 +++++- ceph_devstack/resources/ceph/__init__.py | 2 +- ceph_devstack/resources/test/test_devstack.py | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/ceph_devstack/__init__.py b/ceph_devstack/__init__.py index 9667cacc4..e32b171e2 100644 --- a/ceph_devstack/__init__.py +++ b/ceph_devstack/__init__.py @@ -107,7 +107,11 @@ def parse_args(args: List[str]) -> argparse.Namespace: parser_log = subparsers.add_parser("logs", help="Dump teuthology logs") parser_log.add_argument("-r", "--run-name", type=str, default=None) parser_log.add_argument("-j", "--job-id", type=str, default=None) - parser_log.add_argument("--locate", action=argparse.BooleanOptionalAction) + parser_log.add_argument( + "--locate", + action=argparse.BooleanOptionalAction, + help="Display log file path instead of contents", + ) return parser.parse_args(args) diff --git a/ceph_devstack/resources/ceph/__init__.py b/ceph_devstack/resources/ceph/__init__.py index 6adce9265..8955b00e4 100644 --- a/ceph_devstack/resources/ceph/__init__.py +++ b/ceph_devstack/resources/ceph/__init__.py @@ -247,7 +247,7 @@ async def logs( buffer_size = 8 * 1024 with open(log_file) as f: while chunk := f.read(buffer_size): - print(chunk) + print(chunk, end="") def get_log_file(self, run_name: str = None, job_id: str = None): archive_dir = Teuthology().archive_dir.expanduser() diff --git a/ceph_devstack/resources/test/test_devstack.py b/ceph_devstack/resources/test/test_devstack.py index 281c997b4..742120b30 100644 --- a/ceph_devstack/resources/test/test_devstack.py +++ b/ceph_devstack/resources/test/test_devstack.py @@ -4,6 +4,8 @@ import tempfile import random as rd from datetime import datetime, timedelta +import secrets +import string import pytest @@ -66,6 +68,22 @@ async def test_logs_command_display_log_file_of_latest_run(self): await devstack.logs() assert content in f.getvalue() + async def test_logs_display_roughly_contents_of_log_file(self): + with tempfile.TemporaryDirectory() as data_dir: + config["data_dir"] = data_dir + f = io.StringIO() + content = "".join( + secrets.choice(string.ascii_letters + string.digits) + for _ in range(6 * 8 * 1024) + ) + now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") + self.create_logfile(data_dir, timestamp=now, content=content) + + with contextlib.redirect_stdout(f): + devstack = CephDevStack() + await devstack.logs() + assert content == f.getvalue() + async def test_logs_command_display_log_file_of_given_job_id(self): with tempfile.TemporaryDirectory() as data_dir: config["data_dir"] = data_dir From 7cb3b5e16cdd479b5dd5885828a54d21eddd6c99 Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Wed, 16 Apr 2025 20:18:04 +0100 Subject: [PATCH 07/13] replace os.path usage by Path methods --- ceph_devstack/resources/ceph/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ceph_devstack/resources/ceph/__init__.py b/ceph_devstack/resources/ceph/__init__.py index 8955b00e4..f1d3b38d3 100644 --- a/ceph_devstack/resources/ceph/__init__.py +++ b/ceph_devstack/resources/ceph/__init__.py @@ -254,12 +254,12 @@ def get_log_file(self, run_name: str = None, job_id: str = None): if not run_name: run_name = get_most_recent_run(os.listdir(archive_dir)) - run_dir = os.path.join(archive_dir, run_name) + run_dir = archive_dir.joinpath(run_name) if not job_id: job_id = get_job_id(os.listdir(run_dir)) - log_file = os.path.join(run_dir, job_id, "teuthology.log") - if not os.path.exists(log_file): + log_file = run_dir.joinpath(job_id, "teuthology.log") + if not log_file.exists(): raise FileNotFoundError return log_file From a014e78d3be8f8fb38a89ef0ed59b5a3dc2534ac Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Thu, 17 Apr 2025 15:27:47 +0100 Subject: [PATCH 08/13] provide more descriptive message on multiple jobs found --- ceph_devstack/resources/ceph/__init__.py | 7 ++++--- ceph_devstack/resources/ceph/exceptions.py | 3 ++- ceph_devstack/resources/ceph/utils.py | 2 +- ceph_devstack/resources/test/test_devstack.py | 3 ++- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/ceph_devstack/resources/ceph/__init__.py b/ceph_devstack/resources/ceph/__init__.py index f1d3b38d3..2e6cb2036 100644 --- a/ceph_devstack/resources/ceph/__init__.py +++ b/ceph_devstack/resources/ceph/__init__.py @@ -236,10 +236,11 @@ async def logs( log_file = self.get_log_file(run_name, job_id) except FileNotFoundError: logger.error("No log file found") - except TooManyJobsFound: - logger.error( - "Found too many jobs for target run. Please pick a job id with -j flag." + except TooManyJobsFound as e: + msg = "Found too many jobs: {jobs} for target run. Please pick a job id with -j flag.".format( + jobs=", ".join(e.jobs) ) + logger.error(msg) else: if locate: print(log_file) diff --git a/ceph_devstack/resources/ceph/exceptions.py b/ceph_devstack/resources/ceph/exceptions.py index 35fccb76f..c18d4c407 100644 --- a/ceph_devstack/resources/ceph/exceptions.py +++ b/ceph_devstack/resources/ceph/exceptions.py @@ -1,2 +1,3 @@ class TooManyJobsFound(Exception): - pass + def __init__(self, jobs: list[str]): + self.jobs = jobs diff --git a/ceph_devstack/resources/ceph/utils.py b/ceph_devstack/resources/ceph/utils.py index 7ddcb5b66..7ac2d75c0 100644 --- a/ceph_devstack/resources/ceph/utils.py +++ b/ceph_devstack/resources/ceph/utils.py @@ -40,5 +40,5 @@ def get_job_id(jobs: list[str]): if len(dirs) == 0: raise FileNotFoundError elif len(dirs) > 1: - raise TooManyJobsFound + raise TooManyJobsFound(dirs) return dirs[0] diff --git a/ceph_devstack/resources/test/test_devstack.py b/ceph_devstack/resources/test/test_devstack.py index 742120b30..809bd84b7 100644 --- a/ceph_devstack/resources/test/test_devstack.py +++ b/ceph_devstack/resources/test/test_devstack.py @@ -47,8 +47,9 @@ def test_get_job_id_throws_filenotfound_on_missing_job(self): def test_get_job_id_throws_toomanyjobsfound_on_more_than_one_job(self): jobs = ["1", "2"] - with pytest.raises(TooManyJobsFound): + with pytest.raises(TooManyJobsFound) as exc: get_job_id(jobs) + assert exc.value.dirs == jobs async def test_logs_command_display_log_file_of_latest_run(self): with tempfile.TemporaryDirectory() as data_dir: From 1474b7bbecdc902852ae1941302ae9ab41671f1d Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Thu, 17 Apr 2025 15:34:44 +0100 Subject: [PATCH 09/13] correct test --- ceph_devstack/resources/test/test_devstack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ceph_devstack/resources/test/test_devstack.py b/ceph_devstack/resources/test/test_devstack.py index 809bd84b7..ab0b4be4a 100644 --- a/ceph_devstack/resources/test/test_devstack.py +++ b/ceph_devstack/resources/test/test_devstack.py @@ -49,7 +49,7 @@ def test_get_job_id_throws_toomanyjobsfound_on_more_than_one_job(self): jobs = ["1", "2"] with pytest.raises(TooManyJobsFound) as exc: get_job_id(jobs) - assert exc.value.dirs == jobs + assert exc.value.jobs == jobs async def test_logs_command_display_log_file_of_latest_run(self): with tempfile.TemporaryDirectory() as data_dir: From 77b8416db02f00fa002d51e6d26bff37df7ec643 Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Mon, 21 Apr 2025 21:28:23 +0100 Subject: [PATCH 10/13] replace tempfile usage with pytest fixture --- ceph_devstack/resources/test/test_devstack.py | 185 +++++++++--------- 1 file changed, 92 insertions(+), 93 deletions(-) diff --git a/ceph_devstack/resources/test/test_devstack.py b/ceph_devstack/resources/test/test_devstack.py index ab0b4be4a..77fba289d 100644 --- a/ceph_devstack/resources/test/test_devstack.py +++ b/ceph_devstack/resources/test/test_devstack.py @@ -1,7 +1,6 @@ import os import io import contextlib -import tempfile import random as rd from datetime import datetime, timedelta import secrets @@ -51,98 +50,98 @@ def test_get_job_id_throws_toomanyjobsfound_on_more_than_one_job(self): get_job_id(jobs) assert exc.value.jobs == jobs - async def test_logs_command_display_log_file_of_latest_run(self): - with tempfile.TemporaryDirectory() as data_dir: - config["data_dir"] = data_dir - f = io.StringIO() - content = "custom log content" - now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") - forty_days_ago = (datetime.now() - timedelta(days=40)).strftime( - "%Y-%m-%d_%H:%M:%S" - ) - - self.create_logfile(data_dir, timestamp=now, content=content) - self.create_logfile(data_dir, timestamp=forty_days_ago) - - with contextlib.redirect_stdout(f): - devstack = CephDevStack() - await devstack.logs() - assert content in f.getvalue() - - async def test_logs_display_roughly_contents_of_log_file(self): - with tempfile.TemporaryDirectory() as data_dir: - config["data_dir"] = data_dir - f = io.StringIO() - content = "".join( - secrets.choice(string.ascii_letters + string.digits) - for _ in range(6 * 8 * 1024) - ) - now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") - self.create_logfile(data_dir, timestamp=now, content=content) - - with contextlib.redirect_stdout(f): - devstack = CephDevStack() - await devstack.logs() - assert content == f.getvalue() - - async def test_logs_command_display_log_file_of_given_job_id(self): - with tempfile.TemporaryDirectory() as data_dir: - config["data_dir"] = data_dir - f = io.StringIO() - content = "custom log message" - now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") - - self.create_logfile( - data_dir, - timestamp=now, - test_type="ceph", - job_id="1", - content="another log", - ) - self.create_logfile( - data_dir, timestamp=now, test_type="ceph", job_id="2", content=content - ) - - with contextlib.redirect_stdout(f): - devstack = CephDevStack() - await devstack.logs(job_id="2") - assert content in f.getvalue() - - async def test_logs_display_content_of_provided_run_name(self): - with tempfile.TemporaryDirectory() as data_dir: - config["data_dir"] = data_dir - f = io.StringIO() - content = "custom content" - now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") - three_days_ago = (datetime.now() - timedelta(days=3)).strftime( - "%Y-%m-%d_%H:%M:%S" - ) - - self.create_logfile( - data_dir, - timestamp=now, - ) - run_name = self.create_logfile( - data_dir, - timestamp=three_days_ago, - content=content, - ).split("/")[-3] - - with contextlib.redirect_stdout(f): - devstack = CephDevStack() - await devstack.logs(run_name=run_name) - assert content in f.getvalue() - - async def test_logs_locate_display_file_path_instead_of_config(self): - with tempfile.TemporaryDirectory() as data_dir: - config["data_dir"] = data_dir - f = io.StringIO() - - logfile = self.create_logfile(data_dir) - with contextlib.redirect_stdout(f): - devstack = CephDevStack() - await devstack.logs(locate=True) - assert logfile in f.getvalue() + async def test_logs_command_display_log_file_of_latest_run(self, tmp_path): + data_dir = str(tmp_path) + config["data_dir"] = data_dir + f = io.StringIO() + content = "custom log content" + now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") + forty_days_ago = (datetime.now() - timedelta(days=40)).strftime( + "%Y-%m-%d_%H:%M:%S" + ) + + self.create_logfile(data_dir, timestamp=now, content=content) + self.create_logfile(data_dir, timestamp=forty_days_ago) + + with contextlib.redirect_stdout(f): + devstack = CephDevStack() + await devstack.logs() + assert content in f.getvalue() + + async def test_logs_display_roughly_contents_of_log_file(self, tmp_path): + data_dir = str(tmp_path) + config["data_dir"] = data_dir + f = io.StringIO() + content = "".join( + secrets.choice(string.ascii_letters + string.digits) + for _ in range(6 * 8 * 1024) + ) + now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") + self.create_logfile(data_dir, timestamp=now, content=content) + + with contextlib.redirect_stdout(f): + devstack = CephDevStack() + await devstack.logs() + assert content == f.getvalue() + + async def test_logs_command_display_log_file_of_given_job_id(self, tmp_path): + data_dir = str(tmp_path) + config["data_dir"] = data_dir + f = io.StringIO() + content = "custom log message" + now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") + + self.create_logfile( + data_dir, + timestamp=now, + test_type="ceph", + job_id="1", + content="another log", + ) + self.create_logfile( + data_dir, timestamp=now, test_type="ceph", job_id="2", content=content + ) + + with contextlib.redirect_stdout(f): + devstack = CephDevStack() + await devstack.logs(job_id="2") + assert content in f.getvalue() + + async def test_logs_display_content_of_provided_run_name(self, tmp_path): + data_dir = str(tmp_path) + config["data_dir"] = data_dir + f = io.StringIO() + content = "custom content" + now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") + three_days_ago = (datetime.now() - timedelta(days=3)).strftime( + "%Y-%m-%d_%H:%M:%S" + ) + + self.create_logfile( + data_dir, + timestamp=now, + ) + run_name = self.create_logfile( + data_dir, + timestamp=three_days_ago, + content=content, + ).split("/")[-3] + + with contextlib.redirect_stdout(f): + devstack = CephDevStack() + await devstack.logs(run_name=run_name) + assert content in f.getvalue() + + async def test_logs_locate_display_file_path_instead_of_config(self, tmp_path): + data_dir = str(tmp_path) + + config["data_dir"] = data_dir + f = io.StringIO() + logfile = self.create_logfile(data_dir) + with contextlib.redirect_stdout(f): + devstack = CephDevStack() + await devstack.logs(locate=True) + assert logfile in f.getvalue() def create_logfile(self, data_dir: str, **kwargs): parts = { From 31bd3a90df33d2873734d45db4cdd36319455bfe Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Mon, 21 Apr 2025 21:34:47 +0100 Subject: [PATCH 11/13] replace logfile uses with log_file --- ceph_devstack/resources/test/test_devstack.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ceph_devstack/resources/test/test_devstack.py b/ceph_devstack/resources/test/test_devstack.py index 77fba289d..0b5656168 100644 --- a/ceph_devstack/resources/test/test_devstack.py +++ b/ceph_devstack/resources/test/test_devstack.py @@ -60,8 +60,8 @@ async def test_logs_command_display_log_file_of_latest_run(self, tmp_path): "%Y-%m-%d_%H:%M:%S" ) - self.create_logfile(data_dir, timestamp=now, content=content) - self.create_logfile(data_dir, timestamp=forty_days_ago) + self.create_log_file(data_dir, timestamp=now, content=content) + self.create_log_file(data_dir, timestamp=forty_days_ago) with contextlib.redirect_stdout(f): devstack = CephDevStack() @@ -77,7 +77,7 @@ async def test_logs_display_roughly_contents_of_log_file(self, tmp_path): for _ in range(6 * 8 * 1024) ) now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") - self.create_logfile(data_dir, timestamp=now, content=content) + self.create_log_file(data_dir, timestamp=now, content=content) with contextlib.redirect_stdout(f): devstack = CephDevStack() @@ -91,14 +91,14 @@ async def test_logs_command_display_log_file_of_given_job_id(self, tmp_path): content = "custom log message" now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") - self.create_logfile( + self.create_log_file( data_dir, timestamp=now, test_type="ceph", job_id="1", content="another log", ) - self.create_logfile( + self.create_log_file( data_dir, timestamp=now, test_type="ceph", job_id="2", content=content ) @@ -117,11 +117,11 @@ async def test_logs_display_content_of_provided_run_name(self, tmp_path): "%Y-%m-%d_%H:%M:%S" ) - self.create_logfile( + self.create_log_file( data_dir, timestamp=now, ) - run_name = self.create_logfile( + run_name = self.create_log_file( data_dir, timestamp=three_days_ago, content=content, @@ -137,13 +137,13 @@ async def test_logs_locate_display_file_path_instead_of_config(self, tmp_path): config["data_dir"] = data_dir f = io.StringIO() - logfile = self.create_logfile(data_dir) + log_file = self.create_log_file(data_dir) with contextlib.redirect_stdout(f): devstack = CephDevStack() await devstack.logs(locate=True) - assert logfile in f.getvalue() + assert log_file in f.getvalue() - def create_logfile(self, data_dir: str, **kwargs): + def create_log_file(self, data_dir: str, **kwargs): parts = { "timestamp": (datetime.now() - timedelta(days=rd.randint(1, 100))).strftime( "%Y-%m-%d_%H:%M:%S" From a2d41b18518778c312d338d70f7adb7e8bbc3ffa Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Mon, 21 Apr 2025 21:52:11 +0100 Subject: [PATCH 12/13] make create_log_file method pytest fixture --- ceph_devstack/resources/ceph/__init__.py | 2 +- ceph_devstack/resources/test/test_devstack.py | 86 +++++++++++-------- 2 files changed, 51 insertions(+), 37 deletions(-) diff --git a/ceph_devstack/resources/ceph/__init__.py b/ceph_devstack/resources/ceph/__init__.py index 2e6cb2036..515f80313 100644 --- a/ceph_devstack/resources/ceph/__init__.py +++ b/ceph_devstack/resources/ceph/__init__.py @@ -237,7 +237,7 @@ async def logs( except FileNotFoundError: logger.error("No log file found") except TooManyJobsFound as e: - msg = "Found too many jobs: {jobs} for target run. Please pick a job id with -j flag.".format( + msg = "Found too many jobs ({jobs}) for target run. Please pick a job id with -j flag.".format( jobs=", ".join(e.jobs) ) logger.error(msg) diff --git a/ceph_devstack/resources/test/test_devstack.py b/ceph_devstack/resources/test/test_devstack.py index 0b5656168..fab7bdfac 100644 --- a/ceph_devstack/resources/test/test_devstack.py +++ b/ceph_devstack/resources/test/test_devstack.py @@ -50,7 +50,9 @@ def test_get_job_id_throws_toomanyjobsfound_on_more_than_one_job(self): get_job_id(jobs) assert exc.value.jobs == jobs - async def test_logs_command_display_log_file_of_latest_run(self, tmp_path): + async def test_logs_command_display_log_file_of_latest_run( + self, tmp_path, create_log_file + ): data_dir = str(tmp_path) config["data_dir"] = data_dir f = io.StringIO() @@ -60,15 +62,17 @@ async def test_logs_command_display_log_file_of_latest_run(self, tmp_path): "%Y-%m-%d_%H:%M:%S" ) - self.create_log_file(data_dir, timestamp=now, content=content) - self.create_log_file(data_dir, timestamp=forty_days_ago) + create_log_file(data_dir, timestamp=now, content=content) + create_log_file(data_dir, timestamp=forty_days_ago) with contextlib.redirect_stdout(f): devstack = CephDevStack() await devstack.logs() assert content in f.getvalue() - async def test_logs_display_roughly_contents_of_log_file(self, tmp_path): + async def test_logs_display_roughly_contents_of_log_file( + self, tmp_path, create_log_file + ): data_dir = str(tmp_path) config["data_dir"] = data_dir f = io.StringIO() @@ -77,28 +81,30 @@ async def test_logs_display_roughly_contents_of_log_file(self, tmp_path): for _ in range(6 * 8 * 1024) ) now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") - self.create_log_file(data_dir, timestamp=now, content=content) + create_log_file(data_dir, timestamp=now, content=content) with contextlib.redirect_stdout(f): devstack = CephDevStack() await devstack.logs() assert content == f.getvalue() - async def test_logs_command_display_log_file_of_given_job_id(self, tmp_path): + async def test_logs_command_display_log_file_of_given_job_id( + self, tmp_path, create_log_file + ): data_dir = str(tmp_path) config["data_dir"] = data_dir f = io.StringIO() content = "custom log message" now = datetime.now().strftime("%Y-%m-%d_%H:%M:%S") - self.create_log_file( + create_log_file( data_dir, timestamp=now, test_type="ceph", job_id="1", content="another log", ) - self.create_log_file( + create_log_file( data_dir, timestamp=now, test_type="ceph", job_id="2", content=content ) @@ -107,7 +113,9 @@ async def test_logs_command_display_log_file_of_given_job_id(self, tmp_path): await devstack.logs(job_id="2") assert content in f.getvalue() - async def test_logs_display_content_of_provided_run_name(self, tmp_path): + async def test_logs_display_content_of_provided_run_name( + self, tmp_path, create_log_file + ): data_dir = str(tmp_path) config["data_dir"] = data_dir f = io.StringIO() @@ -117,11 +125,11 @@ async def test_logs_display_content_of_provided_run_name(self, tmp_path): "%Y-%m-%d_%H:%M:%S" ) - self.create_log_file( + create_log_file( data_dir, timestamp=now, ) - run_name = self.create_log_file( + run_name = create_log_file( data_dir, timestamp=three_days_ago, content=content, @@ -132,37 +140,43 @@ async def test_logs_display_content_of_provided_run_name(self, tmp_path): await devstack.logs(run_name=run_name) assert content in f.getvalue() - async def test_logs_locate_display_file_path_instead_of_config(self, tmp_path): + async def test_logs_locate_display_file_path_instead_of_config( + self, tmp_path, create_log_file + ): data_dir = str(tmp_path) config["data_dir"] = data_dir f = io.StringIO() - log_file = self.create_log_file(data_dir) + log_file = create_log_file(data_dir) with contextlib.redirect_stdout(f): devstack = CephDevStack() await devstack.logs(locate=True) assert log_file in f.getvalue() - def create_log_file(self, data_dir: str, **kwargs): - parts = { - "timestamp": (datetime.now() - timedelta(days=rd.randint(1, 100))).strftime( - "%Y-%m-%d_%H:%M:%S" - ), - "test_type": rd.choice(["ceph", "rgw", "rbd", "mds"]), - "job_id": rd.randint(1, 100), - "content": "some log data", - **kwargs, - } - timestamp = parts["timestamp"] - test_type = parts["test_type"] - job_id = parts["job_id"] - content = parts["content"] - - run_name = f"root-{timestamp}-orch:cephadm:{test_type}-small-main-distro-default-testnode" - log_dir = f"{data_dir}/archive/{run_name}/{job_id}" - - os.makedirs(log_dir, exist_ok=True) - log_file = f"{log_dir}/teuthology.log" - with open(log_file, "w") as f: - f.write(content) - return log_file + @pytest.fixture(scope="class") + def create_log_file(self): + def _create_log_file(data_dir: str, **kwargs): + parts = { + "timestamp": ( + datetime.now() - timedelta(days=rd.randint(1, 100)) + ).strftime("%Y-%m-%d_%H:%M:%S"), + "test_type": rd.choice(["ceph", "rgw", "rbd", "mds"]), + "job_id": rd.randint(1, 100), + "content": "some log data", + **kwargs, + } + timestamp = parts["timestamp"] + test_type = parts["test_type"] + job_id = parts["job_id"] + content = parts["content"] + + run_name = f"root-{timestamp}-orch:cephadm:{test_type}-small-main-distro-default-testnode" + log_dir = f"{data_dir}/archive/{run_name}/{job_id}" + + os.makedirs(log_dir, exist_ok=True) + log_file = f"{log_dir}/teuthology.log" + with open(log_file, "w") as f: + f.write(content) + return log_file + + return _create_log_file From 7c8b43b155e73c27cab7eb6af374aa9302b55d9d Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Mon, 21 Apr 2025 21:56:50 +0100 Subject: [PATCH 13/13] improve toomanyjobsfound error message --- ceph_devstack/resources/ceph/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ceph_devstack/resources/ceph/__init__.py b/ceph_devstack/resources/ceph/__init__.py index 515f80313..2962eb472 100644 --- a/ceph_devstack/resources/ceph/__init__.py +++ b/ceph_devstack/resources/ceph/__init__.py @@ -237,7 +237,7 @@ async def logs( except FileNotFoundError: logger.error("No log file found") except TooManyJobsFound as e: - msg = "Found too many jobs ({jobs}) for target run. Please pick a job id with -j flag.".format( + msg = "Found too many jobs ({jobs}) for target run. Please pick a job id with -j option.".format( jobs=", ".join(e.jobs) ) logger.error(msg)