17230751 plan execution should gracefully handle SIGTERM and SIGHUP s11u1-sru s11u1sru20
authorsaurabh.vyas@oracle.com
Thu, 08 May 2014 23:49:21 +0530
branchs11u1-sru
changeset 3084 b971ffcd04be
parent 3038 a6a195ab50bb
child 3095 2303bda251b0
17230751 plan execution should gracefully handle SIGTERM and SIGHUP
src/client.py
src/modules/client/image.py
src/modules/client/imageplan.py
src/tests/cli/t_pkg_install.py
src/tests/cli/t_pkg_nasty.py
src/tests/pkg5unittest.py
--- a/src/client.py	Fri Mar 07 11:26:34 2014 -0800
+++ b/src/client.py	Thu May 08 23:49:21 2014 +0530
@@ -75,6 +75,7 @@
         import pkg.fmri as fmri
         import pkg.misc as misc
         import pkg.pipeutils as pipeutils
+        import pkg.portable as portable
         import pkg.version as version
 
         from pkg.client import global_settings
@@ -106,6 +107,7 @@
 valid_special_attrs = ["action.hash", "action.key", "action.name", "action.raw"]
 
 valid_special_prefixes = ["action."]
+_api_inst = None
 
 def format_update_error(e):
         # This message is displayed to the user whenever an
@@ -6641,6 +6643,24 @@
                 __ret = 99
         return __ret
 
+
+def handle_sighupterm(signum, frame):
+        """Attempt to gracefully handle SIGHUP and SIGTERM by telling the api
+        to abort and record the cancellation before exiting."""
+
+        try:
+                if _api_inst:
+                        _api_inst.abort(result=RESULT_CANCELED)
+        except:
+                # If history operation fails for some reason, drive on.
+                pass
+
+        # Use os module to immediately exit (bypasses standard exit handling);
+        # this is preferred over raising a KeyboardInterupt as whatever module
+        # we interrupted may not expect that if they disabled SIGINT handling.
+        os._exit(EXIT_OOPS)
+
+
 if __name__ == "__main__":
         misc.setlocale(locale.LC_ALL, "", error)
         gettext.install("pkg", "/usr/share/locale",
@@ -6650,6 +6670,13 @@
         import warnings
         warnings.simplefilter('error')
 
+        # Attempt to handle SIGHUP/SIGTERM gracefully.
+        import signal
+        if portable.osname != "windows":
+                # SIGHUP not supported on windows; will cause exception.
+                signal.signal(signal.SIGHUP, handle_sighupterm)
+        signal.signal(signal.SIGTERM, handle_sighupterm)
+
         __retval = handle_errors(main_func)
         if DebugValues["timings"]:
                 def __display_timings():
--- a/src/modules/client/image.py	Fri Mar 07 11:26:34 2014 -0800
+++ b/src/modules/client/image.py	Thu May 08 23:49:21 2014 +0530
@@ -3600,9 +3600,9 @@
                         last_name, last_key, last_offset = None, None, sf.tell()
                         cnt = 0
                         while heap:
-				# This is a tight loop, so try to avoid burning
-				# CPU calling into the progress tracker
-				# excessively.
+                                # This is a tight loop, so try to avoid burning
+                                # CPU calling into the progress tracker
+                                # excessively.
                                 if len(heap) % 100 == 0:
                                         progtrack.job_add_progress(
                                             progtrack.JOB_FAST_LOOKUP)
@@ -3695,6 +3695,22 @@
                 progtrack.job_done(progtrack.JOB_FAST_LOOKUP)
                 return actdict, timestamp
 
+        def _remove_fast_lookups(self):
+                """Remove on-disk database created by _create_fast_lookups.
+                Should be called before updating image state to prevent the
+                client from seeing stale state if _create_fast_lookups is
+                interrupted."""
+
+                for fname in ("actions.stripped", "actions.offsets",
+                    "keys.conflicting"):
+                        try:
+                                portable.remove(os.path.join(
+                                    self.__action_cache_dir, fname))
+                        except EnvironmentError, e:
+                                if e.errno == errno.ENOENT:
+                                        continue
+                                raise apx._convert_error(e)
+
         def _load_actdict(self, progtrack):
                 """Read the file of offsets created in _create_fast_lookups()
                 and return the dictionary mapping action name and key value to
@@ -3752,7 +3768,7 @@
                         # This is a tight loop, so try to avoid burning
                         # CPU calling into the progress tracker excessively.
                         # Since we are already using the offset, we use that
-			# to damp calls back into the progress tracker.
+                        # to damp calls back into the progress tracker.
                         if off % 500 == 0:
                                 progtrack.plan_add_progress(
                                     progtrack.PLAN_ACTION_CONFLICT)
@@ -4223,6 +4239,18 @@
                 cleanup.
                 """
 
+                if DebugValues.get_value("simulate-plan-hang"):
+                        # If pkg5.hang file is present in image dir, then
+                        # sleep after loading configuration until file is
+                        # gone.  This is used by the test suite for signal
+                        # handling testing, etc.
+                        hang_file = os.path.join(self.imgdir, "pkg5.hang")
+                        with open(hang_file, "w") as f:
+                                f.write(str(os.getpid()))
+
+                        while os.path.exists(hang_file):
+                                time.sleep(1)
+
                 # Allow garbage collection of previous plan.
                 self.imageplan = None
 
--- a/src/modules/client/imageplan.py	Fri Mar 07 11:26:34 2014 -0800
+++ b/src/modules/client/imageplan.py	Thu May 08 23:49:21 2014 +0530
@@ -3294,6 +3294,12 @@
                 # image before the current operation is performed is desired.
                 empty_image = self.__is_image_empty()
 
+                if not empty_image:
+                        # Before proceeding, remove fast lookups database so
+                        # that if _create_fast_lookups is interrupted later the
+                        # client isn't left with invalid state.
+                        self.image._remove_fast_lookups()
+
                 self.pd._actuators.exec_prep(self.image)
 
                 self.pd._actuators.exec_pre_actuators(self.image)
--- a/src/tests/cli/t_pkg_install.py	Fri Mar 07 11:26:34 2014 -0800
+++ b/src/tests/cli/t_pkg_install.py	Thu May 08 23:49:21 2014 +0530
@@ -3917,7 +3917,7 @@
                 self.assert_("pci8086,5555" not in self.output)
 
         def test_uninstall_without_perms(self):
-                """Test for bug 4569"""
+                """Verify uninstall fails as expected for unprivileged users."""
 
                 pkg_list = [self.foo10, self.only_attr10, self.only_depend10,
                     self.only_directory10, self.only_file10,
@@ -3936,43 +3936,45 @@
                 name_pat = re.compile("^\s+open\s+(\S+)\@.*$")
 
                 def __manually_check_deps(name, install=True, exit=0):
-                        cmd = "install --no-refresh"
+                        cmd = ["install", "--no-refresh"]
                         if not install:
-                                cmd = "uninstall"
+                                cmd = ["uninstall"]
                         if name == "only_depend" and not install:
-                                self.pkg("uninstall foo", exit=exit)
+                                self.pkg(cmd + ["foo"], exit=exit)
                         elif name == "only_driver":
-                                self.pkg("%s devicebase" % cmd, exit=exit)
+                                self.pkg(cmd + ["devicebase"], exit=exit)
                         elif name == "only_group":
-                                self.pkg("%s basics" % cmd, exit=exit)
+                                self.pkg(cmd + ["basics"], exit=exit)
                         elif name == "only_hardlink":
-                                self.pkg("%s only_file" % cmd, exit=exit)
+                                self.pkg(cmd + ["only_file"], exit=exit)
                         elif name == "only_user":
                                 if install:
-                                        self.pkg("%s basics" % cmd, exit=exit)
-                                        self.pkg("%s only_group" % cmd, exit=exit)
+                                        self.pkg(cmd + ["basics"], exit=exit)
+                                        self.pkg(cmd + ["only_group"],
+                                            exit=exit)
                                 else:
-                                        self.pkg("%s only_group" % cmd, exit=exit)
-                                        self.pkg("%s basics" % cmd, exit=exit)
+                                        self.pkg(cmd + ["only_group"],
+                                            exit=exit)
+                                        self.pkg(cmd + ["basics"], exit=exit)
                 for p in pkg_list:
                         name_mat = name_pat.match(p.splitlines()[1])
                         pname = name_mat.group(1)
                         __manually_check_deps(pname, exit=[0, 4])
-                        self.pkg("install --no-refresh %s" % pname,
+                        self.pkg(["install", "--no-refresh", pname],
                             su_wrap=True, exit=1)
-                        self.pkg("install %s" % pname, su_wrap=True,
+                        self.pkg(["install", pname], su_wrap=True,
                             exit=1)
-                        self.pkg("install --no-refresh %s" % pname)
-                        self.pkg("uninstall %s" % pname, su_wrap=True,
+                        self.pkg(["install", "--no-refresh", pname])
+                        self.pkg(["uninstall", pname], su_wrap=True,
                             exit=1)
-                        self.pkg("uninstall %s" % pname)
+                        self.pkg(["uninstall", pname])
                         __manually_check_deps(pname, install=False)
 
                 for p in pkg_list:
                         name_mat = name_pat.match(p.splitlines()[1])
                         pname = name_mat.group(1)
                         __manually_check_deps(pname, exit=[0, 4])
-                        self.pkg("install --no-refresh %s" % pname)
+                        self.pkg(["install", "--no-refresh", pname])
 
                 for p in pkg_list:
                         self.pkgsend_bulk(self.rurl, p)
@@ -3981,17 +3983,17 @@
 
                 # Modifying operations require permissions needed to create and
                 # manage lock files.
-                self.pkg("update --no-refresh", su_wrap=True, exit=1)
-
-                self.pkg("refresh")
-                self.pkg("update", su_wrap=True, exit=1)
+                self.pkg(["update", "--no-refresh"], su_wrap=True, exit=1)
+
+                self.pkg(["refresh"])
+                self.pkg(["update"], su_wrap=True, exit=1)
                 # Should fail since user doesn't have permission to refresh
                 # publisher metadata.
-                self.pkg("refresh --full", su_wrap=True, exit=1)
-                self.pkg("refresh --full")
-                self.pkg("update --no-refresh", su_wrap=True,
+                self.pkg(["refresh", "--full"], su_wrap=True, exit=1)
+                self.pkg(["refresh", "--full"])
+                self.pkg(["update", "--no-refresh"], su_wrap=True,
                     exit=1)
-                self.pkg("update")
+                self.pkg(["update"])
 
         def test_bug_3222(self):
                 """ Verify that a timestamp of '0' for a passwd file will not
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src/tests/cli/t_pkg_nasty.py	Thu May 08 23:49:21 2014 +0530
@@ -0,0 +1,94 @@
+#!/usr/bin/python
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or http://www.opensolaris.org/os/licensing.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2012, 2014 Oracle and/or its affiliates. All rights reserved.
+#
+
+import testutils
+if __name__ == "__main__":
+        testutils.setup_environment("../../../proto")
+import pkg5unittest
+
+import os
+import unittest
+import signal
+import time
+
+import pkg.portable as portable
+
+class TestNastySig(pkg5unittest.SingleDepotTestCase):
+        # Only start/stop the depot once (instead of for every test)
+        persistent_setup = True
+
+        foo10 = """
+            open [email protected],5.11-0
+            close """
+
+        def setUp(self):
+                pkg5unittest.SingleDepotTestCase.setUp(self)
+                self.pkgsend_bulk(self.rurl, self.foo10)
+
+        def test_00_sig(self):
+                """Verify pkg client handles SIGTERM, SIGHUP, SIGINT gracefully
+                and writes a history entry if possible."""
+
+                if portable.osname == "windows":
+                        # SIGHUP not supported on Windows.
+                        sigs = (signal.SIGINT, signal.SIGTERM)
+                else:
+                        sigs = (signal.SIGHUP, signal.SIGINT, signal.SIGTERM)
+
+                for sig in sigs:
+                        self.pkg_image_create(self.rurl)
+
+                        imgdir = os.path.join(self.img_path(), "var", "pkg")
+                        self.assert_(os.path.exists(imgdir))
+
+                        hfile = os.path.join(imgdir, "pkg5.hang")
+                        self.assert_(not os.path.exists(hfile))
+
+                        self.pkg("purge-history")
+
+                        hndl = self.pkg(
+                            ["-D", "simulate-plan-hang=true", "install", "foo"],
+                            handle=True, coverage=False)
+
+                        # Wait for hang file before sending signal.
+                        while not os.path.exists(hfile):
+                                self.assertEqual(hndl.poll(), None)
+                                time.sleep(0.25)
+
+                        hndl.send_signal(sig)
+                        rc = hndl.wait()
+
+                        self.assertEqual(rc, 1)
+
+                        # Verify that history records operation as canceled.
+                        self.pkg(["history", "-H"])
+                        hentry = self.output.splitlines()[-1]
+                        self.assert_("install" in hentry)
+                        self.assert_("Canceled" in hentry)
+
+
+if __name__ == "__main__":
+        unittest.main()
--- a/src/tests/pkg5unittest.py	Fri Mar 07 11:26:34 2014 -0800
+++ b/src/tests/pkg5unittest.py	Thu May 08 23:49:21 2014 +0530
@@ -393,13 +393,38 @@
         def cmdline_run(self, cmdline, comment="", coverage=True, exit=0,
             handle=False, out=False, prefix="", raise_error=True, su_wrap=None,
             stderr=False, env_arg=None, usepty=False):
+
+                # If caller provides arguments as a string, the shell must
+                # process them.
+                shell = not isinstance(cmdline, list)
+
                 wrapper = ""
                 if coverage:
                         wrapper = self.coverage_cmd
-                su_wrap, su_end = self.get_su_wrapper(su_wrap=su_wrap)
-
-                cmdline = "%s%s%s %s%s" % (prefix, su_wrap, wrapper,
-                    cmdline, su_end)
+                su_wrap, su_end = self.get_su_wrapper(su_wrap=su_wrap,
+                    shell=shell)
+
+                if isinstance(cmdline, list):
+                        if wrapper:
+                                # Coverage command must be split into arguments.
+                                wrapper = wrapper.split()
+                                while wrapper:
+                                        cmdline.insert(0, wrapper.pop())
+                        if su_wrap:
+                                # This ensures that all parts of the command
+                                # line to be passed to 'su -c' are passed as a
+                                # single argument.
+                                while su_wrap[-1] != "-c":
+                                        cmdline.insert(0, su_wrap.pop())
+                                cmdline = [" ".join(cmdline)]
+                                while su_wrap:
+                                        cmdline.insert(0, su_wrap.pop())
+                        if prefix:
+                                cmdline.insert(0, prefix)
+                else:
+                        cmdline = "%s%s%s %s%s" % (prefix, su_wrap, wrapper,
+                            cmdline, su_end)
+
                 self.debugcmd(cmdline)
 
                 newenv = os.environ.copy()
@@ -411,13 +436,14 @@
                 if not usepty:
                         p = subprocess.Popen(cmdline,
                             env=newenv,
-                            shell=True,
+                            shell=shell,
                             stdout=subprocess.PIPE,
                             stderr=subprocess.PIPE)
 
                         if handle:
                                 # Do nothing more.
                                 return p
+
                         self.output, self.errout = p.communicate()
                         retcode = p.returncode
                 else:
@@ -457,7 +483,10 @@
                     subsequent_indent="\t",
                     break_long_words=False,
                     break_on_hyphens=False)
-                res = wrapper.wrap(cmdline.strip())
+                if isinstance(cmdline, list):
+                        res = wrapper.wrap(" ".join(cmdline).strip())
+                else:
+                        res = wrapper.wrap(cmdline.strip())
                 self.debug(" \\\n".join(res))
 
         def debugfilecreate(self, content, path):
@@ -489,18 +518,49 @@
         def set_debugbuf(self, s):
                 self.__debug_buf = s
 
-        def get_su_wrapper(self, su_wrap=None):
-                if su_wrap:
-                        if su_wrap == True:
-                                su_wrap = get_su_wrap_user()
-                        cov_env = " ".join(
-                            ("%s=%s" % e for e in self.coverage_env.items()))
-                        su_wrap = "su %s -c 'LD_LIBRARY_PATH=%s %s " % \
-                            (su_wrap, os.getenv("LD_LIBRARY_PATH", ""), cov_env)
+        def get_su_wrapper(self, su_wrap=None, shell=True):
+                """If 'shell' is True, the wrapper will be returned as a tuple of
+                strings of the form (su_wrap, su_end).  If 'shell' is False, the
+                wrapper willbe returned as a tuple of (args, ignore) where
+                'args' is a list of the commands and their arguments that should
+                come before the command being executed."""
+
+                if not su_wrap:
+                        return "", ""
+
+                if su_wrap == True:
+                        su_user = get_su_wrap_user()
+                else:
+                        su_user = ""
+
+                cov_env = [
+                    "%s=%s" % e
+                    for e in self.coverage_env.items()
+                ]
+
+                su_wrap = ["su"]
+
+                if su_user:
+                        su_wrap.append(su_user)
+
+                if shell:
+                        su_wrap.append("-c 'env LD_LIBRARY_PATH=%s" %
+                            os.getenv("LD_LIBRARY_PATH", ""))
+                else:
+                        # If this ever changes, cmdline_run must be updated.
+                        su_wrap.append("-c")
+                        su_wrap.append("env")
+                        su_wrap.append("LD_LIBRARY_PATH=%s" %
+                            os.getenv("LD_LIBRARY_PATH", ""))
+
+                su_wrap.extend(cov_env)
+
+                if shell:
+                        su_wrap = " ".join(su_wrap)
                         su_end = "'"
                 else:
-                        su_wrap = ""
                         su_end = ""
+
                 return su_wrap, su_end
 
         def getTeardownFunc(self):
@@ -2377,20 +2437,39 @@
 
         def pkg(self, command, exit=0, comment="", prefix="", su_wrap=None,
             out=False, stderr=False, cmd_path=None, use_img_root=True,
-            debug_smf=True, env_arg=None, coverage=True):
-                if debug_smf and "smf_cmds_dir" not in command:
-                        command = "--debug smf_cmds_dir=%s %s" % \
-                            (DebugValues["smf_cmds_dir"], command)
-                command = "-D plandesc_validate=1 %s" % command
-                if use_img_root and "-R" not in command and \
-                    "image-create" not in command and "version" not in command:
-                        command = "-R %s %s" % (self.get_img_path(), command)
+            debug_smf=True, env_arg=None, coverage=True, handle=False):
+
+                if isinstance(command, list):
+                        cmdstr = " ".join(command)
+                else:
+                        cmdstr = command
+
+                cmdline = []
+
                 if not cmd_path:
                         cmd_path = self.pkg_cmdpath
-                cmdline = "%s %s" % (cmd_path, command)
+
+                cmdline.append(cmd_path)
+
+                if use_img_root and "-R" not in cmdstr and \
+                    "image-create" not in cmdstr and "version" not in cmdstr:
+                        cmdline.extend(("-R", self.get_img_path()))
+
+                cmdline.extend(("-D", "plandesc_validate=1"))
+                cmdline.extend(("-D", "manifest_validate=Always"))
+
+                if debug_smf and "smf_cmds_dir" not in cmdstr:
+                        cmdline.extend(("-D", "smf_cmds_dir=%s" %
+                            DebugValues["smf_cmds_dir"]))
+
+                if not isinstance(command, list):
+                        cmdline = "%s %s" % (" ".join(cmdline), command)
+                else:
+                        cmdline.extend(command)
+
                 return self.cmdline_run(cmdline, exit=exit, comment=comment,
                     prefix=prefix, su_wrap=su_wrap, out=out, stderr=stderr,
-                    env_arg=env_arg, coverage=coverage)
+                    env_arg=env_arg, coverage=coverage, handle=handle)
 
         def pkgdepend_resolve(self, args, exit=0, comment="", su_wrap=False):
                 ops = ""