Merge pull request #2153 from rutsky/worker-stop-timeout-exit-code

return non-zero exit code if buildbot-worker stop timeouts

worker/buildbot_worker/scripts/stop.py

@@ -66,11 +66,12 @@ def stopWorker(basedir, quiet, signame="TERM"):
         except OSError:
             if not quiet:
                 print("worker process %d is dead" % pid)
-            return
+            return 0
         timer += 1
         time.sleep(1)
     if not quiet:
         print("never saw process go away")
+    return 1
 
 
 def stop(config, signame="TERM"):
@@ -81,9 +82,8 @@ def stop(config, signame="TERM"):
         return 1
 
     try:
-        stopWorker(basedir, quiet, signame)
+        return stopWorker(basedir, quiet, signame)
     except WorkerNotRunning:
         if not quiet:
             print("worker not running")
-
-    return 0
+        return 0

worker/buildbot_worker/test/unit/test_scripts_stop.py

@@ -78,12 +78,38 @@ def emulated_kill(pid, sig):
 
         # check that stopWorker() sends expected signal to right PID
         # and print correct message to stdout
-        stop.stopWorker(None, False)
+        exit_code = stop.stopWorker(None, False)
+        self.assertEqual(exit_code, 0)
         mocked_kill.assert_has_calls([mock.call(self.PID, signal.SIGTERM),
                                       mock.call(self.PID, 0)])
 
         self.assertStdoutEqual("worker process %s is dead\n" % self.PID)
 
+    @compat.skipUnlessPlatformIs("posix")
+    def test_stop_timeout(self):
+        """
+        test stopWorker() when stop timeouts
+        """
+
+        # patch open() to return a pid file
+        self.setUpOpen(str(self.PID))
+
+        # patch os.kill to emulate successful kill
+        mocked_kill = mock.Mock()
+        self.patch(os, "kill", mocked_kill)
+
+        # don't waste time
+        self.patch(time, "sleep", mock.Mock())
+
+        # check that stopWorker() sends expected signal to right PID
+        # and print correct message to stdout
+        exit_code = stop.stopWorker(None, False)
+        self.assertEqual(exit_code, 1)
+        mocked_kill.assert_has_calls([mock.call(self.PID, signal.SIGTERM),
+                                      mock.call(self.PID, 0)])
+
+        self.assertStdoutEqual("never saw process go away\n")
+
 
 class TestStop(misc.IsWorkerDirMixin,
                misc.StdoutAssertionsMixin,
@@ -121,7 +147,8 @@ def test_no_worker_running(self):
         mock_stopWorker = mock.Mock(side_effect=stop.WorkerNotRunning())
         self.patch(stop, "stopWorker", mock_stopWorker)
 
-        stop.stop(self.config)
+        exit_code = stop.stop(self.config)
+        self.assertEqual(exit_code, 0)
 
         self.assertStdoutEqual("worker not running\n")
 
@@ -134,10 +161,29 @@ def test_successful_stop(self):
         self.setupUpIsWorkerDir(True)
 
         # patch stopWorker() to do nothing
-        mock_stopWorker = mock.Mock()
+        mock_stopWorker = mock.Mock(return_value=0)
+        self.patch(stop, "stopWorker", mock_stopWorker)
+
+        exit_code = stop.stop(self.config)
+        self.assertEqual(exit_code, 0)
+        mock_stopWorker.assert_called_once_with(self.config["basedir"],
+                                                self.config["quiet"],
+                                                "TERM")
+
+    def test_failed_stop(self):
+        """
+        test failing stop()
+        """
+
+        # patch basedir check to always succeed
+        self.setupUpIsWorkerDir(True)
+
+        # patch stopWorker() to do nothing
+        mock_stopWorker = mock.Mock(return_value=17)
         self.patch(stop, "stopWorker", mock_stopWorker)
 
-        stop.stop(self.config)
+        exit_code = stop.stop(self.config)
+        self.assertEqual(exit_code, 17)
         mock_stopWorker.assert_called_once_with(self.config["basedir"],
                                                 self.config["quiet"],
                                                 "TERM")