Skip to content

Commit

Permalink
Fixes #37315 - Drop non-batch triggering
Browse files Browse the repository at this point in the history
  • Loading branch information
adamruzicka committed Apr 4, 2024
1 parent 9cf91af commit 953da9d
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 130 deletions.
136 changes: 25 additions & 111 deletions app/lib/actions/proxy_action.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,60 +32,36 @@ def backtrace
fields! exception: type { variants NilClass, Exception }
end

def plan(proxy, klass, options)
options[:connection_options] ||= {}
default_connection_options.each do |key, value|
options[:connection_options][key] = value unless options[:connection_options].key?(key)
end
plan_self(options.merge(:proxy_url => proxy.url, :proxy_action_name => klass.to_s))
# Just saving the RemoteTask is enough when using batch triggering
# It will be picked up by the ProxyBatchTriggering middleware
if input[:use_batch_triggering] && input.dig(:connection_options, :proxy_batch_triggering)
prepare_remote_task.save!
end
def plan(proxy, _klass, options)
plan_self(options)
prepare_remote_task(proxy).save!
end

def run(event = nil)
with_connection_error_handling(event) do |event|
case event
when nil
start_or_resume
when ::Dynflow::Action::Skip
# do nothing
when ::Dynflow::Action::Cancellable::Cancel
cancel_proxy_task
when ::Dynflow::Action::Cancellable::Abort
abort_proxy_task
when CallbackData
on_data(event.data, event.meta)
when ProxyActionMissing
on_proxy_action_missing
when ProxyActionStoppedEvent
on_proxy_action_stopped(event)
else
raise "Unexpected event #{event.inspect}"
end
case event
when nil
start_or_resume
when ::Dynflow::Action::Skip
# do nothing
when ::Dynflow::Action::Cancellable::Cancel
cancel_proxy_task
when ::Dynflow::Action::Cancellable::Abort
abort_proxy_task
when CallbackData
on_data(event.data, event.meta)
when ProxyActionMissing
on_proxy_action_missing
when ProxyActionStoppedEvent
on_proxy_action_stopped(event)
else
raise "Unexpected event #{event.inspect}"
end
end

def remote_task
@remote_task ||= ForemanTasks::RemoteTask.find_by(:execution_plan_id => execution_plan_id, :step_id => run_step_id)
end

def trigger_proxy_task
suspend do |_suspended_action|
remote_task = prepare_remote_task
ForemanTasks::RemoteTask.batch_trigger(remote_task.operation, [remote_task])
output[:proxy_task_id] = remote_task.remote_task_id
end
end

def trigger_remote_task
suspend do |_suspended_action|
ForemanTasks::RemoteTask.batch_trigger(remote_task.operation, [remote_task])
end
end

def proxy_input(task_id = task.id)
input.merge(:callback => { :task_id => task_id,
:step_id => run_step_id })
Expand Down Expand Up @@ -178,11 +154,7 @@ def proxy_output(live = false)

# The proxy action is able to contribute to continuous output
def fill_continuous_output(continuous_output)
failed_proxy_tasks.each do |failure_data|
message = _('Initialization error: %s') %
"#{failure_data[:exception_class]} - #{failure_data[:exception_message]}"
continuous_output.add_output(message, 'debug', failure_data[:timestamp])
end
# TODO? Should this be removed completely?
end

def proxy_output=(output)
Expand All @@ -194,35 +166,14 @@ def metadata
output[:metadata]
end

def metadata=(thing)
output[:metadata] ||= {}
output[:metadata] = thing
end

def default_connection_options
# Fails if the plan is not finished within 60 seconds from the first task trigger attempt on the smart proxy
# If the triggering fails, it retries 3 more times with 15 second delays
{ :retry_interval => Setting['foreman_tasks_proxy_action_retry_interval'] || 15,
:retry_count => Setting['foreman_tasks_proxy_action_retry_count'] || 4,
:proxy_batch_triggering => Setting['foreman_tasks_proxy_batch_trigger'] || false }
end

def clean_remote_task(*_args)
remote_task.destroy! if remote_task
end

private

def start_or_resume
if remote_task
if remote_task.state == 'external'
trigger_remote_task
else
on_resume
end
else
trigger_proxy_task
end
on_resume if remote_task
suspend
end

Expand All @@ -231,49 +182,12 @@ def get_proxy_data(response)
.try(:fetch, 'output', {}) || {}
end

def failed_proxy_tasks
metadata[:failed_proxy_tasks] ||= []
end

def with_connection_error_handling(event = nil)
yield event
rescue ::RestClient::Exception, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Errno::ETIMEDOUT => e
if event.class == CallbackData
raise e
else
handle_connection_exception(e, event)
end
end

def format_exception(exception)
{ :proxy_task_id => proxy_task_id,
:exception_class => exception.class.name,
:exception_message => exception.message,
:timestamp => Time.now.to_f }
end

def handle_connection_exception(exception, event = nil)
options = input[:connection_options]
failed_proxy_tasks << format_exception(exception)
output[:proxy_task_id] = nil
if failed_proxy_tasks.count < options[:retry_count]
suspend do |suspended_action|
@world.clock.ping suspended_action,
Time.now.getlocal + options[:retry_interval],
event
end
else
raise exception
end
end

def prepare_remote_task
state = input[:use_concurrency_control] ? 'external' : 'new'
def prepare_remote_task(proxy)
::ForemanTasks::RemoteTask.new(:execution_plan_id => execution_plan_id,
:proxy_url => input[:proxy_url],
:proxy_url => proxy.url,
:step_id => run_step_id,
:operation => proxy_operation_name,
:state => state)
:state => 'new')
end

def proxy_task_id
Expand Down
13 changes: 0 additions & 13 deletions app/models/foreman_tasks/remote_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,6 @@ class RemoteTask < ApplicationRecord

delegate :proxy_action_name, :to => :action

# Triggers a task on the proxy "the old way"
def trigger(proxy_action_name, input)
response = begin
proxy.launch_tasks('single', :action_class => proxy_action_name, :action_input => input)
rescue RestClient::Exception => e
logger.warn "Could not trigger task on the smart proxy"
logger.warn e
{}
end
update_from_batch_trigger(response)
save!
end

def self.batch_trigger(operation, remote_tasks)
remote_tasks.group_by(&:proxy_url).each_value do |group|
input_hash = group.reduce({}) do |acc, remote_task|
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20240404161613_drop_batch_triggering_setting.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class DropBatchTriggeringSetting < ActiveRecord::Migration[6.0]
def up
Setting.where(name: 'foreman_tasks_proxy_batch_trigger').delete_all
end
end
7 changes: 1 addition & 6 deletions lib/foreman_tasks/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,9 @@ class Engine < ::Rails::Engine
description: N_('Time in seconds between retries'),
default: 15,
full_name: N_('Proxy action retry interval'))
setting('foreman_tasks_proxy_batch_trigger',
type: :boolean,
description: N_('Allow triggering tasks on the smart proxy in batches'),
default: true,
full_name: N_('Allow proxy batch tasks'))
setting('foreman_tasks_proxy_batch_size',
type: :integer,
description: N_('Number of tasks which should be sent to the smart proxy in one request, if foreman_tasks_proxy_batch_trigger is enabled'),
description: N_('Number of tasks which should be sent to the smart proxy in one request'),
default: 100,
full_name: N_('Proxy tasks batch size'))
setting('foreman_tasks_troubleshooting_url',
Expand Down

0 comments on commit 953da9d

Please sign in to comment.