From 00cd5fa91e1c4078e7ce3ea8ada934dacb0f34dd Mon Sep 17 00:00:00 2001 From: Steven Armstrong Date: Thu, 5 Sep 2013 22:01:21 +0200 Subject: [PATCH 01/12] Save output streams. --- cdist/__init__.py | 49 +++++-- cdist/config.py | 40 +++-- cdist/core/cdist_object.py | 13 +- cdist/core/code.py | 17 ++- cdist/core/manifest.py | 30 ++-- cdist/exec/local.py | 73 +++++++--- cdist/exec/remote.py | 75 ++++++++-- 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 + 19 files changed, 449 insertions(+), 109 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..0022b40c 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 {}" @@ -81,17 +82,47 @@ class CdistBetaRequired(cdist.Error): class CdistObjectError(Error): - """Something went wrong with an object""" + """Something went wrong while working on a specific cdist object""" + def __init__(self, cdist_object, subject=''): + self.cdist_object = cdist_object + self.object_name = cdist_object.name.center(len(cdist_object.name)+2) + if isinstance(subject, Error): + self.original_error = subject + else: + self.original_error = None + self.message = str(subject) + self.line_length = 74 - 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 in os.listdir(self.cdist_object.stderr_path): + stderr_path = os.path.join(self.cdist_object.stderr_path, + stderr_name) + # label = '---- '+ stderr_name +':stderr ' + label = stderr_name + ':stderr ' + if os.path.getsize(stderr_path) > 0: + # output.append(label) + # output.append('{0:-^50}'.format(label.center(len(label)+2))) + output.append('{0:-<{1}}'.format(label, self.line_length)) + with open(stderr_path, 'r') as fd: + output.append(fd.read()) + return '\n'.join(output) def __str__(self): - return '%s: %s (defined at %s)' % (self.name, - self.message, - self.source) + output = [] + output.append(self.message) + output.append('''{label:-<{length}} +name: {o.name} +path: {o.absolute_path} +source: {o.source} +type: {o.cdist_type.absolute_path}'''.format( + label='---- object ', + length=self.line_length, + o=self.cdist_object) + ) + output.append(self.stderr) + return '\n'.join(output) def file_to_list(filename): diff --git a/cdist/config.py b/cdist/config.py index 6ba3d0dc..65cb97d4 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: @@ -453,25 +456,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..c555a84e 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,11 @@ 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, 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..4531bd7a 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) # @@ -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() @@ -199,8 +203,24 @@ class Local(object): self.log.trace("Local mkdir: %s", path) os.makedirs(path, exist_ok=True) + def _get_std_fd(self, which): + if which == 'stdout': + base = self.stdout_base_path + else: + base = self.stderr_base_path + + path = os.path.join(base, 'remote') + stdfd = open(path, 'ba+') + return stdfd + + def _log_std_fd(self, stdfd, which, quiet, save_output): + if not quiet and save_output and stdfd is not None: + stdfd.seek(0, 0) + self.log.trace("Local {}:\n{}\n".format( + which, stdfd.read().decode())) + 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 +228,17 @@ class Local(object): assert isinstance(command, (list, tuple)), ( "list or tuple argument expected, got: %s" % command) + quiet = self.quiet_mode or quiet_mode + + close_stdout = False + close_stderr = False + if not quiet and save_output and not return_output and stdout is None: + stdout = self._get_std_fd('stdout') + close_stdout = True + if not quiet and save_output and stderr is None: + stderr = self._get_std_fd('stderr') + close_stderr = True + if env is None: env = os.environ.copy() # Export __target_host, __target_hostname, __target_fqdn @@ -225,29 +256,20 @@ class Local(object): self.log.trace("Local run: %s", command) try: - if self.quiet_mode or quiet_mode: + if quiet: stderr = subprocess.DEVNULL - else: - stderr = None - if save_output: - output, errout = exec_util.call_get_output( + if return_output: + output = subprocess.check_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() + self._log_std_fd(stderr, 'stderr', quiet, save_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: + if quiet: stdout = subprocess.DEVNULL - else: - stdout = None subprocess.check_call(command, env=env, stderr=stderr, stdout=stdout) + self._log_std_fd(stderr, 'stderr', quiet, save_output) + self._log_std_fd(stdout, 'stdout', quiet, save_output) except subprocess.CalledProcessError as e: exec_util.handle_called_process_error(e, command) except OSError as error: @@ -255,9 +277,13 @@ class Local(object): 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 +297,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..475f4294 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. @@ -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,27 @@ 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 _get_std_fd(self, which): + if which == 'stdout': + base = self.stdout_base_path + else: + base = self.stderr_base_path + + path = os.path.join(base, 'remote') + stdfd = open(path, 'ba+') + return stdfd + + def _log_std_fd(self, stdfd, which): + if stdfd is not None and stdfd != subprocess.DEVNULL: + stdfd.seek(0, 0) + self.log.trace("Remote {}: {}".format( + which, stdfd.read().decode())) + + 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 +320,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 = self._get_std_fd('stdout') + close_stdout = True + if stderr is None: + stderr = self._get_std_fd('stderr') + close_stderr = True + # export target_host, target_hostname, target_fqdn # for use in __remote_{exec,copy} scripts os_environ = os.environ.copy() @@ -305,19 +343,24 @@ 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: + output = subprocess.check_output(command, env=os_environ, + stderr=stderr) + self._log_std_fd(stderr, 'stderr') return output.decode() + else: + subprocess.check_call(command, env=os_environ, stdout=stdout, + stderr=stderr) + self._log_std_fd(stderr, 'stderr') + self._log_std_fd(stdout, 'stdout') except subprocess.CalledProcessError as e: exec_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/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..4043790c 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.NoSuchTypeError, + 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 From be4668bf2dcf11a1d32bf251635ae44217e78623 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 11 Sep 2017 11:36:15 +0200 Subject: [PATCH 02/12] Little refactoring. --- cdist/exec/local.py | 18 ++++-------------- cdist/exec/remote.py | 18 ++++-------------- cdist/exec/util.py | 16 +++++++++++++++- 3 files changed, 23 insertions(+), 29 deletions(-) diff --git a/cdist/exec/local.py b/cdist/exec/local.py index 4531bd7a..b46e8a52 100644 --- a/cdist/exec/local.py +++ b/cdist/exec/local.py @@ -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", ] @@ -203,16 +203,6 @@ class Local(object): self.log.trace("Local mkdir: %s", path) os.makedirs(path, exist_ok=True) - def _get_std_fd(self, which): - if which == 'stdout': - base = self.stdout_base_path - else: - base = self.stderr_base_path - - path = os.path.join(base, 'remote') - stdfd = open(path, 'ba+') - return stdfd - def _log_std_fd(self, stdfd, which, quiet, save_output): if not quiet and save_output and stdfd is not None: stdfd.seek(0, 0) @@ -233,10 +223,10 @@ class Local(object): close_stdout = False close_stderr = False if not quiet and save_output and not return_output and stdout is None: - stdout = self._get_std_fd('stdout') + stdout = util._get_std_fd(self, 'stdout') close_stdout = True if not quiet and save_output and stderr is None: - stderr = self._get_std_fd('stderr') + stderr = util._get_std_fd(self, 'stderr') close_stderr = True if env is None: @@ -271,7 +261,7 @@ class Local(object): self._log_std_fd(stderr, 'stderr', quiet, save_output) self._log_std_fd(stdout, 'stdout', quiet, save_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: diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py index 475f4294..e6047af8 100644 --- a/cdist/exec/remote.py +++ b/cdist/exec/remote.py @@ -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 @@ -295,16 +295,6 @@ class Remote(object): return self._run_command(cmd, env=env, return_output=return_output, stdout=stdout, stderr=stderr) - def _get_std_fd(self, which): - if which == 'stdout': - base = self.stdout_base_path - else: - base = self.stderr_base_path - - path = os.path.join(base, 'remote') - stdfd = open(path, 'ba+') - return stdfd - def _log_std_fd(self, stdfd, which): if stdfd is not None and stdfd != subprocess.DEVNULL: stdfd.seek(0, 0) @@ -326,10 +316,10 @@ class Remote(object): close_stdout = False close_stderr = False if not return_output and stdout is None: - stdout = self._get_std_fd('stdout') + stdout = util._get_std_fd(self, 'stdout') close_stdout = True if stderr is None: - stderr = self._get_std_fd('stderr') + stderr = util._get_std_fd(self, 'stderr') close_stderr = True # export target_host, target_hostname, target_fqdn @@ -354,7 +344,7 @@ class Remote(object): self._log_std_fd(stderr, 'stderr') self._log_std_fd(stdout, 'stdout') 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: diff --git a/cdist/exec/util.py b/cdist/exec/util.py index e3c090cc..561b13aa 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,14 @@ def _call_get_stdout(command, env=None, stderr=None): output = fout.read() return output + + +def _get_std_fd(obj, which): + if which == 'stdout': + base = obj.stdout_base_path + else: + base = obj.stderr_base_path + + path = os.path.join(base, 'remote') + stdfd = open(path, 'ba+') + return stdfd From 8cfa1c37d01d8c74afbdf54d83b8f2c2e3145fe9 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Tue, 12 Sep 2017 10:16:27 +0200 Subject: [PATCH 03/12] Fix copy-paste bug. Refactor and simplify code. --- cdist/exec/local.py | 40 +++++++++++++++++++--------------------- cdist/exec/remote.py | 22 +++++++++------------- cdist/exec/util.py | 16 +++++++++------- 3 files changed, 37 insertions(+), 41 deletions(-) diff --git a/cdist/exec/local.py b/cdist/exec/local.py index b46e8a52..575950c1 100644 --- a/cdist/exec/local.py +++ b/cdist/exec/local.py @@ -203,12 +203,6 @@ class Local(object): self.log.trace("Local mkdir: %s", path) os.makedirs(path, exist_ok=True) - def _log_std_fd(self, stdfd, which, quiet, save_output): - if not quiet and save_output and stdfd is not None: - stdfd.seek(0, 0) - self.log.trace("Local {}:\n{}\n".format( - which, stdfd.read().decode())) - def run(self, command, env=None, return_output=False, message_prefix=None, stdout=None, stderr=None, save_output=True, quiet_mode=False): """Run the given command with the given environment. @@ -219,15 +213,20 @@ class Local(object): "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 not quiet and save_output and not return_output and stdout is None: - stdout = util._get_std_fd(self, 'stdout') - close_stdout = True - if not quiet and save_output and stderr is None: - stderr = util._get_std_fd(self, 'stderr') - close_stderr = True + 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() @@ -246,20 +245,19 @@ class Local(object): self.log.trace("Local run: %s", command) try: - if quiet: - stderr = subprocess.DEVNULL if return_output: output = subprocess.check_output( - command, env=env, stderr=stderr) - self._log_std_fd(stderr, 'stderr', quiet, save_output) - return output.decode() + command, env=env, stderr=stderr).decode() else: - if quiet: - stdout = subprocess.DEVNULL subprocess.check_call(command, env=env, stderr=stderr, stdout=stdout) - self._log_std_fd(stderr, 'stderr', quiet, save_output) - self._log_std_fd(stdout, 'stdout', quiet, save_output) + output = None + + if do_save_output: + util.log_std_fd(self.log, stderr, 'Local stderr') + util.log_std_fd(self.log, stdout, 'Local stdout') + + return output except subprocess.CalledProcessError as e: util.handle_called_process_error(e, command) except OSError as error: diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py index e6047af8..2331d3b2 100644 --- a/cdist/exec/remote.py +++ b/cdist/exec/remote.py @@ -295,12 +295,6 @@ class Remote(object): return self._run_command(cmd, env=env, return_output=return_output, stdout=stdout, stderr=stderr) - def _log_std_fd(self, stdfd, which): - if stdfd is not None and stdfd != subprocess.DEVNULL: - stdfd.seek(0, 0) - self.log.trace("Remote {}: {}".format( - which, stdfd.read().decode())) - def _run_command(self, command, env=None, return_output=False, stdout=None, stderr=None): """Run the given command with the given environment. @@ -316,10 +310,10 @@ class Remote(object): close_stdout = False close_stderr = False if not return_output and stdout is None: - stdout = util._get_std_fd(self, 'stdout') + stdout = util.get_std_fd(self.stdout_base_path, 'remote') close_stdout = True if stderr is None: - stderr = util._get_std_fd(self, 'stderr') + stderr = util.get_std_fd(self.stderr_base_path, 'remote') close_stderr = True # export target_host, target_hostname, target_fqdn @@ -335,14 +329,16 @@ class Remote(object): stderr = subprocess.DEVNULL if return_output: output = subprocess.check_output(command, env=os_environ, - stderr=stderr) - self._log_std_fd(stderr, 'stderr') - return output.decode() + stderr=stderr).decode() else: subprocess.check_call(command, env=os_environ, stdout=stdout, stderr=stderr) - self._log_std_fd(stderr, 'stderr') - self._log_std_fd(stdout, 'stdout') + output = None + + util.log_std_fd(self.log, stderr, 'Remote stderr') + util.log_std_fd(self.log, stdout, 'Remote stdout') + + return output except subprocess.CalledProcessError as e: util.handle_called_process_error(e, command) except OSError as error: diff --git a/cdist/exec/util.py b/cdist/exec/util.py index 561b13aa..3d484013 100644 --- a/cdist/exec/util.py +++ b/cdist/exec/util.py @@ -163,12 +163,14 @@ def _call_get_stdout(command, env=None, stderr=None): return output -def _get_std_fd(obj, which): - if which == 'stdout': - base = obj.stdout_base_path - else: - base = obj.stderr_base_path - - path = os.path.join(base, 'remote') +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, stdfd, prefix): + if stdfd is not None and stdfd != subprocess.DEVNULL: + stdfd.seek(0, 0) + log.trace("{}: {}".format( + prefix, stdfd.read().decode())) From 7a3c6f202449c4d4f8bd2bf0e4c97e22c95d5fdc Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 18 Sep 2017 13:17:38 +0200 Subject: [PATCH 04/12] Add command to stdout/stderr log. --- cdist/exec/local.py | 4 ++-- cdist/exec/remote.py | 4 ++-- cdist/exec/util.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cdist/exec/local.py b/cdist/exec/local.py index 575950c1..eec79399 100644 --- a/cdist/exec/local.py +++ b/cdist/exec/local.py @@ -254,8 +254,8 @@ class Local(object): output = None if do_save_output: - util.log_std_fd(self.log, stderr, 'Local stderr') - util.log_std_fd(self.log, stdout, 'Local stdout') + 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: diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py index 2331d3b2..8fcd0981 100644 --- a/cdist/exec/remote.py +++ b/cdist/exec/remote.py @@ -335,8 +335,8 @@ class Remote(object): stderr=stderr) output = None - util.log_std_fd(self.log, stderr, 'Remote stderr') - util.log_std_fd(self.log, stdout, 'Remote stdout') + 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: diff --git a/cdist/exec/util.py b/cdist/exec/util.py index 3d484013..2f2aa38c 100644 --- a/cdist/exec/util.py +++ b/cdist/exec/util.py @@ -169,8 +169,8 @@ def get_std_fd(base_path, name): return stdfd -def log_std_fd(log, stdfd, prefix): +def log_std_fd(log, command, stdfd, prefix): if stdfd is not None and stdfd != subprocess.DEVNULL: stdfd.seek(0, 0) - log.trace("{}: {}".format( - prefix, stdfd.read().decode())) + log.trace("Command: {}; {}: {}".format( + command, prefix, stdfd.read().decode())) From 5813a6bcf379e0f1cf5adc20c483742d9a6abca9 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 18 Sep 2017 22:12:58 +0200 Subject: [PATCH 05/12] Add missed env param. --- cdist/core/code.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cdist/core/code.py b/cdist/core/code.py index c555a84e..65d095cf 100644 --- a/cdist/core/code.py +++ b/cdist/core/code.py @@ -161,7 +161,8 @@ class Code(object): 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, stdout=stdout, stderr=stderr) + 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.""" From 0f1fa4c041f3b0cd9cb2cfa52d4adb021831ff89 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 18 Sep 2017 23:04:01 +0200 Subject: [PATCH 06/12] Handle init manifest error nicely. --- cdist/__init__.py | 36 ++++++++++++++++++++++++++++++++++++ cdist/core/manifest.py | 14 +++++++++----- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/cdist/__init__.py b/cdist/__init__.py index 0022b40c..cc9c8875 100644 --- a/cdist/__init__.py +++ b/cdist/__init__.py @@ -125,6 +125,42 @@ type: {o.cdist_type.absolute_path}'''.format( return '\n'.join(output) +class InitialManifestError(Error): + """Something went wrong while executing initial manifest""" + def __init__(self, initial_manifest, stdout_path, stderr_path, subject=''): + self.initial_manifest = initial_manifest + self.stdout_path = stdout_path + self.stderr_path = stderr_path + if isinstance(subject, Error): + self.original_error = subject + else: + self.original_error = None + self.message = str(subject) + self.line_length = 74 + + @property + def stderr(self): + output = [] + label = "init" + ':stderr ' + if os.path.getsize(self.stderr_path) > 0: + output.append('{0:-<{1}}'.format(label, self.line_length)) + with open(self.stderr_path, 'r') as fd: + output.append(fd.read()) + return '\n'.join(output) + + def __str__(self): + output = [] + output.append(self.message) + output.append('''{label:-<{length}} +initial manifest: {im}'''.format( + label='---- initial manifest ', + length=self.line_length, + im=self.initial_manifest) + ) + output.append(self.stderr) + return '\n'.join(output) + + def file_to_list(filename): """Return list from \n seperated file""" if os.path.isfile(filename): diff --git a/cdist/core/manifest.py b/cdist/core/manifest.py index 12b5b005..a4277aa3 100644 --- a/cdist/core/manifest.py +++ b/cdist/core/manifest.py @@ -158,11 +158,15 @@ class Manifest(object): 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) + try: + self.local.run_script( + initial_manifest, + env=self.env_initial_manifest(initial_manifest), + message_prefix=message_prefix, + stdout=stdout, stderr=stderr) + except cdist.Error as e: + raise cdist.InitialManifestError(initial_manifest, stdout_path, + stderr_path, e) def env_type_manifest(self, cdist_object): type_manifest = os.path.join(self.local.type_path, From 7ebd69dcace7746d6f67ee1db0cb9c4e3373263b Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 18 Sep 2017 23:14:09 +0200 Subject: [PATCH 07/12] Refactor init manifest error handling. --- cdist/__init__.py | 4 ++-- cdist/config.py | 9 ++++++++- cdist/core/manifest.py | 14 +++++--------- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/cdist/__init__.py b/cdist/__init__.py index cc9c8875..5a78a1b3 100644 --- a/cdist/__init__.py +++ b/cdist/__init__.py @@ -141,8 +141,8 @@ class InitialManifestError(Error): @property def stderr(self): output = [] - label = "init" + ':stderr ' if os.path.getsize(self.stderr_path) > 0: + label = "init" + ':stderr ' output.append('{0:-<{1}}'.format(label, self.line_length)) with open(self.stderr_path, 'r') as fd: output.append(fd.read()) @@ -152,7 +152,7 @@ class InitialManifestError(Error): output = [] output.append(self.message) output.append('''{label:-<{length}} -initial manifest: {im}'''.format( +path: {im}'''.format( label='---- initial manifest ', length=self.line_length, im=self.initial_manifest) diff --git a/cdist/config.py b/cdist/config.py index 65cb97d4..5c62d07d 100644 --- a/cdist/config.py +++ b/cdist/config.py @@ -403,7 +403,14 @@ 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" + stdout_path = os.path.join(self.local.stdout_base_path, which) + stderr_path = os.path.join(self.local.stderr_base_path, which) + raise cdist.InitialManifestError(self.local.initial_manifest, + stdout_path, stderr_path, e) self.iterate_until_finished() self.cleanup() self._remove_files_dirs() diff --git a/cdist/core/manifest.py b/cdist/core/manifest.py index a4277aa3..12b5b005 100644 --- a/cdist/core/manifest.py +++ b/cdist/core/manifest.py @@ -158,15 +158,11 @@ class Manifest(object): stdout_path = os.path.join(self.local.stdout_base_path, which) with open(stderr_path, 'ba+') as stderr, \ open(stdout_path, 'ba+') as stdout: - try: - self.local.run_script( - initial_manifest, - env=self.env_initial_manifest(initial_manifest), - message_prefix=message_prefix, - stdout=stdout, stderr=stderr) - except cdist.Error as e: - raise cdist.InitialManifestError(initial_manifest, stdout_path, - stderr_path, e) + 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, From ca047c7956375b13bb537e3476985492fd6126c7 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Thu, 21 Sep 2017 07:36:53 +0200 Subject: [PATCH 08/12] Better format error output. --- cdist/__init__.py | 102 ++++++++++++++++++++-------------------------- cdist/config.py | 3 +- 2 files changed, 46 insertions(+), 59 deletions(-) diff --git a/cdist/__init__.py b/cdist/__init__.py index 5a78a1b3..832a0dca 100644 --- a/cdist/__init__.py +++ b/cdist/__init__.py @@ -81,84 +81,72 @@ class CdistBetaRequired(cdist.Error): return err_msg.format(*fmt_args) -class CdistObjectError(Error): - """Something went wrong while working on a specific cdist object""" - def __init__(self, cdist_object, subject=''): - self.cdist_object = cdist_object - self.object_name = cdist_object.name.center(len(cdist_object.name)+2) +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) - self.line_length = 74 @property def stderr(self): output = [] - for stderr_name in os.listdir(self.cdist_object.stderr_path): - stderr_path = os.path.join(self.cdist_object.stderr_path, - stderr_name) - # label = '---- '+ stderr_name +':stderr ' - label = stderr_name + ':stderr ' + for stderr_name, stderr_path in self.stderr_paths: if os.path.getsize(stderr_path) > 0: - # output.append(label) - # output.append('{0:-^50}'.format(label.center(len(label)+2))) - output.append('{0:-<{1}}'.format(label, self.line_length)) + 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): output = [] output.append(self.message) - output.append('''{label:-<{length}} -name: {o.name} -path: {o.absolute_path} -source: {o.source} -type: {o.cdist_type.absolute_path}'''.format( - label='---- object ', - length=self.line_length, - o=self.cdist_object) - ) - output.append(self.stderr) + header = '\nError in ' + self.entity_name + ' processing' + 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 InitialManifestError(Error): +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', params, stderr_paths, subject) + + +class InitialManifestError(CdistEntityError): """Something went wrong while executing initial manifest""" - def __init__(self, initial_manifest, stdout_path, stderr_path, subject=''): - self.initial_manifest = initial_manifest - self.stdout_path = stdout_path - self.stderr_path = stderr_path - if isinstance(subject, Error): - self.original_error = subject - else: - self.original_error = None - self.message = str(subject) - self.line_length = 74 - - @property - def stderr(self): - output = [] - if os.path.getsize(self.stderr_path) > 0: - label = "init" + ':stderr ' - output.append('{0:-<{1}}'.format(label, self.line_length)) - with open(self.stderr_path, 'r') as fd: - output.append(fd.read()) - return '\n'.join(output) - - def __str__(self): - output = [] - output.append(self.message) - output.append('''{label:-<{length}} -path: {im}'''.format( - label='---- initial manifest ', - length=self.line_length, - im=self.initial_manifest) - ) - output.append(self.stderr) - return '\n'.join(output) + 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 5c62d07d..dc73830c 100644 --- a/cdist/config.py +++ b/cdist/config.py @@ -407,10 +407,9 @@ class Config(object): self.manifest.run_initial_manifest(self.local.initial_manifest) except cdist.Error as e: which = "init" - stdout_path = os.path.join(self.local.stdout_base_path, which) stderr_path = os.path.join(self.local.stderr_base_path, which) raise cdist.InitialManifestError(self.local.initial_manifest, - stdout_path, stderr_path, e) + stderr_path, e) self.iterate_until_finished() self.cleanup() self._remove_files_dirs() From 21886bd31f4e39677c59476e63b6094bdf95bafd Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Thu, 21 Sep 2017 18:06:37 +0200 Subject: [PATCH 09/12] Change heading. --- cdist/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cdist/__init__.py b/cdist/__init__.py index 832a0dca..1e2c9255 100644 --- a/cdist/__init__.py +++ b/cdist/__init__.py @@ -109,7 +109,7 @@ class CdistEntityError(Error): def __str__(self): output = [] output.append(self.message) - header = '\nError in ' + self.entity_name + ' processing' + header = "\nError processing " + self.entity_name under_header = '=' * len(header) output.append(header) output.append(under_header) @@ -133,7 +133,8 @@ class CdistObjectError(CdistEntityError): stderr_path = os.path.join(cdist_object.stderr_path, stderr_name) stderr_paths.append((stderr_name, stderr_path, )) - super().__init__('object', params, stderr_paths, subject) + super().__init__("object '{}'".format(cdist_object.name), + params, stderr_paths, subject) class InitialManifestError(CdistEntityError): From 4bd1bc11b7d95a8e6cf5970dd5cad83bd9c99d25 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Fri, 22 Sep 2017 21:53:26 +0200 Subject: [PATCH 10/12] changelog++ --- docs/changelog | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog b/docs/changelog index ea98aba7..0d7b467e 100644 --- a/docs/changelog +++ b/docs/changelog @@ -32,6 +32,7 @@ next: * Documentation: Fix documentation for building custom man-pages from non-standard path (Thomas Eckert) * Core: Fix running scripts with execute bit when name without path is specified (Ander Punnar) * Type __process: Add messaging (Thomas Eckert) + * Core: Save output streams (Steven Armstrong, Darko Poljak) 4.7.0: 2017-09-22 * Core: Add configuration/config file support (Darko Poljak) From 6956e0b56cd5dbae9abaf376fa25cff2bd4978d5 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Sun, 22 Oct 2017 16:31:51 +0200 Subject: [PATCH 11/12] Fix changelog --- docs/changelog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog b/docs/changelog index 0d7b467e..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) @@ -32,7 +33,6 @@ next: * Documentation: Fix documentation for building custom man-pages from non-standard path (Thomas Eckert) * Core: Fix running scripts with execute bit when name without path is specified (Ander Punnar) * Type __process: Add messaging (Thomas Eckert) - * Core: Save output streams (Steven Armstrong, Darko Poljak) 4.7.0: 2017-09-22 * Core: Add configuration/config file support (Darko Poljak) From 7f60f70380072a20c2060fd1a222d7999ed68d81 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Tue, 21 Nov 2017 23:16:45 +0100 Subject: [PATCH 12/12] Fix after rebase. --- cdist/test/config/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cdist/test/config/__init__.py b/cdist/test/config/__init__.py index 4043790c..2b0d8b5f 100644 --- a/cdist/test/config/__init__.py +++ b/cdist/test/config/__init__.py @@ -183,7 +183,7 @@ class ConfigRunTestCase(test.CdistTestCase): first = self.object_index['__first/man'] first.requirements = ['__nosuchtype/not/exist'] self.assertRaisesCdistObjectError( - cdist.core.cdist_type.NoSuchTypeError, + cdist.core.cdist_type.InvalidTypeError, self.config.iterate_until_finished) def test_requirement_singleton_where_no_singleton(self):