[PATCH 1/2] Fixup kickstart script logging

Brian C. Lane bcl at redhat.com
Wed Feb 13 22:21:03 UTC 2013


From: "Brian C. Lane" <bcl at redhat.com>

Changes to the execWith* methods removed the ability to handle a string
as stdin/out/err. Change the callers to first open where they want
output to be sent to and pass that instead.
Only stdout is needed because _run_program combines stdout and stderr,
pass stderr through but ignore it.

Also clean up a couple places that were passing strings that don't need
to.
---
 anaconda                |  3 +--
 pyanaconda/iutil.py     | 61 ++++++++++++++++++++++++++++++-------------------
 pyanaconda/kickstart.py | 35 +++++++++++++---------------
 pyanaconda/network.py   |  3 +--
 4 files changed, 55 insertions(+), 47 deletions(-)

diff --git a/anaconda b/anaconda
index 324d31e..06773ca 100755
--- a/anaconda
+++ b/anaconda
@@ -85,8 +85,7 @@ def startMetacityWM():
         try:
             args = ['--display', ':1',
                     '--sm-disable']
-            iutil.execWithRedirect('metacity', args,
-                                   stdout='/dev/null', stderr='/dev/null')
+            iutil.execWithRedirect('metacity', args)
         except BaseException as e:
             # catch all possible exceptions
             log.error("Problems running the window manager: %s" % str(e))
diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
index dbc7c3b..2d2b08c 100644
--- a/pyanaconda/iutil.py
+++ b/pyanaconda/iutil.py
@@ -49,7 +49,15 @@ def augmentEnv():
                })
     return env
 
-def _run_program(argv, root='/', stdin=None, env_prune=None):
+def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None):
+    """ Run an external program, log the output and return it to the caller
+        @param argv The command to run and argument
+        @param root The directory to chroot to before running command.
+        @param stdin The file descriptor to read stdin from.
+        @param stdout Optional file descriptor to write stdout and stderr to.
+        @param env_prune environment variable to remove before execution
+        @return The return code of the command and the output
+    """
     if env_prune is None:
         env_prune = []
 
@@ -75,6 +83,8 @@ def _run_program(argv, root='/', stdin=None, env_prune=None):
             if out:
                 for line in out.splitlines():
                     program_log.info(line)
+                    if stdout:
+                        stdout.write(line)
 
         except OSError as e:
             program_log.error("Error running %s: %s" % (argv[0], e.strerror))
@@ -84,41 +94,44 @@ def _run_program(argv, root='/', stdin=None, env_prune=None):
 
     return (proc.returncode, out)
 
-## Run an external program and redirect the output to a file.
-# @param command The command to run.
-# @param argv A list of arguments.
-# @param stdin The file descriptor to read stdin from.
-# @param stdout The file descriptor to redirect stdout to.
-# @param stderr The file descriptor to redirect stderr to.
-# @param root The directory to chroot to before running command.
-# @return The return code of command.
-def execWithRedirect(command, argv, stdin = None, stdout = None,
-                     stderr = None, root = '/', env_prune=[]):
+def execWithRedirect(command, argv, stdin=None, stdout=None,
+                     stderr=None, root='/', env_prune=[]):
+    """ Run an external program and redirect the output to a file.
+        @param command The command to run
+        @param argv The argument list
+        @param stdin The file descriptor to read stdin from.
+        @param stdout Optional file descriptor to redirect stdout and stderr to.
+        @param stderr not used
+        @param root The directory to chroot to before running command.
+        @param env_prune environment variable to remove before execution
+        @return The return code of the command
+    """
     if flags.testing:
         log.info("not running command because we're testing: %s %s"
                    % (command, " ".join(argv)))
         return 0
 
     argv = [command] + argv
-    return _run_program(argv, stdin=stdin, root=root, env_prune=env_prune)[0]
-
-## Run an external program and capture standard out.
-# @param command The command to run.
-# @param argv A list of arguments.
-# @param stdin The file descriptor to read stdin from.
-# @param stderr The file descriptor to redirect stderr to.
-# @param root The directory to chroot to before running command.
-# @param fatal Boolean to determine if non-zero exit is fatal.
-# @return The output of command from stdout.
-def execWithCapture(command, argv, stdin = None, stderr = None, root='/',
-                    fatal = False):
+    return _run_program(argv, stdin=stdin, stdout=stdout, root=root, env_prune=env_prune)[0]
+
+def execWithCapture(command, argv, stdin=None, stderr=None, root='/',
+                    fatal=False):
+    """ Run an external program and capture standard out and err.
+        @param command The command to run
+        @param argv The argument list
+        @param stdin The file descriptor to read stdin from.
+        @param stderr not used
+        @param root The directory to chroot to before running command.
+        @param fatal not used
+        @return The output of the command
+    """
     if flags.testing:
         log.info("not running command because we're testing: %s %s"
                     % (command, " ".join(argv)))
         return ""
 
     argv = [command] + argv
-    return _run_program(argv, stdin=stdin, root=root)[1]
+    return _run_program(argv, stdin=stdin, stdout=stdout, root=root)[1]
 
 ## Run a shell.
 def execConsole():
diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
index a93c552..35f90e2 100644
--- a/pyanaconda/kickstart.py
+++ b/pyanaconda/kickstart.py
@@ -81,7 +81,17 @@ packagesSeen = False
 topology = None
 
 class AnacondaKSScript(KSScript):
+    """ Execute a kickstart script
+
+        This will write the script to a file named /tmp/ks-script- before
+        execution.
+        Output is logged by the program logger, the path specified by --log
+        or to /tmp/ks-script-*.log
+    """
     def run(self, chroot):
+        """ Run the kickstart script
+            @param chroot directory path to chroot into before execution
+        """
         if self.inChroot:
             scriptRoot = chroot
         else:
@@ -93,9 +103,9 @@ class AnacondaKSScript(KSScript):
         os.close(fd)
         os.chmod(path, 0700)
 
-        # Always log stdout/stderr from scripts.  Using --logfile just lets you
+        # Always log stdout/stderr from scripts.  Using --log just lets you
         # pick where it goes.  The script will also be logged to program.log
-        # because of execWithRedirect, and to anaconda.log if the script fails.
+        # because of execWithRedirect.
         if self.logfile:
             if self.inChroot:
                 messages = "%s/%s" % (scriptRoot, self.logfile)
@@ -110,26 +120,13 @@ class AnacondaKSScript(KSScript):
             # chroot later.
             messages = "/tmp/%s.log" % os.path.basename(path)
 
-        rc = iutil.execWithRedirect(self.interp, ["/tmp/%s" % os.path.basename(path)],
-                                    stdin = messages, stdout = messages, stderr = messages,
-                                    root = scriptRoot)
+        with open(messages, "w") as fp:
+            rc = iutil.execWithRedirect(self.interp, ["/tmp/%s" % os.path.basename(path)],
+                                        stdin=fp, stdout=fp,
+                                        root = scriptRoot)
 
-        # Always log an error.  Only fail if we have a handle on the
-        # windowing system and the kickstart file included --erroronfail.
         if rc != 0:
             log.error("Error code %s running the kickstart script at line %s" % (rc, self.lineno))
-
-            if os.path.isfile(messages):
-                try:
-                    f = open(messages, "r")
-                except IOError as e:
-                    err = None
-                else:
-                    err = f.readlines()
-                    f.close()
-                    for l in err:
-                        log.error("\t%s" % l)
-
             if self.errorOnFail:
                 errorHandler.cb(ScriptError(), self.lineno, err)
                 sys.exit(0)
diff --git a/pyanaconda/network.py b/pyanaconda/network.py
index 294f937..0d352d2 100644
--- a/pyanaconda/network.py
+++ b/pyanaconda/network.py
@@ -836,8 +836,7 @@ def set_hostname(hn):
         return
 
     log.info("setting installation environment hostname to %s" % hn)
-    iutil.execWithRedirect("hostname", ["-v", hn ],
-                           stdout="/dev/tty5", stderr="/dev/tty5")
+    iutil.execWithRedirect("hostname", ["-v", hn ])
 
 def get_NM_settings_value(iface, key1, key2):
     value = ""
-- 
1.8.1



More information about the anaconda-patches mailing list