diff --git a/.github/workflows/scripts/func_test_script.sh b/.github/workflows/scripts/func_test_script.sh new file mode 100755 index 00000000000..b42e73688f5 --- /dev/null +++ b/.github/workflows/scripts/func_test_script.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# Functional test runner with task-queue sampling right after parallel suites. +# Sourced from script.sh via FUNC_TEST_SCRIPT. + +_nightly_args=() +if [[ "${GITHUB_WORKFLOW:-}" =~ "Nightly" ]]; then + _nightly_args=(--nightly) +fi + +_pytest_common=(-v --timeout=300 -r sx --color=yes --suppress-no-test-exit-code --durations=20) + +# Parallel suites first (xdist -n 8), then sample worker-queue waits before serial. +cmd_user_prefix bash -c "pytest ${_pytest_common[*]} --pyargs pulpcore.tests.functional -m parallel -n 8 ${_nightly_args[*]}" +cmd_user_prefix bash -c "pytest ${_pytest_common[*]} --pyargs pulp_file.tests.functional -m parallel -n 8 ${_nightly_args[*]}" +cmd_user_prefix bash -c "pytest ${_pytest_common[*]} --pyargs pulp_certguard.tests.functional -m parallel -n 8 ${_nightly_args[*]}" + +echo "::group::Task queue wait stats (after parallel suites)" +# Short window: capture tasks from the parallel phase before purge/serial dilute the signal. +cmd_user_prefix pulpcore-manager task-queue-stats --hours 1 --top 20 || true +echo "::endgroup::" + +cmd_user_prefix bash -c "pytest ${_pytest_common[*]} --pyargs pulpcore.tests.functional -m 'not parallel' ${_nightly_args[*]}" +cmd_user_prefix bash -c "pytest ${_pytest_common[*]} --pyargs pulp_file.tests.functional -m 'not parallel' ${_nightly_args[*]}" +cmd_user_prefix bash -c "pytest ${_pytest_common[*]} --pyargs pulp_certguard.tests.functional -m 'not parallel' ${_nightly_args[*]}" diff --git a/CHANGES/+task-queue-stats.misc b/CHANGES/+task-queue-stats.misc new file mode 100644 index 00000000000..f59c75f51dc --- /dev/null +++ b/CHANGES/+task-queue-stats.misc @@ -0,0 +1 @@ +Add `pulpcore-manager task-queue-stats` to summarize worker-queue waits from `unblocked_at`. diff --git a/CLAUDE.md b/CLAUDE.md index fc276b00c41..6dbef3719fb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -38,6 +38,8 @@ pulpcore & pulp-file functional tests require both client bindings to be install **Always** use the `oci-env` to run the functional and unit tests. +Do not UPDATE `Task.pulp_created`. A Postgres trigger (`on_update_timestamp_task`) raises `Updating pulp_created is not allowed.` Create rows in the desired order instead. + ## Modifying template_config.yml Use the `plugin-template` tool after any changes made to `template_config.yml`. @@ -57,6 +59,6 @@ When patchback fails to cherry-pick a PR into an older branch, you need to manua ## Contributing -All docs, code comments, and changelogs are in markdown format. Keep comments and changelogs short and concise. Try to keep changelogs to just one line. +All docs, code comments, and changelogs are in markdown format. Keep comments and changelogs short and concise. Try to keep changelogs to just one line. Use single backticks for inline code (`like_this`), not RST double backticks (``like_this``). When preparing to commit and create a PR you **must** follow our [PR checklist](https://pulpproject.org/pulpcore/docs/dev/guides/pull-request-walkthrough/) Important to note is the AI attribution requirement in our commit messages. Also, note that our changelog entries are markdown. diff --git a/pulp_file/tests/functional/api/test_auto_publish.py b/pulp_file/tests/functional/api/test_auto_publish.py index 1ad8cab3bc1..f242e97634f 100644 --- a/pulp_file/tests/functional/api/test_auto_publish.py +++ b/pulp_file/tests/functional/api/test_auto_publish.py @@ -1,5 +1,7 @@ """Tests that sync file plugin repositories.""" +from uuid import uuid4 + import pytest from pulpcore.client.pulp_file import ( @@ -24,8 +26,10 @@ def test_auto_publish_and_distribution( file_random_content_unit, monitor_task, has_pulp_plugin, + random_artifact_factory, ): """Tests auto-publish and auto-distribution""" + # Remote is only needed to assert mirror=True is rejected with autopublish. remote = file_remote_ssl_factory(manifest_path=basic_manifest_path, policy="on_demand") repo = file_bindings.RepositoriesFileApi.read(file_repo_with_auto_publish.pulp_href) distribution = gen_object_with_cleanup( @@ -44,12 +48,18 @@ def test_auto_publish_and_distribution( ) assert distribution.publication is None - # Check what content and artifacts are in the fixture repository - expected_files = get_files_in_manifest(remote.url) - - # Sync from the remote - body = FileRepositorySyncURL(remote=remote.pulp_href) - monitor_task(file_bindings.RepositoriesFileApi.sync(repo.pulp_href, body).task) + # One content unit is enough for version 1; attaching it triggers autopublish. + artifact = random_artifact_factory() + relative_path = f"{uuid4()}.iso" + created = monitor_task( + file_bindings.ContentFilesApi.create( + artifact=artifact.pulp_href, + relative_path=relative_path, + repository=repo.pulp_href, + ).task + ).created_resources + content = file_bindings.ContentFilesApi.read(created[1] if len(created) > 1 else created[0]) + expected_files = {(relative_path, content.sha256, str(artifact.size))} repo = file_bindings.RepositoriesFileApi.read(repo.pulp_href) # Assert that a new repository version was created and a publication was created diff --git a/pulp_file/tests/functional/api/test_filesystem_export.py b/pulp_file/tests/functional/api/test_filesystem_export.py index da92c1b471b..1caf5472691 100644 --- a/pulp_file/tests/functional/api/test_filesystem_export.py +++ b/pulp_file/tests/functional/api/test_filesystem_export.py @@ -174,6 +174,15 @@ def test_fsexport_by_version( } +def _filesystem_domain(pulpcore_bindings, gen_object_with_cleanup): + body = { + "name": str(uuid.uuid4()), + "storage_class": "pulpcore.app.models.storage.FileSystem", + "storage_settings": {"MEDIA_ROOT": "/var/lib/pulp/media/"}, + } + return gen_object_with_cleanup(pulpcore_bindings.DomainsApi, body) + + @pytest.mark.skipif(not settings.DOMAIN_ENABLED, reason="Domains not enabled.") @pytest.mark.parallel def test_fsexport_cross_domain( @@ -181,40 +190,48 @@ def test_fsexport_cross_domain( fs_export_factory, gen_object_with_cleanup, pulpcore_bindings, - pub_and_repo, + file_bindings, + file_repository_factory, + file_publication_factory, + tmp_path, + monitor_task, ): + # Publication and versions live in source_domain; exporter lives in other_domain. + source_domain = _filesystem_domain(pulpcore_bindings, gen_object_with_cleanup) + other_domain = _filesystem_domain(pulpcore_bindings, gen_object_with_cleanup) + + src = tmp_path / "file.dat" + src.write_text("x") + content_href = file_bindings.ContentFilesApi.upload( + relative_path="0.dat", file=str(src), pulp_domain=source_domain.name + ).pulp_href + repository = file_repository_factory(pulp_domain=source_domain.name) + monitor_task( + file_bindings.RepositoriesFileApi.modify( + repository.pulp_href, {"add_content_units": [content_href]} + ).task + ) + repository = file_bindings.RepositoriesFileApi.read(repository.pulp_href) + publication = file_publication_factory( + repository=repository.pulp_href, pulp_domain=source_domain.name + ) + latest = repository.latest_version_href + zeroth = latest.rsplit("/", 2)[0] + "/0/" + exporter = fs_exporter_factory(pulp_domain=other_domain.name) - entities = [{}, {}] - for e in entities: - body = { - "name": str(uuid.uuid4()), - "storage_class": "pulpcore.app.models.storage.FileSystem", - "storage_settings": {"MEDIA_ROOT": "/var/lib/pulp/media/"}, - } - e["domain"] = gen_object_with_cleanup(pulpcore_bindings.DomainsApi, body) - e["publication"], e["repository"] = pub_and_repo(pulp_domain=e["domain"].name) - e["exporter"] = fs_exporter_factory(pulp_domain=e["domain"].name) - body = {"publication": e["publication"].pulp_href} - e["export"] = fs_export_factory(e["exporter"], body=body) - - latest = entities[0]["repository"].latest_version_href - zeroth = latest.replace("/2/", "/0/") - - with pytest.raises(BadRequestException) as e: - body = {"publication": entities[0]["publication"].pulp_href} - fs_export_factory(entities[1]["exporter"], body=body) + with pytest.raises(BadRequestException): + fs_export_factory(exporter, body={"publication": publication.pulp_href}) - with pytest.raises(BadRequestException) as e: - body = {"repository_version": latest} - fs_export_factory(entities[1]["exporter"], body=body) + with pytest.raises(BadRequestException): + fs_export_factory(exporter, body={"repository_version": latest}) - with pytest.raises(BadRequestException) as e: - body = {"repository_version": latest, "start_repository_version": zeroth} - fs_export_factory(entities[1]["exporter"], body=body) + with pytest.raises(BadRequestException): + fs_export_factory( + exporter, body={"repository_version": latest, "start_repository_version": zeroth} + ) - with pytest.raises(BadRequestException) as e: - body = { - "publication": entities[0]["publication"].pulp_href, - "start_repository_version": zeroth, - } - fs_export_factory(entities[1]["exporter"], body=body) + with pytest.raises(BadRequestException): + fs_export_factory( + exporter, + body={"publication": publication.pulp_href, "start_repository_version": zeroth}, + ) diff --git a/pulp_file/tests/functional/api/test_mime_types.py b/pulp_file/tests/functional/api/test_mime_types.py index d44ba9f5b7f..e39d94713fa 100644 --- a/pulp_file/tests/functional/api/test_mime_types.py +++ b/pulp_file/tests/functional/api/test_mime_types.py @@ -13,30 +13,36 @@ def test_content_types( file_bindings, distribution_base_url, file_repo_with_auto_publish, - file_content_unit_with_name_factory, gen_object_with_cleanup, monitor_task, + tmp_path, ): """Test if content-app correctly returns mime-types based on filenames.""" + relative_paths = { + "tar.gz": f"{uuid.uuid4()}.tar.gz", + "xml.gz": f"{uuid.uuid4()}.xml.gz", + "xml.bz2": f"{uuid.uuid4()}.xml.bz2", + "xml.zstd": f"{uuid.uuid4()}.xml.zstd", + "xml.xz": f"{uuid.uuid4()}.xml.xz", + "json.zstd": f"{uuid.uuid4()}.json.zstd", + "json": f"{uuid.uuid4()}.json", + "txt": f"{uuid.uuid4()}.txt", + "xml": f"{uuid.uuid4()}.xml", + "jpg": f"{uuid.uuid4()}.jpg", + "JPG": f"{uuid.uuid4()}.JPG", + "halabala": f"{uuid.uuid4()}.halabala", + "noextension1": f"{uuid.uuid4()}.asd/.asd/a", + "noextension2": f"{uuid.uuid4()}.....f", + } + + blob = tmp_path / "blob" + blob.write_bytes(b"mime-type-test") files = { - "tar.gz": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.tar.gz"), - "xml.gz": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.xml.gz"), - "xml.bz2": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.xml.bz2"), - "xml.zstd": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.xml.zstd"), - "xml.xz": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.xml.xz"), - "json.zstd": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.json.zstd"), - "json": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.json"), - "txt": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.txt"), - "xml": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.xml"), - "jpg": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.jpg"), - "JPG": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.JPG"), - "halabala": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.halabala"), - "noextension1": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.asd/.asd/a"), - "noextension2": file_content_unit_with_name_factory(f"{str(uuid.uuid4())}.....f"), + extension: file_bindings.ContentFilesApi.upload(file=str(blob), relative_path=relative_path) + for extension, relative_path in relative_paths.items() } - units_to_add = list(map(lambda f: f.pulp_href, files.values())) - data = RepositoryAddRemoveContent(add_content_units=units_to_add) + data = RepositoryAddRemoveContent(add_content_units=[f.pulp_href for f in files.values()]) monitor_task( file_bindings.RepositoriesFileApi.modify(file_repo_with_auto_publish.pulp_href, data).task ) @@ -49,18 +55,20 @@ def test_content_types( distribution = gen_object_with_cleanup(file_bindings.DistributionsFileApi, data) distribution_base_url = distribution_base_url(distribution.base_url) - received_mimetypes = {} - for extension, content_unit in files.items(): + async def fetch_mimetypes(): + async with aiohttp.ClientSession() as session: - async def get_content_type(): - async with aiohttp.ClientSession() as session: + async def get_content_type(extension, content_unit): url = urljoin(distribution_base_url, content_unit.relative_path) async with session.get(url) as response: - return response.headers.get("Content-Type") + return extension, response.headers.get("Content-Type") - content_type = asyncio.run(get_content_type()) - received_mimetypes[extension] = content_type + pairs = await asyncio.gather( + *(get_content_type(ext, unit) for ext, unit in files.items()) + ) + return dict(pairs) + received_mimetypes = asyncio.run(fetch_mimetypes()) expected_mimetypes = { "tar.gz": "application/gzip", "xml.gz": "application/gzip", diff --git a/pulp_file/tests/functional/api/test_pulp_export.py b/pulp_file/tests/functional/api/test_pulp_export.py index 792c15812cb..7640a870a56 100644 --- a/pulp_file/tests/functional/api/test_pulp_export.py +++ b/pulp_file/tests/functional/api/test_pulp_export.py @@ -19,7 +19,7 @@ @pytest.fixture def pulp_exporter_factory( - tmpdir, + tmp_path_factory, pulpcore_bindings, gen_object_with_cleanup, add_to_filesystem_cleanup, @@ -31,7 +31,7 @@ def _pulp_exporter_factory( if repositories is None: repositories = [] name = str(uuid.uuid4()) - path = "{}/{}/".format(tmpdir, name) + path = "{}/{}/".format(tmp_path_factory.mktemp("exporter"), name) body = { "name": name, "path": path, @@ -82,7 +82,7 @@ def _pulp_export_factory(exporter, body=None): return _pulp_export_factory -@pytest.fixture +@pytest.fixture(scope="class") def three_synced_repositories( file_bindings, file_repository_factory, @@ -101,7 +101,8 @@ def three_synced_repositories( file_bindings.RepositoriesFileApi.sync(repository.pulp_href, {}).task for repository in repositories ] - [monitor_task(task) for task in sync_tasks] + for task in sync_tasks: + monitor_task(task) repositories = [ file_bindings.RepositoriesFileApi.read(repository.pulp_href) for repository in repositories ] @@ -132,13 +133,25 @@ def shallow_pulp_exporter(pulp_exporter_factory): return pulp_exporter_factory() -@pytest.fixture +@pytest.fixture(scope="class") def full_pulp_exporter( - pulp_exporter_factory, + pulpcore_bindings, + tmp_path_factory, + gen_object_with_cleanup, + add_to_filesystem_cleanup, three_synced_repositories, ): - repositories = three_synced_repositories - return pulp_exporter_factory(repositories=repositories) + """Build exporter inline so this class-scoped fixture need not depend on a function factory.""" + name = str(uuid.uuid4()) + path = "{}/{}/".format(tmp_path_factory.mktemp("full-exporter"), name) + body = { + "name": name, + "path": path, + "repositories": [r.pulp_href for r in three_synced_repositories], + } + exporter = gen_object_with_cleanup(pulpcore_bindings.ExportersPulpApi, body) + add_to_filesystem_cleanup(path) + return exporter @pytest.mark.parallel @@ -169,74 +182,143 @@ def test_crud_exporter(pulpcore_bindings, shallow_pulp_exporter, monitor_task): pulpcore_bindings.ExportersPulpApi.read(exporter.pulp_href) -@pytest.mark.parallel -def test_export(pulpcore_bindings, pulp_export_factory, full_pulp_exporter, monitor_task): - exporter = full_pulp_exporter - assert len(exporter.repositories) == 3 +class TestSyncedRepoExport: + """Don't mark parallel, tests are shorter than setup.""" - # Test export - export = pulp_export_factory(exporter) + def test_export(self, pulpcore_bindings, pulp_export_factory, full_pulp_exporter, monitor_task): + exporter = full_pulp_exporter + assert len(exporter.repositories) == 3 - # Test list and delete - # export 2 more to test on - export_href2, export_href3 = ( - monitor_task( - pulpcore_bindings.ExportersPulpExportsApi.create(exporter.pulp_href, {}).task - ).created_resources[0] - for _ in range(2) - ) - exports = pulpcore_bindings.ExportersPulpExportsApi.list(exporter.pulp_href).results - assert len(exports) == 3 - pulpcore_bindings.ExportersPulpExportsApi.delete(export.pulp_href) - pulpcore_bindings.ExportersPulpExportsApi.delete(export_href2) - exports = pulpcore_bindings.ExportersPulpExportsApi.list(exporter.pulp_href).results - assert len(exports) == 1 - pulpcore_bindings.ExportersPulpExportsApi.delete(export_href3) - exports = pulpcore_bindings.ExportersPulpExportsApi.list(exporter.pulp_href).results - assert len(exports) == 0 + # Test export + export = pulp_export_factory(exporter) + # Test list and delete + # export 2 more to test on + export_href2, export_href3 = ( + monitor_task( + pulpcore_bindings.ExportersPulpExportsApi.create(exporter.pulp_href, {}).task + ).created_resources[0] + for _ in range(2) + ) + exports = pulpcore_bindings.ExportersPulpExportsApi.list(exporter.pulp_href).results + assert len(exports) == 3 + pulpcore_bindings.ExportersPulpExportsApi.delete(export.pulp_href) + pulpcore_bindings.ExportersPulpExportsApi.delete(export_href2) + exports = pulpcore_bindings.ExportersPulpExportsApi.list(exporter.pulp_href).results + assert len(exports) == 1 + pulpcore_bindings.ExportersPulpExportsApi.delete(export_href3) + exports = pulpcore_bindings.ExportersPulpExportsApi.list(exporter.pulp_href).results + assert len(exports) == 0 + + def test_export_by_version_and_chunked( + self, + pulp_exporter_factory, + pulp_export_factory, + three_synced_repositories, + ): + repositories = three_synced_repositories + latest_versions = [r.latest_version_href for r in repositories] + zeroth_versions = [v_href.replace("/1/", "/0/") for v_href in latest_versions] -@pytest.mark.parallel -def test_export_by_version_and_chunked( - pulp_exporter_factory, - pulp_export_factory, - three_synced_repositories, -): - repositories = three_synced_repositories - latest_versions = [r.latest_version_href for r in repositories] - zeroth_versions = [v_href.replace("/1/", "/0/") for v_href in latest_versions] - - # exporter for one repo. specify one version - exporter = pulp_exporter_factory(repositories=[repositories[0]]) - body = {"versions": [latest_versions[0]]} - export = pulp_export_factory(exporter, body) - assert export.exported_resources[0].endswith("/1/") - body = {"versions": [zeroth_versions[0]]} - export = pulp_export_factory(exporter, body) - assert export.exported_resources[0].endswith("/0/") - - # exporter for one repo. specify one *wrong* version - with pytest.raises(ApiException, match="must belong to"): - body = {"versions": [latest_versions[1]]} - pulp_export_factory(exporter, body) + # exporter for one repo. specify one version + exporter = pulp_exporter_factory(repositories=[repositories[0]]) + body = {"versions": [latest_versions[0]]} + export = pulp_export_factory(exporter, body) + assert export.exported_resources[0].endswith("/1/") + body = {"versions": [zeroth_versions[0]]} + export = pulp_export_factory(exporter, body) + assert export.exported_resources[0].endswith("/0/") + + # exporter for one repo. specify one *wrong* version + with pytest.raises(ApiException, match="must belong to"): + body = {"versions": [latest_versions[1]]} + pulp_export_factory(exporter, body) + + # test chunked export + body = {"chunk_size": "250B"} + export = pulp_export_factory(exporter, body) + assert export.output_file_info is not None + assert len(export.output_file_info) > 1 + + # Create a new exporter with two repos + exporter = pulp_exporter_factory(repositories=[repositories[0], repositories[1]]) + # exporter for two repos, specify one version + with pytest.raises(ApiException, match="does not match the number"): + body = {"versions": [latest_versions[0]]} + pulp_export_factory(exporter, body) + + # exporter for two repos, specify one correct and one *wrong* version + with pytest.raises(ApiException, match="must belong to"): + body = {"versions": [latest_versions[0], latest_versions[2]]} + pulp_export_factory(exporter, body) + + def test_export_with_meta(self, pulpcore_bindings, pulp_export_factory, full_pulp_exporter): + exporter = full_pulp_exporter + user_meta = { + "initiator": "ci", + "purpose": "export", + "checksum_type": "md5", # pulp should override only in TOC JSON + } - # test chunked export - body = {"chunk_size": "250B"} - export = pulp_export_factory(exporter, body) - assert export.output_file_info is not None - assert len(export.output_file_info) > 1 + export = pulp_export_factory(exporter, {"meta": user_meta}) - # Create a new exporter with two repos - exporter = pulp_exporter_factory(repositories=[repositories[0], repositories[1]]) - # exporter for two repos, specify one version - with pytest.raises(ApiException, match="does not match the number"): - body = {"versions": [latest_versions[0]]} - pulp_export_factory(exporter, body) + # toc_info contains exactly user meta (unmodified) + meta_info = export.toc_info.get("meta", {}) + assert meta_info == user_meta - # exporter for two repos, specify one correct and one *wrong* version - with pytest.raises(ApiException, match="must belong to"): - body = {"versions": [latest_versions[0], latest_versions[2]]} - pulp_export_factory(exporter, body) + # Validate TOC JSON file content + toc_file_path = export.toc_info.get("file") + assert toc_file_path and isinstance(toc_file_path, str) + + with open(toc_file_path, "r") as f: + toc_data = json.load(f) + + meta_json = toc_data.get("meta", {}) + assert meta_json.get("initiator") == "ci" + assert meta_json.get("purpose") == "export" + # overridden field check + assert meta_json.get("checksum_type") == "crc32" + + def test_export_chunk_ordering_and_naming( + self, + pulp_exporter_factory, + pulp_export_factory, + three_synced_repositories, + ): + exporter = pulp_exporter_factory(repositories=[three_synced_repositories[0]]) + chunk_size_bytes = 100 + body = {"chunk_size": f"{chunk_size_bytes}B"} + export = pulp_export_factory(exporter, body) + + all_paths = [Path(p) for p in export.output_file_info.keys()] + tar_chunks = [p for p in all_paths if ".tar." in p.name] + + assert len(tar_chunks) > 1, f"Expected multiple chunks for {chunk_size_bytes}B limit." + + for index, path in enumerate(tar_chunks): + expected_suffix = f"{index:04d}" + + assert path.name.endswith(expected_suffix), ( + f"Chunk {path} missing suffix {expected_suffix}" + ) + assert path.exists(), f"Chunk file {path} was not found on disk." + + if index < len(tar_chunks) - 1: + assert path.stat().st_size == chunk_size_bytes + + toc_path = Path(export.toc_info["file"]) + with toc_path.open("r", encoding="utf-8") as f: + toc_data = json.load(f) + + toc_filenames = list(toc_data["files"].keys()) + expected_filenames = [p.name for p in tar_chunks] + + assert toc_filenames == expected_filenames, ( + f"TOC order mismatch.\nExpected: {expected_filenames}\nActual: {toc_filenames}" + ) + + assert toc_data["meta"]["chunk_size"] == chunk_size_bytes + assert toc_data["meta"]["checksum_type"] == "crc32" @pytest.mark.parallel @@ -307,65 +389,66 @@ def test_export_incremental( @pytest.mark.skipif(not settings.DOMAIN_ENABLED, reason="Domains not enabled.") @pytest.mark.parallel def test_cross_domain_exporter( - basic_manifest_path, file_bindings, - file_remote_factory, + file_repository_factory, gen_object_with_cleanup, pulpcore_bindings, pulp_export_factory, pulp_exporter_factory, monitor_task, + tmp_path, ): - # Create two domains - # In each, create and sync a repository, create and export an exporter - # Attempt to create an exporter using the *other domain's* repo - # Attempt to update the exporter using the *other domain's* repo and last_export - # Use the exporter and attempt to export the *other domain's* repo-versions - - entities = [{}, {}] - for e in entities: + # Source domain: one uploaded file, exporter, and export (needed for last_export). + # Target domain: empty repo + exporter. Same-domain sync/export in the target is unused. + + def _domain(): body = { "name": str(uuid.uuid4()), "storage_class": "pulpcore.app.models.storage.FileSystem", "storage_settings": {"MEDIA_ROOT": "/var/lib/pulp/media/"}, } - e["domain"] = gen_object_with_cleanup(pulpcore_bindings.DomainsApi, body) - remote = file_remote_factory( - manifest_path=basic_manifest_path, policy="immediate", pulp_domain=e["domain"].name - ) + return gen_object_with_cleanup(pulpcore_bindings.DomainsApi, body) + + source_domain = _domain() + target_domain = _domain() + + src = tmp_path / "file.txt" + src.write_text("x") + content_href = file_bindings.ContentFilesApi.upload( + relative_path="file.txt", file=str(src), pulp_domain=source_domain.name + ).pulp_href + source_repo = file_repository_factory(pulp_domain=source_domain.name) + monitor_task( + file_bindings.RepositoriesFileApi.modify( + source_repo.pulp_href, {"add_content_units": [content_href]} + ).task + ) + source_repo = file_bindings.RepositoriesFileApi.read(source_repo.pulp_href) + source_exporter = pulp_exporter_factory(repositories=[source_repo], pulp_domain=source_domain) + source_export = pulp_export_factory(source_exporter) - repo_body = {"name": str(uuid.uuid4()), "remote": remote.pulp_href} - e["repository"] = gen_object_with_cleanup( - file_bindings.RepositoriesFileApi, repo_body, pulp_domain=e["domain"].name - ) - task = file_bindings.RepositoriesFileApi.sync(e["repository"].pulp_href, {}).task - monitor_task(task) - e["repository"] = file_bindings.RepositoriesFileApi.read(e["repository"].pulp_href) - e["exporter"] = pulp_exporter_factory( - repositories=[e["repository"]], pulp_domain=e["domain"] - ) - e["export"] = pulp_export_factory(e["exporter"]) + other_repo = file_repository_factory(pulp_domain=target_domain.name) + other_exporter = pulp_exporter_factory(repositories=[other_repo], pulp_domain=target_domain) - target_domain = entities[1]["domain"] # cross-create with pytest.raises(BadRequestException) as e: - pulp_exporter_factory(repositories=[entities[0]["repository"]], pulp_domain=target_domain) + pulp_exporter_factory(repositories=[source_repo], pulp_domain=target_domain) assert e.value.status == 400 assert json.loads(e.value.body) == { "non_field_errors": [f"Objects must all be a part of the {target_domain.name} domain."] } # cross-update - body = {"repositories": [entities[0]["repository"].pulp_href]} + body = {"repositories": [source_repo.pulp_href]} with pytest.raises(BadRequestException) as e: - pulpcore_bindings.ExportersPulpApi.partial_update(entities[1]["exporter"].pulp_href, body) + pulpcore_bindings.ExportersPulpApi.partial_update(other_exporter.pulp_href, body) assert e.value.status == 400 assert json.loads(e.value.body) == { "non_field_errors": [f"Objects must all be a part of the {target_domain.name} domain."] } - body = {"last_export": entities[0]["export"].pulp_href} + body = {"last_export": source_export.pulp_href} with pytest.raises(BadRequestException) as e: - pulpcore_bindings.ExportersPulpApi.partial_update(entities[1]["exporter"].pulp_href, body) + pulpcore_bindings.ExportersPulpApi.partial_update(other_exporter.pulp_href, body) assert e.value.status == 400 assert json.loads(e.value.body) == { "non_field_errors": [f"Objects must all be a part of the {target_domain.name} domain."] @@ -373,14 +456,14 @@ def test_cross_domain_exporter( # cross-export with pytest.raises(BadRequestException) as e: - latest_v = entities[0]["repository"].latest_version_href - zero_v = latest_v.replace("/1/", "/0/") + latest_v = source_repo.latest_version_href + zero_v = latest_v.rsplit("/", 2)[0] + "/0/" body = { "start_versions": [latest_v], "versions": [zero_v], "full": False, } - pulp_export_factory(entities[1]["exporter"], body) + pulp_export_factory(other_exporter, body) assert e.value.status == 400 msgs = json.loads(e.value.body) assert "versions" in msgs @@ -391,72 +474,3 @@ def test_cross_domain_exporter( assert msgs["start_versions"] == [ "Requested RepositoryVersions must belong to the Repositories named by the Exporter!" ] - - -@pytest.mark.parallel -def test_export_with_meta(pulpcore_bindings, pulp_export_factory, full_pulp_exporter): - exporter = full_pulp_exporter - user_meta = { - "initiator": "ci", - "purpose": "export", - "checksum_type": "md5", # pulp should override only in TOC JSON - } - - export = pulp_export_factory(exporter, {"meta": user_meta}) - - # toc_info contains exactly user meta (unmodified) - meta_info = export.toc_info.get("meta", {}) - assert meta_info == user_meta - - # Validate TOC JSON file content - toc_file_path = export.toc_info.get("file") - assert toc_file_path and isinstance(toc_file_path, str) - - with open(toc_file_path, "r") as f: - toc_data = json.load(f) - - meta_json = toc_data.get("meta", {}) - assert meta_json.get("initiator") == "ci" - assert meta_json.get("purpose") == "export" - # overridden field check - assert meta_json.get("checksum_type") == "crc32" - - -@pytest.mark.parallel -def test_export_chunk_ordering_and_naming( - pulp_exporter_factory, - pulp_export_factory, - three_synced_repositories, -): - exporter = pulp_exporter_factory(repositories=[three_synced_repositories[0]]) - chunk_size_bytes = 100 - body = {"chunk_size": f"{chunk_size_bytes}B"} - export = pulp_export_factory(exporter, body) - - all_paths = [Path(p) for p in export.output_file_info.keys()] - tar_chunks = [p for p in all_paths if ".tar." in p.name] - - assert len(tar_chunks) > 1, f"Expected multiple chunks for {chunk_size_bytes}B limit." - - for index, path in enumerate(tar_chunks): - expected_suffix = f"{index:04d}" - - assert path.name.endswith(expected_suffix), f"Chunk {path} missing suffix {expected_suffix}" - assert path.exists(), f"Chunk file {path} was not found on disk." - - if index < len(tar_chunks) - 1: - assert path.stat().st_size == chunk_size_bytes - - toc_path = Path(export.toc_info["file"]) - with toc_path.open("r", encoding="utf-8") as f: - toc_data = json.load(f) - - toc_filenames = list(toc_data["files"].keys()) - expected_filenames = [p.name for p in tar_chunks] - - assert toc_filenames == expected_filenames, ( - f"TOC order mismatch.\nExpected: {expected_filenames}\nActual: {toc_filenames}" - ) - - assert toc_data["meta"]["chunk_size"] == chunk_size_bytes - assert toc_data["meta"]["checksum_type"] == "crc32" diff --git a/pulpcore/app/management/commands/task-queue-stats.py b/pulpcore/app/management/commands/task-queue-stats.py new file mode 100644 index 00000000000..ecab536ef22 --- /dev/null +++ b/pulpcore/app/management/commands/task-queue-stats.py @@ -0,0 +1,253 @@ +""" +Report task queue wait statistics from `unblocked_at` / `started_at`. + +Worker-queue wait is `started_at - unblocked_at`: the task was ready to run +(resources free) but no worker had picked it up yet. Resource wait is +`unblocked_at - pulp_created`. + +Only meaningful for `WORKER_TYPE=pulpcore`. Redis workers do not use the +unblock mechanism, so those fields are not a reliable congestion signal there. +""" + +import json +from datetime import timedelta +from gettext import gettext as _ + +from django.conf import settings +from django.core.management import BaseCommand +from django.db.models import F, FloatField, Func +from django.utils import timezone +from django.utils.dateparse import parse_datetime + +from pulpcore.app.models import AppStatus, Task +from pulpcore.constants import TASK_STATES + + +class EpochSeconds(Func): + """PostgreSQL `EXTRACT(EPOCH FROM …)` as a float (subsecond precision).""" + + function = "EXTRACT" + template = "%(function)s(EPOCH FROM %(expressions)s)" + output_field = FloatField() + + +def _percentile(sorted_values, pct): + if not sorted_values: + return None + if len(sorted_values) == 1: + return sorted_values[0] + idx = min(len(sorted_values) - 1, max(0, round(pct / 100 * (len(sorted_values) - 1)))) + return sorted_values[idx] + + +def _fmt_seconds(value): + if value is None: + return "n/a" + return f"{value:.3f}s" + + +def _summarize(values): + if not values: + return {"n": 0, "mean": None, "p50": None, "p90": None, "p99": None, "max": None} + return { + "n": len(values), + "mean": sum(values) / len(values), + "p50": _percentile(values, 50), + "p90": _percentile(values, 90), + "p99": _percentile(values, 99), + "max": values[-1], + } + + +class Command(BaseCommand): + help = _("Summarize how long completed tasks waited for a worker after becoming unblocked.") + + def add_arguments(self, parser): + parser.add_argument( + "--hours", + type=float, + default=None, + help=_("Only include tasks created in the last N hours."), + ) + parser.add_argument( + "--since", + type=str, + default=None, + help=_("Only include tasks created at or after this ISO-8601 timestamp."), + ) + parser.add_argument( + "--top", + type=int, + default=15, + help=_("Show the N task names with the highest mean worker wait (default: 15)."), + ) + parser.add_argument( + "--min-worker-wait", + type=float, + default=0.0, + help=_("Only include tasks whose worker wait is at least this many seconds."), + ) + parser.add_argument( + "--json", + action="store_true", + help=_("Emit machine-readable JSON instead of a text report."), + ) + + def handle(self, *args, **options): + worker_type = getattr(settings, "WORKER_TYPE", "pulpcore") + online_workers = AppStatus.objects.online().filter(app_type="worker").count() + + qs = Task.objects.filter( + state=TASK_STATES.COMPLETED, + unblocked_at__isnull=False, + started_at__isnull=False, + finished_at__isnull=False, + ) + + since = None + if options["since"]: + since = parse_datetime(options["since"]) + if since is None: + self.stderr.write(self.style.ERROR(f"Invalid --since value: {options['since']}")) + return + if timezone.is_naive(since): + since = timezone.make_aware(since, timezone.get_current_timezone()) + elif options["hours"] is not None: + since = timezone.now() - timedelta(hours=options["hours"]) + + if since is not None: + qs = qs.filter(pulp_created__gte=since) + + qs = qs.annotate( + worker_wait=EpochSeconds(F("started_at") - F("unblocked_at")), + resource_wait=EpochSeconds(F("unblocked_at") - F("pulp_created")), + total_wait=EpochSeconds(F("started_at") - F("pulp_created")), + runtime=EpochSeconds(F("finished_at") - F("started_at")), + ) + + min_wait = options["min_worker_wait"] + if min_wait: + qs = qs.filter(worker_wait__gte=min_wait) + + rows = list(qs.values_list("name", "worker_wait", "resource_wait", "total_wait", "runtime")) + + worker_waits = sorted(r[1] for r in rows) + resource_waits = sorted(r[2] for r in rows) + total_waits = sorted(r[3] for r in rows) + runtimes = sorted(r[4] for r in rows) + + by_name = {} + for name, worker_wait, resource_wait, total_wait, runtime in rows: + bucket = by_name.setdefault( + name, {"worker": [], "resource": [], "total": [], "runtime": []} + ) + bucket["worker"].append(worker_wait) + bucket["resource"].append(resource_wait) + bucket["total"].append(total_wait) + bucket["runtime"].append(runtime) + + top_n = options["top"] + top_names = sorted( + ( + { + "name": name, + "n": len(stats["worker"]), + "worker_wait": _summarize(sorted(stats["worker"])), + "resource_wait": _summarize(sorted(stats["resource"])), + "total_wait": _summarize(sorted(stats["total"])), + "runtime": _summarize(sorted(stats["runtime"])), + } + for name, stats in by_name.items() + ), + key=lambda item: ( + item["worker_wait"]["mean"] if item["worker_wait"]["mean"] is not None else -1, + item["n"], + ), + reverse=True, + )[:top_n] + + null_unblocked = Task.objects.filter(state=TASK_STATES.COMPLETED, unblocked_at__isnull=True) + if since is not None: + null_unblocked = null_unblocked.filter(pulp_created__gte=since) + null_unblocked_count = null_unblocked.count() + + currently_waiting_unblocked = None + currently_waiting_blocked = None + if worker_type == "pulpcore": + currently_waiting_unblocked = Task.objects.filter( + state=TASK_STATES.WAITING, unblocked_at__isnull=False + ).count() + currently_waiting_blocked = Task.objects.filter( + state=TASK_STATES.WAITING, unblocked_at__isnull=True + ).count() + + report = { + "worker_type": worker_type, + "online_workers": online_workers, + "since": since.isoformat() if since else None, + "min_worker_wait": min_wait, + "completed_with_null_unblocked_at": null_unblocked_count, + "currently_waiting_unblocked": currently_waiting_unblocked, + "currently_waiting_blocked": currently_waiting_blocked, + "worker_wait": _summarize(worker_waits), + "resource_wait": _summarize(resource_waits), + "total_wait": _summarize(total_waits), + "runtime": _summarize(runtimes), + "top_by_mean_worker_wait": top_names, + "notes": [], + } + + if worker_type != "pulpcore": + report["notes"].append( + "WORKER_TYPE is not pulpcore; unblocked_at is not maintained by Redis workers, " + "so worker_wait is not a reliable congestion signal." + ) + if null_unblocked_count: + report["notes"].append( + f"{null_unblocked_count} completed task(s) have null unblocked_at " + "(common under Redis workers)." + ) + + if options["json"]: + self.stdout.write(json.dumps(report, indent=2, default=str)) + return + + self.stdout.write("Task queue wait stats") + self.stdout.write(f" worker_type: {worker_type}") + self.stdout.write(f" online_workers: {online_workers}") + self.stdout.write(f" since: {since.isoformat() if since else 'all completed tasks'}") + if min_wait: + self.stdout.write(f" min_worker_wait filter: {min_wait}s") + self.stdout.write( + f" currently waiting (unblocked/blocked): " + f"{currently_waiting_unblocked}/{currently_waiting_blocked}" + ) + self.stdout.write(f" completed with null unblocked_at: {null_unblocked_count}") + self.stdout.write("") + + def print_summary(label, summary): + self.stdout.write( + f"{label}: n={summary['n']} mean={_fmt_seconds(summary['mean'])} " + f"p50={_fmt_seconds(summary['p50'])} p90={_fmt_seconds(summary['p90'])} " + f"p99={_fmt_seconds(summary['p99'])} max={_fmt_seconds(summary['max'])}" + ) + + print_summary("WORKER_WAIT (started_at - unblocked_at)", report["worker_wait"]) + print_summary("RESOURCE_WAIT(unblocked_at - pulp_created)", report["resource_wait"]) + print_summary("TOTAL_WAIT (started_at - pulp_created)", report["total_wait"]) + print_summary("RUNTIME (finished_at - started_at)", report["runtime"]) + + if top_names: + self.stdout.write("") + self.stdout.write(f"Top {len(top_names)} task names by mean worker wait:") + for item in top_names: + ww = item["worker_wait"] + short = item["name"].rsplit(".", 1)[-1] + self.stdout.write( + f" mean={_fmt_seconds(ww['mean']):>8} p90={_fmt_seconds(ww['p90']):>8} " + f"max={_fmt_seconds(ww['max']):>8} n={ww['n']:<5} {short} ({item['name']})" + ) + + for note in report["notes"]: + self.stdout.write("") + self.stdout.write(self.style.WARNING(f"Note: {note}")) diff --git a/pulpcore/tests/functional/api/test_api_root_rewrite.py b/pulpcore/tests/functional/api/test_api_root_rewrite.py index 69c8acc17ce..11cc557f62a 100644 --- a/pulpcore/tests/functional/api/test_api_root_rewrite.py +++ b/pulpcore/tests/functional/api/test_api_root_rewrite.py @@ -1,6 +1,8 @@ +import asyncio import json import uuid +import aiohttp import pytest """ @@ -49,14 +51,24 @@ def auth_headers(bindings): def test_list_endpoints(file_bindings, proxy_rewrite_set, pulp_api_v3_path): """Check that ALL rewritten API_ROOT endpoints are accessible.""" API_ROOT = pulp_api_v3_path.encode("utf-8") - for endpoint, url in proxy_rewrite_set.items(): - headers = auth_headers(file_bindings) - response = file_bindings.client.rest_client.request("GET", url, headers=headers) - assert response.status == 200 + headers = auth_headers(file_bindings) + + async def fetch_all(): + async with aiohttp.ClientSession(headers=headers) as session: + + async def get_one(endpoint, url): + async with session.get(url, params={"limit": 1}, ssl=False) as response: + return endpoint, url, response.status, await response.read() + + return await asyncio.gather( + *(get_one(endpoint, url) for endpoint, url in proxy_rewrite_set.items()) + ) + for endpoint, url, status, body in asyncio.run(fetch_all()): + assert status == 200, f"failed on {endpoint}:{url}" if endpoint != "tasks": # Tasks reserved resources can have original API_ROOT - assert API_ROOT not in response.response.data, f"failed on {endpoint}:{url}" + assert API_ROOT not in body, f"failed on {endpoint}:{url}" @pytest.mark.parallel diff --git a/pulpcore/tests/functional/api/test_replication.py b/pulpcore/tests/functional/api/test_replication.py index 17ab460363f..f208a4be082 100644 --- a/pulpcore/tests/functional/api/test_replication.py +++ b/pulpcore/tests/functional/api/test_replication.py @@ -293,21 +293,26 @@ def test_replication_with_repo_based_distribution( gen_object_with_cleanup, file_distribution_factory, file_repository_factory, - file_remote_factory, - basic_manifest_path, add_domain_objects_to_cleanup, + tmp_path, ): """Test replication when upstream distribution uses repository (not publication).""" source_domain = domain_factory() add_domain_objects_to_cleanup(source_domain) - # Create a repo, sync it w/ mirror=True, and distribute via repository (not publication) - remote = file_remote_factory( - pulp_domain=source_domain.name, manifest_path=basic_manifest_path, policy="immediate" + src = tmp_path / "file.txt" + src.write_text("repo-based") + content_href = file_bindings.ContentFilesApi.upload( + relative_path="file.txt", + file=str(src), + pulp_domain=source_domain.name, + ).pulp_href + repo = file_repository_factory(pulp_domain=source_domain.name, autopublish=True) + monitor_task( + file_bindings.RepositoriesFileApi.modify( + repo.pulp_href, {"add_content_units": [content_href]} + ).task ) - repo = file_repository_factory(pulp_domain=source_domain.name) - sync_data = file_bindings.module.FileRepositorySyncURL(remote=remote.pulp_href, mirror=True) - monitor_task(file_bindings.RepositoriesFileApi.sync(repo.pulp_href, sync_data).task) _ = file_distribution_factory(pulp_domain=source_domain.name, repository=repo.pulp_href) # Replicate @@ -369,22 +374,28 @@ def test_replication_multi_distribution_content_update( source_domain = domain_factory() add_domain_objects_to_cleanup(source_domain) - # Create 3 repos with content and publication-based distributions + # Create 2 repos with content and publication-based distributions distros = [] repos = [] - for i in range(3): + modify_tasks = [] + for i in range(2): repo = file_repository_factory(pulp_domain=source_domain.name) repos.append(repo) file_path = tmp_path / f"file_{i}.txt" file_path.write_text(f"content_{i}") - monitor_task( - file_bindings.ContentFilesApi.create( - file=str(file_path), - relative_path=f"file_{i}.txt", - repository=repo.pulp_href, - pulp_domain=source_domain.name, + content_href = file_bindings.ContentFilesApi.upload( + file=str(file_path), + relative_path=f"file_{i}.txt", + pulp_domain=source_domain.name, + ).pulp_href + modify_tasks.append( + file_bindings.RepositoriesFileApi.modify( + repo.pulp_href, {"add_content_units": [content_href]} ).task ) + for task in modify_tasks: + monitor_task(task) + for repo in repos: pub = file_publication_factory(pulp_domain=source_domain.name, repository=repo.pulp_href) distros.append( file_distribution_factory(pulp_domain=source_domain.name, publication=pub.pulp_href) @@ -414,7 +425,7 @@ def test_replication_multi_distribution_content_update( replica_distros = file_bindings.DistributionsFileApi.list( pulp_domain=replica_domain.name ).results - assert len(replica_distros) == 3 + assert len(replica_distros) == 2 initial_versions = {} for rd in replica_distros: assert rd.repository is None @@ -423,17 +434,23 @@ def test_replication_multi_distribution_content_update( initial_versions[rd.name] = rd.repository_version # Add new content to all source repos and update publications + modify_tasks = [] for i, repo in enumerate(repos): file_path = tmp_path / f"file_{i}_v2.txt" file_path.write_text(f"new_content_{i}") - monitor_task( - file_bindings.ContentFilesApi.create( - file=str(file_path), - relative_path=f"file_{i}_v2.txt", - repository=repo.pulp_href, - pulp_domain=source_domain.name, + content_href = file_bindings.ContentFilesApi.upload( + file=str(file_path), + relative_path=f"file_{i}_v2.txt", + pulp_domain=source_domain.name, + ).pulp_href + modify_tasks.append( + file_bindings.RepositoriesFileApi.modify( + repo.pulp_href, {"add_content_units": [content_href]} ).task ) + for task in modify_tasks: + monitor_task(task) + for i, repo in enumerate(repos): repo = file_bindings.RepositoriesFileApi.read(repo.pulp_href) pub = file_publication_factory( pulp_domain=source_domain.name, @@ -455,7 +472,7 @@ def test_replication_multi_distribution_content_update( replica_distros = file_bindings.DistributionsFileApi.list( pulp_domain=replica_domain.name ).results - assert len(replica_distros) == 3 + assert len(replica_distros) == 2 for rd in replica_distros: assert rd.repository is None assert rd.repository_version is not None @@ -561,10 +578,8 @@ def test_replication_optimization( pulp_settings, file_bindings, file_repository_factory, - file_remote_factory, file_distribution_factory, file_publication_factory, - basic_manifest_path, monitor_task, gen_object_with_cleanup, tmp_path, @@ -583,19 +598,20 @@ def test_replication_optimization( pulpcore_bindings.UpstreamPulpsApi, upstream_pulp_body, pulp_domain=non_default_domain.name ) - # sync a repository on the "remote" Pulp instance - upstream_remote = file_remote_factory( - pulp_domain=source_domain.name, manifest_path=basic_manifest_path, policy="immediate" - ) + # One content unit on the "remote" Pulp instance is enough to test skip-sync + src = tmp_path / "file.txt" + src.write_text("replica") + content_href = file_bindings.ContentFilesApi.upload( + relative_path="file.txt", + file=str(src), + pulp_domain=source_domain.name, + ).pulp_href upstream_repository = file_repository_factory(pulp_domain=source_domain.name) - - repository_sync_data = file_bindings.module.FileRepositorySyncURL( - remote=upstream_remote.pulp_href, mirror=True - ) - response = file_bindings.RepositoriesFileApi.sync( - upstream_repository.pulp_href, repository_sync_data + monitor_task( + file_bindings.RepositoriesFileApi.modify( + upstream_repository.pulp_href, {"add_content_units": [content_href]} + ).task ) - monitor_task(response.task) upstream_repository = file_bindings.RepositoriesFileApi.read(upstream_repository.pulp_href) upstream_publication = file_publication_factory( pulp_domain=source_domain.name, repository_version=upstream_repository.latest_version_href @@ -1057,23 +1073,29 @@ def populate_upstream( domain_factory, file_bindings, file_repository_factory, - file_remote_factory, file_distribution_factory, - write_3_iso_file_fixture_data_factory, monitor_task, + tmp_path, ): def _populate_upstream(number, prefix=""): upstream_domain = domain_factory() + src = tmp_path / f"{uuid.uuid4()}.txt" + src.write_text("replica") + content_href = file_bindings.ContentFilesApi.upload( + relative_path="file.txt", + file=str(src), + pulp_domain=upstream_domain.name, + ).pulp_href tasks = [] for i in range(number): repo = file_repository_factory(pulp_domain=upstream_domain.name, autopublish=True) - name = f"{prefix}{i}" - fix = write_3_iso_file_fixture_data_factory(name) - remote = file_remote_factory(pulp_domain=upstream_domain.name, manifest_path=fix) - body = {"remote": remote.pulp_href} - tasks.append(file_bindings.RepositoriesFileApi.sync(repo.pulp_href, body).task) + tasks.append( + file_bindings.RepositoriesFileApi.modify( + repo.pulp_href, {"add_content_units": [content_href]} + ).task + ) file_distribution_factory( - name=name, + name=f"{prefix}{i}", pulp_domain=upstream_domain.name, repository=repo.pulp_href, pulp_labels={"upstream": str(i), "even" if i % 2 == 0 else "odd": ""}, @@ -1097,7 +1119,7 @@ def test_replicate_with_basic_q_select( add_domain_objects_to_cleanup, ): """Test basic label select replication.""" - source_domain = populate_upstream(6) + source_domain = populate_upstream(4) dest_domain = domain_factory() upstream_body = { "name": str(uuid.uuid4()), @@ -1110,14 +1132,14 @@ def test_replicate_with_basic_q_select( upstream = gen_object_with_cleanup( pulpcore_bindings.UpstreamPulpsApi, upstream_body, pulp_domain=dest_domain.name ) - # Run the replicate task and assert that all 6 repos got synced + # Run the replicate task and assert that all repos got synced response = pulpcore_bindings.UpstreamPulpsApi.replicate( upstream.pulp_href, pulpcore_bindings.module.UpstreamPulpReplicate() ) monitor_task_group(response.task_group) add_domain_objects_to_cleanup(dest_domain) result = pulpcore_bindings.DistributionsApi.list(pulp_domain=dest_domain.name) - assert result.count == 6 + assert result.count == 4 # Update q_select to sync only 'even' repos body = {"q_select": "pulp_label_select='even'"} @@ -1127,11 +1149,11 @@ def test_replicate_with_basic_q_select( ) monitor_task_group(response.task_group) result = pulpcore_bindings.DistributionsApi.list(pulp_domain=dest_domain.name) - assert result.count == 3 - assert {d.name for d in result.results} == {"0", "2", "4"} + assert result.count == 2 + assert {d.name for d in result.results} == {"0", "2"} # Update q_select to sync one 'upstream' repo - body["q_select"] = "pulp_label_select='upstream=4'" + body["q_select"] = "pulp_label_select='upstream=2'" pulpcore_bindings.UpstreamPulpsApi.partial_update(upstream.pulp_href, body) response = pulpcore_bindings.UpstreamPulpsApi.replicate( upstream.pulp_href, pulpcore_bindings.module.UpstreamPulpReplicate() @@ -1139,7 +1161,7 @@ def test_replicate_with_basic_q_select( monitor_task_group(response.task_group) result = pulpcore_bindings.DistributionsApi.list(pulp_domain=dest_domain.name) assert result.count == 1 - assert result.results[0].name == "4" + assert result.results[0].name == "2" # Show that basic label select is ANDed together body["q_select"] = "pulp_label_select='even,upstream=0'" @@ -1165,7 +1187,7 @@ def test_replicate_with_per_request_q_select( add_domain_objects_to_cleanup, ): """Test that q_select can be passed per-request to the replicate action.""" - source_domain = populate_upstream(6) + source_domain = populate_upstream(4) dest_domain = domain_factory() add_domain_objects_to_cleanup(dest_domain) @@ -1192,8 +1214,8 @@ def test_replicate_with_per_request_q_select( ) monitor_task_group(response.task_group) result = pulpcore_bindings.DistributionsApi.list(pulp_domain=dest_domain.name) - assert result.count == 3 - assert {d.name for d in result.results} == {"0", "2", "4"} + assert result.count == 2 + assert {d.name for d in result.results} == {"0", "2"} # Selective replicate of 'odd' should NOT delete the 'even' ones (remove_missing skipped) replicate_body = pulpcore_bindings.module.UpstreamPulpReplicate( @@ -1204,8 +1226,8 @@ def test_replicate_with_per_request_q_select( ) monitor_task_group(response.task_group) result = pulpcore_bindings.DistributionsApi.list(pulp_domain=dest_domain.name) - assert result.count == 6 - assert {d.name for d in result.results} == {"0", "1", "2", "3", "4", "5"} + assert result.count == 4 + assert {d.name for d in result.results} == {"0", "1", "2", "3"} # Full replicate (no per-request q_select) should still work and run remove_missing response = pulpcore_bindings.UpstreamPulpsApi.replicate( @@ -1213,7 +1235,7 @@ def test_replicate_with_per_request_q_select( ) monitor_task_group(response.task_group) result = pulpcore_bindings.DistributionsApi.list(pulp_domain=dest_domain.name) - assert result.count == 6 + assert result.count == 4 @pytest.mark.parallel @@ -1228,7 +1250,7 @@ def test_replicate_with_complex_q_select( add_domain_objects_to_cleanup, ): """Test complex q_select replication.""" - source_domain = populate_upstream(6) + source_domain = populate_upstream(4) dest_domain = domain_factory() add_domain_objects_to_cleanup(dest_domain) upstream_body = { @@ -1252,16 +1274,16 @@ def test_replicate_with_complex_q_select( assert result.count == 2 assert {d.name for d in result.results} == {"1", "2"} - # Test odds but not five - body = {"q_select": "pulp_label_select='odd' AND NOT pulp_label_select='upstream=5'"} + # Test odds but not three + body = {"q_select": "pulp_label_select='odd' AND NOT pulp_label_select='upstream=3'"} pulpcore_bindings.UpstreamPulpsApi.partial_update(upstream.pulp_href, body) response = pulpcore_bindings.UpstreamPulpsApi.replicate( upstream.pulp_href, pulpcore_bindings.module.UpstreamPulpReplicate() ) monitor_task_group(response.task_group) result = pulpcore_bindings.DistributionsApi.list(pulp_domain=dest_domain.name) - assert result.count == 2 - assert {d.name for d in result.results} == {"1", "3"} + assert result.count == 1 + assert {d.name for d in result.results} == {"1"} # Test we error when trying to provide an invalid q expression body["q_select"] = "invalid='testing'" @@ -1301,9 +1323,9 @@ def _add_domain_to_cleanup(domain): @pytest.mark.parametrize( "policy,results", [ - ("nodelete", [{"b0", "b1", "a0", "a1", "a2"}, {"b0", "b1", "a0", "a1", "a2"}]), - ("labeled", [{"b0", "b1", "a0", "a1", "a2"}, {"b0", "b1", "a0"}]), - ("all", [{"a0", "a1", "a2"}, {"a0"}]), + ("nodelete", [{"b0", "a0", "a1"}, {"b0", "a0", "a1"}]), + ("labeled", [{"b0", "a0", "a1"}, {"b0", "a0"}]), + ("all", [{"a0", "a1"}, {"a0"}]), ], ) def test_replicate_policy( @@ -1320,8 +1342,8 @@ def test_replicate_policy( gen_object_with_cleanup, ): """Test replicate delete_policy.""" - a_domain = populate_upstream(3, prefix="a") - b_domain = populate_upstream(2, prefix="b") + a_domain = populate_upstream(2, prefix="a") + b_domain = populate_upstream(1, prefix="b") upstream_body = { "name": str(uuid.uuid4()), "base_url": bindings_cfg.host, @@ -1345,10 +1367,10 @@ def test_replicate_policy( assert result.count == len(results[0]) assert {r.name for r in result.results} == results[0] - # delete a1, a2 + # delete a1 result = pulpcore_bindings.DistributionsApi.list(pulp_domain=a_domain.name) - monitor_task(file_bindings.DistributionsFileApi.delete(result.results[0].pulp_href).task) - monitor_task(file_bindings.DistributionsFileApi.delete(result.results[1].pulp_href).task) + a1 = next(d for d in result.results if d.name == "a1") + monitor_task(file_bindings.DistributionsFileApi.delete(a1.pulp_href).task) result = pulpcore_bindings.DistributionsApi.list(pulp_domain=a_domain.name) assert result.count == 1 assert result.results[0].name == "a0" diff --git a/pulpcore/tests/functional/api/test_tasking.py b/pulpcore/tests/functional/api/test_tasking.py index d9e95a56a0c..4b8135882d4 100644 --- a/pulpcore/tests/functional/api/test_tasking.py +++ b/pulpcore/tests/functional/api/test_tasking.py @@ -9,7 +9,6 @@ import pytest from aiohttp import BasicAuth -from django.conf import settings from pulpcore.client.pulpcore import ApiException from pulpcore.constants import IMMEDIATE_TIMEOUT @@ -93,6 +92,10 @@ def test_worker_cleanup_on_missing_worker(dispatch_task, monitor_task, pulpcore_ Test that when a worker dies unexpectedly while executing a task, the worker cleanup process marks the task as failed and releases its locks, allowing subsequent tasks requiring the same resource to execute. + + Prefer the unit test pulpcore.tests.unit.tasking.test_missing_worker_cleanup + for routine coverage of the cleanup path. This e2e test is long_running + (skipped when --timeout < 600) but still runs in nightly CI. """ # Use a unique resource identifier to avoid conflicts with other tests resource = str(uuid4()) @@ -731,54 +734,3 @@ def test_times_out_on_task_worker( ) monitor_task(task_href) assert "timed out after" in ctx.value.task.error["description"] - - -@pytest.mark.parallel -@pytest.mark.skipif( - settings.WORKER_TYPE != "redis", - reason="Only runs with WORKER_TYPE=redis", -) -def test_fetch_task_beyond_initial_batch(dispatch_task, monitor_task, pulpcore_bindings): - """Test that tasks beyond the initial fetch batch are still processed. - - When more than FETCH_TASK_LIMIT tasks are blocked on the same exclusive resource, - the RedisWorker should double the fetch limit and find runnable tasks further - down the queue. - """ - blocker_resource = str(uuid4()) - other_resource = str(uuid4()) - - # Dispatch a long-running task that holds the blocker resource - blocker_href = dispatch_task( - "pulpcore.app.tasks.test.sleep", - args=(60,), - exclusive_resources=[blocker_resource], - ) - time.sleep(2) - - # Dispatch 25 tasks that all need the same blocked resource - blocked_hrefs = [] - for _ in range(25): - href = dispatch_task( - "pulpcore.app.tasks.test.sleep", - args=(0,), - exclusive_resources=[blocker_resource], - ) - blocked_hrefs.append(href) - - # Dispatch a task that uses a completely different resource (position 27 in the queue) - unblocked_href = dispatch_task( - "pulpcore.app.tasks.test.sleep", - args=(0,), - exclusive_resources=[other_resource], - ) - - # The unblocked task should complete even though 25 tasks ahead of it are blocked - unblocked_task = monitor_task(unblocked_href) - assert unblocked_task.state == "completed" - - # Cancel the blocker so blocked tasks can drain - try: - pulpcore_bindings.TasksApi.tasks_cancel(blocker_href, {"state": "canceled"}) - except ApiException: - pass diff --git a/pulpcore/tests/functional/api/using_plugin/test_checkpoint.py b/pulpcore/tests/functional/api/using_plugin/test_checkpoint.py index b3844d7e699..305b650071f 100644 --- a/pulpcore/tests/functional/api/using_plugin/test_checkpoint.py +++ b/pulpcore/tests/functional/api/using_plugin/test_checkpoint.py @@ -2,7 +2,7 @@ import re import uuid -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from time import sleep from urllib.parse import urlparse @@ -13,25 +13,47 @@ from pulpcore.content.handler import Handler +def _wait_until_checkpoint_ts_advances(previous_created): + """Block until wall-clock formats to a later checkpoint timestamp than previous_created. + + Checkpoint URLs are second-resolution, so consecutive publications need distinct + seconds. Fixed sleep(1) is wasteful when create/publish already crossed a second. + """ + previous_ts = Handler._format_checkpoint_timestamp(previous_created) + while Handler._format_checkpoint_timestamp(datetime.now(timezone.utc)) == previous_ts: + sleep(0.05) + + @pytest.fixture(scope="class") -def content_factory(tmp_path_factory, file_bindings, monitor_task): +def content_factory(tmp_path_factory, file_bindings): def _content_factory(name): file = tmp_path_factory.mktemp("content") / name file.write_text(str(uuid.uuid4())) - return monitor_task( - file_bindings.ContentFilesApi.create(relative_path=name, file=str(file)).task - ).created_resources[0] + return file_bindings.ContentFilesApi.upload(relative_path=name, file=str(file)).pulp_href + + def _precreate(names): + return [_content_factory(name) for name in names] + _content_factory.precreate = _precreate return _content_factory @pytest.fixture(scope="class") def create_publication(content_factory, file_bindings, monitor_task): counter = [0] + content_queue = [] + + def precreate(n): + names = [] + for _ in range(n): + names.append(str(counter[0])) + counter[0] += 1 + content_queue.extend(content_factory.precreate(names)) def _create_publication(repo, checkpoint): - content_href = content_factory(f"{counter[0]}") - counter[0] += 1 + if not content_queue: + precreate(1) + content_href = content_queue.pop(0) monitor_task( file_bindings.RepositoriesFileApi.modify( @@ -46,6 +68,7 @@ def _create_publication(repo, checkpoint): ) return file_bindings.PublicationsFileApi.read(response.created_resources[0]) + _create_publication.precreate = precreate return _create_publication @@ -58,16 +81,14 @@ def setup( repo = file_repository_factory() distribution = file_distribution_factory(repository=repo.pulp_href, checkpoint=True) + # Five publications: content creates overlap; only wait between pubs when needed + # for distinct second-resolution checkpoint timestamps. + create_publication.precreate(5) pubs = [] - pubs.append(create_publication(repo, False)) - sleep(1) - pubs.append(create_publication(repo, True)) - sleep(1) - pubs.append(create_publication(repo, False)) - sleep(1) - pubs.append(create_publication(repo, True)) - sleep(1) - pubs.append(create_publication(repo, False)) + for checkpoint in (False, True, False, True, False): + if pubs: + _wait_until_checkpoint_ts_advances(pubs[-1].pulp_created) + pubs.append(create_publication(repo, checkpoint)) return pubs, distribution @@ -82,7 +103,8 @@ def _checkpoint_url(distribution, timestamp): class TestCheckpointDistribution: - @pytest.mark.parallel + """Don't mark parallel, tests are shorter than setup.""" + def test_base_path_lists_checkpoints(self, setup, http_get, distribution_base_url): pubs, distribution = setup @@ -93,7 +115,6 @@ def test_base_path_lists_checkpoints(self, setup, http_get, distribution_base_ur assert Handler._format_checkpoint_timestamp(pubs[1].pulp_created) in checkpoints_ts assert Handler._format_checkpoint_timestamp(pubs[3].pulp_created) in checkpoints_ts - @pytest.mark.parallel def test_distro_root_no_trailing_slash_is_redirected( self, setup, @@ -112,7 +133,6 @@ def test_distro_root_no_trailing_slash_is_redirected( assert Handler._format_checkpoint_timestamp(pubs[1].pulp_created) in checkpoints_ts assert Handler._format_checkpoint_timestamp(pubs[3].pulp_created) in checkpoints_ts - @pytest.mark.parallel def test_timestamped_checkpoint_no_trailing_slash_is_redirected( self, setup, @@ -128,7 +148,6 @@ def test_timestamped_checkpoint_no_trailing_slash_is_redirected( assert f"