From 86c701811333a3c39179bb1ecf794cfa7648b176 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Wed, 13 Jan 2016 15:13:10 +0000 Subject: [PATCH] Change pause/resume actions use (new) assess_status() Implemented new is_paused() and assess_status() functions, and changed the pause and resume actions to use them. Changed existing and added new tests to verify functionality. --- actions/actions.py | 15 ++++++--- hooks/keystone_utils.py | 35 ++++++++++++++++++++ unit_tests/test_actions.py | 54 +++++++++++-------------------- unit_tests/test_keystone_utils.py | 34 +++++++++++++++++++ 4 files changed, 97 insertions(+), 41 deletions(-) diff --git a/actions/actions.py b/actions/actions.py index d14a621..5e27acd 100755 --- a/actions/actions.py +++ b/actions/actions.py @@ -4,9 +4,11 @@ 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( - "maintenance", "Paused. Use 'resume' action to resume normal service.") + with HookData()(): + 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 diff --git a/hooks/keystone_utils.py b/hooks/keystone_utils.py index 87e71b7..62cfdb2 100644 --- a/hooks/keystone_utils.py +++ b/hooks/keystone_utils.py @@ -83,6 +83,7 @@ INFO, WARNING, status_get, + status_set, ) from charmhelpers.fetch import ( @@ -116,6 +117,12 @@ import keystone_context import keystone_ssl as ssl +from charmhelpers.core.unitdata import ( + HookData, + kv, +) + + TEMPLATES = 'templates/' # removed from original: charm-helper-sh @@ -1807,3 +1814,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) diff --git a/unit_tests/test_actions.py b/unit_tests/test_actions.py index a9ce3ec..c1e908a 100644 --- a/unit_tests/test_actions.py +++ b/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 @@ def fake_service_pause(svc): 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 @@ def maybe_kill(svc): actions.actions.pause, []) self.assertEqual(pause_calls, ['haproxy']) - def test_status_mode(self): - """Pause action sets the status to maintenance.""" - status_calls = [] - self.status_set.side_effect = lambda state, msg: status_calls.append( - state) + def test_pause_sets_value(self): + """Pause action sets the unit-paused value to True.""" + self.HookData()().return_value = True actions.actions.pause([]) - self.assertEqual(status_calls, ["maintenance"]) - - 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."]) + self.kv().set.assert_called_with('unit-paused', True) 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 @@ def maybe_kill(svc): actions.actions.resume, []) self.assertEqual(resume_calls, ['haproxy']) - def test_status_mode(self): - """Resume action sets the status to maintenance.""" - status_calls = [] - self.status_set.side_effect = lambda state, msg: status_calls.append( - state) - - actions.actions.resume([]) - self.assertEqual(status_calls, ["active"]) - - 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) + def test_resume_sets_value(self): + """Resume action sets the unit-paused value to False.""" + self.HookData()().return_value = True actions.actions.resume([]) - self.assertEqual(status_calls, [""]) + self.kv().set.assert_called_with('unit-paused', False) class MainTestCase(CharmTestCase): diff --git a/unit_tests/test_keystone_utils.py b/unit_tests/test_keystone_utils.py index 0362623..56096ac 100644 --- a/unit_tests/test_keystone_utils.py +++ b/unit_tests/test_keystone_utils.py @@ -729,3 +729,37 @@ def test_git_post_install(self, check_call, rmtree, copytree, symlink, ] self.assertEquals(render.call_args_list, expected) service_restart.assert_called_with('keystone') + + @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)