Ensure everything is terminated before exiting

On SIGTERM, signals are not propagated as expected and
and driver teardown is not called.

This change fixes that.

pifpaf/__main__.py

@@ -17,7 +17,6 @@
 import operator
 import os
 import signal
-import subprocess
 import sys
 import traceback
 
@@ -31,8 +30,11 @@
 
 import pkg_resources
 
+import psutil
+
 import six
 
+from pifpaf import util
 
 LOG = daiquiri.getLogger("pifpaf")
 
@@ -173,22 +175,55 @@ def expand_urls_var(url):
 
         if command:
             try:
-                with driver:
-                    putenv("PID", str(os.getpid()))
-                    putenv("DAEMON", daemon)
-                    url = os.getenv(driver.env_prefix + "_URL")
-                    putenv("%s_URL" % daemon.upper(), url)
-                    os.putenv(global_urls_variable,
-                              expand_urls_var(url))
-                    try:
-                        c = subprocess.Popen(command)
-                    except Exception:
-                        raise RuntimeError("Unable to start command: %s"
-                                           % " ".join(command))
-                    return c.wait()
+                driver.setUp()
             except fixtures.MultipleExceptions as e:
                 _format_multiple_exceptions(e, debug)
                 sys.exit(1)
+            except Exception:
+                LOG.error("Unable to start %s, "
+                          "use --debug for more information",
+                          daemon, exc_info=True)
+                sys.exit(1)
+
+            putenv("PID", str(os.getpid()))
+            putenv("DAEMON", daemon)
+            url = os.getenv(driver.env_prefix + "_URL")
+            putenv("%s_URL" % daemon.upper(), url)
+            os.putenv(global_urls_variable,
+                      expand_urls_var(url))
+
+            try:
+                c = psutil.Popen(command, preexec_fn=os.setsid)
+            except Exception:
+                driver.cleanUp()
+                raise RuntimeError("Unable to start command: %s"
+                                   % " ".join(command))
+            LOG.error(
+                "Command `%s` (pid %s) is ready:",
+                " ".join(command), c.pid
+            )
+
+            def _cleanup(signum=None, frame=None, ret=0):
+                signal.signal(signal.SIGTERM, signal.SIG_IGN)
+                signal.signal(signal.SIGHUP, signal.SIG_IGN)
+                signal.signal(signal.SIGINT, signal.SIG_IGN)
+                try:
+                    driver.cleanUp()
+                except Exception:
+                    LOG.error("Unexpected cleanUp error", exc_info=True)
+                util.process_cleaner(c)
+                sys.exit(1 if signum == signal.SIGINT else ret)
+
+            signal.signal(signal.SIGTERM, _cleanup)
+            signal.signal(signal.SIGHUP, _cleanup)
+            signal.signal(signal.SIGINT, _cleanup)
+            signal.signal(signal.SIGPIPE, signal.SIG_IGN)
+
+            try:
+                ret = c.wait()
+            except KeyboardInterrupt:
+                ret = 1
+            _cleanup(ret=ret)
         else:
             try:
                 driver.setUp()

pifpaf/drivers/__init__.py

@@ -12,7 +12,6 @@
 # limitations under the License.
 
 import contextlib
-import errno
 import logging
 import os
 import re
@@ -32,6 +31,9 @@
 
 import six
 
+from pifpaf import util
+
+
 try:
     import xattr
 except ImportError:
@@ -99,55 +101,10 @@ def _ensure_xattr_support(self):
             raise RuntimeError("TMPDIR must support xattr for %s" %
                                self.__class__.__name__)
 
-    @staticmethod
-    def _get_procs_of_pgid(wanted_pgid):
-        procs = []
-        for p in psutil.process_iter():
-            try:
-                pgid = os.getpgid(p.pid)
-            except OSError as e:
-                # ESRCH is returned if process just died in the meantime
-                if e.errno != errno.ESRCH:
-                    raise
-                continue
-            if pgid == wanted_pgid:
-                procs.append(p)
-        return procs
-
     def _kill(self, parent):
-        do_sigkill = False
         log_thread = getattr(parent, "_log_thread", None)
-        # NOTE(sileht): Add processes from process tree and process group
-        # Relying on process tree only will not work in case of
-        # parent dying prematuraly and double fork
-        # Relying on process group only will not work in case of
-        # subprocess calling again setsid()
-        procs = set(self._get_procs_of_pgid(parent.pid))
-        try:
-            procs |= set(parent.children(recursive=True))
-            procs.add(parent)
-            parent.terminate()
-        except psutil.NoSuchProcess:
-            LOG.warning("`%s` is already gone, sending SIGKILL to its process "
-                        "group", parent)
-            do_sigkill = True
-        else:
-            # Waiting for all processes to stop
-            gone, alive = psutil.wait_procs(procs, timeout=10)
-            if alive:
-                do_sigkill = True
-                LOG.warning("`%s` didn't terminate cleanly after 10 seconds, "
-                            "sending SIGKILL to its process group", parent)
-
-        if do_sigkill and procs:
-            for p in procs:
-                try:
-                    p.kill()
-                except psutil.NoSuchProcess:
-                    pass
-            gone, alive = psutil.wait_procs(procs, timeout=10)
-            if alive:
-                LOG.warning("`%s` survive SIGKILL", alive)
+
+        util.process_cleaner(parent)
 
         if log_thread:
             # Parent process have been killed

pifpaf/util.py

@@ -0,0 +1,78 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import errno
+import logging
+import os
+
+import psutil
+
+LOG = logging.getLogger(__name__)
+
+
+def _get_procs_of_pgid(wanted_pgid):
+    procs = []
+    for p in psutil.process_iter():
+        try:
+            pgid = os.getpgid(p.pid)
+        except OSError as e:
+            # ESRCH is returned if process just died in the meantime
+            if e.errno != errno.ESRCH:
+                raise
+            continue
+        if pgid == wanted_pgid:
+            procs.append(p)
+    return procs
+
+
+def process_cleaner(parent):
+    do_sigkill = False
+    # NOTE(sileht): Add processes from process tree and process group
+    # Relying on process tree only will not work in case of
+    # parent dying prematuraly and double fork
+    # Relying on process group only will not work in case of
+    # subprocess calling again setsid()
+    procs = set(_get_procs_of_pgid(parent.pid))
+    try:
+        LOG.debug("Terminating %s (%s)",
+                  " ".join(parent.cmdline()), parent.pid)
+        procs |= set(parent.children(recursive=True))
+        procs.add(parent)
+        parent.terminate()
+    except psutil.NoSuchProcess:
+        LOG.warning("`%s` is already gone, sending SIGKILL to its process "
+                    "group", parent)
+        do_sigkill = True
+    else:
+        # Waiting for all processes to stop
+        for p in procs:
+            try:
+                LOG.debug("Waiting %s (%s)", " ".join(p.cmdline()), p.pid)
+            except psutil.NoSuchProcess:
+                pass
+        gone, alive = psutil.wait_procs(procs, timeout=10)
+        if alive:
+            do_sigkill = True
+            LOG.warning("`%s` didn't terminate cleanly after 10 seconds, "
+                        "sending SIGKILL to its process group", parent)
+
+    if do_sigkill and procs:
+        for p in procs:
+            try:
+                LOG.debug("Killing %s (%s)", " ".join(p.cmdline()), p.pid)
+                p.kill()
+            except psutil.NoSuchProcess:
+                pass
+        gone, alive = psutil.wait_procs(procs, timeout=10)
+        if alive:
+            LOG.warning("`%s` survive SIGKILL", alive)