logger refactoring of slapgrid

slapos/grid/SlapObject.py

@@ -28,7 +28,6 @@
 #
 ##############################################################################
 
-import logging
 import os
 import pkg_resources
 import pwd
@@ -58,7 +57,7 @@ REQUIRED_COMPUTER_PARTITION_PERMISSION = 0o750
 class Software(object):
   """This class is responsible for installing a software release"""
 
-  def __init__(self, url, software_root, buildout,
+  def __init__(self, url, software_root, buildout, logger,
                signature_private_key_file=None, signature_certificate_list=None,
                upload_cache_url=None, upload_dir_url=None, shacache_cert_file=None,
                shacache_key_file=None, shadir_cert_file=None, shadir_key_file=None,
@@ -75,7 +74,7 @@ class Software(object):
     self.software_path = os.path.join(self.software_root,
                                       self.software_url_hash)
     self.buildout = buildout
-    self.logger = logging.getLogger('BuildoutManager')
+    self.logger = logger
     self.signature_private_key_file = signature_private_key_file
     self.signature_certificate_list = signature_certificate_list
     self.upload_cache_url = upload_cache_url
@@ -139,7 +138,8 @@ class Software(object):
     it. If it fails, we notify the server.
     """
     root_stat_info = os.stat(self.software_root)
-    os.environ = getCleanEnvironment(pwd.getpwuid(root_stat_info.st_uid).pw_dir)
+    os.environ = getCleanEnvironment(logger=self.logger,
+                                     home_path=pwd.getpwuid(root_stat_info.st_uid).pw_dir)
     if not os.path.isdir(self.software_path):
       os.mkdir(self.software_path)
     extends_cache = tempfile.mkdtemp()
@@ -177,10 +177,13 @@ class Software(object):
       self.createProfileIfMissing(buildout_cfg, self.url)
 
       buildout_parameter_list.extend(['-c', buildout_cfg])
-      utils.bootstrapBuildout(self.software_path, self.buildout,
-          additional_buildout_parametr_list=buildout_parameter_list)
-      utils.launchBuildout(self.software_path,
-                           os.path.join(self.software_path, 'bin', 'buildout'),
+      utils.bootstrapBuildout(path=self.software_path,
+                              buildout=self.buildout,
+                              logger=self.logger,
+                              additional_buildout_parametr_list=buildout_parameter_list)
+      utils.launchBuildout(path=self.software_path,
+                           buildout_binary=os.path.join(self.software_path, 'bin', 'buildout'),
+                           logger=self.logger,
                            additional_buildout_parametr_list=buildout_parameter_list)
     finally:
       shutil.rmtree(extends_cache)
@@ -256,10 +259,12 @@ class Partition(object):
                server_url,
                software_release_url,
                buildout,
+               logger,
                certificate_repository_path=None,
                ):
     """Initialisation of class parameters"""
     self.buildout = buildout
+    self.logger = logger
     self.software_path = software_path
     self.instance_path = instance_path
     self.run_path = os.path.join(self.instance_path, 'etc', 'run')
@@ -268,7 +273,6 @@ class Partition(object):
         supervisord_partition_configuration_path
     self.supervisord_socket = supervisord_socket
     self.computer_partition = computer_partition
-    self.logger = logging.getLogger('Partition')
     self.computer_id = computer_id
     self.partition_id = partition_id
     self.server_url = server_url
@@ -356,8 +360,8 @@ class Partition(object):
                                  'permissions are: 0%o, wanted are 0%o' %
                                  (self.instance_path, permission,
                                   REQUIRED_COMPUTER_PARTITION_PERMISSION))
-    os.environ = getCleanEnvironment(pwd.getpwuid(
-        instance_stat_info.st_uid).pw_dir)
+    os.environ = getCleanEnvironment(logger=self.logger,
+                                     home_path=pwd.getpwuid(instance_stat_info.st_uid).pw_dir)
     # Generates buildout part from template
     template_location = os.path.join(self.software_path, 'instance.cfg')
     # Backward compatibility: "instance.cfg" file was named "template.cfg".
@@ -417,11 +421,13 @@ class Partition(object):
       self.logger.debug('Invoking %r in %r' % (' '.join(invocation_list),
         self.instance_path))
       process_handler = SlapPopen(invocation_list,
-                                  preexec_fn=lambda: dropPrivileges(uid, gid),
+                                  preexec_fn=lambda: dropPrivileges(uid, gid, logger=self.logger),
                                   cwd=self.instance_path,
-                                  env=getCleanEnvironment(pwd.getpwuid(uid).pw_dir),
+                                  env=getCleanEnvironment(logger=self.logger,
+                                                          home_path=pwd.getpwuid(uid).pw_dir),
                                   stdout=subprocess.PIPE,
-                                  stderr=subprocess.STDOUT)
+                                  stderr=subprocess.STDOUT,
+                                  logger=self.logger)
       if process_handler.returncode is None or process_handler.returncode != 0:
         message = 'Failed to bootstrap buildout in %r.' % (self.instance_path)
         self.logger.error(message)
@@ -430,11 +436,17 @@ class Partition(object):
 
     if not os.path.exists(buildout_binary):
       # use own buildout generation
-      utils.bootstrapBuildout(self.instance_path, self.buildout,
-        ['buildout:bin-directory=%s' % os.path.join(self.instance_path, 'sbin')])
+      utils.bootstrapBuildout(path=self.instance_path,
+                              buildout=self.buildout,
+                              logger=self.logger,
+                              additional_buildout_parameter_list=
+                                ['buildout:bin-directory=%s' %
+                                    os.path.join(self.instance_path, 'sbin')])
       buildout_binary = os.path.join(self.instance_path, 'sbin', 'buildout')
     # Launches buildout
-    utils.launchBuildout(self.instance_path, buildout_binary)
+    utils.launchBuildout(path=self.instance_path,
+                         buildout_binary=buildout_binary,
+                         logger=self.logger)
     # Generates supervisord configuration file from template
     self.logger.info("Generating supervisord config file from template...")
     # check if CP/etc/run exists and it is a directory
@@ -510,11 +522,13 @@ class Partition(object):
       uid, gid = self.getUserGroupId()
       self.logger.debug('Invoking %r' % destroy_executable_location)
       process_handler = SlapPopen([destroy_executable_location],
-                                  preexec_fn=lambda: dropPrivileges(uid, gid),
+                                  preexec_fn=lambda: dropPrivileges(uid, gid, logger=self.logger),
                                   cwd=self.instance_path,
-                                  env=getCleanEnvironment(pwd.getpwuid(uid).pw_dir),
+                                  env=getCleanEnvironment(logger=self.logger,
+                                                          home_path=pwd.getpwuid(uid).pw_dir),
                                   stdout=subprocess.PIPE,
-                                  stderr=subprocess.STDOUT)
+                                  stderr=subprocess.STDOUT,
+                                  logger=self.logger)
       if process_handler.returncode is None or process_handler.returncode != 0:
         message = 'Failed to destroy Computer Partition in %r.' % \
             self.instance_path

slapos/grid/svcbackend.py

@@ -46,13 +46,8 @@ def getSupervisorRPC(socket):
       supervisor_transport)
   return getattr(server_proxy, 'supervisor')
 
-class dummylogger(object):
-    def info(self, *args):
-        print args
-    debug = info
 
 def launchSupervisord(socket, configuration_file, logger):
-  #logger = dummylogger()
   if os.path.exists(socket):
     trynum = 1
     while trynum < 6:
@@ -92,7 +87,8 @@ def launchSupervisord(socket, configuration_file, logger):
                                 env={},
                                 executable=sys.executable,
                                 stdout=subprocess.PIPE,
-                                stderr=subprocess.STDOUT)
+                                stderr=subprocess.STDOUT,
+                                logger=logger)
 
   result = supervisord_popen.communicate()[0]
   if supervisord_popen.returncode:

slapos/grid/utils.py

@@ -30,7 +30,6 @@
 
 import grp
 import hashlib
-import logging
 import os
 import pkg_resources
 import pwd
@@ -95,6 +94,7 @@ class SlapPopen(subprocess.Popen):
   log.
   """
   def __init__(self, *args, **kwargs):
+    logger = kwargs.pop('logger')
     kwargs.update(stdin=subprocess.PIPE)
     if sys.platform == 'cygwin' and kwargs.get('env') == {}:
       kwargs['env'] = None
@@ -103,7 +103,6 @@ class SlapPopen(subprocess.Popen):
     self.stdin.close()
     self.stdin = None
 
-    logger = logging.getLogger('SlapProcessManager')
     # XXX-Cedric: this algorithm looks overkill for simple logging.
     output_lines = []
     while True:
@@ -119,8 +118,7 @@ def md5digest(url):
   return hashlib.md5(url).hexdigest()
 
 
-def getCleanEnvironment(home_path='/tmp'):
-  logger = logging.getLogger('CleanEnvironment')
+def getCleanEnvironment(logger, home_path='/tmp'):
   changed_env = {}
   removed_env = []
   env = os.environ.copy()
@@ -137,12 +135,11 @@ def getCleanEnvironment(home_path='/tmp'):
   return env
 
 
-def setRunning(pid_file):
+def setRunning(logger, pidfile):
   """Creates a pidfile. If a pidfile already exists, we exit"""
-  logger = logging.getLogger('Slapgrid')
-  if os.path.exists(pid_file):
+  if os.path.exists(pidfile):
     try:
-      pid = int(open(pid_file, 'r').readline())
+      pid = int(open(pidfile, 'r').readline())
     except ValueError:
       pid = None
     # XXX This could use psutil library.
@@ -150,29 +147,28 @@ def setRunning(pid_file):
       logger.info('New slapos process started, but another slapos '
                   'process is aleady running with pid %s, exiting.' % pid)
       sys.exit(10)
-    logger.info('Existing pid file %r was stale, overwritten' % pid_file)
+    logger.info('Existing pid file %r was stale, overwritten' % pidfile)
   # Start new process
-  write_pid(pid_file)
+  write_pid(logger, pidfile)
 
 
-def setFinished(pid_file):
+def setFinished(pidfile):
   try:
-    os.remove(pid_file)
+    os.remove(pidfile)
   except OSError:
     pass
 
 
-def write_pid(pid_file):
-  logger = logging.getLogger('Slapgrid')
+def write_pid(logger, pidfile):
   try:
-    with open(pid_file, 'w') as fout:
+    with open(pidfile, 'w') as fout:
       fout.write('%s' % os.getpid())
   except (IOError, OSError):
-    logger.critical('slapgrid could not write pidfile %s' % pid_file)
+    logger.critical('slapgrid could not write pidfile %s' % pidfile)
     raise
 
 
-def dropPrivileges(uid, gid):
+def dropPrivileges(uid, gid, logger):
   """Drop privileges to uid, gid if current uid is 0
 
   Do tests to check if dropping was successful and that no system call is able
@@ -180,7 +176,6 @@ def dropPrivileges(uid, gid):
 
   Does nothing in case if uid and gid are not 0
   """
-  logger = logging.getLogger('dropPrivileges')
   # XXX-Cedric: remove format / just do a print, otherwise formatting is done
   # twice
   current_uid, current_gid = os.getuid(), os.getgid()
@@ -232,11 +227,10 @@ def dropPrivileges(uid, gid):
   logger.debug('Succesfully dropped privileges to uid=%r gid=%r' % (uid, gid))
 
 
-def bootstrapBuildout(path, buildout=None,
-    additional_buildout_parametr_list=None):
+def bootstrapBuildout(path, logger, buildout=None,
+                      additional_buildout_parametr_list=None):
   if additional_buildout_parametr_list is None:
     additional_buildout_parametr_list = []
-  logger = logging.getLogger('BuildoutManager')
   # Reads uid/gid of path, launches buildout with thoses privileges
   stat_info = os.stat(path)
   uid = stat_info.st_uid
@@ -270,10 +264,11 @@ def bootstrapBuildout(path, buildout=None,
     logger.debug('Invoking: %r in directory %r' % (' '.join(invocation_list),
       path))
     process_handler = SlapPopen(invocation_list,
-                                preexec_fn=lambda: dropPrivileges(uid, gid),
+                                preexec_fn=lambda: dropPrivileges(uid, gid, logger=logger),
                                 cwd=path,
                                 stdout=subprocess.PIPE,
-                                stderr=subprocess.STDOUT)
+                                stderr=subprocess.STDOUT,
+                                logger=logger)
     if process_handler.returncode is None or process_handler.returncode != 0:
       message = 'Failed to run buildout profile in directory %r' % (path)
       logger.error(message)
@@ -285,10 +280,9 @@ def bootstrapBuildout(path, buildout=None,
     logger.debug('Restore umask from %03o to %03o' % (old_umask, umask))
 
 
-def launchBuildout(path, buildout_binary,
+def launchBuildout(path, buildout_binary, logger,
                    additional_buildout_parametr_list=None):
   """ Launches buildout."""
-  logger = logging.getLogger('BuildoutManager')
   if additional_buildout_parametr_list is None:
     additional_buildout_parametr_list = []
   # Reads uid/gid of path, launches buildout with thoses privileges
@@ -311,11 +305,13 @@ def launchBuildout(path, buildout_binary,
     logger.debug('Invoking: %r in directory %r' % (' '.join(invocation_list),
       path))
     process_handler = SlapPopen(invocation_list,
-                                preexec_fn=lambda: dropPrivileges(uid, gid),
+                                preexec_fn=lambda: dropPrivileges(uid, gid, logger=logger),
                                 cwd=path,
-                                env=getCleanEnvironment(pwd.getpwuid(uid).pw_dir),
+                                env=getCleanEnvironment(logger=logger,
+                                                        home_path=pwd.getpwuid(uid).pw_dir),
                                 stdout=subprocess.PIPE,
-                                stderr=subprocess.STDOUT)
+                                stderr=subprocess.STDOUT,
+                                logger=logger)
     if process_handler.returncode is None or process_handler.returncode != 0:
       message = 'Failed to run buildout profile in directory %r' % (path)
       logger.error(message)

slapos/tests/slapgrid.py

@@ -117,10 +117,15 @@ class BasicMixin:
       'supervisord')
     self.usage_report_periodicity = 1
     self.buildout = None
-    self.grid = slapgrid.Slapgrid(self.software_root, self.instance_root,
-      self.master_url, self.computer_id, self.supervisord_socket,
-      self.supervisord_configuration_path,
-      self.buildout, develop=develop)
+    self.grid = slapgrid.Slapgrid(self.software_root,
+                                  self.instance_root,
+                                  self.master_url,
+                                  self.computer_id,
+                                  self.supervisord_socket,
+                                  self.supervisord_configuration_path,
+                                  self.buildout,
+                                  develop=develop,
+                                  logger=logging.getLogger())
     # monkey patch buildout bootstrap
     def dummy(*args, **kw):
       pass
@@ -1826,7 +1831,7 @@ exit 127""" % {'worked_file': worked_file})
                      slapos.grid.slapgrid.SLAPGRID_PROMISE_FAIL)
     self.assertTrue(os.path.isfile(worked_file))
 
-    self.assertEqual(instance.error_log, 'Error')
+    self.assertEqual(instance.error_log.split('\n')[-1], 'Error')
     self.assertTrue(instance.error)
     self.assertIsNone(instance.state)
 

slapos/tests/slapobject.py

@@ -25,12 +25,14 @@
 #
 ##############################################################################
 
+import logging
+import os
+import unittest
+
 from slapos.grid import SlapObject
 from slapos.grid import utils
 from slapos.grid import networkcache
 from slapos.tests.slapgrid import BasicMixin
-import os
-import unittest
 
 
 class FakeCallAndRead:
@@ -93,6 +95,7 @@ class TestSoftwareSlapObject(BasicMixin, unittest.TestCase):
             url='http://example.com/software.cfg',
             software_root=self.software_root,
             buildout=self.buildout,
+            logger=logging.getLogger(),
             signature_private_key_file='/signature/private/key_file',
             upload_cache_url='http://example.com/uploadcache',
             upload_dir_url='http://example.com/uploaddir',
@@ -129,7 +132,8 @@ class TestSoftwareSlapObject(BasicMixin, unittest.TestCase):
     software = SlapObject.Software(
             url='http://example.com/software.cfg',
             software_root=self.software_root,
-            buildout=self.buildout)
+            buildout=self.buildout,
+            logger=logging.getLogger())
 
     software.install()
 
@@ -161,6 +165,7 @@ class TestSoftwareSlapObject(BasicMixin, unittest.TestCase):
             url='http://example.com/software.cfg',
             software_root=self.software_root,
             buildout=self.buildout,
+            logger=logging.getLogger(),
             signature_private_key_file='/signature/private/key_file',
             upload_cache_url='http://example.com/uploadcache',
             upload_dir_url='http://example.com/uploaddir',
@@ -192,6 +197,7 @@ class TestSoftwareSlapObject(BasicMixin, unittest.TestCase):
             url='http://example.com/software.cfg',
             software_root=self.software_root,
             buildout=self.buildout,
+            logger=logging.getLogger(),
             signature_private_key_file='/signature/private/key_file',
             upload_cache_url='http://example.com/uploadcache',
             upload_dir_url='http://example.com/uploaddir',