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

Content app auto-distributes latest publication #1328

Merged
merged 1 commit into from
May 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/8760.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Content app auto-distributes latest publication if distribution's ``repository`` field is set
23 changes: 16 additions & 7 deletions pulpcore/content/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
BaseDistribution,
ContentArtifact,
Distribution,
Publication,
Remote,
RemoteArtifact,
)
Expand Down Expand Up @@ -368,6 +369,20 @@ async def _match_and_stream(self, path, request):
headers = self.response_headers(rel_path)

publication = getattr(distro, "publication", None)
repository = getattr(distro, "repository", None)
repo_version = getattr(distro, "repository_version", None)
if repository:
repo_version = repository.latest_version()
# Search for publication serving the closest latest version
if not publication:
try:
versions = repository.versions.all()
publications = Publication.objects.filter(
repository_version__in=versions, complete=True
)
publication = publications.latest("repository_version", "-pulp_created")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add an index for this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've backed off on this, we should investigate indexes but we should probably try to measure the impact rather than adding them speculatively.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added task to keep track of this: https://pulp.plan.io/issues/8777

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to exclude publications where complete=False as well here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah great point!

except ObjectDoesNotExist:
pass

if publication:
if rel_path == "" or rel_path[-1] == "/":
Expand Down Expand Up @@ -419,13 +434,7 @@ async def _match_and_stream(self, path, request):
request, StreamResponse(headers=headers), ca
)

repo_version = getattr(distro, "repository_version", None)
repository = getattr(distro, "repository", None)

if repository or repo_version:
if repository:
repo_version = distro.repository.latest_version()

if repo_version:
gerrod3 marked this conversation as resolved.
Show resolved Hide resolved
if rel_path == "" or rel_path[-1] == "/":
try:
index_path = "{}index.html".format(rel_path)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def test_all(self):

* `Pulp #4186 <https://pulp.plan.io/issues/4186>`_
* `Pulp #8475 <https://pulp.plan.io/issues/8475>`_
* `Pulp #8760 <https://pulp.plan.io/issues/8760>`_

Do the following:

Expand All @@ -38,10 +39,11 @@ def test_all(self):
3. Create 2 distributions - using the same publication. Those
distributions will have different ``base_path``.
4. Assert that distributions have the same publication.
5. Create another distribution using same repository version.
5. Assert that distributions are viewable from base url
6. Assert that content in distributions are viewable
7. Select a content unit. Download that content unit from Pulp using
the two different distributions.
the three different distributions.
Assert that content unit has the same checksum when fetched from
different distributions.
"""
Expand Down Expand Up @@ -72,6 +74,12 @@ def test_all(self):
distributions[0]["publication"], distributions[1]["publication"], distributions
)

body = gen_distribution()
body["repository"] = repo["pulp_href"]
distribution = client.using_handler(api.task_handler).post(FILE_DISTRIBUTION_PATH, body)
distributions.append(distribution)
self.addCleanup(client.delete, distribution["pulp_href"])

client.response_handler = api.safe_handler
self.assertEqual(client.get(PULP_CONTENT_BASE_URL).status_code, 200)

Expand All @@ -89,3 +97,8 @@ def test_all(self):
hashlib.sha256(client.get(unit_urls[1]).content).hexdigest(),
unit_urls,
)
self.assertEqual(
hashlib.sha256(client.get(unit_urls[0]).content).hexdigest(),
hashlib.sha256(client.get(unit_urls[2]).content).hexdigest(),
unit_urls,
)
70 changes: 58 additions & 12 deletions pulpcore/tests/functional/api/using_plugin/test_distributions.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
)
from requests.exceptions import HTTPError

from pulpcore.client.pulp_file import (
DistributionsFileApi,
PublicationsFileApi,
RemotesFileApi,
RepositoriesFileApi,
)
from pulpcore.tests.functional.api.using_plugin.constants import (
FILE_CONTENT_NAME,
FILE_DISTRIBUTION_PATH,
Expand All @@ -28,6 +34,8 @@
from pulpcore.tests.functional.api.using_plugin.utils import (
create_file_publication,
gen_file_remote,
gen_file_client,
monitor_task,
)
from pulpcore.tests.functional.api.using_plugin.utils import set_up_module as setUpModule # noqa
from pulpcore.tests.functional.api.using_plugin.utils import skip_if
Expand Down Expand Up @@ -268,33 +276,44 @@ class ContentServePublicationDistributionTestCase(unittest.TestCase):
This test targets the following issue:

`Pulp #4847 <https://pulp.plan.io/issues/4847>`_
`Pulp #8760 <https://pulp.plan.io/issues/8760>`_
"""

@classmethod
def setUpClass(cls):
"""Create class-wide variables."""
cls.cfg = config.get_config()
cls.client = api.Client(cls.cfg)
cls.file_client = gen_file_client()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice bindings tests, ty!

cls.repo_api = RepositoriesFileApi(cls.file_client)
cls.remote_api = RemotesFileApi(cls.file_client)
cls.pub_api = PublicationsFileApi(cls.file_client)
cls.dis_api = DistributionsFileApi(cls.file_client)

def test_content_served(self):
"""Verify that content is served over publication distribution."""
repo = self.client.post(FILE_REPO_PATH, gen_repo())
self.addCleanup(self.client.delete, repo["pulp_href"])
repo, *_, distribution = self.serve_content_workflow(cleanup=self.addCleanup)

remote = self.client.post(FILE_REMOTE_PATH, gen_file_remote())
self.addCleanup(self.client.delete, remote["pulp_href"])
self.assert_content_served(repo.to_dict(), distribution.to_dict())

sync(self.cfg, remote, repo)
repo = self.client.get(repo["pulp_href"])
def test_autopublish_content_served(self):
"""Verify autopublished content is served over publication distributions."""
body = {"repository": {"autopublish": True, "manifest": "PULP_MANIFEST"}}
repo, _, distro = self.serve_content_workflow(
publish=False, bodies=body, cleanup=self.addCleanup
)

publication = create_file_publication(self.cfg, repo)
self.addCleanup(self.client.delete, publication["pulp_href"])
self.assert_content_served(repo.to_dict(), distro.to_dict())

distribution = self.client.post(
FILE_DISTRIBUTION_PATH, gen_distribution(publication=publication["pulp_href"])
)
self.addCleanup(self.client.delete, distribution["pulp_href"])
def test_nonpublished_content_not_served(self):
"""Verify content that hasn't been published is not served."""
*_, distro = self.serve_content_workflow(publish=False, cleanup=self.addCleanup)

with self.assertRaises(HTTPError):
self.download_pulp_manifest(distro.to_dict())

def assert_content_served(self, repo, distribution):
"""Asserts content served at distribution is correct."""
pulp_manifest = parse_pulp_manifest(self.download_pulp_manifest(distribution))

self.assertEqual(len(pulp_manifest), FILE_FIXTURE_COUNT, pulp_manifest)
Expand All @@ -314,6 +333,33 @@ def download_pulp_manifest(self, distribution):
unit_url = reduce(urljoin, (distribution["base_url"], "PULP_MANIFEST"))
return self.client.get(unit_url)

def serve_content_workflow(self, publish=True, bodies=None, cleanup=None):
"""Creates the workflow for serving content."""
all_bodies = bodies or {}
repo_body = gen_repo(**all_bodies.get("repository", {}))
remote_body = gen_file_remote(**all_bodies.get("remote", {}))
repo = self.repo_api.create(repo_body)
remote = self.remote_api.create(remote_body)
sync(self.cfg, remote.to_dict(), repo.to_dict())
repo = self.repo_api.read(repo.pulp_href)
if publish:
pub = create_file_publication(self.cfg, repo.to_dict())
pub = self.pub_api.read(pub["pulp_href"])
dis_body = {"publication": pub.pulp_href}
else:
dis_body = {"repository": repo.pulp_href}
distro_response = self.dis_api.create(gen_distribution(**dis_body))
distro = self.dis_api.read(monitor_task(distro_response.task).created_resources[0])
if cleanup:
cleanup(self.repo_api.delete, repo.pulp_href)
cleanup(self.remote_api.delete, remote.pulp_href)
cleanup(self.dis_api.delete, distro.pulp_href)
if publish:
cleanup(self.pub_api.delete, pub.pulp_href)
if publish:
return repo, remote, pub, distro
return repo, remote, distro


def parse_pulp_manifest(pulp_manifest):
"""Parse pulp manifest."""
Expand Down