Merge lp:~michael.nelson/charm-helpers/add-declarative-support into lp:charm-helpers

[Michael Nelson]

Add declarative machine state support.

This branch adds the ability to declare the machine state, rather than writing procedural code with fragile tests that need to be updated whenever the procedural code for machine state is updated.

See the docstring below for more details.

I'm using this on a private charm at the moment. If you've access, you can see how simple the hooks are at:

https://bazaar.launchpad.net/~michael.nelson/canonical-marshal/click-index-new-helpers/view/head:/hooks/hooks.py

The machine_states and templates directories might also be relevant to look at there. I'm testing that private charm using a deploy setup with an apache front end, cache lb, cache, the app lb + apps. If/when this branch lands, I'll create a bootstrap public charm + blog post showing how it can be used.

[Matthew Wedgwood (mew)]

Per our discussion in IRC, this looks like a good candidate for charmhelpers.contrib.saltstack, rather than to be the more generally-named "declarative." This naming will make it clear which config format is in play and helps keep the namespace clean for future puppet, chef, etc. modules.

[lp:~michael.nelson/charm-helpers/add-declarative-support]

31. By Michael Nelson on 2013-06-13

Move declarative->saltstates.

[Michael Nelson (michael.nelson)]

On Thu, Jun 13, 2013 at 1:03 AM, Matthew Wedgwood <
<email address hidden>> wrote:

> Per our discussion in IRC, this looks like a good candidate for
> charmhelpers.contrib.saltstack, rather than to be the more generally-named
> "declarative." This naming will make it clear which config format is in
> play and helps keep the namespace clean for future puppet, chef, etc.
> modules.
>
>
Great - thanks Matthew. I've ended up moving it to
charmhelpers.contrib.saltstates so that it's clearer that we're just using
saltstacks support of local machine states, rather than the full saltstack
support (ie. master+slave). If you'd prefer saltstack, just let me know and
I'll switch it.

[Kapil Thangavelu (hazmat)]

Looks good, although, be warned salt has close to zero testing. Ansible is
a nice alternative.

On Thu, Jun 13, 2013 at 5:28 AM, Michael Nelson <
<email address hidden>> wrote:

> On Thu, Jun 13, 2013 at 1:03 AM, Matthew Wedgwood <
> <email address hidden>> wrote:
>
> > Per our discussion in IRC, this looks like a good candidate for
> > charmhelpers.contrib.saltstack, rather than to be the more
> generally-named
> > "declarative." This naming will make it clear which config format is in
> > play and helps keep the namespace clean for future puppet, chef, etc.
> > modules.
> >
> >
> Great - thanks Matthew. I've ended up moving it to
> charmhelpers.contrib.saltstates so that it's clearer that we're just using
> saltstacks support of local machine states, rather than the full saltstack
> support (ie. master+slave). If you'd prefer saltstack, just let me know and
> I'll switch it.
>
> --
>
> https://code.launchpad.net/~michael.nelson/charm-helpers/add-declarative-support/+merge/168961
> Your team Charm Helper Maintainers is requested to review the proposed
> merge of lp:~michael.nelson/charm-helpers/add-declarative-support into
> lp:charm-helpers.
>

[Michael Nelson (michael.nelson)]

Thanks Kapil. Ansible does look nice too - if I get time I'll look at
adding helpers for it to contrib too.

On Thu, Jun 13, 2013 at 2:55 PM, Kapil Thangavelu <
<email address hidden>> wrote:

> Looks good, although, be warned salt has close to zero testing. Ansible is
> a nice alternative.
>
>
> On Thu, Jun 13, 2013 at 5:28 AM, Michael Nelson <
> <email address hidden>> wrote:
>
> > On Thu, Jun 13, 2013 at 1:03 AM, Matthew Wedgwood <
> > <email address hidden>> wrote:
> >
> > > Per our discussion in IRC, this looks like a good candidate for
> > > charmhelpers.contrib.saltstack, rather than to be the more
> > generally-named
> > > "declarative." This naming will make it clear which config format is in
> > > play and helps keep the namespace clean for future puppet, chef, etc.
> > > modules.
> > >
> > >
> > Great - thanks Matthew. I've ended up moving it to
> > charmhelpers.contrib.saltstates so that it's clearer that we're just
> using
> > saltstacks support of local machine states, rather than the full
> saltstack
> > support (ie. master+slave). If you'd prefer saltstack, just let me know
> and
> > I'll switch it.
> >
> > --
> >
> >
> https://code.launchpad.net/~michael.nelson/charm-helpers/add-declarative-support/+merge/168961
> > Your team Charm Helper Maintainers is requested to review the proposed
> > merge of lp:~michael.nelson/charm-helpers/add-declarative-support into
> > lp:charm-helpers.
> >
>
> --
>
> https://code.launchpad.net/~michael.nelson/charm-helpers/add-declarative-support/+merge/168961
> You are the owner of
> lp:~michael.nelson/charm-helpers/add-declarative-support.
>

[Matthew Wedgwood (mew)]

> Great - thanks Matthew. I've ended up moving it to
> charmhelpers.contrib.saltstates so that it's clearer that we're just using
> saltstacks support of local machine states, rather than the full saltstack
> support (ie. master+slave). If you'd prefer saltstack, just let me know and
> I'll switch it.

I think it's fine calling this saltstack. A lot of Puppet sites run masterless, so I wouldn't say that the lack of master/slave support makes this less "full." If someone were to add additional support for saltstack, it should go in this module.

Thanks again! Sorry to be so picky.

[lp:~michael.nelson/charm-helpers/add-declarative-support]

32. By Michael Nelson on 2013-06-14

s/saltstates/saltstack

[Michael Nelson (michael.nelson)]

On Thu, Jun 13, 2013 at 6:06 PM, Matthew Wedgwood <
<email address hidden>> wrote:

> I think it's fine calling this saltstack. A lot of Puppet sites run
> masterless, so I wouldn't say that the lack of master/slave support makes
> this less "full." If someone were to add additional support for saltstack,
> it should go in this module.
>
> Thanks again! Sorry to be so picky.
>

No problem - that makes sense and it's an easy s/saltstates/saltstack :)
Pushed with r32.

[Matthew Wedgwood (mew)]

Perfect. +1

review: Approve

=== added directory 'charmhelpers/contrib/saltstack'
=== added file 'charmhelpers/contrib/saltstack/__init__.py'
--- charmhelpers/contrib/saltstack/__init__.py	1970-01-01 00:00:00 +0000
+++ charmhelpers/contrib/saltstack/__init__.py	2013-06-14 07:11:26 +0000
@@ -0,0 +1,124 @@
+"""Charm Helpers saltstack - declare the state of your machines.
+
+This helper enables you to declare your machine state, rather than
+program it procedurally (and have to test each change to your procedures).
+Your install hook can be as simple as:
+
+{{{
+from charmhelpers.contrib.saltstack import (
+    install_salt_support,
+    update_machine_state,
+)
+
+
+def install():
+    install_salt_support()
+    update_machine_state('machine_states/dependencies.yaml')
+    update_machine_state('machine_states/installed.yaml')
+}}}
+
+and won't need to change (nor will its tests) when you change the machine
+state.
+
+It's using a python package called salt-minion which allows various formats for
+specifying resources, such as:
+
+{{{
+/srv/{{ basedir }}:
+    file.directory:
+        - group: ubunet
+        - user: ubunet
+        - require:
+            - user: ubunet
+        - recurse:
+            - user
+            - group
+
+ubunet:
+    group.present:
+        - gid: 1500
+    user.present:
+        - uid: 1500
+        - gid: 1500
+        - createhome: False
+        - require:
+            - group: ubunet
+}}}
+
+The docs for all the different state definitions are at:
+    http://docs.saltstack.com/ref/states/all/
+
+
+TODO:
+  * Add test helpers which will ensure that machine state definitions
+    are functionally (but not necessarily logically) correct (ie. getting
+    salt to parse all state defs.
+  * Add a link to a public bootstrap charm example / blogpost.
+  * Find a way to obviate the need to use the grains['charm_dir'] syntax
+    in templates.
+"""
+# Copyright 2013 Canonical Ltd.
+#
+# Authors:
+#  Charm Helpers Developers <[e-mail]>
+import os
+import subprocess
+import yaml
+
+import charmhelpers.core.host
+import charmhelpers.core.hookenv
+
+
+charm_dir = os.environ.get('CHARM_DIR', '')
+salt_grains_path = '/etc/salt/grains'
+
+
+def install_salt_support(from_ppa=True):
+    """Installs the salt-minion helper for machine state.
+
+    By default the salt-minion package is installed from
+    the saltstack PPA. If from_ppa is False you must ensure
+    that the salt-minion package is available in the apt cache.
+    """
+    if from_ppa:
+        subprocess.check_call([
+            '/usr/bin/add-apt-repository',
+            '--yes',
+            'ppa:saltstack/salt',
+        ])
+        subprocess.check_call(['/usr/bin/apt-get', 'update'])
+    charmhelpers.core.host.apt_install('salt-minion')
+
+
+def update_machine_state(state_path):
+    """Update the machine state using the provided state declaration."""
+    juju_config_2_grains()
+    subprocess.check_call([
+        'salt-call',
+        '--local',
+        'state.template',
+        state_path,
+    ])
+
+
+def juju_config_2_grains():
+    """Insert the juju config as salt grains for use in state templates.
+
+    This includes any current relation-get data, and the charm
+    directory.
+    """
+    config = charmhelpers.core.hookenv.config()
+
+    # Add the charm_dir which we will need to refer to charm
+    # file resources etc.
+    config['charm_dir'] = charm_dir
+    config['local_unit'] = charmhelpers.core.hookenv.local_unit()
+    config.update(charmhelpers.core.hookenv.relation_get())
+
+    # Don't use non-standard tags for unicode which will not
+    # work when salt uses yaml.load_safe.
+    yaml.add_representer(unicode, lambda dumper,
+                         value: dumper.represent_scalar(
+                             u'tag:yaml.org,2002:str', value))
+    with open(salt_grains_path, "w+") as fp:
+        fp.write(config.yaml())

=== modified file 'setup.py'
--- setup.py	2013-05-31 10:34:33 +0000
+++ setup.py	2013-06-14 07:11:26 +0000
@@ -19,8 +19,10 @@
         "charmhelpers.core",
         "charmhelpers.fetch",
         "charmhelpers.payload",
+        "charmhelpers.contrib",
         "charmhelpers.contrib.charmhelpers",
         "charmhelpers.contrib.charmsupport",
+        "charmhelpers.contrib.saltstack",
         "charmhelpers.contrib.hahelpers",
         "charmhelpers.contrib.jujugui",
     ],

=== added directory 'tests/contrib/saltstates'
=== added file 'tests/contrib/saltstates/__init__.py'
=== added file 'tests/contrib/saltstates/test_saltstates.py'
--- tests/contrib/saltstates/test_saltstates.py	1970-01-01 00:00:00 +0000
+++ tests/contrib/saltstates/test_saltstates.py	2013-06-14 07:11:26 +0000
@@ -0,0 +1,158 @@
+# Copyright 2013 Canonical Ltd.
+#
+# Authors:
+#  Charm Helpers Developers <[e-mail]>
+import json
+import mock
+import tempfile
+import unittest
+import yaml
+
+import charmhelpers.core.hookenv
+import charmhelpers.contrib.saltstack
+
+
+class InstallSaltSupportTestCase(unittest.TestCase):
+
+    def setUp(self):
+        super(InstallSaltSupportTestCase, self).setUp()
+
+        patcher = mock.patch('charmhelpers.contrib.saltstack.subprocess')
+        self.mock_subprocess = patcher.start()
+        self.addCleanup(patcher.stop)
+
+        patcher = mock.patch('charmhelpers.core')
+        self.mock_charmhelpers_core = patcher.start()
+        self.addCleanup(patcher.stop)
+
+    def test_adds_ppa_by_default(self):
+        charmhelpers.contrib.saltstack.install_salt_support()
+
+        self.assertEqual(self.mock_subprocess.check_call.call_count, 2)
+        self.assertEqual([(([
+                '/usr/bin/add-apt-repository',
+                '--yes',
+                'ppa:saltstack/salt',
+            ],), {}),
+            (([
+                '/usr/bin/apt-get',
+                'update',
+            ],), {})
+        ], self.mock_subprocess.check_call.call_args_list)
+        self.mock_charmhelpers_core.host.apt_install.assert_called_once_with(
+            'salt-minion')
+
+    def test_no_ppa(self):
+        charmhelpers.contrib.saltstack.install_salt_support(
+            from_ppa=False)
+
+        self.assertEqual(self.mock_subprocess.check_call.call_count, 0)
+        self.mock_charmhelpers_core.host.apt_install.assert_called_once_with(
+            'salt-minion')
+
+
+class UpdateMachineStateTestCase(unittest.TestCase):
+
+    def setUp(self):
+        super(UpdateMachineStateTestCase, self).setUp()
+
+        patcher = mock.patch('charmhelpers.contrib.saltstack.subprocess')
+        self.mock_subprocess = patcher.start()
+        self.addCleanup(patcher.stop)
+
+        patcher = mock.patch('charmhelpers.contrib.saltstack.'
+                             'juju_config_2_grains')
+        self.mock_config_2_grains = patcher.start()
+        self.addCleanup(patcher.stop)
+
+    def test_calls_local_salt_template(self):
+        charmhelpers.contrib.saltstack.update_machine_state(
+            'states/install.yaml')
+
+        self.mock_subprocess.check_call.assert_called_once_with([
+            'salt-call',
+            '--local',
+            'state.template',
+            'states/install.yaml',
+        ])
+
+    def test_updates_grains(self):
+        charmhelpers.contrib.saltstack.update_machine_state(
+            'states/install.yaml')
+
+        self.mock_config_2_grains.assert_called_once_with()
+
+
+class JujuConfig2GrainsTestCase(unittest.TestCase):
+    def setUp(self):
+        super(JujuConfig2GrainsTestCase, self).setUp()
+
+        # Hookenv patches (a single patch to hookenv doesn't work):
+        patcher = mock.patch('charmhelpers.core.hookenv.config')
+        self.mock_config = patcher.start()
+        self.addCleanup(patcher.stop)
+        patcher = mock.patch('charmhelpers.core.hookenv.relation_get')
+        self.mock_relation_get = patcher.start()
+        self.mock_relation_get.return_value = {}
+        self.addCleanup(patcher.stop)
+        patcher = mock.patch('charmhelpers.core.hookenv.local_unit')
+        self.mock_local_unit = patcher.start()
+        self.addCleanup(patcher.stop)
+
+        # patches specific to this test class.
+        grain_file = tempfile.NamedTemporaryFile()
+        self.grain_path = grain_file.name
+        self.addCleanup(grain_file.close)
+
+        patcher = mock.patch.object(charmhelpers.contrib.saltstack,
+                                    'salt_grains_path', self.grain_path)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+
+        patcher = mock.patch.object(charmhelpers.contrib.saltstack,
+                                    'charm_dir', '/tmp/charm_dir')
+        patcher.start()
+        self.addCleanup(patcher.stop)
+
+
+    def test_output_without_relation(self):
+        self.mock_config.return_value = charmhelpers.core.hookenv.Serializable({
+           'group_code_owner': 'webops_deploy',
+           'user_code_runner': 'ubunet',
+        })
+        self.mock_local_unit.return_value = "click-index/3"
+
+        charmhelpers.contrib.saltstack.juju_config_2_grains()
+
+        with open(self.grain_path, 'r') as grain_file:
+            result = yaml.load(grain_file.read())
+            self.assertEqual({
+                "charm_dir": "/tmp/charm_dir",
+                "group_code_owner": "webops_deploy",
+                "user_code_runner": "ubunet",
+                "local_unit": "click-index/3",
+            }, result)
+
+    def test_output_with_relation(self):
+        self.mock_config.return_value = charmhelpers.core.hookenv.Serializable({
+           'group_code_owner': 'webops_deploy',
+           'user_code_runner': 'ubunet',
+        })
+        self.mock_relation_get.return_value = {
+            'relation_key1': 'relation_value1',
+            'relation_key2': 'relation_value2',
+        }
+        self.mock_local_unit.return_value = "click-index/3"
+
+        charmhelpers.contrib.saltstack.juju_config_2_grains()
+
+        with open(self.grain_path, 'r') as grain_file:
+            result = yaml.load(grain_file.read())
+            self.assertEqual({
+                "charm_dir": "/tmp/charm_dir",
+                "group_code_owner": "webops_deploy",
+                "user_code_runner": "ubunet",
+                "relation_key1": "relation_value1",
+                "relation_key2": "relation_value2",
+                "local_unit": "click-index/3",
+            }, result)