Merge lp:~gnuoy/charm-helpers/custom-restart into lp:charm-helpers

[Liam Young]

The charms sometimes have to deal with badly behaved init scripts which may incorrectly assert the state of a service. Sometimes a workaround is needed while upstream bugs are filed and fixed.

This change allows a charm to pass in custom function to be used for restarting a specific service. For example the code below will result in pgrep_restart being called when svc1 needs restarting.

def pgrep_restart(service):
    service_stop(service)
    while pgrep(service) != []:
        time.sleep(1)
    service_start(service)

@restart_on_change({'/etc/my.conf': ['svc1', 'svc2'], restart_functions={'svc1': pgrep_restart})

[Alex Kavanagh (ajkavanagh)]

A corner case is that if called with stopstart=True, then the custom restart_functions will be called twice, once for stop and once for start. I've added a comment below to with some thoughts on how to handle this.

[Liam Young (gnuoy)]

Thanks, update to follow

[lp:~gnuoy/charm-helpers/custom-restart]

565. By Liam Young on 2016-04-08

Fix double call to restart function and other fixes from tinwoods review

=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- charmhelpers/contrib/openstack/utils.py	2016-04-04 18:34:36 +0000
+++ charmhelpers/contrib/openstack/utils.py	2016-04-08 15:09:22 +0000
@@ -1535,7 +1535,8 @@
     return _assess_status_func
 
 
-def pausable_restart_on_change(restart_map, stopstart=False):
+def pausable_restart_on_change(restart_map, stopstart=False,
+                               restart_functions=None):
     """A restart_on_change decorator that checks to see if the unit is
     paused. If it is paused then the decorated function doesn't fire.
 
@@ -1568,6 +1569,7 @@
                 return f(*args, **kwargs)
             # otherwise, normal restart_on_change functionality
             return restart_on_change_helper(
-                (lambda: f(*args, **kwargs)), restart_map, stopstart)
+                (lambda: f(*args, **kwargs)), restart_map, stopstart,
+                restart_functions)
         return wrapped_f
     return wrap

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2016-04-01 13:10:03 +0000
+++ charmhelpers/core/host.py	2016-04-08 15:09:22 +0000
@@ -423,7 +423,7 @@
     pass
 
 
-def restart_on_change(restart_map, stopstart=False):
+def restart_on_change(restart_map, stopstart=False, restart_functions=None):
     """Restart services based on configuration files changing
 
     This function is used a decorator, for example::
@@ -444,18 +444,22 @@
 
     @param restart_map: {path_file_name: [service_name, ...]
     @param stopstart: DEFAULT false; whether to stop, start OR restart
+    @param restart_functions: nonstandard functions to use to restart services
+                              {svc: func, ...}
     @returns result from decorated function
     """
     def wrap(f):
         @functools.wraps(f)
         def wrapped_f(*args, **kwargs):
             return restart_on_change_helper(
-                (lambda: f(*args, **kwargs)), restart_map, stopstart)
+                (lambda: f(*args, **kwargs)), restart_map, stopstart,
+                restart_functions)
         return wrapped_f
     return wrap
 
 
-def restart_on_change_helper(lambda_f, restart_map, stopstart=False):
+def restart_on_change_helper(lambda_f, restart_map, stopstart=False,
+                             restart_functions=None):
     """Helper function to perform the restart_on_change function.
 
     This is provided for decorators to restart services if files described
@@ -464,8 +468,12 @@
     @param lambda_f: function to call.
     @param restart_map: {file: [service, ...]}
     @param stopstart: whether to stop, start or restart a service
+    @param restart_functions: nonstandard functions to use to restart services
+                              {svc: func, ...}
     @returns result of lambda_f()
     """
+    if restart_functions is None:
+        restart_functions = {}
     checksums = {path: path_hash(path) for path in restart_map}
     r = lambda_f()
     # create a list of lists of the services to restart
@@ -476,9 +484,12 @@
     services_list = list(OrderedDict.fromkeys(itertools.chain(*restarts)))
     if services_list:
         actions = ('stop', 'start') if stopstart else ('restart',)
-        for action in actions:
-            for service_name in services_list:
-                service(action, service_name)
+        for service_name in services_list:
+            if service_name in restart_functions:
+                restart_functions[service_name](service_name)
+            else:
+                for action in actions:
+                    service(action, service_name)
     return r
 
 

=== modified file 'tests/core/test_host.py'
--- tests/core/test_host.py	2016-04-01 13:10:03 +0000
+++ tests/core/test_host.py	2016-04-08 15:09:22 +0000
@@ -1281,6 +1281,37 @@
 
         self.assertEquals([call('restart', 'service')], service.call_args_list)
 
+    @patch.object(host, 'service_reload')
+    @patch.object(host, 'service')
+    @patch('os.path.exists')
+    @patch('glob.iglob')
+    def test_restart_on_change_restart_functs(self, iglob, exists, service,
+                                              service_reload):
+        file_name_one = '/etc/cinder/cinder.conf'
+        file_name_two = '/etc/haproxy/haproxy.conf'
+        restart_map = OrderedDict([
+            (file_name_one, ['some-api']),
+            (file_name_two, ['haproxy'])
+        ])
+        iglob.side_effect = [[], [file_name_two],
+                             [file_name_one], [file_name_two]]
+        exists.return_value = True
+
+        restart_funcs = {
+            'some-api': service_reload,
+        }
+
+        @host.restart_on_change(restart_map, restart_functions=restart_funcs)
+        def make_some_changes():
+            pass
+
+        with patch_open() as (mock_open, mock_file):
+            mock_file.read.side_effect = [b'exists', b'missing', b'exists2']
+            make_some_changes()
+
+        self.assertEquals([call('restart', 'haproxy')], service.call_args_list)
+        self.assertEquals([call('some-api')], service_reload.call_args_list)
+
     def test_lsb_release(self):
         result = {
             "DISTRIB_ID": "Ubuntu",