Fix BZ1368296 by quietly recollecting facts if the cache is removed

* Add python logging
* Add testing system via 'make ci'

9 files changed, 220 insertions, 43 deletions

diff --git a/utils/.gitignore b/utils/.gitignore
index 68759c0ba..7e72a43c3 100644
--- a/utils/.gitignore
+++ b/utils/.gitignore
@@ -43,3 +43,5 @@ coverage.xml
 
 # Sphinx documentation
 docs/_build/
+oo-install
+oo-installenv
diff --git a/utils/Makefile b/utils/Makefile
new file mode 100644
index 000000000..55696afe5
--- /dev/null
+++ b/utils/Makefile
@@ -0,0 +1,83 @@
+########################################################
+
+# Makefile for OpenShift: Atomic Quick Installer
+#
+# useful targets (not all implemented yet!):
+#   make clean               -- Clean up garbage
+#   make ci ------------------- Execute CI steps (for travis or jenkins)
+
+########################################################
+
+# > VARIABLE = value
+#
+# Normal setting of a variable - values within it are recursively
+# expanded when the variable is USED, not when it's declared.
+#
+# > VARIABLE := value
+#
+# Setting of a variable with simple expansion of the values inside -
+# values within it are expanded at DECLARATION time.
+
+########################################################
+
+
+NAME := oo-install
+TESTPACKAGE := oo-install
+SHORTNAME := ooinstall
+
+sdist: clean
+	python setup.py sdist
+	rm -fR $(SHORTNAME).egg-info
+
+clean:
+	@find . -type f -regex ".*\.py[co]$$" -delete
+	@find . -type f \( -name "*~" -or -name "#*" \) -delete
+	@rm -fR build dist rpm-build MANIFEST htmlcov .coverage cover ooinstall.egg-info oo-install
+	@rm -fR $(NAME)env
+
+virtualenv:
+	@echo "#############################################"
+	@echo "# Creating a virtualenv"
+	@echo "#############################################"
+	virtualenv $(NAME)env
+	. $(NAME)env/bin/activate && pip install -r requirements.txt
+	. $(NAME)env/bin/activate && pip install pep8 nose coverage mock flake8 PyYAML click
+
+#       If there are any special things to install do it here
+#       . $(NAME)env/bin/activate && INSTALL STUFF
+
+ci-unittests:
+	@echo "#############################################"
+	@echo "# Running Unit Tests in virtualenv"
+	@echo "#############################################"
+#	. $(NAME)env/bin/activate && nosetests -v --with-cover --cover-html --cover-min-percentage=80 --cover-package=$(TESTPACKAGE) test/
+	. $(NAME)env/bin/activate && nosetests -v test/
+
+ci-pylint:
+	@echo "#############################################"
+	@echo "# Running PyLint Tests in virtualenv"
+	@echo "#############################################"
+	python -m pylint --rcfile ../git/.pylintrc src/ooinstall/cli_installer.py src/ooinstall/oo_config.py src/ooinstall/openshift_ansible.py src/ooinstall/variants.py
+
+ci-list-deps:
+	@echo "#############################################"
+	@echo "# Listing all pip deps"
+	@echo "#############################################"
+	. $(NAME)env/bin/activate && pip freeze
+
+ci-pep8:
+	@echo "#############################################"
+	@echo "# Running PEP8 Compliance Tests in virtualenv"
+	@echo "#############################################"
+	@echo "Skipping PEP8 tests until we clean them up"
+# . $(NAME)env/bin/activate && pep8 --ignore=E501,E121,E124 src/$(SHORTNAME)/
+
+ci-pep8-real:
+	@echo "#############################################"
+	@echo "# Running PEP8 Compliance Tests in virtualenv"
+	@echo "#############################################"
+	. $(NAME)env/bin/activate && pep8 --ignore=E501,E121,E124 src/$(SHORTNAME)/
+
+
+ci: clean virtualenv ci-list-deps ci-pylint ci-pep8 ci-unittests
+	:
diff --git a/utils/README.md b/utils/README.md
new file mode 100644
index 000000000..bead4c44f
--- /dev/null
+++ b/utils/README.md
@@ -0,0 +1,37 @@
+# Running Tests (NEW)
+
+Run the command:
+
+    make ci
+
+to run an array of unittests locally.
+
+# Running From Source
+
+You will need to setup a **virtualenv** to run from source:
+
+    $ virtualenv oo-install
+    $ source ./oo-install/bin/activate
+    $ virtualenv --relocatable ./oo-install/
+    $ python setup.py install
+
+The virtualenv `bin` directory should now be at the start of your
+`$PATH`, and `oo-install` is ready to use from your shell.
+
+You can exit the virtualenv with:
+
+    $ deactivate
+
+# Testing (OLD)
+
+*This section is deprecated, but still works*
+
+First, run the **virtualenv setup steps** described above.
+
+Install some testing libraries: (we cannot do this via setuptools due to the version virtualenv bundles)
+
+$ pip install mock nose
+
+Then run the tests with:
+
+$ oo-install/bin/nosetests
diff --git a/utils/README.txt b/utils/README.txt
deleted file mode 100644
index 6a6a1d24d..000000000
--- a/utils/README.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-## Running From Source
-
-You will need to setup a virtualenv to run from source and execute the unit tests.
-
-$ virtualenv oo-install
-$ source ./oo-install/bin/activate
-$ virtualenv --relocatable ./oo-install/
-$ python setup.py install
-
-The virtualenv bin directory should now be at the start of your $PATH, and oo-install is ready to use from your shell.
-
-You can exit the virtualenv with:
-
-$ deactivate
-
-## Testing
-
-Install some testing libraries: (we cannot do this via setuptools due to the version virtualenv bundles)
-
-$ pip install mock nose
-
-Then run the tests with:
-
-$ oo-install/bin/nosetests
diff --git a/utils/requirements.txt b/utils/requirements.txt
new file mode 100644
index 000000000..8b1378917
--- /dev/null
+++ b/utils/requirements.txt
@@ -0,0 +1 @@
+
diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py
index 2b070a3d2..a8595177c 100644
--- a/utils/src/ooinstall/cli_installer.py
+++ b/utils/src/ooinstall/cli_installer.py
@@ -13,6 +13,17 @@ from ooinstall.oo_config import OOConfigInvalidHostError
 from ooinstall.oo_config import Host, Role
 from ooinstall.variants import find_variant, get_variant_version_combos
 
+import logging
+installer_log = logging.getLogger('installer')
+installer_log.setLevel(logging.CRITICAL)
+installer_file_handler = logging.FileHandler('/tmp/installer.txt')
+installer_file_handler.setFormatter(
+    logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s'))
+# Example output:
+#   2016-08-23 07:34:58,480 - installer - DEBUG - Going to 'load_system_facts'
+installer_file_handler.setLevel(logging.DEBUG)
+installer_log.addHandler(installer_file_handler)
+
 DEFAULT_ANSIBLE_CONFIG = '/usr/share/atomic-openshift-utils/ansible.cfg'
 DEFAULT_PLAYBOOK_DIR = '/usr/share/ansible/openshift-ansible/'
 
@@ -798,11 +809,14 @@ def set_infra_nodes(hosts):
     default="/tmp/ansible.log")
 @click.option('-v', '--verbose',
     is_flag=True, default=False)
+@click.option('-d', '--debug',
+    help="Enable installer debugging (/tmp/installer.log)",
+    is_flag=True, default=False)
 @click.help_option('--help', '-h')
 #pylint: disable=too-many-arguments
 #pylint: disable=line-too-long
 # Main CLI entrypoint, not much we can do about too many arguments.
-def cli(ctx, unattended, configuration, ansible_playbook_directory, ansible_config, ansible_log_path, verbose):
+def cli(ctx, unattended, configuration, ansible_playbook_directory, ansible_config, ansible_log_path, verbose, debug):
     """
     atomic-openshift-installer makes the process for installing OSE or AEP
     easier by interactively gathering the data needed to run on each host.
@@ -810,6 +824,14 @@ def cli(ctx, unattended, configuration, ansible_playbook_directory, ansible_conf
 
     Further reading: https://docs.openshift.com/enterprise/latest/install_config/install/quick_install.html
     """
+    if debug:
+        # DEFAULT log level threshold is set to CRITICAL (the
+        # highest), anything below that (we only use debug/warning
+        # presently) is not logged. If '-d' is given though, we'll
+        # lower the threshold to debug (almost everything gets through)
+        installer_log.setLevel(logging.DEBUG)
+        installer_log.debug("Quick Installer debugging initialized")
+
     ctx.obj = {}
     ctx.obj['unattended'] = unattended
     ctx.obj['configuration'] = configuration
@@ -991,7 +1013,6 @@ def install(ctx, force, gen_inventory):
     hosts_to_run_on, callback_facts = get_hosts_to_run_on(
         oo_cfg, callback_facts, ctx.obj['unattended'], force, verbose)
 
-
     # We already verified this is not the case for unattended installs, so this can
     # only trigger for live CLI users:
     # TODO: if there are *new* nodes and this is a live install, we may need the  user
diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py
index f2990662e..0cd5fdabf 100644
--- a/utils/src/ooinstall/oo_config.py
+++ b/utils/src/ooinstall/oo_config.py
@@ -5,6 +5,9 @@ import sys
 import yaml
 from pkg_resources import resource_filename
 
+import logging
+installer_log = logging.getLogger('installer')
+
 CONFIG_PERSIST_SETTINGS = [
     'ansible_ssh_user',
     'ansible_callback_facts_yaml',
@@ -25,6 +28,15 @@ DEPLOYMENT_VARIABLES_BLACKLIST = [
 DEFAULT_REQUIRED_FACTS = ['ip', 'public_ip', 'hostname', 'public_hostname']
 PRECONFIGURED_REQUIRED_FACTS = ['hostname', 'public_hostname']
 
+def print_read_config_error(error, path='the configuration file'):
+            message = """
+Error loading config. {}.
+
+See https://docs.openshift.com/enterprise/latest/install_config/install/quick_install.html#defining-an-installation-configuration-file
+for information on creating a configuration file or delete {} and re-run the installer.
+"""
+            print message.format(error, path)
+
 class OOConfigFileError(Exception):
     """The provided config file path can't be read/written
     """
@@ -164,25 +176,20 @@ class OOConfig(object):
         self._read_config()
         self._set_defaults()
 
-
-# pylint: disable=too-many-branches
+    # pylint: disable=too-many-branches
+    #         Lots of different checks ran in a single method, could
+    #         use a little refactoring-love some time
     def _read_config(self):
-        def _print_read_config_error(error, path='the configuration file'):
-            message = """
-Error loading config. {}.
-
-See https://docs.openshift.com/enterprise/latest/install_config/install/quick_install.html#defining-an-installation-configuration-file
-for information on creating a configuration file or delete {} and re-run the installer.
-"""
-            print message.format(error, path)
-
+        installer_log.debug("Attempting to read the OO Config")
         try:
+            installer_log.debug("Attempting to see if the provided config file exists: %s", self.config_path)
             if os.path.exists(self.config_path):
+                installer_log.debug("We think the config file exists: %s", self.config_path)
                 with open(self.config_path, 'r') as cfgfile:
                     loaded_config = yaml.safe_load(cfgfile.read())
 
                 if not 'version' in loaded_config:
-                    _print_read_config_error('Legacy configuration file found', self.config_path)
+                    print_read_config_error('Legacy configuration file found', self.config_path)
                     sys.exit(0)
 
                 if loaded_config.get('version', '') == 'v1':
@@ -192,14 +199,31 @@ for information on creating a configuration file or delete {} and re-run the ins
                     host_list = loaded_config['deployment']['hosts']
                     role_list = loaded_config['deployment']['roles']
                 except KeyError as e:
-                    _print_read_config_error("No such key: {}".format(e), self.config_path)
+                    print_read_config_error("No such key: {}".format(e), self.config_path)
+                    print "Error loading config, required key missing: {}".format(e)
                     sys.exit(0)
 
                 for setting in CONFIG_PERSIST_SETTINGS:
-                    try:
-                        self.settings[setting] = str(loaded_config[setting])
-                    except KeyError:
-                        continue
+                    persisted_value = loaded_config.get(setting)
+                    if persisted_value is not None:
+                        self.settings[setting] = str(persisted_value)
+
+                # We've loaded any persisted configs, let's verify any
+                # paths which are required for a correct and complete
+                # install
+
+                # - ansible_callback_facts_yaml - Settings from a
+                #   pervious run. If the file doesn't exist then we
+                #   will just warn about it for now and recollect the
+                #   facts.
+                if self.settings.get('ansible_callback_facts_yaml', None) is not None:
+                    if not os.path.exists(self.settings['ansible_callback_facts_yaml']):
+                        # Cached callback facts file does not exist
+                        installer_log.warning("The specified 'ansible_callback_facts_yaml'"
+                                              "file does not exist (%s)",
+                                              self.settings['ansible_callback_facts_yaml'])
+                        installer_log.debug("Remote system facts will be collected again later")
+                        self.settings.pop('ansible_callback_facts_yaml')
 
                 for setting in loaded_config['deployment']:
                     try:
@@ -224,6 +248,8 @@ for information on creating a configuration file or delete {} and re-run the ins
         except yaml.scanner.ScannerError:
             raise OOConfigFileError(
                 'Config file "{}" is not a valid YAML document'.format(self.config_path))
+        installer_log.debug("Parsed the config file")
+
 
     def _upgrade_v1_config(self, config):
         new_config_data = {}
@@ -257,11 +283,21 @@ for information on creating a configuration file or delete {} and re-run the ins
         return new_config_data
 
     def _set_defaults(self):
+        installer_log.debug("Setting defaults, current OOConfig settings: %s", self.settings)
 
         if 'ansible_inventory_directory' not in self.settings:
             self.settings['ansible_inventory_directory'] = self._default_ansible_inv_dir()
+
         if not os.path.exists(self.settings['ansible_inventory_directory']):
+            installer_log.debug("'ansible_inventory_directory' does not exist, "
+                                "creating it now (%s)",
+                                self.settings['ansible_inventory_directory'])
             os.makedirs(self.settings['ansible_inventory_directory'])
+        else:
+            installer_log.debug("We think this 'ansible_inventory_directory' "
+                                "is OK: %s",
+                                self.settings['ansible_inventory_directory'])
+
         if 'ansible_plugins_directory' not in self.settings:
             self.settings['ansible_plugins_directory'] = \
                 resource_filename(__name__, 'ansible_plugins')
@@ -269,8 +305,14 @@ for information on creating a configuration file or delete {} and re-run the ins
             self.settings['version'] = 'v2'
 
         if 'ansible_callback_facts_yaml' not in self.settings:
+            installer_log.debug("No 'ansible_callback_facts_yaml' in self.settings")
             self.settings['ansible_callback_facts_yaml'] = '%s/callback_facts.yaml' % \
                 self.settings['ansible_inventory_directory']
+            installer_log.debug("Value: %s", self.settings['ansible_callback_facts_yaml'])
+        else:
+            installer_log.debug("'ansible_callback_facts_yaml' already set "
+                                "in self.settings: %s",
+                                self.settings['ansible_callback_facts_yaml'])
 
         if 'ansible_ssh_user' not in self.settings:
             self.settings['ansible_ssh_user'] = ''
@@ -283,6 +325,8 @@ for information on creating a configuration file or delete {} and re-run the ins
             if not self.settings[setting]:
                 self.settings.pop(setting)
 
+        installer_log.debug("Updated OOConfig settings: %s", self.settings)
+
     def _default_ansible_inv_dir(self):
         return os.path.normpath(
             os.path.dirname(self.config_path) + "/.ansible")
diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py
index ef7906828..570b48dda 100644
--- a/utils/src/ooinstall/openshift_ansible.py
+++ b/utils/src/ooinstall/openshift_ansible.py
@@ -6,6 +6,8 @@ import sys
 import os
 import yaml
 from ooinstall.variants import find_variant
+import logging
+installer_log = logging.getLogger('installer')
 
 CFG = None
 
@@ -216,17 +218,21 @@ def load_system_facts(inventory_file, os_facts_path, env_vars, verbose=False):
     """
     Retrieves system facts from the remote systems.
     """
+    installer_log.debug("Inside load_system_facts")
     FNULL = open(os.devnull, 'w')
     args = ['ansible-playbook', '-v'] if verbose \
         else ['ansible-playbook']
     args.extend([
         '--inventory-file={}'.format(inventory_file),
         os_facts_path])
+    installer_log.debug("Going to subprocess out to ansible now with these args: %s", ' '.join(args))
     status = subprocess.call(args, env=env_vars, stdout=FNULL)
     if not status == 0:
+        installer_log.debug("Exit status from subprocess was not 0")
         return [], 1
 
     with open(CFG.settings['ansible_callback_facts_yaml'], 'r') as callback_facts_file:
+        installer_log.debug("Going to try to read this file: %s", CFG.settings['ansible_callback_facts_yaml'])
         try:
             callback_facts = yaml.safe_load(callback_facts_file)
         except yaml.YAMLError, exc:
@@ -239,6 +245,7 @@ def load_system_facts(inventory_file, os_facts_path, env_vars, verbose=False):
 
 def default_facts(hosts, verbose=False):
     global CFG
+    installer_log.debug("Current global CFG vars here: %s", CFG)
     inventory_file = generate_inventory(hosts)
     os_facts_path = '{}/playbooks/byo/openshift_facts.yml'.format(CFG.ansible_playbook_directory)
 
@@ -250,6 +257,9 @@ def default_facts(hosts, verbose=False):
         facts_env["ANSIBLE_LOG_PATH"] = CFG.settings['ansible_log_path']
     if 'ansible_config' in CFG.settings:
         facts_env['ANSIBLE_CONFIG'] = CFG.settings['ansible_config']
+
+    installer_log.debug("facts_env: %s", facts_env)
+    installer_log.debug("Going to 'load_system_facts' next")
     return load_system_facts(inventory_file, os_facts_path, facts_env, verbose)
 
 
diff --git a/utils/src/ooinstall/variants.py b/utils/src/ooinstall/variants.py
index b32370cd5..ce4d772ee 100644
--- a/utils/src/ooinstall/variants.py
+++ b/utils/src/ooinstall/variants.py
@@ -11,6 +11,9 @@ to be specified by the user, and to point the generic variants to the latest
 version.
 """
 
+import logging
+installer_log = logging.getLogger('installer')
+
 
 class Version(object):
     def __init__(self, name, ansible_key):