Re: [Kimchi-devel] [PATCH] Fixed the comments for debugreports in report tool and removed the check for platform s390x and introduced wok_log errors in log_errors.

Tuesday, 29 December 2015

Hi Aline,

Please find my comments below.

Regards,
Megha

...
 Subject: 	Re: [Kimchi-devel] [PATCH] Fixed the comments for 
 debugreports in report tool and removed the check for platform s390x 
 and introduced wok_log errors in log_errors.
 Date: 	Mon, 30 Nov 2015 12:03:46 -0200
 From: 	Aline Manera <alinefm(a)linux.vnet.ibm.com&gt;
 To: 	mesmriti(a)linux.vnet.ibm.com, Kimchi Devel <kimchi-devel(a)ovirt.org&gt;

 Almost there! =)

 Some comments below

 On 19/11/2015 02:56,mesmriti@linux.vnet.ibm.com  wrote:
 > From: root&lt;root(a)localhost.localdomain&gt;
 >
 > Signed-off-by:mesmriti@linux.vnet.ibm.com
 > ---
 >   src/wok/plugins/gingerbase/i18n.py               |   4 +-
 >   src/wok/plugins/gingerbase/model/debugreports.py | 181 ++++++++++++++++++-----
 >   2 files changed, 143 insertions(+), 42 deletions(-)
 >
 > diff --git a/src/wok/plugins/gingerbase/i18n.py
b/src/wok/plugins/gingerbase/i18n.py
 > index af75c70..6ff783b 100644
 > --- a/src/wok/plugins/gingerbase/i18n.py
 > +++ b/src/wok/plugins/gingerbase/i18n.py
 > @@ -36,7 +36,9 @@ messages = {
 >       "GGBDR0007E": _("Debug report name must be a string. Only
letters, digits, underscore ('_') and "
 >                       "hyphen ('-') are allowed."),
 >       "GGBDR0008E": _("The debug report with specified name
\"%(name)s\" already exists. Please use another one."),
 > -
 > +    "GGBDR0009E": _("Unable to create dbginfo report with
%(retcode)s. Details: %(err)s"),
 > +    "GGBDR0010E": _("Unable to compress the final debug report tar
file with %(retcode)s. Details: %(error)s"),
 > +    "GGBDR0011E": _("Unable to generate final debug report %(name)s.
Details: %(err)s"),
 >       "GGBHOST0001E": _("Unable to shutdown host machine as there are
running virtual machines"),
 >       "GGBHOST0002E": _("Unable to reboot host machine as there are
running virtual machines"),
 >       "GGBHOST0003E": _("There may be virtual machines running on the
host"),
 > diff --git a/src/wok/plugins/gingerbase/model/debugreports.py
b/src/wok/plugins/gingerbase/model/debugreports.py
 > index 94ab7fe..c6b60e8 100644
 > --- a/src/wok/plugins/gingerbase/model/debugreports.py
 > +++ b/src/wok/plugins/gingerbase/model/debugreports.py
 > @@ -19,13 +19,13 @@
 >   # License along with this library; if not, write to the Free Software
 >   # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
 >
 > -import fnmatch
 >   import glob
 >   import logging
 >   import os
 >   import shutil
 >   import subprocess
 >   import time
 > +import re
 >
 >   from wok.exception import InvalidParameter, NotFoundError, OperationFailed
 >   from wok.exception import WokException
 > @@ -71,54 +71,107 @@ class DebugReportsModel(object):
 >           raise OperationFailed("GGBDR0002E")
 >
 >       @staticmethod
 > -    def sosreport_generate(cb, name):
 > +    def debugreport_generate(cb, name):
 >           def log_error(e):
 > -            log = logging.getLogger('Model')
 > -            log.warning('Exception in generating debug file: %s', e)
 > +            wok_log = logging.getLogger('Model')
 > +            wok_log.warning('Exception in generating debug file: %s', e)
 >
 >           try:
 > -            command = ['sosreport', '--batch', '--name=%s'
% name]
 > +            # Sosreport generation
 > +            sosreport_file = sosreport_collection(name)
 > +            md5_report_file = sosreport_file + '.md5'
 > +            report_file_extension = '.' + sosreport_file.split('.',
1)[1]
 > +            # If the platform is a system Z machine.
 > +            path_debugreport = '/var/tmp/'
 > +            dbgreport_regex = '(\S+\s+)(' + path_debugreport + \
 > +                              'DBGINFO-[\d+]{4}-[\d+]{2}' \
 > +                              '-[\d+]{2}-[\d+]{2}-[\d+]{2}' \
 > +                              '-[\d+]{2}-\w+-\d+\S+)(\s+\S+)'
 > +            command = ['/usr/sbin/dbginfo.sh', '-d',
path_debugreport]
 >               output, error, retcode = run_command(command)
 > -
 >               if retcode != 0:
 > -                raise OperationFailed("GGBDR0003E", {'name':
name,
 > -                                                     'err': retcode})
 > -
 > -            # SOSREPORT might create file in /tmp or /var/tmp
 > -            # FIXME: The right way should be passing the tar.xz file directory
 > -            # though the parameter '--tmp-dir', but it is failing in Fedora
20
 > -            patterns = ['/tmp/sosreport-%s-*',
'/var/tmp/sosreport-%s-*']
 > -            reports = []
 > -            reportFile = None
 > -            for p in patterns:
 > -                reports = reports + [f for f in glob.glob(p % name)]
 > -            for f in reports:
 > -                if not fnmatch.fnmatch(f, '*.md5'):
 > -                    reportFile = f
 > +                raise OperationFailed("GGBDR0009E",
 > +                                      {'retcode': retcode, 'err':
error})

 > +            output = output.splitlines()
 > +            dbginfo_report = None
 > +            for line in output:
 > +                line = line.strip()
 > +                n = re.match(dbgreport_regex, line)
 > +                if n:
 > +                    dbginfo_report = n.groups()[1]
 >                       break

 As you know where dbg report will be created, you can change the
 dbgreport_regex to be a string pattern and use glob.

 import glob

 file = glob.glob(dgbreport_pattern)
 if len(file) == 0:
      # error   [Megha:] Will incorporate this in my next patch.
...
 I included I more detailed example on how use glob below.

 > -            # Some error in sosreport happened
 > -            if reportFile is None:
 > -                wok_log.error('Debug report file not found. See sosreport
'
 > -                              'output for detail:\n%s', output)
 > -                fname = (patterns[0] % name).split('/')[-1]
 > -                raise OperationFailed('GGBDR0004E', {'name':
fname})
 > -
 > -            md5_report_file = reportFile + '.md5'
 > -            report_file_extension = '.' + reportFile.split('.',
1)[1]
 > +            final_tar_report_name = name + report_file_extension
 > +            sosreport_tar = sosreport_file.split('/', 3)[3]
 > +            dbginfo_tar = dbginfo_report.split('/', 3)[3]
 > +            msg = 'Compressing the sosreport and debug info files into ' \
 > +                  'final report file'
 > +            wok_log.info(msg)
 > +            # Compressing the sosreport and dbginfo reports into one
 > +            # tar file
 > +            command = ['tar', '-cvzf', '%s' %
final_tar_report_name,
 > +                       '-C', path_debugreport, dbginfo_tar,
 > +                       sosreport_tar]
 > +            output, error, retcode = run_command(command)
 > +            if retcode != 0:
 > +                raise OperationFailed("GGBDR0010E",
 > +                                      {'retcode': retcode,
 > +                                       'error': error})
 >               path = config.get_debugreports_path()
 > -            target = os.path.join(path, name + report_file_extension)
 > -            # Moving report
 > -            msg = 'Moving debug report file "%s" to
"%s"' % (reportFile,
 > -                                                             target)
 > +            dbg_target = os.path.join(path,
 > +                                      name + report_file_extension)
 > +            # Moving final tar file to debugreports path
 > +            msg = 'Moving final debug  report file "%s" to
"%s"' % \
 > +                  (final_tar_report_name, dbg_target)
 > +            wok_log.info(msg)
 > +            shutil.move(final_tar_report_name, dbg_target)
 > +            # Deleting the sosreport md5 file
 > +            delete_the_sosreport_md5_file(md5_report_file)
 > +            # Deleting the dbingo report file
 > +            msg = 'Deleting the dbginfo file "%s" ' \
 > +                  % dbginfo_report
 >               wok_log.info(msg)
 > -            shutil.move(reportFile, target)
 > -            # Deleting md5
 > -            msg = 'Deleting report md5 file: "%s"' %
(md5_report_file)
 > +            os.remove(dbginfo_report)
 > +            # Deleting the sosreport file
 > +            msg = 'Deleting the sosreport file "%s" ' %
sosreport_file
 >               wok_log.info(msg)
 > -            with open(md5_report_file) as f:
 > -                md5 = f.read().strip()
 > -                wok_log.info('Md5 file content: "%s"', md5)
 > -            os.remove(md5_report_file)
 > +            os.remove(sosreport_file)
 > +            wok_log.info('The debug report file has been moved')
 > +            cb('OK', True)
 > +            return
 > +

 > +        except WokException as e:
 > +            log_error(e)
 > +            raise
 > +
 > +        except OSError as e:
 > +            log_error(e)
 > +            raise
 > +

 Please, raise an appropriated exception. Otherwise, the user will not be
 able to understand what went wrong.
 [Megha]: As far as exceptions are concerned These are the exception I think can be thrown
back from the try block and                        it is also similar to  what is
done in sosreport. 
Please correct me if I any suggestions.
...
 > +        except Exception, e:
 > +            # No need to call cb to update the task status here.
 > +            # The task object will catch the exception raised here
 > +            # and update the task status there
 > +            log_error(e)
 > +            raise OperationFailed("GGBDR0011E", {'name': name,
'err': e})
 > +
 > +    @staticmethod
 > +    def sosreport_generate(cb, name):
 > +        def log_error(e):
 > +            wok_log = logging.getLogger('Model')
 > +            wok_log.warning('Exception in generating debug file: %s', e)
 > +        try:
 > +            # Sosreport collection
 > +            sosreport_file = sosreport_collection(name)
 > +            md5_report_file = sosreport_file + '.md5'
 > +            report_file_extension = '.' + sosreport_file.split('.',
1)[1]
 > +            path = config.get_debugreports_path()
 > +            sosreport_target = os.path.join(path,
 > +                                            name + report_file_extension)
 > +            msg = 'Moving debug report file "%s" to
"%s"' \
 > +                  % (sosreport_file, sosreport_target)
 > +            wok_log.info(msg)
 > +            shutil.move(sosreport_file, sosreport_target)
 > +            delete_the_sosreport_md5_file(md5_report_file)
 >               cb('OK', True)
 >               return
 >
 > @@ -142,7 +195,9 @@ class DebugReportsModel(object):
 >           # Please add new possible debug report command here
 >           # and implement the report generating function
 >           # based on the new report command
 > -        report_tools = ({'cmd': 'sosreport --help',
 > +        report_tools = ({'cmd': '/usr/sbin/dbginfo.sh --help',
 > +                         'fn': DebugReportsModel.debugreport_generate},
 > +                        {'cmd': 'sosreport --help',
 >                            'fn': DebugReportsModel.sosreport_generate},)
 >
 >           # check if the command can be found by shell one by one
 > @@ -213,3 +268,47 @@ class DebugReportContentModel(object):
 >
 >       def lookup(self, name):
 >           return self._debugreport.lookup(name)
 > +
 > +
 > +def delete_the_sosreport_md5_file(md5_file):
 > +    """
 > +    Deleting md5 file and displaying the contents of the same.
 > +    """
 > +    msg = 'Deleting report md5 file: "%s"' % md5_file
 > +    wok_log.info(msg)
 > +    with open(md5_file) as f:
 > +        md5 = f.read().strip()
 > +        wok_log.info('Md5 file content: "%s"', md5)
 > +    os.remove(md5_file)
 > +
 > +
 > +def sosreport_collection(name):
 > +    """
 > +    Code for the collection of sosreport n the path
 > +    /var/tmp as specified in the command.
 > +    """
 > +    path_sosreport = '/var/tmp/'
 > +    command = ['sosreport', '--batch', '--name=%s' % name,
 > +               '--tmp-dir=%s' % path_sosreport]
 > +    output, error, retcode = run_command(command)
 > +    if retcode != 0:
 > +        raise OperationFailed("GGBDR0003E", {'name': name,
 > +                                             'err': retcode})
 > +

 > +    sosreport_name_regex = '(\s+)(' + path_sosreport + \
 > +                           'sosreport-' +\
 > +                           name + '-\d+.tar.xz)'
 > +    sosreport_file = None
 > +    output = output.splitlines()
 > +    for line in output:
 > +        if line:
 > +            matched_name = re.match(sosreport_name_regex, line)
 > +            if matched_name:
 > +                sosreport_file = matched_name.groups()[1]
 > +                break
 > +    # Some error in sosreport happened
 > +    if sosreport_file is None:
 > +        wok_log.error('Debug report file not found. See sosreport '
 > +                      'output for detail:\n%s', output)
 > +        raise OperationFailed('GGBDR0004E', {'name': name})
 > +    return sosreport_file

 You know the sosreport file was created under /var/tmp so you don't need
 to query the command output to know the file was created or not.

 If you want to confirm the file was created you can use 'glob'

 import glob

 sosreport_pattern = path_sosreport + 'sosreport-' + name + '.tar.xz'
 sosreport_file = glob.glob(sosreport_pattern)

 if len(sosreport_file) == 0:
      raise OperationFailed()

 return sosreport_file[0]

 [Megha]: Will incorporate in my next patch
 _______________________________________________
 Kimchi-devel mailing list
 Kimchi-devel(a)ovirt.org
 http://lists.ovirt.org/mailman/listinfo/kimchi-devel

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

Re: [Kimchi-devel] [PATCH] Fixed the comments for debugreports in report tool and removed the check for platform s390x and introduced wok_log errors in log_errors.