Merge lp:~rcj/charms/trusty/ntp/lp1366880 into lp:charms/trusty/ntp

[Robert C Jennings]

[Matt Bruzek (mbruzek)]

+1 LGTM I was able to deploy this successfully thanks to the automated tests!

review: Approve

=== added directory 'hooks/charmhelpers/contrib'
=== added file 'hooks/charmhelpers/contrib/__init__.py'
=== added directory 'hooks/charmhelpers/contrib/templating'
=== added file 'hooks/charmhelpers/contrib/templating/__init__.py'
=== added file 'hooks/charmhelpers/contrib/templating/jinja.py'
--- hooks/charmhelpers/contrib/templating/jinja.py	1970-01-01 00:00:00 +0000
+++ hooks/charmhelpers/contrib/templating/jinja.py	2014-09-08 16:44:10 +0000
@@ -0,0 +1,23 @@
+"""
+Templating using the python-jinja2 package.
+"""
+from charmhelpers.fetch import (
+    apt_install,
+)
+
+
+DEFAULT_TEMPLATES_DIR = 'templates'
+
+
+try:
+    import jinja2
+except ImportError:
+    apt_install(["python-jinja2"])
+    import jinja2
+
+
+def render(template_name, context, template_dir=DEFAULT_TEMPLATES_DIR):
+    templates = jinja2.Environment(
+        loader=jinja2.FileSystemLoader(template_dir))
+    template = templates.get_template(template_name)
+    return template.render(context)

=== modified file 'hooks/charmhelpers/core/hookenv.py'
--- hooks/charmhelpers/core/hookenv.py	2014-07-16 05:40:55 +0000
+++ hooks/charmhelpers/core/hookenv.py	2014-09-08 16:44:10 +0000
@@ -156,12 +156,15 @@
 
 
 class Config(dict):
-    """A Juju charm config dictionary that can write itself to
-    disk (as json) and track which values have changed since
-    the previous hook invocation.
-
-    Do not instantiate this object directly - instead call
-    ``hookenv.config()``
+    """A dictionary representation of the charm's config.yaml, with some
+    extra features:
+
+    - See which values in the dictionary have changed since the previous hook.
+    - For values that have changed, see what the previous value was.
+    - Store arbitrary data for use in a later hook.
+
+    NOTE: Do not instantiate this object directly - instead call
+    ``hookenv.config()``, which will return an instance of :class:`Config`.
 
     Example usage::
 
@@ -170,8 +173,8 @@
         >>> config = hookenv.config()
         >>> config['foo']
         'bar'
+        >>> # store a new key/value for later use
         >>> config['mykey'] = 'myval'
-        >>> config.save()
 
 
         >>> # user runs `juju set mycharm foo=baz`
@@ -188,22 +191,23 @@
         >>> # keys/values that we add are preserved across hooks
         >>> config['mykey']
         'myval'
-        >>> # don't forget to save at the end of hook!
-        >>> config.save()
 
     """
     CONFIG_FILE_NAME = '.juju-persistent-config'
 
     def __init__(self, *args, **kw):
         super(Config, self).__init__(*args, **kw)
+        self.implicit_save = True
         self._prev_dict = None
         self.path = os.path.join(charm_dir(), Config.CONFIG_FILE_NAME)
         if os.path.exists(self.path):
             self.load_previous()
 
     def load_previous(self, path=None):
-        """Load previous copy of config from disk so that current values
-        can be compared to previous values.
+        """Load previous copy of config from disk.
+
+        In normal usage you don't need to call this method directly - it
+        is called automatically at object initialization.
 
         :param path:
 
@@ -218,8 +222,8 @@
             self._prev_dict = json.load(f)
 
     def changed(self, key):
-        """Return true if the value for this key has changed since
-        the last save.
+        """Return True if the current value for this key is different from
+        the previous value.
 
         """
         if self._prev_dict is None:
@@ -228,7 +232,7 @@
 
     def previous(self, key):
         """Return previous value for this key, or None if there
-        is no "previous" value.
+        is no previous value.
 
         """
         if self._prev_dict:
@@ -238,7 +242,13 @@
     def save(self):
         """Save this config to disk.
 
-        Preserves items in _prev_dict that do not exist in self.
+        If the charm is using the :mod:`Services Framework <services.base>`
+        or :meth:'@hook <Hooks.hook>' decorator, this
+        is called automatically at the end of successful hook execution.
+        Otherwise, it should be called directly by user code.
+
+        To disable automatic saves, set ``implicit_save=False`` on this
+        instance.
 
         """
         if self._prev_dict:
@@ -285,8 +295,9 @@
         raise
 
 
-def relation_set(relation_id=None, relation_settings={}, **kwargs):
+def relation_set(relation_id=None, relation_settings=None, **kwargs):
     """Set relation information for the current unit"""
+    relation_settings = relation_settings if relation_settings else {}
     relation_cmd_line = ['relation-set']
     if relation_id is not None:
         relation_cmd_line.extend(('-r', relation_id))
@@ -477,6 +488,9 @@
         hook_name = os.path.basename(args[0])
         if hook_name in self._hooks:
             self._hooks[hook_name]()
+            cfg = config()
+            if cfg.implicit_save:
+                cfg.save()
         else:
             raise UnregisteredHookError(hook_name)
 

=== modified file 'hooks/charmhelpers/core/host.py'
--- hooks/charmhelpers/core/host.py	2014-08-05 05:12:15 +0000
+++ hooks/charmhelpers/core/host.py	2014-09-08 16:44:10 +0000
@@ -12,6 +12,8 @@
 import string
 import subprocess
 import hashlib
+import shutil
+from contextlib import contextmanager
 
 from collections import OrderedDict
 
@@ -52,7 +54,7 @@
 def service_running(service):
     """Determine whether a system service is running"""
     try:
-        output = subprocess.check_output(['service', service, 'status'])
+        output = subprocess.check_output(['service', service, 'status'], stderr=subprocess.STDOUT)
     except subprocess.CalledProcessError:
         return False
     else:
@@ -62,6 +64,16 @@
             return False
 
 
+def service_available(service_name):
+    """Determine whether a system service is available"""
+    try:
+        subprocess.check_output(['service', service_name, 'status'], stderr=subprocess.STDOUT)
+    except subprocess.CalledProcessError:
+        return False
+    else:
+        return True
+
+
 def adduser(username, password=None, shell='/bin/bash', system_user=False):
     """Add a user to the system"""
     try:
@@ -320,12 +332,29 @@
 
     '''
     import apt_pkg
+    from charmhelpers.fetch import apt_cache
     if not pkgcache:
-        apt_pkg.init()
-        # Force Apt to build its cache in memory. That way we avoid race
-        # conditions with other applications building the cache in the same
-        # place.
-        apt_pkg.config.set("Dir::Cache::pkgcache", "")
-        pkgcache = apt_pkg.Cache()
+        pkgcache = apt_cache()
     pkg = pkgcache[package]
     return apt_pkg.version_compare(pkg.current_ver.ver_str, revno)
+
+
+@contextmanager
+def chdir(d):
+    cur = os.getcwd()
+    try:
+        yield os.chdir(d)
+    finally:
+        os.chdir(cur)
+
+
+def chownr(path, owner, group):
+    uid = pwd.getpwnam(owner).pw_uid
+    gid = grp.getgrnam(group).gr_gid
+
+    for root, dirs, files in os.walk(path):
+        for name in dirs + files:
+            full = os.path.join(root, name)
+            broken_symlink = os.path.lexists(full) and not os.path.exists(full)
+            if not broken_symlink:
+                os.chown(full, uid, gid)

=== modified file 'hooks/charmhelpers/fetch/__init__.py'
--- hooks/charmhelpers/fetch/__init__.py	2014-07-16 05:40:55 +0000
+++ hooks/charmhelpers/fetch/__init__.py	2014-09-08 16:44:10 +0000
@@ -1,4 +1,5 @@
 import importlib
+from tempfile import NamedTemporaryFile
 import time
 from yaml import safe_load
 from charmhelpers.core.host import (
@@ -116,14 +117,7 @@
 
 def filter_installed_packages(packages):
     """Returns a list of packages that require installation"""
-    import apt_pkg
-    apt_pkg.init()
-
-    # Tell apt to build an in-memory cache to prevent race conditions (if
-    # another process is already building the cache).
-    apt_pkg.config.set("Dir::Cache::pkgcache", "")
-
-    cache = apt_pkg.Cache()
+    cache = apt_cache()
     _pkgs = []
     for package in packages:
         try:
@@ -136,6 +130,16 @@
     return _pkgs
 
 
+def apt_cache(in_memory=True):
+    """Build and return an apt cache"""
+    import apt_pkg
+    apt_pkg.init()
+    if in_memory:
+        apt_pkg.config.set("Dir::Cache::pkgcache", "")
+        apt_pkg.config.set("Dir::Cache::srcpkgcache", "")
+    return apt_pkg.Cache()
+
+
 def apt_install(packages, options=None, fatal=False):
     """Install one or more packages"""
     if options is None:
@@ -201,6 +205,27 @@
 
 
 def add_source(source, key=None):
+    """Add a package source to this system.
+
+    @param source: a URL or sources.list entry, as supported by
+    add-apt-repository(1). Examples:
+        ppa:charmers/example
+        deb https://stub:[e-mail]/ubuntu trusty main
+
+    In addition:
+        'proposed:' may be used to enable the standard 'proposed'
+        pocket for the release.
+        'cloud:' may be used to activate official cloud archive pockets,
+        such as 'cloud:icehouse'
+
+    @param key: A key to be added to the system's APT keyring and used
+    to verify the signatures on packages. Ideally, this should be an
+    ASCII format GPG public key including the block headers. A GPG key
+    id may also be used, but be aware that only insecure protocols are
+    available to retrieve the actual public key from a public keyserver
+    placing your Juju environment at risk. ppa and cloud archive keys
+    are securely added automtically, so sould not be provided.
+    """
     if source is None:
         log('Source is not present. Skipping')
         return
@@ -225,10 +250,23 @@
         release = lsb_release()['DISTRIB_CODENAME']
         with open('/etc/apt/sources.list.d/proposed.list', 'w') as apt:
             apt.write(PROPOSED_POCKET.format(release))
+    else:
+        raise SourceConfigError("Unknown source: {!r}".format(source))
+
     if key:
-        subprocess.check_call(['apt-key', 'adv', '--keyserver',
-                               'hkp://keyserver.ubuntu.com:80', '--recv',
-                               key])
+        if '-----BEGIN PGP PUBLIC KEY BLOCK-----' in key:
+            with NamedTemporaryFile() as key_file:
+                key_file.write(key)
+                key_file.flush()
+                key_file.seek(0)
+                subprocess.check_call(['apt-key', 'add', '-'], stdin=key_file)
+        else:
+            # Note that hkp: is in no way a secure protocol. Using a
+            # GPG key id is pointless from a security POV unless you
+            # absolutely trust your network and DNS.
+            subprocess.check_call(['apt-key', 'adv', '--keyserver',
+                                   'hkp://keyserver.ubuntu.com:80', '--recv',
+                                   key])
 
 
 def configure_sources(update=False,
@@ -238,7 +276,8 @@
     Configure multiple sources from charm configuration.
 
     The lists are encoded as yaml fragments in the configuration.
-    The frament needs to be included as a string.
+    The frament needs to be included as a string. Sources and their
+    corresponding keys are of the types supported by add_source().
 
     Example config:
         install_sources: |