resync hooks/charmhelpers

Makefile

@@ -22,7 +22,7 @@ bin/charm_helpers_sync.py:
 
 sync: bin/charm_helpers_sync.py
 	@$(PYTHON) bin/charm_helpers_sync.py -c charm-helpers-hooks.yaml
-	@$(PYTHON) bin/charm_helpers_sync.py -c charm-helpers-tests.yaml
+#	@$(PYTHON) bin/charm_helpers_sync.py -c charm-helpers-tests.yaml
 
 publish: lint
 	bzr push lp:charms/ceph-radosgw

hooks/charmhelpers/contrib/python/packages.py

@@ -36,6 +36,8 @@ __author__ = "Jorge Niedbalski <jorge.niedbalski@canonical.com>"
 def parse_options(given, available):
     """Given a set of options, check if available"""
     for key, value in sorted(given.items()):
+        if not value:
+            continue
         if key in available:
             yield "--{0}={1}".format(key, value)
 

hooks/charmhelpers/core/hookenv.py

@@ -21,7 +21,9 @@
 #  Charm Helpers Developers <juju@lists.ubuntu.com>
 
 from __future__ import print_function
+from distutils.version import LooseVersion
 from functools import wraps
+import glob
 import os
 import json
 import yaml
@@ -242,29 +244,7 @@ class Config(dict):
         self.path = os.path.join(charm_dir(), Config.CONFIG_FILE_NAME)
         if os.path.exists(self.path):
             self.load_previous()
-
+        atexit(self._implicit_save)
-    def __getitem__(self, key):
-        """For regular dict lookups, check the current juju config first,
-        then the previous (saved) copy. This ensures that user-saved values
-        will be returned by a dict lookup.
-
-        """
-        try:
-            return dict.__getitem__(self, key)
-        except KeyError:
-            return (self._prev_dict or {})[key]
-
-    def get(self, key, default=None):
-        try:
-            return self[key]
-        except KeyError:
-            return default
-
-    def keys(self):
-        prev_keys = []
-        if self._prev_dict is not None:
-            prev_keys = self._prev_dict.keys()
-        return list(set(prev_keys + list(dict.keys(self))))
 
     def load_previous(self, path=None):
         """Load previous copy of config from disk.
@@ -283,6 +263,9 @@ class Config(dict):
         self.path = path or self.path
         with open(self.path) as f:
             self._prev_dict = json.load(f)
+        for k, v in self._prev_dict.items():
+            if k not in self:
+                self[k] = v
 
     def changed(self, key):
         """Return True if the current value for this key is different from
@@ -314,13 +297,13 @@ class Config(dict):
         instance.
 
         """
-        if self._prev_dict:
-            for k, v in six.iteritems(self._prev_dict):
-                if k not in self:
-                    self[k] = v
         with open(self.path, 'w') as f:
             json.dump(self, f)
 
+    def _implicit_save(self):
+        if self.implicit_save:
+            self.save()
+
 
 @cached
 def config(scope=None):
@@ -587,10 +570,14 @@ class Hooks(object):
             hooks.execute(sys.argv)
     """
 
-    def __init__(self, config_save=True):
+    def __init__(self, config_save=None):
         super(Hooks, self).__init__()
         self._hooks = {}
-        self._config_save = config_save
+
+        # For unknown reasons, we allow the Hooks constructor to override
+        # config().implicit_save.
+        if config_save is not None:
+            config().implicit_save = config_save
 
     def register(self, name, function):
         """Register a hook"""
@@ -598,13 +585,16 @@ class Hooks(object):
 
     def execute(self, args):
         """Execute a registered hook based on args[0]"""
+        _run_atstart()
         hook_name = os.path.basename(args[0])
         if hook_name in self._hooks:
+            try:
                 self._hooks[hook_name]()
-            if self._config_save:
+            except SystemExit as x:
-                cfg = config()
+                if x.code is None or x.code == 0:
-                if cfg.implicit_save:
+                    _run_atexit()
-                    cfg.save()
+                raise
+            _run_atexit()
         else:
             raise UnregisteredHookError(hook_name)
 
@@ -732,13 +722,79 @@ def leader_get(attribute=None):
 @translate_exc(from_exc=OSError, to_exc=NotImplementedError)
 def leader_set(settings=None, **kwargs):
     """Juju leader set value(s)"""
-    log("Juju leader-set '%s'" % (settings), level=DEBUG)
+    # Don't log secrets.
+    # log("Juju leader-set '%s'" % (settings), level=DEBUG)
     cmd = ['leader-set']
     settings = settings or {}
     settings.update(kwargs)
-    for k, v in settings.iteritems():
+    for k, v in settings.items():
         if v is None:
             cmd.append('{}='.format(k))
         else:
             cmd.append('{}={}'.format(k, v))
     subprocess.check_call(cmd)
+
+
+@cached
+def juju_version():
+    """Full version string (eg. '1.23.3.1-trusty-amd64')"""
+    # Per https://bugs.launchpad.net/juju-core/+bug/1455368/comments/1
+    jujud = glob.glob('/var/lib/juju/tools/machine-*/jujud')[0]
+    return subprocess.check_output([jujud, 'version'],
+                                   universal_newlines=True).strip()
+
+
+@cached
+def has_juju_version(minimum_version):
+    """Return True if the Juju version is at least the provided version"""
+    return LooseVersion(juju_version()) >= LooseVersion(minimum_version)
+
+
+_atexit = []
+_atstart = []
+
+
+def atstart(callback, *args, **kwargs):
+    '''Schedule a callback to run before the main hook.
+
+    Callbacks are run in the order they were added.
+
+    This is useful for modules and classes to perform initialization
+    and inject behavior. In particular:
+        - Run common code before all of your hooks, such as logging
+          the hook name or interesting relation data.
+        - Defer object or module initialization that requires a hook
+          context until we know there actually is a hook context,
+          making testing easier.
+        - Rather than requiring charm authors to include boilerplate to
+          invoke your helper's behavior, have it run automatically if
+          your object is instantiated or module imported.
+
+    This is not at all useful after your hook framework as been launched.
+    '''
+    global _atstart
+    _atstart.append((callback, args, kwargs))
+
+
+def atexit(callback, *args, **kwargs):
+    '''Schedule a callback to run on successful hook completion.
+
+    Callbacks are run in the reverse order that they were added.'''
+    _atexit.append((callback, args, kwargs))
+
+
+def _run_atstart():
+    '''Hook frameworks must invoke this before running the main hook body.'''
+    global _atstart
+    for callback, args, kwargs in _atstart:
+        callback(*args, **kwargs)
+    del _atstart[:]
+
+
+def _run_atexit():
+    '''Hook frameworks must invoke this after the main hook body has
+    successfully completed. Do not invoke it if the hook fails.'''
+    global _atexit
+    for callback, args, kwargs in reversed(_atexit):
+        callback(*args, **kwargs)
+    del _atexit[:]

hooks/charmhelpers/core/services/base.py

@@ -128,15 +128,18 @@ class ServiceManager(object):
         """
         Handle the current hook by doing The Right Thing with the registered services.
         """
+        hookenv._run_atstart()
+        try:
             hook_name = hookenv.hook_name()
             if hook_name == 'stop':
                 self.stop_services()
             else:
                 self.reconfigure_services()
                 self.provide_data()
-        cfg = hookenv.config()
+        except SystemExit as x:
-        if cfg.implicit_save:
+            if x.code is None or x.code == 0:
-            cfg.save()
+                hookenv._run_atexit()
+        hookenv._run_atexit()
 
     def provide_data(self):
         """