Fix for status-set race - related to bug 1588462
This change fixes the obvious race for a status_set() between check_optional_interfaces() and assess_status() as the later calls the former which calls status_set(), returns the status, which is then potentially set again by the assess_status() function. This cleans up the code so that only a single status_set() is performed when calling assess_status(). Change-Id: Ie37a4d98de9c5e7bd26304e096796ce6287ea52b Related-Bug:#1588462
This commit is contained in:
parent
5d319f5b2b
commit
b570e36cba
4
.gitignore
vendored
4
.gitignore
vendored
@ -5,3 +5,7 @@ bin
|
||||
tags
|
||||
*.sw[nop]
|
||||
*.pyc
|
||||
.unit-state.db
|
||||
trusty/
|
||||
xenial/
|
||||
tests/cirros-*-disk.img
|
||||
|
@ -49,7 +49,6 @@ from charmhelpers.contrib.openstack.utils import (
|
||||
os_release,
|
||||
os_requires_version,
|
||||
sync_db_with_multi_ipv6_addresses,
|
||||
set_os_workload_status,
|
||||
pausable_restart_on_change as restart_on_change,
|
||||
is_unit_paused_set,
|
||||
)
|
||||
@ -96,9 +95,8 @@ from nova_cc_utils import (
|
||||
guard_map,
|
||||
get_topics,
|
||||
setup_ipv6,
|
||||
REQUIRED_INTERFACES,
|
||||
check_optional_relations,
|
||||
is_db_initialised,
|
||||
assess_status,
|
||||
)
|
||||
|
||||
from charmhelpers.contrib.hahelpers.cluster import (
|
||||
@ -1068,8 +1066,7 @@ def main():
|
||||
hooks.execute(sys.argv)
|
||||
except UnregisteredHookError as e:
|
||||
log('Unknown hook {} - skipping.'.format(e))
|
||||
set_os_workload_status(CONFIGS, REQUIRED_INTERFACES,
|
||||
charm_func=check_optional_relations)
|
||||
assess_status(CONFIGS)
|
||||
|
||||
if __name__ == '__main__':
|
||||
main()
|
||||
|
@ -37,7 +37,6 @@ from charmhelpers.contrib.openstack.utils import (
|
||||
is_ip,
|
||||
os_release,
|
||||
save_script_rc as _save_script_rc,
|
||||
set_os_workload_status,
|
||||
is_unit_paused_set,
|
||||
make_assess_status_func,
|
||||
pause_unit,
|
||||
@ -61,7 +60,6 @@ from charmhelpers.core.hookenv import (
|
||||
DEBUG,
|
||||
INFO,
|
||||
ERROR,
|
||||
status_get,
|
||||
status_set,
|
||||
related_units,
|
||||
local_unit,
|
||||
@ -1167,27 +1165,44 @@ def git_post_install(projects_yaml):
|
||||
apt_install(LATE_GIT_PACKAGES, fatal=True)
|
||||
|
||||
|
||||
def get_optional_interfaces():
|
||||
"""Return the optional interfaces that should be checked if the relavent
|
||||
relations have appeared.
|
||||
|
||||
:returns: {general_interface: [specific_int1, specific_int2, ...], ...}
|
||||
"""
|
||||
optional_interfaces = {}
|
||||
if relation_ids('quantum-network-service'):
|
||||
optional_interfaces['quantum'] = ['quantum-network-service']
|
||||
if relation_ids('cinder-volume-service'):
|
||||
optional_interfaces['cinder'] = ['cinder-volume-service']
|
||||
if relation_ids('neutron-api'):
|
||||
optional_interfaces['neutron-api'] = ['neutron-api']
|
||||
|
||||
return optional_interfaces
|
||||
|
||||
|
||||
def check_optional_relations(configs):
|
||||
required_interfaces = {}
|
||||
"""Check that if we have a relation_id for high availability that we can
|
||||
get the hacluster config. If we can't then we are blocked.
|
||||
|
||||
This function is called from assess_status/set_os_workload_status as the
|
||||
charm_func and needs to return either None, None if there is no problem or
|
||||
the status, message if there is a problem.
|
||||
|
||||
:param configs: an OSConfigRender() instance.
|
||||
:return 2-tuple: (string, string) = (status, message)
|
||||
"""
|
||||
if relation_ids('ha'):
|
||||
required_interfaces['ha'] = ['cluster']
|
||||
try:
|
||||
get_hacluster_config()
|
||||
except:
|
||||
return ('blocked',
|
||||
'hacluster missing configuration: '
|
||||
'vip, vip_iface, vip_cidr')
|
||||
if relation_ids('quantum-network-service'):
|
||||
required_interfaces['quantum'] = ['quantum-network-service']
|
||||
if relation_ids('cinder-volume-service'):
|
||||
required_interfaces['cinder'] = ['cinder-volume-service']
|
||||
if relation_ids('neutron-api'):
|
||||
required_interfaces['neutron-api'] = ['neutron-api']
|
||||
if required_interfaces:
|
||||
set_os_workload_status(configs, required_interfaces)
|
||||
return status_get()
|
||||
else:
|
||||
return 'unknown', 'No optional relations'
|
||||
# return 'unknown' as the lowest priority to not clobber an existing
|
||||
# status.
|
||||
return "unknown", ""
|
||||
|
||||
|
||||
def is_api_ready(configs):
|
||||
@ -1215,14 +1230,20 @@ def assess_status_func(configs):
|
||||
Used directly by assess_status() and also for pausing and resuming
|
||||
the unit.
|
||||
|
||||
NOTE: REQUIRED_INTERFACES is augmented with the optional interfaces
|
||||
depending on the current config before being passed to the
|
||||
make_assess_status_func() function.
|
||||
|
||||
NOTE(ajkavanagh) ports are not checked due to race hazards with services
|
||||
that don't behave sychronously w.r.t their service scripts. e.g.
|
||||
apache2.
|
||||
@param configs: a templating.OSConfigRenderer() object
|
||||
@return f() -> None : a function that assesses the unit's workload status
|
||||
"""
|
||||
required_interfaces = REQUIRED_INTERFACES.copy()
|
||||
required_interfaces.update(get_optional_interfaces())
|
||||
return make_assess_status_func(
|
||||
configs, REQUIRED_INTERFACES,
|
||||
configs, required_interfaces,
|
||||
charm_func=check_optional_relations,
|
||||
services=services(), ports=None)
|
||||
|
||||
|
@ -911,6 +911,7 @@ class NovaCCUtilsTests(CharmTestCase):
|
||||
asf.assert_called_once_with('test-config')
|
||||
callee.assert_called_once_with()
|
||||
|
||||
@patch.object(utils, 'get_optional_interfaces')
|
||||
@patch.object(utils, 'check_optional_relations')
|
||||
@patch.object(utils, 'REQUIRED_INTERFACES')
|
||||
@patch.object(utils, 'services')
|
||||
@ -921,13 +922,17 @@ class NovaCCUtilsTests(CharmTestCase):
|
||||
determine_ports,
|
||||
services,
|
||||
REQUIRED_INTERFACES,
|
||||
check_optional_relations):
|
||||
check_optional_relations,
|
||||
get_optional_interfaces):
|
||||
services.return_value = 's1'
|
||||
REQUIRED_INTERFACES.copy.return_value = {'int': ['test 1']}
|
||||
get_optional_interfaces.return_value = {'opt': ['test 2']}
|
||||
determine_ports.return_value = 'p1'
|
||||
utils.assess_status_func('test-config')
|
||||
# ports=None whilst port checks are disabled.
|
||||
make_assess_status_func.assert_called_once_with(
|
||||
'test-config', REQUIRED_INTERFACES,
|
||||
'test-config',
|
||||
{'int': ['test 1'], 'opt': ['test 2']},
|
||||
charm_func=check_optional_relations, services='s1',
|
||||
ports=None)
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user