Protect distributed repo versions from deletion #4750

daviddavis · 2023-11-21T15:12:50Z

This change protects repo versions from being deleted up if their content is being served by distributions.

pulpcore/app/models/repository.py

ipanova

This code does not work with plugins that do not have publications and repository/version can be served directly from distribution. Please update the code so it accounts for such plugins.

$ http DELETE https://puffy.example.com/pulp/api/v3/repositories/container/container/018bfb9a-ffa4-72e3-a9f4-f884f8719305/versions/1/ --auth admin:password
HTTP/1.1 500 Internal Server Error
Access-Control-Expose-Headers: Correlation-ID
Connection: keep-alive
Content-Length: 145
Content-Type: text/html; charset=utf-8
Correlation-ID: 0d33ccdcae8e431fbb34871ae3436d05
Cross-Origin-Opener-Policy: same-origin
Date: Mon, 27 Nov 2023 15:59:02 GMT
Referrer-Policy: same-origin
Server: nginx
X-Content-Type-Options: nosniff
X-Frame-Options: DENY

<!doctype html>
<html lang="en">
<head>
  <title>Server Error (500)</title>
</head>
<body>
  <h1>Server Error (500)</h1><p></p>
</body>
</html>

pulpcore-api[92257]:     if version in version.repository.protected_versions():
pulpcore-api[92257]:   File "/pulpcore/pulpcore/app/models/repository.py", line 333, in protected_versions
pulpcore-api[92257]:     latest_publication = publications.select_related("repository_version").latest(
pulpcore-api[92257]:   File "/usr/local/lib/pulp/lib64/python3.10/site-packages/django/db/models/query.py", line 1045, in latest
pulpcore-api[92257]:     return self.reverse()._earliest(*fields)
pulpcore-api[92257]:   File "/usr/local/lib/pulp/lib64/python3.10/site-packages/django/db/models/query.py", line 1028, in _earliest
pulpcore-api[92257]:     return obj.get()
pulpcore-api[92257]:   File "/usr/local/lib/pulp/lib64/python3.10/site-packages/django/db/models/query.py", line 637, in get
pulpcore-api[92257]:     raise self.model.DoesNotExist(
pulpcore-api[92257]: pulpcore.app.models.publication.Publication.DoesNotExist: Publication matching query does not exist.
^C

daviddavis · 2023-11-27T16:34:31Z

@ipanova done.

pulpcore/app/models/repository.py

docs/workflows/repo-versioning.rst

pulpcore/app/models/repository.py

ipanova

@daviddavis Left some linguistic suggestions to account for plugins that do not have publications and one technical one. Otherwise it looks good, thank you!

pulpcore/app/models/repository.py

daviddavis · 2023-12-05T14:10:00Z

@ipanova @dralley thanks for the review. I think I've addressed everything.

Old approval

I believe this prevents the content app from serving unpublished content when a repo is set on a distribution: pulp/pulpcore@27a0dc7 Also, this setting will be used to protect repo versions once the repo version protection work is merged: pulp/pulpcore#4750 fixes pulp#976

I don't know if this is strictly required since pulp_python defines its own content handler function but SERVE_FROM_PUBLICATION ought to be set from a technical correctness standpoint. Also future work might expect it to be set such as the repo version protection PR I have open against pulpcore: pulp/pulpcore#4750

I don't know if this is strictly required since pulp_python defines its own content handler function but SERVE_FROM_PUBLICATION ought to be set from a technical correctness standpoint. Also future work might expect it to be set such as the repo version protection PR I have open against pulpcore: pulp/pulpcore#4750 [noissue]

mdellweg · 2023-12-07T16:52:01Z

pulpcore/app/models/repository.py

+        )
+
+        if distro := Distribution.objects.filter(repository=self.pk).first():
+            if distro.cast().SERVE_FROM_PUBLICATION:


Suggested change

if distro.cast().SERVE_FROM_PUBLICATION:

if distro.detail_model().SERVE_FROM_PUBLICATION:

This change protects repo versions from being deleted if they are being used to distribute content. It also protects repo versions from being cleaned up by the retain repo versions code. fixes pulp#2705

mdellweg · 2023-12-07T17:00:47Z

pulpcore/app/models/repository.py

-            ]:
+            # Consider only completed versions that aren't protected for cleanup
+            versions = self.versions.complete().exclude(pk__in=self.protected_versions())
+            for version in versions.order_by("-number")[self.retain_repo_versions :]:


Just for my understanding. This logic will not only protect these versions, but also not consider them for counting when selecting the last retain_repo_versions for not deleting.
So if $$N_{\textrm{rrv}} = 3$$ and $$RV_{n-2}$$ is protected, this will also protect $$RV_{n-3}$$. Or in other words, we aim to keep retain_repo_versions unprotected versions.

Exactly, as I would say, protected repo versions are excluded fromretain_repo_versions.

And I think this makes sense. Otherwise, if retain_repo_versions was 1 and you already had 1 protected version, then if you created another version, it would immediately get cleaned up.

I believe this prevents the content app from serving unpublished content when a repo is set on a distribution: pulp/pulpcore@27a0dc7 Also, this setting will be used to protect repo versions once the repo version protection work is merged: pulp/pulpcore#4750 fixes pulp#976

[noissue] The workflow had to be adjusted because of the breaking change introduced in pulp/pulpcore#4750.

[noissue] The workflow had to be adjusted because of the breaking change introduced in pulp/pulpcore#4750. (cherry picked from commit 4972e62)

daviddavis force-pushed the fix2705 branch 5 times, most recently from 435d8a2 to ae25495 Compare November 21, 2023 15:20

daviddavis changed the title ~~Protect distributed repo versions from cleanup~~ Protect distributed repo versions from deletion Nov 27, 2023

daviddavis force-pushed the fix2705 branch from ae25495 to b369fec Compare November 27, 2023 13:11

mdellweg previously approved these changes Nov 27, 2023

View reviewed changes

pulpcore/app/models/repository.py Show resolved Hide resolved

daviddavis force-pushed the fix2705 branch 2 times, most recently from b257111 to acc380f Compare November 27, 2023 13:47

ipanova requested changes Nov 27, 2023

View reviewed changes

daviddavis force-pushed the fix2705 branch from acc380f to c83ffbb Compare November 27, 2023 16:34

daviddavis force-pushed the fix2705 branch from c83ffbb to 7b23efd Compare November 27, 2023 16:35

gerrod3 reviewed Nov 27, 2023

View reviewed changes

pulpcore/app/models/repository.py Outdated Show resolved Hide resolved

daviddavis force-pushed the fix2705 branch from 7b23efd to c6aa6bf Compare November 28, 2023 17:15

ggainey requested a review from ipanova November 30, 2023 20:09

ipanova reviewed Dec 1, 2023

View reviewed changes

docs/workflows/repo-versioning.rst Outdated Show resolved Hide resolved

ipanova reviewed Dec 1, 2023

View reviewed changes

pulpcore/app/models/repository.py Outdated Show resolved Hide resolved

ipanova reviewed Dec 1, 2023

View reviewed changes

pulpcore/app/models/repository.py Outdated Show resolved Hide resolved

ipanova reviewed Dec 1, 2023

View reviewed changes

pulpcore/app/models/repository.py Outdated Show resolved Hide resolved

ipanova reviewed Dec 1, 2023

View reviewed changes

pulpcore/app/models/repository.py Outdated Show resolved Hide resolved

ipanova reviewed Dec 1, 2023

View reviewed changes

ipanova mentioned this pull request Dec 1, 2023

Cleanup old publications - general cleanup #4817

Closed

dralley reviewed Dec 1, 2023

View reviewed changes

pulpcore/app/models/repository.py Outdated Show resolved Hide resolved

daviddavis force-pushed the fix2705 branch 2 times, most recently from 90ed9c1 to 63fd180 Compare December 5, 2023 13:51

ggainey requested a review from mdellweg December 5, 2023 14:30

daviddavis mentioned this pull request Dec 6, 2023

Set SERVE_FROM_PUBLICATION on AptDistribution pulp/pulp_deb#977

Merged

ipanova previously approved these changes Dec 6, 2023

View reviewed changes

daviddavis mentioned this pull request Dec 6, 2023

Set SERVE_FROM_PUBLICATION on PythonDistribution pulp/pulp_python#637

Closed

daviddavis dismissed ipanova’s stale review via d524476 December 7, 2023 11:48

daviddavis force-pushed the fix2705 branch from 1224e30 to d524476 Compare December 7, 2023 11:48

ipanova self-requested a review December 7, 2023 12:04

daviddavis force-pushed the fix2705 branch from d524476 to 7bc3f1d Compare December 7, 2023 14:23

ipanova previously approved these changes Dec 7, 2023

View reviewed changes

mdellweg reviewed Dec 7, 2023

View reviewed changes

Protect distributed repo versions from deletion

0695508

This change protects repo versions from being deleted if they are being used to distribute content. It also protects repo versions from being cleaned up by the retain repo versions code. fixes pulp#2705

daviddavis dismissed ipanova’s stale review via 0695508 December 7, 2023 16:54

daviddavis force-pushed the fix2705 branch from 7bc3f1d to 0695508 Compare December 7, 2023 16:54

mdellweg approved these changes Dec 7, 2023

View reviewed changes

ipanova approved these changes Dec 7, 2023

View reviewed changes

ipanova merged commit c4e842c into pulp:main Dec 7, 2023
16 checks passed

This was referenced Dec 11, 2023

[PR #977/47a759ab backport][3.0] Set SERVE_FROM_PUBLICATION on AptDistribution pulp/pulp_deb#980

Merged

[PR #977/47a759ab backport][3.1] Set SERVE_FROM_PUBLICATION on AptDistribution pulp/pulp_deb#981

Merged

lubosmj added a commit to lubosmj/pulp_ostree that referenced this pull request Jan 17, 2024

Make sure a distribution is deleted before a repo version

9e0a728

[noissue] The workflow had to be adjusted because of the breaking change introduced in pulp/pulpcore#4750.

lubosmj mentioned this pull request Jan 17, 2024

Make sure a distribution is deleted before a repo version pulp/pulp_ostree#329

Merged

lubosmj added a commit to lubosmj/pulp_ostree that referenced this pull request Jan 17, 2024

Make sure a distribution is deleted before a repo version

61a84a6

[noissue] The workflow had to be adjusted because of the breaking change introduced in pulp/pulpcore#4750.

lubosmj added a commit to lubosmj/pulp_ostree that referenced this pull request Jan 17, 2024

Make sure a distribution is deleted before a repo version

151ac30

[noissue] The workflow had to be adjusted because of the breaking change introduced in pulp/pulpcore#4750.

lubosmj added a commit to lubosmj/pulp_ostree that referenced this pull request Jan 17, 2024

Make sure a distribution is deleted before a repo version

c5ae233

[noissue] The workflow had to be adjusted because of the breaking change introduced in pulp/pulpcore#4750.

lubosmj added a commit to pulp/pulp_ostree that referenced this pull request Jan 17, 2024

Make sure a distribution is deleted before a repo version

4972e62

[noissue] The workflow had to be adjusted because of the breaking change introduced in pulp/pulpcore#4750.

patchback bot mentioned this pull request Jan 18, 2024

[PR #329/4972e625 backport][2.2] Make sure a distribution is deleted before a repo version pulp/pulp_ostree#330

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Protect distributed repo versions from deletion #4750

Protect distributed repo versions from deletion #4750

daviddavis commented Nov 21, 2023 •

edited

Loading

ipanova left a comment •

edited

Loading

daviddavis commented Nov 27, 2023

ipanova left a comment

daviddavis commented Dec 5, 2023

mdellweg Dec 7, 2023

mdellweg Dec 7, 2023

daviddavis Dec 7, 2023 •

edited

Loading

	if distro.cast().SERVE_FROM_PUBLICATION:
	if distro.detail_model().SERVE_FROM_PUBLICATION:

Protect distributed repo versions from deletion #4750

Protect distributed repo versions from deletion #4750

Conversation

daviddavis commented Nov 21, 2023 • edited Loading

ipanova left a comment • edited Loading

Choose a reason for hiding this comment

daviddavis commented Nov 27, 2023

ipanova left a comment

Choose a reason for hiding this comment

daviddavis commented Dec 5, 2023

mdellweg Dec 7, 2023

Choose a reason for hiding this comment

mdellweg Dec 7, 2023

Choose a reason for hiding this comment

daviddavis Dec 7, 2023 • edited Loading

Choose a reason for hiding this comment

daviddavis commented Nov 21, 2023 •

edited

Loading

ipanova left a comment •

edited

Loading

daviddavis Dec 7, 2023 •

edited

Loading