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 #37139 - Block content view publish during repository publication tasks #10879

Merged
merged 2 commits into from
Mar 4, 2024
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
8 changes: 7 additions & 1 deletion app/lib/actions/katello/repository/metadata_generate.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
module Actions
module Katello
module Repository
class MetadataGenerate < Actions::Base
class MetadataGenerate < Actions::EntryAction
def plan(repository, options = {})
action_subject(repository)
repository.check_ready_to_act!
source_repository = options.fetch(:source_repository, nil)
source_repository ||= repository.target_repository if repository.link?
smart_proxy = options.fetch(:smart_proxy, SmartProxy.pulp_primary)
Expand All @@ -15,6 +17,10 @@ def plan(repository, options = {})
:source_repository => source_repository,
:matching_content => matching_content)
end

def resource_locks
:link
end
end
end
end
Expand Down
1 change: 1 addition & 0 deletions app/lib/actions/katello/repository/remove_content.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class RemoveContent < Actions::EntryAction
include Dynflow::Action::WithSubPlans

def plan(repository, content_units, options = {})
repository.check_ready_to_act!
sync_capsule = options.fetch(:sync_capsule, true)
if repository.redhat?
fail _("Cannot remove content from a non-custom repository")
Expand Down
1 change: 1 addition & 0 deletions app/lib/actions/katello/repository/sync.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class Sync < Actions::EntryAction
# of Katello and we just need to finish the rest of the orchestration
def plan(repo, options = {})
action_subject(repo)
repo.check_ready_to_act!

validate_contents = options.fetch(:validate_contents, false)
skip_metadata_check = options.fetch(:skip_metadata_check, false) || (validate_contents && (repo.yum? || repo.deb?))
Expand Down
1 change: 1 addition & 0 deletions app/lib/actions/katello/repository/upload_files.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module Repository
class UploadFiles < Actions::EntryAction
def plan(repository, files, content_type = nil, options = {})
action_subject(repository)
repository.check_ready_to_act!
repository.clear_smart_proxy_sync_histories
tmp_files = prepare_tmp_files(files)

Expand Down
22 changes: 22 additions & 0 deletions app/models/katello/content_view.rb
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,7 @@ def check_ready_to_publish!(importing: false, syncable: false)
check_ready_to_import!
else
fail _("Import-only content views can not be published directly") if import_only? && !syncable
check_repositories_blocking_publish!
check_composite_action_allowed!(organization.library)
check_docker_repository_names!([organization.library])
check_orphaned_content_facets!(environments: self.environments)
Expand All @@ -637,6 +638,16 @@ def check_ready_to_publish!(importing: false, syncable: false)
true
end

def check_repositories_blocking_publish!
blocking_tasks = repositories&.map { |repo| repo.blocking_task }&.compact

if blocking_tasks&.any?
errored_tasks = blocking_tasks.uniq.map { |task| "- #{Setting['foreman_url']}/foreman_tasks/tasks/#{task&.id}" }.join("\n")
fail _("Pending tasks detected in repositories of this content view. Please wait for the tasks: " +
errored_tasks + " before publishing.")
end
end

def check_docker_repository_names!(environments)
environments.each do |environment|
repositories = []
Expand Down Expand Up @@ -881,6 +892,17 @@ def filtered?
filters.present?
end

def blocking_task
blocking_task_labels = [
::Actions::Katello::ContentView::Publish.name
]
ForemanTasks::Task::DynflowTask.where(:label => blocking_task_labels)
.where.not(state: 'stopped')
.for_resource(self)
.order(:started_at)
.last
end

protected

def remove_repository(repository)
Expand Down
27 changes: 27 additions & 0 deletions app/models/katello/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,33 @@ def latest_dynflow_sync
for_resource(self).order(:started_at).last
end

def blocking_task
blocking_task_labels = [
::Actions::Katello::Repository::Sync.name,
::Actions::Katello::Repository::UploadFiles.name,
::Actions::Katello::Repository::RemoveContent.name,
::Actions::Katello::Repository::MetadataGenerate.name
]
ForemanTasks::Task::DynflowTask.where(:label => blocking_task_labels)
.where.not(state: 'stopped')
.for_resource(self)
.order(:started_at)
.last
end
Copy link
Contributor

Choose a reason for hiding this comment

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

do you anticipate that we may use this in other areas in the future?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not necessarily..However, this is the list of tasks with potential to create a new publication. So if we need to block anything on itthe future, a repo.check_ready_to_act! should work for that use-case.


def check_ready_to_act!
blocking_tasks = content_views&.map { |cv| cv.blocking_task }&.compact

if blocking_tasks&.any?
errored_tasks = blocking_tasks
.uniq
.map { |task| "- #{Setting['foreman_url']}/foreman_tasks/tasks/#{task&.id}" }
.join("\n")
fail _("This repository has pending tasks in associated content views. Please wait for the tasks: " + errored_tasks +
" to complete before proceeding.")
end
end

# returns other instances of this repo with the same library
# equivalent of repo
def environmental_instances(view)
Expand Down
13 changes: 11 additions & 2 deletions test/actions/katello/content_view_test.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
require 'katello_test_helper'

module ::Actions::Katello::ContentView
class TestBase < ActiveSupport::TestCase
include Dynflow::Testing
Expand All @@ -8,7 +7,7 @@ class TestBase < ActiveSupport::TestCase

let(:action) { create_action action_class }
let(:success_task) { ForemanTasks::Task::DynflowTask.create!(state: :success, result: "good") }

let(:pending_task) { ForemanTasks::Task::DynflowTask.create!(state: :pending, result: "good", id: 123) }
before(:all) do
set_user
end
Expand All @@ -17,6 +16,7 @@ class TestBase < ActiveSupport::TestCase
class PublishTest < TestBase
let(:action_class) { ::Actions::Katello::ContentView::Publish }
let(:content_view) { katello_content_views(:no_environment_view) }
let(:repository) { katello_repositories(:fedora_17_x86_64) }
before do
Dynflow::Testing::DummyPlannedAction.any_instance.stubs(:repository_mapping).returns({})
end
Expand All @@ -36,6 +36,15 @@ class PublishTest < TestBase
end
end

it 'fails when planning if child repo is being acted upon' do
content_view.repositories = [repository]
repository.expects(:blocking_task).returns(pending_task)
action.stubs(:task).returns(success_task)
assert_raises(RuntimeError) do
plan_action(action, content_view)
end
end

it 'uses override_components properly' do
action.stubs(:task).returns(success_task)
action.expects(:include_other_components).with('mock', content_view).returns('mock')
Expand Down
7 changes: 7 additions & 0 deletions test/actions/katello/repository/metadata_generate_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module Actions

let(:action_class) { ::Actions::Katello::Repository::MetadataGenerate }
let(:pulp_metadata_generate_class) { ::Actions::Pulp3::Orchestration::Repository::GenerateMetadata }
let(:success_task) { ForemanTasks::Task::DynflowTask.create!(state: :success, result: "good") }
let(:yum_repo) { katello_repositories(:fedora_17_x86_64) }
let(:yum_repo2) { katello_repositories(:fedora_17_x86_64_dev) }
let(:action_options) do
Expand All @@ -20,6 +21,8 @@ module Actions

it 'plans a yum metadata generate' do
action = create_action(action_class)
action.stubs(:task).returns(success_task)
action.expects(:action_subject).with(yum_repo)
plan_action(action, yum_repo)

assert_action_planned_with(action, pulp_metadata_generate_class, yum_repo, SmartProxy.pulp_primary,
Expand All @@ -31,6 +34,7 @@ module Actions
Location.current = taxonomies(:location1)

action = create_action(action_class)
action.stubs(:task).returns(success_task)
plan_action(action, yum_repo)

assert_action_planned_with(action, pulp_metadata_generate_class, yum_repo, SmartProxy.pulp_primary,
Expand All @@ -41,6 +45,7 @@ module Actions

it 'plans a yum refresh with source repo' do
action = create_action(action_class)
action.stubs(:task).returns(success_task)
plan_action(action, yum_repo, :source_repository => yum_repo2)

yum_action_options = action_options.clone
Expand All @@ -52,6 +57,7 @@ module Actions

it 'plans a yum refresh with matching content true' do
action = create_action(action_class)
action.stubs(:task).returns(success_task)
plan_action(action, yum_repo, :matching_content => true)

yum_action_options = action_options.clone
Expand All @@ -62,6 +68,7 @@ module Actions

it 'plans a yum refresh with matching content set to some deferred object' do
action = create_action(action_class)
action.stubs(:task).returns(success_task)
not_falsey = Object.new
plan_action(action, yum_repo, :matching_content => not_falsey)

Expand Down
14 changes: 13 additions & 1 deletion test/actions/katello/repository_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,8 @@ class SyncTest < TestBase
let(:action_class) { ::Actions::Katello::Repository::Sync }
let(:pulp3_action_class) { ::Actions::Pulp3::Orchestration::Repository::Sync }
let(:pulp3_metadata_generate_action_class) { ::Actions::Pulp3::Orchestration::Repository::GenerateMetadata }
let(:content_view) { katello_content_views(:no_environment_view) }
let(:pending_task) { ForemanTasks::Task::DynflowTask.create!(state: :pending, result: "good", id: 123) }

it 'skips applicability if non-yum and non-deb' do
action = create_action action_class
Expand All @@ -644,7 +646,7 @@ class SyncTest < TestBase
refute_action_planed action, ::Actions::Katello::Applicability::Repository::Regenerate
end

it 'plans verift checksum when validate_contents is passed' do
it 'plans verify checksum when validate_contents is passed' do
action = create_action action_class
action.stubs(:action_subject).with(repository)
plan_action action, repository, :validate_contents => true, :skip_candlepin_check => true
Expand All @@ -659,6 +661,16 @@ class SyncTest < TestBase
assert_action_planned(action, ::Actions::Katello::Repository::ErrataMail)
end

it 'fails when planning if parent CV is being published' do
action = create_action action_class
repository.content_views = [content_view]
content_view.expects(:blocking_task).returns(pending_task)
action.stubs(:action_subject).with(repository)
assert_raises(RuntimeError) do
plan_action(action, repository)
end
end

it 'plans pulp3 orchestration actions with file repo' do
action = create_action pulp3_action_class
action.stubs(:action_subject).with(repository_pulp3)
Expand Down
Loading