From 9703e0f08e979640fc1956fbe7fd2e763dde39f1 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Tue, 9 Jan 2018 09:31:40 +0100 Subject: [PATCH] Save output streams. Implementation is 99% based on Steven's initial implementation. --- cdist/__init__.py | 76 ++++++++-- cdist/config.py | 48 +++--- cdist/core/cdist_object.py | 13 +- cdist/core/code.py | 18 ++- cdist/core/manifest.py | 30 ++-- cdist/exec/local.py | 71 +++++---- cdist/exec/remote.py | 67 ++++++--- cdist/exec/util.py | 18 ++- cdist/shell.py | 1 - cdist/test/capture_output/__init__.py | 137 ++++++++++++++++++ .../fixtures/conf/manifest/init | 4 + .../gencode-local | 6 + .../gencode-remote | 6 + .../__write_to_stdout_and_stderr/manifest | 4 + .../__write_to_stdout_and_stderr/singleton | 0 cdist/test/code/__init__.py | 7 +- cdist/test/config/__init__.py | 53 +++++-- cdist/test/exec/remote.py | 38 +++-- cdist/test/explorer/__init__.py | 4 +- cdist/test/manifest/__init__.py | 1 + docs/changelog | 1 + 21 files changed, 483 insertions(+), 120 deletions(-) create mode 100644 cdist/test/capture_output/__init__.py create mode 100755 cdist/test/capture_output/fixtures/conf/manifest/init create mode 100755 cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/gencode-local create mode 100755 cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/gencode-remote create mode 100755 cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/manifest create mode 100644 cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/singleton diff --git a/cdist/__init__.py b/cdist/__init__.py index 37651fb5..1e2c9255 100644 --- a/cdist/__init__.py +++ b/cdist/__init__.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # # 2010-2015 Nico Schottelius (nico-cdist at schottelius.org) +# 2012-2017 Steven Armstrong (steven-cdist at armstrong.cc) # # This file is part of cdist. # @@ -42,7 +43,7 @@ BANNER = """ "P' "" "" """ -REMOTE_COPY = "scp -o User=root" +REMOTE_COPY = "scp -o User=root -q" REMOTE_EXEC = "ssh -o User=root" REMOTE_CMDS_CLEANUP_PATTERN = "ssh -o User=root -O exit -S {}" @@ -80,18 +81,73 @@ class CdistBetaRequired(cdist.Error): return err_msg.format(*fmt_args) -class CdistObjectError(Error): - """Something went wrong with an object""" +class CdistEntityError(Error): + """Something went wrong while executing cdist entity""" + def __init__(self, entity_name, entity_params, stderr_paths, subject=''): + self.entity_name = entity_name + self.entity_params = entity_params + self.stderr_paths = stderr_paths + if isinstance(subject, Error): + self.original_error = subject + else: + self.original_error = None + self.message = str(subject) - def __init__(self, cdist_object, message): - self.name = cdist_object.name - self.source = " ".join(cdist_object.source) - self.message = message + @property + def stderr(self): + output = [] + for stderr_name, stderr_path in self.stderr_paths: + if os.path.getsize(stderr_path) > 0: + label_begin = '---- BEGIN ' + stderr_name + ':stderr ----' + label_end = '---- END ' + stderr_name + ':stderr ----' + output.append('\n' + label_begin) + with open(stderr_path, 'r') as fd: + output.append(fd.read()) + output.append(label_end) + return '\n'.join(output) def __str__(self): - return '%s: %s (defined at %s)' % (self.name, - self.message, - self.source) + output = [] + output.append(self.message) + header = "\nError processing " + self.entity_name + under_header = '=' * len(header) + output.append(header) + output.append(under_header) + for param_name, param_value in self.entity_params: + output.append(param_name + ': ' + str(param_value)) + output.append(self.stderr + '\n') + return '\n'.join(output) + + +class CdistObjectError(CdistEntityError): + """Something went wrong while working on a specific cdist object""" + def __init__(self, cdist_object, subject=''): + params = [ + ('name', cdist_object.name, ), + ('path', cdist_object.absolute_path, ), + ('source', " ".join(cdist_object.source), ), + ('type', cdist_object.cdist_type.absolute_path, ), + ] + stderr_paths = [] + for stderr_name in os.listdir(cdist_object.stderr_path): + stderr_path = os.path.join(cdist_object.stderr_path, + stderr_name) + stderr_paths.append((stderr_name, stderr_path, )) + super().__init__("object '{}'".format(cdist_object.name), + params, stderr_paths, subject) + + +class InitialManifestError(CdistEntityError): + """Something went wrong while executing initial manifest""" + def __init__(self, initial_manifest, stderr_path, subject=''): + params = [ + ('path', initial_manifest, ), + ] + stderr_paths = [] + stderr_paths = [ + ('init', stderr_path, ), + ] + super().__init__('initial manifest', params, stderr_paths, subject) def file_to_list(filename): diff --git a/cdist/config.py b/cdist/config.py index 6ba3d0dc..dc73830c 100644 --- a/cdist/config.py +++ b/cdist/config.py @@ -2,6 +2,7 @@ # -*- coding: utf-8 -*- # # 2010-2015 Nico Schottelius (nico-cdist at schottelius.org) +# 2013-2017 Steven Armstrong (steven-cdist at armstrong.cc) # 2016-2017 Darko Poljak (darko.poljak at gmail.com) # # This file is part of cdist. @@ -353,7 +354,9 @@ class Config(object): base_path=args.remote_out_path, quiet_mode=args.quiet, archiving_mode=args.use_archiving, - configuration=configuration) + configuration=configuration, + stdout_base_path=local.stdout_base_path, + stderr_base_path=local.stderr_base_path) cleanup_cmds = [] if cleanup_cmd: @@ -400,7 +403,13 @@ class Config(object): self._init_files_dirs() self.explorer.run_global_explorers(self.local.global_explorer_out_path) - self.manifest.run_initial_manifest(self.local.initial_manifest) + try: + self.manifest.run_initial_manifest(self.local.initial_manifest) + except cdist.Error as e: + which = "init" + stderr_path = os.path.join(self.local.stderr_base_path, which) + raise cdist.InitialManifestError(self.local.initial_manifest, + stderr_path, e) self.iterate_until_finished() self.cleanup() self._remove_files_dirs() @@ -453,25 +462,30 @@ class Config(object): objects_changed = False for cdist_object in self.object_list(): - if cdist_object.requirements_unfinished(cdist_object.requirements): - """We cannot do anything for this poor object""" - continue + try: + if cdist_object.requirements_unfinished( + cdist_object.requirements): + """We cannot do anything for this poor object""" + continue - if cdist_object.state == core.CdistObject.STATE_UNDEF: - """Prepare the virgin object""" + if cdist_object.state == core.CdistObject.STATE_UNDEF: + """Prepare the virgin object""" - self.object_prepare(cdist_object) - objects_changed = True + self.object_prepare(cdist_object) + objects_changed = True - if cdist_object.requirements_unfinished(cdist_object.autorequire): - """The previous step created objects we depend on - - wait for them - """ - continue + if cdist_object.requirements_unfinished( + cdist_object.autorequire): + """The previous step created objects we depend on - + wait for them + """ + continue - if cdist_object.state == core.CdistObject.STATE_PREPARED: - self.object_run(cdist_object) - objects_changed = True + if cdist_object.state == core.CdistObject.STATE_PREPARED: + self.object_run(cdist_object) + objects_changed = True + except cdist.Error as e: + raise cdist.CdistObjectError(cdist_object, e) return objects_changed diff --git a/cdist/core/cdist_object.py b/cdist/core/cdist_object.py index 8c7ce635..e2cd22a2 100644 --- a/cdist/core/cdist_object.py +++ b/cdist/core/cdist_object.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # -# 2011 Steven Armstrong (steven-cdist at armstrong.cc) +# 2011-2017 Steven Armstrong (steven-cdist at armstrong.cc) # 2011-2015 Nico Schottelius (nico-cdist at schottelius.org) # 2014 Daniel Heule (hda at sfs.biz) # @@ -80,6 +80,8 @@ class CdistObject(object): self.code_local_path = os.path.join(self.path, "code-local") self.code_remote_path = os.path.join(self.path, "code-remote") self.parameter_path = os.path.join(self.path, "parameter") + self.stdout_path = os.path.join(self.absolute_path, "stdout") + self.stderr_path = os.path.join(self.absolute_path, "stderr") @classmethod def list_objects(cls, object_base_path, type_base_path, object_marker): @@ -246,10 +248,11 @@ class CdistObject(object): """Create this cdist object on the filesystem. """ try: - os.makedirs(self.absolute_path, exist_ok=allow_overwrite) - absolute_parameter_path = os.path.join(self.base_path, - self.parameter_path) - os.makedirs(absolute_parameter_path, exist_ok=allow_overwrite) + for path in (self.absolute_path, + os.path.join(self.base_path, self.parameter_path), + self.stdout_path, + self.stderr_path): + os.makedirs(path, exist_ok=allow_overwrite) except EnvironmentError as error: raise cdist.Error(('Error creating directories for cdist object: ' '%s: %s') % (self, error)) diff --git a/cdist/core/code.py b/cdist/core/code.py index ad02e199..65d095cf 100644 --- a/cdist/core/code.py +++ b/cdist/core/code.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # -# 2011 Steven Armstrong (steven-cdist at armstrong.cc) +# 2011-2017 Steven Armstrong (steven-cdist at armstrong.cc) # 2011-2013 Nico Schottelius (nico-cdist at schottelius.org) # 2014 Daniel Heule (hda at sfs.biz) # @@ -127,8 +127,13 @@ class Code(object): '__object_name': cdist_object.name, }) message_prefix = cdist_object.name - return self.local.run_script(script, env=env, return_output=True, - message_prefix=message_prefix) + stderr_path = os.path.join(cdist_object.stderr_path, + 'gencode-' + which) + with open(stderr_path, 'ba+') as stderr: + return self.local.run_script(script, env=env, + return_output=True, + message_prefix=message_prefix, + stderr=stderr) def run_gencode_local(self, cdist_object): """Run the gencode-local script for the given cdist object.""" @@ -152,7 +157,12 @@ class Code(object): which_exec = getattr(self, which) script = os.path.join(which_exec.object_path, getattr(cdist_object, 'code_%s_path' % which)) - return which_exec.run_script(script, env=env) + stderr_path = os.path.join(cdist_object.stderr_path, 'code-' + which) + stdout_path = os.path.join(cdist_object.stdout_path, 'code-' + which) + with open(stderr_path, 'ba+') as stderr, \ + open(stdout_path, 'ba+') as stdout: + return which_exec.run_script(script, env=env, stdout=stdout, + stderr=stderr) def run_code_local(self, cdist_object): """Run the code-local script for the given cdist object.""" diff --git a/cdist/core/manifest.py b/cdist/core/manifest.py index fccd3a2f..12b5b005 100644 --- a/cdist/core/manifest.py +++ b/cdist/core/manifest.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # -# 2011 Steven Armstrong (steven-cdist at armstrong.cc) +# 2011-2013 Steven Armstrong (steven-cdist at armstrong.cc) # 2011-2013 Nico Schottelius (nico-cdist at schottelius.org) # # This file is part of cdist. @@ -153,10 +153,16 @@ class Manifest(object): message_prefix = "initialmanifest" self.log.verbose("Running initial manifest " + initial_manifest) - self.local.run_script(initial_manifest, - env=self.env_initial_manifest(initial_manifest), - message_prefix=message_prefix, - save_output=False) + which = "init" + stderr_path = os.path.join(self.local.stderr_base_path, which) + stdout_path = os.path.join(self.local.stdout_base_path, which) + with open(stderr_path, 'ba+') as stderr, \ + open(stdout_path, 'ba+') as stdout: + self.local.run_script( + initial_manifest, + env=self.env_initial_manifest(initial_manifest), + message_prefix=message_prefix, + stdout=stdout, stderr=stderr) def env_type_manifest(self, cdist_object): type_manifest = os.path.join(self.local.type_path, @@ -178,10 +184,16 @@ class Manifest(object): type_manifest = os.path.join(self.local.type_path, cdist_object.cdist_type.manifest_path) message_prefix = cdist_object.name + which = 'manifest' if os.path.isfile(type_manifest): self.log.verbose("Running type manifest %s for object %s", type_manifest, cdist_object.name) - self.local.run_script(type_manifest, - env=self.env_type_manifest(cdist_object), - message_prefix=message_prefix, - save_output=False) + stderr_path = os.path.join(cdist_object.stderr_path, which) + stdout_path = os.path.join(cdist_object.stdout_path, which) + with open(stderr_path, 'ba+') as stderr, \ + open(stdout_path, 'ba+') as stdout: + self.local.run_script( + type_manifest, + env=self.env_type_manifest(cdist_object), + message_prefix=message_prefix, + stdout=stdout, stderr=stderr) diff --git a/cdist/exec/local.py b/cdist/exec/local.py index 8b54a142..eec79399 100644 --- a/cdist/exec/local.py +++ b/cdist/exec/local.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # -# 2011 Steven Armstrong (steven-cdist at armstrong.cc) +# 2011-2017 Steven Armstrong (steven-cdist at armstrong.cc) # 2011-2015 Nico Schottelius (nico-cdist at schottelius.org) # 2016-2017 Darko Poljak (darko.poljak at gmail.com) # @@ -34,7 +34,7 @@ import datetime import cdist import cdist.message from cdist import core -import cdist.exec.util as exec_util +import cdist.exec.util as util CONF_SUBDIRS_LINKED = ["explorer", "files", "manifest", "type", ] @@ -120,9 +120,11 @@ class Local(object): "explorer") self.object_path = os.path.join(self.base_path, "object") self.messages_path = os.path.join(self.base_path, "messages") - self.files_path = os.path.join(self.conf_path, "files") + self.stdout_base_path = os.path.join(self.base_path, "stdout") + self.stderr_base_path = os.path.join(self.base_path, "stderr") # Depending on conf_path + self.files_path = os.path.join(self.conf_path, "files") self.global_explorer_path = os.path.join(self.conf_path, "explorer") self.manifest_path = os.path.join(self.conf_path, "manifest") self.initial_manifest = (self.custom_initial_manifest or @@ -165,6 +167,8 @@ class Local(object): self.mkdir(self.object_path) self.mkdir(self.bin_path) self.mkdir(self.cache_path) + self.mkdir(self.stdout_base_path) + self.mkdir(self.stderr_base_path) def create_files_dirs(self): self._init_directories() @@ -200,7 +204,7 @@ class Local(object): os.makedirs(path, exist_ok=True) def run(self, command, env=None, return_output=False, message_prefix=None, - save_output=True, quiet_mode=False): + stdout=None, stderr=None, save_output=True, quiet_mode=False): """Run the given command with the given environment. Return the output as a string. @@ -208,6 +212,22 @@ class Local(object): assert isinstance(command, (list, tuple)), ( "list or tuple argument expected, got: %s" % command) + quiet = self.quiet_mode or quiet_mode + do_save_output = save_output and not quiet + + close_stdout = False + close_stderr = False + if quiet: + stderr = subprocess.DEVNULL + stdout = subprocess.DEVNULL + elif do_save_output: + if not return_output and stdout is None: + stdout = util.get_std_fd(self.stdout_base_path, 'local') + close_stdout = True + if stderr is None: + stderr = util.get_std_fd(self.stderr_base_path, 'local') + close_stderr = True + if env is None: env = os.environ.copy() # Export __target_host, __target_hostname, __target_fqdn @@ -225,39 +245,33 @@ class Local(object): self.log.trace("Local run: %s", command) try: - if self.quiet_mode or quiet_mode: - stderr = subprocess.DEVNULL + if return_output: + output = subprocess.check_output( + command, env=env, stderr=stderr).decode() else: - stderr = None - if save_output: - output, errout = exec_util.call_get_output( - command, env=env, stderr=stderr) - self.log.trace("Command: {}; local stdout: {}".format( - command, output)) - # Currently, stderr is not captured. - # self.log.trace("Local stderr: {}".format(errout)) - if return_output: - return output.decode() - else: - # In some cases no output is saved. - # This is used for shell command, stdout and stderr - # must not be catched. - if self.quiet_mode or quiet_mode: - stdout = subprocess.DEVNULL - else: - stdout = None subprocess.check_call(command, env=env, stderr=stderr, stdout=stdout) + output = None + + if do_save_output: + util.log_std_fd(self.log, command, stderr, 'Local stderr') + util.log_std_fd(self.log, command, stdout, 'Local stdout') + + return output except subprocess.CalledProcessError as e: - exec_util.handle_called_process_error(e, command) + util.handle_called_process_error(e, command) except OSError as error: raise cdist.Error(" ".join(command) + ": " + error.args[1]) finally: if message_prefix: message.merge_messages() + if close_stdout: + stdout.close() + if close_stderr: + stderr.close() def run_script(self, script, env=None, return_output=False, - message_prefix=None, save_output=True): + message_prefix=None, stdout=None, stderr=None): """Run the given script with the given environment. Return the output as a string. @@ -271,8 +285,9 @@ class Local(object): script, " ".join(command)) command.append(script) - return self.run(command=command, env=env, return_output=return_output, - message_prefix=message_prefix, save_output=save_output) + return self.run(command, env=env, return_output=return_output, + message_prefix=message_prefix, stdout=stdout, + stderr=stderr) def _cache_subpath_repl(self, matchobj): if matchobj.group(2) == '%P': diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py index 44bc2927..8fcd0981 100644 --- a/cdist/exec/remote.py +++ b/cdist/exec/remote.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # -# 2011 Steven Armstrong (steven-cdist at armstrong.cc) +# 2011-2017 Steven Armstrong (steven-cdist at armstrong.cc) # 2011-2013 Nico Schottelius (nico-cdist at schottelius.org) # # This file is part of cdist. @@ -27,7 +27,7 @@ import logging import multiprocessing import cdist -import cdist.exec.util as exec_util +import cdist.exec.util as util import cdist.util.ipaddr as ipaddr from cdist.mputil import mp_pool_run @@ -63,7 +63,9 @@ class Remote(object): base_path=None, quiet_mode=None, archiving_mode=None, - configuration=None): + configuration=None, + stdout_base_path=None, + stderr_base_path=None): self.target_host = target_host self._exec = remote_exec self._copy = remote_copy @@ -79,6 +81,9 @@ class Remote(object): else: self.configuration = {} + self.stdout_base_path = stdout_base_path + self.stderr_base_path = stderr_base_path + self.conf_path = os.path.join(self.base_path, "conf") self.object_path = os.path.join(self.base_path, "object") @@ -105,7 +110,7 @@ class Remote(object): self._open_logger() def _init_env(self): - """Setup environment for scripts - HERE????""" + """Setup environment for scripts.""" # FIXME: better do so in exec functions that require it! os.environ['__remote_copy'] = self._copy os.environ['__remote_exec'] = self._exec @@ -237,7 +242,8 @@ class Remote(object): self.log.trace(("Multiprocessing for parallel transfer " "finished")) - def run_script(self, script, env=None, return_output=False): + def run_script(self, script, env=None, return_output=False, stdout=None, + stderr=None): """Run the given script with the given environment on the remote side. Return the output as a string. @@ -249,9 +255,11 @@ class Remote(object): ] command.append(script) - return self.run(command, env, return_output) + return self.run(command, env=env, return_output=return_output, + stdout=stdout, stderr=stderr) - def run(self, command, env=None, return_output=False): + def run(self, command, env=None, return_output=False, stdout=None, + stderr=None): """Run the given command with the given environment on the remote side. Return the output as a string. @@ -284,9 +292,11 @@ class Remote(object): cmd.append(string_cmd) else: cmd.extend(command) - return self._run_command(cmd, env=env, return_output=return_output) + return self._run_command(cmd, env=env, return_output=return_output, + stdout=stdout, stderr=stderr) - def _run_command(self, command, env=None, return_output=False): + def _run_command(self, command, env=None, return_output=False, stdout=None, + stderr=None): """Run the given command with the given environment. Return the output as a string. @@ -294,6 +304,18 @@ class Remote(object): assert isinstance(command, (list, tuple)), ( "list or tuple argument expected, got: %s" % command) + if return_output and stdout is not subprocess.PIPE: + self.log.debug("return_output is True, ignoring stdout") + + close_stdout = False + close_stderr = False + if not return_output and stdout is None: + stdout = util.get_std_fd(self.stdout_base_path, 'remote') + close_stdout = True + if stderr is None: + stderr = util.get_std_fd(self.stderr_base_path, 'remote') + close_stderr = True + # export target_host, target_hostname, target_fqdn # for use in __remote_{exec,copy} scripts os_environ = os.environ.copy() @@ -305,19 +327,26 @@ class Remote(object): try: if self.quiet_mode: stderr = subprocess.DEVNULL - else: - stderr = None - output, errout = exec_util.call_get_output( - command, env=os_environ, stderr=stderr) - self.log.trace("Command: {}; remote stdout: {}".format( - command, output)) - # Currently, stderr is not captured. - # self.log.trace("Remote stderr: {}".format(errout)) if return_output: - return output.decode() + output = subprocess.check_output(command, env=os_environ, + stderr=stderr).decode() + else: + subprocess.check_call(command, env=os_environ, stdout=stdout, + stderr=stderr) + output = None + + util.log_std_fd(self.log, command, stderr, 'Remote stderr') + util.log_std_fd(self.log, command, stdout, 'Remote stdout') + + return output except subprocess.CalledProcessError as e: - exec_util.handle_called_process_error(e, command) + util.handle_called_process_error(e, command) except OSError as error: raise cdist.Error(" ".join(command) + ": " + error.args[1]) except UnicodeDecodeError: raise DecodeError(command) + finally: + if close_stdout: + stdout.close() + if close_stderr: + stderr.close() diff --git a/cdist/exec/util.py b/cdist/exec/util.py index e3c090cc..2f2aa38c 100644 --- a/cdist/exec/util.py +++ b/cdist/exec/util.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # -# 2016 Darko Poljak (darko.poljak at gmail.com) +# 2016-2017 Darko Poljak (darko.poljak at gmail.com) # # This file is part of cdist. # @@ -20,6 +20,7 @@ # import subprocess +import os from tempfile import TemporaryFile import cdist @@ -115,6 +116,7 @@ import cdist # return (result.stdout, result.stderr) +# Currently not used. def call_get_output(command, env=None, stderr=None): """Run the given command with the given environment. Return the tuple of stdout and stderr output as a byte strings. @@ -145,6 +147,7 @@ def handle_called_process_error(err, command): " ".join(command), err.returncode, output)) +# Currently not used. def _call_get_stdout(command, env=None, stderr=None): """Run the given command with the given environment. Return the stdout output as a byte string, stderr is ignored. @@ -158,3 +161,16 @@ def _call_get_stdout(command, env=None, stderr=None): output = fout.read() return output + + +def get_std_fd(base_path, name): + path = os.path.join(base_path, name) + stdfd = open(path, 'ba+') + return stdfd + + +def log_std_fd(log, command, stdfd, prefix): + if stdfd is not None and stdfd != subprocess.DEVNULL: + stdfd.seek(0, 0) + log.trace("Command: {}; {}: {}".format( + command, prefix, stdfd.read().decode())) diff --git a/cdist/shell.py b/cdist/shell.py index a9457aaf..60b6a9f0 100644 --- a/cdist/shell.py +++ b/cdist/shell.py @@ -89,7 +89,6 @@ class Shell(object): self._init_environment() log.trace("Starting shell...") - # save_output=False -> do not catch stdout and stderr self.local.run([self.shell], self.env, save_output=False) log.trace("Finished shell.") diff --git a/cdist/test/capture_output/__init__.py b/cdist/test/capture_output/__init__.py new file mode 100644 index 00000000..229cbf70 --- /dev/null +++ b/cdist/test/capture_output/__init__.py @@ -0,0 +1,137 @@ +# -*- coding: utf-8 -*- +# +# 2011-2013 Steven Armstrong (steven-cdist at armstrong.cc) +# 2012-2013 Nico Schottelius (nico-cdist at schottelius.org) +# +# This file is part of cdist. +# +# cdist is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# cdist is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with cdist. If not, see . +# +# + +import os +import shutil + +import cdist +from cdist import core +from cdist import test +from cdist.exec import local +from cdist.exec import remote +from cdist.core import code +from cdist.core import manifest + +import os.path as op +my_dir = op.abspath(op.dirname(__file__)) +fixtures = op.join(my_dir, 'fixtures') +conf_dir = op.join(fixtures, 'conf') + + +class CaptureOutputTestCase(test.CdistTestCase): + + def setUp(self): + # logging.root.setLevel(logging.TRACE) + self.temp_dir = self.mkdtemp() + + self.local_dir = os.path.join(self.temp_dir, "local") + self.hostdir = cdist.str_hash(self.target_host[0]) + self.host_base_path = os.path.join(self.local_dir, self.hostdir) + os.makedirs(self.host_base_path) + self.local = local.Local( + target_host=self.target_host, + target_host_tags=None, + base_root_path=self.host_base_path, + host_dir_name=self.hostdir, + exec_path=cdist.test.cdist_exec_path, + add_conf_dirs=[conf_dir]) + self.local.create_files_dirs() + + self.remote_dir = self.mkdtemp() + remote_exec = self.remote_exec + remote_copy = self.remote_copy + self.remote = remote.Remote( + target_host=self.target_host, + remote_exec=remote_exec, + remote_copy=remote_copy, + base_path=self.remote_dir, + stdout_base_path=self.local.stdout_base_path, + stderr_base_path=self.local.stderr_base_path) + self.remote.create_files_dirs() + + self.code = code.Code(self.target_host, self.local, self.remote) + + self.manifest = manifest.Manifest(self.target_host, self.local) + + self.cdist_type = core.CdistType(self.local.type_path, + '__write_to_stdout_and_stderr') + self.cdist_object = core.CdistObject(self.cdist_type, + self.local.object_path, + self.local.object_marker_name, + '') + self.cdist_object.create() + self.output_dirs = { + 'object': { + 'stdout': os.path.join(self.cdist_object.absolute_path, + 'stdout'), + 'stderr': os.path.join(self.cdist_object.absolute_path, + 'stderr'), + }, + 'init': { + 'stdout': os.path.join(self.local.base_path, 'stdout'), + 'stderr': os.path.join(self.local.base_path, 'stderr'), + }, + } + + def tearDown(self): + shutil.rmtree(self.local_dir) + shutil.rmtree(self.remote_dir) + shutil.rmtree(self.temp_dir) + + def _test_output(self, which, target, streams=('stdout', 'stderr')): + for stream in streams: + _should = '{0}: {1}\n'.format(which, stream) + stream_path = os.path.join(self.output_dirs[target][stream], which) + with open(stream_path, 'r') as fd: + _is = fd.read() + self.assertEqual(_should, _is) + + def test_capture_code_output(self): + self.cdist_object.code_local = self.code.run_gencode_local( + self.cdist_object) + self._test_output('gencode-local', 'object', ('stderr',)) + + self.code.run_code_local(self.cdist_object) + self._test_output('code-local', 'object') + + self.cdist_object.code_remote = self.code.run_gencode_remote( + self.cdist_object) + self._test_output('gencode-remote', 'object', ('stderr',)) + + self.code.transfer_code_remote(self.cdist_object) + self.code.run_code_remote(self.cdist_object) + self._test_output('code-remote', 'object') + + def test_capture_manifest_output(self): + self.manifest.run_type_manifest(self.cdist_object) + self._test_output('manifest', 'object') + + def test_capture_init_manifest_output(self): + initial_manifest = os.path.join(conf_dir, 'manifest', 'init') + self.manifest.run_initial_manifest(initial_manifest) + self._test_output('init', 'init') + + +if __name__ == "__main__": + import unittest + + unittest.main() diff --git a/cdist/test/capture_output/fixtures/conf/manifest/init b/cdist/test/capture_output/fixtures/conf/manifest/init new file mode 100755 index 00000000..68d7da97 --- /dev/null +++ b/cdist/test/capture_output/fixtures/conf/manifest/init @@ -0,0 +1,4 @@ +#!/bin/sh + +echo "init: stdout" +echo "init: stderr" >&2 diff --git a/cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/gencode-local b/cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/gencode-local new file mode 100755 index 00000000..1946dbd3 --- /dev/null +++ b/cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/gencode-local @@ -0,0 +1,6 @@ +#!/bin/sh + +echo "gencode-local: stderr" >&2 + +echo "echo \"code-local: stdout\"" +echo "echo \"code-local: stderr\" >&2" diff --git a/cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/gencode-remote b/cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/gencode-remote new file mode 100755 index 00000000..f713b932 --- /dev/null +++ b/cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/gencode-remote @@ -0,0 +1,6 @@ +#!/bin/sh + +echo "gencode-remote: stderr" >&2 + +echo "echo \"code-remote: stdout\"" +echo "echo \"code-remote: stderr\" >&2" diff --git a/cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/manifest b/cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/manifest new file mode 100755 index 00000000..4f122f25 --- /dev/null +++ b/cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/manifest @@ -0,0 +1,4 @@ +#!/bin/sh + +echo "manifest: stdout" +echo "manifest: stderr" >&2 diff --git a/cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/singleton b/cdist/test/capture_output/fixtures/conf/type/__write_to_stdout_and_stderr/singleton new file mode 100644 index 00000000..e69de29b diff --git a/cdist/test/code/__init__.py b/cdist/test/code/__init__.py index c1d19d33..bf80110d 100644 --- a/cdist/test/code/__init__.py +++ b/cdist/test/code/__init__.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # -# 2011 Steven Armstrong (steven-cdist at armstrong.cc) +# 2011-2017 Steven Armstrong (steven-cdist at armstrong.cc) # 2012-2015 Nico Schottelius (nico-cdist at schottelius.org) # # This file is part of cdist. @@ -61,7 +61,9 @@ class CodeTestCase(test.CdistTestCase): target_host=self.target_host, remote_exec=remote_exec, remote_copy=remote_copy, - base_path=self.remote_dir) + base_path=self.remote_dir, + stdout_base_path=self.local.stdout_base_path, + stderr_base_path=self.local.stderr_base_path) self.remote.create_files_dirs() self.code = code.Code(self.target_host, self.local, self.remote) @@ -152,6 +154,7 @@ class CodeTestCase(test.CdistTestCase): self.code.transfer_code_remote(self.cdist_object) self.code.run_code_remote(self.cdist_object) + if __name__ == '__main__': import unittest unittest.main() diff --git a/cdist/test/config/__init__.py b/cdist/test/config/__init__.py index 252b7c8f..2b0d8b5f 100644 --- a/cdist/test/config/__init__.py +++ b/cdist/test/config/__init__.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # -# 2010-2011 Steven Armstrong (steven-cdist at armstrong.cc) +# 2010-2017 Steven Armstrong (steven-cdist at armstrong.cc) # 2012-2015 Nico Schottelius (nico-cdist at schottelius.org) # 2014 Daniel Heule (hda at sfs.biz) # @@ -45,6 +45,19 @@ expected_object_names = sorted([ '__third/moon']) +class CdistObjectErrorContext(object): + def __init__(self, original_error): + self.original_error = original_error + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, tb): + if exc_type is not None: + if exc_value.original_error: + raise exc_value.original_error + + class ConfigRunTestCase(test.CdistTestCase): def setUp(self): @@ -87,7 +100,9 @@ class ConfigRunTestCase(test.CdistTestCase): target_host=self.target_host, remote_copy=self.remote_copy, remote_exec=self.remote_exec, - base_path=self.remote_dir) + base_path=self.remote_dir, + stdout_base_path=self.local.stdout_base_path, + stderr_base_path=self.local.stderr_base_path) self.local.object_path = self.object_base_path self.local.type_path = type_base_path @@ -102,6 +117,20 @@ class ConfigRunTestCase(test.CdistTestCase): os.environ = self.orig_environ shutil.rmtree(self.temp_dir) + def assertRaisesCdistObjectError(self, original_error, callable_obj): + """ + Test if a raised CdistObjectError was caused by the given + original_error. + """ + with self.assertRaises(original_error): + try: + callable_obj() + except cdist.CdistObjectError as e: + if e.original_error: + raise e.original_error + else: + raise + def test_dependency_resolution(self): first = self.object_index['__first/man'] second = self.object_index['__second/on-the'] @@ -137,29 +166,33 @@ class ConfigRunTestCase(test.CdistTestCase): first.requirements = [second.name] second.requirements = [first.name] - with self.assertRaises(cdist.UnresolvableRequirementsError): - self.config.iterate_until_finished() + self.assertRaisesCdistObjectError( + cdist.UnresolvableRequirementsError, + self.config.iterate_until_finished) def test_missing_requirements(self): """Throw an error if requiring something non-existing""" first = self.object_index['__first/man'] first.requirements = ['__first/not/exist'] - with self.assertRaises(cdist.UnresolvableRequirementsError): - self.config.iterate_until_finished() + self.assertRaisesCdistObjectError( + cdist.UnresolvableRequirementsError, + self.config.iterate_until_finished) def test_requirement_broken_type(self): """Unknown type should be detected in the resolving process""" first = self.object_index['__first/man'] first.requirements = ['__nosuchtype/not/exist'] - with self.assertRaises(cdist.core.cdist_type.InvalidTypeError): - self.config.iterate_until_finished() + self.assertRaisesCdistObjectError( + cdist.core.cdist_type.InvalidTypeError, + self.config.iterate_until_finished) def test_requirement_singleton_where_no_singleton(self): """Missing object id should be detected in the resolving process""" first = self.object_index['__first/man'] first.requirements = ['__first'] - with self.assertRaises(cdist.core.cdist_object.MissingObjectIdError): - self.config.iterate_until_finished() + self.assertRaisesCdistObjectError( + cdist.core.cdist_object.MissingObjectIdError, + self.config.iterate_until_finished) def test_dryrun(self): """Test if the dryrun option is working like expected""" diff --git a/cdist/test/exec/remote.py b/cdist/test/exec/remote.py index 5137c59c..a7fe384d 100644 --- a/cdist/test/exec/remote.py +++ b/cdist/test/exec/remote.py @@ -40,12 +40,24 @@ class RemoteTestCase(test.CdistTestCase): ) # another temp dir for remote base path self.base_path = self.mkdtemp() + self.remote = self.create_remote() + + def create_remote(self, *args, **kwargs): + if not args: + args = (self.target_host,) + kwargs.setdefault('base_path', self.base_path) user = getpass.getuser() - remote_exec = "ssh -o User=%s -q" % user - remote_copy = "scp -o User=%s -q" % user - self.remote = remote.Remote(self.target_host, base_path=self.base_path, - remote_exec=remote_exec, - remote_copy=remote_copy) + kwargs.setdefault('remote_exec', 'ssh -o User=%s -q' % user) + kwargs.setdefault('remote_copy', 'scp -o User=%s -q' % user) + if 'stdout_base_path' not in kwargs: + stdout_path = os.path.join(self.temp_dir, 'stdout') + os.makedirs(stdout_path, exist_ok=True) + kwargs['stdout_base_path'] = stdout_path + if 'stderr_base_path' not in kwargs: + stderr_path = os.path.join(self.temp_dir, 'stderr') + os.makedirs(stderr_path, exist_ok=True) + kwargs['stderr_base_path'] = stderr_path + return remote.Remote(*args, **kwargs) def tearDown(self): shutil.rmtree(self.temp_dir) @@ -155,8 +167,8 @@ class RemoteTestCase(test.CdistTestCase): os.chmod(remote_exec_path, 0o755) remote_exec = remote_exec_path remote_copy = "echo" - r = remote.Remote(self.target_host, base_path=self.base_path, - remote_exec=remote_exec, remote_copy=remote_copy) + r = self.create_remote(remote_exec=remote_exec, + remote_copy=remote_copy) self.assertEqual(r.run('true', return_output=True), "%s\n" % self.target_host[0]) @@ -167,8 +179,8 @@ class RemoteTestCase(test.CdistTestCase): os.chmod(remote_exec_path, 0o755) remote_exec = remote_exec_path remote_copy = "echo" - r = remote.Remote(self.target_host, base_path=self.base_path, - remote_exec=remote_exec, remote_copy=remote_copy) + r = self.create_remote(remote_exec=remote_exec, + remote_copy=remote_copy) handle, script = self.mkstemp(dir=self.temp_dir) with os.fdopen(handle, "w") as fd: fd.writelines(["#!/bin/sh\n", "true"]) @@ -189,8 +201,8 @@ class RemoteTestCase(test.CdistTestCase): os.chmod(remote_exec_path, 0o755) remote_exec = remote_exec_path remote_copy = "echo" - r = remote.Remote(self.target_host, base_path=self.base_path, - remote_exec=remote_exec, remote_copy=remote_copy) + r = self.create_remote(remote_exec=remote_exec, + remote_copy=remote_copy) output = r.run_script(script, return_output=True) self.assertEqual(output, "no_env\n") @@ -202,8 +214,8 @@ class RemoteTestCase(test.CdistTestCase): env = { '__object': 'test_object', } - r = remote.Remote(self.target_host, base_path=self.base_path, - remote_exec=remote_exec, remote_copy=remote_copy) + r = self.create_remote(remote_exec=remote_exec, + remote_copy=remote_copy) output = r.run_script(script, env=env, return_output=True) self.assertEqual(output, "test_object\n") diff --git a/cdist/test/explorer/__init__.py b/cdist/test/explorer/__init__.py index dcdd91f1..1c4e4bc4 100644 --- a/cdist/test/explorer/__init__.py +++ b/cdist/test/explorer/__init__.py @@ -64,7 +64,9 @@ class ExplorerClassTestCase(test.CdistTestCase): target_host=self.target_host, remote_exec=self.remote_exec, remote_copy=self.remote_copy, - base_path=self.remote_base_path) + base_path=self.remote_base_path, + stdout_base_path=self.local.stdout_base_path, + stderr_base_path=self.local.stderr_base_path) self.remote.create_files_dirs() self.explorer = explorer.Explorer( diff --git a/cdist/test/manifest/__init__.py b/cdist/test/manifest/__init__.py index fffa77b8..68e777a4 100644 --- a/cdist/test/manifest/__init__.py +++ b/cdist/test/manifest/__init__.py @@ -109,6 +109,7 @@ class ManifestTestCase(test.CdistTestCase): cdist_object = core.CdistObject(cdist_type, self.local.object_path, self.local.object_marker_name, 'whatever') + cdist_object.create() handle, output_file = self.mkstemp(dir=self.temp_dir) os.close(handle) os.environ['__cdist_test_out'] = output_file diff --git a/docs/changelog b/docs/changelog index ea98aba7..23b01635 100644 --- a/docs/changelog +++ b/docs/changelog @@ -11,6 +11,7 @@ next: * Type __letsencrypt_cert: Add nonparallel; make admin-email required (Kamila Součková) * Type __package_pkgng_freebsd: Redirect stdout and stderr to /dev/null instead of closing them (michal-hanu-la) * Type __daemontools: Make it more robust and clean up the code (Kamila Součková) + * Core: Save output streams (Steven Armstrong, Darko Poljak) 4.7.3: 2017-11-10 * Type __ccollect_source: Add create destination parameter (Dominique Roux)