openstack
/
charm-keystone
Code Issues Proposed changes

Merge "Ensure that manager.py is stopped when keystone restarted"

This commit is contained in:
Zuul 2019-04-10 15:25:36 +00:00 committed by Gerrit Code Review
parent 0faecdf97a 78f0191aa8
commit 3cce6af1de
4 changed files with 134 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
hooks/keystone_hooks.py
View File

@ -125,6 +125,7 @@ from keystone_utils import (
pause_unit_helper,
resume_unit_helper,
remove_old_packages,
stop_manager_instance,
)
from charmhelpers.contrib.hahelpers.cluster import (
@ -243,6 +244,7 @@ def config_changed_postupgrade():
CONFIGS.write(WSGI_KEYSTONE_API_CONF)
if not is_unit_paused_set():
restart_pid_check('apache2')
stop_manager_instance()
if enable_memcache(release=release):
# If charm or OpenStack have been upgraded then the list of required
@ -262,6 +264,7 @@ def config_changed_postupgrade():
if snap_install_requested() and not is_unit_paused_set():
service_restart('snap.keystone.*')
stop_manager_instance()
if (is_db_initialised() and is_elected_leader(CLUSTER_RES) and not
is_unit_paused_set()):
@ -670,6 +673,7 @@ def upgrade_charm():
log("Package purge detected, restarting services", "INFO")
for s in services():
service_restart(s)
stop_manager_instance()
if is_elected_leader(CLUSTER_RES):
log('Cluster leader - ensuring endpoint configuration is up to '

59
hooks/keystone_utils.py
View File

@ -557,14 +557,38 @@ def restart_pid_check(service_name, ptable_string=None):
def restart_function_map():
"""Return a dict of services with any custom functions that should be
used to restart that service
@returns dict of {'svc1': restart_func, 'svc2', other_func, ...}
:returns: dict of {'svc1': restart_func, 'svc2', other_func, ...}
:rtype: Dict[str, Callable]
"""
rfunc_map = {}
if run_in_apache():
rfunc_map['apache2'] = restart_pid_check
rfunc_map[keystone_service()] = restart_keystone
return rfunc_map
def restart_keystone(*args):
"""Restart the keystone process.
This will either keystone or apache2 depending on OpenStack version.
Also stop the ManagerServer (and thus manager.py script) which will
reconnect to keystone on next usage of the ManagerServer.
Note, as restart_keystone is used in the restart_functions map, when it is
called it is passed the service name. However, this function determines
the actual service name to call, so that is discarded, hence the *args in
the function signature.
"""
if not is_unit_paused_set():
if snap_install_requested():
service_restart('snap.keystone.*')
else:
if run_in_apache():
restart_pid_check(keystone_service())
else:
service_restart(keystone_service())
stop_manager_instance()
def run_in_apache(release=None):
"""Return true if keystone API is run under apache2 with mod_wsgi in
this release.
@ -770,6 +794,7 @@ def migrate_database():
service_start(keystone_service())
time.sleep(10)
leader_set({'db-initialised': True})
stop_manager_instance()
# OLD
@ -1083,12 +1108,25 @@ def _get_server_instance():
return _the_manager_instance.server
def stop_manager_instance():
"""If a ManagerServer instance exists, then try to kill it, clean-up the
environment and reset the global singleton for it.
"""
global _the_manager_instance
if _the_manager_instance is not None:
_the_manager_instance.clean_up()
_the_manager_instance = None
# If a ManagerServer is still running at the end of the charm hook execution
# then kill it off:
atexit(stop_manager_instance)
class ManagerServer():
"""This is a singleton server that launches and kills the manager.py script
that is used to allow 'calling' into Keystone when it is in a completely
different process. The object handles kill/quiting the manager.py script
when this keystone charm exits using the atexit charmhelpers `atexit()`
command to do the cleanup.
different process.
The server() method also ensures that the manager.py script is still
running, and if not, relaunches it. This is to try to make the using the
@ -1099,7 +1137,6 @@ class ManagerServer():
self.pvar = None
self._server = None
self.socket_file = os.path.join(tempfile.gettempdir(), "keystone-uds")
atexit(lambda: self.clean_up())
@property
def server(self):
@ -2092,14 +2129,6 @@ def post_snap_install():
shutil.copy(PASTE_SRC, PASTE_DST)
def restart_keystone():
if not is_unit_paused_set():
if snap_install_requested():
service_restart('snap.keystone.*')
else:
service_restart(keystone_service())
def key_setup():
"""Initialize Fernet and Credential encryption key repositories

31
unit_tests/test_keystone_hooks.py
View File

@ -38,6 +38,7 @@ with patch('charmhelpers.core.hookenv.config') as config, \
mock_dec.side_effect = (lambda *dargs, **dkwargs: lambda f:
lambda *args, **kwargs: f(*args, **kwargs))
with patch.object(utils, 'run_in_apache') as mock_run_in_apache:
mock_run_in_apache.return_value = True
import keystone_hooks as hooks
importlib.reload(hooks)
@ -542,7 +543,9 @@ class KeystoneRelationTests(CharmTestCase):
@patch.object(hooks, 'is_db_initialised')
@patch('keystone_utils.log')
@patch('keystone_utils.relation_ids')
@patch.object(hooks, 'stop_manager_instance')
def test_upgrade_charm_leader(self,
mock_stop_manager_instance,
mock_relation_ids,
mock_log,
mock_is_db_initialised,
@ -562,6 +565,7 @@ class KeystoneRelationTests(CharmTestCase):
self.assertTrue(update.called)
self.remove_old_packages.assert_called_once_with()
self.service_restart.assert_called_with('apache2')
mock_stop_manager_instance.assert_called_once_with()
@patch.object(hooks, 'update_all_identity_relation_units')
@patch.object(hooks, 'is_db_initialised')
@ -707,7 +711,9 @@ class KeystoneRelationTests(CharmTestCase):
@patch.object(utils, 'os_release')
@patch('keystone_utils.log')
@patch('keystone_utils.relation_ids')
@patch.object(hooks, 'stop_manager_instance')
def test_upgrade_charm_not_leader(self,
mock_stop_manager_instance,
mock_relation_ids,
mock_log,
os_release, update):
@ -719,6 +725,7 @@ class KeystoneRelationTests(CharmTestCase):
self.assertTrue(self.apt_install.called)
self.assertTrue(self.log.called)
self.assertFalse(update.called)
mock_stop_manager_instance.assert_called_once()
def test_domain_backend_changed_v2(self):
self.get_api_version.return_value = 2
@ -741,9 +748,9 @@ class KeystoneRelationTests(CharmTestCase):
@patch.object(hooks, 'is_unit_paused_set')
@patch.object(hooks, 'is_db_initialised')
@patch.object(utils, 'run_in_apache')
@patch.object(utils, 'service_restart')
@patch.object(utils, 'restart_pid_check')
def test_domain_backend_changed_complete(self,
service_restart,
restart_pid_check,
run_in_apache,
is_db_initialised,
is_unit_paused_set):
@ -770,7 +777,7 @@ class KeystoneRelationTests(CharmTestCase):
rid=None),
])
self.create_or_show_domain.assert_called_with('mydomain')
service_restart.assert_called_with('apache2')
restart_pid_check.assert_called_with('apache2')
mock_kv.set.assert_called_with('domain-restart-nonce-mydomain',
'nonce2')
self.assertTrue(mock_kv.flush.called)
@ -778,9 +785,9 @@ class KeystoneRelationTests(CharmTestCase):
@patch.object(hooks, 'is_unit_paused_set')
@patch.object(hooks, 'is_db_initialised')
@patch.object(utils, 'run_in_apache')
@patch.object(utils, 'service_restart')
@patch.object(utils, 'restart_pid_check')
def test_domain_backend_changed_complete_follower(self,
service_restart,
restart_pid_check,
run_in_apache,
is_db_initialised,
is_unit_paused_set):
@ -808,7 +815,7 @@ class KeystoneRelationTests(CharmTestCase):
])
# Only lead unit will create the domain
self.assertFalse(self.create_or_show_domain.called)
service_restart.assert_called_with('apache2')
restart_pid_check.assert_called_with('apache2')
mock_kv.set.assert_called_with('domain-restart-nonce-mydomain',
'nonce2')
self.assertTrue(mock_kv.flush.called)
@ -818,10 +825,10 @@ class KeystoneRelationTests(CharmTestCase):
@patch.object(hooks, 'is_unit_paused_set')
@patch.object(hooks, 'is_db_initialised')
@patch.object(utils, 'run_in_apache')
@patch.object(utils, 'service_restart')
@patch.object(utils, 'restart_pid_check')
def test_fid_service_provider_changed_complete(
self,
service_restart,
restart_pid_check,
run_in_apache,
is_db_initialised,
is_unit_paused_set,
@ -847,7 +854,7 @@ class KeystoneRelationTests(CharmTestCase):
self.relation_get.assert_has_calls([
call('restart-nonce'),
])
service_restart.assert_called_with('apache2')
restart_pid_check.assert_called_with('apache2')
mock_kv.set.assert_called_with(
'fid-restart-nonce-{}'.format(rel), 'nonce2')
self.assertTrue(mock_kv.flush.called)
@ -857,10 +864,10 @@ class KeystoneRelationTests(CharmTestCase):
@patch.object(hooks, 'is_unit_paused_set')
@patch.object(hooks, 'is_db_initialised')
@patch.object(utils, 'run_in_apache')
@patch.object(utils, 'service_restart')
@patch.object(utils, 'restart_pid_check')
def test_fid_service_provider_changed_complete_follower(
self,
service_restart,
restart_pid_check,
run_in_apache,
is_db_initialised,
is_unit_paused_set,
@ -886,7 +893,7 @@ class KeystoneRelationTests(CharmTestCase):
self.relation_get.assert_has_calls([
call('restart-nonce'),
])
service_restart.assert_called_with('apache2')
restart_pid_check.assert_called_with('apache2')
mock_kv.set.assert_called_with(
'fid-restart-nonce-{}'.format(rel),
'nonce2')

74
unit_tests/test_keystone_utils.py
View File

@ -302,8 +302,9 @@ class TestKeystoneUtils(CharmTestCase):
_leader_get.return_value = None
self.assertFalse(utils.is_db_initialised())
@patch.object(utils, 'stop_manager_instance')
@patch.object(utils, 'leader_set')
def test_migrate_database(self, _leader_set):
def test_migrate_database(self, _leader_set, mock_stop_manager_instance):
self.os_release.return_value = 'havana'
utils.migrate_database()
@ -312,6 +313,7 @@ class TestKeystoneUtils(CharmTestCase):
self.subprocess.check_output.assert_called_with(cmd)
self.service_start.assert_called_with('keystone')
_leader_set.assert_called_with({'db-initialised': True})
mock_stop_manager_instance.assert_called_once_with()
@patch.object(utils, 'leader_get')
@patch.object(utils, 'get_api_version')
@ -913,16 +915,74 @@ class TestKeystoneUtils(CharmTestCase):
f.assert_called_once_with('assessor', services='s1', ports='p1')
@patch.object(utils, 'run_in_apache')
@patch.object(utils, 'restart_pid_check')
def test_restart_function_map(self, restart_pid_check, run_in_apache):
@patch.object(utils, 'restart_keystone')
def test_restart_function_map(self, restart_keystone, run_in_apache):
run_in_apache.return_value = True
self.assertEqual(utils.restart_function_map(),
{'apache2': restart_pid_check})
{'apache2': restart_keystone})
@patch.object(utils, 'stop_manager_instance')
@patch.object(utils, 'is_unit_paused_set')
def test_restart_keystone_unit_paused(self,
mock_is_unit_paused_set,
mock_stop_manager_instance):
mock_is_unit_paused_set.return_value = True
utils.restart_keystone()
mock_stop_manager_instance.assert_not_called()
@patch.object(utils, 'snap_install_requested')
@patch.object(utils, 'service_restart')
@patch.object(utils, 'stop_manager_instance')
@patch.object(utils, 'is_unit_paused_set')
def test_restart_keystone_unit_not_paused_snap_install(
self,
mock_is_unit_paused_set,
mock_stop_manager_instance,
mock_service_restart,
mock_snap_install_requested):
mock_is_unit_paused_set.return_value = False
mock_snap_install_requested.return_value = True
utils.restart_keystone()
mock_service_restart.assert_called_once_with('snap.keystone.*')
mock_stop_manager_instance.assert_called_once_with()
@patch.object(utils, 'run_in_apache')
def test_restart_function_map_legacy(self, run_in_apache):
run_in_apache.return_value = False
self.assertEqual(utils.restart_function_map(), {})
@patch.object(utils, 'snap_install_requested')
@patch.object(utils, 'service_restart')
@patch.object(utils, 'stop_manager_instance')
@patch.object(utils, 'is_unit_paused_set')
def test_restart_keystone_unit_not_paused_legacy(
self,
mock_is_unit_paused_set,
mock_stop_manager_instance,
mock_service_restart,
mock_snap_install_requested,
mock_run_in_apache):
mock_is_unit_paused_set.return_value = False
mock_snap_install_requested.return_value = False
mock_run_in_apache.return_value = False
utils.restart_keystone()
mock_service_restart.assert_called_once_with('keystone')
mock_stop_manager_instance.assert_called_once_with()
@patch.object(utils, 'run_in_apache')
@patch.object(utils, 'snap_install_requested')
@patch.object(utils, 'restart_pid_check')
@patch.object(utils, 'stop_manager_instance')
@patch.object(utils, 'is_unit_paused_set')
def test_restart_keystone_unit_not_paused(
self,
mock_is_unit_paused_set,
mock_stop_manager_instance,
mock_restart_pid_check,
mock_snap_install_requested,
mock_run_in_apache):
mock_is_unit_paused_set.return_value = False
mock_snap_install_requested.return_value = False
mock_run_in_apache.return_value = True
utils.restart_keystone()
mock_restart_pid_check.assert_called_once_with('apache2')
mock_stop_manager_instance.assert_called_once_with()
def test_restart_pid_check(self):
self.subprocess.call.return_value = 1