From a57863d2b4b14b3ca9186f5b4dd6bbae50706741 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Bouy=C3=A9?= <romain@bouye.fr>
Date: Tue, 19 May 2020 13:47:45 +0100
Subject: [PATCH] synctl warns when no process is stopped and avoids start
 (#6598)

* If an error occurs when stopping a process synctl now logs a warning.
* During a restart, synctl will avoid attempting to start Synapse if an error
  occurs during stopping Synapse.
---
 changelog.d/6590.misc |  1 +
 synctl                | 31 ++++++++++++++++++++++++++++---
 2 files changed, 29 insertions(+), 3 deletions(-)
 create mode 100644 changelog.d/6590.misc

diff --git a/changelog.d/6590.misc b/changelog.d/6590.misc
new file mode 100644
index 000000000..05a015621
--- /dev/null
+++ b/changelog.d/6590.misc
@@ -0,0 +1 @@
+`synctl` now warns if it was unable to stop Synapse and will not attempt to start Synapse if nothing was stopped. Contributed by Romain Bouyé.
diff --git a/synctl b/synctl
index bbccd0529..81e9bf6b5 100755
--- a/synctl
+++ b/synctl
@@ -142,12 +142,23 @@ def start_worker(app: str, configfile: str, worker_configfile: str) -> bool:
         return False
 
 
-def stop(pidfile, app):
+def stop(pidfile: str, app: str) -> bool:
+    """Attempts to kill a synapse worker from the pidfile.
+    Args:
+        pidfile: path to file containing worker's pid
+        app: name of the worker's appservice
+
+    Returns:
+        True if the process stopped successfully
+        False if process was already stopped or an error occured
+    """
+
     if os.path.exists(pidfile):
         pid = int(open(pidfile).read())
         try:
             os.kill(pid, signal.SIGTERM)
             write("stopped %s" % (app,), colour=GREEN)
+            return True
         except OSError as err:
             if err.errno == errno.ESRCH:
                 write("%s not running" % (app,), colour=YELLOW)
@@ -155,6 +166,14 @@ def stop(pidfile, app):
                 abort("Cannot stop %s: Operation not permitted" % (app,))
             else:
                 abort("Cannot stop %s: Unknown error" % (app,))
+            return False
+    else:
+        write(
+            "No running worker of %s found (from %s)\nThe process might be managed by another controller (e.g. systemd)"
+            % (app, pidfile),
+            colour=YELLOW,
+        )
+    return False
 
 
 Worker = collections.namedtuple(
@@ -300,11 +319,17 @@ def main():
     action = options.action
 
     if action == "stop" or action == "restart":
+        has_stopped = True
         for worker in workers:
-            stop(worker.pidfile, worker.app)
+            if not stop(worker.pidfile, worker.app):
+                # A worker could not be stopped.
+                has_stopped = False
 
         if start_stop_synapse:
-            stop(pidfile, "synapse.app.homeserver")
+            if not stop(pidfile, "synapse.app.homeserver"):
+                has_stopped = False
+        if not has_stopped:
+            sys.exit(1)
 
     # Wait for synapse to actually shutdown before starting it again
     if action == "restart":