Issue #1406
closedUploading the same Content Unit twice causes a 500 error
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
Updated by bmbouter almost 9 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
Updated by bmbouter almost 9 years ago
- Related to Issue #494: better handle repositories with duplicate NVREAs added
Updated by ipanova@redhat.com almost 9 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
Updated by bmbouter almost 9 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.
Updated by mhrivnak almost 9 years ago
- Priority changed from Low to Normal
- Triaged changed from No to Yes
Recommend raising a PulpCodedException
Updated by bmbouter almost 9 years ago
- Has duplicate Issue #1473: Rpm upload fails with unknown error if you have the rpm in an orphan state. added
Updated by bmbouter almost 9 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.
Updated by pthomas@redhat.com almost 9 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.
Updated by placko almost 9 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?
Updated by mhrivnak almost 9 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.
Updated by cduryee almost 9 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.
Updated by bmbouter almost 9 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.
Updated by cduryee almost 9 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.
Updated by jortel@redhat.com almost 9 years ago
- Status changed from NEW to ASSIGNED
- Assignee set to jortel@redhat.com
Updated by jortel@redhat.com almost 9 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.
Updated by bmbouter almost 9 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 ^.
Updated by bmbouter almost 9 years ago
Oh and I don't have a comment on 2.8.0 versus 2.8.1.
Updated by mhrivnak almost 9 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
Updated by mhrivnak over 8 years ago
- Assignee changed from jortel@redhat.com to dkliban@redhat.com
Added by dkliban@redhat.com over 8 years ago
Added by dkliban@redhat.com over 8 years ago
Revision 78bc80a2 | View on GitHub
Adds check for duplicate unit key
re #1406
Added by dkliban@redhat.com over 8 years ago
Revision 2305d8fc | View on GitHub
Adds check for duplicate unit key
re #1406
Added by dkliban@redhat.com over 8 years ago
Revision 2305d8fc | View on GitHub
Adds check for duplicate unit key
re #1406
Added by dkliban@redhat.com over 8 years ago
Revision 2305d8fc | View on GitHub
Adds check for duplicate unit key
re #1406
Added by dkliban@redhat.com over 8 years ago
Revision 2305d8fc | View on GitHub
Adds check for duplicate unit key
re #1406
Added by dkliban@redhat.com over 8 years ago
Revision a4490625 | View on GitHub
Adds check for duplicate unit key
re #1406
Updated by dkliban@redhat.com over 8 years ago
- Status changed from ASSIGNED to POST
Updated by dkliban@redhat.com over 8 years ago
- Status changed from POST to MODIFIED
Updated by dkliban@redhat.com over 8 years ago
- Status changed from MODIFIED to 5
Updated by pthomas@redhat.com over 8 years ago
- Status changed from 5 to 6
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 ~]#
Updated by dkliban@redhat.com over 8 years ago
- Status changed from 6 to CLOSED - CURRENTRELEASE
Adds a check for duplicate unit keys
re #1406