From 212217611e030c70fadff7c040977f6a15d6c87a Mon Sep 17 00:00:00 2001 From: Dennis Kliban Date: Thu, 30 Nov 2023 13:43:41 -0500 Subject: [PATCH] Fixed repository version repair with S3 and Azure storage 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 | 2 ++ CHANGES/4806.bugfix | 2 ++ pulpcore/app/tasks/repository.py | 7 ++-- .../functional/api/pulp_file/test_domains.py | 10 ++++++ .../api/using_plugin/test_repair.py | 34 ++++--------------- 5 files changed, 26 insertions(+), 29 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 00000000000..2be7924eefb --- /dev/null +++ b/CHANGES/4776.bugfix @@ -0,0 +1,2 @@ +Fixed a bug in the repository version repair API which would present itself when using Pulp with S3 +and Azure storage backends. diff --git a/CHANGES/4806.bugfix b/CHANGES/4806.bugfix new file mode 100644 index 00000000000..2be7924eefb --- /dev/null +++ b/CHANGES/4806.bugfix @@ -0,0 +1,2 @@ +Fixed a bug in the repository version repair API which would present itself when using Pulp with S3 +and Azure storage backends. diff --git a/pulpcore/app/tasks/repository.py b/pulpcore/app/tasks/repository.py index f949d07d290..815f8183360 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,8 +97,10 @@ async def _repair_ca(content_artifact, repaired=None): def _verify_artifact(artifact): + domain = get_domain() + storage = domain.get_storage() + assert domain.pk == artifact.pulp_domain_id try: - # verify files digest hasher = hashlib.sha256() with artifact.file as fp: for chunk in fp.chunks(CHUNK_SIZE): @@ -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 a2a6a259db7..78d3585cec1 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 bc6a43ae298..c100507a953 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