Merge remote-tracking branch 'steven/master'

doc/dev/todo/steven

@@ -10,6 +10,22 @@ logging:
          ...
       then one could filter e.g. on explorer.*
 
+   - more granular debug output,
+   [2] http://blog.ooz.ie/2011/03/python-logging-extending-standard.html
+
+
+exec local & remote:
+   - don't capture output by default
+   - add new mechanism to capture output explicitly
+
+
+config_install:
+   - move code for running global and type explorer run to cdist.core.explorer
+
+tests:
+   - aufraeumen
+   - test suite
+
 
 tests:
 

lib/cdist/context.py

@@ -41,6 +41,8 @@ class Context(object):
         debug=False):
 
         self.debug      = debug
+        if self.debug:
+            os.environ['__debug'] = 'yes'
 
         self.target_host    = target_host
 

lib/cdist/core/__init__.py

@@ -21,6 +21,7 @@
 
 from cdist.core.type import Type
 from cdist.core.object import Object
+from cdist.core.object import IllegalObjectIdError
 from cdist.core.explorer import Explorer
 from cdist.core.manifest import Manifest
 from cdist.core.code import Code

lib/cdist/core/code.py

@@ -104,7 +104,7 @@ class Code(object):
                 '__object_id': cdist_object.object_id,
                 '__object_fq': cdist_object.path,
             })
-            return self.local.run_script(script, env=env)
+            return self.local.run_script(script, env=env, return_output=True)
 
     def run_gencode_local(self, cdist_object):
         """Run the gencode-local script for the given cdist object."""

lib/cdist/core/explorer.py

@@ -88,7 +88,7 @@ class Explorer(object):
     def run_global_explorer(self, explorer):
         """Run the given global explorer and return it's output."""
         script = os.path.join(self.remote.global_explorer_path, explorer)
-        return self.remote.run_script(script, env=self.env)
+        return self.remote.run_script(script, env=self.env, return_output=True)
 
     ### type
 
@@ -126,4 +126,4 @@ class Explorer(object):
             '__type_explorer': os.path.join(self.remote.type_path, cdist_type.explorer_path)
         })
         script = os.path.join(self.remote.type_path, cdist_type.explorer_path, explorer)
-        return self.remote.run_script(script, env=env)
+        return self.remote.run_script(script, env=env, return_output=True)

lib/cdist/core/manifest.py

@@ -80,7 +80,7 @@ class Manifest(object):
         env.update(self.env)
         env['__manifest'] = self.local.manifest_path
         env['__cdist_manifest'] = script
-        return self.local.run_script(script, env=env)
+        self.local.run_script(script, env=env)
 
     def run_type_manifest(self, cdist_object):
         script = os.path.join(self.local.type_path, cdist_object.type.manifest_path)
@@ -94,4 +94,4 @@ class Manifest(object):
                 '__type': cdist_object.type.absolute_path,
                 '__cdist_manifest': script,
             })
-            return self.local.run_script(script, env=env)
+            self.local.run_script(script, env=env)

lib/cdist/core/object.py

@@ -33,6 +33,15 @@ log = logging.getLogger(__name__)
 DOT_CDIST = '.cdist'
 
 
+class IllegalObjectIdError(cdist.Error):
+    def __init__(self, object_id, message=None):
+        self.object_id = object_id
+        self.message = message or 'Illegal object id'
+
+    def __str__(self):
+        return '%s: %s' % (self.message, self.object_id)
+
+
 class Object(object):
     """Represents a cdist object.
 
@@ -79,6 +88,8 @@ class Object(object):
         return self.__class__(self.type.__class__(type_path, type_name), object_path, object_id=object_id)
 
     def __init__(self, cdist_type, base_path, object_id=None):
+        if object_id and object_id.startswith('/'):
+            raise IllegalObjectIdError(object_id, 'object_id may not start with /')
         self.type = cdist_type # instance of Type
         self.base_path = base_path
         self.object_id = object_id

lib/cdist/emulator.py

@@ -28,6 +28,16 @@ from cdist import core
 
 log = logging.getLogger(__name__)
 
+
+class IllegalRequirementError(cdist.Error):
+    def __init__(self, requirement, message=None):
+        self.requirement = requirement
+        self.message = message or 'Illegal requirement'
+
+    def __str__(self):
+        return '%s: %s' % (self.message, self.requirement)
+
+
 def run(argv):
     """Emulate type commands (i.e. __file and co)"""
     global_path = os.environ['__global']
@@ -106,8 +116,19 @@ def run(argv):
     # Record requirements
     if "require" in os.environ:
         requirements = os.environ['require']
-        log.debug("%s:Writing requirements: %s" % (cdist_object.path, requirements))
+        for requirement in requirements.split(" "):
-        cdist_object.requirements.extend(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)
 
     # Record / Append source
     cdist_object.source.append(object_source)

lib/cdist/exec/local.py

@@ -87,7 +87,7 @@ class Local(object):
         # FIXME: dont set mode here, fix unittest mkdtemp instead
         os.makedirs(path, mode=0o700, exist_ok=True)
 
-    def run(self, command, env=None):
+    def run(self, command, env=None, return_output=False):
         """Run the given command with the given environment.
         Return the output as a string.
 
@@ -95,13 +95,16 @@ class Local(object):
         assert isinstance(command, (list, tuple)), "list or tuple argument expected, got: %s" % command
         self.log.debug("Local run: %s", command)
         try:
-            return subprocess.check_output(command, env=env).decode()
+            if return_output:
+                return subprocess.check_output(command, env=env).decode()
+            else:
+                subprocess.check_call(command, env=env)
         except subprocess.CalledProcessError:
             raise cdist.Error("Command failed: " + " ".join(command))
         except OSError as error:
             raise cdist.Error(" ".join(*args) + ": " + error.args[1])
 
-    def run_script(self, script, env=None):
+    def run_script(self, script, env=None, return_output=False):
         """Run the given script with the given environment.
         Return the output as a string.
 
@@ -114,7 +117,10 @@ class Local(object):
             self.log.debug("Local run script env: %s", env)
         
         try:
-            return subprocess.check_output(command, env=env).decode()
+            if return_output:
+                return subprocess.check_output(command, env=env).decode()
+            else:
+                subprocess.check_call(command, env=env)
         except subprocess.CalledProcessError as error:
             script_content = self.run(["cat", script])
             self.log.error("Code that raised the error:\n%s", script_content)

lib/cdist/exec/remote.py

@@ -85,7 +85,7 @@ class Remote(object):
         command.extend(["-r", source, self.target_host + ":" + destination])
         self.run_command(command)
 
-    def run(self, command, env=None):
+    def run(self, command, env=None, return_output=False):
         """Run the given command with the given environment on the remote side.
         Return the output as a string.
 
@@ -94,9 +94,9 @@ class Remote(object):
         cmd = self._exec.split()
         cmd.append(self.target_host)
         cmd.extend(command)
-        return self.run_command(cmd, env=env)
+        return self.run_command(cmd, env=env, return_output=return_output)
 
-    def run_command(self, command, env=None):
+    def run_command(self, command, env=None, return_output=False):
         """Run the given command with the given environment.
         Return the output as a string.
 
@@ -113,13 +113,16 @@ class Remote(object):
 
         self.log.debug("Remote run: %s", command)
         try:
-            return subprocess.check_output(cmd).decode()
+            if return_output:
+                return subprocess.check_output(command).decode()
+            else:
+                subprocess.check_call(command)
         except subprocess.CalledProcessError:
             raise cdist.Error("Command failed: " + " ".join(command))
         except OSError as error:
             raise cdist.Error(" ".join(*args) + ": " + error.args[1])
 
-    def run_script(self, script, env=None):
+    def run_script(self, script, env=None, return_output=False):
         """Run the given script with the given environment on the remote side.
         Return the output as a string.
 
@@ -140,7 +143,10 @@ class Remote(object):
             self.log.debug("Remote run script env: %s", env)
         
         try:
-            return subprocess.check_output(command).decode()
+            if return_output:
+                return subprocess.check_output(command).decode()
+            else:
+                subprocess.check_call(command)
         except subprocess.CalledProcessError as error:
             script_content = self.run(["cat", script])
             self.log.error("Code that raised the error:\n%s", script_content)

lib/cdist/test/code/__init__.py

@@ -105,33 +105,11 @@ class CodeTestCase(unittest.TestCase):
         destination = os.path.join(self.remote.object_path, self.cdist_object.code_remote_path)
         self.assertTrue(os.path.isfile(destination))
 
-    def test_run_code_local_environment(self):
+    def test_run_code_local(self):
         self.cdist_object.code_local = self.code.run_gencode_local(self.cdist_object)
-        output_string = self.code.run_code_local(self.cdist_object)
+        self.code.run_code_local(self.cdist_object)
-        output_dict = {}
-        for line in output_string.split('\n'):
-            if line:
-                key,value = line.split(': ')
-                output_dict[key] = value
-        self.assertEqual(output_dict['__target_host'], self.local.target_host)
-        self.assertEqual(output_dict['__global'], self.local.out_path)
-        self.assertEqual(output_dict['__type'], self.cdist_type.absolute_path)
-        self.assertEqual(output_dict['__object'], self.cdist_object.absolute_path)
-        self.assertEqual(output_dict['__object_id'], self.cdist_object.object_id)
-        self.assertEqual(output_dict['__object_fq'], self.cdist_object.path)
         
     def test_run_code_remote_environment(self):
         self.cdist_object.code_remote = self.code.run_gencode_remote(self.cdist_object)
         self.code.transfer_code_remote(self.cdist_object)
-        output_string = self.code.run_code_remote(self.cdist_object)
+        self.code.run_code_remote(self.cdist_object)
-        output_dict = {}
-        for line in output_string.split('\n'):
-            if line:
-                key,value = line.split(': ')
-                output_dict[key] = value
-        self.assertEqual(output_dict['__target_host'], self.local.target_host)
-        self.assertEqual(output_dict['__global'], self.local.out_path)
-        self.assertEqual(output_dict['__type'], self.cdist_type.absolute_path)
-        self.assertEqual(output_dict['__object'], self.cdist_object.absolute_path)
-        self.assertEqual(output_dict['__object_id'], self.cdist_object.object_id)
-        self.assertEqual(output_dict['__object_fq'], self.cdist_object.path)

lib/cdist/test/exec/local.py

@@ -114,7 +114,7 @@ class LocalTestCase(unittest.TestCase):
         fd = open(script, "w")
         fd.writelines(["#!/bin/sh\n", "echo foobar"])
         fd.close()
-        self.assertEqual(self.local.run_script(script), "foobar\n")
+        self.assertEqual(self.local.run_script(script, return_output=True), "foobar\n")
 
     def test_mkdir(self):
         temp_dir = self.mkdtemp(dir=self.temp_dir)

lib/cdist/test/exec/remote.py

@@ -92,7 +92,7 @@ class RemoteTestCase(unittest.TestCase):
         fd = open(script, "w")
         fd.writelines(["#!/bin/sh\n", "echo foobar"])
         fd.close()
-        self.assertEqual(self.remote.run_script(script), "foobar\n")
+        self.assertEqual(self.remote.run_script(script, return_output=True), "foobar\n")
 
     def test_mkdir(self):
         temp_dir = self.mkdtemp(dir=self.temp_dir)

lib/cdist/test/manifest/__init__.py

@@ -63,33 +63,9 @@ class ManifestTestCase(unittest.TestCase):
 
     def test_initial_manifest_environment(self):
         initial_manifest = os.path.join(self.local.manifest_path, "dump_environment")
-        output_string = self.manifest.run_initial_manifest(initial_manifest)
+        self.manifest.run_initial_manifest(initial_manifest)
-        output_dict = {}
-        for line in output_string.split('\n'):
-            if line:
-                key,value = line.split(': ')
-                output_dict[key] = value
-        self.assertTrue(output_dict['PATH'].startswith(self.local.bin_path))
-        self.assertEqual(output_dict['__target_host'], self.local.target_host)
-        self.assertEqual(output_dict['__global'], self.local.out_path)
-        self.assertEqual(output_dict['__cdist_type_base_path'], self.local.type_path)
-        self.assertEqual(output_dict['__manifest'], self.local.manifest_path)
 
     def test_type_manifest_environment(self):
         cdist_type = core.Type(self.local.type_path, '__dump_environment')
         cdist_object = core.Object(cdist_type, self.local.object_path, 'whatever')
-        
+        self.manifest.run_type_manifest(cdist_object)
-        output_string = self.manifest.run_type_manifest(cdist_object)
-        output_dict = {}
-        for line in output_string.split('\n'):
-            if line:
-                key,value = line.split(': ')
-                output_dict[key] = value
-        self.assertTrue(output_dict['PATH'].startswith(self.local.bin_path))
-        self.assertEqual(output_dict['__target_host'], self.local.target_host)
-        self.assertEqual(output_dict['__global'], self.local.out_path)
-        self.assertEqual(output_dict['__cdist_type_base_path'], self.local.type_path)
-        self.assertEqual(output_dict['__type'], cdist_type.absolute_path)
-        self.assertEqual(output_dict['__object'], cdist_object.absolute_path)
-        self.assertEqual(output_dict['__object_id'], cdist_object.object_id)
-        self.assertEqual(output_dict['__object_fq'], cdist_object.path)

lib/cdist/test/object/__init__.py

@@ -52,6 +52,14 @@ class ObjectClassTestCase(unittest.TestCase):
         self.assertEqual(objects, objects_expected)
 
 
+class ObjectIdTestCase(unittest.TestCase):
+    def test_illegal_object_id(self):
+        cdist_type = core.Type(type_base_path, '__third')
+        illegal_object_id = '/object_id/may/not/start/with/slash'
+        with self.assertRaises(core.IllegalObjectIdError):
+            core.Object(cdist_type, object_base_path, illegal_object_id)
+
+
 class ObjectTestCase(unittest.TestCase):
 
     def setUp(self):