Merge lp:~thedac/charm-helpers/openstack-workload-status into lp:charm-helpers

[David Ames]

Workload status setting decorator and functions based on openstack contexts for openstack charms

Workload status setting decorator and functions based on openstack contexts for openstack charms

[lp:~thedac/charm-helpers/openstack-workload-status]

402. By David Ames on 2015-07-10

Use functools wraps, which fixes scoping problems with multiple decorators

403. By David Ames on 2015-07-13

Adapt status_get to allow for --include-data and service
Add status_compound to "add" together statuses
Tests for new features

404. By David Ames on 2015-07-15

status_get should always return a dictionary and include-data
Log status change on status set

[James Page (james-page)]

David

I think we need to differentiate between blocked and waiting to provide granualarity to end users - some suggestions in the diff.

Cheers

James

review: Needs Fixing

Unmerged revisions

404. By David Ames on 2015-07-15

status_get should always return a dictionary and include-data
Log status change on status set

403. By David Ames on 2015-07-13

Adapt status_get to allow for --include-data and service
Add status_compound to "add" together statuses
Tests for new features

=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- charmhelpers/contrib/openstack/utils.py	2015-06-30 15:28:49 +0000
+++ charmhelpers/contrib/openstack/utils.py	2015-07-15 14:48:13 +0000
@@ -40,7 +40,8 @@
     charm_dir,
     INFO,
     relation_ids,
-    relation_set
+    relation_set,
+    status_set
 )
 
 from charmhelpers.contrib.storage.linux.lvm import (
@@ -703,3 +704,63 @@
         return projects[key]
 
     return None
+
+
+def context_status(configs, required_interfaces):
+    """
+    Decorator to set workload status based on complete contexts
+    after contexts have been acted on
+    """
+    def wrap(f):
+        @wraps(f)
+        def wrapped_f(*args, **kwargs):
+            # Run the original function first
+            f(*args, **kwargs)
+            # Set workload status now that contexts have been
+            # acted on
+            set_context_status(configs, required_interfaces)
+        return wrapped_f
+    return wrap
+
+
+def set_context_status(configs, required_interfaces):
+    """
+    Set workload status based on complete contexts
+    Left outside of the decorator, context_status,
+    to allow manual use
+    """
+    incomplete_ctxts = incomplete_contexts(configs, required_interfaces)
+    if incomplete_ctxts:
+        status_set(
+            'blocked',
+            '{} contexts are absent or incomplete'
+            ''.format(', '.join(incomplete_ctxts))
+        )
+    else:
+        status_set('active', 'Contexts are present and complete')
+
+
+def incomplete_contexts(configs, required_interfaces):
+    """
+    Check complete contexts against required_interfaces
+    Return list of incomplete contexts
+
+    configs is an OSConfigRenderer object with configs registered
+
+    required_interfaces is a dictionary of required general interfaces
+    with list values of possible specific interfaces.
+    The interface is said to be satisfied if anyone of the interfaces in the
+    list has a complete context.
+    Example:
+    required_interfaces = {'database': ['shared-db', 'pgsql-db']}
+    """
+    complete_ctxts = configs.complete_contexts()
+    incomplete_contexts = []
+    for svc_type in required_interfaces.keys():
+        found_ctxt = False
+        for interface in required_interfaces[svc_type]:
+            if interface in complete_ctxts:
+                found_ctxt = True
+        if not found_ctxt:
+            incomplete_contexts.append(svc_type)
+    return incomplete_contexts

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2015-06-30 15:28:49 +0000
+++ charmhelpers/core/hookenv.py	2015-07-15 14:48:13 +0000
@@ -660,35 +660,103 @@
         )
     cmd = ['status-set', workload_state, message]
     try:
-        ret = subprocess.call(cmd)
-        if ret == 0:
+        subprocess.call(cmd)
+    except OSError as e:
+        if e.errno == errno.ENOENT:
+            log_message = 'status-set failed: {} {}'.format(workload_state,
+                                                            message)
+            log(log_message, level='INFO')
             return
-    except OSError as e:
-        if e.errno != errno.ENOENT:
+        else:
             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
+    # Log the status change
+    if workload_state == 'blocked' or workload_state == 'waiting':
+        lvl = 'WARN'
+    else:
+        lvl = 'INFO'
+    log('{}: {}'.format(workload_state, message), lvl)
+
+
+def status_get(include_data=True, service=False):
+    """Retrieve the previously set juju workload state and return as a
+    dictionary.
+
+    If service is set return status for all units of this service if this unit
+    is the leader
 
     If the status-set command is not found then assume this is juju < 1.23 and
     return 'unknown'
     """
-    cmd = ['status-get']
+    cmd = ['status-get', '--format', 'json']
+    if include_data:
+        cmd.append('--include-data')
+    if service:
+        cmd.append('--service')
     try:
-        raw_status = subprocess.check_output(cmd, universal_newlines=True)
-        status = raw_status.rstrip()
-        return status
+        return json.loads(subprocess.check_output(cmd,
+                                                  universal_newlines=True))
     except OSError as e:
         if e.errno == errno.ENOENT:
-            return 'unknown'
+            return {'status': 'unknown'}
         else:
             raise
 
 
+def status_compound(current_status, workload_state, message):
+    """Compound statuses: Take the current status as a dictionary
+    and new workload state and message then status_set with the highest
+    severity workload state and a compound message
+    Example:
+      status_compound(status_get(include-data=True),
+                      'waiting',
+                      'waiting on relations data')
+    """
+    hierarchy = {'unknown': -1,
+                 'active': 0,
+                 'maintenance': 1,
+                 'waiting': 2,
+                 'blocked': 3,
+                 }
+    current_workload_state = current_status.get('status')
+    current_message = current_status.get('message')
+
+    # Do not set unknown or other invalid state
+    if hierarchy.get(workload_state) is None:
+        return {}
+    elif hierarchy.get(workload_state) < 0:
+        return {}
+    if hierarchy.get(current_workload_state) is None:
+        current_workload_state = 'unknown'
+        current_message = None
+    elif hierarchy.get(current_workload_state) < 0:
+        current_message = None
+
+    # New state of 'active' overrides all others
+    if workload_state == 'active':
+        hierarchy['active'] = 10
+        current_message = None
+
+    # Set workload_state based on hierarchy of statuses
+    if hierarchy.get(current_workload_state) > hierarchy.get(workload_state):
+        workload_state = current_workload_state
+    else:
+        if current_workload_state == 'active':
+            current_message = None
+
+    # Compound the message
+    if current_message and message:
+        message = message + "; " + current_message
+    elif current_message and not message:
+        message = current_message
+
+    # Set new status
+    if workload_state:
+        status_set(workload_state, message)
+
+    # Return new status
+    return {'status': workload_state, 'message': message}
+
+
 def translate_exc(from_exc, to_exc):
     def inner_translate_exc1(f):
         def inner_translate_exc2(*args, **kwargs):

=== modified file 'tests/contrib/openstack/test_openstack_utils.py'
--- tests/contrib/openstack/test_openstack_utils.py	2015-07-14 18:12:04 +0000
+++ tests/contrib/openstack/test_openstack_utils.py	2015-07-15 14:48:13 +0000
@@ -824,5 +824,48 @@
         openstack.git_src_dir(openstack_origin_git, 'keystone')
         join.assert_called_with('/mnt/openstack-git', 'keystone')
 
+    def test_incomplete_contexts(self):
+        configs = MagicMock()
+        configs.complete_contexts.return_value = ['pgsql-db', 'amqp']
+        required_interfaces = {
+            'database': ['shared-db', 'pgsql-db'],
+            'message': ['amqp', 'zeromq-configuration'],
+            'identity': ['identity-service']}
+        expected_result = ['identity']
+
+        self.assertEquals(openstack.incomplete_contexts(configs,
+                                                        required_interfaces),
+                          expected_result)
+
+    @patch('charmhelpers.contrib.openstack.utils.status_set')
+    def test_set_context_status_complete(self, status_set):
+        configs = MagicMock()
+        configs.complete_contexts.return_value = ['shared-db',
+                                                  'amqp',
+                                                  'identity-service']
+        required_interfaces = {
+            'database': ['shared-db', 'pgsql-db'],
+            'message': ['amqp', 'zeromq-configuration'],
+            'identity': ['identity-service']}
+
+        openstack.set_context_status(configs, required_interfaces)
+        status_set.assert_called_with('active',
+                                      'Contexts are present and complete')
+
+    @patch('charmhelpers.contrib.openstack.utils.status_set')
+    def test_set_context_status_incomplete(self, status_set):
+        configs = MagicMock()
+        configs.complete_contexts.return_value = ['shared-db', 'amqp']
+        required_interfaces = {
+            'database': ['shared-db', 'pgsql-db'],
+            'message': ['amqp', 'zeromq-configuration'],
+            'identity': ['identity-service']}
+
+        openstack.set_context_status(configs, required_interfaces)
+        status_set.assert_called_with('blocked',
+                                      'identity contexts are absent '
+                                      'or incomplete')
+
+
 if __name__ == '__main__':
     unittest.main()

=== modified file 'tests/core/test_hookenv.py'
--- tests/core/test_hookenv.py	2015-06-26 14:17:43 +0000
+++ tests/core/test_hookenv.py	2015-07-15 14:48:13 +0000
@@ -1293,17 +1293,21 @@
         self.assertRaises(ValueError, hookenv.status_set, 'random', 'message')
 
     @patch('subprocess.call')
-    def test_status(self, call):
-        call.return_value = 0
+    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!'])
+        calls = [call(['status-set', 'active', 'Everything is Awesome!']),
+                 call(['juju-log', '-l', 'INFO',
+                       'active: Everything is Awesome!'])]
+        _call.assert_has_calls(calls, any_order=True)
 
     @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')
+        log.assert_called_with('status-set failed: active Everything is Awesome!',
+                               level='INFO')
 
     @patch('subprocess.call')
     @patch.object(hookenv, 'log')
@@ -1314,23 +1318,148 @@
 
     @patch('subprocess.check_output')
     def test_status_get(self, check_output):
-        check_output.return_value = 'active\n'
+        check_output.return_value = json.dumps({'status': 'active'})
         result = hookenv.status_get()
-        self.assertEqual(result, 'active')
-        check_output.assert_called_with(['status-get'], universal_newlines=True)
+        self.assertEqual(result, {'status': 'active'})
+        check_output.assert_called_with(['status-get',
+                                         '--format',
+                                         'json',
+                                         '--include-data'],
+                                        universal_newlines=True)
+
+    @patch('subprocess.check_output')
+    def test_status_get_include_data_false(self, check_output):
+        return_dict = {"status": "active"}
+        check_output.return_value = json.dumps(return_dict)
+        result = hookenv.status_get(include_data=False)
+        self.assertEqual(return_dict, result)
+        check_output.assert_called_with(['status-get',
+                                         '--format', 'json'],
+                                        universal_newlines=True)
+
+    @patch('subprocess.check_output')
+    def test_status_get_service(self, check_output):
+        return_dict = {"service-status":
+                       {"message": "all good",
+                        "status": "active",
+                        "status-data": {},
+                        "units": {"test/0":
+                                  {"message": "all good",
+                                   "status": "active",
+                                   "status-data": {}
+                                   }
+                                  }
+                        }
+                       }
+
+        check_output.return_value = json.dumps(return_dict)
+        result = hookenv.status_get(service=True)
+        self.assertEqual(return_dict, result)
+        check_output.assert_called_with(['status-get',
+                                         '--format', 'json',
+                                         '--include-data',
+                                         '--service'],
+                                        universal_newlines=True)
 
     @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'], universal_newlines=True)
+        self.assertEqual(result, {'status': 'unknown'})
+        check_output.assert_called_with(['status-get',
+                                         '--format', 'json',
+                                         '--include-data'],
+                                        universal_newlines=True)
 
     @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'], universal_newlines=True)
+        check_output.assert_called_with(['status-get',
+                                         '--format', 'json',
+                                         '--include-data'],
+                                        universal_newlines=True)
+
+    @patch('charmhelpers.core.hookenv.status_set')
+    def test_status_compound_active_to_worse(self, status_set):
+        current_status = {"status": "active", "message": "all good"}
+        new_state = "blocked"
+        new_message = "blocked on bad things"
+        hookenv.status_compound(current_status, new_state, new_message)
+        status_set.assert_called_with(new_state, new_message)
+
+    @patch('charmhelpers.core.hookenv.status_set')
+    def test_status_compound_bad_to_active(self, status_set):
+        current_status = {"status": "blocked", "message": "blocked on bad things"}
+        new_state = "active"
+        new_message = "all good"
+        hookenv.status_compound(current_status, new_state, new_message)
+        status_set.assert_called_with(new_state, new_message)
+
+    @patch('charmhelpers.core.hookenv.status_set')
+    def test_status_compound_bad_to_worse(self, status_set):
+        current_status = {"status": "waiting", "message": "waiting on value"}
+        new_state = "blocked"
+        new_message = "blocked on bad things"
+        hookenv.status_compound(current_status, new_state, new_message)
+        status_set.assert_called_with(new_state,
+                                      "{}; {}"
+                                      "".format(new_message,
+                                                current_status['message']))
+
+    @patch('charmhelpers.core.hookenv.status_set')
+    def test_status_compound_bad_to_better(self, status_set):
+        current_status = {"status": "waiting", "message": "waiting on value"}
+        new_state = "maintenance"
+        new_message = "maintaing things"
+        hookenv.status_compound(current_status, new_state, new_message)
+        status_set.assert_called_with(current_status["status"],
+                                      "{}; {}"
+                                      "".format(new_message,
+                                                current_status['message']))
+
+    @patch('charmhelpers.core.hookenv.status_set')
+    def test_status_compound_equal(self, status_set):
+        current_status = {"status": "waiting", "message": "waiting on value"}
+        new_state = "waiting"
+        new_message = "waiting on new things"
+        hookenv.status_compound(current_status, new_state, new_message)
+        status_set.assert_called_with(new_state,
+                                      "{}; {}"
+                                      "".format(new_message,
+                                                current_status['message']))
+
+    @patch('charmhelpers.core.hookenv.status_set')
+    def test_status_compound_unknown_to_any(self, status_set):
+        current_status = {"status": "unknown", "message": "Bogus message"}
+        new_state = "waiting"
+        new_message = "waiting on things"
+        hookenv.status_compound(current_status, new_state, new_message)
+        status_set.assert_called_with(new_state, new_message)
+
+    @patch('charmhelpers.core.hookenv.status_set')
+    def test_status_compound_any_to_unknown(self, status_set):
+        current_status = {"status": "waiting", "message": "waiting on value"}
+        new_state = "unknown"
+        new_message = "Bogus message"
+        hookenv.status_compound(current_status, new_state, new_message)
+        assert not status_set.called
+
+    @patch('charmhelpers.core.hookenv.status_set')
+    def test_status_compound_invalid(self, status_set):
+        current_status = {"status": "waiting", "message": "waiting on value"}
+        new_state = "invalid"
+        new_message = "Bogus message"
+        hookenv.status_compound(current_status, new_state, new_message)
+        assert not status_set.called
+
+    @patch('charmhelpers.core.hookenv.status_set')
+    def test_status_compound_none_to_any(self, status_set):
+        current_status = {}
+        new_state = "waiting"
+        new_message = "waiting on things"
+        hookenv.status_compound(current_status, new_state, new_message)
+        status_set.assert_called_with(new_state, new_message)
 
     @patch('subprocess.check_output')
     @patch('glob.glob')