[PATCH 0/2 V2] #749 Dbginfo report generation
From: Megha Smriti <mesmriti@linux.vnet.ibm.com> The reveiw comments have been fixed.We have now different methods for sosreport generation and dbginfo report generation combined with sosreport for s390x platform.Modifications were made in the dbginfo.sh command such that the dbginfo reports can now be generated in the desired location. So the sosreport and dbgreport are generated in /var/tmp path and finally compressed and moved to config.path() root (1): Dbginfo report generation src/wok/plugins/gingerbase/i18n.py | 4 +- src/wok/plugins/gingerbase/model/debugreports.py | 212 ++++++++++++++++++----- 2 files changed, 167 insertions(+), 49 deletions(-) -- 2.4.3
From: root <root@localhost.localdomain> Signed-off-by: root <root@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) 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'): + 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] + 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) + # 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) + 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) + 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) + 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'] + 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 + sosreport_name = name.replace('_', '') + 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 + 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 -- 2.4.3
On 17/11/2015 20:05, mesmriti@linux.vnet.ibm.com wrote:
From: root <root@localhost.localdomain>
Signed-off-by: root <root@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
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. Regards Chandra On 23/11/15 11:47 PM, Aline Manera wrote:
On 17/11/2015 20:05, mesmriti@linux.vnet.ibm.com wrote:
From: root <root@localhost.localdomain>
Signed-off-by: root <root@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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
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@linux.vnet.ibm.com wrote:
From: root <root@localhost.localdomain>
Signed-off-by: root <root@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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
participants (3)
-
Aline Manera -
Chandra Shekhar Reddy Potula -
mesmriti＠linux.vnet.ibm.com