Merge lp:~stub/charm-helpers/integration into lp:charm-helpers

[Stuart Bishop]

This is an integration branch containing the last few months worth of outstanding merge proposals. If you review and land this, all my other outstanding ones will automatically get flagged as merged.

Or if you rather, review and land the smaller focused MPs which will leave this one empty.

https://code.launchpad.net/~stub/charm-helpers/improve-peerstorage/+merge/243519
https://code.launchpad.net/~stub/charm-helpers/py3/+merge/244668
https://code.launchpad.net/~stub/charm-helpers/improve-cache-decorator/+merge/245929
https://code.launchpad.net/~stub/charm-helpers/unit-public-ip/+merge/245930
https://code.launchpad.net/~stub/charm-helpers/maintain-service-ordering/+merge/245962
https://code.launchpad.net/~stub/charm-helpers/bug-1413596-remote_unit-fix/+merge/247289
https://code.launchpad.net/~stub/charm-helpers/bug-1440073-unison/+merge/255192

[Liam Young (gnuoy)]

Approve

review: Approve

=== modified file 'charmhelpers/contrib/peerstorage/__init__.py'
--- charmhelpers/contrib/peerstorage/__init__.py	2015-01-22 06:06:03 +0000
+++ charmhelpers/contrib/peerstorage/__init__.py	2015-04-29 13:11:45 +0000
@@ -73,6 +73,8 @@
     exc_list = exc_list if exc_list else []
     peerdb_settings = peer_retrieve('-', relation_name=relation_name)
     matched = {}
+    if peerdb_settings is None:
+        return matched
     for k, v in peerdb_settings.items():
         full_prefix = prefix + delimiter
         if k.startswith(full_prefix):

=== modified file 'charmhelpers/contrib/unison/__init__.py'
--- charmhelpers/contrib/unison/__init__.py	2015-01-28 22:09:21 +0000
+++ charmhelpers/contrib/unison/__init__.py	2015-04-29 13:11:45 +0000
@@ -63,6 +63,7 @@
 from charmhelpers.core.host import (
     adduser,
     add_user_to_group,
+    pwgen,
 )
 
 from charmhelpers.core.hookenv import (
@@ -140,7 +141,7 @@
     ssh_dir = os.path.join(home_dir, '.ssh')
     auth_keys = os.path.join(ssh_dir, 'authorized_keys')
     log('Syncing authorized_keys @ %s.' % auth_keys)
-    with open(auth_keys, 'wb') as out:
+    with open(auth_keys, 'w') as out:
         for k in keys:
             out.write('%s\n' % k)
 
@@ -152,16 +153,16 @@
     khosts = []
     for host in hosts:
         cmd = ['ssh-keyscan', '-H', '-t', 'rsa', host]
-        remote_key = check_output(cmd).strip()
+        remote_key = check_output(cmd, universal_newlines=True).strip()
         khosts.append(remote_key)
     log('Syncing known_hosts @ %s.' % known_hosts)
-    with open(known_hosts, 'wb') as out:
+    with open(known_hosts, 'w') as out:
         for host in khosts:
             out.write('%s\n' % host)
 
 
 def ensure_user(user, group=None):
-    adduser(user)
+    adduser(user, pwgen())
     if group:
         add_user_to_group(user, group)
 

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2015-04-07 16:12:51 +0000
+++ charmhelpers/core/hookenv.py	2015-04-29 13:11:45 +0000
@@ -21,6 +21,7 @@
 #  Charm Helpers Developers <[e-mail]>
 
 from __future__ import print_function
+from functools import wraps
 import os
 import json
 import yaml
@@ -58,15 +59,17 @@
 
     will cache the result of unit_get + 'test' for future calls.
     """
+    @wraps(func)
     def wrapper(*args, **kwargs):
         global cache
         key = str((func, args, kwargs))
         try:
             return cache[key]
         except KeyError:
-            res = func(*args, **kwargs)
-            cache[key] = res
-            return res
+            pass  # Drop out of the exception handler scope.
+        res = func(*args, **kwargs)
+        cache[key] = res
+        return res
     return wrapper
 
 
@@ -178,7 +181,7 @@
 
 def remote_unit():
     """The remote unit for the current relation hook"""
-    return os.environ['JUJU_REMOTE_UNIT']
+    return os.environ.get('JUJU_REMOTE_UNIT', None)
 
 
 def service_name():
@@ -250,6 +253,12 @@
         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:
@@ -509,6 +518,11 @@
         return None
 
 
+def unit_public_ip():
+    """Get this unit's public IP address"""
+    return unit_get('public-address')
+
+
 def unit_private_ip():
     """Get this unit's private IP address"""
     return unit_get('private-address')

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2015-03-12 10:56:57 +0000
+++ charmhelpers/core/host.py	2015-04-29 13:11:45 +0000
@@ -90,7 +90,7 @@
             ['service', service_name, 'status'],
             stderr=subprocess.STDOUT).decode('UTF-8')
     except subprocess.CalledProcessError as e:
-        return 'unrecognized service' not in e.output
+        return b'unrecognized service' not in e.output
     else:
         return True
 

=== modified file 'charmhelpers/core/services/base.py'
--- charmhelpers/core/services/base.py	2015-01-22 06:06:03 +0000
+++ charmhelpers/core/services/base.py	2015-04-29 13:11:45 +0000
@@ -17,7 +17,7 @@
 import os
 import re
 import json
-from collections import Iterable
+from collections import Iterable, OrderedDict
 
 from charmhelpers.core import host
 from charmhelpers.core import hookenv
@@ -119,7 +119,7 @@
         """
         self._ready_file = os.path.join(hookenv.charm_dir(), 'READY-SERVICES.json')
         self._ready = None
-        self.services = {}
+        self.services = OrderedDict()
         for service in services or []:
             service_name = service['service']
             self.services[service_name] = service

=== modified file 'charmhelpers/fetch/__init__.py'
--- charmhelpers/fetch/__init__.py	2015-01-22 06:11:15 +0000
+++ charmhelpers/fetch/__init__.py	2015-04-29 13:11:45 +0000
@@ -158,7 +158,7 @@
 
 def apt_cache(in_memory=True):
     """Build and return an apt cache"""
-    import apt_pkg
+    from apt import apt_pkg
     apt_pkg.init()
     if in_memory:
         apt_pkg.config.set("Dir::Cache::pkgcache", "")

=== modified file 'tests/contrib/peerstorage/test_peerstorage.py'
--- tests/contrib/peerstorage/test_peerstorage.py	2014-08-19 09:38:07 +0000
+++ tests/contrib/peerstorage/test_peerstorage.py	2015-04-29 13:11:45 +0000
@@ -146,6 +146,14 @@
         self.assertEquals(peerstorage.peer_retrieve_by_prefix(rel_id), settings)
 
     @patch.object(peerstorage, 'peer_retrieve')
+    def test_peer_retrieve_by_prefix_empty_relation(self, peer_retrieve):
+        # If relation-get returns None, peer_retrieve_by_prefix returns
+        # an empty dictionary.
+        peer_retrieve.return_value = None
+        rel_id = 'db:2'
+        self.assertEquals(peerstorage.peer_retrieve_by_prefix(rel_id), {})
+
+    @patch.object(peerstorage, 'peer_retrieve')
     def test_peer_retrieve_by_prefix_exc_list(self, peer_retrieve):
         rel_id = 'db:2'
         settings = {

=== modified file 'tests/contrib/unison/test_unison.py'
--- tests/contrib/unison/test_unison.py	2015-01-05 10:30:39 +0000
+++ tests/contrib/unison/test_unison.py	2015-04-29 13:11:45 +0000
@@ -1,5 +1,5 @@
 
-from mock import call, patch, MagicMock
+from mock import call, patch, MagicMock, sentinel
 from testtools import TestCase
 
 from tests.helpers import patch_open, FakeRelation
@@ -146,7 +146,7 @@
         ]
         with patch_open() as (_open, _file):
             unison.write_authorized_keys('foo', keys)
-            _open.assert_called_with('/home/foo/.ssh/authorized_keys', 'wb')
+            _open.assert_called_with('/home/foo/.ssh/authorized_keys', 'w')
             for k in keys:
                 self.assertIn(call('%s\n' % k), _file.write.call_args_list)
 
@@ -160,15 +160,17 @@
         self.check_output.side_effect = keys
         with patch_open() as (_open, _file):
             unison.write_known_hosts('foo', ['10.0.0.1', '10.0.0.2'])
-            _open.assert_called_with('/home/foo/.ssh/known_hosts', 'wb')
+            _open.assert_called_with('/home/foo/.ssh/known_hosts', 'w')
             for k in keys:
                 self.assertIn(call('%s\n' % k), _file.write.call_args_list)
 
+    @patch.object(unison, 'pwgen')
     @patch.object(unison, 'add_user_to_group')
     @patch.object(unison, 'adduser')
-    def test_ensure_user(self, adduser, to_group):
+    def test_ensure_user(self, adduser, to_group, pwgen):
+        pwgen.return_value = sentinel.password
         unison.ensure_user('foo', group='foobar')
-        adduser.assert_called_with('foo')
+        adduser.assert_called_with('foo', sentinel.password)
         to_group.assert_called_with('foo', 'foobar')
 
     @patch.object(unison, '_run_as_user')

=== modified file 'tests/core/test_hookenv.py'
--- tests/core/test_hookenv.py	2015-03-12 10:31:22 +0000
+++ tests/core/test_hookenv.py	2015-04-29 13:11:45 +0000
@@ -3,8 +3,7 @@
 from subprocess import CalledProcessError
 import shutil
 import tempfile
-from mock import patch, call, mock_open
-from mock import MagicMock
+from mock import call, MagicMock, mock_open, patch, sentinel
 from testtools import TestCase
 import yaml
 
@@ -128,11 +127,47 @@
         self.assertEqual(c['foo'], 'bar')
         self.assertEqual(c['baz'], 'bam')
 
+    def test_get(self):
+        c = hookenv.Config(dict(foo='bar'))
+        c.save()
+        c = hookenv.Config(dict(baz='bam'))
+
+        self.assertIsNone(c.get('missing'))
+        self.assertIs(c.get('missing', sentinel.missing), sentinel.missing)
+        self.assertEqual(c.get('foo'), 'bar')
+        self.assertEqual(c.get('baz'), 'bam')
+
     def test_keys(self):
         c = hookenv.Config(dict(foo='bar'))
         c["baz"] = "bar"
         self.assertEqual(sorted([six.u("foo"), "baz"]), sorted(c.keys()))
 
+    def test_in(self):
+        # Test behavior of the in operator.
+
+        # Items that exist in the dict exist. Items that don't don't.
+        c = hookenv.Config(dict(foo='one'))
+        self.assertTrue('foo' in c)
+        self.assertTrue('bar' not in c)
+        c.save()
+        self.assertTrue('foo' in c)
+        self.assertTrue('bar' not in c)
+
+        # Adding items works as expected.
+        c['foo'] = 'two'
+        c['bar'] = 'two'
+        self.assertTrue('foo' in c)
+        self.assertTrue('bar' in c)
+        c.save()
+        self.assertTrue('foo' in c)
+        self.assertTrue('bar' in c)
+
+        # Removing items works as expected.
+        del c['foo']
+        self.assertTrue('foo' not in c)
+        c.save()
+        self.assertTrue('foo' not in c)
+
 
 class SerializableTest(TestCase):
     def test_serializes_object_to_json(self):
@@ -298,10 +333,16 @@
         self.assertEqual(hookenv.local_unit(), 'foo')
 
     @patch('charmhelpers.core.hookenv.unit_get')
+    def test_gets_unit_public_ip(self, _unitget):
+        _unitget.return_value = sentinel.public_ip
+        self.assertEqual(sentinel.public_ip, hookenv.unit_public_ip())
+        _unitget.assert_called_once_with('public-address')
+
+    @patch('charmhelpers.core.hookenv.unit_get')
     def test_gets_unit_private_ip(self, _unitget):
-        _unitget.return_value = 'foo'
-        self.assertEqual("foo", hookenv.unit_private_ip())
-        _unitget.assert_called_with('private-address')
+        _unitget.return_value = sentinel.private_ip
+        self.assertEqual(sentinel.private_ip, hookenv.unit_private_ip())
+        _unitget.assert_called_once_with('private-address')
 
     @patch('charmhelpers.core.hookenv.os')
     def test_checks_that_is_running_in_relation_hook(self, os_):
@@ -447,6 +488,11 @@
 
         self.assertEqual(hookenv.remote_unit(), 'foo')
 
+    @patch('charmhelpers.core.hookenv.os')
+    def test_no_remote_unit(self, os_):
+        os_.environ = {}
+        self.assertEqual(hookenv.remote_unit(), None)
+
     @patch('charmhelpers.core.hookenv.remote_unit')
     @patch('charmhelpers.core.hookenv.relation_get')
     def test_gets_relation_for_unit(self, relation_get, remote_unit):

=== modified file 'tests/core/test_services.py'
--- tests/core/test_services.py	2015-03-19 15:00:13 +0000
+++ tests/core/test_services.py	2015-04-29 13:11:45 +0000
@@ -27,6 +27,18 @@
                          'qux': 'baz'},
         })
 
+    def test_register_preserves_order(self):
+        service_list = [dict(service='a'), dict(service='b')]
+
+        # Test that the services list order is preserved by checking
+        # both forwards and backwards - only one of these will be
+        # dictionary order, and if both work we know order is being
+        # preserved.
+        manager = services.ServiceManager(service_list)
+        self.assertEqual(list(manager.services.keys()), ['a', 'b'])
+        manager = services.ServiceManager(reversed(service_list))
+        self.assertEqual(list(manager.services.keys()), ['b', 'a'])
+
     @mock.patch.object(services.ServiceManager, 'reconfigure_services')
     @mock.patch.object(services.ServiceManager, 'stop_services')
     @mock.patch.object(hookenv, 'hook_name')

=== modified file 'tests/core/test_templating.py'
--- tests/core/test_templating.py	2015-02-03 21:52:36 +0000
+++ tests/core/test_templating.py	2015-04-29 13:11:45 +0000
@@ -1,9 +1,9 @@
-import os
 import pkg_resources
 import shutil
 import tempfile
 import unittest
 import jinja2
+import os.path
 import pwd
 import grp
 
@@ -17,7 +17,8 @@
 class TestTemplating(unittest.TestCase):
     def setUp(self):
         self.charm_dir = pkg_resources.resource_filename(__name__, '')
-        self._charm_dir_patch = mock.patch.object(templating.hookenv, 'charm_dir')
+        self._charm_dir_patch = mock.patch.object(templating.hookenv,
+                                                  'charm_dir')
         self._charm_dir_mock = self._charm_dir_patch.start()
         self._charm_dir_mock.side_effect = lambda: self.charm_dir
 
@@ -28,9 +29,8 @@
     @mock.patch.object(templating.host, 'mkdir')
     @mock.patch.object(templating.host, 'log')
     def test_render(self, log, mkdir, fchown):
-        _, fn1 = tempfile.mkstemp()
-        _, fn2 = tempfile.mkstemp()
-        try:
+        with tempfile.NamedTemporaryFile() as fn1, \
+                tempfile.NamedTemporaryFile() as fn2:
             context = {
                 'nats': {
                     'port': '1234',
@@ -41,23 +41,19 @@
                 },
                 'nginx_port': 80,
             }
-            templating.render('fake_cc.yml', fn1,
+            templating.render('fake_cc.yml', fn1.name,
                               context, templates_dir=TEMPLATES_DIR)
-            contents = open(fn1).read()
+            contents = open(fn1.name).read()
             self.assertRegexpMatches(contents, 'port: 1234')
             self.assertRegexpMatches(contents, 'host: example.com')
             self.assertRegexpMatches(contents, 'domain: api.foo.com')
 
-            templating.render('test.conf', fn2, context,
+            templating.render('test.conf', fn2.name, context,
                               templates_dir=TEMPLATES_DIR)
-            contents = open(fn2).read()
+            contents = open(fn2.name).read()
             self.assertRegexpMatches(contents, 'listen 80')
             self.assertEqual(fchown.call_count, 2)
             self.assertEqual(mkdir.call_count, 2)
-        finally:
-            for fn in (fn1, fn2):
-                if os.path.exists(fn):
-                    os.remove(fn)
 
     @mock.patch.object(templating.host.os, 'fchown')
     @mock.patch.object(templating.host, 'log')
@@ -80,8 +76,10 @@
     @mock.patch.object(templating, 'hookenv')
     @mock.patch('jinja2.Environment')
     def test_load_error(self, Env, hookenv):
-        Env().get_template.side_effect = jinja2.exceptions.TemplateNotFound('fake_cc.yml')
+        Env().get_template.side_effect = jinja2.exceptions.TemplateNotFound(
+            'fake_cc.yml')
         self.assertRaises(
             jinja2.exceptions.TemplateNotFound, templating.render,
             'fake.src', 'fake.tgt', {}, templates_dir='tmpl')
-        hookenv.log.assert_called_once_with('Could not load template fake.src from tmpl.', level=hookenv.ERROR)
+        hookenv.log.assert_called_once_with(
+            'Could not load template fake.src from tmpl.', level=hookenv.ERROR)