Bug 2091438

Summary: Use of content.count() in app/models/repository.py seems to hit an error
Product: Red Hat Satellite Reporter: Pablo Hess <phess>
Component: PulpAssignee: Grant Gainey <ggainey>
Status: CLOSED ERRATA QA Contact: Vladimír Sedmík <vsedmik>
Severity: high Docs Contact:
Priority: high    
Version: 6.9.9CC: ahumbe, dalley, dkliban, ggainey, iballou, osousa, rchan
Target Milestone: 6.9.10Keywords: Triaged, Upgrades
Target Release: Unused   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: python3-pulp-2to3-migration-0.11.13 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2022-11-17 17:17:17 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Pablo Hess 2022-05-29 21:05:32 UTC
Description of problem:

During the migration from pulp2 to pulp3 on a Satellite 6.9.9 an error happens during the content-prepare phase while migrating RPMs, logging this traceback:
~~~
 "pulp_href": "/pulp/api/v3/tasks/e3ef9998-a19d-4def-8f69-305ffff669b3/",
            "pulp_created": "2022-05-27T17:08:43.443046Z",
            "state": "failed",
            "name": "pulp_2to3_migration.app.migration.complex_repo_migration",
            "started_at": "2022-05-27T17:19:30.170348Z",
            "finished_at": "2022-05-27T17:19:31.831923Z",
            "error": {
                "traceback": "  File "/usr/lib/python3.6/site-packages/rq/worker.py", line 936, in perform_job\n    rv = job.perform()\n  File "/usr/lib/python3.6/site-packages/rq/job.py", line 684, in perform\n    self._result = self._execute()\n  File "/usr/lib/python3.6/site-packages/rq/job.py", line 690, in _execute\n    return self.func(*self.args, **self.kwargs)\n  File "/usr/lib/python3.6/site-packages/pulp_2to3_migration/app/migration.py", line 201, in complex_repo_migration\n    create_repo_version(progress_rv, pulp2_repo, pulp3_remote)\n  File "/usr/lib/python3.6/site-packages/pulp_2to3_migration/app/migration.py", line 451, in create_repo_version\n    resolve_path_overlap(new_version)\n  File "/usr/lib/python3.6/site-packages/pulp_2to3_migration/app/migration.py", line 418, in resolve_path_overlap\n    version.remove_content(cas_with_conflicts[0].content)\n  File "/usr/lib/python3.6/site-packages/pulpcore/app/models/repository.py", line 618, in remove_content\n    if not content or not content.count():\n",
                "description": "'Content' object has no attribute 'count'"
~~~

Reformatting the traceback for readability:
~~~
"traceback": "  File "/usr/lib/python3.6/site-packages/rq/worker.py", line 936, in perform_job
    rv = job.perform()
  File "/usr/lib/python3.6/site-packages/rq/job.py", line 684, in perform
    self._result = self._execute()
  File "/usr/lib/python3.6/site-packages/rq/job.py", line 690, in _execute
    return self.func(*self.args, **self.kwargs)
  File "/usr/lib/python3.6/site-packages/pulp_2to3_migration/app/migration.py", line 201, in complex_repo_migration
    create_repo_version(progress_rv, pulp2_repo, pulp3_remote)
  File "/usr/lib/python3.6/site-packages/pulp_2to3_migration/app/migration.py", line 451, in create_repo_version
    resolve_path_overlap(new_version)
  File "/usr/lib/python3.6/site-packages/pulp_2to3_migration/app/migration.py", line 418, in resolve_path_overlap
    version.remove_content(cas_with_conflicts[0].content)
  File "/usr/lib/python3.6/site-packages/pulpcore/app/models/repository.py", line 618, in remove_content
    if not content or not content.count():",
                "description": "'Content' object has no attribute 'count'
~~~

Inspecting the use of content.count() in app/models/repository.py, we find out the Content class really doesn't implement a count() method, so this condition seems bound to fail every single time.
I wonder if content.count() here was intended as a test whether `content` is iterable, or maybe as a check that `content` contains more than 0 items (and is, by definition, iterable).


Assuming the former, I modified the test in app/models/repository.py, from:

    if not content or not content.count():

to:

    if not content or not hasattr(content, '__iter__'):


The modified code ran the migration successfully.

I don't know yet what would cause `content` to be iterable or not -- I suppose it could hold either a single Content object or a collection of Content objects.


Version-Release number of selected component (if applicable):
satellite-6.9.9-1.el7sat.noarch
python3-pulpcore-3.7.9-1.el7pc.noarch             
python3-pulp-2to3-migration-0.11.10-1.el7pc.noarch
python3-pulp-certguard-1.0.3-1.el7pc.noarch       
python3-pulp-container-2.1.2-1.el7pc.noarch       
python3-pulp-file-1.3.0-1.el7pc.noarch            
python3-pulp-rpm-3.11.4-1.el7pc.noarch            



How reproducible:
100% in this particular Satellite attempting a pulp-2to3 migration.

Steps to Reproduce:
1. Have a Satellite 6.9.9 with some particular (or not) contents.
2. Prepare contents for a pulp-2to3 migration for a Satellite upgrade: satellite-maintain content prepare

Actual results:
Some RPMs into the migration process, the traceback above printed as part of the pulp task failure.

Expected results:
The content prepare step works.

Additional info:

As an actual first step in the investigation I did for this support ticket, I simply removed the `not content.count()` condition and restarted the content prepare task, which failed at a different point with a traceback stating "content object is not iterable". So I assumed the `content.count()` condition was to check for iterability and went with my modified condition, `not hasattr(content, '__iter__')`, which in this particular case resolved the issue.

Comment 1 Pablo Hess 2022-06-01 18:25:19 UTC
Pull Request is now merged upstream.

The fix ended up being on the code that passes a Content object to remove_content() -- it was fixed to pass a QuerySet object instead, so content.count() would work as expected.

Comment 4 Vladimír Sedmík 2022-11-02 14:21:01 UTC
Reproduced on 6.9.9
Fails on 6.9.10 snap 1 with different error: TypeError: 'UUID' object is not iterable

Adding Grant as assignee as he already looked into it and helped me with the reproducer.

Comment 5 Daniel Alley 2022-11-03 04:37:10 UTC
PR: https://github.com/pulp/pulp-2to3-migration/pull/594

Looks like it is experimentally verified with a very small additional patch.  However, that means we need a new snap.

Comment 8 Vladimír Sedmík 2022-11-10 16:09:50 UTC
Verified in 6.9.10 snap 2 with python3-pulp-2to3-migration-0.11.13-1.el7pc.noarch

Steps to reproduce/verify:
1) On a blank 6.9 Sat (with pulp2) create a Prod and a file-type repo w/o upstream url, upload 2 files (1.iso and 2.iso for example)
2) Rename the 2.iso so its name overlaps the 1.iso path in mongodb:
[root@sat ~]# mongo pulp_database
> db.units_iso.find()
{ "_id" : "0aca3b86-8004-45ac-bec1-e3e7f36b1606", "pulp_user_metadata" : {  }, "_last_updated" : 1668071456, "_storage_path" : "/var/lib/pulp/content/units/iso/07/b4c128c58e7e1bd281fb03575a71c2371ff0b8656e157d35a6b0aa5b3ecba0", "downloaded" : true, "name" : "1.iso", "checksum" : "1c4ef7df278004ece0ba14e0ed69c0a4e69762dc7e12cf7ee150b21ad612f42c", "size" : 10485760, "_ns" : "units_iso", "_content_type_id" : "iso" }
{ "_id" : "b7ae32d3-5129-45f6-8779-beb71f250c95", "pulp_user_metadata" : {  }, "_last_updated" : 1668071456, "_storage_path" : "/var/lib/pulp/content/units/iso/05/5fb578119dbf971e7310e6ac06d639bb5af1155089be7ab3c9ad121dd5f6c7", "downloaded" : true, "name" : "2.iso", "checksum" : "d34e9a108c6ee81e094cd6e26955686c9ba9eb036b4f0d64c82ebe1bebb73b5d", "size" : 10485760, "_ns" : "units_iso", "_content_type_id" : "iso" }
> db.units_iso.update( {name: { $eq: "2.iso" } }, { $set: {name: "1.iso/22.iso"} } )
WriteResult({ "nMatched" : 1, "nUpserted" : 0, "nModified" : 1 })
> db.units_iso.find()
{ "_id" : "0aca3b86-8004-45ac-bec1-e3e7f36b1606", "pulp_user_metadata" : {  }, "_last_updated" : 1668071456, "_storage_path" : "/var/lib/pulp/content/units/iso/07/b4c128c58e7e1bd281fb03575a71c2371ff0b8656e157d35a6b0aa5b3ecba0", "downloaded" : true, "name" : "1.iso", "checksum" : "1c4ef7df278004ece0ba14e0ed69c0a4e69762dc7e12cf7ee150b21ad612f42c", "size" : 10485760, "_ns" : "units_iso", "_content_type_id" : "iso" }
{ "_id" : "b7ae32d3-5129-45f6-8779-beb71f250c95", "pulp_user_metadata" : {  }, "_last_updated" : 1668071456, "_storage_path" : "/var/lib/pulp/content/units/iso/05/5fb578119dbf971e7310e6ac06d639bb5af1155089be7ab3c9ad121dd5f6c7", "downloaded" : true, "name" : "1.iso/22.iso", "checksum" : "d34e9a108c6ee81e094cd6e26955686c9ba9eb036b4f0d64c82ebe1bebb73b5d", "size" : 10485760, "_ns" : "units_iso", "_content_type_id" : "iso" }
3) Run the content migration:
# satellite-maintain prep-6.10-upgrade
# satellite-maintain content prepare

The content migration succeeded without error and the renamed file remained accessible after the switchover (upgrade to 6.10).

Comment 11 pulp-infra@redhat.com 2022-11-16 14:08:42 UTC
The Pulp upstream bug status is at closed. Updating the external tracker on this bug.

Comment 15 errata-xmlrpc 2022-11-17 17:17:17 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Important: Satellite 6.9.10 Async Security Update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2022:8532