Merge lp:~saviq/charm-helpers/glob-restart into lp:charm-helpers

[Michał Sawicz]

[saviq] Add support for globs in restart_on_changed

[lp:~saviq/charm-helpers/glob-restart]

382. By Michał Sawicz on 2015-05-29

Abstract path_hash for easier mocking

383. By Michał Sawicz on 2015-05-29

Add mention of 'glob' module to restart_on_change

[Corey Bryant (corey.bryant)]

Hi Michal, Looks good to me except for the lint errors.

review: Needs Fixing

[lp:~saviq/charm-helpers/glob-restart]

384. By Michał Sawicz on 2015-06-01

Fix lint

[Corey Bryant (corey.bryant)]

Confirmed lint fixes and approved. Although I can't land to charm-helpers.

review: Approve

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2015-04-29 12:52:18 +0000
+++ charmhelpers/core/host.py	2015-06-01 17:16:53 +0000
@@ -24,6 +24,7 @@
 import os
 import re
 import pwd
+import glob
 import grp
 import random
 import string
@@ -269,6 +270,21 @@
         return None
 
 
+def path_hash(path):
+    """
+    Generate a hash checksum of all files matching 'path'. Standard wildcards
+    like '*' and '?' are supported, see documentation for the 'glob' module for
+    more information.
+
+    :return: dict: A { filename: hash } dictionary for all matched files.
+                   Empty if none found.
+    """
+    return {
+        filename: file_hash(filename)
+        for filename in glob.iglob(path)
+    }
+
+
 def check_hash(path, checksum, hash_type='md5'):
     """
     Validate a file using a cryptographic checksum.
@@ -296,23 +312,25 @@
 
         @restart_on_change({
             '/etc/ceph/ceph.conf': [ 'cinder-api', 'cinder-volume' ]
+            '/etc/apache/sites-enabled/*': [ 'apache2' ]
             })
-        def ceph_client_changed():
+        def config_changed():
             pass  # your code here
 
     In this example, the cinder-api and cinder-volume services
     would be restarted if /etc/ceph/ceph.conf is changed by the
-    ceph_client_changed function.
+    ceph_client_changed function. The apache2 service would be
+    restarted if any file matching the pattern got changed, created
+    or removed. Standard wildcards are supported, see documentation
+    for the 'glob' module for more information.
     """
     def wrap(f):
         def wrapped_f(*args, **kwargs):
-            checksums = {}
-            for path in restart_map:
-                checksums[path] = file_hash(path)
+            checksums = {path: path_hash(path) for path in restart_map}
             f(*args, **kwargs)
             restarts = []
             for path in restart_map:
-                if checksums[path] != file_hash(path):
+                if path_hash(path) != checksums[path]:
                     restarts += restart_map[path]
             services_list = list(OrderedDict.fromkeys(restarts))
             if not stopstart:

=== modified file 'tests/core/test_host.py'
--- tests/core/test_host.py	2015-04-29 12:52:18 +0000
+++ tests/core/test_host.py	2015-06-01 17:16:53 +0000
@@ -679,12 +679,13 @@
 
     @patch.object(host, 'service')
     @patch('os.path.exists')
-    def test_restart_no_changes(self, exists, service):
+    @patch('glob.iglob')
+    def test_restart_no_changes(self, iglob, exists, service):
         file_name = '/etc/missing.conf'
         restart_map = {
             file_name: ['test-service']
         }
-        exists.side_effect = [False, False]
+        iglob.return_value = []
 
         @host.restart_on_change(restart_map)
         def make_no_changes():
@@ -693,19 +694,18 @@
         make_no_changes()
 
         assert not service.called
-
-        exists.assert_has_calls([
-            call(file_name),
-        ])
+        assert not exists.called
 
     @patch.object(host, 'service')
     @patch('os.path.exists')
-    def test_restart_on_change(self, exists, service):
+    @patch('glob.iglob')
+    def test_restart_on_change(self, iglob, exists, service):
         file_name = '/etc/missing.conf'
         restart_map = {
             file_name: ['test-service']
         }
-        exists.side_effect = [False, True]
+        iglob.side_effect = [[], [file_name]]
+        exists.return_value = True
 
         @host.restart_on_change(restart_map)
         def make_some_changes(mock_file):
@@ -723,14 +723,17 @@
 
     @patch.object(host, 'service')
     @patch('os.path.exists')
-    def test_multiservice_restart_on_change(self, exists, service):
+    @patch('glob.iglob')
+    def test_multiservice_restart_on_change(self, iglob, exists, service):
         file_name_one = '/etc/missing.conf'
         file_name_two = '/etc/exists.conf'
         restart_map = {
             file_name_one: ['test-service'],
             file_name_two: ['test-service', 'test-service2']
         }
-        exists.side_effect = [False, True, True, True]
+        iglob.side_effect = [[], [file_name_two],
+                             [file_name_one], [file_name_two]]
+        exists.return_value = True
 
         @host.restart_on_change(restart_map)
         def make_some_changes():
@@ -752,12 +755,17 @@
 
     @patch.object(host, 'service')
     @patch('os.path.exists')
-    def test_multiservice_restart_on_change_in_order(self, exists, service):
+    @patch('glob.iglob')
+    def test_multiservice_restart_on_change_in_order(self, iglob, exists, service):
+        file_name_one = '/etc/cinder/cinder.conf'
+        file_name_two = '/etc/haproxy/haproxy.conf'
         restart_map = OrderedDict([
-            ('/etc/cinder/cinder.conf', ['some-api']),
-            ('/etc/haproxy/haproxy.conf', ['haproxy'])
+            (file_name_one, ['some-api']),
+            (file_name_two, ['haproxy'])
         ])
-        exists.side_effect = [False, True, True, True]
+        iglob.side_effect = [[], [file_name_two],
+                             [file_name_one], [file_name_two]]
+        exists.return_value = True
 
         @host.restart_on_change(restart_map)
         def make_some_changes():
@@ -775,6 +783,106 @@
         ]
         self.assertEquals(expected, service.call_args_list)
 
+    @patch.object(host, 'service')
+    @patch('os.path.exists')
+    @patch('glob.iglob')
+    def test_glob_no_restart(self, iglob, exists, service):
+        glob_path = '/etc/service/*.conf'
+        file_name_one = '/etc/service/exists.conf'
+        file_name_two = '/etc/service/exists2.conf'
+        restart_map = {
+            glob_path: ['service']
+        }
+        iglob.side_effect = [[file_name_one, file_name_two],
+                             [file_name_one, file_name_two]]
+        exists.return_value = 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 = [b'content', b'content2',
+                                          b'content', b'content2']
+            make_some_changes()
+
+        self.assertEquals([], service.call_args_list)
+
+    @patch.object(host, 'service')
+    @patch('os.path.exists')
+    @patch('glob.iglob')
+    def test_glob_restart_on_change(self, iglob, exists, service):
+        glob_path = '/etc/service/*.conf'
+        file_name_one = '/etc/service/exists.conf'
+        file_name_two = '/etc/service/exists2.conf'
+        restart_map = {
+            glob_path: ['service']
+        }
+        iglob.side_effect = [[file_name_one, file_name_two],
+                             [file_name_one, file_name_two]]
+        exists.return_value = 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 = [b'content', b'content2',
+                                          b'changed', b'content2']
+            make_some_changes()
+
+        self.assertEquals([call('restart', 'service')], service.call_args_list)
+
+    @patch.object(host, 'service')
+    @patch('os.path.exists')
+    @patch('glob.iglob')
+    def test_glob_restart_on_create(self, iglob, exists, service):
+        glob_path = '/etc/service/*.conf'
+        file_name_one = '/etc/service/exists.conf'
+        file_name_two = '/etc/service/missing.conf'
+        restart_map = {
+            glob_path: ['service']
+        }
+        iglob.side_effect = [[file_name_one],
+                             [file_name_one, file_name_two]]
+        exists.return_value = 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 = [b'exists',
+                                          b'exists', b'created']
+            make_some_changes()
+
+        self.assertEquals([call('restart', 'service')], service.call_args_list)
+
+    @patch.object(host, 'service')
+    @patch('os.path.exists')
+    @patch('glob.iglob')
+    def test_glob_restart_on_delete(self, iglob, exists, service):
+        glob_path = '/etc/service/*.conf'
+        file_name_one = '/etc/service/exists.conf'
+        file_name_two = '/etc/service/exists2.conf'
+        restart_map = {
+            glob_path: ['service']
+        }
+        iglob.side_effect = [[file_name_one, file_name_two],
+                             [file_name_two]]
+        exists.return_value = 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 = [b'exists', b'exists2',
+                                          b'exists2']
+            make_some_changes()
+
+        self.assertEquals([call('restart', 'service')], service.call_args_list)
+
     def test_lsb_release(self):
         result = {
             "DISTRIB_ID": "Ubuntu",