From 6cad0455b07b9270dd0d9650c3e655fe6fa53f1e Mon Sep 17 00:00:00 2001 From: Alexander Bayandin Date: Sat, 27 Jul 2024 20:01:10 +0100 Subject: [PATCH] CI(test_runner): Upload all test artifacts if preserve_database_files is enabled (#7990) ## Problem There's a `NeonEnvBuilder#preserve_database_files` parameter that allows you to keep database files for debugging purposes (by default, files get cleaned up), but there's no way to get these files from a CI run. This PR adds handling of `NeonEnvBuilder#preserve_database_files` and adds the compressed test output directory to Allure reports (for tests with this parameter enabled). Ref https://github.com/neondatabase/neon/issues/6967 ## Summary of changes - Compress and add the whole test output directory to Allure reports - Currently works only with `neon_env_builder` fixture - Remove `preserve_database_files = True` from sharding tests as unneeded --------- Co-authored-by: Christian Schwarz --- test_runner/README.md | 2 +- test_runner/fixtures/neon_fixtures.py | 20 +++++++++++++++++--- test_runner/fixtures/utils.py | 11 ++++++++++- test_runner/regress/test_sharding.py | 4 ---- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/test_runner/README.md b/test_runner/README.md index 7d95634ea8..e2f26a19ce 100644 --- a/test_runner/README.md +++ b/test_runner/README.md @@ -81,7 +81,7 @@ should go. Useful parameters and commands: `--preserve-database-files` to preserve pageserver (layer) and safekeer (segment) timeline files on disk -after running a test suite. Such files might be large, so removed by default; but might be useful for debugging or creation of svg images with layer file contents. +after running a test suite. Such files might be large, so removed by default; but might be useful for debugging or creation of svg images with layer file contents. If `NeonEnvBuilder#preserve_database_files` set to `True` for a particular test, the whole `repo` directory will be attached to Allure report (thus uploaded to S3) as `everything.tar.zst` for this test. Let stdout, stderr and `INFO` log messages go to the terminal instead of capturing them: `./scripts/pytest -s --log-cli-level=INFO ...` diff --git a/test_runner/fixtures/neon_fixtures.py b/test_runner/fixtures/neon_fixtures.py index 0a06398391..d98b2564df 100644 --- a/test_runner/fixtures/neon_fixtures.py +++ b/test_runner/fixtures/neon_fixtures.py @@ -1394,7 +1394,7 @@ def _shared_simple_env( pg_distrib_dir=pg_distrib_dir, pg_version=pg_version, run_id=run_id, - preserve_database_files=pytestconfig.getoption("--preserve-database-files"), + preserve_database_files=cast(bool, pytestconfig.getoption("--preserve-database-files")), test_name=request.node.name, test_output_dir=test_output_dir, pageserver_virtual_file_io_engine=pageserver_virtual_file_io_engine, @@ -1469,7 +1469,7 @@ def neon_env_builder( pg_version=pg_version, broker=default_broker, run_id=run_id, - preserve_database_files=pytestconfig.getoption("--preserve-database-files"), + preserve_database_files=cast(bool, pytestconfig.getoption("--preserve-database-files")), pageserver_virtual_file_io_engine=pageserver_virtual_file_io_engine, test_name=request.node.name, test_output_dir=test_output_dir, @@ -1478,6 +1478,11 @@ def neon_env_builder( pageserver_default_tenant_config_compaction_algorithm=pageserver_default_tenant_config_compaction_algorithm, ) as builder: yield builder + # Propogate `preserve_database_files` to make it possible to use in other fixtures, + # like `test_output_dir` fixture for attaching all database files to Allure report. + request.node.user_properties.append( + ("preserve_database_files", builder.preserve_database_files) + ) @dataclass @@ -4478,7 +4483,16 @@ def test_output_dir( yield test_dir - allure_attach_from_dir(test_dir) + preserve_database_files = False + for k, v in request.node.user_properties: + # NB: the neon_env_builder fixture uses this fixture (test_output_dir). + # So, neon_env_builder's cleanup runs before here. + # The cleanup propagates NeonEnvBuilder.preserve_database_files into this user property. + if k == "preserve_database_files": + assert isinstance(v, bool) + preserve_database_files = v + + allure_attach_from_dir(test_dir, preserve_database_files) class FileAndThreadLock: diff --git a/test_runner/fixtures/utils.py b/test_runner/fixtures/utils.py index 0989dc1893..7f54eb0b0a 100644 --- a/test_runner/fixtures/utils.py +++ b/test_runner/fixtures/utils.py @@ -240,9 +240,18 @@ ATTACHMENT_NAME_REGEX: re.Pattern = re.compile( # type: ignore[type-arg] ) -def allure_attach_from_dir(dir: Path): +def allure_attach_from_dir(dir: Path, preserve_database_files: bool = False): """Attach all non-empty files from `dir` that matches `ATTACHMENT_NAME_REGEX` to Allure report""" + if preserve_database_files: + zst_file = dir.with_suffix(".tar.zst") + with zst_file.open("wb") as zst: + cctx = zstandard.ZstdCompressor() + with cctx.stream_writer(zst) as compressor: + with tarfile.open(fileobj=compressor, mode="w") as tar: + tar.add(dir, arcname="") + allure.attach.file(zst_file, "everything.tar.zst", "application/zstd", "tar.zst") + for attachment in Path(dir).glob("**/*"): if ATTACHMENT_NAME_REGEX.fullmatch(attachment.name) and attachment.stat().st_size > 0: name = str(attachment.relative_to(dir)) diff --git a/test_runner/regress/test_sharding.py b/test_runner/regress/test_sharding.py index f8770e70fe..7f30b2d7a7 100644 --- a/test_runner/regress/test_sharding.py +++ b/test_runner/regress/test_sharding.py @@ -48,8 +48,6 @@ def test_sharding_smoke( # that the scrubber doesn't barf when it sees a sharded tenant. neon_env_builder.enable_pageserver_remote_storage(s3_storage()) - neon_env_builder.preserve_database_files = True - env = neon_env_builder.init_start( initial_tenant_shard_count=shard_count, initial_tenant_shard_stripe_size=stripe_size ) @@ -372,8 +370,6 @@ def test_sharding_split_smoke( # that the scrubber doesn't barf when it sees a sharded tenant. neon_env_builder.enable_pageserver_remote_storage(s3_storage()) - neon_env_builder.preserve_database_files = True - non_default_tenant_config = {"gc_horizon": 77 * 1024 * 1024} env = neon_env_builder.init_configs(True)