Skip to content

Commit

Permalink
Fixes #35328 - Propagate errors when remote parent task fails (#691)
Browse files Browse the repository at this point in the history
* Fixes #35328 - Propagate errors when remote parent task fails

Before we notified the local child actions that their remote counterpart
has failed, but we didn't include any details. This made the local
actions ask the smart proxy for status, get back a 404, which the local
actions did not expect.

If we know that the remote parent failed during triggering, it is clear
there will not be any remote tasks to ask about, so we can fail straight
away, without having to ask for status.

* Refs #35328 - Properly deal with tasks disappearing from the proxy

It can still happen that we can try asking for details about a remote
task which no longer exists.

* Refs #35328 - Workaround for serialization issues

Apparently the exception contains both the response and the original
request through the response. If I recall correctly, the body of the
response is read on demand and the actual read is synchronized so it is
thread safe. The monitor used for this synchronization cannot be
serialized, pass the boundaries of a process and then be instantiated
again. It is ugly, but so far we only care about the exception's class
and its backtrace.
  • Loading branch information
adamruzicka authored Aug 22, 2022
1 parent ed2ec14 commit 0b70f3f
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 7 deletions.
24 changes: 19 additions & 5 deletions app/lib/actions/proxy_action.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,15 @@ def backtrace
end
end

class ProxyActionStopped; end
class ProxyActionStopped < RuntimeError
def backtrace
[]
end
end

ProxyActionStoppedEvent = ::Algebrick.type do
fields! exception: type { variants NilClass, Exception }
end

def plan(proxy, klass, options)
options[:connection_options] ||= {}
Expand Down Expand Up @@ -52,8 +60,8 @@ def run(event = nil)
on_data(event.data, event.meta)
when ProxyActionMissing
on_proxy_action_missing
when ProxyActionStopped
on_proxy_action_stopped
when ProxyActionStoppedEvent
on_proxy_action_stopped(event)
else
raise "Unexpected event #{event.inspect}"
end
Expand Down Expand Up @@ -94,6 +102,8 @@ def check_task_status
else
suspend
end
rescue RestClient::NotFound
on_proxy_action_missing
end

def cancel_proxy_task
Expand Down Expand Up @@ -133,8 +143,12 @@ def on_proxy_action_missing
error! ProxyActionMissing.new(_('Proxy task gone missing from the smart proxy'))
end

def on_proxy_action_stopped
check_task_status
def on_proxy_action_stopped(event)
if event.exception
error! ProxyActionStopped.new(_('Failed to trigger task on the smart proxy: ') + event.exception.message)
else
check_task_status
end
end

# @override String name of an action to be triggered on server
Expand Down
5 changes: 4 additions & 1 deletion app/lib/actions/trigger_proxy_batch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ def trigger_remote_tasks_batch
rescue => e
action_logger.warn "Could not trigger task on the smart proxy"
action_logger.warn e
batch.each { |remote_task| remote_task.update_from_batch_trigger({}) }
# The response contains non-serializable objects
# TypeError: no _dump_data is defined for class Monitor
e.response = nil
batch.each { |remote_task| remote_task.update_from_batch_trigger({ 'exception' => e }) }
output[:failed_count] += batch.size
end

Expand Down
3 changes: 2 additions & 1 deletion app/models/foreman_tasks/remote_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,11 @@ def update_from_batch_trigger(data, parent = {})
self.parent_task_id = parent['task_id']
self.state = 'parent-triggered'
else
exception = data['exception']
# Tell the action the task on the smart proxy stopped
ForemanTasks.dynflow.world.event execution_plan_id,
step_id,
::Actions::ProxyAction::ProxyActionStopped.new,
::Actions::ProxyAction::ProxyActionStoppedEvent[exception],
optional: true
end
save!
Expand Down

0 comments on commit 0b70f3f

Please sign in to comment.