Skip to content

Commit d8279a7

Browse files
Guylain Lavoieggainey
authored andcommitted
Allow filesystem export of streamed content from file:// remotes
Filesystem export rejected any repository whose content was synced with a streamed policy from a file:// remote: with no downloaded Artifact, every ContentArtifact has artifact=None, so _validate_fs_export raised UnexportableArtifactException even though the bytes were available on the local filesystem. Read the bytes from the file:// local path when a ContentArtifact has no downloaded Artifact, for both publication and repository-version filesystem exports. closes #7746 Assisted-By: Claude (Anthropic)
1 parent c5ad4ef commit d8279a7

3 files changed

Lines changed: 165 additions & 11 deletions

File tree

CHANGES/7746.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Enabled filesystem export of content synced with a streamed policy from a file:// remote, by reading the files from their local path instead of failing because no Artifact was downloaded.

pulpcore/app/tasks/export.py

Lines changed: 119 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
import logging
33
import os
44
import os.path
5+
import shutil
56
import tarfile
67
from gettext import gettext as _
78
from glob import glob
89
from pathlib import Path
10+
from urllib.parse import unquote, urlparse
911

1012
from django.conf import settings
1113

@@ -27,7 +29,7 @@
2729
RepositoryVersion,
2830
Task,
2931
)
30-
from pulpcore.app.models.content import ContentArtifact
32+
from pulpcore.app.models.content import ContentArtifact, RemoteArtifact
3133
from pulpcore.app.serializers import PulpExportSerializer
3234
from pulpcore.app.util import Crc32Hasher, HashingFileWriter, compute_file_hash
3335
from pulpcore.constants import FS_EXPORT_METHODS
@@ -65,9 +67,92 @@ def _validate_fs_export(content_artifacts):
6567
Raises:
6668
RuntimeError: If Artifacts are not downloaded or when trying to link non-fs files
6769
"""
68-
if content_artifacts.filter(artifact=None).exists():
70+
missing_ca_qs = content_artifacts.filter(artifact=None)
71+
if not missing_ca_qs.exists():
72+
return
73+
74+
# Allow streamed content whose remote URL is a local file:// path (e.g. an rsync'd
75+
# local mirror). Those artifacts can be exported directly even though they were
76+
# never downloaded into Pulp storage.
77+
missing_pks = set(missing_ca_qs.values_list("pk", flat=True))
78+
79+
# Reject if any missing artifact has a non-file:// remote (truly needs downloading).
80+
if (
81+
RemoteArtifact.objects.filter(content_artifact__pk__in=missing_pks)
82+
.exclude(url__startswith="file://")
83+
.exists()
84+
):
6985
raise UnexportableArtifactException()
7086

87+
# Reject if any missing artifact has no remote at all (nothing to fall back to).
88+
ca_pks_with_remote = set(
89+
RemoteArtifact.objects.filter(content_artifact__pk__in=missing_pks).values_list(
90+
"content_artifact_id", flat=True
91+
)
92+
)
93+
if missing_pks - ca_pks_with_remote:
94+
raise UnexportableArtifactException()
95+
96+
97+
def _local_path_from_file_url(url):
98+
"""Convert a file:// URL to an absolute local filesystem path."""
99+
parsed = urlparse(url)
100+
return os.path.abspath(os.path.join(parsed.netloc, unquote(parsed.path)))
101+
102+
103+
def _file_url_local_paths(content_artifacts):
104+
"""Map ContentArtifact pk -> local path for missing artifacts backed by file:// remotes.
105+
106+
Returns an empty dict when there is no such content. Lets streamed content whose
107+
bytes live on the local filesystem be exported without a downloaded Artifact.
108+
"""
109+
missing_ca_pks = set(content_artifacts.filter(artifact=None).values_list("pk", flat=True))
110+
if not missing_ca_pks:
111+
return {}
112+
113+
ca_pk_to_local_path = {}
114+
for ra in RemoteArtifact.objects.filter(
115+
content_artifact__pk__in=missing_ca_pks,
116+
url__startswith="file://",
117+
).values("content_artifact_id", "url"):
118+
ca_pk_to_local_path[ra["content_artifact_id"]] = _local_path_from_file_url(ra["url"])
119+
return ca_pk_to_local_path
120+
121+
122+
def _export_local_to_file_system(
123+
path, relative_paths_to_local_paths, method=FS_EXPORT_METHODS.WRITE
124+
):
125+
"""
126+
Export artifacts backed by file:// remotes directly from their local path.
127+
128+
Used for streamed content that has no downloaded Artifact but whose bytes are
129+
available locally. The base export directory must already exist (created by
130+
_export_to_file_system).
131+
132+
Args:
133+
path (str): The already-created export root directory.
134+
relative_paths_to_local_paths: A dict with {relative_path: local_fs_path} mapping.
135+
method: FS_EXPORT_METHODS constant (WRITE, SYMLINK, or HARDLINK).
136+
"""
137+
for relative_path, src_path in relative_paths_to_local_paths.items():
138+
dest = os.path.join(path, relative_path)
139+
os.makedirs(os.path.split(dest)[0], exist_ok=True)
140+
141+
if method == FS_EXPORT_METHODS.SYMLINK:
142+
os.path.lexists(dest) and os.unlink(dest)
143+
os.symlink(src_path, dest)
144+
elif method == FS_EXPORT_METHODS.HARDLINK:
145+
os.path.lexists(dest) and os.unlink(dest)
146+
try:
147+
os.link(src_path, dest)
148+
except OSError:
149+
log.info(_("Hard link cannot be created, file will be copied."))
150+
shutil.copy2(src_path, dest)
151+
elif method == FS_EXPORT_METHODS.WRITE:
152+
shutil.copy2(src_path, dest)
153+
else:
154+
raise RuntimeError(_("Unsupported export method '{}'.").format(method))
155+
71156

72157
def _export_to_file_system(path, relative_paths_to_artifacts, method=FS_EXPORT_METHODS.WRITE):
73158
"""
@@ -151,29 +236,42 @@ def _export_publication_to_file_system(
151236
)
152237
)
153238

239+
# Map ContentArtifact pk -> local path for any streamed file:// content, so it can
240+
# be exported directly even though it has no downloaded Artifact.
241+
ca_pk_to_local_path = _file_url_local_paths(content_artifacts)
242+
154243
relative_path_to_artifacts = {}
244+
relative_path_to_local_paths = {}
155245
if publication.pass_through:
156-
relative_path_to_artifacts = {
157-
ca.relative_path: ca.artifact
158-
for ca in content_artifacts.select_related("artifact").iterator()
159-
if (start_repo_version is None) or (ca.pk in difference_content_artifacts)
160-
}
246+
for ca in content_artifacts.select_related("artifact").iterator():
247+
if (start_repo_version is None) or (ca.pk in difference_content_artifacts):
248+
if ca.artifact:
249+
relative_path_to_artifacts[ca.relative_path] = ca.artifact
250+
elif ca.pk in ca_pk_to_local_path:
251+
relative_path_to_local_paths[ca.relative_path] = ca_pk_to_local_path[ca.pk]
161252

162253
publication_metadata_paths = set(
163254
publication.published_metadata.values_list("relative_path", flat=True)
164255
)
165256
for pa in publication.published_artifact.select_related(
166257
"content_artifact", "content_artifact__artifact"
167258
).iterator():
168-
# Artifact isn't guaranteed to be present
169-
if pa.content_artifact.artifact and (
259+
if (
170260
start_repo_version is None
171261
or pa.relative_path in publication_metadata_paths
172262
or pa.content_artifact.pk in difference_content_artifacts
173263
):
174-
relative_path_to_artifacts[pa.relative_path] = pa.content_artifact.artifact
264+
# Artifact isn't guaranteed to be present
265+
if pa.content_artifact.artifact:
266+
relative_path_to_artifacts[pa.relative_path] = pa.content_artifact.artifact
267+
elif pa.content_artifact.pk in ca_pk_to_local_path:
268+
relative_path_to_local_paths[pa.relative_path] = ca_pk_to_local_path[
269+
pa.content_artifact.pk
270+
]
175271

176272
_export_to_file_system(path, relative_path_to_artifacts, method)
273+
if relative_path_to_local_paths:
274+
_export_local_to_file_system(path, relative_path_to_local_paths, method)
177275

178276

179277
def _export_location_is_clean(path):
@@ -286,12 +384,22 @@ def fs_repo_version_export(exporter_pk, repo_version_pk, start_repo_version_pk=N
286384
)
287385
)
288386

387+
# Map ContentArtifact pk -> local path for any streamed file:// content, so it can
388+
# be exported directly even though it has no downloaded Artifact.
389+
ca_pk_to_local_path = _file_url_local_paths(content_artifacts)
390+
289391
relative_path_to_artifacts = {}
392+
relative_path_to_local_paths = {}
290393
for ca in content_artifacts.select_related("artifact").iterator():
291394
if start_repo_version is None or ca.pk in difference_content_artifacts:
292-
relative_path_to_artifacts[ca.relative_path] = ca.artifact
395+
if ca.artifact:
396+
relative_path_to_artifacts[ca.relative_path] = ca.artifact
397+
elif ca.pk in ca_pk_to_local_path:
398+
relative_path_to_local_paths[ca.relative_path] = ca_pk_to_local_path[ca.pk]
293399

294400
_export_to_file_system(exporter.path, relative_path_to_artifacts, exporter.method)
401+
if relative_path_to_local_paths:
402+
_export_local_to_file_system(exporter.path, relative_path_to_local_paths, exporter.method)
295403

296404

297405
def _get_versions_to_export(the_exporter, the_export):

pulpcore/tests/functional/api/using_plugin/test_filesystemexport.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,51 @@ def test_create_exporter_export(create_exporter, create_exporter_export, publica
140140
assert export is not None
141141

142142

143+
@pytest.mark.parallel
144+
def test_export_streamed_file_url_content(
145+
file_bindings,
146+
file_repository_factory,
147+
write_3_iso_file_fixture_data_factory,
148+
file_fixtures_root,
149+
gen_object_with_cleanup,
150+
monitor_task,
151+
create_exporter,
152+
create_exporter_export,
153+
):
154+
"""Export a publication whose content was streamed from a file:// remote."""
155+
# Freshly generated (random) content so its artifacts cannot already be in
156+
# storage from another immediate sync -- otherwise QueryExistingArtifacts
157+
# would attach them by digest and mask the bug.
158+
manifest = write_3_iso_file_fixture_data_factory(str(uuid.uuid4()))
159+
file_url = (file_fixtures_root / manifest.lstrip("/")).as_uri()
160+
161+
repo = file_repository_factory(autopublish=True)
162+
remote = gen_object_with_cleanup(
163+
file_bindings.RemotesFileApi,
164+
{"name": str(uuid.uuid4()), "url": file_url, "policy": "streamed"},
165+
)
166+
repository_sync_data = RepositorySyncURL(remote=remote.pulp_href)
167+
sync_response = file_bindings.RepositoriesFileApi.sync(repo.pulp_href, repository_sync_data)
168+
monitor_task(sync_response.task)
169+
repo = file_bindings.RepositoriesFileApi.read(repo.pulp_href)
170+
171+
# The precondition the export must handle: the 3 files are present, but none
172+
# has a downloaded artifact. Asserting this keeps the test honest -- if the
173+
# content were ever downloaded the export would succeed even unpatched.
174+
content = file_bindings.ContentFilesApi.list(repository_version=repo.latest_version_href)
175+
assert content.count == 3
176+
assert all(unit.artifact is None for unit in content.results)
177+
178+
publication = file_bindings.PublicationsFileApi.list(repository=repo.pulp_href).results[0]
179+
180+
# Without the fix this raises: the export task fails with
181+
# UnexportableArtifactException ("Cannot export artifacts that haven't been
182+
# downloaded"). With the fix the file:// bytes are read from disk.
183+
exporter, _ = create_exporter({"method": "write"})
184+
export = create_exporter_export(exporter, publication)
185+
assert export is not None
186+
187+
143188
@pytest.mark.parallel
144189
def test_list_exporter_exports(
145190
pulpcore_bindings,

0 commit comments

Comments
 (0)