Project

Profile

Help

Issue #1406

Uploading the same Content Unit twice causes a 500 error

Added by bmbouter over 3 years ago. Updated 5 days ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
High
Category:
-
Sprint/Milestone:
-
Severity:
2. Medium
Version:
Master
Platform Release:
2.8.0
Blocks Release:
OS:
Backwards Incompatible:
No
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Pulp 2
QA Contact:
Complexity:
Smash Test:
Verified:
Yes
Verification Required:
No
Sprint:

Description

This is problem for all content types, and this issue needs to fix them all. The goal is to make 2.8.0 behavior the same as the pre 2.8.0 behavior. I'm not sure if the second upload overwrites the first, or if it fails silently because the original is already. We need to check to make sure we do the same thing again.

Unfortunately, since the unit is saved in each plugin it likely needs to be fixed in each plugin because after the error occurs the unit still needs to be associated with the repository. Perhaps there is a way to do this in platform?

To reproduce, on a fresh installation of Pulp 2.8.0, run:

pulp-admin login -u admin -p admin
pulp-admin rpm repo create --repo-id zoo --feed http://repos.fedorapeople.org/repos/pulp/pulp/demo_repos/zoo/
wget https://repos.fedorapeople.org/repos/pulp/pulp/demo_repos/zoo/mouse-0.1.12-1.noarch.rpm
pulp-admin rpm repo uploads rpm --file ./mouse-0.1.12-1.noarch.rpm --repo-id zoo
pulp-admin rpm repo uploads rpm --file ./mouse-0.1.12-1.noarch.rpm --repo-id zoo

On the cli you'll be shown:

Task Failed

unexpected error occurred importing uploaded file

In the logs you'll see:

unexpected error occurred importing uploaded file
Traceback (most recent call last):
  File "/home/vagrant/devel/pulp_rpm/plugins/pulp_rpm/plugins/importers/yum/upload.py", line 100, in upload
    handlers[type_id](repo, type_id, unit_key, metadata, file_path, conduit, config)
  File "/home/vagrant/devel/pulp_rpm/plugins/pulp_rpm/plugins/importers/yum/upload.py", line 318, in _handle_package
    unit.save()
  File "/usr/lib/python2.7/site-packages/mongoengine/document.py", line 359, in save
    raise NotUniqueError(message %% unicode(err))
NotUniqueError: Tried to save duplicate unique keys (E11000 duplicate key error index: pulp_database.units_rpm.$name_1_epoch_1_version_1_release_1_arch_1_checksumtype_1_checksum_1 dup key: { : "mouse", : "0", : "0.1.12", : "1", : "noarch", : "sha256", : "f4200643b0845fdc55ee002c92c0404a9f3a2a49f596c78b40ab56749de226ce" })

The expectation is that there would be better error handling/reporting. There should be no traceback in the logs, and an indication should be shown to the user about why the rpm failed to upload/import.


Related issues

Related to RPM Support - Story #213: Add upload option to have an rpm/srpm/drpm/etc overwrite an existing one with the same unit_key fields (omitting checksums) CLOSED - WONTFIX Actions
Related to RPM Support - Issue #494: better handle repositories with duplicate NVREAs CLOSED - CURRENTRELEASE Actions
Duplicated by Pulp - Issue #1473: Rpm upload fails with unknown error if you have the rpm in an orphan state. CLOSED - DUPLICATE Actions

Associated revisions

Revision f95eb3f4 View on GitHub
Added by dkliban@redhat.com about 3 years ago

Adds a check for duplicate unit keys

re #1406

Revision 78bc80a2 View on GitHub
Added by dkliban@redhat.com about 3 years ago

Adds check for duplicate unit key

re #1406

Revision 2305d8fc View on GitHub
Added by dkliban@redhat.com about 3 years ago

Adds check for duplicate unit key

re #1406

Revision 2305d8fc View on GitHub
Added by dkliban@redhat.com about 3 years ago

Adds check for duplicate unit key

re #1406

Revision a4490625 View on GitHub
Added by dkliban@redhat.com about 3 years ago

Adds check for duplicate unit key

re #1406

History

#1 Updated by bmbouter over 3 years ago

  • Related to Story #213: Add upload option to have an rpm/srpm/drpm/etc overwrite an existing one with the same unit_key fields (omitting checksums) added

#2 Updated by bmbouter over 3 years ago

  • Related to Issue #494: better handle repositories with duplicate NVREAs added

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

I think at this point 1st rpm is just replaced with 2nd uploaded.
i was not able to reproduce the situation:

$ pulp-admin rpm repo uploads rpm --repo-id test2 --file /home/ipanova/mouse-0.1.12-1.noarch.rpm

+----------------------------------------------------------------------+
                              Unit Upload
+----------------------------------------------------------------------+

Extracting necessary metadata for each request...
[==================================================] 100%
Analyzing: mouse-0.1.12-1.noarch.rpm
... completed

Creating upload requests on the server...
[==================================================] 100%
Initializing: mouse-0.1.12-1.noarch.rpm
... completed

Starting upload of selected units. If this process is stopped through ctrl+c,
the uploads will be paused and may be resumed later using the resume command or
canceled entirely using the cancel command.

Uploading: mouse-0.1.12-1.noarch.rpm
[==================================================] 100%
2476/2476 bytes
... completed

Importing into the repository...
This command may be exited via ctrl+c without affecting the request.

[\]
Running...

Task Succeeded

Deleting the upload request...
... completed

[ipanova@ina ~]$ pulp-admin rpm repo uploads rpm --repo-id test2 --file /home/ipanova/mouse-0.1.12-1.noarch.rpm

+----------------------------------------------------------------------+
                              Unit Upload
+----------------------------------------------------------------------+

Extracting necessary metadata for each request...
[==================================================] 100%
Analyzing: mouse-0.1.12-1.noarch.rpm
... completed

Creating upload requests on the server...
[==================================================] 100%
Initializing: mouse-0.1.12-1.noarch.rpm
... completed

Starting upload of selected units. If this process is stopped through ctrl+c,
the uploads will be paused and may be resumed later using the resume command or
canceled entirely using the cancel command.

Uploading: mouse-0.1.12-1.noarch.rpm
[==================================================] 100%
2476/2476 bytes
... completed

Importing into the repository...
This command may be exited via ctrl+c without affecting the request.

[\]
Running...

Task Succeeded

Deleting the upload request...
... completed

#4 Updated by bmbouter over 3 years ago

Once the rpm conversion to mongoengine is done it will raise the ValidationError because there are new uniqueness constraints that prevent the second save() from occurring.

#5 Updated by mhrivnak over 3 years ago

  • Priority changed from Low to Normal
  • Triaged changed from No to Yes

Recommend raising a PulpCodedException

#6 Updated by bmbouter over 3 years ago

  • Duplicated by Issue #1473: Rpm upload fails with unknown error if you have the rpm in an orphan state. added

#7 Updated by bmbouter about 3 years ago

  • Subject changed from Uploading the same RPM twice causes a 500 error to Uploading the same Content Unit twice causes a 500 error
  • Description updated (diff)
  • Severity changed from 1. Low to 2. Medium
  • Version changed from 2.7.0 to Master
  • Platform Release set to 2.8.1

Raising severity because this causes false positives when pulp-smash is re-run without clearing the database.

#8 Updated by pthomas@redhat.com about 3 years ago

Because of how this issue is affecting satellite and for the fact that its a 2.8 regression, Can we bump the priority on this so that we can get this addressed in 2.8.

#9 Updated by placko about 3 years ago

What is correct/expected behavior here? I don't get any exception after uploading same unit 2x in 2.7 neither in 2.8. What code should be returned upon failure?

#10 Updated by mhrivnak about 3 years ago

This appears to be a problem only with the rendering of output to the user. The correct behavior seems to be happening, followed by an ugly message on screen.

Also, I haven't seen any indication that this is currently impacting katello or satellite. It was moved to 2.8.1 in collaboration with that team. If there is new information in that regard, please add it here.

#11 Updated by ttereshc about 3 years ago

Partially(?) fixed here, PR

#12 Updated by cduryee about 3 years ago

I hit an issue related to this today. I created a puppet repo, then uploaded a puppet module, then deleted it. I wanted the module back again so I went to upload it, and it errored out.

I was testing this as part of an unrelated bug, but I think this is a common use case.

#13 Updated by bmbouter about 3 years ago

I agree it's a common use case. Until the bug is fixed, one workaround for the workflow you described is to do an orphan cleanup after the delete and before the second upload.

#14 Updated by cduryee about 3 years ago

A different method to hit this bug which I hit today:

  • sync puppet forge in one repo to get all puppet modules
  • wait some time and upload the puppet manifests you want manually to a different puppet repo so you can have the modules you need while the long-running sync completes

If the unit you are uploading has already been created via the other sync but not yet associated to the repo, you'll get this error. The workaround is to wait for the completion of the sync process.

#15 Updated by jortel@redhat.com about 3 years ago

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

#16 Updated by jortel@redhat.com about 3 years ago

This is a general problem related to mongoengine conversion. The save_unit() method in the conduits caught duplicate key constraint violations. When caught (by the conduit), the unit in the DB was updated and associated to the repository. Some of the mongoengine converted plugins are missing try/catch NotUniqueError logic. The danger of ignoring this is that the unit already stored in the DB is not associated to the repository as expected and in some cases fails silently. I think this needs to be aligned to 2.8.0.

#17 Updated by bmbouter about 3 years ago

I agree the mongoengine conversion causes the plugins to create-and-save a unit directly within plugin code, so we're not in a good position to fix this all in one place. Anywhere we do want it fixed, we could override the save() method to catch DuplicateKeyErrors and return the existing object. We could introduce that in the inheritance tree of RPM's mongoengine models which is probably best. I'm not sure we want all ContentUnits to catch DuplicateKeyErrors in all cases.

Another option is to put Try/Catch exceptions around the save() calls throughout the plugin code, which does not seem as good as ^.

#18 Updated by bmbouter about 3 years ago

Oh and I don't have a comment on 2.8.0 versus 2.8.1.

#19 Updated by jortel@redhat.com about 3 years ago

  • Triaged changed from Yes to No

#20 Updated by mhrivnak about 3 years ago

  • Priority changed from Normal to High
  • Platform Release changed from 2.8.1 to 2.8.0
  • Triaged changed from No to Yes

#21 Updated by mhrivnak about 3 years ago

  • Assignee changed from jortel@redhat.com to dkliban@redhat.com

#23 Updated by dkliban@redhat.com about 3 years ago

  • Status changed from POST to MODIFIED

#24 Updated by dkliban@redhat.com about 3 years ago

  • Status changed from MODIFIED to ON_QA

#25 Updated by pthomas@redhat.com about 3 years ago

  • Status changed from ON_QA to VERIFIED

verified
[root@mgmt5 ~]# rpm -qa pulp-server
pulp-server-2.8.0-0.9.rc.el7.noarch
[root@mgmt5 ~]#

[root@mgmt5 ~]# pulp-admin rpm repo uploads rpm --repo-id upload f pulp-test-package-0.2.1-1.fc11.x86_64.rpm -vv
---------------------------------------------------------------------

Unit Upload
--------------------------------------------------------------------

Extracting necessary metadata for each request...
[==================================================] 100%
Analyzing: pulp-test-package-0.2.1-1.fc11.x86_64.rpm
... completed

Files to be uploaded:
pulp-test-package-0.2.1-1.fc11.x86_64.rpm

Creating upload requests on the server...
[==================================================] 100%
Initializing: pulp-test-package-0.2.1-1.fc11.x86_64.rpm
... completed

Starting upload of selected units. If this process is stopped through ctrl+c,
the uploads will be paused and may be resumed later using the resume command or
canceled entirely using the cancel command.

Uploading: pulp-test-package-0.2.1-1.fc11.x86_64.rpm
[==================================================] 100%
2216/2216 bytes
... completed

Importing into the repository...
This command may be exited via ctrl+c without affecting the request.

[\]
Running...

Task Succeeded

Deleting the upload request...
... completed

[root@mgmt5 ~]# pulp-admin rpm repo delete --repo-id upload
This command may be exited via ctrl+c without affecting the request.

[\]
Running...

Repository [upload] successfully deleted

[\]
Running...
-- Task Tags: pulp:consumer:starfish, pulp:repository:upload,
pulp:repository_distributor:yum_distributor, pulp:action:agent_unbind ----

Repository [upload] successfully deleted

[root@mgmt5 ~]# pulp-admin orphan list
--------------------------------------------------------------------
Summary
--------------------------------------------------------------------

Distribution: 0
Docker Blob: 0
Docker Image: 0
Docker Manifest: 0
Docker Tag: 0
Drpm: 0
Erratum: 0
Iso: 0
Package Category: 0
Package Environment: 0
Package Group: 0
Puppet Module: 0
Rpm: 1
Srpm: 0
Yum Repo Metadata File: 0
Total: 1

[root@mgmt5 ~]# pulp-admin orphan list --type rpm --details
Arch: x86_64
Checksum: 4dbde07b4a8eab57e42ed0c9203083f1d61e0b13935d1a569193ed8efc9ecfd7
Checksumtype: sha256
Epoch: 0
Id: d0f05b70-da4f-4d78-93f1-38796356d4b6
Name: pulp-test-package
Release: 1.fc11
Version: 0.2.1

--------------------------------------------------------------------
Summary
--------------------------------------------------------------------

Rpm: 1
Total: 1

[root@mgmt5 ~]# pulp-admin rpm repo create --repo-id upload
Successfully created repository [upload]

[root@mgmt5 ~]# pulp-admin rpm repo uploads rpm --repo-id upload f pulp-test-package-0.2.1-1.fc11.x86_64.rpm -vv
---------------------------------------------------------------------

Unit Upload
--------------------------------------------------------------------

Extracting necessary metadata for each request...
[==================================================] 100%
Analyzing: pulp-test-package-0.2.1-1.fc11.x86_64.rpm
... completed

Files to be uploaded:
pulp-test-package-0.2.1-1.fc11.x86_64.rpm

Creating upload requests on the server...
[==================================================] 100%
Initializing: pulp-test-package-0.2.1-1.fc11.x86_64.rpm
... completed

Starting upload of selected units. If this process is stopped through ctrl+c,
the uploads will be paused and may be resumed later using the resume command or
canceled entirely using the cancel command.

Uploading: pulp-test-package-0.2.1-1.fc11.x86_64.rpm
[==================================================] 100%
2216/2216 bytes
... completed

Importing into the repository...
This command may be exited via ctrl+c without affecting the request.

[\]
Running...

Task Succeeded

Deleting the upload request...
... completed

[root@mgmt5 ~]# pulp-admin orphan list
--------------------------------------------------------------------
Summary
--------------------------------------------------------------------

Distribution: 0
Docker Blob: 0
Docker Image: 0
Docker Manifest: 0
Docker Tag: 0
Drpm: 0
Erratum: 0
Iso: 0
Package Category: 0
Package Environment: 0
Package Group: 0
Puppet Module: 0
Rpm: 0
Srpm: 0
Yum Repo Metadata File: 0
Total: 0

[root@mgmt5 ~]# pulp-admin orphan list --type rpm --details+----------------------------------------------------------------------+
Summary
--------------------------------------------------------------------

Total: 0

[root@mgmt5 ~]#

#26 Updated by dkliban@redhat.com about 3 years ago

  • Status changed from VERIFIED to CLOSED - CURRENTRELEASE

#27 Updated by pulpbot about 2 years ago

  • Verified changed from No to Yes

#28 Updated by bmbouter 5 days ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF