Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mirrored repositories for repos with distribution trees #2044

Merged
merged 1 commit into from Jul 21, 2021

Conversation

dralley
Copy link
Contributor

@dralley dralley commented Jul 19, 2021

Fix mirrored repositories for repos with distribution trees

Two problems:

  • Mirrored repositories were creating one publication per
    sub-repository, which is in contrast to the way that pulp normally
    works. Standard publish puts the sub-repos into the same publication
    in a direct sub-directory because this is the only way to have
    everything distributed properly. Otherwise the other publications are
    in-accessible.
  • Even once we fix that, we still have the issue that the original
    .treeinfo contains the original relative paths. So even for the
    mirrored case, we still have to rewrite the .treeinfo data to look at
    the new repos. Luckily it's not signed or checksummed so we aren't
    prevented from doing this.

closes: #9098
https://pulp.plan.io/issues/9098

@dralley dralley requested a review from goosemania July 19, 2021 03:10
@dralley
Copy link
Contributor Author

dralley commented Jul 19, 2021

Note that problem #2 is not fixed yet in this PR. @goosemania I'm curious if you have any thoughts about it.

Sadness all around. This has taught me how little I actually understand the mechanics of distribution trees, so I would appreciate more knowledgeable people checking my work here :(

@pulpbot
Copy link
Member

pulpbot commented Jul 19, 2021

Attached issue: https://pulp.plan.io/issues/9098

@goosemania
Copy link
Member

@dralley, I agree with both problem statements. It's a pity I missed they obvious (to me) first one during code review, apologies for that.
I think the second problem is only for the centos8 case where the path to the AppStream is relative, e.g. ../../../../AppStream or something like that. For other repositories, I think no modification is needed, aka we need to fix it but the majority will work as is.
@dkliban is more knowledgeable in disttree metadata modifications, so he might see/remember more than I do.

@dralley
Copy link
Contributor Author

dralley commented Jul 19, 2021

I think the second problem is only for the centos8 case where the path to the AppStream is relative, e.g. ../../../../AppStream or something like that.

I assume that we would still need it to avoid clients hitting the original repos? Thinking about the disconnected use case, it doesn't matter if the upstream provided an absolute URL or not, because that repo is in-accessible.

Or do you mean relative backwards, rather than hosted in the same repo.

@goosemania
Copy link
Member

goosemania commented Jul 20, 2021

I think the second problem is only for the centos8 case where the path to the AppStream is relative, e.g. ../../../../AppStream or something like that.

I assume that we would still need it to avoid clients hitting the original repos? Thinking about the disconnected use case, it doesn't matter if the upstream provided an absolute URL or not, because that repo is in-accessible.

Or do you mean relative backwards, rather than hosted in the same repo.

IIUC your last sentence, yes, the ones outside of a repo.

I was talking about paths in .treeinfo, they are always relative to the root of a repo, so you won't hit any original repos any way but the centos8 case is super weird and I think it's the main (if not the only) reason we have to rewrite the paths in treeinfo.
Let me try to show what I mean, I'm hoping not to confuse you.

I think we have all the options in our fixtures https://fixtures.pulpproject.org/rpm-distribution-tree/.treeinfo :

  • main variant
[variant-Sea]
addons = Sea-Dolphin,Sea-Whale
id = Sea
name = Sea
packages = Packages
repository = .        <---- path within the repo, we do not need to rewrite anything
type = variant
uid = Sea
  • alternative variant "normal"
[variant-Land]
id = Land
name = Land
packages = Packages       <---- this is actually a mistake in our fixtures I think :/ it should be `variants/land`
repository = variants/land  <---- path within the repo, we do not need to rewrite anything
type = variant
uid = Land
  • alternative variant "abnormal aka centos8"
[variant-External]
id = External
name = External
packages = ../rpm-signed/   <---- this needs to be rewritten to the path below + /Packages
repository = ../rpm-signed/   <---- path is outside of the repo, we need to rewrite it because we publish any variant inside the repo
type = variant
uid = External

So in the majority of cases original paths to repository and packages will work as is, but in some cases they need to be rewritten:

  • case 1, variant or addon are not within the repo path
  • case 2, packages path doesn't have the pattern <repository_path>/Packages

From the Implementation point of view it's easier/cleaner to rewrite paths every time I think but these 2 cases are the reason why we are doing it.

Mirrored repository syncs were creating one publication per
sub-repository, which is in contrast to the way that pulp normally
works. Standard publish puts the sub-repos into the same publication
in a direct sub-directory because this is the only way to have
everything distributed properly. Otherwise the other publications are
in-accessible.

closes: #9098
https://pulp.plan.io/issues/9098
@goosemania
Copy link
Member

We agreed to fix problem 2 separately

@dralley
Copy link
Contributor Author

dralley commented Jul 21, 2021

Thank you @goosemania

@dralley dralley merged commit b7ba381 into pulp:master Jul 21, 2021
@dralley dralley deleted the mirror-publication branch July 21, 2021 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants