Merge lp:~gnuoy/charm-helpers/status-set into lp:charm-helpers

[Liam Young]

[Stuart Bishop (stub)]

Looks good. Would have looked even better if I had seen it before repeating your work (see https://code.launchpad.net/~stub/charm-helpers/status-set/+merge/258618 ).

One minor issue - you need to add 'universal_newlines=True' to the subprocess.check_output command or you will get back a byte string under Python3.

I'm approving your work, pending the Py3 fix. However, before landing you might want to check out my branch and see if you prefer my approach. The major departure is that I chose to make status_set implicitly terminate on waiting and blocked, which also allowed me to put the unit into an error state as a fallback if the charm attempts to set blocked and status-set is unavailable. However, I just realized that this will not play well with Config's implicit saves so more work would be needed.

review: Approve

[Liam Young (gnuoy)]

Thanks for the review, I'll get that py3 fix added. I took a look at your branch and I'm not keen on having the exits in status_set tbh, I was planning to run status_set multiple times in the same hook.

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2015-04-07 16:12:51 +0000
+++ charmhelpers/core/hookenv.py	2015-05-07 17:36:13 +0000
@@ -605,3 +605,49 @@
 
     The results set by action_set are preserved."""
     subprocess.check_call(['action-fail', message])
+
+
+def status_set(workload_state, message):
+    """Set the workload state with a message
+
+    Use status-set to set the workload state with a message which is visible
+    to the user via juju status. If the status-set command is not found then
+    assume this is juju < 1.23 and juju-log the message unstead.
+
+    workload_state -- valid juju workload state.
+    message        -- status update message
+    """
+    valid_states = ['maintenance', 'blocked', 'waiting', 'active']
+    if workload_state not in valid_states:
+        raise ValueError(
+            '{!r} is not a valid workload state'.format(workload_state)
+        )
+    cmd = ['status-set', workload_state, message]
+    try:
+        ret = subprocess.call(cmd)
+        if ret == 0:
+            return
+    except OSError as e:
+        if e.errno != errno.ENOENT:
+            raise
+    log_message = 'status-set failed: {} {}'.format(workload_state,
+                                                    message)
+    log(log_message, level='INFO')
+
+
+def status_get():
+    """Retrieve the previously set juju workload state
+
+    If the status-set command is not found then assume this is juju < 1.23 and
+    return 'unknown'
+    """
+    cmd = ['status-get']
+    try:
+        raw_status = subprocess.check_output(cmd)
+        status = raw_status.rstrip()
+        return status
+    except OSError as e:
+        if e.errno == errno.ENOENT:
+            return 'unknown'
+        else:
+            raise

=== 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-05-07 17:36:13 +0000
@@ -1115,3 +1115,46 @@
         message = "Ooops, the action failed"
         hookenv.action_fail(message)
         check_call.assert_called_with(['action-fail', message])
+
+    def test_status_set_invalid_state(self):
+        self.assertRaises(ValueError, hookenv.status_set, 'random', 'message')
+
+    @patch('subprocess.call')
+    def test_status(self, call):
+        call.return_value = 0
+        hookenv.status_set('active', 'Everything is Awesome!')
+        call.assert_called_with(['status-set', 'active', 'Everything is Awesome!'])
+
+    @patch('subprocess.call')
+    @patch.object(hookenv, 'log')
+    def test_status_enoent(self, log, call):
+        call.side_effect = OSError(2, 'fail')
+        hookenv.status_set('active', 'Everything is Awesome!')
+        log.assert_called_with('status-set failed: active Everything is Awesome!', level='INFO')
+
+    @patch('subprocess.call')
+    @patch.object(hookenv, 'log')
+    def test_status_statuscmd_fail(self, log, call):
+        call.side_effect = OSError(3, 'fail')
+        self.assertRaises(OSError, hookenv.status_set, 'active', 'msg')
+        call.assert_called_with(['status-set', 'active', 'msg'])
+
+    @patch('subprocess.check_output')
+    def test_status_get(self, check_output):
+        check_output.return_value = 'active\n'
+        result = hookenv.status_get()
+        self.assertEqual(result, 'active')
+        check_output.assert_called_with(['status-get'])
+
+    @patch('subprocess.check_output')
+    def test_status_get_nostatus(self, check_output):
+        check_output.side_effect = OSError(2, 'fail')
+        result = hookenv.status_get()
+        self.assertEqual(result, 'unknown')
+        check_output.assert_called_with(['status-get'])
+
+    @patch('subprocess.check_output')
+    def test_status_get_status_error(self, check_output):
+        check_output.side_effect = OSError(3, 'fail')
+        self.assertRaises(OSError, hookenv.status_get)
+        check_output.assert_called_with(['status-get'])