Bug 2263243

Summary: Incremental update of *multiple* CVs with same repo of different content generates wrong katello content
Product: Red Hat Satellite Reporter: Pavel Moravec <pmoravec>
Component: Content ViewsAssignee: satellite6-bugs <satellite6-bugs>
Status: CLOSED ERRATA QA Contact: Sam Bible <sbible>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 6.13.6CC: ahumbe, bbuckingham, damoore, iballou, jfindysz, jpasqual, osousa, rlavi, sbible, vsedmik, zhunting
Target Milestone: 6.15.0Keywords: Triaged
Target Release: Unused   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
: 2266110 (view as bug list) Environment:
Last Closed: 2024-04-23 17:17:49 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:
Attachments:
Description Flags
Hotfix RPM for Satellite 6.14.2 none

Description Pavel Moravec 2024-02-07 19:39:36 UTC 
Description of problem:
Running incremental update of *multiple* CVs (something click-able when applying errata to multiple Hosts that belong to different CVs) can generate increental update with wrong content.

For the sake of simplicity, I will incrementally add just packages, but the same applies to errata.

Assume CV containing a small repo that was gradually published, such that:
- version 1.0 contains one package P1
- version 2.0 contains packages P1 and P2
- the repo itself contains also package P3

Now, adding P3 to *both* CVs altogether via an incremental update, we will always get versions 1.1 and 2.1 with *same* content - either the one corresponding to version 1.1 or to version 2.1. The outcome depends on ordering.

So, we can easily do an incremental update that ends up in much less content.


Version-Release number of selected component (if applicable):
Sat 6.13 or newer (I bet older versions as well)


How reproducible:
100%


Steps to Reproduce:
I wrote very limited reproducer. The original one is "we added an errata to two CCVs that shared a CV in different versions, and incremental updates of the component CV got already bad". A fix should treat this scenario as well (Composite CV, errata and not only packages).

1. Have in /var/tmp/sos_packages/ a few sos packages for a custom product:
sos-4.2-1.el8.noarch.rpm
sos-4.2-3.el8.noarch.rpm
sos-4.3-1.el8.noarch.rpm
sos-4.3-2.el8.noarch.rpm
sos-4.6.1-1.el8.noarch.rpm

2. Have reproducer script that follows the "Assume CV" above, just adds two packages to every CV version (for a clear evidence what incr.update did):

~~~
get_cv_version_id() {
	cv=$1
	vers=$2
	hammer content-view version list --organization-id 1 --content-view $cv | grep $vers | awk '{ print $1 }'
}

# create product and repo, add many sos packages there - iteratively, while creating CV versions for it
hammer product create --organization-id 1 --name sos_product
hammer repository create --organization-id 1 --product sos_product --name sos_repo --content-type yum

hammer repository upload-content --organization-id 1 --product sos_product --name sos_repo --content-type rpm --path /var/tmp/sos_packages/sos-4.2-1.el8.noarch.rpm
hammer repository upload-content --organization-id 1 --product sos_product --name sos_repo --content-type rpm --path /var/tmp/sos_packages/sos-4.2-3.el8.noarch.rpm

repoid=$(hammer repository info --organization-id 1 --product sos_product --name sos_repo | grep -m1 "^Id:" | awk '{ print $2 }')

hammer content-view create --organization-id 1 --name cv_base_sos --repository-ids ${repoid}
hammer content-view publish --organization-id 1 --name cv_base_sos # version 1.0 to contain just sos-4.2-1 and sos-4.2-3
cv_sos_vers1=$(get_cv_version_id cv_base_sos "1\.0")

hammer repository upload-content --organization-id 1 --product sos_product --name sos_repo --content-type rpm --path /var/tmp/sos_packages/sos-4.3-1.el8.noarch.rpm
hammer repository upload-content --organization-id 1 --product sos_product --name sos_repo --content-type rpm --path /var/tmp/sos_packages/sos-4.3-2.el8.noarch.rpm
hammer content-view publish --organization-id 1 --name cv_base_sos # version 2.0 to contain sos-4.2-1, sos-4.5.4-1, sos-4.3-1 and sos-4.3-2
cv_sos_vers2=$(get_cv_version_id cv_base_sos "2\.0")

hammer repository upload-content --organization-id 1 --product sos_product --name sos_repo --content-type rpm --path /var/tmp/sos_packages/sos-4.6.1-1.el8.noarch.rpm # repo contains also sos-4.6.1-1

# package ID of the last RPM, to incrementally add to both CV versions
pkgid=$(hammer package list --organization-id 1 --product sos_product --repository sos_repo | grep sos-4.6.1-1.el8.noarch.rpm | awk '{ print $1 }')

cat > incremental_update.json << EOF
{"add_content": {
    "package_ids": ["${pkgid}"]
  },
  "content_view_version_environments": [
    {
      "content_view_version_id": ${cv_sos_vers2},
      "environment_ids": []
    },
    {
      "content_view_version_id": ${cv_sos_vers1},
      "environment_ids": []
    }
  ],
  "resolve_dependencies": true,
  "organization_id": 1
}
EOF

curl -u admin:redhat -X POST -H "Accept: application/json" -H "Content-Type: application/json" -d @incremental_update.json "https://$(hostname -f)/katello/api/v2/content_view_versions/incremental_update?organization_id=1"

while [ $(hammer task list --search "label = Actions::Katello::ContentView::IncrementalUpdates AND state = running" | grep -c running) -gt 0 ]; do
	sleep 5
done

su - postgres -c "psql foreman -c \"SELECT major,minor,content_counts FROM katello_content_view_versions WHERE content_view_id = (SELECT id FROM katello_content_views WHERE name = 'cv_base_sos') ORDER BY major,minor;\"" | grep -e major -e "---" -e " rpm: "
~~~


3. Now, RESET the reproducer by removing both CVs and the product:
~~~
hammer content-view remove-from-environment --organization-id 1 --name cv_base_sos --lifecycle-environment Library
hammer content-view delete --organization-id 1 --name cv_base_sos
hammer product delete --organization-id 1 --name sos_product
~~~

4. Re-run the reproducer, but with swapped content_view_version_id, i.e.:

~~~
cat > incremental_update.json << EOF
{"add_content": {
    "package_ids": ["${pkgid}"]
  },
  "content_view_version_environments": [
    {
      "content_view_version_id": ${cv_sos_vers1},
      "environment_ids": []
    },
    {
      "content_view_version_id": ${cv_sos_vers2},
      "environment_ids": []
    }
  ],
  "resolve_dependencies": true,
  "organization_id": 1
}
EOF
~~~


Actual results:
2. shows both incremental versions having the expected content of 2.1:

 major | minor |     content_counts
-------+-------+-------------------------
     1 |     0 | ---                    +
       |       | rpm: 2                 +
     1 |     1 | ---                    +
       |       | rpm: 5                 +
     2 |     0 | ---                    +
       |       | rpm: 4                 +
     2 |     1 | ---                    +
       |       | rpm: 5                 +

(we wanted to add *just* one RPM to 1.0, but 1.1 got more!)

4. shows both incremental versions having the expected content of 1.1:

 major | minor |     content_counts      
-------+-------+-------------------------
     1 |     0 | ---                    +
       |       | rpm: 2                 +
     1 |     1 | ---                    +
       |       | rpm: 3                 +
     2 |     0 | ---                    +
       |       | rpm: 4                 +
     2 |     1 | ---                    +
       |       | rpm: 3                 +

(2.1 has less content than 2.0, see?)


Expected results:
2. and 4. to show:

 major | minor |     content_counts      
-------+-------+-------------------------
     1 |     0 | ---                    +
       |       | rpm: 2                 +
     1 |     1 | ---                    +
       |       | rpm: 3                 +
     2 |     0 | ---                    +
       |       | rpm: 4                 +
     2 |     1 | ---                    +
       |       | rpm: 5                 +



Additional info:
Again, this is a very limited reproducer. A fix must cover composite CVs on top of this, and applying errata also.
Comment 1 Pavel Moravec 2024-02-07 19:49:53 UTC 
Hint for developers: katello_repositories.version_href of the "CV version with bad content" is wrongly updated at some stage - it is bumped to the pulp repo version of the *other* repo. See:

after step 2:

foreman=# SELECT id,content_view_version_id,relative_path,version_href FROM katello_repositories WHERE relative_path LIKE 'RedHat/content_views/cv_base_sos/%' ORDER BY relative_path;
  id  | content_view_version_id |                          relative_path                           |                                    version_href                                    
------+-------------------------+------------------------------------------------------------------+------------------------------------------------------------------------------------
 6581 |                     303 | RedHat/content_views/cv_base_sos/1.0/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/6c351027-dbac-412a-9791-42bdc61acb15/versions/2/
 6584 |                     305 | RedHat/content_views/cv_base_sos/1.1/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/b6f874f8-52ec-435b-90c3-b0413ba70c5b/versions/4/
 6583 |                     304 | RedHat/content_views/cv_base_sos/2.0/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/6c351027-dbac-412a-9791-42bdc61acb15/versions/4/
 6585 |                     306 | RedHat/content_views/cv_base_sos/2.1/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/b6f874f8-52ec-435b-90c3-b0413ba70c5b/versions/4/
(4 rows)

.. and after step 4:

foreman=# SELECT id,content_view_version_id,relative_path,version_href FROM katello_repositories WHERE relative_path LIKE 'RedHat/content_views/cv_base_sos/%' ORDER BY relative_path;
  id  | content_view_version_id |                          relative_path                           |                                    version_href                                    
------+-------------------------+------------------------------------------------------------------+------------------------------------------------------------------------------------
 6587 |                     307 | RedHat/content_views/cv_base_sos/1.0/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/de433865-c5b8-46e0-a7dc-23db907dfd1e/versions/2/
 6591 |                     310 | RedHat/content_views/cv_base_sos/1.1/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/cd851165-5690-4484-8932-ef394a1ce3c9/versions/4/
 6589 |                     308 | RedHat/content_views/cv_base_sos/2.0/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/de433865-c5b8-46e0-a7dc-23db907dfd1e/versions/4/
 6590 |                     309 | RedHat/content_views/cv_base_sos/2.1/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/cd851165-5690-4484-8932-ef394a1ce3c9/versions/4/
(4 rows)

Checking task export, I do see that the repo is properly backed by proper pulp repo version first - but then it is somewhere wrongly bumped :( .
Comment 2 Pavel Moravec 2024-02-09 12:52:19 UTC 
I attempted to come up with a fix as well, but I no luck.

The key problem is that expected katello_repository <-> pulp version href is like:

foreman=# SELECT id,content_view_version_id,relative_path,version_href FROM katello_repositories WHERE relative_path LIKE 'RedHat/content_views/cv_base_sos/%' ORDER BY relative_path;
  id  | content_view_version_id |                          relative_path                           |                                    version_href                                    
------+-------------------------+------------------------------------------------------------------+------------------------------------------------------------------------------------
 6677 |                     375 | RedHat/content_views/cv_base_sos/1.0/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/1b484ed7-0edb-4fc0-854a-66298fad7419/versions/2/
 6680 |                     377 | RedHat/content_views/cv_base_sos/1.1/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/a7306586-4201-493d-8503-c879d03ebc73/versions/2/
 6679 |                     376 | RedHat/content_views/cv_base_sos/2.0/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/1b484ed7-0edb-4fc0-854a-66298fad7419/versions/4/
 6681 |                     378 | RedHat/content_views/cv_base_sos/2.1/custom/sos_product/sos_repo | /pulp/api/v3/repositories/rpm/rpm/a7306586-4201-493d-8503-c879d03ebc73/versions/5/
(4 rows)

I.e. repo for CV version 1.1 has pulp repo a7306586-.. version 2, while repo for CV version 2.1 has the same pulp repo but version 5.

But I always see *same* version, either /2/ or /4/ .

That is a consequence of app/services/katello/pulp3/repository/yum.rb ,  multi_copy_units being always called with

:config=>[{:source_repo_version=>"/pulp/api/v3/repositories/rpm/rpm/71d750a9-7170-4452-9f7c-349ba375da93/versions/5/", :dest_repo=>"/pulp/api/v3/repositories/rpm/rpm/a482a20f-fa5d-4011-82bd-ecdce97a1153/", :content=>["/pulp/api/v3/content/rpm/packages/c35f842e-dbbb-4988-a228-27883b25713d/"]

When called during the reproducer - twice in the same way for both minor CV version repos - pulp responds with "here is version 4":

 27: Actions::Pulp3::Repository::MultiCopyUnits (success) [ 1.27s / 0.50s ]
---
repo_map:
  ? - 6664
  : dest_repo: 6668
..
 Output:
---
pulp_tasks:
- pulp_href: "/pulp/api/v3/tasks/4b86ec76-40a8-43a3-bae0-daabcdcf4918/"
  pulp_created: '2024-02-09T10:38:46.503+00:00'
  state: completed
  name: pulp_rpm.app.tasks.copy.copy_content
  logging_cid: 515ef89d-32aa-4ec8-b476-0422ba2518ab
  started_at: '2024-02-09T10:38:46.546+00:00'
  finished_at: '2024-02-09T10:38:46.734+00:00'
  worker: "/pulp/api/v3/workers/19d83007-2c5a-448d-9ced-4ad786c4245b/"
  child_tasks: []
  progress_reports: []
  created_resources:
  - "/pulp/api/v3/repositories/rpm/rpm/ac97b172-8dcb-49b8-acfc-c6fd0ba60fd7/versions/4/"
  reserved_resources_record:
  - "/pulp/api/v3/repositories/rpm/rpm/ac97b172-8dcb-49b8-acfc-c6fd0ba60fd7/"
  - shared:/pulp/api/v3/repositories/rpm/rpm/8694206a-af14-44ad-916e-e697adc64e56/

once and "no new version needed" next time:

 70: Actions::Pulp3::Repository::MultiCopyUnits (success) [ 1.71s / 0.49s ] 
---
repo_map:
  ? - 6664
  : dest_repo: 6669
..
 Output:
---
pulp_tasks:
- pulp_href: "/pulp/api/v3/tasks/81ae8a8d-9253-4bfd-8f89-183d6d8d62e0/"
  pulp_created: '2024-02-09T10:38:46.970+00:00'
  state: completed
  name: pulp_rpm.app.tasks.copy.copy_content
  logging_cid: 515ef89d-32aa-4ec8-b476-0422ba2518ab
  started_at: '2024-02-09T10:38:47.019+00:00'
  finished_at: '2024-02-09T10:38:47.237+00:00'
  worker: "/pulp/api/v3/workers/cc7ac605-2e70-4e23-aa08-5aecaadc4f97/"
  child_tasks: []
  progress_reports: []
  created_resources: []
  reserved_resources_record:
  - "/pulp/api/v3/repositories/rpm/rpm/ac97b172-8dcb-49b8-acfc-c6fd0ba60fd7/"
  - shared:/pulp/api/v3/repositories/rpm/rpm/8694206a-af14-44ad-916e-e697adc64e56/

The "no new version needed" is the bad thing here.


The pulp tasks dont overlap, but still the later one does not create a new repo version. So the later katello repo gets the same pulp repo version like the other katello repo - that is the BAD.


I tried also replacing in IncrementalUpdates the "concurrence do .. plan_action(ContentViewVersion::IncrementalUpdate, .." by calling the IncrementalUpdate sequentially - and this *helps*.

So a patch like:

--- /usr/share/gems/gems/katello-4.7.0.35/app/lib/actions/katello/content_view/incremental_updates.rb.orig	2024-02-09 13:46:29.362462539 +0100
+++ /usr/share/gems/gems/katello-4.7.0.35/app/lib/actions/katello/content_view/incremental_updates.rb.new	2024-02-09 13:46:39.303519075 +0100
@@ -10,7 +10,7 @@ module Actions
           output_for_version_ids = []
 
           sequence do
-           concurrence do
+	    sequence do
               version_environments.each do |version_environment|
                 version = version_environment[:content_view_version]
                 if version.content_view.generated?

does help, but we sacrifise performance.

Well, we can call "sequence do" just when a repo is present multiple times, and "concurrence" otherwise..?
Comment 6 Ian Ballou 2024-02-15 15:32:50 UTC 
This upstream issue with an open PR may be related: https://projects.theforeman.org/issues/36871
Comment 7 Ian Ballou 2024-02-15 15:40:14 UTC 
From reading the report, it sounds like "multiple CVs" actually means "multiple CVVs (content view versions)" within a single content view. If that is true, then I'm even more confident that https://projects.theforeman.org/issues/36871 is related.
Comment 8 Pavel Moravec 2024-02-15 17:22:21 UTC 
(In reply to Ian Ballou from comment #7)
> From reading the report, it sounds like "multiple CVs" actually means
> "multiple CVVs (content view versions)" within a single content view. If
> that is true, then I'm even more confident that
> https://projects.theforeman.org/issues/36871 is related.

Indeed, that matches. And the PR seems to be in the right direction!

Will the fix work also when called on incremental update of Composite CVs (that contain two different versions of CV that would suffer the problem)? I will test it in a few days..
Comment 10 Ian Ballou 2024-02-15 19:08:36 UTC 
> Will the fix work also when called on incremental update of Composite CVs (that contain two different versions of CV that would suffer the problem)?

That's a good question, I'll take it into account when testing the PR.
Comment 13 Pavel Moravec 2024-02-16 12:14:54 UTC 
I can confirm the backport of upstream fix https://github.com/Katello/katello/pull/10781/files works great on either simple CV and also on a Composite CV. Backport tried on Sat6.13, it is straightforwardly applicable to any 6.13+ version.
Comment 14 Ian Ballou 2024-02-22 23:06:26 UTC 
Created attachment 2018232 [details]
Hotfix RPM for Satellite 6.14.2

A hotfix RPM is available for this BZ for Satellite 6.14.2 on RHEL8

INSTALL INSTRUCTIONS (Satellite 6.14.2 on RHEL8):

1. Take a complete backup or snapshot of Satellite 6.14.2 server

2. Download the hotfix RPM from this attachment

3. # dnf install ./rubygem-katello-4.9.0.21-2.HOTFIXRHBZ2263243.el8sat.noarch.rpm --disableplugin=foreman-protector

4. # satellite-maintain service restart
Comment 16 Sam Bible 2024-03-13 20:18:15 UTC 
Verified on 6.15 - 10.1

Steps to Verify:
1. Setup the system as mentioned above in the reproducer:
    - Create a folder containing 5 different versions of the sos rpm
    - Create the reproducer script 
2. Follow the steps mentioned above:
    - Run the reproducer script
    - Clean out the created artifacts
    - Run it again, with the versions swapped


Expected Results: 
The versions have the expected content count, where each version correctly increments up one package in the x.1 version, instead of copying either 1.0 or 2.0s packages.

Actual Results:
The versions have the expected content count, where each version correctly increments up one package in the x.1 version, instead of copying either 1.0 or 2.0s packages.
Comment 19 errata-xmlrpc 2024-04-23 17:17:49 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.15.0 release), 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-2024:2010