fix: change detection and speed for releases_managed

* clean up many of the function calls (use keyword arguments
for most of the Helm global parameters)
* improve communication of changes to end user
* add documentation to all module (but not state) functions

Refs salt-formulas/salt-formula-helm#6
Refs salt-formulas/salt-formula-helm#9
diff --git a/README.rst b/README.rst
index 91bb364..aeb10c0 100644
--- a/README.rst
+++ b/README.rst
@@ -4,7 +4,88 @@
 ============
 
 This formula installs Helm client, installs Tiller on Kubernetes cluster and
-creates releases in it.
+manages releases in it.
+
+Availale States
+===============
+
+The default state applied by this formula (e.g. if just applying `helm`) will
+apply the `helm.releases_managed` state.
+
+`kubectl_installed`
+------------------
+
+Optionally installs the kubectl binary per the configured pillar values,
+such as the version of `kubectl` to instlal and the path where the binary should
+be installed.
+
+`kubectl_configured`
+------------------
+
+Manages a kubectl configuration file and gce_token json file per the configured
+pillar values. Note that the available configuration values allow the path of
+the kube config file to be placed at a different location than the default 
+installation path; this is recommended to avoid confusion if the kubectl 
+binary on the minion might be manually used with multiple contexts.
+
+**includes**:
+* `kubectl_installed`
+
+`client_installed`
+------------------
+
+Installs the helm client binary per the configured pillar values, such as where 
+helm home should be, which version of the helm binary to install and that path
+for the helm binary.
+
+**includes**:
+* `kubectl_installed
+
+`tiller_installed`
+------------------
+
+Optionally installs a Tiller deployment to the kubernetes cluster per the
+`helm:client:tiller:install` pillar value. If the pillar value is set to 
+install tiller to the cluster, the version of the tiller installation will
+match the version of the Helm client installed per the `helm:client:version`
+configuration parameter
+
+**includes**:
+* `client_installed`
+* `kubectl_configured`
+
+`repos_managed`
+------------------
+
+Ensures the repositories configured per the pillar (and only those repositories) 
+are registered at the configured helm home, and synchronizes the local cache 
+with the remote repository with each state execution.
+
+**includes**:
+* `client_installed`
+
+`releases_managed`
+------------------
+
+Ensures the releases configured with the pillar are in the expected state with
+the Kubernetes cluster. This state includes change detection to determine 
+whether the pillar configurations match the release's state in the cluster.
+
+Note that changes to an existing release's namespace will trigger a deletion and 
+re-installation of the release to the cluster.
+
+**includes**:
+* `client_installed`
+* `tiller_installed`
+* `kubectl_configured`
+* `repos_managed`
+
+Availale Modules
+===============
+
+To view documentation on the available modules, run: 
+
+`salt '{{ tgt }}' sys.doc helm`
 
 
 Sample pillars
@@ -120,6 +201,30 @@
             user_name: gce_user
             gce_service_token: base64_of_json_token_downloaded_from_cloud_console
 
+Known Issues
+============
+
+1. Unable to remove all user supplied values
+
+If a release previously has had user supplied value overrides (via the 
+release's `values` key in the pillar), subsequently removing all `values`
+overrides (so that there is no more `values` key for the release in the 
+pillar) will not actually update the Helm deployment. To get around this,
+specify a fake key/value pair in the release's pillar; Tiller will override
+all previously user-supplied values with the new fake key and value. For 
+example:
+
+
+.. code-block:: yaml
+    helm:
+      client:
+        releases:
+          zoo1:
+            enabled: true
+            ...
+            values:
+              fake_key: fake_value
+
 
 More Information
 ================
diff --git a/_modules/helm.py b/_modules/helm.py
index 087c751..a4ce0b3 100644
--- a/_modules/helm.py
+++ b/_modules/helm.py
@@ -1,4 +1,5 @@
 import logging
+import re
 
 from salt.serializers import yaml
 from salt.exceptions import CommandExecutionError
@@ -37,12 +38,46 @@
         'env': env,
     }
 
+def _parse_release(output):
+  result = {}
+  chart_match = re.search(r'CHART\: ([^0-9]+)-([^\s]+)', output)
+  if chart_match:
+    result['chart'] = chart_match.group(1)
+    result['version'] = chart_match.group(2)
+  
+  user_values_match = re.search(r"(?<=USER-SUPPLIED VALUES\:\n)(\n*.+)+?(?=\n*COMPUTED VALUES\:)", output, re.MULTILINE)
+  if user_values_match:
+    result['values'] = yaml.deserialize(user_values_match.group(0))
+
+  computed_values_match = re.search(r"(?<=COMPUTED VALUES\:\n)(\n*.+)+?(?=\n*HOOKS\:)", output, re.MULTILINE)
+  if computed_values_match:
+    result['computed_values'] = yaml.deserialize(computed_values_match.group(0))
+
+  manifest_match = re.search(r"(?<=MANIFEST\:\n)(\n*(?!Release \".+\" has been upgraded).*)+", output, re.MULTILINE)
+  if manifest_match:
+    result['manifest'] = manifest_match.group(0)
+
+  namespace_match = re.search(r"(?<=NAMESPACE\: )(.*)", output)
+  if namespace_match:
+    result['namespace'] = namespace_match.group(0)
+
+  return result
+
 def _parse_repo(repo_string = None):
   split_string = repo_string.split('\t')
   return {
     "name": split_string[0].strip(),
     "url": split_string[1].strip()
   }
+  
+
+def _get_release_namespace(name, tiller_namespace="kube-system", **kwargs):
+  cmd = _helm_cmd("list", name, **kwargs)
+  result = __salt__['cmd.run_stdout'](**cmd)
+  if not result or len(result.split("\n")) < 2:
+    return None
+
+  return result.split("\n")[1].split("\t")[5]
 
 def list_repos(**kwargs):
   '''
@@ -222,77 +257,92 @@
   cmd = _helm_cmd('repo', 'update', **kwargs)
   return __salt__['cmd.run_stdout'](**cmd)
 
-def release_exists(name, namespace='default',
-                   tiller_namespace='kube-system', tiller_host=None,
-                   kube_config=None, gce_service_token=None, helm_home=None):
-    cmd = _helm_cmd('list', '--short', '--all', '--namespace', namespace, name,
-                    tiller_namespace=tiller_namespace, tiller_host=tiller_host,
-                    kube_config=kube_config,
-                    gce_service_token=gce_service_token,
-                    helm_home=helm_home)
-    return __salt__['cmd.run_stdout'](**cmd) == name
+def get_release(name, tiller_namespace="kube-system", **kwargs):
+  '''
+  Get the parsed release metadata from calling `helm get {{ release }}` for the 
+  supplied release name, or None if no release is found. The following keys may 
+  or may not be in the returned dict:
 
+    * chart
+    * version
+    * values
+    * computed_values
+    * manifest
+    * namespace
+  '''
+  kwargs['tiller_namespace'] = tiller_namespace
+  cmd = _helm_cmd('get', name, **kwargs)
+  result = __salt__['cmd.run_stdout'](**cmd)
+  if not result:
+    return None
+
+  release = _parse_release(result)
+
+  #
+  # `helm get {{ release }}` doesn't currently (2.6.2) return the namespace, so 
+  # separately retrieve it if it's not available
+  #
+  if not 'namespace' in release:
+    release['namespace'] = _get_release_namespace(name, **kwargs)
+  return release
+
+def release_exists(name, tiller_namespace="kube-system", **kwargs):
+  '''
+  Determine whether a release exists in the cluster with the supplied name
+  '''
+  kwargs['tiller_namespace'] = tiller_namespace
+  return get_release(name, **kwargs) is not None
 
 def release_create(name, chart_name, namespace='default',
                    version=None, values_file=None,
-                   tiller_namespace='kube-system', tiller_host=None,
-                   kube_config=None, gce_service_token=None,
-                   helm_home=None):
-    kwargs = {
-        'tiller_namespace': tiller_namespace,
-        'tiller_host': tiller_host,
-        'kube_config': kube_config,
-        'gce_service_token': gce_service_token,
-        'helm_home': helm_home
-    }
+                   tiller_namespace='kube-system', **kwargs):
+    '''
+    Install a release. There must not be a release with the supplied name 
+    already installed to the Kubernetes cluster.
+
+    Note that if a release already exists with the specified name, you'll need
+    to use the release_upgrade function instead; unless the release is in a
+    different namespace, in which case you'll need to delete and purge the 
+    existing release (using release_delete) and *then* use this function to
+    install a new release to the desired namespace.
+    '''
     args = []
     if version is not None:
         args += ['--version', version]
     if values_file is not None:
         args += ['--values', values_file]
-    cmd = _helm_cmd('install', '--namespace', namespace,
-                    '--name', name, chart_name, *args, **kwargs)
+    cmd = _helm_cmd('install', '--namespace', namespace, '--name', name, chart_name, 
+                    *args, **kwargs)
     LOG.debug('Creating release with args: %s', cmd)
     return ok_or_output(cmd, 'Failed to create release "{}"'.format(name))
 
 
-def release_delete(name, tiller_namespace='kube-system', tiller_host=None,
-                   kube_config=None, gce_service_token=None, helm_home=None):
-    cmd = _helm_cmd('delete', '--purge', name,
-                    tiller_namespace=tiller_namespace, tiller_host=tiller_host,
-                    kube_config=kube_config,
-                    gce_service_token=gce_service_token,
-                    helm_home=helm_home)
+def release_delete(name, tiller_namespace='kube-system', **kwargs):
+    '''
+    Delete and purge any release found with the supplied name.
+    '''
+    kwargs['tiller_namespace'] = tiller_namespace
+    cmd = _helm_cmd('delete', '--purge', name, **kwargs)
     return ok_or_output(cmd, 'Failed to delete release "{}"'.format(name))
 
 
 def release_upgrade(name, chart_name, namespace='default',
                     version=None, values_file=None,
-                    tiller_namespace='kube-system', tiller_host=None,
-                    kube_config=None, gce_service_token=None, helm_home=None):
-    kwargs = {
-        'tiller_namespace': tiller_namespace,
-        'tiller_host': tiller_host,
-        'kube_config': kube_config,
-        'gce_service_token': gce_service_token,
-        'helm_home': helm_home
-    }
+                    tiller_namespace='kube-system', **kwargs):
+    '''
+    Upgrade an existing release. There must be a release with the supplied name
+    already installed to the Kubernetes cluster.
+
+    If attempting to change the namespace for the release, this function will
+    fail; you will need to first delete and purge the release and then use the
+    release_create function to create a new release in the desired namespace.
+    '''
+    kwargs['tiller_namespace'] = tiller_namespace
     args = []
     if version is not None:
       args += ['--version', version]
     if values_file is not None:
       args += ['--values', values_file]
-    cmd = _helm_cmd('upgrade', '--namespace', namespace,
-                    name, chart_name, *args, **kwargs)
+    cmd = _helm_cmd('upgrade', '--namespace', namespace, name, chart_name, **kwargs)
     LOG.debug('Upgrading release with args: %s', cmd)
     return ok_or_output(cmd, 'Failed to upgrade release "{}"'.format(name))
-
-
-def get_values(name, tiller_namespace='kube-system', tiller_host=None,
-               kube_config=None, gce_service_token=None, helm_home=None):
-    cmd = _helm_cmd('get', 'values', '--all', name,
-                    tiller_namespace=tiller_namespace, tiller_host=tiller_host,
-                    kube_config=kube_config,
-                    gce_service_token=gce_service_token,
-                    helm_home=helm_home)
-    return yaml.deserialize(__salt__['cmd.run_stdout'](**cmd))
diff --git a/_states/helm_release.py b/_states/helm_release.py
index 3c1be90..7a66b10 100644
--- a/_states/helm_release.py
+++ b/_states/helm_release.py
@@ -1,77 +1,160 @@
 import difflib
+import os 
+import logging
 
+from salt.exceptions import CommandExecutionError
 from salt.serializers import yaml
 
+LOG = logging.getLogger(__name__)
 
-def failure(name, message):
+def _get_values_from_file(values_file=None):
+    if values_file:
+      try:
+        with open(values_file) as values_stream:
+          values = yaml.deserialize(values_stream)
+        return values
+      except e:
+        raise CommandExecutionError("encountered error reading from values "
+                                    "file (%s): %s" % (values_file, e))
+    return None
+
+def _get_yaml_diff(new_yaml=None, old_yaml=None):
+  if not new_yaml and not old_yaml:
+    return None
+  
+  old_str = yaml.serialize(old_yaml, default_flow_style=False)
+  new_str = yaml.serialize(new_yaml, default_flow_style=False)
+  return difflib.unified_diff(old_str.split('\n'), new_str.split('\n'))
+
+def _failure(name, message, changes={}):
     return {
         'name': name,
-        'changes': {},
+        'changes': changes,
         'result': False,
         'comment': message,
     }
 
-
 def present(name, chart_name, namespace, version=None, values_file=None,
-            tiller_namespace='kube-system', tiller_host=None,
-            kube_config=None, gce_service_token=None, helm_home=None):
-    kwargs = {
-        'tiller_namespace': tiller_namespace,
-        'tiller_host': tiller_host,
-        'kube_config': kube_config,
-        'gce_service_token': gce_service_token,
-        'helm_home': helm_home
-    }
-    exists = __salt__['helm.release_exists'](name, namespace, **kwargs)
-    if not exists:
+            tiller_namespace='kube-system', **kwargs):
+    '''
+    Ensure that a release with the supplied name is in the desired state in the 
+    Tiller installation. This state will handle change detection to determine 
+    whether an installation or update needs to be made. 
+
+    In the event the namespace to which a release is installed changes, the
+    state will first delete and purge the release and the re-install it into
+    the new namespace, since Helm does not support updating a release into a 
+    new namespace.
+
+    name
+        The name of the release to ensure is present
+
+    chart_name
+        The name of the chart to install, including the repository name as 
+        applicable (such as `stable/mysql`)
+
+    namespace
+        The namespace to which the release should be (re-)installed
+
+    version
+        The version of the chart to install. Defaults to the latest version
+
+    values_file
+        The path to the a values file containing all the chart values that 
+        should be applied to the release. Note that this should not be passed
+        if there are not chart value overrides required.
+
+    '''
+    kwargs['tiller_namespace'] = tiller_namespace
+    old_release = __salt__['helm.get_release'](name, **kwargs)
+    if not old_release:
         err = __salt__['helm.release_create'](
-            name, chart_name, namespace, version, values_file, **kwargs)
+            name, chart_name, namespace, version, values_file, **kwargs
+        )
         if err:
-            return failure(name, err)
+            return _failure(name, err)
         return {
             'name': name,
-            'changes': {name: 'CREATED'},
+            'changes': {
+                'name': name,
+                'chart_name': chart_name,
+                'namespace': namespace,
+                'version': version,
+                'values': _get_values_from_file(values_file)
+            },
             'result': True,
             'comment': 'Release "{}" was created'.format(name),
         }
 
-    old_values = __salt__['helm.get_values'](name, **kwargs)
-    err = __salt__['helm.release_upgrade'](
-        name, chart_name, namespace, version, values_file, **kwargs)
-    if err:
-        return failure(name, err)
+    changes = {}
+    warnings = []
+    if old_release.get('chart') != chart_name.split("/")[1]:
+      changes['chart'] = { 'old': old_release['chart'], 'new': chart_name }
 
-    new_values = __salt__['helm.get_values'](name, **kwargs)
-    if new_values == old_values:
-        return {
-            'name': name,
-            'changes': {},
-            'result': True,
-            'comment': 'Release "{}" already exists'.format(name),
-        }
+    if old_release.get('version') != version:
+      changes['version'] = { 'old': old_release['version'], 'new': version }
 
-    old_str = yaml.serialize(old_values, default_flow_style=False)
-    new_str = yaml.serialize(new_values, default_flow_style=False)
-    diff = difflib.unified_diff(
-        old_str.split('\n'), new_str.split('\n'), lineterm='')
-    return {
+    if old_release.get('namespace') != namespace:
+        changes['namespace'] = { 'old': old_release['namespace'], 'new': namespace }
+
+    if (not values_file and old_release.get("values") or
+        not old_release.get("values") and values_file):
+      changes['values'] = { 'old': old_release['values'], 'new': values_file }
+
+    values = _get_values_from_file(values_file)
+    diff = _get_yaml_diff(values, old_release.get('values'))
+    
+    if diff:
+      diff_string = '\n'.join(diff)
+      if diff_string:
+        changes['values'] = diff_string
+
+    if not changes:
+      return {
         'name': name,
-        'changes': {'values': '\n'.join(diff)},
+        'result': True,
+        'changes': {},
+        'comment': 'Release "{}" is already in the desired state'.format(name)
+      }
+
+    module_fn = 'helm.release_upgrade'
+    if changes.get("namespace"):
+      LOG.debug("purging old release (%s) due to namespace change" % name)
+      err = __salt__['helm.release_delete'](name, **kwargs)
+      if err:
+        return _failure(name, err, changes)
+      module_fn = 'helm.release_create'
+      warnings.append('Release (%s) was replaced due to namespace change' % name)
+
+    err = __salt__[module_fn](
+        name, chart_name, namespace, version, values_file, **kwargs
+    )
+    if err:
+      return _failure(name, err, changes)
+
+    ret = {
+        'name': name,
+        'changes': changes,
         'result': True,
         'comment': 'Release "{}" was updated'.format(name),
     }
 
+    if warnings:
+      ret['warnings'] = warnings
 
-def absent(name, namespace, tiller_namespace='kube-system', tiller_host=None,
-           kube_config=None, gce_service_token=None, helm_home=None):
-    kwargs = {
-        'tiller_namespace': tiller_namespace,
-        'tiller_host': tiller_host,
-        'kube_config': kube_config,
-        'gce_service_token': gce_service_token,
-        'helm_home': helm_home
-    }
-    exists = __salt__['helm.release_exists'](name, namespace, **kwargs)
+    return ret
+
+
+def absent(name, tiller_namespace='kube-system', **kwargs):
+    '''
+    Ensure that any release with the supplied release name is absent from the
+    tiller installation.
+
+    name
+        The name of the release to ensure is absent
+    '''
+    kwargs['tiller_namespace'] = tiller_namespace
+    exists = __salt__['helm.release_exists'](name, **kwargs)
     if not exists:
         return {
             'name': name,
@@ -81,7 +164,7 @@
         }
     err = __salt__['helm.release_delete'](name, **kwargs)
     if err:
-        return failure(name, err)
+        return _failure(name, err)
     return {
         'name': name,
         'changes': {name: 'DELETED'},
diff --git a/helm/releases_managed.sls b/helm/releases_managed.sls
index 0be9dff..b0a84a6 100644
--- a/helm/releases_managed.sls
+++ b/helm/releases_managed.sls
@@ -10,20 +10,19 @@
 {%- for release_id, release in config.releases.items() %}
 {%- set release_name = release.get('name', release_id) %}
 {%- set namespace = release.get('namespace', 'default') %}
-{%- set values_file = None %}
-{%- if release.get('values') %}
 {%- set values_file = config.values_dir + "/" + release_name + ".yaml" %}
-{%- endif %}
-
 
 {%- if release.get('enabled', True) %}
 
-{%- if values_file %}
+{%- if release.get("values") %}
 {{ values_file }}:
   file.managed:
     - makedirs: True
     - contents: |
         {{ release['values'] | yaml(false) | indent(8) }}
+{%- else %}
+{{ values_file }}:
+  file.absent
 {%- endif %}
 
 ensure_{{ release_id }}_release:
@@ -38,7 +37,7 @@
     {%- if release.get('version') %}
     - version: {{ release['version'] }}
     {%- endif %}
-    {%- if values_file %}
+    {%- if release.get("values") %}
     - values_file: {{ values_file }}
     {%- endif %}
     - require:
@@ -54,7 +53,7 @@
 
 {%- else %}{# not release.enabled #}
 
-{%- if values_file %}
+{%- if release.get("values") %}
 {{ values_file }}:
   file.absent
 {%- endif %}