openstack
/
charm-nova-compute
Code Issues Proposed changes

Merge "Do not manage subordinate service restarts"

This commit is contained in:
Zuul 2023-08-22 13:32:57 +00:00 committed by Gerrit Code Review
parent 191670b0aa cb04103e08
commit f6892d228c
4 changed files with 100 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
hooks/nova_compute_hooks.py
View File

@ -15,6 +15,7 @@
# limitations under the License.
import base64
import functools
import json
import platform
import sys
@ -103,7 +104,6 @@ from nova_compute_utils import (
public_ssh_key,
restart_map,
services,
restart_failed_subordinate_services,
register_configs,
NOVA_CONF,
ceph_config_file, CEPH_SECRET,
@ -744,7 +744,7 @@ def nova_ceilometer_joined(relid=None, remote_restart=False):
@restart_on_change(restart_map())
def nova_ceilometer_relation_changed():
update_all_configs()
restart_failed_subordinate_services()
trigger_ceilometer_service_restart()
@hooks.hook('nova-vgpu-relation-joined')
@ -759,7 +759,7 @@ def nova_vgpu_joined(relid=None, remote_restart=False):
@restart_on_change(restart_map())
def nova_vgpu_relation_changed():
update_all_configs()
restart_failed_subordinate_services()
trigger_nova_vgpu_service_restart()
@hooks.hook('nrpe-external-master-relation-joined',
@ -804,7 +804,8 @@ def neutron_plugin_changed():
apt_purge('nova-api-metadata', fatal=True)
service_restart_handler(default_service='nova-compute')
CONFIGS.write(NOVA_CONF)
restart_failed_subordinate_services()
trigger_ceilometer_service_restart()
trigger_nova_vgpu_service_restart()
# TODO(jamespage): Move this into charmhelpers for general reuse.
@ -954,6 +955,36 @@ def shared_db_relation_joined():
"WARNING")
def trigger_subordinate_service_restart(relation_name, hook_method):
"""Send restart trigger to subordinate relaion
:param relation_name: Name of relation
:type relation_name: str
:param hook_method: Method to call to send trigger
:type hook_method: callable
"""
if not is_unit_paused_set():
log(
"Sending restart trigger down {} relation".format(relation_name),
"INFO")
for rid in relation_ids(relation_name):
hook_method(
rid,
remote_restart=True)
trigger_ceilometer_service_restart = functools.partial(
trigger_subordinate_service_restart,
'nova-ceilometer',
nova_ceilometer_joined)
trigger_nova_vgpu_service_restart = functools.partial(
trigger_subordinate_service_restart,
'nova-vgpu',
nova_vgpu_joined)
def main():
try:
hooks.execute(sys.argv)

22
hooks/nova_compute_utils.py
View File

@ -43,7 +43,6 @@ from charmhelpers.core.fstab import Fstab
from charmhelpers.core.host import (
mkdir,
service_restart,
service_running,
lsb_release,
rsync,
CompareHostReleases,
@ -465,27 +464,6 @@ def services():
list(get_subordinate_services()))
def restart_failed_subordinate_services():
'''
Restarts all subordinate services if they aren't running.
Doesn't do anything if the unit is paused.
Subordinate charms can advertise a list of services to this principal charm
so that this principal charm can properly manage them when pausing,
resuming and upgrading.
It can be useful to call this function once a relation with a subordinate
charm has been established, in order to mitigate a race condition described
in lp:1947585 in which the subordinate services may have failed to come up
if they were started before the principal services.
'''
if not is_unit_paused_set():
for s in get_subordinate_services():
if not service_running(s):
service_restart(s)
def register_configs():
'''
Returns an OSTemplateRenderer object with all required configs registered.

77
unit_tests/test_nova_compute_hooks.py
View File

@ -105,7 +105,6 @@ TO_PATCH = [
'render',
'remove_old_packages',
'services',
'restart_failed_subordinate_services',
'send_application_name',
]
@ -582,15 +581,20 @@ class NovaComputeRelationsTests(CharmTestCase):
prefix='nova',
)
def test_nova_ceilometer_relation_changed(self):
@patch.object(hooks, 'trigger_ceilometer_service_restart')
def test_nova_ceilometer_relation_changed(
self,
trigger_ceilometer_service_restart):
hooks.nova_ceilometer_relation_changed()
self.update_all_configs.assert_called()
self.restart_failed_subordinate_services.assert_called()
trigger_ceilometer_service_restart.assert_called()
def test_nova_vgpu_relation_changed(self):
hooks.nova_ceilometer_relation_changed()
@patch.object(hooks, 'trigger_nova_vgpu_service_restart')
def test_nova_vgpu_relation_changed(self,
trigger_nova_vgpu_service_restart):
hooks.nova_vgpu_relation_changed()
self.update_all_configs.assert_called()
self.restart_failed_subordinate_services.assert_called()
trigger_nova_vgpu_service_restart.assert_called()
def test_ceph_joined(self):
self.libvirt_daemon.return_value = 'libvirt-bin'
@ -969,10 +973,14 @@ class NovaComputeRelationsTests(CharmTestCase):
app_name='rbd',
compression_mode='fake')
@patch.object(hooks, 'trigger_nova_vgpu_service_restart')
@patch.object(hooks, 'trigger_ceilometer_service_restart')
@patch.object(hooks, 'service_restart_handler')
@patch.object(hooks, 'CONFIGS')
def test_neutron_plugin_changed(self, configs,
service_restart_handler):
service_restart_handler,
trigger_ceilometer_service_restart,
trigger_nova_vgpu_service_restart):
self.nova_metadata_requirement.return_value = (True,
'sharedsecret')
hooks.neutron_plugin_changed()
@ -982,12 +990,17 @@ class NovaComputeRelationsTests(CharmTestCase):
configs.write.assert_called_with('/etc/nova/nova.conf')
service_restart_handler.assert_called_with(
default_service='nova-compute')
self.restart_failed_subordinate_services.assert_called()
trigger_ceilometer_service_restart.assert_called()
trigger_nova_vgpu_service_restart.assert_called()
@patch.object(hooks, 'trigger_nova_vgpu_service_restart')
@patch.object(hooks, 'trigger_ceilometer_service_restart')
@patch.object(hooks, 'service_restart_handler')
@patch.object(hooks, 'CONFIGS')
def test_neutron_plugin_changed_nometa(self, configs,
service_restart_handler):
service_restart_handler,
trigger_ceilometer_service_restart,
trigger_nova_vgpu_service_restart):
self.nova_metadata_requirement.return_value = (False, None)
hooks.neutron_plugin_changed()
self.apt_purge.assert_called_with('nova-api-metadata',
@ -995,12 +1008,17 @@ class NovaComputeRelationsTests(CharmTestCase):
configs.write.assert_called_with('/etc/nova/nova.conf')
service_restart_handler.assert_called_with(
default_service='nova-compute')
self.restart_failed_subordinate_services.assert_called()
trigger_ceilometer_service_restart.assert_called()
trigger_nova_vgpu_service_restart.assert_called()
@patch.object(hooks, 'trigger_nova_vgpu_service_restart')
@patch.object(hooks, 'trigger_ceilometer_service_restart')
@patch.object(hooks, 'service_restart_handler')
@patch.object(hooks, 'CONFIGS')
def test_neutron_plugin_changed_meta(self, configs,
service_restart_handler):
service_restart_handler,
trigger_ceilometer_service_restart,
trigger_nova_vgpu_service_restart):
self.nova_metadata_requirement.return_value = (True, None)
hooks.neutron_plugin_changed()
self.apt_install.assert_called_with(['nova-api-metadata'],
@ -1008,7 +1026,8 @@ class NovaComputeRelationsTests(CharmTestCase):
configs.write.assert_called_with('/etc/nova/nova.conf')
service_restart_handler.assert_called_with(
default_service='nova-compute')
self.restart_failed_subordinate_services.assert_called()
trigger_ceilometer_service_restart.assert_called()
trigger_nova_vgpu_service_restart.assert_called()
@patch('nova_compute_context.config')
@patch.object(hooks, 'get_hugepage_number')
@ -1294,3 +1313,37 @@ class NovaComputeRelationsTests(CharmTestCase):
hooks.upgrade_charm()
self.remove_old_packages.assert_called_once_with()
self.service_restart.assert_called_once_with('nova-compute')
@patch.object(hooks, 'is_unit_paused_set')
@patch.object(hooks, 'nova_ceilometer_joined')
def test_trigger_ceilometer_service_restart(self,
nova_ceilometer_joined,
is_unit_paused_set):
self.uuid.uuid4.return_value = 'uuid1234'
self.relation_ids.return_value = ['nova-ceilometer:43']
is_unit_paused_set.return_value = True
hooks.trigger_ceilometer_service_restart()
self.assertFalse(nova_ceilometer_joined.called)
is_unit_paused_set.return_value = False
hooks.trigger_ceilometer_service_restart()
self.relation_set.assert_called_with(
relation_id='nova-ceilometer:43',
relation_settings={
'restart-trigger': 'uuid1234'})
@patch.object(hooks, 'is_unit_paused_set')
@patch.object(hooks, 'nova_vgpu_joined')
def test_trigger_nova_vgpu_service_restart(self,
nova_vgpu_joined,
is_unit_paused_set):
self.uuid.uuid4.return_value = 'uuid1234'
self.relation_ids.return_value = ['nova-vgpu:12']
is_unit_paused_set.return_value = True
hooks.trigger_nova_vgpu_service_restart()
self.assertFalse(nova_vgpu_joined.called)
is_unit_paused_set.return_value = False
hooks.trigger_nova_vgpu_service_restart()
self.relation_set.assert_called_with(
relation_id='nova-vgpu:12',
relation_settings={
'restart-trigger': 'uuid1234'})

37
unit_tests/test_nova_compute_utils.py
View File

@ -1440,43 +1440,6 @@ class NovaComputeUtilsTests(CharmTestCase):
self.assertEqual(expected_service_set, set(actual_service_list))
self.assertEqual(expected_last_service, actual_service_list[-1])
@patch.object(utils, 'service_restart')
@patch.object(utils, 'service_running')
@patch.object(utils, 'get_subordinate_services')
@patch.object(utils, 'is_unit_paused_set')
def test_restart_failed_subordinate_services(self, _is_unit_paused_set,
_get_subordinate_services,
_service_running,
_service_restart):
_is_unit_paused_set.return_value = True
utils.restart_failed_subordinate_services()
_service_restart.assert_not_called()
# Validate that when a single service is not running, it is restarted
_service_restart.reset_mock()
_is_unit_paused_set.return_value = False
_get_subordinate_services.return_value = ['ceilometer-agent']
_service_running.return_value = False
utils.restart_failed_subordinate_services()
_service_restart.assert_called_once_with('ceilometer-agent')
# Validate that when multiple subordinate services exist, only ones
# that are stopped are restarted.
_service_restart.reset_mock()
def is_running(service):
if service == 'ceilometer-agent':
return True
else:
return False
_service_running.side_effect = is_running
_get_subordinate_services.return_value = ['ceilometer-agent', 'foo']
utils.restart_failed_subordinate_services()
_service_restart.assert_has_calls([
call('foo'),
])
@patch.object(utils, 'render')
def test_install_mount_override(self, render):
utils.install_mount_override('/srv/test')