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

Add support for Replicate feature #1650

Merged
merged 1 commit into from
Jun 20, 2024
Merged

Conversation

MichalPysik
Copy link
Member

@MichalPysik MichalPysik commented May 28, 2024

Pulp Container now supports the Replication feature.

closes #1648

@MichalPysik MichalPysik force-pushed the replication_feature branch 5 times, most recently from 4b537c8 to f7b6439 Compare June 3, 2024 12:39
@MichalPysik MichalPysik force-pushed the replication_feature branch 3 times, most recently from cc568d2 to ab9918e Compare June 7, 2024 10:09
@MichalPysik MichalPysik marked this pull request as ready for review June 7, 2024 10:12
@MichalPysik MichalPysik requested a review from lubosmj June 7, 2024 10:12
@MichalPysik MichalPysik force-pushed the replication_feature branch from ab9918e to 49a0099 Compare June 7, 2024 10:12
pulp_container/app/replica.py Outdated Show resolved Hide resolved


class ContainerReplicator(Replicator):
distribution_ctx_cls = PulpContainerDistributionContext
Copy link
Member

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

pulp_container/app/replica.py Outdated Show resolved Hide resolved
Copy link
Member

@lubosmj lubosmj left a 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
}

CHANGES/1648.feature Outdated Show resolved Hide resolved
pulp_container/app/replica.py Outdated Show resolved Hide resolved
pulp_container/app/replica.py Outdated Show resolved Hide resolved
return {
"repository": get_url(repository),
"base_path": upstream_distribution["base_path"],
}
Copy link
Member

@lubosmj lubosmj Jun 7, 2024

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.

Copy link
Member

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

Copy link
Member Author

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?

@MichalPysik MichalPysik force-pushed the replication_feature branch from 49a0099 to d733835 Compare June 7, 2024 11:58
@MichalPysik
Copy link
Member Author

MichalPysik commented Jun 7, 2024

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
}

@lubosmj this happens:

Error: Task group /pulp/api/v3/task-groups/018ff2bd-dd87-7ce9-97c3-13b4d83c28a1/ has failed/canceled tasks: ''publication''

I guess there's a bug to be fixed

Edit: The bug has been killed, doesn't even move its legs anymore 🐛

@MichalPysik MichalPysik force-pushed the replication_feature branch 6 times, most recently from a98f10d to 9fb8cf6 Compare June 10, 2024 16:59
@lubosmj
Copy link
Member

lubosmj commented Jun 10, 2024

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.

@MichalPysik MichalPysik force-pushed the replication_feature branch 4 times, most recently from bfc0640 to 6a7cb4a Compare June 11, 2024 13:43
@MichalPysik MichalPysik force-pushed the replication_feature branch from 6a7cb4a to b93e7c2 Compare June 11, 2024 14:44
@MichalPysik MichalPysik force-pushed the replication_feature branch 2 times, most recently from 2aedb19 to 5da02ce Compare June 18, 2024 12:15
pulp_container/app/replica.py Outdated Show resolved Hide resolved
@MichalPysik MichalPysik force-pushed the replication_feature branch from 5da02ce to 36a00bd Compare June 20, 2024 12:33
Pulp Container now supports the Replication feature.

closes pulp#1648
@MichalPysik MichalPysik force-pushed the replication_feature branch from 3a03be6 to d784190 Compare June 20, 2024 14:18
@lubosmj lubosmj merged commit ce05037 into pulp:main Jun 20, 2024
16 checks passed
@MichalPysik MichalPysik deleted the replication_feature branch June 20, 2024 15:47
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.

Add support for the Replication feature
3 participants