Merge lp:~ev/ubuntu-ci-services-itself/better-structure-and-logging-part2 into lp:ubuntu-ci-services-itself

[Evan]

WIP.

[lp:~ev/ubuntu-ci-services-itself/better-structure-and-logging-part2]

314. By Evan on 2014-03-09

Fix references to uid.

315. By Evan on 2014-03-09

Fix reference to user and group on config object.

316. By Evan on 2014-03-09

Fix tarball extraction code and set dir permissions appropriately.

317. By Evan on 2014-03-09

Create the config dir. Support upgrading the charm by reinstalling it. Log the error when the charm fails.

318. By Evan on 2014-03-09

Kill globals.

319. By Evan on 2014-03-09

Add a test for create_dirs in the rabbitmq-worker charm.

320. By Evan on 2014-03-09

Oops. Don't always return non-zero.

321. By Evan on 2014-03-09

Fix writing of upstart job and add a test for it.

322. By Evan on 2014-03-09

Don't collide on names with the logging module.

323. By Evan on 2014-03-09

Export, not env. The former ensures the variable gets passed through to run_worker.

Unmerged revisions

323. By Evan on 2014-03-09

Export, not env. The former ensures the variable gets passed through to run_worker.

322. By Evan on 2014-03-09

Don't collide on names with the logging module.

321. By Evan on 2014-03-09

Fix writing of upstart job and add a test for it.

320. By Evan on 2014-03-09

Oops. Don't always return non-zero.

319. By Evan on 2014-03-09

Add a test for create_dirs in the rabbitmq-worker charm.

318. By Evan on 2014-03-09

Kill globals.

317. By Evan on 2014-03-09

Create the config dir. Support upgrading the charm by reinstalling it. Log the error when the charm fails.

316. By Evan on 2014-03-09

Fix tarball extraction code and set dir permissions appropriately.

315. By Evan on 2014-03-09

Fix reference to user and group on config object.

314. By Evan on 2014-03-09

Fix references to uid.

=== modified file 'branch-source-builder/bsbuilder/run_worker.py'
--- branch-source-builder/bsbuilder/run_worker.py	2014-03-06 05:57:13 +0000
+++ branch-source-builder/bsbuilder/run_worker.py	2014-03-09 17:04:23 +0000
@@ -15,16 +15,14 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import json
-import logging
 import time
 
 import upload_package
 import watch_ppa
 
-from ci_utils import amqp_utils, dump_stack
+from ci_utils import (amqp_utils, dump_stack, logger)
 
-logging.basicConfig(level=logging.INFO)
-log = logging.getLogger(__name__)
+log = logger.setup_logging('bsb-worker')
 
 TIME_BETWEEN_CHECKS = 60
 

=== modified file 'charms/precise/rabbitmq-worker/config.yaml'
--- charms/precise/rabbitmq-worker/config.yaml	2014-02-25 22:29:04 +0000
+++ charms/precise/rabbitmq-worker/config.yaml	2014-03-09 17:04:23 +0000
@@ -33,11 +33,11 @@
        <install_root>/<unit>/unit_config
       where <unit> is the location the branch is extracted to.
 
-  uid:
+  user:
     type: string
     default: nobody
     description: User to run service as
-  gid:
+  group:
     type: string
     default: nogroup
     description: Group to run service as

=== modified file 'charms/precise/rabbitmq-worker/hooks/hooks.py'
--- charms/precise/rabbitmq-worker/hooks/hooks.py	2014-03-07 20:45:24 +0000
+++ charms/precise/rabbitmq-worker/hooks/hooks.py	2014-03-09 17:04:23 +0000
@@ -2,86 +2,90 @@
 
 import base64
 import os
-import json
 import shutil
 import subprocess
 import sys
 import tempfile
 import textwrap
+import tempfile
 import charmhelpers.fetch
-
-
-def juju_info(msg):
-    subprocess.check_call(['juju-log', '-l', 'INFO', msg])
-    pass
-
-
-def _config():
-    output = subprocess.check_output(['config-get', '--format=json'])
-    return json.loads(output)
-
-
-def _relation_get(key):
-    return subprocess.check_output(['relation-get', key]).strip()
-
-
-def _relation_set(keyvalues, relation_id=None):
-    args = ['relation-set']
-    if relation_id:
-        args.extend(['-r', relation_id])
-    args.extend(["{}={}".format(k, v or '') for k, v in keyvalues.items()])
-    subprocess.check_call(args)
-
-
-def _service_name(config):
-    unit = os.environ['JUJU_UNIT_NAME'].split('/')[0]
+from charmhelpers.core import (hookenv, host)
+
+
+def _service_name():
+    unit = hookenv.service_name()
     for x in (':', '-', '/', '"', "'"):
         unit = unit.replace(x, '_')
     return unit
 
 
-def _service_dir(config):
-    return os.path.join(config['install_root'], _service_name(config))
+hooks = hookenv.Hooks()
+
+root_dir = lambda cfg: os.path.join(cfg['install_root'], _service_name())
+code_dir = lambda cfg: os.path.join(root_dir(cfg), 'code')
+conf_dir = lambda cfg: os.path.join(root_dir(cfg), 'conf')
+logs_dir = lambda cfg: os.path.join(root_dir(cfg), 'logs')
+vars_dir = lambda cfg: os.path.join(root_dir(cfg), 'var')
+
+
+def create_dirs(config):
+    """Create the directory structure expected by the deployment.
+    /srv/rabbitmq_worker/{code,conf,logs,var}/
+    """
+    user = config['user']
+    group = config['group']
+
+    host.mkdir(root_dir(config), perms=0755)
+    host.mkdir(code_dir(config), perms=0755)
+    host.mkdir(conf_dir(config), perms=0755)
+    host.mkdir(logs_dir(config), owner=user, group=group, perms=0755)
+    host.mkdir(vars_dir(config), owner=user, group=group, perms=0755)
 
 
 def pip_install(package):
     cmd_line = ['pip', 'install', '-b', '/tmp/']
-    if package.startswith('svn+') or package.startswith('git+') or \
-       package.startswith('hg+') or package.startswith('bzr+'):
-           cmd_line.append('-e')
+    svn = package.startswith('svn+')
+    git = package.startswith('git+')
+    hg = package.startswith('hg+')
+    bzr = package.startswith('bzr+')
+
+    if (svn or git or hg or bzr):
+        cmd_line.append('-e')
     cmd_line.append(package)
-    return(subprocess.call(cmd_line))
+    return subprocess.check_call(cmd_line)
 
 
 def _install_from_tarball(config):
-    juju_info('grabbing service from tarball...')
-    sdir = _service_dir(config)
-    if os.path.exists(sdir):
-        juju_info('deleting pre-existing service directory: %s' % sdir)
-        shutil.rmtree(sdir)
-    os.mkdir(sdir)
-    cmd = 'curl %s | tar -xzC %s' % (config['tarball'], sdir)
+    hookenv.log('grabbing service from tarball...')
+    cdir = code_dir(config)
+    if os.listdir(cdir):
+        hookenv.log('deleting pre-existing code directory: %s' % cdir)
+        shutil.rmtree(cdir)
+        os.mkdir(cdir)
+    cmd = 'curl %s | tar -xzC %s' % (config['tarball'], cdir)
     subprocess.check_call(cmd, shell=True)
 
 
 def _install_from_bzr(config):
-    juju_info('grabbing service from bzr...')
-
-    sdir = _service_dir(config)
-    if os.path.exists(sdir):
-        juju_info('deleting pre-existing service directory: %s' % sdir)
-        shutil.rmtree(sdir)
+    hookenv.log('grabbing service from bzr...')
+    cdir = code_dir(config)
+    if os.path.exists(cdir):
+        hookenv.log('deleting pre-existing code directory: %s' % cdir)
+        shutil.rmtree(cdir)
 
     args = ['bzr', 'branch']
     rev = config.get('revno', '')
     if rev:
         args.extend(['-r', rev])
     args.append(config['branch'])
-    args.append(sdir)
+    args.append(cdir)
     subprocess.check_call(args)
 
 
-def install(config):
[e-mail]()
[e-mail]('upgrade-charm')
+def install():
+    config = hookenv.config()
     charmhelpers.fetch.configure_sources(update=True)
 
     pkgs = [x for x in config.get('packages', '').split(' ') if x]
@@ -89,15 +93,16 @@
     pkgs.append('python-pip')
     pkgs.append('bzr')
 
-    juju_info('installing apt packages...')
+    hookenv.log('installing apt packages...')
     charmhelpers.fetch.apt_install(pkgs)
 
     pip_pkgs = [x for x in config.get('pip-packages', '').split(' ') if x]
     if pip_pkgs:
-        juju_info('installing pip packages...')
+        hookenv.log('installing pip packages...')
         for package in pip_pkgs:
             pip_install(package.strip())
 
+    create_dirs(config)
     if config.get('vcs') == 'tarball':
         _install_from_tarball(config)
     else:
@@ -105,7 +110,7 @@
 
     unit_config = config.get('unit-config')
     if unit_config:
-        with open(os.path.join(_service_dir(config), 'unit_config'), 'w') as f:
+        with open(os.path.join(conf_dir(config), 'unit_config'), 'w') as f:
             f.write(base64.b64decode(unit_config))
 
 
@@ -121,36 +126,63 @@
     # use sigint so python code just needs to catch KeyboardInterrupt
     kill signal SIGINT
 
+    export PYTHONPATH={conf_dir}
+    export CONFDIR={conf_dir}
+    export VARDIR={vars_dir}
+    export LOGDIR={logs_dir}
+
     # If the process quits unexpectadly trigger respawn it, unless it
     # fails 15 times within 5 seconds
     respawn
     respawn limit 15 5
 
-    setuid {uid}
-    setgid {gid}
-    chdir {sdir}
+    setuid {user}
+    setgid {group}
+    chdir {code_dir}
 
     exec {main}
     ''')
     params = {
         'main': config['main'],
-        'sdir': _service_dir(config),
-        'uid': config.get('uid', 'nobody'),
-        'gid': config.get('gid', 'nogroup'),
+        'conf_dir': conf_dir(config),
+        'code_dir': code_dir(config),
+        'vars_dir': vars_dir(config),
+        'logs_dir': logs_dir(config),
+        'user': config.get('user', 'nobody'),
+        'group': config.get('group', 'nogroup'),
     }
+<<<<<<< TREE
     path = '/etc/init/{}.conf'.format(_service_name(config))
     with tempfile.NamedTemporaryFile('w', delete=False) as f:
+=======
+    # Atomic file creation.
+    with tempfile.NamedTemporaryFile('w', delete=False) as f:
+>>>>>>> MERGE-SOURCE
         f.write(template.format(**params))
+<<<<<<< TREE
     # Rename to final destination
     os.rename(f.name, path)
+=======
+        # need this so the file is ready for the subprocess call
+        f.flush()
+        os.fsync(f.fileno())
+    os.rename(f.name, '/etc/init/%s.conf' % _service_name())
+>>>>>>> MERGE-SOURCE
 
 
 def _create_amqp_config(config):
+<<<<<<< TREE
     path = os.path.join(_service_dir(config), 'amqp_config.py')
     with tempfile.NamedTemporaryFile('w', delete=False) as f:
+=======
+    path = os.path.join(conf_dir(config), 'amqp_config.py')
+    # Atomic file creation.
+    with tempfile.NamedTemporaryFile('w', delete=False) as f:
+>>>>>>> MERGE-SOURCE
         f.write('# DO NOT EDIT. Generated by restish charm hook\n')
         f.write('AMQP_USER = "%s"\n' % config['amqp-user'])
         f.write('AMQP_VHOST = "%s"\n' % config['amqp-vhost'])
+<<<<<<< TREE
         f.write('AMQP_HOST = "%s"\n' % _relation_get('private-address'))
         f.write('AMQP_PASSWORD = "%s"\n' % _relation_get('password'))
     # Now that the file is complete (and closed) rename it to its final
@@ -168,37 +200,52 @@
 
 
 def amqp_relation_changed(config):
+=======
+        f.write('AMQP_HOST = "%s"\n' % hookenv.relation_get('private-address'))
+        f.write('AMQP_PASSWORD = "%s"\n' % hookenv.relation_get('password'))
+    # Rename to final location.
+    os.rename(f.name, path)
+
+
[e-mail]('amqp-relation-joined')
+def amqp_relation_joined():
+    config = hookenv.config()
+    hookenv.relation_set(
+        relation_settings={
+            'username': config['amqp-user'],
+            'vhost': config['amqp-vhost'],
+        }
+    )
+
+
[e-mail]('amqp-relation-changed')
+def amqp_relation_changed():
+    config = hookenv.config()
+>>>>>>> MERGE-SOURCE
     _create_upstart(config)
     _create_amqp_config(config)
     try:
-        subprocess.check_call(['/sbin/restart', _service_name(config)])
+        subprocess.check_call(['/sbin/restart', _service_name()])
     except subprocess.CalledProcessError:
         # if it wasn't running, restart fails, so just try to start
-        subprocess.check_call(['/sbin/start', _service_name(config)])
-
-
-def amqp_relation_broken(config):
-    subprocess.call(['/sbin/stop', _service_name(config)])
-    os.unlink(os.path.join(_service_dir(config), 'amqp_config.py'))
+        subprocess.check_call(['/sbin/start', _service_name()])
+
+
[e-mail]('amqp-relation-broken')
+def amqp_relation_broken():
+    config = hookenv.config()
+    host.service_stop(_service_name())
+    os.unlink(os.path.join(conf_dir(config), 'amqp_config.py'))
 
 
 def main():
-    hook = os.path.basename(sys.argv[0])
-    juju_info("Running hook: %s" % hook)
-
-    hook_py = hook.replace('-', '_')
-    funcs = globals()
-    if hook_py not in funcs:
-        print("Unknown hook: %s" % hook)
+    try:
+        hooks.execute(sys.argv)
+        return 0
+    except Exception as e:
+        hookenv.log('hook failed: %s' % str(e))
         return 1
 
-    config = _config()
-    try:
-        return funcs[hook_py](config)
-    except subprocess.CalledProcessError as e:
-        juju_info('Error running: %s: %s' % (e.cmd, e.output))
-        return e.returncode
-
 
 if __name__ == '__main__':
     exit(main())

=== added file 'charms/precise/rabbitmq-worker/hooks/test_hooks.py'
--- charms/precise/rabbitmq-worker/hooks/test_hooks.py	1970-01-01 00:00:00 +0000
+++ charms/precise/rabbitmq-worker/hooks/test_hooks.py	2014-03-09 17:04:23 +0000
@@ -0,0 +1,34 @@
+import os
+import testtools
+import hooks
+import mock
+import ci_utils
+
+class T(testtools.TestCase):
+    # Give it a name.
+    @mock.patch.dict(os.environ, {'JUJU_UNIT_NAME': 'test_unit'})
+    # charmhelpers wants to call juju-log.
+    @mock.patch('subprocess.call')
+    # You need to be root to chown to a different user/group.
+    @mock.patch('os.chown')
+    def test_create_dirs(self, mock_chown, mock_call):
+        with ci_utils.TmpDir() as d:
+            cfg = {'install_root': d, 'user': 'nobody', 'group': 'nogroup'}
+            hooks.create_dirs(cfg)
+            paths = ('code', 'conf', 'logs', 'var')
+            for p in map(lambda x: os.path.join(d, 'test_unit', x), paths):
+                self.assertTrue(os.path.exists(p))
+
+    # Give it a name.
+    @mock.patch.dict(os.environ, {'JUJU_UNIT_NAME': 'test_unit'})
+    # We cannot rename to /etc/init/...
+    @mock.patch('os.rename')
+    def test_create_upstart(self, mock_rename):
+        with ci_utils.TmpDir() as d:
+            u = 'nobody'
+            g = 'nogroup'
+            p = '/usr/bin/program'
+            cfg = {'install_root': d, 'user': u, 'group': g, 'main': p}
+            hooks._create_upstart(cfg)
+            expected = '/etc/init/test_unit.conf'
+            self.assertEqual(mock_rename.call_args[0][1], expected)

=== added symlink 'charms/precise/rabbitmq-worker/hooks/upgrade-charm'
=== target is u'hooks.py'
=== added file 'ci-utils/ci_utils/logger.py'
--- ci-utils/ci_utils/logger.py	1970-01-01 00:00:00 +0000
+++ ci-utils/ci_utils/logger.py	2014-03-09 17:04:23 +0000
@@ -0,0 +1,34 @@
+# Ubuntu CI Engine
+# Copyright 2013, 2014 Canonical Ltd.
+
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Affero General Public License version 3, as
+# published by the Free Software Foundation.
+
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU Affero General Public License for more details.
+
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import logging
+import os
+
+
+def setup_logging(name):
+    fmt = '%(asctime)s,%(msecs)d %(name)s %(levelname)s %(message)s'
+    kwargs = {'format': fmt, 'datefmt': '%H:%M:%S', 'level': logging.INFO}
+    logging.basicConfig(**kwargs)
+    log = logging.getLogger(name)
+
+    if 'LOGDIR' in os.environ:
+        path = '%s.log' % name
+        path = os.path.join(os.environ['LOGDIR'], path)
+        # 10 MB
+        tenmb = 10 * 1024 * 1024
+        fh = logging.handlers.RotatingFileHandler(path, maxBytes=tenmb)
+        log.addHandler(fh)
+
+    return log

=== modified file 'juju-deployer/image-builder.yaml.tmpl'
--- juju-deployer/image-builder.yaml.tmpl	2014-03-09 17:04:23 +0000
+++ juju-deployer/image-builder.yaml.tmpl	2014-03-09 17:04:23 +0000
@@ -28,7 +28,7 @@
                 branch: ${CI_BRANCH}
                 tarball: ${CI_PAYLOAD_URL}
                 packages: "qemu-utils python-glanceclient python-swiftclient"
-                uid: root
+                user: root
                 unit-config: include-base64://configs/unit_config.yaml
                 install_sources: |
                     - ${CI_PPA}

=== modified file 'juju-deployer/test-runner.yaml.tmpl'
--- juju-deployer/test-runner.yaml.tmpl	2014-03-09 17:04:23 +0000
+++ juju-deployer/test-runner.yaml.tmpl	2014-03-09 17:04:23 +0000
@@ -36,7 +36,7 @@
                 tarball: ${CI_PAYLOAD_URL}
                 packages: "python-requests python-novaclient python-swiftclient"
                 unit-config: include-base64://configs/unit_config.yaml
-                uid: root
+                user: root
                 install_sources: |
                     - ${CI_PPA}
                 install_keys: |

=== modified file 'test_runner/tstrun/run_worker.py'
--- test_runner/tstrun/run_worker.py	2014-03-06 17:33:33 +0000
+++ test_runner/tstrun/run_worker.py	2014-03-09 17:04:23 +0000
@@ -15,16 +15,12 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import json
-import logging
-
-from ci_utils import amqp_utils
-from ci_utils import dump_stack
-from ci_utils import data_store
+from ci_utils import (amqp_utils, dump_stack, data_store, logger)
 import tstrun
 from tstrun import testbed
 
-logging.basicConfig(level=logging.INFO)
-log = logging.getLogger(__name__)
+
+log = logger.setup_logging('test-runner-worker')
 
 
 def on_message(msg):