Skip to content

Commit 023e815

Browse files
committed
Fix an edge case where cache wasn't flushed
Make sure we flush the cache (and update DistributedPublication) for publications which were indirectly served via Distribution.repository_version, rather than directly. closes #7993 Assisted-By: Claude Opus 4.6
1 parent 997120d commit 023e815

3 files changed

Lines changed: 173 additions & 12 deletions

File tree

CHANGES/7993.bugfix

Whitespace-only changes.

pulpcore/app/models/publication.py

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,20 @@ def delete(self, **kwargs):
177177
except Publication.DoesNotExist:
178178
pass
179179

180+
# A distribution serving this publication's version directly (repository_version)
181+
# resolves to the latest publication of that version. Invalidate those distributions
182+
# when this is that latest publication.
183+
try:
184+
version_latest = Publication.objects.filter(
185+
repository_version=self.repository_version, complete=True
186+
).latest("pulp_created")
187+
if self.pk == version_latest.pk:
188+
base_paths |= Distribution.objects.filter(
189+
repository_version=self.repository_version
190+
).values_list("base_path", flat=True)
191+
except Publication.DoesNotExist:
192+
pass
193+
180194
# Invalidate cache for all distributions serving this publication
181195
if base_paths:
182196
Cache().delete(base_key=cache_key(base_paths))
@@ -234,17 +248,16 @@ def __exit__(self, exc_type, exc_val, exc_tb):
234248
self.delete()
235249
raise
236250

237-
# Create distributed publication for repository auto-publish scenario
251+
# Refresh distributed publications for the auto-publish scenario. A distribution can
252+
# distribute this publication indirectly either through its repository (latest
253+
# publication of the latest version) or through its repository_version (latest
254+
# publication of that version), without the distribution itself changing.
238255
if retain_distributed_pub_enabled():
239-
for distro in Distribution.objects.filter(repository=self.repository):
240-
detail_distro = distro.cast()
241-
if not detail_distro.SERVE_FROM_PUBLICATION:
242-
continue
243-
_, _, latest_repo_publication = (
244-
detail_distro.get_repository_publication_and_version()
245-
)
246-
if self == latest_repo_publication:
247-
DistributedPublication(distribution=distro, publication=self).save()
256+
for distro in Distribution.objects.filter(
257+
models.Q(repository=self.repository_version.repository)
258+
| models.Q(repository_version=self.repository_version)
259+
):
260+
distro.set_distributed_publication()
248261

249262
# Unmark old checkpoints if retention is configured
250263
if self.checkpoint:
@@ -254,7 +267,8 @@ def __exit__(self, exc_type, exc_val, exc_tb):
254267
# invalidate cache
255268
if settings.CACHE_ENABLED:
256269
base_paths = Distribution.objects.filter(
257-
repository=self.repository_version.repository
270+
models.Q(repository=self.repository_version.repository)
271+
| models.Q(repository_version=self.repository_version)
258272
).values_list("base_path", flat=True)
259273
if base_paths:
260274
Cache().delete(base_key=cache_key(base_paths))
@@ -800,13 +814,24 @@ def get_fallback_ca(self, path):
800814
is_not=None,
801815
)
802816
def set_distributed_publication(self):
803-
"""Track the publication being served when a distribution is created or changed."""
817+
"""
818+
Track the publication currently served by this distribution.
819+
820+
Records a DistributedPublication for the publication this distribution resolves to
821+
(directly, or indirectly via its repository/repository_version). Idempotent: does
822+
nothing if the active DistributedPublication already points at that publication.
823+
"""
804824
detail = self.cast()
805825
if not detail.SERVE_FROM_PUBLICATION or not retain_distributed_pub_enabled():
806826
return
807827
_, _, pub = detail.get_repository_publication_and_version()
808828
if pub is None:
809829
return
830+
already_current = DistributedPublication.objects.filter(
831+
distribution=self, publication=pub, expires_at__isnull=True
832+
).exists()
833+
if already_current:
834+
return
810835
DistributedPublication(distribution=self, publication=pub).save()
811836

812837
@hook(

pulpcore/tests/unit/models/test_publication_retention.py

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import hashlib
22
import uuid
3+
from datetime import timedelta
4+
from unittest import mock
35

46
import pytest
57

@@ -9,6 +11,7 @@
911
DistributedPublication,
1012
PublishedArtifact,
1113
)
14+
from pulpcore.app.util import cache_key
1215

1316
from pulp_file.app.models import (
1417
FileContent,
@@ -59,6 +62,34 @@ def update_dist(dist, repo=UNSET, repover=UNSET, pub=UNSET):
5962
dist.save()
6063

6164

65+
def publish(repo_version, pass_through=True):
66+
"""
67+
Create and complete a publication through the Publication context manager.
68+
69+
`CreatedResource` is mocked out because it requires a current Task, which isn't set up in
70+
unit tests. This still runs the `__exit__` finalization that invalidates caches and records
71+
distributed publications.
72+
"""
73+
with mock.patch("pulpcore.app.models.publication.CreatedResource"):
74+
with FilePublication.create(repo_version, pass_through=pass_through) as pub:
75+
pass
76+
return pub
77+
78+
79+
def invalidated_base_paths(mock_cache):
80+
"""Collect the set of base_paths passed to a mocked ``Cache().delete``."""
81+
paths = set()
82+
for call in mock_cache.return_value.delete.call_args_list:
83+
base_key = call.kwargs.get("base_key")
84+
if base_key is None and call.args:
85+
base_key = call.args[0]
86+
if isinstance(base_key, str):
87+
paths.add(base_key)
88+
elif base_key is not None:
89+
paths.update(base_key)
90+
return paths
91+
92+
6293
def create_version(repo, add=None, remove=None):
6394
"""
6495
Create a RepositoryVersion adding and/or removing content by path.
@@ -137,6 +168,25 @@ def test_unaffected_when_older_repository_version_deleted(self, db):
137168
v1.delete()
138169
assert DistributedPublication.objects.filter(distribution=dist).count() == 1
139170

171+
def test_created_when_new_publication_for_distributed_repository(self, db):
172+
# A distribution serving a repository directly indirectly distributes the latest
173+
# publication. Creating a new publication should record it as distributed.
174+
repo = FileRepository.objects.create(name=f"repo-{uuid.uuid4().hex[:8]}")
175+
version = create_version(repo, add=["some-file.txt"])
176+
dist = dist_factory(repo=repo)
177+
pub = publish(version)
178+
assert DistributedPublication.objects.filter(distribution=dist, publication=pub).exists()
179+
180+
def test_created_when_new_publication_for_distributed_repository_version(self, db):
181+
# A distribution serving a repository_version (with SERVE_FROM_PUBLICATION) indirectly
182+
# distributes the latest publication of that version. Creating a new publication should
183+
# record it as distributed, just like the repository case above.
184+
repo = FileRepository.objects.create(name=f"repo-{uuid.uuid4().hex[:8]}")
185+
version = create_version(repo, add=["some-file.txt"])
186+
dist = dist_factory(repover=version)
187+
pub = publish(version)
188+
assert DistributedPublication.objects.filter(distribution=dist, publication=pub).exists()
189+
140190

141191
@pytest.mark.django_db
142192
class TestGetFallbackCa:
@@ -210,3 +260,89 @@ def version_without_content(self, version_with_content):
210260
@pytest.fixture
211261
def expected_ca(self, version_with_content):
212262
return ContentArtifact.objects.get(relative_path=self.content_path)
263+
264+
265+
@pytest.mark.django_db
266+
class TestCacheInvalidationOnPublicationCreate:
267+
"""
268+
Creating a new publication changes the content served by every distribution that
269+
indirectly distributes that publication's repository/repository_version. The cache
270+
must be invalidated for all of them, not just the ones with a direct ``repository`` FK.
271+
"""
272+
273+
def test_invalidates_repository_distribution(self, settings):
274+
settings.CACHE_ENABLED = True
275+
repo = FileRepository.objects.create(name=f"repo-{uuid.uuid4().hex[:8]}")
276+
version = create_version(repo, add=["some-file.txt"])
277+
dist = dist_factory(repo=repo)
278+
with mock.patch("pulpcore.app.models.publication.Cache") as mock_cache:
279+
publish(version)
280+
assert cache_key(dist.base_path) in invalidated_base_paths(mock_cache)
281+
282+
def test_invalidates_repository_version_distribution(self, settings):
283+
settings.CACHE_ENABLED = True
284+
repo = FileRepository.objects.create(name=f"repo-{uuid.uuid4().hex[:8]}")
285+
version = create_version(repo, add=["some-file.txt"])
286+
dist = dist_factory(repover=version)
287+
with mock.patch("pulpcore.app.models.publication.Cache") as mock_cache:
288+
publish(version)
289+
assert cache_key(dist.base_path) in invalidated_base_paths(mock_cache)
290+
291+
292+
@pytest.mark.django_db
293+
class TestCacheInvalidationOnPublicationDelete:
294+
"""
295+
Deleting the publication currently served by a distribution changes what that distribution
296+
serves, so its cache must be invalidated -- including distributions that serve the
297+
publication indirectly through their repository_version.
298+
"""
299+
300+
def test_invalidates_repository_distribution(self, settings):
301+
settings.CACHE_ENABLED = True
302+
repo = FileRepository.objects.create(name=f"repo-{uuid.uuid4().hex[:8]}")
303+
version = create_version(repo, add=["some-file.txt"])
304+
pub = pub_factory(version, pass_through=True)
305+
dist = dist_factory(repo=repo)
306+
with mock.patch("pulpcore.app.models.publication.Cache") as mock_cache:
307+
pub.delete()
308+
assert cache_key(dist.base_path) in invalidated_base_paths(mock_cache)
309+
310+
def test_invalidates_repository_version_distribution(self, settings):
311+
settings.CACHE_ENABLED = True
312+
repo = FileRepository.objects.create(name=f"repo-{uuid.uuid4().hex[:8]}")
313+
version = create_version(repo, add=["some-file.txt"])
314+
pub = pub_factory(version, pass_through=True)
315+
dist = dist_factory(repover=version)
316+
with mock.patch("pulpcore.app.models.publication.Cache") as mock_cache:
317+
pub.delete()
318+
assert cache_key(dist.base_path) in invalidated_base_paths(mock_cache)
319+
320+
def test_does_not_invalidate_repository_version_distribution_for_other_version(self, settings):
321+
# Deleting a publication of a different version must not invalidate a distribution that
322+
# serves an unrelated repository_version.
323+
settings.CACHE_ENABLED = True
324+
repo = FileRepository.objects.create(name=f"repo-{uuid.uuid4().hex[:8]}")
325+
version1 = create_version(repo, add=["v1.txt"])
326+
version2 = create_version(repo, add=["v2.txt"])
327+
pub2 = pub_factory(version2, pass_through=True)
328+
dist = dist_factory(repover=version1)
329+
with mock.patch("pulpcore.app.models.publication.Cache") as mock_cache:
330+
pub2.delete()
331+
assert cache_key(dist.base_path) not in invalidated_base_paths(mock_cache)
332+
333+
def test_does_not_invalidate_when_deleting_superseded_publication(self, settings):
334+
# Deleting an older (non-latest) publication of the served version must not invalidate the
335+
# repository_version distribution, which still serves the newer publication.
336+
settings.CACHE_ENABLED = True
337+
repo = FileRepository.objects.create(name=f"repo-{uuid.uuid4().hex[:8]}")
338+
version = create_version(repo, add=["some-file.txt"])
339+
old_pub = pub_factory(version, pass_through=True)
340+
new_pub = pub_factory(version, pass_through=True)
341+
# Force deterministic ordering (pulp_created is auto_now_add).
342+
FilePublication.objects.filter(pk=old_pub.pk).update(
343+
pulp_created=new_pub.pulp_created - timedelta(seconds=1)
344+
)
345+
dist = dist_factory(repover=version)
346+
with mock.patch("pulpcore.app.models.publication.Cache") as mock_cache:
347+
old_pub.delete()
348+
assert cache_key(dist.base_path) not in invalidated_base_paths(mock_cache)

0 commit comments

Comments
 (0)