From 5859915898b1725efcbaf72cac3539f9e963d9ed Mon Sep 17 00:00:00 2001 From: Martin Magr Date: Mon, 23 Sep 2013 11:43:31 +0200 Subject: [PATCH] Change message in certain cases In some cases Puppet error messages are useless. This patch implements algorithm which makes certain error messages more userfriendly. Change-Id: Iac6f1c5382b3c3d605df3477477558858eca69ef Fixes: rhbz#989334, rhbz#1006476, rhbz#1003959 --- packstack/installer/exceptions.py | 3 + packstack/modules/ospluginutils.py | 70 ------------------- packstack/modules/puppet.py | 101 ++++++++++++++++++++++++++++ packstack/plugins/puppet_950.py | 9 ++- tests/installer/test_run_setup.py | 8 +-- tests/installer/test_utils.py | 8 +++ tests/modules/__init__.py | 0 tests/modules/test_ospluginutils.py | 31 +++++++++ tests/modules/test_puppet.py | 62 +++++++++++++++++ tests/test_ospluginutils.py | 60 ----------------- 10 files changed, 213 insertions(+), 139 deletions(-) create mode 100644 packstack/modules/puppet.py create mode 100644 tests/modules/__init__.py create mode 100644 tests/modules/test_ospluginutils.py create mode 100644 tests/modules/test_puppet.py delete mode 100644 tests/test_ospluginutils.py diff --git a/packstack/installer/exceptions.py b/packstack/installer/exceptions.py index fd6e679e4..cf0dc4d4c 100644 --- a/packstack/installer/exceptions.py +++ b/packstack/installer/exceptions.py @@ -24,6 +24,9 @@ class PackStackError(Exception): self.stderr = kwargs.get('stderr', None) +class PuppetError(Exception): + """Raised when Puppet will have some problems.""" + class MissingRequirements(PackStackError): """Raised when minimum install requirements are not met.""" diff --git a/packstack/modules/ospluginutils.py b/packstack/modules/ospluginutils.py index d30db424d..d4fc0e004 100644 --- a/packstack/modules/ospluginutils.py +++ b/packstack/modules/ospluginutils.py @@ -90,73 +90,3 @@ def gethostlist(CONF): if host and host not in hosts: hosts.append(host) return hosts - - -_error_exceptions = [ - # puppet preloads a provider using the mysql command before it is installed - re.compile('Command mysql is missing'), - # puppet preloads a database_grant provider which fails if /root/.my.cnf - # this is ok because it will be retried later if needed - re.compile('Could not prefetch database_grant provider.*?\\.my\\.cnf'), - # swift puppet module tries to install swift-plugin-s3, there is no such - # pakage on RHEL, fixed in the upstream puppet module - re.compile('yum.*?install swift-plugin-s3'), -] - - -def isErrorException(line): - for ee in _error_exceptions: - if ee.search(line): - return True - return False - - -_re_color = re.compile('\x1b.*?\d\dm') -_re_errorline = re.compile('err: | Syntax error at|^Duplicate definition:|' - '^No matching value for selector param|' - '^Parameter name failed:|Error: |^Invalid tag |' - '^Invalid parameter |^Duplicate declaration: ' - '^Could not find resource |^Could not parse for ') - - -def validate_puppet_logfile(logfile): - """ - Check a puppet log file for errors and raise an error if we find any - """ - fp = open(logfile) - data = fp.read() - fp.close() - manifestfile = os.path.splitext(logfile)[0] - for line in data.split('\n'): - line = line.strip() - - if _re_errorline.search(line) is None: - continue - - message = _re_color.sub('', line) # remove colors - if isErrorException(line): - logging.info("Ignoring expected error during puppet run %s : %s" % - (manifestfile, message)) - continue - - message = "Error during puppet run : " + message - logging.error("Error during remote puppet apply of " + manifestfile) - logging.error(data) - raise PackStackError(message) - - -def scan_puppet_logfile(logfile): - """ - Returns list of packstack_info/packstack_warn notices parsed from - given puppet log file. - """ - output = [] - notice = re.compile(r"notice: .*Notify\[packstack_info\]" - "\/message: defined \'message\' as " - "\'(?P.*)\'") - with open(logfile) as content: - for line in content: - match = notice.search(line) - if match: - output.append(match.group('message')) - return output diff --git a/packstack/modules/puppet.py b/packstack/modules/puppet.py new file mode 100644 index 000000000..abaf805cf --- /dev/null +++ b/packstack/modules/puppet.py @@ -0,0 +1,101 @@ +# -*- coding: utf-8 -*- + +import logging +import os +import re + +from packstack.installer.exceptions import PuppetError + + +# TODO: Fill logger name when logging system will be refactored +logger = logging.getLogger() + +re_color = re.compile('\x1b.*?\d\dm') +re_error = re.compile( + 'err:|Syntax error at|^Duplicate definition:|^Invalid tag|' + '^No matching value for selector param|^Parameter name failed:|Error:|' + '^Invalid parameter|^Duplicate declaration:|^Could not find resource|' + '^Could not parse for|^\/usr\/bin\/env\: jruby\: No such file or directory' +) +re_ignore = re.compile( + # Puppet preloads a provider using the mysql command before it is installed + 'Command mysql is missing|' + # Puppet preloads a database_grant provider which fails if /root/.my.cnf + # is missing, this is ok because it will be retried later if needed + 'Could not prefetch database_grant provider.*?\\.my\\.cnf|' + # Swift Puppet module tries to install swift-plugin-s3, there is no such + # package on RHEL, fixed in the upstream puppet module + 'yum.*?install swift-plugin-s3' +) +re_notice = re.compile(r"notice: .*Notify\[packstack_info\]" + "\/message: defined \'message\' as " + "\'(?P.*)\'") + +surrogates = [ + # Value in /etc/sysctl.conf cannot be changed + ('Sysctl::Value\[.*\]\/Sysctl\[(?P.*)\].*Field \'val\' is required', + 'Cannot change value of %(arg1)s in /etc/sysctl.conf'), + # Package is not found in yum repos + ('Package\[.*\]\/ensure.*yum.*install (?P.*)\'.*Nothing to do', + 'Package %(arg1)s has not been found in enabled Yum repos.'), + # Packstack does not cooperate with jruby + ('jruby', 'Your Puppet installation uses jruby instead of ruby. Package ' + 'jruby does not cooperate with Packstack well. You will have to ' + 'fix this manually.'), +] + + +def validate_logfile(logpath): + """ + Check given Puppet log file for errors and raise PuppetError if there is + any error + """ + manifestpath = os.path.splitext(logpath)[0] + manifestfile = os.path.basename(manifestpath) + with open(logpath) as logfile: + for line in logfile: + line = line.strip() + + if re_error.search(line) is None: + continue + + error = re_color.sub('', line) # remove colors + if re_ignore.search(line): + msg = ('Ignoring expected error during Puppet run %s: %s' % + (manifestfile, error)) + logger.debug(msg) + continue + + for regex, surrogate in surrogates: + match = re.search(regex, error) + if match is None: + continue + + args = {} + num = 1 + while True: + try: + args['arg%d' % num] = match.group(num) + num += 1 + except IndexError: + break + error = surrogate % args + + message = ('Error appeared during Puppet run: %s\n%s\n' + 'You will find full trace in log %s' % + (manifestfile, error, logpath)) + raise PuppetError(message) + + +def scan_logfile(logpath): + """ + Returns list of packstack_info/packstack_warn notices parsed from + given puppet log file. + """ + output = [] + with open(logpath) as logfile: + for line in logfile: + match = re_notice.search(line) + if match: + output.append(match.group('message')) + return output diff --git a/packstack/plugins/puppet_950.py b/packstack/plugins/puppet_950.py index dfb3bb443..7ec747506 100644 --- a/packstack/plugins/puppet_950.py +++ b/packstack/plugins/puppet_950.py @@ -12,9 +12,8 @@ from packstack.installer import basedefs, output_messages from packstack.installer.exceptions import ScriptRuntimeError from packstack.modules.common import filtered_hosts -from packstack.modules.ospluginutils import (manifestfiles, - scan_puppet_logfile, - validate_puppet_logfile) +from packstack.modules.ospluginutils import manifestfiles +from packstack.modules.puppet import scan_logfile, validate_logfile # Controller object will be initialized from main flow controller = None @@ -162,10 +161,10 @@ def waitforpuppet(currently_running): continue # check log file for relevant notices - controller.MESSAGES.extend(scan_puppet_logfile(log)) + controller.MESSAGES.extend(scan_logfile(log)) # check the log file for errors - validate_puppet_logfile(log) + validate_logfile(log) sys.stdout.write(("\r%s : " % log_file).ljust(space_len)) print ("[ " + utils.color_text(output_messages.INFO_DONE, 'green') + " ]") diff --git a/tests/installer/test_run_setup.py b/tests/installer/test_run_setup.py index e69310608..80fcc9b71 100644 --- a/tests/installer/test_run_setup.py +++ b/tests/installer/test_run_setup.py @@ -20,7 +20,7 @@ import shutil import sys from unittest import TestCase -from packstack.modules import ospluginutils +from packstack.modules import ospluginutils, puppet from packstack.installer import run_setup, basedefs from ..test_base import PackstackTestCaseMixin @@ -55,9 +55,9 @@ class CommandLineTestCase(PackstackTestCaseMixin, TestCase): # There is no puppet logfile to validate, so replace # ospluginutils.validate_puppet_logfile with a mock function - orig_validate_logfile = ospluginutils.validate_puppet_logfile - ospluginutils.validate_puppet_logfile = lambda a: None - ospluginutils.scan_puppet_logfile = lambda a: [] + orig_validate_logfile = puppet.validate_logfile + puppet.validate_logfile = lambda a: None + puppet.scan_logfile = lambda a: [] # If there is a error in a plugin sys.exit() gets called, this masks # the actual error that should be reported, so we replace it to diff --git a/tests/installer/test_utils.py b/tests/installer/test_utils.py index 9376620bc..be9f5a3c8 100644 --- a/tests/installer/test_utils.py +++ b/tests/installer/test_utils.py @@ -106,3 +106,11 @@ class ParameterTestCase(PackstackTestCaseMixin, TestCase): mask_list=["'text'"], replace_list=[("'", "'\\''")]) self.assertEqual(masked, 'test %s' % STR_MASK) + + def test_shortcuts(self): + """Test packstack.installer.utils.shortcuts functions""" + conf = {"A_HOST": "1.1.1.1", "B_HOSTS": "2.2.2.2,1.1.1.1", + "C_HOSTS": "3.3.3.3/vdc"} + hostlist = list(hosts(conf)) + hostlist.sort() + self.assertEquals(['1.1.1.1', '2.2.2.2', '3.3.3.3'], hostlist) diff --git a/tests/modules/__init__.py b/tests/modules/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/modules/test_ospluginutils.py b/tests/modules/test_ospluginutils.py new file mode 100644 index 000000000..0b802ebc4 --- /dev/null +++ b/tests/modules/test_ospluginutils.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2013, Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import os +from unittest import TestCase + +from ..test_base import PackstackTestCaseMixin +from packstack.modules.ospluginutils import gethostlist + + +class OSPluginUtilsTestCase(PackstackTestCaseMixin, TestCase): + def test_gethostlist(self): + conf = {"A_HOST": "1.1.1.1", "B_HOSTS": "2.2.2.2,1.1.1.1", + "C_HOSTS": "3.3.3.3/vdc"} + hosts = gethostlist(conf) + hosts.sort() + self.assertEquals(['1.1.1.1', '2.2.2.2', '3.3.3.3'], hosts) diff --git a/tests/modules/test_puppet.py b/tests/modules/test_puppet.py new file mode 100644 index 000000000..c3ea9a0bc --- /dev/null +++ b/tests/modules/test_puppet.py @@ -0,0 +1,62 @@ +# -*- coding: utf-8 -*- +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2013, Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import os + +from unittest import TestCase +from ..test_base import PackstackTestCaseMixin + +from packstack.installer.exceptions import PuppetError +from packstack.modules.puppet import validate_logfile, scan_logfile + + +class PuppetTestCase(PackstackTestCaseMixin, TestCase): + + def test_validate_logfile(self): + """Test packstack.modules.validate_logfile""" + filename = os.path.join(self.tempdir, "puppet.log") + # test valid run + with open(filename, "w") as fp: + fp.write("Everything went ok") + validate_logfile(filename) + # test invalid run + with open(filename, "w") as fp: + fp.write("No matching value for selector param 'Fedora' ...") + self.assertRaises(PuppetError, validate_logfile, filename) + # test run with error exception + with open(filename, "w") as fp: + err = ("err: Could not prefetch database_grant provider 'mysql': " + "Execution of '/usr/bin/mysql --defaults-file=/root/.my.cnf" + " mysql -Be describe user' returned 1: Could not open " + "required defaults file: /root/.my.cnf") + fp.write(err) + validate_logfile(filename) + # test surrogate + with open(filename, "w") as fp: + err = ("err: /Stage[main]/Vswitch::Ovs/Package[openvswitch]/ensure" + ": change from absent to present failed: Execution of " + "'/usr/bin/yum -d 0 -e 0 -y install openvswitch' returned " + "1: Error: Nothing to do") + fp.write(err) + self.assertRaises(PuppetError, validate_logfile, filename) + try: + validate_logfile(filename) + except PuppetError as ex: + ex_msg = str(ex) + sr_msg = ("Package openvswitch has not been found in enabled Yum " + "repos") + assert sr_msg in ex_msg diff --git a/tests/test_ospluginutils.py b/tests/test_ospluginutils.py deleted file mode 100644 index 79a904ac0..000000000 --- a/tests/test_ospluginutils.py +++ /dev/null @@ -1,60 +0,0 @@ -# -*- coding: utf-8 -*- -# vim: tabstop=4 shiftwidth=4 softtabstop=4 - -# Copyright 2013, Red Hat, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -import os -from unittest import TestCase - -from test_base import PackstackTestCaseMixin -from packstack.modules.ospluginutils import gethostlist, \ - validate_puppet_logfile, \ - PackStackError - - -class OSPluginUtilsTestCase(PackstackTestCaseMixin, TestCase): - def test_gethostlist(self): - conf = {"A_HOST": "1.1.1.1", "B_HOSTS": "2.2.2.2,1.1.1.1", - "C_HOSTS": "3.3.3.3/vdc"} - 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) - - def test_validate_puppet_logfile_okerror(self): - filename = os.path.join(self.tempdir, "puppet.log") - fp = open(filename, "w") - fp.write("err: Could not prefetch database_grant provider 'mysql': " - "Execution of '/usr/bin/mysql --defaults-file=/root/.my.cnf " - "mysql -Be describe user' returned 1: Could not open required" - " defaults file: /root/.my.cnf") - fp.close() - - validate_puppet_logfile(filename)