Skip to content

Commit

Permalink
Fixes #18963 - Use batch processing for bulk actions
Browse files Browse the repository at this point in the history
  • Loading branch information
adamruzicka authored and iNecas committed Mar 24, 2017
1 parent 8143632 commit 6abdbae
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 3 deletions.
1 change: 1 addition & 0 deletions app/lib/actions/action_with_sub_plans.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ class Actions::ActionWithSubPlans < Actions::EntryAction
middleware.use Actions::Middleware::KeepCurrentUser

include Dynflow::Action::WithSubPlans
include Dynflow::Action::WithBulkSubPlans

def plan(*_args)
raise NotImplementedError
Expand Down
10 changes: 9 additions & 1 deletion app/lib/actions/bulk_action.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def humanized_input
def create_sub_plans
action_class = input[:action_class].constantize
target_class = input[:target_class].constantize
targets = target_class.where(:id => input[:target_ids])
targets = target_class.where(:id => current_batch)

targets.map do |target|
trigger(action_class, target, *input[:args])
Expand All @@ -57,5 +57,13 @@ def check_targets!(targets)
raise Foreman::Exception, N_('The targets are of different types')
end
end

def batch(from, size)
input[:target_ids].slice(from, size)
end

def total_count
input[:target_ids].count
end
end
end
18 changes: 18 additions & 0 deletions app/models/foreman_tasks/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,24 @@ def add_missing_task_groups(groups)
(groups - task_groups).each { |group| task_groups << group }
end

def sub_tasks_counts
result = %w(cancelled error pending success warning).zip([0].cycle).to_h
result.update sub_tasks.group(:result).count
sum = result.values.reduce(:+)
if respond_to?(:main_action) && main_action.respond_to?(:total_count)
result[:total] = main_action.total_count
# In case of batch planning there might be some plans still unplanned (not present in database).
# To get correct counts we need to add them to either:
# cancelled when the parent is stopped
# pending when the parent is still running.
key = state == 'stopped' ? 'cancelled' : 'pending'
result[key] += result[:total] - sum
else
result[:total] = sum
end
result.symbolize_keys
end

protected

def generate_id
Expand Down
13 changes: 12 additions & 1 deletion app/models/foreman_tasks/task/dynflow_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def update_from_dynflow(data)
self.started_at = utc_zone.parse(data[:started_at]) unless data[:started_at].nil?
self.ended_at = utc_zone.parse(data[:ended_at]) unless data[:ended_at].nil?
self.state = data[:state].to_s
self.result = data[:result].to_s
self.result = map_result(data[:result])
self.start_at = utc_zone.parse(data[:start_at]) if data[:start_at]
self.start_before = utc_zone.parse(data[:start_before]) if data[:start_before]
self.parent_task_id ||= begin
Expand Down Expand Up @@ -117,5 +117,16 @@ def self.new_for_execution_plan(execution_plan_id, data)
def self.model_name
superclass.model_name
end

private

def map_result(result)
result = :cancelled if result == :error && cancelled?
result.to_s
end

def cancelled?
execution_plan.errors.map(&:exception).any? { |exception| exception.class == ::ForemanTasks::Task::TaskCancelledException }
end
end
end
2 changes: 1 addition & 1 deletion foreman-tasks.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ DESC
s.extra_rdoc_files = Dir['README*', 'LICENSE']

s.add_dependency "foreman-tasks-core"
s.add_dependency "dynflow", '~> 0.8.17'
s.add_dependency "dynflow", '~> 0.8.22'
s.add_dependency "sequel" # for Dynflow process persistence
s.add_dependency "sinatra" # for Dynflow web console
s.add_dependency "daemons" # for running remote executor
Expand Down
46 changes: 46 additions & 0 deletions test/unit/task_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,50 @@ class TasksTest < ActiveSupport::TestCase
inconsistent_task.reload.state.must_equal 'planned'
end
end

describe 'subtask count querying' do
let(:result_base) do
{
:error => 0,
:warning => 0,
:total => 0,
:success => 0,
:cancelled => 0,
:pending => 0
}
end
let(:task) { FactoryGirl.create(:dynflow_task) }

describe 'without sub tasks' do
it 'calculates the progress report correctly' do
task.sub_tasks_counts.must_equal result_base
end
end

describe 'with sub tasks' do
let(:failed) { FactoryGirl.create(:dynflow_task).tap { |t| t.result = :error } }
let(:success) { FactoryGirl.create(:dynflow_task).tap { |t| t.result = :success } }
before { task.sub_tasks = [success, failed] }

it 'calculate the progress report correctly' do
expected_result = result_base.merge(:success => 1, :error => 1, :total => 2)
task.sub_tasks_counts.must_equal expected_result
end

it 'calculates the progress report correctly when using batch planning' do
result_base = result_base.merge(:success => 1, :error => 1, :total => 25)
fake_action = OpenStruct.new(:total_count => 25)
task.stubs(:main_action).returns(fake_action)

task.state = 'stopped'
expected_result = result_base.merge(:cancelled => 23)
task.sub_tasks_counts.must_equal expected_result

task.state = 'pending'
expected_result = result_base.merge(:pending => 23)
task.sub_tasks_counts.must_equal expected_result
end
end

end
end

0 comments on commit 6abdbae

Please sign in to comment.