openstack/charm-rabbitmq-server
Code Issues Proposed changes

Fix for multiple status_set() in assess_status()

Browse Source 
This fixes a multiple status_set() bug in the implementation of assess_status()
on the charm, when it is determining whether it is active AND clustered.
The change hooks into the _determine_os_workload_status() directly in
charmhelpers to ensure that status_set() is only called once when
assess_status() is called.

Change-Id: Idfd93126c3edb41f83897c82ee7f20fe5f366559
Related-Bug:#1588462
This commit is contained in:
Alex Kavanagh 2016-06-17 13:53:34 +00:00
parent 701fb3c7b6
commit 406c8b69a3
2 changed files with 27 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
hooks/rabbit_utils.py
View File

@ -15,7 +15,7 @@ from rabbitmq_context import (
from charmhelpers.core.templating import render
from charmhelpers.contrib.openstack.utils import (
make_assess_status_func,
_determine_os_workload_status,
pause_unit,
resume_unit,
is_unit_paused_set,
@ -27,7 +27,6 @@ from charmhelpers.core.hookenv import (
log, ERROR,
INFO,
service_name,
status_get,
status_set,
cached,
relation_set,
@ -686,10 +685,9 @@ def assess_cluster_status(*args):
# General status check
ret = wait_app()
if ret:
if clustered():
return 'active', 'Unit is ready and clustered'
else:
return 'active', 'Unit is ready'
# we're active - so just return the 'active' state, but if 'active'
# is returned, then it is ignored by the assess_status system.
return 'active', "message is ignored"
else:
return 'waiting', 'RabbitMQ is not yet installed'
@ -751,10 +749,6 @@ def assess_status(configs):
@returns None - this function is executed for its side-effect
"""
assess_status_func(configs)()
# Charm has a bespoke status message when clustered
if status_get() == ('active', 'Unit is ready') and clustered():
if clustered():
status_set('active', 'Unit is ready and clustered')
def assess_status_func(configs):
@ -771,10 +765,16 @@ def assess_status_func(configs):
@param configs: a templating.OSConfigRenderer() object
@return f() -> None : a function that assesses the unit's workload status
"""
return make_assess_status_func(
configs, {},
charm_func=assess_cluster_status,
services=services(), ports=None)
def _assess_status_func():
state, message = _determine_os_workload_status(
configs, {},
charm_func=assess_cluster_status,
services=services(), ports=None)
if state == 'active' and clustered():
message = 'Unit is ready and clustered'
status_set(state, message)
return _assess_status_func
def pause_unit_helper(configs):

18
unit_tests/test_rabbit_utils.py
View File

@ -236,19 +236,27 @@ class UtilsTests(unittest.TestCase):
asf.assert_called_once_with('test-config')
callee.assert_called_once_with()
@mock.patch.object(rabbit_utils, 'clustered')
@mock.patch.object(rabbit_utils, 'status_set')
@mock.patch.object(rabbit_utils, 'assess_cluster_status')
@mock.patch.object(rabbit_utils, 'services')
@mock.patch.object(rabbit_utils, 'make_assess_status_func')
@mock.patch.object(rabbit_utils, '_determine_os_workload_status')
def test_assess_status_func(self,
make_assess_status_func,
_determine_os_workload_status,
services,
assess_cluster_status):
assess_cluster_status,
status_set,
clustered):
services.return_value = 's1'
rabbit_utils.assess_status_func('test-config')
_determine_os_workload_status.return_value = ('active', '')
clustered.return_value = True
rabbit_utils.assess_status_func('test-config')()
# ports=None whilst port checks are disabled.
make_assess_status_func.assert_called_once_with(
_determine_os_workload_status.assert_called_once_with(
'test-config', {}, charm_func=assess_cluster_status, services='s1',
ports=None)
status_set.assert_called_once_with('active',
'Unit is ready and clustered')
def test_pause_unit_helper(self):
with mock.patch.object(rabbit_utils, '_pause_resume_helper') as prh: