-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add support for Replicate feature #1650
Conversation
4b537c8
to
f7b6439
Compare
cc568d2
to
ab9918e
Compare
ab9918e
to
49a0099
Compare
|
||
|
||
class ContainerReplicator(Replicator): | ||
distribution_ctx_cls = PulpContainerDistributionContext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pulp container plugin contains 2 distribution types(regular and pull-thru), if we replicate one pulp to another then we will create only regular distribution type and ignore pull-thru cache ones, which is probably fine(?) because we will mirror its 'sub-distributions', thoughts?
As a result of this replication process, the replicated instance will contain only regular repos, remotes and distributions and it will work fine, just it will not be a complete replication of original pulp and definitely these 2 instances will not be interchangeable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! 🧑🍳 I left a couple of suggestions. It is so unfortunate that we cannot test the changes in our CI. In pulp_gem, we utilized domains to mock different Pulp instances.
Can you test what happens if a user wants to replicate a distribution that does not serve any content?
pulp container distribution create --name bla --base-path bla
Started background task /pulp/api/v3/tasks/018ff259-9cc2-7e12-882a-5f8cae23e223/
Done.
{
"pulp_created": "2024-06-07T10:58:30.298313Z",
"content_guard": "/pulp/api/v3/contentguards/core/content_redirect/018fedd3-7710-72da-b2bb-0a807a6a5cc2/",
"name": "bla",
"pulp_last_updated": "2024-06-07T10:58:30.298333Z",
"pulp_labels": {},
"repository": null,
"hidden": false,
"pulp_href": "/pulp/api/v3/distributions/container/container/018ff259-9d19-7b34-82d3-5287faaa60cf/",
"base_path": "bla",
"repository_version": null,
"registry_path": "localhost:5001/bla",
"remote": null,
"namespace": "/pulp/api/v3/pulp_container/namespaces/018ff259-9c9c-79ae-a41a-7b4dacbda975/",
"private": false,
"description": null
}
pulp_container/app/replica.py
Outdated
return { | ||
"repository": get_url(repository), | ||
"base_path": upstream_distribution["base_path"], | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to add the private
attribute here? I assume yes: https://staging-docs.pulpproject.org/pulp_container/docs/admin/learn/rbac/?h=private#private-repositories, https://github.com/pulp/pulpcore/blob/02bb455315a17a39cb0d936a12a13fbbdb887bb3/pulpcore/app/replica.py#L190. Though, I am not sure how replicas work with users' permissions. Once a distribution is private, and no users' permissions were "replicated", only admins can access the distribution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should replicate objects with same options, if it was private upstream then it should be private downstream. the admin will need to set proper permissions downstream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and what about the description
field which container distributions have?
49a0099
to
d733835
Compare
@lubosmj this happens:
I guess there's a bug to be fixed Edit: The bug has been killed, doesn't even move its legs anymore 🐛 |
a98f10d
to
9fb8cf6
Compare
Do you think we should bump up the minimal pulpcore requirement too? The only supported version we can benefit from is 3.49, after backporting the fix to older pulpcore versions. I added the backport label to https://github.com/pulp/pulpcore/pull/5465/files. |
bfc0640
to
6a7cb4a
Compare
6a7cb4a
to
b93e7c2
Compare
2aedb19
to
5da02ce
Compare
5da02ce
to
36a00bd
Compare
Pulp Container now supports the Replication feature. closes pulp#1648
3a03be6
to
d784190
Compare
Pulp Container now supports the Replication feature.
closes #1648