From ad1e51cb2e75799831ed8d89ce1c2bc2bb8ac43a Mon Sep 17 00:00:00 2001 From: Nico Schottelius Date: Sat, 15 Oct 2011 01:11:54 +0200 Subject: [PATCH 1/8] catch unicodedecodeerror Signed-off-by: Nico Schottelius --- lib/cdist/exec/remote.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/cdist/exec/remote.py b/lib/cdist/exec/remote.py index 9610290b..c876adf1 100644 --- a/lib/cdist/exec/remote.py +++ b/lib/cdist/exec/remote.py @@ -40,6 +40,13 @@ class RemoteScriptError(cdist.Error): def __str__(self): return "Remote script execution failed: %s %s" % (self.script, self.command) +class DecodeError(cdist.Error): + def __init__(self, command): + self.command = command + + def __str__(self): + return "Cannot decode output of " + " ".join(self.command) + class Remote(object): """Execute commands remotely. @@ -121,6 +128,8 @@ class Remote(object): raise cdist.Error("Command failed: " + " ".join(command)) except OSError as error: raise cdist.Error(" ".join(*args) + ": " + error.args[1]) + except UnicodeDecodeError: + raise DecodeError(command) def run_script(self, script, env=None, return_output=False): """Run the given script with the given environment on the remote side. From 67b109471257b11dc3d5c2b6bc844d0bcc2bb2c8 Mon Sep 17 00:00:00 2001 From: Nico Schottelius Date: Sat, 15 Oct 2011 01:25:57 +0200 Subject: [PATCH 2/8] flatten error message in remotescripterror as well Signed-off-by: Nico Schottelius --- lib/cdist/exec/remote.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/cdist/exec/remote.py b/lib/cdist/exec/remote.py index c876adf1..943b8992 100644 --- a/lib/cdist/exec/remote.py +++ b/lib/cdist/exec/remote.py @@ -38,7 +38,8 @@ class RemoteScriptError(cdist.Error): self.script_content = script_content def __str__(self): - return "Remote script execution failed: %s %s" % (self.script, self.command) + plain_command = " ".join(self.command) + return "Remote script execution failed: %s" % plain_command class DecodeError(cdist.Error): def __init__(self, command): From a415cc4b91e531ac6eb5a53c578403fd878d480c Mon Sep 17 00:00:00 2001 From: Nico Schottelius Date: Sat, 15 Oct 2011 01:38:22 +0200 Subject: [PATCH 3/8] add correct logger in manifest Signed-off-by: Nico Schottelius --- lib/cdist/core/manifest.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/cdist/core/manifest.py b/lib/cdist/core/manifest.py index 8f9903a5..50964fdc 100644 --- a/lib/cdist/core/manifest.py +++ b/lib/cdist/core/manifest.py @@ -25,9 +25,6 @@ import os import cdist -log = logging.getLogger(__name__) - - ''' common: runs only locally, does not need remote @@ -68,13 +65,16 @@ class Manifest(object): def __init__(self, target_host, local): self.target_host = target_host self.local = local + + self.log = logging.getLogger(self.target_host) + self.env = { 'PATH': "%s:%s" % (self.local.bin_path, os.environ['PATH']), '__target_host': self.target_host, '__global': self.local.out_path, '__cdist_type_base_path': self.local.type_path, # for use in type emulator } - if log.getEffectiveLevel() == logging.DEBUG: + if self.log.getEffectiveLevel() == logging.DEBUG: self.env.update({'__debug': "yes" }) @@ -83,7 +83,7 @@ class Manifest(object): env.update(self.env) env['__manifest'] = self.local.manifest_path env['__cdist_manifest'] = script - log.info("Running initial manifest " + self.local.manifest_path) + self.log.info("Running initial manifest " + self.local.manifest_path) self.local.run_script(script, env=env) def run_type_manifest(self, cdist_object): From 00a1f1eeb9b7c718eb451e366d7d42a8ecc9c6c0 Mon Sep 17 00:00:00 2001 From: Nico Schottelius Date: Sat, 15 Oct 2011 01:41:11 +0200 Subject: [PATCH 4/8] ++ more host prefixing log code Signed-off-by: Nico Schottelius --- lib/cdist/config_install.py | 4 ++-- lib/cdist/core/explorer.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/cdist/config_install.py b/lib/cdist/config_install.py index ce7c5736..a71998f7 100644 --- a/lib/cdist/config_install.py +++ b/lib/cdist/config_install.py @@ -70,8 +70,8 @@ class ConfigInstall(object): start_time = time.time() self.deploy_to() self.cleanup() - self.log.info("Finished run of %s in %s seconds", - self.context.target_host, time.time() - start_time) + self.log.info("Finished run in %s seconds", + time.time() - start_time) def stage_prepare(self): """Do everything for a deploy, minus the actual code stage""" diff --git a/lib/cdist/core/explorer.py b/lib/cdist/core/explorer.py index 90201072..b4f8531e 100644 --- a/lib/cdist/core/explorer.py +++ b/lib/cdist/core/explorer.py @@ -25,9 +25,6 @@ import os import cdist -log = logging.getLogger(__name__) - - ''' common: runs only remotely, needs local and remote to construct paths @@ -67,13 +64,16 @@ class Explorer(object): """ def __init__(self, target_host, local, remote): self.target_host = target_host + + self.log = logging.getLogger(target_host) + self.local = local self.remote = remote self.env = { '__target_host': self.target_host, '__explorer': self.remote.global_explorer_path, } - if log.getEffectiveLevel() == logging.DEBUG: + if self.log.getEffectiveLevel() == logging.DEBUG: self.env.update({'__debug': "yes" }) ### global @@ -106,7 +106,7 @@ class Explorer(object): """Transfer the type explorers for the given type to the remote side.""" if cdist_type.explorers: if cdist_type.explorers_transferred: - log.debug("Skipping retransfer of type explorers for: %s", cdist_type) + self.log.debug("Skipping retransfer of type explorers for: %s", cdist_type) else: source = os.path.join(self.local.type_path, cdist_type.explorer_path) destination = os.path.join(self.remote.type_path, cdist_type.explorer_path) From e002710c4c8170a8a5e144b3bf559b3ba8eb4a91 Mon Sep 17 00:00:00 2001 From: Nico Schottelius Date: Sat, 15 Oct 2011 02:31:40 +0200 Subject: [PATCH 5/8] rewrite emulator to class based approach for better logging Signed-off-by: Nico Schottelius --- bin/cdist | 7 +- lib/cdist/emulator.py | 191 ++++++++++++++++++++++++------------------ 2 files changed, 116 insertions(+), 82 deletions(-) diff --git a/bin/cdist b/bin/cdist index 88cdf399..4d93842e 100755 --- a/bin/cdist +++ b/bin/cdist @@ -145,6 +145,10 @@ def configinstall(args, mode): log.info("Total processing time for %s host(s): %s", len(args.host), (time_end - time_start)) +def emulator(): + """Prepare and run emulator""" + emulator = cdist.emulator.Emulator(sys.argv) + emulator.run() if __name__ == "__main__": try: @@ -152,7 +156,8 @@ if __name__ == "__main__": if re.match(TYPE_PREFIX, os.path.basename(sys.argv[0])): import cdist.emulator - cdist.emulator.run(sys.argv) + + emulator() else: import cdist.banner import cdist.config diff --git a/lib/cdist/emulator.py b/lib/cdist/emulator.py index 0b303b23..edf7ab1c 100644 --- a/lib/cdist/emulator.py +++ b/lib/cdist/emulator.py @@ -26,8 +26,6 @@ import os import cdist from cdist import core -log = logging.getLogger(__name__) - class IllegalRequirementError(cdist.Error): def __init__(self, requirement, message=None): @@ -37,100 +35,131 @@ class IllegalRequirementError(cdist.Error): def __str__(self): return '%s: %s' % (self.message, self.requirement) +class Emulator(object): + def __init__(self, argv): + self.argv = argv + self.object_id = False -def run(argv): - """Emulate type commands (i.e. __file and co)""" - global_path = os.environ['__global'] - object_source = os.environ['__cdist_manifest'] - target_host = os.environ['__target_host'] - type_name = os.path.basename(argv[0]) + self.global_path = os.environ['__global'] + self.object_source = os.environ['__cdist_manifest'] + self.target_host = os.environ['__target_host'] + self.type_base_path = os.environ['__cdist_type_base_path'] + + self.object_base_path = os.path.join(self.global_path, "object") - # Logsetup - FIXME: add object_fq as soon as setup! - #id = target_host + ": " + cdist_type + '/' + object_id - id = target_host + ": " - # logformat = '%(levelname)s: ' + target_host + ": " + cdist_type + '/' + object_id + ': %(message)s' - logformat = '%(levelname)s: ' + id + ': %(message)s' - logging.basicConfig(format=logformat) + self.type_name = os.path.basename(argv[0]) + self.cdist_type = core.Type(self.type_base_path, self.type_name) - if '__debug' in os.environ: - logging.root.setLevel(logging.DEBUG) - else: - logging.root.setLevel(logging.INFO) + self.__init_log() - object_base_path = os.path.join(global_path, "object") - type_base_path = os.environ['__cdist_type_base_path'] - cdist_type = core.Type(type_base_path, type_name) + def filter(self, record): + """Add hostname and object to logs via logging Filter""" - if '__install' in os.environ: - if not cdist_type.is_install: - log.debug("Running in install mode, ignoring non install type") - return True + prefix = self.target_host + ": " - parser = argparse.ArgumentParser(add_help=False) + if self.object_id: + prefix = prefix + self.type_name + "/" + self.object_id - for parameter in cdist_type.optional_parameters: - argument = "--" + parameter - parser.add_argument(argument, action='store', required=False) - for parameter in cdist_type.required_parameters: - argument = "--" + parameter - parser.add_argument(argument, action='store', required=True) + record.msg = prefix + ": " + record.msg - # If not singleton support one positional parameter - if not cdist_type.is_singleton: - parser.add_argument("object_id", nargs=1) + return True - # And finally verify parameter - args = parser.parse_args(argv[1:]) + def run(self): + """Emulate type commands (i.e. __file and co)""" - # Setup object_id - if cdist_type.is_singleton: - object_id = "singleton" - else: - object_id = args.object_id[0] - del args.object_id + if '__install' in os.environ: + if not self.cdist_type.is_install: + self.log.debug("Running in install mode, ignoring non install type") + return True - # strip leading slash from object_id - object_id = object_id.lstrip('/') + self.commandline() + self.setup_object() + self.record_requirements() + self.log.debug("Finished %s %s" % (self.cdist_object.path, self.parameters)) - # Instantiate the cdist object whe are defining - cdist_object = core.Object(cdist_type, object_base_path, object_id) + def __init_log(self): + """Setup logging facility""" + logformat = '%(levelname)s: %(message)s' + logging.basicConfig(format=logformat) - # FIXME: verify object id - log.debug('#### emulator args: %s' % args) + if '__debug' in os.environ: + logging.root.setLevel(logging.DEBUG) + else: + logging.root.setLevel(logging.INFO) - # Create object with given parameters - parameters = {} - for key,value in vars(args).items(): - if value is not None: - parameters[key] = value - - if cdist_object.exists: - if cdist_object.parameters != parameters: - raise cdist.Error("Object %s already exists with conflicting parameters:\n%s: %s\n%s: %s" - % (cdist_object, " ".join(cdist_object.source), cdist_object.parameters, object_source, parameters) + self.log = logging.getLogger(__name__) + self.log.addFilter(self) + + def commandline(self): + """Parse command line""" + + parser = argparse.ArgumentParser(add_help=False) + + for parameter in self.cdist_type.optional_parameters: + argument = "--" + parameter + parser.add_argument(argument, action='store', required=False) + for parameter in self.cdist_type.required_parameters: + argument = "--" + parameter + parser.add_argument(argument, action='store', required=True) + + # If not singleton support one positional parameter + if not self.cdist_type.is_singleton: + parser.add_argument("object_id", nargs=1) + + # And finally parse/verify parameter + self.args = parser.parse_args(self.argv[1:]) + self.log.debug('Emulator args: %s' % self.args) + + + def setup_object(self): + # FIXME: verify object id + + # Setup object_id + if self.cdist_type.is_singleton: + self.object_id = "singleton" + else: + self.object_id = self.args.object_id[0] + del self.args.object_id + + # strip leading slash from object_id + self.object_id = self.object_id.lstrip('/') + + # Instantiate the cdist object we are defining + self.cdist_object = core.Object(self.cdist_type, self.object_base_path, self.object_id) + + # Create object with given parameters + self.parameters = {} + for key,value in vars(self.args).items(): + if value is not None: + self.parameters[key] = value + + if self.cdist_object.exists: + if cdist_object.parameters != self.parameters: + raise cdist.Error("Object %s already exists with conflicting parameters:\n%s: %s\n%s: %s" + % (self.cdist_object, " ".join(self.cdist_object.source), self.cdist_object.parameters, self.object_source, self.parameters) ) - else: - cdist_object.create() - cdist_object.parameters = parameters + else: + self.cdist_object.create() + self.cdist_object.parameters = self.parameters - # Record requirements - if "require" in os.environ: - requirements = os.environ['require'] - for requirement in requirements.split(" "): - requirement_parts = requirement.split(os.sep, 1) - requirement_parts.reverse() - requirement_type_name = requirement_parts.pop() - try: - requirement_object_id = requirement_parts.pop() - except IndexError: - # no object id, must be singleton - requirement_object_id = 'singleton' - if requirement_object_id.startswith('/'): - raise IllegalRequirementError(requirement, 'requirements object_id may not start with /') - log.debug("Recording requirement: %s -> %s" % (cdist_object.path, requirement)) - cdist_object.requirements.append(requirement) + def record_requirements(self): + """record requirements""" - # Record / Append source - cdist_object.source.append(object_source) + if "require" in os.environ: + requirements = os.environ['require'] + for requirement in requirements.split(" "): + requirement_parts = requirement.split(os.sep, 1) + requirement_parts.reverse() + requirement_type_name = requirement_parts.pop() + try: + requirement_object_id = requirement_parts.pop() + except IndexError: + # no object id, must be singleton + requirement_object_id = 'singleton' + if requirement_object_id.startswith('/'): + raise IllegalRequirementError(requirement, 'requirements object_id may not start with /') + self.log.debug("Recording requirement: %s -> %s" % (self.cdist_object.path, requirement)) + self.cdist_object.requirements.append(requirement) - log.debug("Finished %s %s" % (cdist_object.path, parameters)) + # Record / Append source + self.cdist_object.source.append(self.object_source) From a7a3ee6f1908575dbe5d06b9ced6432acd7ffdea Mon Sep 17 00:00:00 2001 From: Nico Schottelius Date: Sat, 15 Oct 2011 02:36:33 +0200 Subject: [PATCH 6/8] adjust prefix Signed-off-by: Nico Schottelius --- lib/cdist/emulator.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cdist/emulator.py b/lib/cdist/emulator.py index edf7ab1c..a5e9d306 100644 --- a/lib/cdist/emulator.py +++ b/lib/cdist/emulator.py @@ -55,10 +55,10 @@ class Emulator(object): def filter(self, record): """Add hostname and object to logs via logging Filter""" - prefix = self.target_host + ": " + prefix = self.target_host + ": (emulator)" if self.object_id: - prefix = prefix + self.type_name + "/" + self.object_id + prefix = prefix + " " + self.type_name + "/" + self.object_id record.msg = prefix + ": " + record.msg @@ -108,7 +108,7 @@ class Emulator(object): # And finally parse/verify parameter self.args = parser.parse_args(self.argv[1:]) - self.log.debug('Emulator args: %s' % self.args) + self.log.debug('Args: %s' % self.args) def setup_object(self): From 0dd38f75c28a8745259b1086a5e46e7d040edfc5 Mon Sep 17 00:00:00 2001 From: Nico Schottelius Date: Sat, 15 Oct 2011 02:46:28 +0200 Subject: [PATCH 7/8] remove leading / from object_id of requirement Signed-off-by: Nico Schottelius --- lib/cdist/emulator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cdist/emulator.py b/lib/cdist/emulator.py index a5e9d306..3aa8ab14 100644 --- a/lib/cdist/emulator.py +++ b/lib/cdist/emulator.py @@ -156,8 +156,8 @@ class Emulator(object): except IndexError: # no object id, must be singleton requirement_object_id = 'singleton' - if requirement_object_id.startswith('/'): - raise IllegalRequirementError(requirement, 'requirements object_id may not start with /') + + requirement_object_id = requirement_object_id.lstrip('/') self.log.debug("Recording requirement: %s -> %s" % (self.cdist_object.path, requirement)) self.cdist_object.requirements.append(requirement) From ee1c568c7bf832806e8201f1b0a15c4a7ce7eb52 Mon Sep 17 00:00:00 2001 From: Nico Schottelius Date: Sat, 15 Oct 2011 02:54:54 +0200 Subject: [PATCH 8/8] almost finish correct requirement loading, but need sleep Signed-off-by: Nico Schottelius --- lib/cdist/emulator.py | 7 ++- other/tests_reintegrate/test_exec.py | 79 ---------------------------- 2 files changed, 5 insertions(+), 81 deletions(-) delete mode 100644 other/tests_reintegrate/test_exec.py diff --git a/lib/cdist/emulator.py b/lib/cdist/emulator.py index 3aa8ab14..70f491ac 100644 --- a/lib/cdist/emulator.py +++ b/lib/cdist/emulator.py @@ -148,8 +148,12 @@ class Emulator(object): if "require" in os.environ: requirements = os.environ['require'] for requirement in requirements.split(" "): + self.log.debug("Recording requirement: " + requirement) requirement_parts = requirement.split(os.sep, 1) - requirement_parts.reverse() + # FIXME: continue here + FAILHERE,PLEASE()[]! + print(requirement) + print(requirement_parts) requirement_type_name = requirement_parts.pop() try: requirement_object_id = requirement_parts.pop() @@ -158,7 +162,6 @@ class Emulator(object): requirement_object_id = 'singleton' requirement_object_id = requirement_object_id.lstrip('/') - self.log.debug("Recording requirement: %s -> %s" % (self.cdist_object.path, requirement)) self.cdist_object.requirements.append(requirement) # Record / Append source diff --git a/other/tests_reintegrate/test_exec.py b/other/tests_reintegrate/test_exec.py deleted file mode 100644 index c2bb52f6..00000000 --- a/other/tests_reintegrate/test_exec.py +++ /dev/null @@ -1,79 +0,0 @@ -#!/usr/bin/env python3 -# -*- coding: utf-8 -*- -# -# 2011 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 tempfile -import unittest - -import cdist.exec - - -class ExecTestCase(unittest.TestCase): - def setUp(self): - """Create shell code and co.""" - - self.temp_dir = tempfile.mkdtemp() - self.shell_false = os.path.join(self.temp_dir, "shell_false") - self.shell_true = os.path.join(self.temp_dir, "shell_true") - - true_fd = open(self.shell_true, "w") - true_fd.writelines(["#!/bin/sh\n", "/bin/true"]) - true_fd.close() - - false_fd = open(self.shell_false, "w") - false_fd.writelines(["#!/bin/sh\n", "/bin/false"]) - false_fd.close() - - target_host = "does.not.exist" - remote_exec = "ssh -o User=root -q" - remote_copy = "scp -o User=root -q" - self.wrapper = cdist.exec.Wrapper(target_host, remote_exec, remote_copy) - - def tearDown(self): - shutil.rmtree(self.temp_dir) - - def test_local_success_shell(self): - try: - self.wrapper.shell_run_or_debug_fail(self.shell_true, [self.shell_true]) - except cdist.Error: - failed = True - else: - failed = False - self.assertFalse(failed) - - def test_local_fail_shell(self): - self.assertRaises(cdist.Error, self.wrapper.shell_run_or_debug_fail, - self.shell_false, [self.shell_false]) - - def test_local_success(self): - try: - self.wrapper.run_or_fail(["/bin/true"]) - except cdist.Error: - failed = True - else: - failed = False - self.assertFalse(failed) - - def test_local_fail(self): - self.assertRaises(cdist.Error, self.wrapper.run_or_fail, ["/bin/false"])