From 9b11d24d40d4c8dfffe90a264e0273cacfd676f3 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Wed, 27 Jan 2021 17:57:28 +0000 Subject: [PATCH] Implementation of deferred restarts Add deferred restart actions and config. Change-Id: I334df5ca932e9f94e128d9fa66c1ab91d60233b4 --- actions.yaml | 27 ++++ actions/os_actions.py | 132 ++++++++++++++++++ actions/pause | 2 +- actions/pause_resume.py | 66 --------- actions/restart-services | 1 + actions/resume | 2 +- actions/run-deferred-hooks | 1 + actions/show-deferred-events | 1 + config.yaml | 6 + hooks/neutron_ovs_hooks.py | 23 ++- hooks/neutron_ovs_utils.py | 35 ++++- tests/tests.yaml | 2 + unit_tests/test_neutron_ovs_hooks.py | 34 ++++- unit_tests/test_neutron_ovs_utils.py | 17 ++- ...est_pause_resume.py => test_os_actions.py} | 2 +- 15 files changed, 266 insertions(+), 85 deletions(-) create mode 100755 actions/os_actions.py delete mode 100755 actions/pause_resume.py create mode 120000 actions/restart-services create mode 120000 actions/run-deferred-hooks create mode 120000 actions/show-deferred-events rename unit_tests/{test_pause_resume.py => test_os_actions.py} (98%) diff --git a/actions.yaml b/actions.yaml index 45db66c9..70d7dce3 100644 --- a/actions.yaml +++ b/actions.yaml @@ -26,3 +26,30 @@ pause: resume: descrpition: | Resume the neutron-openvswitch unit. This action will start neutron-openvswitch services. +restart-services: + description: | + Restarts services this charm manages. + params: + deferred-only: + type: boolean + default: false + description: | + Restart all deferred services. + services: + type: string + default: "" + description: | + List of services to restart. + run-hooks: + type: boolean + default: true + description: | + Run any hooks which have been deferred. +run-deferred-hooks: + description: | + Run deferable hooks and restart services. + . + NOTE: Service will be restarted as needed irrespective of enable-auto-restarts +show-deferred-events: + descrpition: | + Show the outstanding restarts diff --git a/actions/os_actions.py b/actions/os_actions.py new file mode 100755 index 00000000..55081f62 --- /dev/null +++ b/actions/os_actions.py @@ -0,0 +1,132 @@ +#!/usr/bin/env python3 +# +# Copyright 2021 Canonical Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import sys + +sys.path.append('hooks/') + +import charmhelpers.contrib.openstack.deferred_events as deferred_events +import charmhelpers.contrib.openstack.utils as os_utils +from charmhelpers.core.hookenv import ( + action_get, + action_fail, +) +import neutron_ovs_hooks +from neutron_ovs_utils import ( + assess_status, + pause_unit_helper, + resume_unit_helper, + register_configs, +) + + +def pause(args): + """Pause the neutron-openvswitch services. + @raises Exception should the service fail to stop. + """ + pause_unit_helper(register_configs(), + exclude_services=['openvswitch-switch']) + + +def resume(args): + """Resume the neutron-openvswitch services. + @raises Exception should the service fail to start.""" + resume_unit_helper(register_configs(), + exclude_services=['openvswitch-switch']) + + +def restart(args): + """Restart services. + + :param args: Unused + :type args: List[str] + """ + deferred_only = action_get("deferred-only") + services = action_get("services").split() + # Check input + if deferred_only and services: + action_fail("Cannot set deferred-only and services") + return + if not (deferred_only or services): + action_fail("Please specify deferred-only or services") + return + if action_get('run-hooks'): + _run_deferred_hooks() + if deferred_only: + os_utils.restart_services_action(deferred_only=True) + else: + os_utils.restart_services_action(services=services) + assess_status(register_configs()) + + +def _run_deferred_hooks(): + """Run supported deferred hooks as needed. + + Run supported deferred hooks as needed. If support for deferring a new + hook is added to the charm then this method will need updating. + """ + if not deferred_events.is_restart_permitted(): + deferred_hooks = deferred_events.get_deferred_hooks() + if deferred_hooks and 'config-changed' in deferred_hooks: + neutron_ovs_hooks.config_changed(check_deferred_restarts=False) + deferred_events.clear_deferred_hook('config-changed') + + +def run_deferred_hooks(args): + """Run deferred hooks. + + :param args: Unused + :type args: List[str] + """ + _run_deferred_hooks() + os_utils.restart_services_action(deferred_only=True) + assess_status(register_configs()) + + +def show_deferred_events(args): + """Show the deferred events. + + :param args: Unused + :type args: List[str] + """ + os_utils.show_deferred_events_action_helper() + + +# A dictionary of all the defined actions to callables (which take +# parsed arguments). +ACTIONS = {"pause": pause, "resume": resume, "restart-services": restart, + "show-deferred-events": show_deferred_events, + "run-deferred-hooks": run_deferred_hooks} + + +def main(args): + action_name = os.path.basename(args[0]) + try: + action = ACTIONS[action_name] + except KeyError: + s = "Action {} undefined".format(action_name) + action_fail(s) + return s + else: + try: + action(args) + except Exception as e: + action_fail("Action {} failed: {}".format(action_name, str(e))) + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/actions/pause b/actions/pause index bd4c0e00..93db4170 120000 --- a/actions/pause +++ b/actions/pause @@ -1 +1 @@ -pause_resume.py \ No newline at end of file +os_actions.py \ No newline at end of file diff --git a/actions/pause_resume.py b/actions/pause_resume.py deleted file mode 100755 index ee7ab5e6..00000000 --- a/actions/pause_resume.py +++ /dev/null @@ -1,66 +0,0 @@ -#!/usr/bin/env python3 -# -# Copyright 2016 Canonical Ltd -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import os -import sys - -sys.path.append('hooks/') - -from charmhelpers.core.hookenv import action_fail -from neutron_ovs_utils import ( - pause_unit_helper, - resume_unit_helper, - register_configs, -) - - -def pause(args): - """Pause the neutron-openvswitch services. - @raises Exception should the service fail to stop. - """ - pause_unit_helper(register_configs(), - exclude_services=['openvswitch-switch']) - - -def resume(args): - """Resume the neutron-openvswitch services. - @raises Exception should the service fail to start.""" - resume_unit_helper(register_configs(), - exclude_services=['openvswitch-switch']) - - -# A dictionary of all the defined actions to callables (which take -# parsed arguments). -ACTIONS = {"pause": pause, "resume": resume} - - -def main(args): - action_name = os.path.basename(args[0]) - try: - action = ACTIONS[action_name] - except KeyError: - s = "Action {} undefined".format(action_name) - action_fail(s) - return s - else: - try: - action(args) - except Exception as e: - action_fail("Action {} failed: {}".format(action_name, str(e))) - - -if __name__ == "__main__": - sys.exit(main(sys.argv)) diff --git a/actions/restart-services b/actions/restart-services new file mode 120000 index 00000000..93db4170 --- /dev/null +++ b/actions/restart-services @@ -0,0 +1 @@ +os_actions.py \ No newline at end of file diff --git a/actions/resume b/actions/resume index bd4c0e00..93db4170 120000 --- a/actions/resume +++ b/actions/resume @@ -1 +1 @@ -pause_resume.py \ No newline at end of file +os_actions.py \ No newline at end of file diff --git a/actions/run-deferred-hooks b/actions/run-deferred-hooks new file mode 120000 index 00000000..93db4170 --- /dev/null +++ b/actions/run-deferred-hooks @@ -0,0 +1 @@ +os_actions.py \ No newline at end of file diff --git a/actions/show-deferred-events b/actions/show-deferred-events new file mode 120000 index 00000000..93db4170 --- /dev/null +++ b/actions/show-deferred-events @@ -0,0 +1 @@ +os_actions.py \ No newline at end of file diff --git a/config.yaml b/config.yaml index c2f44efe..4546e3d9 100644 --- a/config.yaml +++ b/config.yaml @@ -445,3 +445,9 @@ options: in an expected data plane outage while the service restarts. . (Available from Mitaka) + enable-auto-restarts: + type: boolean + default: True + description: | + Allow the charm and packages to restart services automatically when + required. diff --git a/hooks/neutron_ovs_hooks.py b/hooks/neutron_ovs_hooks.py index 638377f3..544e2603 100755 --- a/hooks/neutron_ovs_hooks.py +++ b/hooks/neutron_ovs_hooks.py @@ -22,14 +22,18 @@ from copy import deepcopy from charmhelpers.contrib.openstack import context as os_context from charmhelpers.contrib.openstack.utils import ( - pausable_restart_on_change as restart_on_change, + os_restart_on_change as restart_on_change, series_upgrade_prepare, series_upgrade_complete, - is_unit_paused_set, + is_hook_allowed, CompareOpenStackReleases, os_release, ) +from charmhelpers.contrib.openstack.deferred_events import ( + configure_deferred_restarts, +) + from charmhelpers.core.hookenv import ( Hooks, UnregisteredHookError, @@ -72,6 +76,7 @@ from neutron_ovs_utils import ( determine_purge_packages, purge_sriov_systemd_files, use_fqdn_hint, + deferrable_services, ) hooks = Hooks() @@ -127,11 +132,16 @@ def upgrade_charm(): @restart_on_change({cfg: services for cfg, services in restart_map().items() if cfg != OVS_DEFAULT}) -def config_changed(): +def config_changed(check_deferred_restarts=True): + configure_deferred_restarts(deferrable_services()) + # policy_rcd.remove_policy_file() # if we are paused, delay doing any config changed hooks. # It is forced on the resume. - if is_unit_paused_set(): - log("Unit is pause or upgrading. Skipping config_changed", "WARN") + allowed, reason = is_hook_allowed( + 'config-changed', + check_deferred_restarts=check_deferred_restarts) + if not allowed: + log(reason, "WARN") return install_packages() @@ -257,7 +267,8 @@ def amqp_changed(): @hooks.hook('neutron-control-relation-changed') -@restart_on_change(restart_map(), stopstart=True) +@restart_on_change(restart_map(), + stopstart=True) def restart_check(): CONFIGS.write_all() diff --git a/hooks/neutron_ovs_utils.py b/hooks/neutron_ovs_utils.py index 12545c7a..3e2dcbd0 100644 --- a/hooks/neutron_ovs_utils.py +++ b/hooks/neutron_ovs_utils.py @@ -32,6 +32,10 @@ from charmhelpers.contrib.openstack.utils import ( os_release, sequence_status_check_functions, ) +from charmhelpers.contrib.openstack.deferred_events import ( + deferrable_svc_restart, + check_restart_timestamps, +) from charmhelpers.core.unitdata import kv from collections import OrderedDict import neutron_ovs_context @@ -482,6 +486,28 @@ def services(exclude_services=None): return list(s_set) +def deferrable_services(): + """Services which should be stopped from restarting. + + All services from services() are deferable. But the charm may + install a package which install a service that the charm does not add + to its restart_map. In that case it will be missing from + self.services. However one of the jobs of deferred events is to ensure + that packages updates outside of charms also do not restart services. + To ensure there is a complete list take the services from services() + and also add in a known list of networking services. + + NOTE: It does not matter if one of the services in the list is not + installed on the system. + + """ + + _svcs = services() + _svcs.extend(['ovs-vswitchd', 'ovsdb-server', 'ovs-vswitchd-dpdk', + 'openvswitch-switch']) + return list(set(_svcs)) + + def determine_ports(): """Assemble a list of API ports for services the charm is managing @@ -558,7 +584,9 @@ def enable_ovs_dpdk(): ) if ((values_changed and any(values_changed)) and not is_unit_paused_set()): - service_restart('openvswitch-switch') + deferrable_svc_restart( + 'openvswitch-switch', + restart_reason='DPDK Config changed') def enable_hw_offload(): @@ -571,7 +599,9 @@ def enable_hw_offload(): ] if ((values_changed and any(values_changed)) and not is_unit_paused_set()): - service_restart('openvswitch-switch') + deferrable_svc_restart( + 'openvswitch-switch', + restart_reason='Hardware offload config changed') def install_tmpfilesd(): @@ -908,6 +938,7 @@ def assess_status(configs): @param configs: a templating.OSConfigRenderer() object @returns None - this function is executed for its side-effect """ + check_restart_timestamps() exclude_services = [] if is_unit_paused_set(): exclude_services = ['openvswitch-switch'] diff --git a/tests/tests.yaml b/tests/tests.yaml index 6d1215e4..ec89ca8e 100644 --- a/tests/tests.yaml +++ b/tests/tests.yaml @@ -55,11 +55,13 @@ target_deploy_status: workload-status-message: Vault needs to be initialized tests: + - zaza.openstack.charm_tests.neutron.tests.NeutronOVSDeferredRestartTest - zaza.openstack.charm_tests.neutron.tests.NeutronNetworkingTest - zaza.openstack.charm_tests.neutron.tests.NeutronOpenvSwitchTest - zaza.openstack.charm_tests.neutron.tests.NeutronOvsVsctlTest - zaza.openstack.charm_tests.neutron.tests.NeutronBridgePortMappingTest - migrate-ovn: + - zaza.openstack.charm_tests.neutron.tests.NeutronOVSDeferredRestartTest - zaza.openstack.charm_tests.neutron.tests.NeutronNetworkingTest - zaza.openstack.charm_tests.neutron.tests.NeutronOvsVsctlTest - zaza.openstack.charm_tests.neutron.tests.NeutronBridgePortMappingTest diff --git a/unit_tests/test_neutron_ovs_hooks.py b/unit_tests/test_neutron_ovs_hooks.py index d3f17cc2..fd1542b0 100644 --- a/unit_tests/test_neutron_ovs_hooks.py +++ b/unit_tests/test_neutron_ovs_hooks.py @@ -52,6 +52,7 @@ TO_PATCH = [ 'purge_packages', 'determine_purge_packages', 'is_container', + 'is_hook_allowed', ] NEUTRON_CONF_DIR = "/etc/neutron" @@ -66,6 +67,7 @@ class NeutronOVSHooksTests(CharmTestCase): self.config.side_effect = self.test_config.get self.is_container.return_value = False hooks.hooks._config_save = False + self.is_hook_allowed.return_value = (True, '') def _call_hook(self, hookname): hooks.hooks.execute([ @@ -117,13 +119,25 @@ class NeutronOVSHooksTests(CharmTestCase): self.CONFIGS.write.assert_not_called() self.assertEqual(0, mock_restart.call_count) - def test_config_changed_dvr(self): + @patch.object(hooks, 'deferrable_services') + @patch.object(hooks, 'configure_deferred_restarts') + def test_config_changed_dvr(self, mock_configure_deferred_restarts, + mock_deferrable_services): + mock_deferrable_services.return_value = ['ovs-vswitchd'] self._call_hook('config-changed') self.install_packages.assert_called_with() self.assertTrue(self.CONFIGS.write_all.called) self.configure_ovs.assert_called_with() + mock_deferrable_services.assert_called_once_with() + mock_configure_deferred_restarts.assert_called_once_with( + ['ovs-vswitchd']) - def test_config_changed_sysctl_overrides(self): + @patch.object(hooks, 'deferrable_services') + @patch.object(hooks, 'configure_deferred_restarts') + def test_config_changed_sysctl_overrides(self, + mock_configure_deferred_restarts, + mock_deferrable_services): + mock_deferrable_services.return_value = ['ovs-vswitchd'] self.test_config.set( 'sysctl', '{foo : bar}' @@ -132,8 +146,15 @@ class NeutronOVSHooksTests(CharmTestCase): self.create_sysctl.assert_called_with( '{foo : bar}', '/etc/sysctl.d/50-openvswitch.conf') + mock_deferrable_services.assert_called_once_with() + mock_configure_deferred_restarts.assert_called_once_with( + ['ovs-vswitchd']) - def test_config_changed_sysctl_container(self): + @patch.object(hooks, 'deferrable_services') + @patch.object(hooks, 'configure_deferred_restarts') + def test_config_changed_sysctl_container(self, + mock_configure_deferred_restarts, + mock_deferrable_services): self.test_config.set( 'sysctl', '{foo : bar}' @@ -142,8 +163,13 @@ class NeutronOVSHooksTests(CharmTestCase): self._call_hook('config-changed') self.create_sysctl.assert_not_called() + @patch.object(hooks, 'deferrable_services') + @patch.object(hooks, 'configure_deferred_restarts') @patch.object(hooks, 'neutron_plugin_joined') - def test_config_changed_rocky_upgrade(self, _plugin_joined): + def test_config_changed_rocky_upgrade(self, + _plugin_joined, + mock_configure_deferred_restarts, + mock_deferrable_services): self.determine_purge_packages.return_value = ['python-neutron'] self.relation_ids.return_value = ['neutron-plugin:42'] self._call_hook('config-changed') diff --git a/unit_tests/test_neutron_ovs_utils.py b/unit_tests/test_neutron_ovs_utils.py index 48a3d23f..d621dd19 100644 --- a/unit_tests/test_neutron_ovs_utils.py +++ b/unit_tests/test_neutron_ovs_utils.py @@ -69,6 +69,7 @@ TO_PATCH = [ 'modprobe', 'is_container', 'is_unit_paused_set', + 'deferrable_svc_restart', ] head_pkg = 'linux-headers-3.15.0-5-generic' @@ -1062,7 +1063,8 @@ class TestNeutronOVSUtils(CharmTestCase): DummyContext(return_value={'shared_secret': 'supersecret'}) self.assertEqual(nutils.get_shared_secret(), 'supersecret') - def test_assess_status(self): + @patch.object(nutils, 'check_restart_timestamps') + def test_assess_status(self, mock_check_restart_timestamps): with patch.object(nutils, 'assess_status_func') as asf: callee = MagicMock() asf.return_value = callee @@ -1072,6 +1074,7 @@ class TestNeutronOVSUtils(CharmTestCase): self.os_application_version_set.assert_called_with( nutils.VERSION_PACKAGE ) + mock_check_restart_timestamps.assert_called_once_with() @patch('charmhelpers.contrib.openstack.context.config') def test_check_ext_port_data_port_config(self, mock_config): @@ -1196,7 +1199,9 @@ class TestNeutronOVSUtils(CharmTestCase): _check_call.assert_called_once_with( nutils.UPDATE_ALTERNATIVES + [nutils.OVS_DPDK_BIN] ) - self.service_restart.assert_called_with('openvswitch-switch') + self.deferrable_svc_restart.assert_called_with( + 'openvswitch-switch', + restart_reason='DPDK Config changed') @patch.object(nutils, 'is_unit_paused_set') @patch.object(nutils.subprocess, 'check_call') @@ -1229,7 +1234,9 @@ class TestNeutronOVSUtils(CharmTestCase): _check_call.assert_called_once_with( nutils.UPDATE_ALTERNATIVES + [nutils.OVS_DPDK_BIN] ) - self.service_restart.assert_called_with('openvswitch-switch') + self.deferrable_svc_restart.assert_called_with( + 'openvswitch-switch', + restart_reason='DPDK Config changed') @patch.object(nutils.context, 'NeutronAPIContext') @patch.object(nutils, 'is_container') @@ -1298,7 +1305,9 @@ class TestNeutronOVSUtils(CharmTestCase): call('other_config:hw-offload', 'true'), call('other_config:max-idle', '30000'), ]) - self.service_restart.assert_called_once_with('openvswitch-switch') + self.deferrable_svc_restart.assert_called_once_with( + 'openvswitch-switch', + restart_reason='Hardware offload config changed') @patch.object(nutils, 'set_Open_vSwitch_column_value') def test_enable_hw_offload_unit_paused(self, _ovs_set): diff --git a/unit_tests/test_pause_resume.py b/unit_tests/test_os_actions.py similarity index 98% rename from unit_tests/test_pause_resume.py rename to unit_tests/test_os_actions.py index 592a4532..6071b183 100644 --- a/unit_tests/test_pause_resume.py +++ b/unit_tests/test_os_actions.py @@ -19,7 +19,7 @@ from test_utils import CharmTestCase with patch('neutron_ovs_utils.register_configs') as configs: configs.return_value = 'test-config' - import pause_resume as actions + import os_actions as actions class PauseTestCase(CharmTestCase):