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

Fixes #29122 - Correct CV deletion behavior for pulp3 #8594

Merged
merged 1 commit into from Mar 12, 2020

Conversation

jlsherrill
Copy link
Member

@jlsherrill jlsherrill commented Mar 2, 2020

This fixes four situations:

  • Deleting a CVV would delete the pulp3 repository, deleting repo
    versions across all CVVs in that content view
  • Deleting a CV would NOT delete the pulp3 repository, when it should
  • Removing a CVV from a Lifecycle Env would not properly delete the
    distribution
  • If you deleted a CVV, that had re-used the version_href and
    publication_href from a Library repo, it would delete the library
    repo's pulp3 repo version (and thus its publication too).

@theforeman-bot
Copy link

Issues: #29122

@jlsherrill
Copy link
Member Author

jlsherrill commented Mar 2, 2020

TODO:

  • - add more testing

Testing scenarios:

I'd recommend running this query to get a good idea of what is in pulp3 and what gets deleted:

sudo -u postgres psql pulpcore


select (select count(*) as repos from core_repository ), (select count(*) as repo_versions from core_repositoryversion), (select count(*) as publications from core_publication), (select count(*) as distributions from core_basedistribution);
  1. Create a file repository in library. Counts:
 repos | repo_versions | publications | distributions 
-------+---------------+--------------+---------------
     1 |             1 |            1 |             1
  1. Create a content view, add the file repository, and publish
 repos | repo_versions | publications | distributions 
-------+---------------+--------------+---------------
     2 |             2 |            1 |             2
  1. Remove the CVV from the library lifecycle env
 repos | repo_versions | publications | distributions 
-------+---------------+--------------+---------------
     2 |             2 |            1 |             1
  1. Remove the 1.0 CVV altogether:
 repos | repo_versions | publications | distributions 
-------+---------------+--------------+---------------
     2 |             2 |            1 |             1
  1. Delete the CV:
 repos | repo_versions | publications | distributions 
-------+---------------+--------------+---------------
     1 |             1 |            1 |             1

This fixes three situations:

* Deleting a CVV would delete the pulp3 repository, deleting repo
  versions across all CVVs in that content view
* Deleting a CV would NOT delete the pulp3 repository, when it should
* Removing a CVV from a Lifecycle Env would not properly delete the
  distribution
* If you deleted a CVV, that had re-used the version_href and
  publication_href from a Library repo, it would delete the library
  repo's pulp3 repo version (and thus its publication too).
Copy link
Member

@jjeffers jjeffers left a comment

Choose a reason for hiding this comment

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

Worked as described, but I did have a question re the force_pulp3 parameter...

@@ -172,8 +172,8 @@ def self.with_type(content_type)
joins(:root).where("#{RootRepository.table_name}.content_type" => content_type)
end

def backend_service(smart_proxy)
if smart_proxy.pulp3_support?(self)
def backend_service(smart_proxy, force_pulp3 = false)
Copy link
Member

Choose a reason for hiding this comment

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

When would we use the force_pulp3 parameter?

Copy link
Member Author

Choose a reason for hiding this comment

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

Its used in one place at the moment. When we delete a content view: https://github.com/Katello/katello/pull/8594/files#diff-d4b12ef3369bd829bf21911a41392ee5R17

If at this point we aren't using pulp3 for that type of content, but for some reason a Pulp3::RepositoryReference exists, it means we have to have run the content migration already, but are deleting stuff before actually switching over to pulp3. So it makes sense to force it in order to get the backend service object to delete the pulp3 repository.

@jlsherrill jlsherrill merged commit 665d679 into Katello:master Mar 12, 2020
@jlsherrill jlsherrill deleted the CV_DELETE branch March 12, 2020 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants