New: [rhinstaller/lorax/pulls/34 master] Fix pylorax and livemedia-creator to work with --no-virt and mock installs
From: "Brian C. Lane" bcl@redhat.com
Running anaconda from lmc needs to override the language reset, otherwise libuser crashes because it cannot support "C". --- src/pylorax/executils.py | 17 +++++++++++------ src/sbin/livemedia-creator | 3 ++- 2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/pylorax/executils.py b/src/pylorax/executils.py index 37cb3a6..313cf77 100644 --- a/src/pylorax/executils.py +++ b/src/pylorax/executils.py @@ -122,7 +122,8 @@ def preexec(): preexec_fn=preexec, cwd=root, env=env, **kwargs)
def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_output=True, - binary_output=False, filter_stderr=False, raise_err=False, callback=None): + binary_output=False, filter_stderr=False, raise_err=False, callback=None, + env_add=None, reset_handlers=True, reset_lang=True): """ Run an external program, log the output and return it to the caller
:param argv: The command to run and argument @@ -145,7 +146,8 @@ def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_ou stderr = subprocess.STDOUT
proc = startProgram(argv, root=root, stdin=stdin, stdout=subprocess.PIPE, stderr=stderr, - env_prune=env_prune, universal_newlines=not binary_output) + env_prune=env_prune, universal_newlines=not binary_output, + env_add=env_add, reset_handlers=reset_handlers, reset_lang=reset_lang)
if callback: while callback(proc) and proc.poll() is None: @@ -190,7 +192,8 @@ def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_ou return (proc.returncode, output_string)
def execWithRedirect(command, argv, stdin=None, stdout=None, root='/', env_prune=None, - log_output=True, binary_output=False, raise_err=False, callback=None): + log_output=True, binary_output=False, raise_err=False, callback=None, + env_add=None, reset_handlers=True, reset_lang=True): """ Run an external program and redirect the output to a file.
:param command: The command to run @@ -207,10 +210,11 @@ def execWithRedirect(command, argv, stdin=None, stdout=None, root='/', env_prune """ argv = [command] + list(argv) return _run_program(argv, stdin=stdin, stdout=stdout, root=root, env_prune=env_prune, - log_output=log_output, binary_output=binary_output, raise_err=raise_err, callback=callback)[0] + log_output=log_output, binary_output=binary_output, raise_err=raise_err, callback=callback, + env_add=env_add, reset_handlers=reset_handlers, reset_lang=reset_lang)[0]
def execWithCapture(command, argv, stdin=None, root='/', log_output=True, filter_stderr=False, - raise_err=False, callback=None): + raise_err=False, callback=None, env_add=None, reset_handlers=True, reset_lang=True): """ Run an external program and capture standard out and err.
:param command: The command to run @@ -224,7 +228,8 @@ def execWithCapture(command, argv, stdin=None, root='/', log_output=True, filter """ argv = [command] + list(argv) return _run_program(argv, stdin=stdin, root=root, log_output=log_output, filter_stderr=filter_stderr, - raise_err=raise_err, callback=callback)[1] + raise_err=raise_err, callback=callback, env_add=env_add, + reset_handlers=reset_handlers, reset_lang=reset_lang)[1]
def runcmd(cmd, **kwargs): """ run execWithRedirect with raise_err=True diff --git a/src/sbin/livemedia-creator b/src/sbin/livemedia-creator index 26e606d..1f4ac56 100755 --- a/src/sbin/livemedia-creator +++ b/src/sbin/livemedia-creator @@ -614,7 +614,8 @@ def novirt_install(opts, disk_img, disk_size, repo_url): os.environ["ANACONDA_PRODUCTVERSION"] = opts.releasever log.info("Running anaconda.") try: - execWithRedirect("anaconda", args, raise_err=True, + execWithRedirect("anaconda", args, raise_err=True, reset_lang=False, + env_add={"ANACONDA_PRODUCTNAME": opts.project, "ANACONDA_PRODUCTVERSION": opts.releasever}, callback=lambda p: not novirt_log_check(log_monitor.server.log_check, p))
# Make sure the new filesystem is correctly labeled
From: "Brian C. Lane" bcl@redhat.com
Returns output in realtime instead of buffering it. --- src/pylorax/executils.py | 96 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 92 insertions(+), 4 deletions(-)
diff --git a/src/pylorax/executils.py b/src/pylorax/executils.py index 313cf77..ac87a9d 100644 --- a/src/pylorax/executils.py +++ b/src/pylorax/executils.py @@ -20,8 +20,8 @@
import os import subprocess +from subprocess import TimeoutExpired import signal -from time import sleep
import logging log = logging.getLogger("pylorax") @@ -151,9 +151,13 @@ def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_ou
if callback: while callback(proc) and proc.poll() is None: - sleep(1) - - (output_string, err_string) = proc.communicate() + try: + (output_string, err_string) = proc.communicate(timeout=1) + break + except TimeoutExpired: + pass + else: + (output_string, err_string) = proc.communicate() if output_string: if binary_output: output_lines = [output_string] @@ -224,6 +228,7 @@ def execWithCapture(command, argv, stdin=None, root='/', log_output=True, filter :param log_output: Whether to log the output of command :param filter_stderr: Whether stderr should be excluded from the returned output :param raise_err: whether to raise a CalledProcessError if the returncode is non-zero + :param raise_err: whether to raise a CalledProcessError if the returncode is non-zero :return: The output of the command """ argv = [command] + list(argv) @@ -231,6 +236,89 @@ def execWithCapture(command, argv, stdin=None, root='/', log_output=True, filter raise_err=raise_err, callback=callback, env_add=env_add, reset_handlers=reset_handlers, reset_lang=reset_lang)[1]
+def execReadlines(command, argv, stdin=None, root='/', env_prune=None, filter_stderr=False, + raise_err=False, callback=lambda x: True, env_add=None, reset_handlers=True, reset_lang=True): + """ Execute an external command and return the line output of the command + in real-time. + + This method assumes that there is a reasonably low delay between the + end of output and the process exiting. If the child process closes + stdout and then keeps on truckin' there will be problems. + + NOTE/WARNING: UnicodeDecodeError will be raised if the output of the + external command can't be decoded as UTF-8. + + :param command: The command to run + :param argv: The argument list + :param stdin: The file object to read stdin from. + :param stdout: Optional file object to redirect stdout and stderr to. + :param root: The directory to chroot to before running command. + :param env_prune: environment variable to remove before execution + :param filter_stderr: Whether stderr should be excluded from the returned output + :param raise_err: whether to raise a CalledProcessError if the returncode is non-zero + :param raise_err: whether to raise a CalledProcessError if the returncode is non-zero + + Output from the file is not logged to program.log + This returns an iterator with the lines from the command until it has finished + """ + + class ExecLineReader(object): + """Iterator class for returning lines from a process and cleaning + up the process when the output is no longer needed. + """ + + def __init__(self, proc, argv, callback): + self._proc = proc + self._argv = argv + self._callback = callback + + def __iter__(self): + return self + + def __del__(self): + # See if the process is still running + if self._proc.poll() is None: + # Stop the process and ignore any problems that might arise + try: + self._proc.terminate() + except OSError: + pass + + def __next__(self): + # Read the next line, blocking if a line is not yet available + line = self._proc.stdout.readline().decode("utf-8") + if line == '' or not self._callback(self._proc): + # Output finished, wait for the process to end + self._proc.communicate() + + # Check for successful exit + if self._proc.returncode < 0: + raise OSError("process '%s' was killed by signal %s" % + (self._argv, -self._proc.returncode)) + elif self._proc.returncode > 0: + raise OSError("process '%s' exited with status %s" % + (self._argv, self._proc.returncode)) + raise StopIteration + + return line.strip() + + argv = [command] + argv + + if filter_stderr: + stderr = subprocess.DEVNULL + else: + stderr = subprocess.STDOUT + + try: + proc = startProgram(argv, root=root, stdin=stdin, stdout=subprocess.PIPE, stderr=stderr, bufsize=1, + env_prune=env_prune, env_add=env_add, reset_handlers=reset_handlers, reset_lang=reset_lang) + except OSError as e: + with program_log_lock: + program_log.error("Error running %s: %s", argv[0], e.strerror) + raise + + return ExecLineReader(proc, argv, callback) + def runcmd(cmd, **kwargs): """ run execWithRedirect with raise_err=True """
In reply to line 231 of src/pylorax/executils.py:
This seems a little bit extra here. ;)
In reply to line 259 of src/pylorax/executils.py:
And it got copied over here too. Plus it doesn't seem to be used anywhere in this function.
In reply to line 259 of src/pylorax/executils.py:
whoops, that'll be fixed locally. I'll also do a review of all the docstrings first.
From: "Brian C. Lane" bcl@redhat.com
Log output from anaconda as it is received so that progress can be monitored. --- src/sbin/livemedia-creator | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/src/sbin/livemedia-creator b/src/sbin/livemedia-creator index 1f4ac56..6b05045 100755 --- a/src/sbin/livemedia-creator +++ b/src/sbin/livemedia-creator @@ -51,7 +51,7 @@ from pylorax.imgutils import PartitionMount, mksparse, mkext4img, loop_detach from pylorax.imgutils import get_loop_name, dm_detach, mount, umount, Mount from pylorax.imgutils import mksquashfs, mkqcow2, mktar, mkrootfsimg from pylorax.imgutils import copytree -from pylorax.executils import execWithRedirect, runcmd +from pylorax.executils import execWithRedirect, execReadlines, runcmd from pylorax.monitor import LogMonitor from pylorax.mount import IsoMountpoint
@@ -610,13 +610,13 @@ def novirt_install(opts, disk_img, disk_size, repo_url): args += ["--remotelog", "%s:%s" % (log_monitor.host, log_monitor.port)]
# Make sure anaconda has the right product and release - os.environ["ANACONDA_PRODUCTNAME"] = opts.project - os.environ["ANACONDA_PRODUCTVERSION"] = opts.releasever log.info("Running anaconda.") try: - execWithRedirect("anaconda", args, raise_err=True, reset_lang=False, - env_add={"ANACONDA_PRODUCTNAME": opts.project, "ANACONDA_PRODUCTVERSION": opts.releasever}, - callback=lambda p: not novirt_log_check(log_monitor.server.log_check, p)) + for line in execReadlines("anaconda", args, raise_err=True, reset_lang=False, + env_add={"ANACONDA_PRODUCTNAME": opts.project, + "ANACONDA_PRODUCTVERSION": opts.releasever}, + callback=lambda p: not novirt_log_check(log_monitor.server.log_check, p)): + log.info(line)
# Make sure the new filesystem is correctly labeled args = ["-e", "/proc", "-e", "/sys", "-e", "/dev", @@ -627,12 +627,15 @@ def novirt_install(opts, disk_img, disk_size, repo_url): with PartitionMount(disk_img) as img_mount: if img_mount and img_mount.mount_dir: execWithRedirect("setfiles", args, root=img_mount.mount_dir) - except subprocess.CalledProcessError as e: + except (subprocess.CalledProcessError, OSError) as e: log.error("Running anaconda failed: %s", e) raise InstallError("novirt_install failed") finally: log_monitor.shutdown()
+ # If anaconda failed there may be things needing cleanup + execWithRedirect("anaconda-cleanup", []) + # Move the anaconda logs over to a log directory log_dir = os.path.abspath(os.path.dirname(opts.logfile)) log_anaconda = joinpaths(log_dir, "anaconda") @@ -642,11 +645,7 @@ def novirt_install(opts, disk_img, disk_size, repo_url): shutil.copy2(l, log_anaconda) os.unlink(l)
- if opts.make_iso or opts.make_fsimage: - umount(ROOT_PATH) - else: - # If anaconda failed the disk image may still be in use by dm - execWithRedirect("anaconda-cleanup", []) + if not opts.make_iso and not opts.make_fsimage: dm_name = os.path.splitext(os.path.basename(disk_img))[0] dm_path = "/dev/mapper/"+dm_name if os.path.exists(dm_path):
Looks good to me otherwise.
Added label: ACK.
anaconda-patches@lists.fedorahosted.org
-
dashea