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
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 app/lib/actions/katello/content_view/destroy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def plan(content_view, options = {})
end
end

plan_action(Actions::Pulp3::ContentView::DeleteRepositoryReferences, content_view, SmartProxy.pulp_master)
plan_self
end
end
Expand Down
26 changes: 26 additions & 0 deletions app/lib/actions/pulp3/content_view/delete_repository_references.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
module Actions
module Pulp3
module ContentView
class DeleteRepositoryReferences < Pulp3::AbstractAsyncTask
def plan(content_view, smart_proxy)
if content_view.repository_references.any?
plan_self(:content_view_id => content_view.id, :smart_proxy_id => smart_proxy.id)
end
end

def invoke_external_task
tasks = []
content_view = ::Katello::ContentView.find(input[:content_view_id])
content_view.repository_references.each do |repository_reference|
repo = repository_reference.root_repository.library_instance
#force pulp3 in case we've done migrations, but haven't switched over yet
tasks << repo.backend_service(smart_proxy, true).delete(repository_reference.repository_href)
end
content_view.repository_references.destroy_all

output[:pulp_tasks] = tasks
end
end
end
end
end
9 changes: 8 additions & 1 deletion app/lib/actions/pulp3/orchestration/repository/delete.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@ def plan(repository, smart_proxy)
sequence do
plan_action(Actions::Pulp3::Repository::DeleteRemote, repository.id, smart_proxy) if repository.remote_href
plan_action(Actions::Pulp3::Repository::DeleteDistributions, repository.id, smart_proxy)
plan_action(Actions::Pulp3::Repository::Delete, repository.id, smart_proxy)

if repository.content_view.default?
#we're deleting the library instance, so just delete the whole pulp3 repo
plan_action(Actions::Pulp3::Repository::Delete, repository.id, smart_proxy)
elsif repository.environment.nil?
#we're deleting the archived instance, so delete the version
plan_action(Actions::Pulp3::Repository::DeleteVersion, repository, smart_proxy)
end
end
end
end
Expand Down
20 changes: 20 additions & 0 deletions app/lib/actions/pulp3/repository/delete_version.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module Actions
module Pulp3
module Repository
class DeleteVersion < Pulp3::AbstractAsyncTask
def plan(repository, smart_proxy)
#A version from library might be reused in a composite, or could have been copied
# from a library repository, so only delete the version if its the last
if ::Katello::Repository.where(:version_href => repository.version_href).count == 1
plan_self(:repository_id => repository.id, :smart_proxy_id => smart_proxy.id)
end
end

def invoke_external_task
repo = ::Katello::Repository.find(input[:repository_id])
output[:response] = repo.backend_service(smart_proxy).delete_version
end
end
end
end
end
4 changes: 2 additions & 2 deletions app/models/katello/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.

if force_pulp3 || smart_proxy.pulp3_support?(self)
@service ||= Katello::Pulp3::Repository.instance_for_type(self, smart_proxy)
else
@service ||= Katello::Pulp::Repository.instance_for_type(self, smart_proxy)
Expand Down
4 changes: 4 additions & 0 deletions app/services/katello/pulp3/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ def copy_version(from_repository)
create_version(:base_version => from_repository.version_href)
end

def delete_version
api.repository_versions_api.delete(repo.version_href)
end

def create_version(options = {})
api.repositories_api.modify(repository_reference.repository_href, options)
end
Expand Down
28 changes: 28 additions & 0 deletions test/actions/pulp3/content_view/delete_repository_references.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
require 'katello_test_helper'

module ::Actions::Pulp3::ContentView
class DeleteRepositoryReferenceTest < ActiveSupport::TestCase
include Katello::Pulp3Support

def setup
@master = FactoryBot.create(:smart_proxy, :default_smart_proxy, :with_pulp3)
@repo = katello_repositories(:generic_file_archive)
@content_view = @repo.content_view
ensure_creatable(@repo, @master)
end

def test_create
ForemanTasks.sync_task(::Actions::Pulp3::Orchestration::Repository::Create, @repo, @master)
repo_reference = Katello::Pulp3::RepositoryReference.find_by(:content_view => @content_view, :root_repository_id => @repo.root_id)
assert repo_reference
assert Katello::Pulp3::Api::File.new(@master).repositories_api.read(repo_reference.repository_href)

ForemanTasks.sync_task(::Actions::Pulp3::ContentView::DeleteRepositoryReferences, @content_view, @master)
refute Katello::Pulp3::RepositoryReference.find_by(:id => repo_reference.id)

assert_raises(PulpFileClient::ApiError) do
Katello::Pulp3::Api::File.new(@master).repositories_api.read(repo_reference.repository_href)
end
end
end
end
Loading