Bug 1412486

Summary:	Katello::<Unit type>.import_all function breaks repository association after upgrading satellite or running reindex rake task
Product:	Red Hat Satellite	Reporter:	Hao Chang Yu <hyu>
Component:	Content Management	Assignee:	Justin Sherrill <jsherril>
Status:	CLOSED ERRATA	QA Contact:	jcallaha
Severity:	high	Docs Contact:
Priority:	high
Version:	6.2.6	CC:	bbuckingham, egolov, jcallaha, jdeenada, jsherril, mmccune, oshtaier, stbenjam, zhunting
Target Milestone:	Unspecified	Keywords:	PrioBumpField, Regression, Triaged
Target Release:	Unused
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	rubygem-katello-3.0.0.94-1	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2017-01-26 10:47:31 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:
Bug Depends On:
Bug Blocks:	1410795

Description Hao Chang Yu 2017-01-12 06:59:14 UTC

Description of problem:
After upgrading to satellite 6.2.6, the repositories package count went unreasonably less in the Web UI

For example: Software collection repo only shows 30 packages


After investigating further I notice a bug in "import_all" function.

Katello::Rpm.import_all is fetching rpms from Pulp by chunk (default 100 units per chunk) and call the "sync_repository_associations" function on every chunk which breaks the repository associations in Katello. That is because "sync_repository_associations" will delete the existing associations that are not in the chunk.

For example:
Existing associations = [1, 2, 3, 4]

Pulp chunk 1 = [1, 2, 3, 5]

# Will be added
new associations = Pulp chunk 1 - Existing associations
[5] = [1, 2, 3, 5] - [1, 2, 3, 4]

# Will be deleted
old association = Existing associations - Pulp chunk 1
[4] = [1, 2, 3, 4] - [1, 2, 3, 5]

As a result the existing associations will become [1, 2, 3, 5].

Now Pulp chunk 2 = [6, 7, 8]

# Will be deleted
old association = Existing associations - Pulp chunk 2
[1, 2, 3, 5] = [1, 2, 3, 5] - [6, 7, 8]

As a result the existing associations will now become [6, 7, 8].


Version-Release number of selected component (if applicable):
6.2.6. Could also happen in the earlier versions because customer complain that it happened multiple times in different versions.

How reproducible:
Run satellite-installer --upgrade

Or

foreman-rake katello::reindex

Comment 1 Hao Chang Yu 2017-01-12 07:06:40 UTC

I try to patch the code which seems to work.


--- app/models/katello/concerns/pulp_database_unit.rb	2016-12-13 05:04:36.000000000 +1000
+++ /root/pulp_database_unit.rb	2017-01-12 17:04:58.898195979 +1000
@@ -52,8 +52,7 @@
             end
             item.update_from_json(unit)
           end
-          update_repository_associations(units, additive) if index_repository_association && self.manage_repository_association
-          units.count
+          units
         end
 
         if uuids
@@ -61,7 +60,12 @@
         else
           results = content_unit_class.fetch_all(&process_block)
         end
-        results.inject(:+)
+
+        if results.present? && index_repository_association && self.manage_repository_association
+          update_repository_associations(results, additive)
+        end
+
+        results.size
       end

Comment 2 Hao Chang Yu 2017-01-12 09:19:36 UTC

(In reply to Hao Chang Yu from comment #1)
> I try to patch the code which seems to work.
> 
> 
> --- app/models/katello/concerns/pulp_database_unit.rb	2016-12-13
> 05:04:36.000000000 +1000
> +++ /root/pulp_database_unit.rb	2017-01-12 17:04:58.898195979 +1000
> @@ -52,8 +52,7 @@
>              end
>              item.update_from_json(unit)
>            end
> -          update_repository_associations(units, additive) if
> index_repository_association && self.manage_repository_association
> -          units.count
> +          units
>          end
>  
>          if uuids
> @@ -61,7 +60,12 @@
>          else
>            results = content_unit_class.fetch_all(&process_block)
>          end
> -        results.inject(:+)
> +
> +        if results.present? && index_repository_association &&
> self.manage_repository_association

Correction:
- if results.present? && index_repository_association && self.manage_repository_association
+ if index_repository_association && self.manage_repository_association


> +          update_repository_associations(results, additive)
> +        end
> +
> +        results.size
>        end

Comment 5 Justin Sherrill 2017-01-17 20:47:38 UTC

I have confirmed the issue.  To reproduce, sync some large repo with ~1000 rpms, then run:

foreman-rake katello:upgrades:2.4:import_rpms

navigate to the UI and check the package count for that repo.  The supplied patch does fix the issue but would likely revert the benefits of https://bugzilla.redhat.com/show_bug.cgi?id=1399294 

Will look into an alternative fix.

Comment 6 Justin Sherrill 2017-01-17 21:13:25 UTC

Created redmine issue http://projects.theforeman.org/issues/18116 from this bug

Comment 7 Satellite Program 2017-01-17 23:07:25 UTC

Upstream bug assigned to jsherril

Comment 8 Satellite Program 2017-01-17 23:07:29 UTC

Upstream bug assigned to jsherril

Comment 14 jcallaha 2017-01-24 20:20:30 UTC

Verified in Satellite 6.2.7 Snap 3. Below are the results of testing the installer as a means to correct errors from the reindex bug. 


--- Before reimport ---
cv - all
56788 Packages
6106 Errata ( 1332  3958  816  )

cv - rhel6
30696 Packages
4197 Errata ( 1085  2609  503  )

cv - rhel7
25250 Packages
2109 Errata ( 410  1366  333  )

--- After Reimport (pre snap 3) ---
cv - all
409 Packages
246 Errata ( 42  158  46  )

cv -rhel6
279 Packages
174 Errata ( 32  112  30  )

cv- rhel7
99 Packages
84 Errata ( 16  52  16  )

--- After satellite-instaler --upgrade ---
cv - all
56788 Packages
6106 Errata ( 1332  3958  816  )

cv - rhel6
30696 Packages
4197 Errata ( 1085  2609  503  )

cv - rhel7
25250 Packages
2109 Errata ( 410  1366  333  )


Finally, running the 6.2.7 Snap 3 reimport task no longer produces the errors seen before snap 3.

Comment 16 errata-xmlrpc 2017-01-26 10:47:31 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, 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/RHBA-2017:0197