The operatingsystem fact on Fedora begins in a uppercase F

The error being thrown by this on Fedora was being silently ignored
added regex for this
Also adding adding a unit test for this
Adding the effected module to pep8 tests

Change-Id: I4e4d72e6de0bce597474434a4e18112f79913718

packstack/modules/ospluginutils.py

@@ -102,8 +102,12 @@ def isErrorException(line):
     return False
 
 
-_re_errorline = re.compile('err: | Syntax error at|^Duplicate definition:|^Parameter name failed:')
 _re_color = re.compile('\x1b.*?\d\dm')
+_re_errorline = re.compile('err: | Syntax error at|^Duplicate definition:|'
+                           '^Parameter name failed:|'
+                           '^No matching value for selector param')
+
+
 def validate_puppet_logfile(logfile):
     """
     Check a puppet log file for errors and raise an error if we find any
@@ -115,7 +119,7 @@ def validate_puppet_logfile(logfile):
     for line in data.split('\n'):
         line = line.strip()
 
-        if _re_errorline.search(line) == None:
+        if _re_errorline.search(line) is None:
             continue
 
         message = _re_color.sub('', line)  # remove colors

packstack/puppet/templates/postscript.pp

@@ -1,6 +1,6 @@
 exec { 'persist-firewall':
   command     => $operatingsystem ? {
-    'debian'          => '/sbin/iptables-save > /etc/iptables/rules.v4',
-    /(fedora|RedHat|CentOS)/ 		  => '/sbin/iptables-save > /etc/sysconfig/iptables',
+    'debian'                  => '/sbin/iptables-save > /etc/iptables/rules.v4',
+    /(Fedora|RedHat|CentOS)/  => '/sbin/iptables-save > /etc/sysconfig/iptables',
   },
-}
+}

tests/test.py

@@ -14,12 +14,17 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import shutil
+import tempfile
+
 from unittest import TestCase
 
 
 class TestCase(TestCase):
     def setUp(self):
-        pass
+        # Creating a temp directory that can be used by tests
+        self.tempdir = tempfile.mkdtemp()
 
     def tearDown(self):
-        pass
+        # remove the temp directory
+        shutil.rmtree(self.tempdir)

tests/test_ospluginutils.py

@@ -14,9 +14,12 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import os
 from test import TestCase
 
-from packstack.modules.ospluginutils import gethostlist
+from packstack.modules.ospluginutils import gethostlist, \
+                                            validate_puppet_logfile, \
+                                            PackStackError
 
 
 class OSPluginUtilsTestCase(TestCase):
@@ -26,3 +29,19 @@ class OSPluginUtilsTestCase(TestCase):
         hosts = gethostlist(conf)
         hosts.sort()
         self.assertEquals(['1.1.1.1', '2.2.2.2', '3.3.3.3'], hosts)
+
+    def test_validate_puppet_logfile(self):
+        filename = os.path.join(self.tempdir, "puppet.log")
+        fp = open(filename, "w")
+        fp.write("Everything went ok")
+        fp.close()
+
+        validate_puppet_logfile(filename)
+
+    def test_validate_puppet_logfile_error(self):
+        filename = os.path.join(self.tempdir, "puppet.log")
+        fp = open(filename, "w")
+        fp.write("No matching value for selector param 'Fedora' ...")
+        fp.close()
+
+        self.assertRaises(PackStackError, validate_puppet_logfile, filename)

tox.ini

@@ -17,7 +17,8 @@ downloadcache = ~/cache/pip
 
 [testenv:pep8]
 deps=pep8==1.2
-commands = pep8 --exclude=*.pyc --repeat --show-source setup.py
+commands = pep8 --exclude=*.pyc --repeat --show-source \
+           packstack/modules tests setup.py
 
 [testenv:cover]
 setenv = NOSE_WITH_COVERAGE=1