Implementation of deferred restarts

Add deferred restart actions and config.

Change-Id: I334df5ca932e9f94e128d9fa66c1ab91d60233b4

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

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))

actions/pause

@@ -1 +1 @@
-pause_resume.py
+os_actions.py

actions/pause_resume.py

@@ -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))

actions/restart-services

@@ -0,0 +1 @@
+os_actions.py

actions/resume

@@ -1 +1 @@
-pause_resume.py
+os_actions.py

actions/run-deferred-hooks

@@ -0,0 +1 @@
+os_actions.py

actions/show-deferred-events

@@ -0,0 +1 @@
+os_actions.py

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.

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()
 

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']

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

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')

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):

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):