pulp_rpm icon indicating copy to clipboard operation
pulp_rpm copied to clipboard

Published .treeinfo metadata not matching expectations (remaining issues)

Open fao89 opened this issue 4 years ago • 5 comments

Author: @dralley (dalley)

Redmine Issue: 9208, https://pulp.plan.io/issues/9208


  1. "packages" field on variants may not match with reality in the mirror case. You can see that the "repository" and "packages" fields for the "variant-External" variant which previously had both the repository and packages listed as "../rpm-signed/" now maps to "External" and "External/Packages", respectively. But in the event of mirror sync, this is incorrect.

Likewise, the Land variant is wrong - but this may be a fixture issue. There is no such "Packages" directory. https://fixtures.pulpproject.org/rpm-distribution-tree/variants/land/

 ('change', 'variant-Land.packages', ('Packages', 'Land/Packages')),
 ('change', 'variant-Land.repository', ('variants/land', 'Land')),
 ('change',
  'variant-External.packages',
  ('../rpm-signed/', 'External/Packages')),
 ('change', 'variant-External.repository', ('../rpm-signed/', 'External'))]
  1. The order of the "variants" is different from the original - since productmd sometimes places significance on the first variant in the list, this could possibly have an impact in some cases.
[('change', 'general.variants', ('Land,Sea,External', 'External,Land,Sea')),
 ('change', 'tree.variants', ('Land,Sea,External', 'External,Land,Sea')),

test_publish.py::DistributionTreeMetadataTestCase needs to be updated for both of these

fao89 avatar Dec 22 '21 15:12 fao89

From: @dralley (dalley) Date: 2021-08-04T19:21:35Z


The "packages" issue may extend to the database, it looks like we're saving the rewritten, nonexistent paths in postgresql.

I think that doesn't necessarily impact the metadata generation though.

fao89 avatar Dec 22 '21 15:12 fao89

You can see that the "repository" and "packages" fields for the "variant-External" variant which previously had both the repository and packages listed as "../rpm-signed/" now maps to "External" and "External/Packages", respec tively. But in the event of mirror sync, this is incorrect.

Some notes:

  • In the remote repo, there is this ../rpm-signed/ reference to a subrepo in a parent directory which is outside of the distribution repo.
  • In no-metadata-mirror mode we can include this subrepo in our generated distree-repo and update the treeinfo, so the distribution is still valid.
  • In metadata mirror mode, we can't do this because it violates not touching the metadata.
  • An alternative would be if we could assure a pulp distribution lives in that location and that it refers to the correct subreepo, but that doesn't sound something we would do.

So it seems that we should add another metatada mirror restriction similar to this:

Distribution tree md-mirror sync is not supported if its treeinfo contains reference to external sources (acknowledging a relative path to outside the distribution repository as "external").

Any other thought on this?

pedro-psb avatar Jun 12 '24 15:06 pedro-psb

Thoughts from rpm-team-mtg: we prob need to own/rewrite treeinfo even in the mirror-complete case.

While this "breaks" mirror-complete, the practical reality is "upstreams do the .. Thing in treeinfo, and users want to complete-mirror such upstreams, and treeinfo isn't controlled by (say) metadata-signing" - so, in the interest of not-breaking lots of users, we're just going to be pragmatic and rewrite treeinfo if/when/as we must.

ggainey avatar Jun 13 '24 15:06 ggainey

  1. The order of the "variants" is different from the original - since productmd sometimes places significance on the first variant in the list, this could possibly have an impact in some cases.

Productmd looks optionated to really take the first sorted item. See:

  1. This comment on the treeinfo spec
  2. This implementation of choosing the "main_variant" (aka general.variant)
  3. This commit, which implement a way of overriding this behavior programatically (which we already use here).

Having said that, I wonder if its worth force-keeping the original order, since the thing that could be affected (as far as I can tell, its only the election of general.variant) is already being preserved by the means provided by productmd. Also, we shouldn't care about mirror_complete, since this is already an exceptional case.

I'm inclined to leave these inconsistencies as they are (except for the wrong fixture) and only update related comments/docstrings (maybe linking this issue as justification).

pedro-psb avatar Jun 17 '24 19:06 pedro-psb

I'm inclined to leave these inconsistencies as they are (except for the wrong fixture) and only update related comments/docstrings (maybe linking this issue as justification).

That's completely fair. This was filed when we didn't know if those differences would have any impact, and after a few years nobody has complained.

dralley avatar Jun 17 '24 19:06 dralley