Project

Profile

Help

Issue #1944

YumMetadataFile copy does not save its new storage_path

Added by mhrivnak over 4 years ago. Updated 4 months ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
High
Sprint/Milestone:
-
Start date:
Due date:
Estimated time:
Severity:
3. High
Version:
2.8.3
Platform Release:
2.8.5
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Pulp 2
Sprint:
Sprint 3
Quarter:

Description

This bug was introduced in pulp 2.8.

When a YumMetadataFile is copied from repo A to repo B, a new unit gets created. A new copy of the file also gets made.

As of pulp 2.8.3, the code works like this:

1. create a new unit (or find a pre-existing one)
2. save the new unit
3. calculate a new storage path for the new unit
4. copy the file to the new storage path

The problem is that the new storage path calculated in step 3 never gets saved. Swapping the order of 2 and 3 fixes the problem.

The effect is that the new unit's storage_path references the old unit's file. If either unit gets deleted, it blows away the file, leaving one or more remaining units referencing a non-existing file.

You can reproduce this by syncing a repo with a metadata file, such as a "productid", and then copying that unit to another repo. You can then see in the database that two units exist referencing the same file on disk.

This diff corrects the behavior:

diff --git a/plugins/pulp_rpm/plugins/importers/yum/associate.py b/plugins/pulp_rpm/plugins/importers/yum/associate.py
index 2cb4088..b86f31c 100644
--- a/plugins/pulp_rpm/plugins/importers/yum/associate.py
+++ b/plugins/pulp_rpm/plugins/importers/yum/associate.py
@@ -354,6 +354,8 @@ def associate_copy_for_repo(unit, dest_repo, set_content=False):
     """
     new_unit = unit.clone()
     new_unit.repo_id = dest_repo.repo_id
+    # calculate a new storage path since the unit key has changed
+    new_unit.set_storage_path(os.path.basename(unit.storage_path))

     try:
         new_unit.save()
@@ -365,7 +367,6 @@ def associate_copy_for_repo(unit, dest_repo, set_content=False):
         new_unit.save()

     if set_content:
-        new_unit.set_storage_path(os.path.basename(unit._storage_path))
         new_unit.safe_import_content(unit._storage_path)

     repo_controller.associate_single_unit(repository=dest_repo, unit=new_unit)

A migration will also be required to fix up existing units. Probably the best we can do is re-calculate each unit's storage path, copy its referenced file to that location, and then save the unit with the new storage path.

Unfortunately that doesn't fix published data. We might just have to live with that, and recommend that users re-publish any repos with YumMetadatafiles in them. This is the problem users could encounter:

Consider a YumMetadataFile gets sync'd into repo A. It then gets copied from A to B, and from B to C. Then our migration runs, and it ensures each unit references a unique file. The problem is that all three publications have symlinks to the file in A.

If/when a sync of A replaces that file with a new one, B and C will have a published symlink that is broken. The easiest fix is to just re-publish B and C, but we generally try to avoid that in migrations.


Related issues

Related to RPM Support - Task #1935: Redesign the yum_repo_metadata_file modelCLOSED - WONTFIX

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Related to RPM Support - Issue #2248: metadata file copy results in error 'Content import of FILENAME failed - must be an existing file'CLOSED - WONTFIX<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

Associated revisions

Revision b1df3862 View on GitHub
Added by ipanova@redhat.com over 4 years ago

YumMetadataFile copy does not save its new storage_path.

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

History

#1 Updated by mhrivnak over 4 years ago

  • Related to Task #1935: Redesign the yum_repo_metadata_file model added

#2 Updated by dkliban@redhat.com over 4 years ago

  • Triaged changed from No to Yes

#4 Updated by mhrivnak over 4 years ago

  • Sprint/Milestone set to 21

#5 Updated by ipanova@redhat.com over 4 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to ipanova@redhat.com

#6 Updated by ipanova@redhat.com over 4 years ago

  • Status changed from ASSIGNED to POST
  • Platform Release set to 2.8.5

#7 Updated by ipanova@redhat.com over 4 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100

#8 Updated by ipanova@redhat.com over 4 years ago

  • Status changed from MODIFIED to ASSIGNED

#9 Updated by ipanova@redhat.com over 4 years ago

  • Status changed from ASSIGNED to POST

#10 Updated by ipanova@redhat.com over 4 years ago

  • Status changed from POST to MODIFIED

#11 Updated by semyers over 4 years ago

  • Status changed from MODIFIED to 5

#12 Updated by pthomas@redhat.com about 4 years ago

  • Status changed from 5 to 6

verified

[root@qe-blade-10 ~]# rpm -qa pulp-server
pulp-server-2.8.5-0.1.beta.el7.noarch
[root@qe-blade-10 ~]#

[root@qe-blade-10 ~]# pulp-admin rpm repo copy metafile -f rhel7 -t issue1944
This command may be exited via ctrl+c without affecting the request.

[\]
Running...

Copied:
productid

{ "_id" : "0db0a62a-ea8b-4041-9a90-a4bccfc8b39c", "pulp_user_metadata" : {  }, "_last_updated" : 1466623194, "_storage_path" : "/var/lib/pulp/content/units/yum_repo_metadata_file/46/f013ec598b38b306dfd761b41a3ebf1c496f09f440679a1d7b2d4188145fda/productid.gz", "downloaded" : true, "data_type" : "productid", "repo_id" : "rhel7", "checksum" : "b4d8b3c9afd39d822901b882f4214704275ef09f", "checksum_type" : "sha1", "_ns" : "units_yum_repo_metadata_file", "_content_type_id" : "yum_repo_metadata_file" }
{ "_id" : "7b3d09dd-15fc-4e8e-a8db-5b107cc747d1", "pulp_user_metadata" : {  }, "_last_updated" : 1466692546, "_storage_path" : "/var/lib/pulp/content/units/yum_repo_metadata_file/2a/3b531d8ce1897a09e86914d7f2fbdfb29840e218527f972052f8425ed6992b/productid.gz", "downloaded" : true, "data_type" : "productid", "repo_id" : "issue1944", "checksum" : "b4d8b3c9afd39d822901b882f4214704275ef09f", "checksum_type" : "sha1", "_ns" : "units_yum_repo_metadata_file", "_content_type_id" : "yum_repo_metadata_file" }
> 

#13 Updated by semyers about 4 years ago

  • Status changed from 6 to CLOSED - CURRENTRELEASE

#15 Updated by ipanova@redhat.com over 3 years ago

  • Related to Issue #2248: metadata file copy results in error 'Content import of FILENAME failed - must be an existing file' added

#16 Updated by bmbouter over 2 years ago

  • Sprint set to Sprint 3

#17 Updated by bmbouter over 2 years ago

  • Sprint/Milestone deleted (21)

#18 Updated by bmbouter over 1 year ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF