From bc8c26f5a92cc6ce417cb72e720e8ead02d79882 Mon Sep 17 00:00:00 2001
From: Joe Rafaniello <jrafanie@redhat.com>
Date: Mon, 24 Jul 2017 14:34:11 -0400
Subject: [PATCH] Let ansible worker gracefully stop

https://bugzilla.redhat.com/show_bug.cgi?id=1474508

When the ansible worker is not responding, let it try to gracefuly exit
by marking it as stopping.  Allow, the stopping worker code to eventually
kill the worker if it doesn't exit on it's own.

While we're allowing the worker to gracefully exit, don't allow multiple
ansible monitor workers to run at once. Normally, a replacement worker
is started the moment we ask a prior worker to exit.
---
 app/models/embedded_ansible_worker.rb       |  6 ++--
 spec/models/embedded_ansible_worker_spec.rb | 40 ++++++++++-----------
 2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/app/models/embedded_ansible_worker.rb b/app/models/embedded_ansible_worker.rb
index f9f052006d8..810f2151415 100644
--- a/app/models/embedded_ansible_worker.rb
+++ b/app/models/embedded_ansible_worker.rb
@@ -2,6 +2,9 @@ class EmbeddedAnsibleWorker < MiqWorker
   require_nested :Runner
   include_concern 'ObjectManagement'
 
+  # Don't allow multiple ansible monitor workers to run at once
+  self.include_stopping_workers_on_synchronize = true
+
   self.required_roles = ['embedded_ansible']
 
   def start_runner
@@ -48,9 +51,6 @@ def find_worker_thread_object
     end
   end
 
-  alias terminate kill
-  alias stop kill
-
   def status_update
     # don't monitor the memory/cpu usage of this process yet
     # If we don't have a pid of a process we want to monitor,super will catch an Errno::ESRCH and abort the worker
diff --git a/spec/models/embedded_ansible_worker_spec.rb b/spec/models/embedded_ansible_worker_spec.rb
index 08cf3b9853e..bf187c84078 100644
--- a/spec/models/embedded_ansible_worker_spec.rb
+++ b/spec/models/embedded_ansible_worker_spec.rb
@@ -161,27 +161,25 @@
       end
     end
 
-    %w(kill stop terminate).each do |stop_method|
-      describe "##{stop_method}" do
-        it "exits the monitoring thread and destroys the worker row" do
-          thread_double = double
-          expect(thread_double).to receive(:exit)
-          allow(subject).to receive(:find_worker_thread_object).and_return(thread_double)
-          subject.public_send(stop_method)
-          expect { subject.reload }.to raise_error(ActiveRecord::RecordNotFound)
-        end
-
-        it "destroys the worker row but refuses to kill the main thread" do
-          allow(subject).to receive(:find_worker_thread_object).and_return(Thread.main)
-          subject.public_send(stop_method)
-          expect { subject.reload }.to raise_error(ActiveRecord::RecordNotFound)
-        end
-
-        it "destroys the worker row if the monitor thread is not found" do
-          allow(subject).to receive(:find_worker_thread_object).and_return(nil)
-          subject.public_send(stop_method)
-          expect { subject.reload }.to raise_error(ActiveRecord::RecordNotFound)
-        end
+    describe "#kill" do
+      it "exits the monitoring thread and destroys the worker row" do
+        thread_double = double
+        expect(thread_double).to receive(:exit)
+        allow(subject).to receive(:find_worker_thread_object).and_return(thread_double)
+        subject.kill
+        expect { subject.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it "destroys the worker row but refuses to kill the main thread" do
+        allow(subject).to receive(:find_worker_thread_object).and_return(Thread.main)
+        subject.kill
+        expect { subject.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it "destroys the worker row if the monitor thread is not found" do
+        allow(subject).to receive(:find_worker_thread_object).and_return(nil)
+        subject.kill
+        expect { subject.reload }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
   end