From 95a8f47cd54c8a8026cecc9e049d8356e398f130 Mon Sep 17 00:00:00 2001 From: Dennis Kliban Date: Thu, 30 Nov 2023 13:43:41 -0500 Subject: [PATCH] Fixes repository version repair API for non-default domains. The 'current_domain' ContextVar was not getting coppied into the thread that was running the artifact verification. The context is now explicitly copied into that thread. This patch also adds repair API tests for S3 and Azure storage backends. This patch also adds test assertions that a repository version repair task can be run in a non-default domain. fixes: #4776 fixes: #4806 --- CHANGES/4776.bugfix | 1 + CHANGES/4806.bugfix | 1 + pulpcore/app/tasks/repository.py | 5 ++- .../functional/api/pulp_file/test_domains.py | 10 ++++++ .../api/using_plugin/test_repair.py | 34 ++++--------------- 5 files changed, 23 insertions(+), 28 deletions(-) create mode 100644 CHANGES/4776.bugfix create mode 100644 CHANGES/4806.bugfix diff --git a/CHANGES/4776.bugfix b/CHANGES/4776.bugfix new file mode 100644 index 0000000000..77b91bfdeb --- /dev/null +++ b/CHANGES/4776.bugfix @@ -0,0 +1 @@ +Fixed a bug in the repository version repair API for non-default domains. diff --git a/CHANGES/4806.bugfix b/CHANGES/4806.bugfix new file mode 100644 index 0000000000..77b91bfdeb --- /dev/null +++ b/CHANGES/4806.bugfix @@ -0,0 +1 @@ +Fixed a bug in the repository version repair API for non-default domains. diff --git a/pulpcore/app/tasks/repository.py b/pulpcore/app/tasks/repository.py index f949d07d29..5cc7cff21f 100644 --- a/pulpcore/app/tasks/repository.py +++ b/pulpcore/app/tasks/repository.py @@ -1,4 +1,5 @@ from concurrent.futures import ThreadPoolExecutor +from contextvars import copy_context from gettext import gettext as _ from logging import getLogger import asyncio @@ -96,6 +97,8 @@ async def _repair_ca(content_artifact, repaired=None): def _verify_artifact(artifact): + domain = get_domain() + assert domain.pk == artifact.pulp_domain_id try: # verify files digest hasher = hashlib.sha256() @@ -144,7 +147,7 @@ async def _repair_artifacts_for_content(subset=None, verify_checksums=True): # Should stay in (an) executor so that at least it doesn't completely block # downloads. valid = await loop.run_in_executor( - checksum_executor, _verify_artifact, artifact + checksum_executor, copy_context().run, _verify_artifact, artifact ) if not valid: await corrupted.aincrement() diff --git a/pulpcore/tests/functional/api/pulp_file/test_domains.py b/pulpcore/tests/functional/api/pulp_file/test_domains.py index a2a6a259db..78d3585cec 100644 --- a/pulpcore/tests/functional/api/pulp_file/test_domains.py +++ b/pulpcore/tests/functional/api/pulp_file/test_domains.py @@ -5,6 +5,7 @@ from pulpcore.app import settings from pulpcore.client.pulp_file import ApiException from pulpcore.client.pulpcore import ApiException as CoreApiException +from pulpcore.client.pulpcore import Repair from pulpcore.tests.functional.utils import generate_iso, download_file @@ -180,6 +181,7 @@ def test_content_upload( def test_content_promotion( domains_api_client, file_repository_api_client, + file_repository_version_api_client, basic_manifest_path, file_remote_factory, file_publication_api_client, @@ -233,6 +235,14 @@ def test_content_promotion( assert download.response_obj.status == 200 assert len(download.body) == 1024 + # Test that a repository version repair operation can be run without error + response = file_repository_version_api_client.repair( + repo.latest_version_href, Repair(verify_checksums=True) + ) + results = monitor_task(response.task) + assert results.state == "completed" + assert results.error is None + # Cleanup to delete the domain task = file_repository_api_client.delete(repo.pulp_href).task monitor_task(task) diff --git a/pulpcore/tests/functional/api/using_plugin/test_repair.py b/pulpcore/tests/functional/api/using_plugin/test_repair.py index bc6a43ae29..c100507a95 100644 --- a/pulpcore/tests/functional/api/using_plugin/test_repair.py +++ b/pulpcore/tests/functional/api/using_plugin/test_repair.py @@ -1,29 +1,14 @@ import pytest -import os +from django.core.files.storage import default_storage from random import sample from pulpcore.client.pulpcore import Repair from pulpcore.client.pulp_file import RepositorySyncURL -from pulpcore.app import settings - from pulpcore.tests.functional.utils import get_files_in_manifest -SUPPORTED_STORAGE_FRAMEWORKS = [ - "django.core.files.storage.FileSystemStorage", - "pulpcore.app.models.storage.FileSystem", -] - -pytestmark = pytest.mark.skipif( - settings.DEFAULT_FILE_STORAGE not in SUPPORTED_STORAGE_FRAMEWORKS, - reason="Cannot simulate bit-rot on this storage platform ({}).".format( - settings.DEFAULT_FILE_STORAGE - ), -) - - @pytest.fixture def repository_with_corrupted_artifacts( file_repository_api_client, @@ -42,19 +27,14 @@ def repository_with_corrupted_artifacts( # STEP 2: sample artifacts that will be modified on the filesystem later on content1, content2 = sample(get_files_in_manifest(remote.url), 2) - # Modify one artifact on disk. - artifact1_path = os.path.join( - settings.MEDIA_ROOT, artifacts_api_client.list(sha256=content1[1]).results[0].file - ) - with open(artifact1_path, "r+b") as f: + # Modify an artifact + artifact1_path = artifacts_api_client.list(sha256=content1[1]).results[0].file + with default_storage.open(artifact1_path, "w+b") as f: f.write(b"$a bit rot") - # Delete another one from disk. - artifact2_path = os.path.join( - settings.MEDIA_ROOT, artifacts_api_client.list(sha256=content2[1]).results[0].file - ) - os.remove(artifact2_path) - + # Delete an artifact + artifact2_path = artifacts_api_client.list(sha256=content2[1]).results[0].file + default_storage.delete(artifact2_path) return repo