[tinwood,r=thedac] Fixes Bug#1526511 change pause/resume actions use (new) assess_status()

actions/actions.py

@@ -4,9 +4,11 @@ import sys
 import os
 
 from charmhelpers.core.host import service_pause, service_resume
-from charmhelpers.core.hookenv import action_fail, status_set
+from charmhelpers.core.hookenv import action_fail
+from charmhelpers.core.unitdata import HookData, kv
 
-from hooks.keystone_utils import services
+from hooks.keystone_utils import services, assess_status
+from hooks.keystone_hooks import CONFIGS
 
 
 def pause(args):
@@ -18,8 +20,9 @@ def pause(args):
         stopped = service_pause(service)
         if not stopped:
             raise Exception("{} didn't stop cleanly.".format(service))
-    status_set(
+    with HookData()():
-        "maintenance", "Paused. Use 'resume' action to resume normal service.")
+        kv().set('unit-paused', True)
+    assess_status(CONFIGS)
 
 
 def resume(args):
@@ -31,7 +34,9 @@ def resume(args):
         started = service_resume(service)
         if not started:
             raise Exception("{} didn't start cleanly.".format(service))
-    status_set("active", "")
+    with HookData()():
+        kv().set('unit-paused', False)
+    assess_status(CONFIGS)
 
 
 # A dictionary of all the defined actions to callables (which take

hooks/keystone_hooks.py

@@ -47,7 +47,6 @@ from charmhelpers.contrib.openstack.utils import (
     git_install_requested,
     openstack_upgrade_available,
     sync_db_with_multi_ipv6_addresses,
-    set_os_workload_status,
 )
 
 from keystone_utils import (
@@ -81,11 +80,10 @@ from keystone_utils import (
     force_ssl_sync,
     filter_null,
     ensure_ssl_dirs,
-    REQUIRED_INTERFACES,
-    check_optional_relations,
     ensure_pki_cert_paths,
     is_service_present,
     delete_service_entry,
+    assess_status,
 )
 
 from charmhelpers.contrib.hahelpers.cluster import (
@@ -646,8 +644,7 @@ def main():
         hooks.execute(sys.argv)
     except UnregisteredHookError as e:
         log('Unknown hook {} - skipping.'.format(e))
-    set_os_workload_status(CONFIGS, REQUIRED_INTERFACES,
+    assess_status(CONFIGS)
-                           charm_func=check_optional_relations)
 
 
 if __name__ == '__main__':

hooks/keystone_utils.py

@@ -83,6 +83,7 @@ from charmhelpers.core.hookenv import (
     INFO,
     WARNING,
     status_get,
+    status_set,
 )
 
 from charmhelpers.fetch import (
@@ -116,6 +117,12 @@ from charmhelpers.core.templating import render
 import keystone_context
 import keystone_ssl as ssl
 
+from charmhelpers.core.unitdata import (
+    HookData,
+    kv,
+)
+
+
 TEMPLATES = 'templates/'
 
 # removed from original: charm-helper-sh
@@ -1829,3 +1836,31 @@ def check_optional_relations(configs):
         return status_get()
     else:
         return 'unknown', 'No optional relations'
+
+
+def is_paused(status_get=status_get):
+    """Is the unit paused?"""
+    with HookData()():
+        if kv().get('unit-paused'):
+            return True
+        else:
+            return False
+
+
+def assess_status(configs):
+    """Assess status of current unit
+
+    Decides what the state of the unit should be based on the current
+    configuration.
+
+    @param configs: a templating.OSConfigRenderer() object
+    """
+
+    if is_paused():
+        status_set("maintenance",
+                   "Paused. Use 'resume' action to resume normal service.")
+        return
+
+    # set the status according to the current state of the contexts
+    set_os_workload_status(
+        configs, REQUIRED_INTERFACES, charm_func=check_optional_relations)

unit_tests/test_actions.py

@@ -1,15 +1,19 @@
 import mock
+from mock import patch
 
 from test_utils import CharmTestCase
 
-import actions.actions
+with patch('actions.hooks.keystone_utils.is_paused') as is_paused:
+    with patch('actions.hooks.keystone_utils.register_configs') as configs:
+        import actions.actions
 
 
 class PauseTestCase(CharmTestCase):
 
     def setUp(self):
         super(PauseTestCase, self).setUp(
-            actions.actions, ["service_pause", "status_set"])
+            actions.actions, ["service_pause", "HookData", "kv",
+                              "assess_status"])
 
     def test_pauses_services(self):
         """Pause action pauses all Keystone services."""
@@ -22,7 +26,8 @@ class PauseTestCase(CharmTestCase):
         self.service_pause.side_effect = fake_service_pause
 
         actions.actions.pause([])
-        self.assertEqual(pause_calls, ['haproxy', 'keystone', 'apache2'])
+        self.assertItemsEqual(
+            pause_calls, ['haproxy', 'keystone', 'apache2'])
 
     def test_bails_out_early_on_error(self):
         """Pause action fails early if there are errors stopping a service."""
@@ -41,32 +46,20 @@ class PauseTestCase(CharmTestCase):
             actions.actions.pause, [])
         self.assertEqual(pause_calls, ['haproxy'])
 
-    def test_status_mode(self):
+    def test_pause_sets_value(self):
-        """Pause action sets the status to maintenance."""
+        """Pause action sets the unit-paused value to True."""
-        status_calls = []
+        self.HookData()().return_value = True
-        self.status_set.side_effect = lambda state, msg: status_calls.append(
-            state)
 
         actions.actions.pause([])
-        self.assertEqual(status_calls, ["maintenance"])
+        self.kv().set.assert_called_with('unit-paused', True)
-
-    def test_status_message(self):
-        """Pause action sets a status message reflecting that it's paused."""
-        status_calls = []
-        self.status_set.side_effect = lambda state, msg: status_calls.append(
-            msg)
-
-        actions.actions.pause([])
-        self.assertEqual(
-            status_calls, ["Paused. "
-                           "Use 'resume' action to resume normal service."])
 
 
 class ResumeTestCase(CharmTestCase):
 
     def setUp(self):
         super(ResumeTestCase, self).setUp(
-            actions.actions, ["service_resume", "status_set"])
+            actions.actions, ["service_resume", "HookData", "kv",
+                              "assess_status"])
 
     def test_resumes_services(self):
         """Resume action resumes all Keystone services."""
@@ -97,23 +90,12 @@ class ResumeTestCase(CharmTestCase):
             actions.actions.resume, [])
         self.assertEqual(resume_calls, ['haproxy'])
 
-    def test_status_mode(self):
+    def test_resume_sets_value(self):
-        """Resume action sets the status to maintenance."""
+        """Resume action sets the unit-paused value to False."""
-        status_calls = []
+        self.HookData()().return_value = True
-        self.status_set.side_effect = lambda state, msg: status_calls.append(
-            state)
 
         actions.actions.resume([])
-        self.assertEqual(status_calls, ["active"])
+        self.kv().set.assert_called_with('unit-paused', False)
-
-    def test_status_message(self):
-        """Resume action sets an empty status message."""
-        status_calls = []
-        self.status_set.side_effect = lambda state, msg: status_calls.append(
-            msg)
-
-        actions.actions.resume([])
-        self.assertEqual(status_calls, [""])
 
 
 class MainTestCase(CharmTestCase):

unit_tests/test_keystone_utils.py

@@ -725,3 +725,37 @@ class TestKeystoneUtils(CharmTestCase):
         KeystoneManager.return_value = mock_keystone
         utils.delete_service_entry('bob', 'bill')
         mock_keystone.api.services.delete.assert_called_with('sid1')
+
+    @patch.object(utils, 'HookData')
+    @patch.object(utils, 'kv')
+    def test_is_paused(self, kv, HookData):
+        """test_is_paused: Test is_paused() returns value
+        from kv('unit-paused')"""
+        HookData()().return_value = True
+        kv().get.return_value = True
+        self.assertEqual(utils.is_paused(), True)
+        kv().get.assert_called_with('unit-paused')
+        kv().get.return_value = False
+        self.assertEqual(utils.is_paused(), False)
+
+    @patch.object(utils, 'is_paused')
+    @patch.object(utils, 'status_set')
+    def test_assess_status(self, status_set, is_paused):
+        """test_assess_status: verify that it does pick the right status"""
+        # check that paused status does the right thing
+        is_paused.return_value = True
+        utils.assess_status(None)
+        status_set.assert_called_with(
+            "maintenance",
+            "Paused. Use 'resume' action to resume normal service.")
+
+        # if it isn't paused, the assess_status() calls
+        # set_os_workload_status()
+        is_paused.return_value = False
+        with patch.object(utils, 'set_os_workload_status') \
+                as set_os_workload_status:
+            utils.assess_status("TEST CONFIG")
+            set_os_workload_status.assert_called_with(
+                "TEST CONFIG",
+                utils.REQUIRED_INTERFACES,
+                charm_func=utils.check_optional_relations)