[Kimchi-devel] [PATCH] Dbginfo report generation
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Nov 30 10:53:27 UTC 2015
On 25/11/2015 15:36, Chandra Shekhar Reddy Potula wrote:
> One suggestion on where the final tar report file should get generated
> and deletion of files after the final tar creation,
>
> I would prefer to choose /tmp location to do all these operations
> before I copy it over to final target folder for the following reasons.
> 1. In case developer do mistake in coding, it should not impact the
> files already exists in the final folder (ie.
> /var/lib/gingerbase/debugreports/). May be due code errors it should
> not accidentally delete other files exists
> 2. In general /tmp will be used for such operation before we get the
> final report.
>
Good point!
Also while creating the new debug reports in the final directory it will
be listed in the transient GET /debugreports request but it may be not
completed generated yet.
It is OK to use /tmp for that.
I only recommend to use --tmp-dir to do not need to query the sosreport
command output to get the debug report name/location.
Regards,
Aline Manera
> Regards
> Chandra
>
> On 23/11/15 11:47 PM, Aline Manera wrote:
>>
>>
>> On 17/11/2015 20:05, mesmriti at linux.vnet.ibm.com wrote:
>>> From: root <root at localhost.localdomain>
>>>
>>> Signed-off-by: root <root at localhost.localdomain>
>>> ---
>>> src/wok/plugins/gingerbase/i18n.py | 4 +-
>>> src/wok/plugins/gingerbase/model/debugreports.py | 212
>>> ++++++++++++++++++-----
>>> 2 files changed, 167 insertions(+), 49 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..927e173 100644
>>> --- a/src/wok/plugins/gingerbase/model/debugreports.py
>>> +++ b/src/wok/plugins/gingerbase/model/debugreports.py
>>> @@ -19,13 +19,14 @@
>>> # 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 platform
>>> +import re
>>>
>>> from wok.exception import InvalidParameter, NotFoundError,
>>> OperationFailed
>>> from wok.exception import WokException
>>> @@ -71,54 +72,109 @@ 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)
>>
>> I know it is already there prior to your patch, but we should use
>> wok_log to log the errors.
>> This special wrapper can be removed.
>>
>>> try:
>>> - command = ['sosreport', '--batch', '--name=%s' % name]
>>> - 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
>>> - break
>>> - # 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]
>>> + # 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.
>>> + if platform.machine().startswith('s390'):
>>
>> The debugreport_generate() function will be called only if the system
>> has the dbginfo command
>> So you don't need to rely in the system arch to decide on that.
>> So the 'if' statement above can be safely removed.
>>
>>> + 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("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
>>> + final_tar_report_name = name + report_file_extension
>>> + if dbginfo_report is not None:
>>> + 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]
>>
>> You can use final_tar_report_name with the right debug report
>> location to avoid need to move the file after that.
>>
>>> + output, error, retcode = run_command(command)
>>> + if retcode != 0:
>>> + raise OperationFailed("GGBDR0010E",
>>> + {'retcode': retcode,
>>> + 'error': error})
>>
>>> + path = config.get_debugreports_path()
>>> + dbg_target = os.path.join(path,
>>> + name +
>>> report_file_extension)
>>
>> Use the dbg_target value while running the 'tar' command above. So we
>> don't need to move the file.
>>
>>> + # 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)
>>
>> The above block can be removed.
>>
>>> + # 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)
>>> + os.remove(dbginfo_report)
>>> + # Deleting the sosreport file
>>> + msg = 'Deleting the sosreport file "%s" ' %
>>> sosreport_file
>>> + wok_log.info(msg)
>>> + 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
>>> +
>>> + 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):
>>> + log = logging.getLogger('Model')
>>> + log.warning('Exception in generating debug file: %s', e)
>>
>> The same I commented before. We can use wok_log to log the errors.
>>
>>> + 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()
>>> - target = os.path.join(path, name + report_file_extension)
>>> - # Moving report
>>> - msg = 'Moving debug report file "%s" to "%s"' %
>>> (reportFile,
>>> - target)
>>> + 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(reportFile, target)
>>> - # Deleting md5
>>> - msg = 'Deleting report md5 file: "%s"' % (md5_report_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)
>>
>>> + shutil.move(sosreport_file, sosreport_target)
>>
>> As we are using --tmp-dir option, we know exactly where the sosreport
>> will be saved. So we don't need to move the file around.
>>
>>> + delete_the_sosreport_md5_file(md5_report_file)
>>> cb('OK', True)
>>> return
>>>
>>> @@ -142,17 +198,27 @@ 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
>>> for helper_tool in report_tools:
>>> try:
>>> - retcode = subprocess.call(helper_tool['cmd'],
>>> shell=True,
>>> - stdout=subprocess.PIPE,
>>> - stderr=subprocess.PIPE)
>>> - if retcode == 0:
>>> - return helper_tool['fn']
>>
>>> + if 'cmd' == '/usr/sbin/dbginfo.sh --help':
>>> + retcode = subprocess.call(helper_tool['cmd'],
>>> shell=True,
>>> + stdout=subprocess.PIPE,
>>> + stderr=subprocess.PIPE)
>>> + if retcode == 0:
>>> + return helper_tool['fn']
>>
>> You don't need to a special condition to cover the dbginfo command.
>> The former code already does that.
>>
>>> + else:
>>> +
>>> + retcode = subprocess.call(helper_tool['cmd'],
>>> shell=True,
>>> + stdout=subprocess.PIPE,
>>> + stderr=subprocess.PIPE)
>>> + if retcode == 0:
>>> + return helper_tool['fn']
>>> except Exception, e:
>>> wok_log.info('Exception running command: %s', e)
>>>
>>> @@ -213,3 +279,53 @@ 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 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
>>
>> You can remove this comment.
>>
>>> + sosreport_name = name.replace('_', '')
>>
>> The same I commented before. We should not change the user input
>> without informing user about it.
>> So it is better to block underscore in the debug report name. Please,
>> update API.json accordingly.
>>
>>> + sosreport_name_regex = '(\s+)(' + path_sosreport + \
>>> + ')(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:
>>> + path = matched_name.groups()[1]
>>> + fname = matched_name.groups()[2]
>>> + sosreport_file = path + fname
>>
>> While using --tmp-dir you know exactly where the sosreport file will
>> be: <tmp-dir>-sosreport-<name>.tar.gz
>> So you can need to parse the command output.
>>
>>> + 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
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
More information about the Kimchi-devel
mailing list