Merge lp:~gandelman-a/charm-helpers/restart_on_change_in_order into lp:charm-helpers

[Adam Gandelman]

Users of core.host.restart_on_change() should be able to take special care to create a restart_map using OrderedDict, to describe ordering of service restarts on config file changes. The current function uses set() on the list of services to be restarted and breaks any attempt at restarting in a desired order.

[James Page (james-page)]

review: Approve

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2013-06-20 21:21:42 +0000
+++ charmhelpers/core/host.py	2013-06-24 03:29:27 +0000
@@ -12,6 +12,8 @@
 import subprocess
 import hashlib
 
+from collections import OrderedDict
+
 from hookenv import log, execution_environment
 
 
@@ -255,7 +257,7 @@
             for path in restart_map:
                 if checksums[path] != file_hash(path):
                     restarts += restart_map[path]
-            for service_name in list(set(restarts)):
+            for service_name in list(OrderedDict.fromkeys(restarts)):
                 service('restart', service_name)
         return wrapped_f
     return wrap

=== modified file 'tests/core/test_host.py'
--- tests/core/test_host.py	2013-06-20 21:21:42 +0000
+++ tests/core/test_host.py	2013-06-24 03:29:27 +0000
@@ -1,3 +1,4 @@
+from collections import OrderedDict
 from contextlib import contextmanager
 import os
 import subprocess
@@ -696,12 +697,36 @@
             make_some_changes()
 
         # Restart should only happen once per service
-        service.assert_has_calls([
-                            call('restart', 'test-service2'),
-                            call('restart', 'test-service')
-                            ])
+        for svc in ['test-service2', 'test-service']:
+            c = call('restart', svc)
+            self.assertEquals(1, service.call_args_list.count(c))
 
         exists.assert_has_calls([
             call(file_name_one),
             call(file_name_two)
         ])
+
+    @patch.object(host, 'service')
+    @patch('os.path.exists')
+    def test_multiservice_restart_on_change_in_order(self, exists, service):
+        restart_map = OrderedDict([
+            ('/etc/cinder/cinder.conf', ['some-api']),
+            ('/etc/haproxy/haproxy.conf',  ['haproxy'])
+        ])
+        exists.side_effect = [False, True, True, True]
+
+        @host.restart_on_change(restart_map)
+        def make_some_changes():
+            pass
+
+        with patch_open() as (mock_open, mock_file):
+            mock_file.read.side_effect = ['exists', 'missing', 'exists2']
+            make_some_changes()
+
+        # Restarts should happen in the order they are described in the
+        # restart map.
+        expected = [
+            call('restart', 'some-api'),
+            call('restart', 'haproxy')
+        ]
+        self.assertEquals(expected, service.call_args_list)