Bug 1899310

+++ This bug was initially created as a clone of Bug #1879126 +++

Description of problem:
There are few scenarios leading to a redundant "real-op" publish(*) of a repo in pulp. 

(recall: each repo sync automatically triggers publish of the repo; if nothing has changed, the publish is no-op - BUT the decision "nothing has changed" wrongly detects "a change" even if the sync didnt update the repo at all)

Particular reproducer:
- republish repository metadata of a repo
  - this will forcefully publish the repo
- then invoke optimised repo sync
  - this wont sync anything new, BUT the repo will be re-published, redundantly

(alternate reproducer: do a full repo sync, followed by optimised/normal repo sync - again, the 2nd (no-op) sync triggers "real-op" publish)

The cause is here: https://github.com/pulp/pulp/blob/2-master/server/pulp/server/controllers/repository.py#L1201

Here, we compare previous and current publish override options. So if a new repo sync does not change anything but it was called with different (even empty) override config, the repo *will be re-published - since same_override=False and https://github.com/pulp/pulp/blob/2-master/server/pulp/server/controllers/repository.py#L1215 evals to False .


As a consequence, redundant repo is re-published (which can also trigger redundant sync of the repo to Capsule, after all).


Version-Release number of selected component (if applicable):
Sat 6.7
pulp-server-2.21.0.1-1.el7sat.noarch


How reproducible:
100%


Steps to Reproduce:
1. Have a fully synced repo in Sat
2. Do either action:
  - invoke Complete Sync
  - or Republish Repository metadata
3. Check the repo was re-published
4. Invoke a regular sync
5. Check the repo was republished

Standalone pulp minimalistic reproducer: call repo publish once with override_config\":{\"force_full\":true}, then without it (or with force_full:false) - even the 2nd publish will be a "real op".


Actual results:
5. Repo was republished


Expected results:
5. Repo isnt republished


Additional info:
I understand there can be legit situations where diff between previous and current override_config *must* result in re-publish. I just dont know them and the current logic is simply too strict..

--- Additional comment from  on 2020-09-17T14:44:25Z 

Some minimalistic patch to ignore _changes_ in force_full override config:


--- a/pulp/server/controllers/repository.py
+++ b/pulp/server/controllers/repository.py
@@ -1160,13 +1160,17 @@ def check_publish(repo_obj, dist_id, dis
         skip_for_predistributor = published_after_predistributor or not \
             predistributor_last_published
 
-    same_override = dist.last_override_config == config_override
-    if not same_override:
+    if not (dist.last_override_config == config_override):
         # Use raw pymongo not to fire the signal hander
         model.Distributor.objects(
             repo_id=repo_obj.repo_id,
             distributor_id=dist_id).update(set__last_override_config=config_override)
 
+    # do not compare force_full from config override - this particular difference itself does not
+    # determine the need of repo publish
+    config_override = {k: v for k, v in config_override.items() if k != 'force_full'}
+    dist.last_override_config = {k: v for k, v in dist.last_override_config.items() if k != 'force_full'}
+    same_override = dist.last_override_config == config_override
     # Check if content has not changed since last publish and a predistributor is not defined.
     unchanged_content_and_no_predistributor = last_published and not last_updated and \
         not units_removed and not predistributor_id


(a review from pulp devels is welcomed, though..)

--- Additional comment from  on 2020-09-18T06:43:21Z 

Pavel, thanks for the patch. It looks reasonable to me.
Feel free to open the upstream PR. I'll file the upstream bug.

--- Additional comment from  on 2020-09-18T07:05:41Z 

The Pulp upstream bug status is at NEW. Updating the external tracker on this bug.

--- Additional comment from  on 2020-09-18T07:05:42Z 

The Pulp upstream bug priority is at Normal. Updating the external tracker on this bug.

--- Additional comment from  on 2020-09-29T20:16:26Z 

(In reply to Tanya Tereshchenko from comment #2)
> Pavel, thanks for the patch. It looks reasonable to me.
> Feel free to open the upstream PR. I'll file the upstream bug.

Here you are :)

https://github.com/pulp/pulp/pull/3999


(majority of redundant repo publishes were in my case really due to config_override altering from {} to {force_full=False})

--- Additional comment from  on 2020-10-22T19:06:18Z 

The Pulp upstream bug status is at POST. Updating the external tracker on this bug.

--- Additional comment from  on 2020-10-26T13:06:45Z 

The Pulp upstream bug status is at MODIFIED. Updating the external tracker on this bug.

--- Additional comment from  on 2020-10-26T14:07:41Z 

All upstream Pulp bugs are at MODIFIED+. Moving this bug to POST.

--- Additional comment from  on 2020-11-02T16:08:24Z 

The Pulp upstream bug status is at CLOSED - CURRENTRELEASE. Updating the external tracker on this bug.

--- Additional comment from  on 2020-11-04T22:19:28Z 

*** Satellite 6.7.4 and 6.7.5 Hotfix Available ***

1) Download pulp-server-2.21.0.4-2.HOTFIXRHBZ1879126.el7sat.noarch.rpm from this bugzilla to your Satellite

2) stop services:

satellite-maintain service stop

3) Install:

rpm -Uvh pulp-server-2.21.0.4-2.HOTFIXRHBZ1879126.el7sat.noarch.rpm

4) restart:

satellite-maintain service start

5) resume operations

--- Additional comment from  on 2020-11-04T22:20:26Z 

Created attachment 1726745 [details]
pulp-server-2.21.0.4-2.HOTFIXRHBZ1879126.el7sat.noarch.rpm