Skip to content

Commit

Permalink
Fix a multi-repo-copy bug
Browse files Browse the repository at this point in the history
Results of the copy accidentally depend on the order in which the user
specified the source-destination repository pairs in the config.

closes: #6868
https://pulp.plan.io/issues/6868

[nocoverage]
  • Loading branch information
dralley committed Jun 1, 2020
1 parent ace69e6 commit 2a72980
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 33 deletions.
2 changes: 1 addition & 1 deletion CHANGES/6332.doc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Added documentation for the RPM copy API.
Added documentation for the RPM copy API.
2 changes: 1 addition & 1 deletion CHANGES/6861.bugfix
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Make 'last_sync_revision_number' nullable in all migrations.
Make 'last_sync_revision_number' nullable in all migrations.
2 changes: 2 additions & 0 deletions CHANGES/6868.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed a bug where the behavior of RPM advanced copy with dependency solving differed depending
on the order of the source-destination repository pairs provided by the user.
86 changes: 55 additions & 31 deletions pulp_rpm/app/tasks/copy.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,51 +78,75 @@ def copy_content(config, dependency_solving):
criteria MUST be validated before being passed to this task.
content_pks: a list of content pks to copy from source to destination
"""
for entry in config:
source_repo_version = RepositoryVersion.objects.get(pk=entry["source_repo_version"])
dest_repo = RpmRepository.objects.get(pk=entry["dest_repo"])

dest_version_provided = bool(entry.get("dest_base_version"))
if dest_version_provided:
dest_repo_version = RepositoryVersion.objects.get(pk=entry["dest_base_version"])
else:
dest_repo_version = dest_repo.latest_version()

if entry.get("content"):
content_filter = Q(pk__in=entry.get("content"))
else:
content_filter = Q()

if not dependency_solving:
if not dependency_solving:
# No Dependency Solving Branch
# ============================
for entry in config:
source_repo_version = RepositoryVersion.objects.get(pk=entry["source_repo_version"])
dest_repo = RpmRepository.objects.get(pk=entry["dest_repo"])

dest_version_provided = bool(entry.get("dest_base_version"))
if dest_version_provided:
dest_repo_version = RepositoryVersion.objects.get(pk=entry["dest_base_version"])
else:
dest_repo_version = dest_repo.latest_version()

if entry.get("content"):
content_filter = Q(pk__in=entry.get("content"))
else:
content_filter = Q()

content_to_copy = source_repo_version.content.filter(content_filter)
content_to_copy |= find_children_of_content(content_to_copy, source_repo_version)

base_version = dest_repo_version if dest_version_provided else None
with dest_repo.new_version(base_version=base_version) as new_version:
new_version.add_content(content_to_copy)

continue

else:
# Dependency Solving Branch
# =========================
content_to_copy = {}

# TODO: add lookaside repos here
repo_mapping = {source_repo_version: dest_repo_version}
# TODO: a more structured way to store this state would be nice.
content_to_copy = {}
repo_mapping = {}
libsolv_repo_names = {}
base_versions = {}

solver = Solver()

for src in repo_mapping.keys():
repo_name = solver.load_source_repo(src)
libsolv_repo_names[repo_name] = src
for entry in config:
source_repo_version = RepositoryVersion.objects.get(pk=entry["source_repo_version"])
dest_repo = RpmRepository.objects.get(pk=entry["dest_repo"])

content = src.content.filter(content_filter)
children = find_children_of_content(content, src)
content_to_copy[repo_name] = content | children
dest_version_provided = bool(entry.get("dest_base_version"))
base_versions[source_repo_version] = dest_version_provided
if dest_version_provided:
dest_repo_version = RepositoryVersion.objects.get(pk=entry["dest_base_version"])
else:
dest_repo_version = dest_repo.latest_version()

for tgt in repo_mapping.values():
solver.load_target_repo(tgt)
repo_mapping[source_repo_version] = dest_repo_version

if entry.get("content"):
content_filter = Q(pk__in=entry.get("content"))
else:
content_filter = Q()

# Load the content from the source and destination repository versions into the solver
source_repo_name = solver.load_source_repo(source_repo_version)
solver.load_target_repo(dest_repo_version)

# Store the correspondance between the libsolv name of a repo version and the
# actual Pulp repo version, so that we can work backwards to get the latter
# from the former.
libsolv_repo_names[source_repo_name] = source_repo_version

# Find all of the matching content in the repository version, then determine
# child relationships (e.g. RPM children of Errata/Advisories), then combine
# those two sets to copy the specified content + children.
content = source_repo_version.content.filter(content_filter)
children = find_children_of_content(content, source_repo_version)
content_to_copy[source_repo_name] = content | children

solver.finalize()

Expand All @@ -131,6 +155,6 @@ def copy_content(config, dependency_solving):
for from_repo, units in content_to_copy.items():
src_repo_version = libsolv_repo_names[from_repo]
dest_repo_version = repo_mapping[src_repo_version]
base_version = dest_repo_version if dest_version_provided else None
base_version = dest_repo_version if base_versions[src_repo_version] else None
with dest_repo_version.repository.new_version(base_version=base_version) as new_version:
new_version.add_content(Content.objects.filter(pk__in=units))

0 comments on commit 2a72980

Please sign in to comment.