From 0b70f3f8aae9f7b6ca9744e5c27a717a40d787d6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adam=20R=C5=AF=C5=BEi=C4=8Dka?=
 <adamruzicka@users.noreply.github.com>
Date: Mon, 22 Aug 2022 11:54:31 +0200
Subject: [PATCH] Fixes #35328 - Propagate errors when remote parent task fails
 (#691)

* 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.
---
 app/lib/actions/proxy_action.rb         | 24 +++++++++++++++++++-----
 app/lib/actions/trigger_proxy_batch.rb  |  5 ++++-
 app/models/foreman_tasks/remote_task.rb |  3 ++-
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/app/lib/actions/proxy_action.rb b/app/lib/actions/proxy_action.rb
index 6d9cf9a41..05e73daf6 100644
--- a/app/lib/actions/proxy_action.rb
+++ b/app/lib/actions/proxy_action.rb
@@ -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] ||= {}
@@ -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
@@ -94,6 +102,8 @@ def check_task_status
       else
         suspend
       end
+    rescue RestClient::NotFound
+      on_proxy_action_missing
     end
 
     def cancel_proxy_task
@@ -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
diff --git a/app/lib/actions/trigger_proxy_batch.rb b/app/lib/actions/trigger_proxy_batch.rb
index a587fc85b..72cd5aade 100644
--- a/app/lib/actions/trigger_proxy_batch.rb
+++ b/app/lib/actions/trigger_proxy_batch.rb
@@ -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
 
diff --git a/app/models/foreman_tasks/remote_task.rb b/app/models/foreman_tasks/remote_task.rb
index 68453e17f..830591ced 100644
--- a/app/models/foreman_tasks/remote_task.rb
+++ b/app/models/foreman_tasks/remote_task.rb
@@ -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!