Reckoner v1.0.1 (#69)

* Added version bump

* Fixed error handling

Reckoner now raises an error to the user and is handled gracefully by click.

* Added error handling to click plot command to expect reckoner exceptions
* Fixed inheritance of reckoner exceptions
* Added tests around this new behavior
* Added testing to assure if at least a single chart in --only is correct, the run still succeeds but warns you at the end of missing charts

* Adjustments

Fixed the logging behavior of hook events: Now hooks will alwyas show the output they have (stderr and/or stdout).

* Fixed bug with incorrect impl for python2 for truthy class objects
* Added tests to assure we're logging errors on failed exit codes
* Added tests to assure we're catching uncaught exceptions so click can handle them

* changelog

* Fixing output visibility

* Added tests around not firing webhooks

* Added text and fixed sig

* Added Issue 60

* Assertions for hooks running from working dir of course.yml
* Asserts for hooks being string or lists
* Renaming 'file' to not override the default behavior of 'file' in python

* Added changelog

* More testing of error raising

* The dirname() was returning "" empty white space which was causing subprocess to try and cd to "" dir which cannot exist. This may not have been caught if ./ was used in testing

* Added tests

Testing to assure that this contract doesn't break in the future. Assert that course_base_directory never returns an empty string for any valid URL paths

* Rearranged a test for better actionability on failures.

* Adjusted to ignore tests inclusion for packaging

CHANGELOG.md

@@ -4,6 +4,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [1.0.1]
+
+### Fixed
+* Reckoner would silently fail when a --only or --heading was defined for a
+  chart that didn't exist in your course.  
+  This behavior will still succeed if you provide at least one valid --heading
+  value. If no values are in your course then this bubbles up an error
+* Internal Fix: Reckoner Errors now are a single parent exception class
+* Fixed hook output to user instead of hidden only in debug (#59)
+* Fixed hooks to run from the same path as course.yml. This makes PWD relative
+  to course.yml for easier groking of paths for hook pre/post install.
+
 ## [1.0.0]
 
 ### Fixed

reckoner/chart.py

@@ -130,30 +130,59 @@ def post_install_hook(self):
 
     def run_hook(self, hook_type):
         """ Hook Type. Runs the commands defined by the hook """
-        commands = self.__get_hook(hook_type)
+        commands = self._get_hook(hook_type)
         if commands is None:
             return commands
         if type(commands) == str:
             commands = [commands]
 
-        for com in commands:
+        for command in commands:
             if self.config.local_development or self.config.dryrun:
-                logging.info("Hook not run due to --dry-run: {}".format(com))
+                logging.warning("Hook not run due to --dry-run: {}".format(command))
                 continue
             else:
-                logging.info("Running {} hook.".format(hook_type))
+                logging.info("Running {} hook...".format(hook_type))
 
             try:
-                r = call(com, shell=True, executable="/bin/bash")
-                logging.debug(r)
-            except ReckonerCommandException, e:
+                result = call(
+                    command,
+                    shell=True,
+                    executable="/bin/bash",
+                    path=self.config.course_base_directory
+                )
+            except Exception as error:
+                logging.error(error)
+                raise ReckonerCommandException(
+                    "Uncaught exception while running hook "
+                    "'{}'".format(command)
+                )
+
+            logging.info("Ran Hook: '{}'".format(result.command_string))
+            _output_level = logging.INFO  # The level to log the command output
+
+            # HACK We're not relying opon the truthiness of this object due to
+            # the inability to mock is well in code
+            # Python 3 may have better mocking functionality or we should
+            # refactor to leverage more method on the function for .succeeded()
+            # or something similar.
+            if result.exitcode == 0:
+                logging.info("{} hook ran successfully".format(hook_type))
+            else:
                 logging.error("{} hook failed to run".format(hook_type))
-                logging.debug('Hook stderr {}'.format(e.stderr))
-                raise e
+                logging.error("Returned exit code: {}".format(result.exitcode))
+                # Override message level response to bubble up error visibility
+                _output_level = logging.ERROR
+            # only print stdout if there is content
+            if result.stdout:
+                logging.log(_output_level,
+                            "Returned stdout: {}".format(result.stdout))
+            # only print stderr if there is content
+            if result.stderr:
+                logging.log(_output_level,
+                            "Returned stderr: {}".format(result.stderr))
 
     def rollback(self):
         """ Rollsback most recent release of the course chart """
-
         release = [release for release in self.helm.releases.deployed if release.name == self._release_name][0]
         if release:
             release.rollback()
@@ -204,7 +233,7 @@ def install(self, namespace=None, context=None):
         self.build_helm_arguments_for_chart()
 
         # Check and Error if we're missing required env vars
-        self.__check_env_vars()
+        self._check_env_vars()
 
         # Perform the upgrade with the arguments
         try:
@@ -269,30 +298,6 @@ def build_helm_arguments_for_chart(self):
         # Build the list of --set-string arguments
         self.build_set_string_arguments()
 
-    def _merge_set_and_values(self):
-        """
-        This is a temporary method that will be gone once the values: vs set:
-        debacle has been resolved. (See https://github.com/reactiveops/reckoner/issues/7)
-
-        NOTE ONLY RUN THIS ONCE BECAUSE IT'S NOT IDEMPOTENT
-        """
-        if self._hack_set_values_already_merged:
-            raise StandardError('This method cannot be called twice. '
-                                'If you are seeing this please open an '
-                                'issue in github.')
-
-        def merge_dicts(values, sets):
-            """This does a dict merge and prefers "sets" values"""
-            new_dict = values.copy()
-            new_dict.update(sets)
-            return new_dict
-
-        logging.debug('Merging values: into sets: - sets: take precedence.')
-        logging.debug('Original value of sets: {}'.format(self.set_values))
-        self.set_values = merge_dicts(self.values, self.set_values)
-        logging.debug('New value of sets: {}'.format(self.set_values))
-        self._hack_set_values_already_merged = True
-
     def build_temp_values_files(self):
         """
         This method builds temporary files based on the values: settings
@@ -372,14 +377,38 @@ def _format_set_list(self, key, value):
     def __getattr__(self, key):
         return self._chart.get(key)
 
+    def _merge_set_and_values(self):
+        """
+        This is a temporary method that will be gone once the values: vs set:
+        debacle has been resolved. (See https://github.com/reactiveops/reckoner/issues/7)
+
+        NOTE ONLY RUN THIS ONCE BECAUSE IT'S NOT IDEMPOTENT
+        """
+        if self._hack_set_values_already_merged:
+            raise StandardError('This method cannot be called twice. '
+                                'If you are seeing this please open an '
+                                'issue in github.')
+
+        def merge_dicts(values, sets):
+            """This does a dict merge and prefers "sets" values"""
+            new_dict = values.copy()
+            new_dict.update(sets)
+            return new_dict
+
+        logging.debug('Merging values: into sets: - sets: take precedence.')
+        logging.debug('Original value of sets: {}'.format(self.set_values))
+        self.set_values = merge_dicts(self.values, self.set_values)
+        logging.debug('New value of sets: {}'.format(self.set_values))
+        self._hack_set_values_already_merged = True
+
     def __str__(self):
         return str(dict(self._chart))
 
-    def __get_hook(self, hook_type):
+    def _get_hook(self, hook_type):
         if self.hooks is not None:
             return self.hooks.get(hook_type)
 
-    def __check_env_vars(self):
+    def _check_env_vars(self):
         """
         accepts list of args
         if any of those appear to be env vars

reckoner/cli.py

@@ -15,14 +15,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import logging
 import coloredlogs
 import click
-import shutil
-from reckoner import Reckoner
-import pkg_resources
-
-from meta import __version__
+from .reckoner import Reckoner
+import exception
+from .meta import __version__
 
 
 @click.group(invoke_without_command=True)
@@ -39,7 +36,7 @@ def cli(ctx, log_level, *args, **kwargs):
 
 @cli.command()
 @click.pass_context
-@click.argument('file', type=click.File('rb'))
+@click.argument('course_file', type=click.File('rb'))
 @click.option("--dry-run", is_flag=True, help='Pass --dry-run to helm so no action is taken. Implies --debug and '
                                               'skips hooks.')
 @click.option("--debug", is_flag=True, help='DEPRECATED - use --dry-run instead, or pass to --helm-args')
@@ -51,10 +48,16 @@ def cli(ctx, log_level, *args, **kwargs):
                                                                        'where Tiller is not required and no helm '
                                                                        'commands are run. Useful for rapid or offline '
                                                                        'development.')
-def plot(ctx, file=None, dry_run=False, debug=False, only=None, helm_args=None, local_development=False):
+def plot(ctx, course_file=None, dry_run=False, debug=False, only=None, helm_args=None, local_development=False):
     """ Install charts with given arguments as listed in yaml file argument """
-    h = Reckoner(file=file, dryrun=dry_run, debug=debug, helm_args=helm_args, local_development=local_development)
-    h.install(only)
+    try:
+        h = Reckoner(course_file=course_file, dryrun=dry_run, debug=debug, helm_args=helm_args, local_development=local_development)
+        # Convert tuple to list
+        only = list(only)
+        h.install(only)
+    except exception.ReckonerException:
+        # This handles exceptions cleanly, no expected stack traces from reckoner code
+        ctx.exit(1)
 
 
 @cli.command()

reckoner/command_line_caller/__init__.py

@@ -34,31 +34,33 @@ class Response(object):
     - exitcode
 
     Returns:
-    - Instance of Response() is truthy where Reponse.exitcode == 0
-    - Instance Response() is falsey where Reponse.exitcode != 0
+    - Instance of Response() is truthy where Response.exitcode == 0
+    - Instance Response() is falsey where Response.exitcode != 0
     """
 
-    def __init__(self, stdout, stderr, exitcode):
+    def __init__(self, stdout, stderr, exitcode, command_string):
 
         self._dict = {}
         self._dict['stdout'] = stdout
         self._dict['stderr'] = stderr
         self._dict['exitcode'] = exitcode
+        self._dict['command_string'] = command_string
 
     def __getattr__(self, name):
         return self._dict.get(name)
 
     def __str__(self):
         return str(self._dict)
 
-    def __bool__(self):
+    # TODO Python3 candidate for migration to __bool__()
+    def __nonzero__(self):
         return not self._dict['exitcode']
 
     def __eq__(self, other):
         return self._dict == other._dict
 
 
-def call(args, shell=False, executable=None):
+def call(args, shell=False, executable=None, path=None):
     """
     Description:
     - Wrapper for subprocess.Popen. Joins `args` and passes
@@ -75,8 +77,8 @@ def call(args, shell=False, executable=None):
     else:
         args_string = ' '.join(args)
     logging.debug(args_string)
-    p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=shell, executable=executable)
+    p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=shell, executable=executable, cwd=path)
     stdout, stderr = p.communicate()
     exitcode = p.returncode
 
-    return Response(stdout, stderr, exitcode)
+    return Response(stdout, stderr, exitcode, args_string)

reckoner/command_line_caller/tests/test_command_line_caller.py

@@ -0,0 +1,22 @@
+import unittest
+from reckoner.command_line_caller import Response
+
+
+class TestResponse(unittest.TestCase):
+    def test_properties(self):
+        required_attrs = {
+            'stdout': 'stdout value',
+            'stderr': 'stderr value',
+            'exitcode': 0,
+            'command_string': 'command -that -ran',
+        }
+        response = Response(**required_attrs)
+        for attr, value in required_attrs.items():
+            self.assertEqual(getattr(response, attr), value)
+
+    def test_bool(self):
+        response = Response('', '', 127, '')
+        self.assertFalse(response, "Any response where exitcode is not 0 should return false")
+
+        response = Response('', '', 0, '')
+        self.assertTrue(response, "All responses with exitcode 0 should return True")

reckoner/config.py

@@ -17,6 +17,7 @@
 
 import os
 import logging
+from os.path import dirname
 
 from .command_line_caller import call
 
@@ -67,6 +68,10 @@ def archive(self):
 
         return self._config['archive']
 
+    @property
+    def course_base_directory(self):
+        return dirname(self.course_path) or None
+
     def __setattr__(self, name, value):
         object.__setattr__(self, name, value)
 

reckoner/course.py

@@ -21,11 +21,11 @@
 
 import oyaml as yaml
 
-from config import Config
-from chart import Chart
-from repository import Repository
-from exception import MinimumVersionException, ReckonerCommandException
-from helm.client import HelmClient, HelmClientException
+from .config import Config
+from .chart import Chart
+from .repository import Repository
+from .exception import MinimumVersionException, ReckonerCommandException, NoChartsToInstall
+from .helm.client import HelmClient, HelmClientException
 
 from meta import __version__ as reckoner_version
 
@@ -47,12 +47,12 @@ class Course(object):
 
     """
 
-    def __init__(self, file):
+    def __init__(self, course_file):
         """
         Parse course.yml contents into instances.
         """
         self.config = Config()
-        self._dict = yaml.load(file)
+        self._dict = yaml.load(course_file)
         if not self.config.helm_args:
             self.config.helm_args = self._dict.get('helm_args')
         self.helm = HelmClient(default_helm_arguments=self.config.helm_args)
@@ -103,7 +103,6 @@ def plot(self, charts_to_install):
         charts in the course and calls Chart.install()
 
         """
-        _charts = []
         _failed_charts = []
         self._charts_to_install = []
 
@@ -115,6 +114,19 @@ def plot(self, charts_to_install):
         for chart in self.charts:
             if chart.release_name in charts_to_install:
                 self._charts_to_install.append(chart)
+                charts_to_install.remove(chart.release_name)
+            else:
+                logging.debug(
+                    'Skipping {} in course.yml, not found '
+                    'in your requested charts list'.format(chart.release_name)
+                )
+
+        if len(self._charts_to_install) == 0:
+            raise NoChartsToInstall(
+                'No charts found from requested list ({}). They do not exist '
+                'in the course.yml. Verify you are using the release-name '
+                'and not the chart name.'.format(', '.join(charts_to_install))
+            )
 
         for chart in self._charts_to_install:
             logging.info("Installing {}".format(chart.release_name))
@@ -125,15 +137,24 @@ def plot(self, charts_to_install):
                     logging.error(e.stderr)
                 if type(e) == Exception:
                     logging.error(e)
-                logging.error('Helm upgrade failed. Rolling back {}'.format(chart.release_name))
+                logging.error('Helm upgrade failed on {}'.format(chart.release_name))
                 logging.debug(traceback.format_exc())
-                chart.rollback
+                # chart.rollback #TODO Fix this - it doesn't actually fire or work
                 _failed_charts.append(chart)
 
         if _failed_charts:
-            logging.error("ERROR: Some charts failed to install and were rolled back")
+            logging.error("ERROR: Some charts failed to install.")
             for chart in _failed_charts:
                 logging.error(" - {}".format(chart.release_name))
+
+        if charts_to_install:
+            for missing_chart in charts_to_install:
+                logging.warning(
+                    'Could not find {} in course.yml'.format(missing_chart)
+                )
+            logging.warning('Some of the requested charts were not found in '
+                            'your course.yml')
+
         return True
 
     def _compare_required_versions(self):