Merge lp:~james-page/charm-helpers/is-relation-made into lp:charm-helpers

[James Page]

Add is_relation_made helper for deep testing of relation state.

For example, the ceph charm only passes an auth key to clients once
its bootstrapped; until then the client can do anything:

  if is_relation_made('ceph:0', 'key'):
     configure_and_do_stuff()
  else:
     don_t()

Unit tests as well.

Thanks for looking.

[James Page (james-page)]

I also tidied the majority of lint in the project;

make lint is a good check when reviewing merge proposals; don't think everyone has been doing that!

[lp:~james-page/charm-helpers/is-relation-made]

82. By James Page on 2013-10-20

Rebase against parent branch

83. By James Page on 2013-10-20

Rebase on parent

[Adam Gandelman (gandelman-a)]

This looks good, but any chance we can allow is_relation_made() to also take a list of keys? That way we could use the same for relations that require a bit more data, ie:

keys = ['password, 'db-host']
if is_relation_made('shared-db:0', settings):
    configure_db()

[Adam Gandelman (gandelman-a)]

Also, I think the current behavior of relation_get() will cause this to evalutate as True even if the relation setting has not been set:

if relation_get(key, rid=r_id, unit=unit):
   return True

Might be a good idea to also check whatever relation_get() returns != ''

review: Needs Fixing

[lp:~james-page/charm-helpers/is-relation-made]

84. By James Page on 2013-10-21

Add support for multiple key checks in is_relation_made

85. By James Page on 2013-10-21

Rework a bit for lists

[James Page (james-page)]

relation_get for a missing key should always return None so I think that is OK.

I updated to support lists as well as single strings as keys.

review: Needs Resubmitting

=== modified file 'charmhelpers/cli/host.py'
--- charmhelpers/cli/host.py	2013-06-20 15:03:24 +0000
+++ charmhelpers/cli/host.py	2013-10-21 22:54:42 +0000
@@ -7,6 +7,7 @@
     "List mounts"
     return host.mounts()
 
+
 @cmdline.subcommand_builder('service', description="Control system services")
 def service(subparser):
     subparser.add_argument("action", help="The action to perform (start, stop, etc...)")

=== modified file 'charmhelpers/contrib/ssl/__init__.py'
--- charmhelpers/contrib/ssl/__init__.py	2013-08-28 09:12:05 +0000
+++ charmhelpers/contrib/ssl/__init__.py	2013-10-21 22:54:42 +0000
@@ -34,7 +34,7 @@
         cmd = ["/usr/bin/openssl", "req", "-new", "-newkey",
                "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509",
                "-keyout", keyfile,
-               "-out",    certfile, "-config", config]
+               "-out", certfile, "-config", config]
     elif subject:
         ssl_subject = ""
         if "country" in subject:
@@ -50,8 +50,8 @@
         if "cn" in subject:
             ssl_subject = ssl_subject + "/CN={}".format(subject["cn"])
         else:
-            hookenv.log("When using \"subject\" argument you must " \
-                "provide \"cn\" field at very least")
+            hookenv.log("When using \"subject\" argument you must "
+                        "provide \"cn\" field at very least")
             return False
         if "email" in subject:
             ssl_subject = ssl_subject + "/emailAddress={}".format(subject["email"])
@@ -59,16 +59,16 @@
         cmd = ["/usr/bin/openssl", "req", "-new", "-newkey",
                "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509",
                "-keyout", keyfile,
-               "-out",    certfile, "-subj", ssl_subject]
+               "-out", certfile, "-subj", ssl_subject]
     elif cn:
         cmd = ["/usr/bin/openssl", "req", "-new", "-newkey",
                "rsa:{}".format(keysize), "-days", "365", "-nodes", "-x509",
                "-keyout", keyfile,
-               "-out",    certfile, "-subj", "/CN={}".format(cn)]
+               "-out", certfile, "-subj", "/CN={}".format(cn)]
 
     if not cmd:
-        hookenv.log("No config, subject or cn provided," \
-                "unable to generate self signed SSL certificates")
+        hookenv.log("No config, subject or cn provided,"
+                    "unable to generate self signed SSL certificates")
         return False
     try:
         subprocess.check_call(cmd)
@@ -76,4 +76,3 @@
     except Exception as e:
         print "Execution of openssl command failed:\n{}".format(e)
         return False
-

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2013-10-21 08:34:03 +0000
+++ charmhelpers/core/hookenv.py	2013-10-21 22:54:42 +0000
@@ -285,6 +285,26 @@
     return rels
 
 
+@cached
+def is_relation_made(relation, keys='private-address'):
+    '''
+    Determine whether a relation is established by checking for
+    presence of key(s).  If a list of keys is provided, they
+    must all be present for the relation to be identified as made
+    '''
+    if isinstance(keys, str):
+        keys = [keys]
+    for r_id in relation_ids(relation):
+        for unit in related_units(r_id):
+            context = {}
+            for k in keys:
+                context[k] = relation_get(k, rid=r_id,
+                                          unit=unit)
+            if None not in context.values():
+                return True
+    return False
+
+
 def open_port(port, protocol="TCP"):
     """Open a service network port"""
     _args = ['open-port']

=== modified file 'charmhelpers/fetch/bzrurl.py'
--- charmhelpers/fetch/bzrurl.py	2013-08-22 10:19:51 +0000
+++ charmhelpers/fetch/bzrurl.py	2013-10-21 22:54:42 +0000
@@ -12,6 +12,7 @@
     apt_install("python-bzrlib")
     from bzrlib.branch import Branch
 
+
 class BzrUrlFetchHandler(BaseFetchHandler):
     """Handler for bazaar branches via generic and lp URLs"""
     def can_handle(self, source):
@@ -46,4 +47,3 @@
         except OSError as e:
             raise UnhandledSource(e.strerror)
         return dest_dir
-

=== modified file 'tests/contrib/ssl/test_ssl.py'
--- tests/contrib/ssl/test_ssl.py	2013-08-27 17:08:44 +0000
+++ tests/contrib/ssl/test_ssl.py	2013-10-21 22:54:42 +0000
@@ -1,6 +1,4 @@
-import subprocess
-
-from mock import patch, call, MagicMock
+from mock import patch
 from testtools import TestCase
 
 from charmhelpers.contrib import ssl
@@ -39,7 +37,6 @@
         result = ssl.generate_selfsigned("mykey.key", "mycert.crt", subject=subject)
         self.assertFalse(result)
 
-
     @patch('subprocess.check_call')
     def test_generate_selfsigned_file(self, mock_call):
         ssl.generate_selfsigned("mykey.key", "mycert.crt", config="test.cnf")
@@ -49,7 +46,6 @@
                                       'mykey.key', '-out', 'mycert.crt',
                                       '-config', 'test.cnf'])
 
-
     @patch('subprocess.check_call')
     def test_generate_selfsigned_cn_key(self, mock_call):
         ssl.generate_selfsigned("mykey.key", "mycert.crt", keysize="2048", cn="mysite.example.com")

=== modified file 'tests/core/test_hookenv.py'
--- tests/core/test_hookenv.py	2013-10-21 08:34:03 +0000
+++ tests/core/test_hookenv.py	2013-10-21 22:54:42 +0000
@@ -498,6 +498,85 @@
             },
         })
 
+    @patch('charmhelpers.core.hookenv.relation_ids')
+    @patch('charmhelpers.core.hookenv.related_units')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    def test_is_relation_made(self, relation_get, related_units,
+                              relation_ids):
+        relation_get.return_value = 'hostname'
+        related_units.return_value = ['test/1']
+        relation_ids.return_value = ['test:0']
+        self.assertTrue(hookenv.is_relation_made('test'))
+        relation_get.assert_called_with('private-address',
+                                        rid='test:0', unit='test/1')
+
+    @patch('charmhelpers.core.hookenv.relation_ids')
+    @patch('charmhelpers.core.hookenv.related_units')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    def test_is_relation_made_multi_unit(self, relation_get, related_units,
+                                         relation_ids):
+        relation_get.side_effect = [None, 'hostname']
+        related_units.return_value = ['test/1', 'test/2']
+        relation_ids.return_value = ['test:0']
+        self.assertTrue(hookenv.is_relation_made('test'))
+
+    @patch('charmhelpers.core.hookenv.relation_ids')
+    @patch('charmhelpers.core.hookenv.related_units')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    def test_is_relation_made_different_key(self,
+                                            relation_get, related_units,
+                                            relation_ids):
+        relation_get.return_value = 'hostname'
+        related_units.return_value = ['test/1']
+        relation_ids.return_value = ['test:0']
+        self.assertTrue(hookenv.is_relation_made('test', keys='auth'))
+        relation_get.assert_called_with('auth',
+                                        rid='test:0', unit='test/1')
+
+    @patch('charmhelpers.core.hookenv.relation_ids')
+    @patch('charmhelpers.core.hookenv.related_units')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    def test_is_relation_made_multiple_keys(self,
+                                            relation_get, related_units,
+                                            relation_ids):
+        relation_get.side_effect = ['password', 'hostname']
+        related_units.return_value = ['test/1']
+        relation_ids.return_value = ['test:0']
+        self.assertTrue(hookenv.is_relation_made('test',
+                                                 keys=['auth', 'host']))
+        relation_get.assert_has_calls(
+            [call('auth', rid='test:0', unit='test/1'),
+             call('host', rid='test:0', unit='test/1')]
+        )
+
+    @patch('charmhelpers.core.hookenv.relation_ids')
+    @patch('charmhelpers.core.hookenv.related_units')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    def test_is_relation_made_not_made(self,
+                                       relation_get, related_units,
+                                       relation_ids):
+        relation_get.return_value = None
+        related_units.return_value = ['test/1']
+        relation_ids.return_value = ['test:0']
+        self.assertFalse(hookenv.is_relation_made('test'))
+
+    @patch('charmhelpers.core.hookenv.relation_ids')
+    @patch('charmhelpers.core.hookenv.related_units')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    def test_is_relation_made_not_made_multiple_keys(self,
+                                                     relation_get,
+                                                     related_units,
+                                                     relation_ids):
+        relation_get.side_effect = ['password', None]
+        related_units.return_value = ['test/1']
+        relation_ids.return_value = ['test:0']
+        self.assertFalse(hookenv.is_relation_made('test',
+                                                  keys=['auth', 'host']))
+        relation_get.assert_has_calls(
+            [call('auth', rid='test:0', unit='test/1'),
+             call('host', rid='test:0', unit='test/1')]
+        )
+
     @patch('charmhelpers.core.hookenv.config')
     @patch('charmhelpers.core.hookenv.relation_type')
     @patch('charmhelpers.core.hookenv.local_unit')

=== modified file 'tests/fetch/test_bzrurl.py'
--- tests/fetch/test_bzrurl.py	2013-08-21 11:45:37 +0000
+++ tests/fetch/test_bzrurl.py	2013-10-21 22:54:42 +0000
@@ -41,7 +41,6 @@
         )
         self.fh = bzrurl.BzrUrlFetchHandler()
 
-
     def test_handles_bzr_urls(self):
         for url in self.valid_urls:
             result = self.fh.can_handle(url)
@@ -50,7 +49,6 @@
             result = self.fh.can_handle(url)
             self.assertNotEqual(result, True, url)
 
-
     @patch('bzrlib.branch.Branch.open')
     def test_branch(self, _open):
         dest_path = "/destination/path"
@@ -65,7 +63,6 @@
             with patch.dict('os.environ', {'CHARM_DIR': 'foo'}):
                 self.assertRaises(UnhandledSource, self.fh.branch, url, dest_path)
 
-
     @patch('charmhelpers.fetch.bzrurl.mkdir')
     def test_installs(self, _mkdir):
         self.fh.branch = MagicMock()
@@ -77,4 +74,3 @@
                 where = self.fh.install(url)
             self.assertEqual(where, dest)
             _mkdir.assert_called_with(where, perms=0755)
-