Merge lp:~michael.nelson/charm-helpers/create-hook-symlinks into lp:charm-helpers

[Michael Nelson]

Don't require charm author to create symlinks for each hook used in the playbook.

Can't currently get this to work, as python-support is installed with ansible, so when the install hook runs, it doesn't seem possible to import ansible.utils in the same process, as sys.path hasn't been setup by python-support.

Tried manually adding /usr/share/pyshared but it's not enough (and a dangerous path).

[lp:~michael.nelson/charm-helpers/create-hook-symlinks]

147. By Michael Nelson on 2014-04-30

Update available_tags during hook execution if it couldn't be set at init time.

148. By Michael Nelson on 2014-05-05

Ensure pymodules on the path.

149. By Michael Nelson on 2014-05-05

Use /usr/share/pyshared

Unmerged revisions

149. By Michael Nelson on 2014-05-05

Use /usr/share/pyshared

148. By Michael Nelson on 2014-05-05

Ensure pymodules on the path.

147. By Michael Nelson on 2014-04-30

Update available_tags during hook execution if it couldn't be set at init time.

146. By Michael Nelson on 2014-04-16

Set required symlinks based on playbook tags.

145. By Michael Nelson on 2014-04-16

Deprecation warning.

144. By Michael Nelson on 2014-04-16

Hook-up with get_tags_for_playbook.

143. By Michael Nelson on 2014-04-16

Add get_tags_for_playbook.

142. By Michael Nelson on 2014-04-16

Add available tags to module - need to populate during install.

=== modified file 'charmhelpers/contrib/ansible/__init__.py'
--- charmhelpers/contrib/ansible/__init__.py	2014-05-05 13:21:40 +0000
+++ charmhelpers/contrib/ansible/__init__.py	2014-05-05 13:21:40 +0000
@@ -67,7 +67,9 @@
 [2] http://www.ansibleworks.com/docs/modules.html
 """
 import os
+import contextlib
 import subprocess
+import sys
 import warnings
 
 import charmhelpers.contrib.templating.contexts
@@ -84,6 +86,17 @@
 available_tags = set([])
 
 
[e-mail]
+def chdir(dir_name):
+    """Context manager for changing cwd for short symlinks."""
+    try:
+        saved_path = os.getcwd()
+        os.chdir(dir_name)
+        yield
+    finally:
+        os.chdir(saved_path)
+
+
 def install_ansible_support(from_ppa=True):
     """Installs the ansible package.
 
@@ -130,6 +143,12 @@
     Discussion whether --list-tags should be a feature of ansible at
     http://goo.gl/6gXd50
     """
+    # The first install hook run is before ansible is installed. Ansible
+    # is installed into pymodules (it depends on python-support) which is
+    # on the python path *next* time python runs, but not yet this time.
+    ansible_install_dir = '/usr/share/pyshared'
+    if ansible_install_dir not in sys.path:
+        sys.path.append(ansible_install_dir)
     import ansible.utils
     import ansible.callbacks
     import ansible.playbook
@@ -192,27 +211,42 @@
 
         self.playbook_path = playbook_path
 
+        if default_hooks is not None:
+            warnings.warn(
+                "The use of default_hooks is deprecated. Ansible is now "
+                "used to query your playbook for available tags.",
+                DeprecationWarning)
+
         # The hooks decorator is created at module load time, which on the
         # first run, will be before ansible is itself installed.
         try:
             available_tags.update(get_tags_for_playbook(playbook_path))
         except ImportError:
-            available_tags.add('install')
+            pass
 
-        if default_hooks is not None:
-            warnings.warn(
-                "The use of default_hooks is deprecated. Ansible is now "
-                "used to query your playbook for available tags.",
-                DeprecationWarning)
         noop = lambda *args, **kwargs: None
+        if 'install' not in available_tags:
+            self.register('install', noop)
         for hook in available_tags:
             self.register(hook, noop)
 
     def execute(self, args):
         """Execute the hook followed by the playbook using the hook as tag."""
         super(AnsibleHooks, self).execute(args)
-        hook_name = os.path.basename(args[0])
-
-        if hook_name in available_tags:
+        current_hook_name = os.path.basename(args[0])
+
+        if len(available_tags) == 0:
+            available_tags.update(get_tags_for_playbook(self.playbook_path))
+
+        # Ensure all the symlinked hooks are set in the hooks dir,
+        # so ansible charms don't need to add the symlink whenever
+        # adding support for a new hook.
+        if current_hook_name in ['install', 'upgrade-charm']:
+            with chdir(os.path.join(charm_dir, 'hooks')):
+                for hook_name in available_tags:
+                    if not os.path.exists(hook_name):
+                        os.symlink('hooks.py', hook_name)
+
+        if current_hook_name in available_tags:
             charmhelpers.contrib.ansible.apply_playbook(
-                self.playbook_path, tags=[hook_name])
+                self.playbook_path, tags=[current_hook_name])

=== modified file 'tests/contrib/ansible/test_ansible.py'
--- tests/contrib/ansible/test_ansible.py	2014-05-05 13:21:40 +0000
+++ tests/contrib/ansible/test_ansible.py	2014-05-05 13:21:40 +0000
@@ -72,6 +72,23 @@
         'public-address': '123.123.123.123',
     }
 
+    def make_hooks_dir(self):
+        charm_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, charm_dir)
+        patcher = mock.patch.object(charmhelpers.contrib.ansible,
+                                    'charm_dir', charm_dir)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+
+        hooks_dir = os.path.join(charm_dir, 'hooks')
+        os.mkdir(os.path.join(charm_dir, 'hooks'))
+        with open(os.path.join(hooks_dir, 'hooks.py'), 'w+'):
+            pass
+
+        with charmhelpers.contrib.ansible.chdir(hooks_dir):
+            os.symlink('hooks.py', 'install')
+        return hooks_dir
+
     def setUp(self):
         super(ApplyPlaybookTestCases, self).setUp()
 
@@ -133,6 +150,10 @@
         self.addCleanup(patcher.stop)
         self.mock_get_tags_for_playbook.return_value = []
 
+        self.hooks_dir = self.make_hooks_dir()
+
+        charmhelpers.contrib.ansible.available_tags = set([])
+
     def test_calls_ansible_playbook(self):
         charmhelpers.contrib.ansible.apply_playbook(
             'playbooks/dependencies.yaml')
@@ -226,7 +247,11 @@
     def test_install_hook(self):
         """Install hook is run even though ansible wasn't available
         when trying to query for available tags."""
-        self.mock_get_tags_for_playbook.side_effect = ImportError
+        side_effects = [ImportError, ['install', 'start', 'stop']]
+
+        def side_effects_cb(*args):
+            return side_effects.pop()
+        self.mock_get_tags_for_playbook.side_effect = side_effects_cb
         hooks = charmhelpers.contrib.ansible.AnsibleHooks('my/playbook.yaml')
 
         hooks.execute(['install'])
@@ -236,7 +261,11 @@
             '--tags', 'install'])
 
     def test_register_overrides_existing_hook(self):
-        self.mock_get_tags_for_playbook.side_effect = ImportError
+        side_effects = [ImportError, ['install', 'start', 'stop']]
+
+        def side_effects_cb(*args):
+            return side_effects.pop()
+        self.mock_get_tags_for_playbook.side_effect = side_effects_cb
         hooks = charmhelpers.contrib.ansible.AnsibleHooks('my/playbook.yaml')
         install_hook = mock.Mock()
         hooks.register('install', install_hook)
@@ -251,7 +280,7 @@
     def test_using_default_hooks_raises_deprecation_warning(self):
         with warnings.catch_warnings(record=True) as warns:
             warnings.simplefilter("always")
-            hooks = charmhelpers.contrib.ansible.AnsibleHooks(
+            charmhelpers.contrib.ansible.AnsibleHooks(
                 'my/playbook.yaml', default_hooks=['foo'])
 
             self.assertEqual(len(warns), 1)
@@ -260,6 +289,21 @@
             self.assertIn("default_hooks is deprecated",
                           unicode(warning.message))
 
+    def test_install_hook_creates_missing_symlinks(self):
+        side_effects = [ImportError, ['start', 'stop']]
+
+        def side_effects_cb(*args):
+            return side_effects.pop()
+
+        self.mock_get_tags_for_playbook.side_effect = side_effects_cb
+        hooks = charmhelpers.contrib.ansible.AnsibleHooks('my/playbook.yaml')
+        hooks.register('install', mock.Mock())
+
+        hooks.execute(['install'])
+
+        self.assertEqual(set(['install', 'start', 'stop', 'hooks.py']),
+                         set(os.listdir(self.hooks_dir)))
+
 
 class GetTagsForPlaybookTestCases(unittest.TestCase):
     """Verify that get_tags_for_playbook follows the current ansible contract.