Project

Profile

Help

Issue #1944

closed

YumMetadataFile copy does not save its new storage_path

Added by mhrivnak almost 8 years ago. Updated almost 4 years 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

Actions
Related to RPM Support - Issue #2248: metadata file copy results in error 'Content import of FILENAME failed - must be an existing file'CLOSED - WONTFIXActions
Actions #1

Updated by mhrivnak almost 8 years ago

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

Updated by dkliban@redhat.com almost 8 years ago

  • Triaged changed from No to Yes
Actions #4

Updated by mhrivnak almost 8 years ago

  • Sprint/Milestone set to 21
Actions #5

Updated by ipanova@redhat.com almost 8 years ago

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

Updated by ipanova@redhat.com almost 8 years ago

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

Added by ipanova@redhat.com almost 8 years ago

Revision b1df3862 | View on GitHub

YumMetadataFile copy does not save its new storage_path.

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

Actions #7

Updated by ipanova@redhat.com almost 8 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100
Actions #8

Updated by ipanova@redhat.com almost 8 years ago

  • Status changed from MODIFIED to ASSIGNED
Actions #9

Updated by ipanova@redhat.com almost 8 years ago

  • Status changed from ASSIGNED to POST
Actions #10

Updated by ipanova@redhat.com almost 8 years ago

  • Status changed from POST to MODIFIED
Actions #11

Updated by semyers almost 8 years ago

  • Status changed from MODIFIED to 5
Actions #12

Updated by pthomas@redhat.com almost 8 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" }
> 
Actions #13

Updated by semyers almost 8 years ago

  • Status changed from 6 to CLOSED - CURRENTRELEASE
Actions #15

Updated by ipanova@redhat.com about 7 years ago

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

Updated by bmbouter about 6 years ago

  • Sprint set to Sprint 3
Actions #17

Updated by bmbouter about 6 years ago

  • Sprint/Milestone deleted (21)
Actions #18

Updated by bmbouter about 5 years ago

  • Tags Pulp 2 added

Also available in: Atom PDF