Merge lp:~axwalk/charm-helpers/hookenv-storage into lp:charm-helpers

[Andrew Wilkins]

This MP adds two new functions to hookenv: storage_list and storage_get. These will, respectively, list and get details of the unit's storage attachments.

storage_list will return [] of the storage-list hook tool cannot be found in $PATH. storage_get does not filter exceptions, because there's no reason to use it unless you're in a storage hook, or you've called it with the results of storage_list. As such, the following sort of code:
    for s in storage_list():
        storage_get(storage_id=s)
will work regardless of the version of Juju.

[Tim Van Steenburgh (tvansteenburgh)]

The last three tests all do:

    self.assertEqual(result, result)

...which will always be true. Presumably an oversight, but one of those 'result' vars needs to be renamed in each test.

review: Needs Fixing

[lp:~axwalk/charm-helpers/hookenv-storage]

456. By Andrew Wilkins on 2015-09-24

Fix tautological tests

[Andrew Wilkins (axwalk)]

> The last three tests all do:
>
> self.assertEqual(result, result)
>
> ...which will always be true. Presumably an oversight, but one of those
> 'result' vars needs to be renamed in each test.

Wow, sorry about that :/
Fixed.

[Tim Van Steenburgh (tvansteenburgh)]

+1 LGTM. Bonus point for using "tautological" in a commit message.

review: Approve

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2015-08-26 17:32:36 +0000
+++ charmhelpers/core/hookenv.py	2015-09-24 01:29:26 +0000
@@ -623,6 +623,38 @@
     return unit_get('private-address')
 
 
+@cached
+def storage_get(attribute="", storage_id=""):
+    """Get storage attributes"""
+    _args = ['storage-get', '--format=json']
+    if storage_id:
+        _args.extend(('-s', storage_id))
+    if attribute:
+        _args.append(attribute)
+    try:
+        return json.loads(subprocess.check_output(_args).decode('UTF-8'))
+    except ValueError:
+        return None
+
+
+@cached
+def storage_list(storage_name=""):
+    """List the storage IDs for the unit"""
+    _args = ['storage-list', '--format=json']
+    if storage_name:
+        _args.append(storage_name)
+    try:
+        return json.loads(subprocess.check_output(_args).decode('UTF-8'))
+    except ValueError:
+        return None
+    except OSError as e:
+        import errno
+        if e.errno == errno.ENOENT:
+            # storage-list does not exist
+            return []
+        raise
+
+
 class UnregisteredHookError(Exception):
     """Raised when an undefined hook is called"""
     pass

=== modified file 'tests/core/test_hookenv.py'
--- tests/core/test_hookenv.py	2015-08-26 17:32:36 +0000
+++ tests/core/test_hookenv.py	2015-09-24 01:29:26 +0000
@@ -1500,3 +1500,79 @@
     def test_action_tag(self):
         with patch.dict('os.environ', JUJU_ACTION_TAG='action-jack'):
             self.assertEqual(hookenv.action_tag(), 'action-jack')
+
+    @patch('subprocess.check_output')
+    def test_storage_list(self, check_output):
+        ids = ['data/0', 'data/1', 'data/2']
+        check_output.return_value = json.dumps(ids).encode('UTF-8')
+
+        storage_name = 'arbitrary'
+        result = hookenv.storage_list(storage_name)
+
+        self.assertEqual(result, ids)
+        check_output.assert_called_with(['storage-list', '--format=json',
+                                         storage_name])
+
+    @patch('subprocess.check_output')
+    def test_storage_list_notexist(self, check_output):
+        import errno
+        e = OSError()
+        e.errno = errno.ENOENT
+        check_output.side_effect = e
+
+        result = hookenv.storage_list()
+
+        self.assertEqual(result, [])
+        check_output.assert_called_with(['storage-list', '--format=json'])
+
+    @patch('subprocess.check_output')
+    def test_storage_get_notexist(self, check_output):
+        # storage_get does not catch ENOENT, because there's no reason why you
+        # should be calling storage_get except from a storage hook, or with
+        # the result of storage_list (which will return [] as shown above).
+
+        import errno
+        e = OSError()
+        e.errno = errno.ENOENT
+        check_output.side_effect = e
+        self.assertRaises(OSError, hookenv.storage_get)
+
+    @patch('subprocess.check_output')
+    def test_storage_get(self, check_output):
+        expect = {
+            'location': '/dev/sda',
+            'kind': 'block',
+        }
+        check_output.return_value = json.dumps(expect).encode('UTF-8')
+
+        result = hookenv.storage_get()
+
+        self.assertEqual(result, expect)
+        check_output.assert_called_with(['storage-get', '--format=json'])
+
+    @patch('subprocess.check_output')
+    def test_storage_get_attr(self, check_output):
+        expect = '/dev/sda'
+        check_output.return_value = json.dumps(expect).encode('UTF-8')
+
+        attribute = 'location'
+        result = hookenv.storage_get(attribute)
+
+        self.assertEqual(result, expect)
+        check_output.assert_called_with(['storage-get', '--format=json',
+                                         attribute])
+
+    @patch('subprocess.check_output')
+    def test_storage_get_with_id(self, check_output):
+        expect = {
+            'location': '/dev/sda',
+            'kind': 'block',
+        }
+        check_output.return_value = json.dumps(expect).encode('UTF-8')
+
+        storage_id = 'data/0'
+        result = hookenv.storage_get(storage_id=storage_id)
+
+        self.assertEqual(result, expect)
+        check_output.assert_called_with(['storage-get', '--format=json',
+                                         '-s', storage_id])