ops/model.py: Proper support for status-get (#311)

The StatusBase types weren't being properly registered without
instantiating one. Now they are registered once they are declared,
rather than only after being created.
Further, actually parse the output of status-get properly, and the
application status output is structured differently from the unit status
output.

ops/model.py

@@ -713,14 +713,14 @@ class StatusBase:
     """
 
     _statuses = {}
+    name = None
 
     def __init__(self, message: str):
         self.message = message
 
     def __new__(cls, *args, **kwargs):
         if cls is StatusBase:
             raise TypeError("cannot instantiate a base class")
-        cls._statuses[cls.name] = cls
         return super().__new__(cls)
 
     def __eq__(self, other):
@@ -739,7 +739,15 @@ def from_name(cls, name: str, message: str):
         else:
             return cls._statuses[name](message)
 
+    @classmethod
+    def register(cls, child):
+        if child.name is None:
+            raise AttributeError('cannot register a Status which has no name')
+        cls._statuses[child.name] = child
+        return child
+
 
+@StatusBase.register
 class UnknownStatus(StatusBase):
     """The unit status is unknown.
 
@@ -757,6 +765,7 @@ def __repr__(self):
         return "UnknownStatus()"
 
 
+@StatusBase.register
 class ActiveStatus(StatusBase):
     """The unit is ready.
 
@@ -768,6 +777,7 @@ def __init__(self, message: str = ''):
         super().__init__(message)
 
 
+@StatusBase.register
 class BlockedStatus(StatusBase):
     """The unit requires manual intervention.
 
@@ -776,6 +786,7 @@ class BlockedStatus(StatusBase):
     name = 'blocked'
 
 
+@StatusBase.register
 class MaintenanceStatus(StatusBase):
     """The unit is performing maintenance tasks.
 
@@ -787,6 +798,7 @@ class MaintenanceStatus(StatusBase):
     name = 'maintenance'
 
 
+@StatusBase.register
 class WaitingStatus(StatusBase):
     """A unit is unable to progress.
 
@@ -1051,7 +1063,30 @@ def status_get(self, *, is_app=False):
             is_app: A boolean indicating whether the status should be retrieved for a unit
                 or an application.
         """
-        return self._run('status-get', '--include-data', '--application={}'.format(is_app))
+        content = self._run(
+            'status-get', '--include-data', '--application={}'.format(is_app),
+            use_json=True,
+            return_output=True)
+        # Unit status looks like (in YAML):
+        # message: 'load: 0.28 0.26 0.26'
+        # status: active
+        # status-data: {}
+        # Application status looks like (in YAML):
+        # application-status:
+        #   message: 'load: 0.28 0.26 0.26'
+        #   status: active
+        #   status-data: {}
+        #   units:
+        #     uo/0:
+        #       message: 'load: 0.28 0.26 0.26'
+        #       status: active
+        #       status-data: {}
+
+        if is_app:
+            return {'status': content['application-status']['status'],
+                    'message': content['application-status']['message']}
+        else:
+            return content
 
     def status_set(self, status, message='', *, is_app=False):
         """Set a status of a unit or an application.

test/charms/test_main/src/charm.py

@@ -88,6 +88,7 @@ def __init__(self, *args):
             self.framework.observe(self.on.start_action, self._on_start_action)
             self.framework.observe(self.on.foo_bar_action, self._on_foo_bar_action)
             self.framework.observe(self.on.get_model_name_action, self._on_get_model_name_action)
+            self.framework.observe(self.on.get_status_action, self._on_get_status_action)
 
         self.framework.observe(self.on.collect_metrics, self._on_collect_metrics)
 
@@ -184,6 +185,11 @@ def _on_foo_bar_action(self, event):
         self._state['observed_event_types'].append(type(event))
         self._write_state()
 
+    def _on_get_status_action(self, event):
+        self._state['status_name'] = self.unit.status.name
+        self._state['status_message'] = self.unit.status.message
+        self._write_state()
+
     def _on_collect_metrics(self, event):
         self._state['on_collect_metrics'].append(type(event))
         self._state['observed_event_types'].append(type(event))

test/test_lib.py

@@ -198,7 +198,7 @@ def test_incomplete(self):
 
     def test_too_long(self):
         """Check that if the file is too long, nothing is returned"""
-        m = self._mkmod('foo', '\n'*ops.lib._MAX_LIB_LINES + '''
+        m = self._mkmod('foo', '\n' * ops.lib._MAX_LIB_LINES + '''
         LIBNAME = "foo"
         LIBAPI = 2
         LIBPATCH = 42

test/test_main.py

@@ -161,7 +161,9 @@ def _prepare_actions(self):
 start:
     description: Start the unit.
 get-model-name:
-    description: Return the name of the unit
+    description: Return the name of the model
+get-status:
+    description: Return the Status of the unit
 '''
         actions_dir_name = 'actions'
         actions_meta_file = 'actions.yaml'
@@ -170,7 +172,7 @@ def _prepare_actions(self):
             f.write(actions_meta)
         actions_dir = self.JUJU_CHARM_DIR / actions_dir_name
         actions_dir.mkdir()
-        for action_name in ('start', 'foo-bar', 'get-model-name'):
+        for action_name in ('start', 'foo-bar', 'get-model-name', 'get-status'):
             self._setup_entry_point(actions_dir, action_name)
 
     def _read_and_clear_state(self):
@@ -513,6 +515,33 @@ def test_sets_model_name(self):
         self.assertIsNotNone(state)
         self.assertEqual(state['_on_get_model_name_action'], ['test-model-name'])
 
+    def test_has_valid_status(self):
+        self._prepare_actions()
+
+        actions_charm_config = base64.b64encode(pickle.dumps({
+            'STATE_FILE': self._state_file,
+            'USE_ACTIONS': True,
+        }))
+
+        fake_script(self, 'action-get', "echo '{}'")
+        fake_script(self, 'status-get', """echo '{"status": "unknown", "message": ""}'""")
+        state = self._simulate_event(EventSpec(
+            ActionEvent, 'get_status_action',
+            env_var='JUJU_ACTION_NAME',
+            charm_config=actions_charm_config))
+        self.assertIsNotNone(state)
+        self.assertEqual(state['status_name'], 'unknown')
+        self.assertEqual(state['status_message'], '')
+        fake_script(
+            self, 'status-get', """echo '{"status": "blocked", "message": "help meeee"}'""")
+        state = self._simulate_event(EventSpec(
+            ActionEvent, 'get_status_action',
+            env_var='JUJU_ACTION_NAME',
+            charm_config=actions_charm_config))
+        self.assertIsNotNone(state)
+        self.assertEqual(state['status_name'], 'blocked')
+        self.assertEqual(state['status_message'], 'help meeee')
+
 
 class TestMainWithNoDispatch(TestMain, unittest.TestCase):
     has_dispatch = False

test/test_model.py

@@ -13,12 +13,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from collections import OrderedDict
+import json
+import ipaddress
 import os
 import pathlib
+from textwrap import dedent
 import unittest
-import json
-import ipaddress
-from collections import OrderedDict
 
 import ops.model
 import ops.charm
@@ -482,6 +483,12 @@ def test_base_status_instance_raises(self):
         with self.assertRaises(TypeError):
             ops.model.StatusBase('test')
 
+        class NoNameStatus(ops.model.StatusBase):
+            pass
+
+        with self.assertRaises(AttributeError):
+            ops.model.StatusBase.register_status(NoNameStatus)
+
     def test_status_repr(self):
         test_cases = {
             "ActiveStatus('Seashell')": ops.model.ActiveStatus('Seashell'),
@@ -948,6 +955,39 @@ def test_relation_tool_errors(self):
                 run()
             self.assertEqual(fake_script_calls(self, clear=True), calls)
 
+    def test_status_get(self):
+        # taken from actual Juju output
+        content = '{"message": "", "status": "unknown", "status-data": {}}'
+        fake_script(self, 'status-get', "echo '{}'".format(content))
+        s = self.backend.status_get(is_app=False)
+        self.assertEqual(s['status'], "unknown")
+        self.assertEqual(s['message'], "")
+        # taken from actual Juju output
+        content = dedent("""
+            {
+                "application-status": {
+                    "message": "installing",
+                    "status": "maintenance",
+                    "status-data": {},
+                    "units": {
+                        "uo/0": {
+                            "message": "",
+                            "status": "active",
+                            "status-data": {}
+                        }
+                    }
+                }
+            }
+            """)
+        fake_script(self, 'status-get', "echo '{}'".format(content))
+        s = self.backend.status_get(is_app=True)
+        self.assertEqual(s['status'], "maintenance")
+        self.assertEqual(s['message'], "installing")
+        self.assertEqual(fake_script_calls(self, clear=True), [
+            ['status-get', '--include-data', '--application=False', '--format=json'],
+            ['status-get', '--include-data', '--application=True', '--format=json'],
+        ])
+
     def test_status_is_app_forced_kwargs(self):
         fake_script(self, 'status-get', 'exit 1')
         fake_script(self, 'status-set', 'exit 1')