Skip to content

Commit

Permalink
Fixed repository version repair with S3 and Azure storage
Browse files Browse the repository at this point in the history
This patch also adds repair API tests for S3 and Azure storage backends.

This patch also adds assertions that a repository version repair task can be run in a non-default
domain.

fixes: pulp#4776
fixes: pulp#4806
  • Loading branch information
dkliban committed Dec 4, 2023
1 parent 701bbc4 commit 4a820a9
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 30 deletions.
2 changes: 2 additions & 0 deletions 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.
2 changes: 2 additions & 0 deletions 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.
9 changes: 6 additions & 3 deletions 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
Expand Down Expand Up @@ -96,10 +97,12 @@ 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:
with storage.open(artifact.file.name) as fp:
for chunk in fp.chunks(CHUNK_SIZE):
hasher.update(chunk)
return hasher.hexdigest() == artifact.sha256
Expand Down Expand Up @@ -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()
Expand Down
10 changes: 10 additions & 0 deletions pulpcore/tests/functional/api/pulp_file/test_domains.py
Expand Up @@ -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


Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
34 changes: 7 additions & 27 deletions 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,
Expand All @@ -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


Expand Down

0 comments on commit 4a820a9

Please sign in to comment.