[PATCH V2] Fix debug report - NEED TESTS

In Fedora 20, the sosreport tool saves the report file in /var/tmp, which crashes kimchi. The --tmp-dir options should be the right approach but it is crashing in Fedora 20 (RHEL works). While waiting sosreport fix, this patch searches report file in /var/tmp too. This code works on Fedora 20. Need more tests in Ubuntu(s), RHEL and older Fedora. Rodrigo Trujillo (1): Fix host debug report for Fedora 20 src/kimchi/model/debugreports.py | 61 ++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 21 deletions(-) -- 1.8.5.3

The tool sosreport does not save the report file in /tmp in F20, this causes a error in Kimchi, which has /tmp hardcoded. This fixes this problem searching report files in /var/tmp too. Notice that is possible to pass --tmp-dir as the directory to save the report file, but this option is crashing in Fedora 20, so the code is not using it. This patch also improves the logging. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/model/debugreports.py | 61 ++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/src/kimchi/model/debugreports.py b/src/kimchi/model/debugreports.py index a1cb19c..8337709 100644 --- a/src/kimchi/model/debugreports.py +++ b/src/kimchi/model/debugreports.py @@ -32,6 +32,7 @@ from kimchi import config from kimchi.exception import NotFoundError, OperationFailed from kimchi.model.tasks import TaskModel from kimchi.utils import add_task, kimchi_log +from kimchi.utils import run_command class DebugReportsModel(object): @@ -63,36 +64,54 @@ class DebugReportsModel(object): @staticmethod def sosreport_generate(cb, name): - command = 'sosreport --batch --name "%s"' % name try: - retcode = subprocess.call(command, shell=True, - stdout=subprocess.PIPE) + command = ['sosreport', '--batch', '--name=%s' % name] + output, error, retcode = run_command(command) + if retcode < 0: raise OperationFailed('Command terminated with signal') elif retcode > 0: raise OperationFailed('Command failed: rc = %i' % 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 pattern = '/tmp/sosreport-%s-*' % name - for reportFile in glob.glob(pattern): - if not fnmatch.fnmatch(reportFile, '*.md5'): - output = reportFile + reports = glob.glob(pattern) + reportFile = None + for f in reports: + if not fnmatch.fnmatch(f, '*.md5'): + reportFile = f break - else: - # sosreport tends to change the name mangling rule and - # compression file format between different releases. - # It's possible to fail to match a report file even sosreport - # runs successfully. In future we might have a general name - # mangling function in kimchi to format the name before passing - # it to sosreport. Then we can delete this exception. - raise OperationFailed('Can not find generated debug report ' - 'named by %s' % pattern) - ext = output.split('.', 1)[1] + if reportFile is None: + # Search in /var/tmp + reports = glob.glob('/var' + pattern) + for f in reports: + if not fnmatch.fnmatch(f, '*.md5'): + reportFile = f + break + + # Some error in sosreport happened + if reportFile is None: + kimchi_log.error('Debug report file not found. See sosreport ' + 'output for detail:\n%s', output) + raise OperationFailed('Debug report file not found.') + + md5_report_file = reportFile + '.md5' + report_file_extension = '.' + reportFile.split('.', 1)[1] path = config.get_debugreports_path() - target = os.path.join(path, name) - target_file = '%s.%s' % (target, ext) - shutil.move(output, target_file) - os.remove('%s.md5' % output) + target = os.path.join(path, name + report_file_extension) + # Moving report + msg = 'Moving debug report file "%s" to "%s"' % (reportFile, target) + kimchi_log.info(msg) + shutil.move(reportFile, target) + # Deleting md5 + msg = 'Deleting report md5 file: "%s"' % (md5_report_file) + kimchi_log.info(msg) + md5 = open(md5_report_file).read().strip() + kimchi_log.info('Md5 file content: "%s"', md5) + os.remove(md5_report_file) cb('OK', True) - return except OSError: -- 1.8.5.3

On 02/06/2014 12:27 AM, Rodrigo Trujillo wrote:
The tool sosreport does not save the report file in /tmp in F20, this causes a error in Kimchi, which has /tmp hardcoded. This fixes this problem searching report files in /var/tmp too. Notice that is possible to pass --tmp-dir as the directory to save the report file, but this option is crashing in Fedora 20, so the code is not using it. This patch also improves the logging.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/model/debugreports.py | 61 ++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 21 deletions(-)
diff --git a/src/kimchi/model/debugreports.py b/src/kimchi/model/debugreports.py index a1cb19c..8337709 100644 --- a/src/kimchi/model/debugreports.py +++ b/src/kimchi/model/debugreports.py @@ -32,6 +32,7 @@ from kimchi import config from kimchi.exception import NotFoundError, OperationFailed from kimchi.model.tasks import TaskModel from kimchi.utils import add_task, kimchi_log +from kimchi.utils import run_command
class DebugReportsModel(object): @@ -63,36 +64,54 @@ class DebugReportsModel(object):
@staticmethod def sosreport_generate(cb, name): - command = 'sosreport --batch --name "%s"' % name try: - retcode = subprocess.call(command, shell=True, - stdout=subprocess.PIPE) + command = ['sosreport', '--batch', '--name=%s' % name]
why did you remove the stdout redirection? That way we will display a lot of useless messages to the user.
+ output, error, retcode = run_command(command) + if retcode < 0: raise OperationFailed('Command terminated with signal') elif retcode > 0: raise OperationFailed('Command failed: rc = %i' % 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 pattern = '/tmp/sosreport-%s-*' % name
Let's avoid duplicating code. How about: patterns = ['/tmp/sosreport-%s-*', '/var/tmp/sosreport-%s-*'] reports = [] for p in patterns: reports.append(glob.glob(p % name)) for f in reports: ...
- for reportFile in glob.glob(pattern): - if not fnmatch.fnmatch(reportFile, '*.md5'): - output = reportFile + reports = glob.glob(pattern) + reportFile = None + for f in reports: + if not fnmatch.fnmatch(f, '*.md5'): + reportFile = f break - else: - # sosreport tends to change the name mangling rule and - # compression file format between different releases. - # It's possible to fail to match a report file even sosreport - # runs successfully. In future we might have a general name - # mangling function in kimchi to format the name before passing - # it to sosreport. Then we can delete this exception. - raise OperationFailed('Can not find generated debug report ' - 'named by %s' % pattern) - ext = output.split('.', 1)[1] + if reportFile is None: + # Search in /var/tmp + reports = glob.glob('/var' + pattern) + for f in reports: + if not fnmatch.fnmatch(f, '*.md5'): + reportFile = f + break + + # Some error in sosreport happened + if reportFile is None: + kimchi_log.error('Debug report file not found. See sosreport ' + 'output for detail:\n%s', output) + raise OperationFailed('Debug report file not found.') + + md5_report_file = reportFile + '.md5' + report_file_extension = '.' + reportFile.split('.', 1)[1] path = config.get_debugreports_path() - target = os.path.join(path, name) - target_file = '%s.%s' % (target, ext) - shutil.move(output, target_file) - os.remove('%s.md5' % output) + target = os.path.join(path, name + report_file_extension) + # Moving report + msg = 'Moving debug report file "%s" to "%s"' % (reportFile, target) + kimchi_log.info(msg) + shutil.move(reportFile, target) + # Deleting md5 + msg = 'Deleting report md5 file: "%s"' % (md5_report_file) + kimchi_log.info(msg) + md5 = open(md5_report_file).read().strip() + kimchi_log.info('Md5 file content: "%s"', md5) + os.remove(md5_report_file) cb('OK', True) - return
except OSError:

On 02/10/2014 12:39 PM, Aline Manera wrote:
On 02/06/2014 12:27 AM, Rodrigo Trujillo wrote:
The tool sosreport does not save the report file in /tmp in F20, this causes a error in Kimchi, which has /tmp hardcoded. This fixes this problem searching report files in /var/tmp too. Notice that is possible to pass --tmp-dir as the directory to save the report file, but this option is crashing in Fedora 20, so the code is not using it. This patch also improves the logging.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/model/debugreports.py | 61 ++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 21 deletions(-)
diff --git a/src/kimchi/model/debugreports.py b/src/kimchi/model/debugreports.py index a1cb19c..8337709 100644 --- a/src/kimchi/model/debugreports.py +++ b/src/kimchi/model/debugreports.py @@ -32,6 +32,7 @@ from kimchi import config from kimchi.exception import NotFoundError, OperationFailed from kimchi.model.tasks import TaskModel from kimchi.utils import add_task, kimchi_log +from kimchi.utils import run_command
class DebugReportsModel(object): @@ -63,36 +64,54 @@ class DebugReportsModel(object):
@staticmethod def sosreport_generate(cb, name): - command = 'sosreport --batch --name "%s"' % name try: - retcode = subprocess.call(command, shell=True, - stdout=subprocess.PIPE) + command = ['sosreport', '--batch', '--name=%s' % name]
why did you remove the stdout redirection? That way we will display a lot of useless messages to the user. No, no, I just moved from subprocess to run_command ... which in turns also uses subprocess and handles the stdout and stderr. See the line below:
+ output, error, retcode = run_command(command) + if retcode < 0: raise OperationFailed('Command terminated with signal') elif retcode > 0: raise OperationFailed('Command failed: rc = %i' % 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 pattern = '/tmp/sosreport-%s-*' % name
Let's avoid duplicating code. How about:
patterns = ['/tmp/sosreport-%s-*', '/var/tmp/sosreport-%s-*'] reports = [] for p in patterns: reports.append(glob.glob(p % name))
for f in reports: ...
ok
- for reportFile in glob.glob(pattern): - if not fnmatch.fnmatch(reportFile, '*.md5'): - output = reportFile + reports = glob.glob(pattern) + reportFile = None + for f in reports: + if not fnmatch.fnmatch(f, '*.md5'): + reportFile = f break - else: - # sosreport tends to change the name mangling rule and - # compression file format between different releases. - # It's possible to fail to match a report file even sosreport - # runs successfully. In future we might have a general name - # mangling function in kimchi to format the name before passing - # it to sosreport. Then we can delete this exception. - raise OperationFailed('Can not find generated debug report ' - 'named by %s' % pattern) - ext = output.split('.', 1)[1] + if reportFile is None: + # Search in /var/tmp + reports = glob.glob('/var' + pattern) + for f in reports: + if not fnmatch.fnmatch(f, '*.md5'): + reportFile = f + break + + # Some error in sosreport happened + if reportFile is None: + kimchi_log.error('Debug report file not found. See sosreport ' + 'output for detail:\n%s', output) + raise OperationFailed('Debug report file not found.') + + md5_report_file = reportFile + '.md5' + report_file_extension = '.' + reportFile.split('.', 1)[1] path = config.get_debugreports_path() - target = os.path.join(path, name) - target_file = '%s.%s' % (target, ext) - shutil.move(output, target_file) - os.remove('%s.md5' % output) + target = os.path.join(path, name + report_file_extension) + # Moving report + msg = 'Moving debug report file "%s" to "%s"' % (reportFile, target) + kimchi_log.info(msg) + shutil.move(reportFile, target) + # Deleting md5 + msg = 'Deleting report md5 file: "%s"' % (md5_report_file) + kimchi_log.info(msg) + md5 = open(md5_report_file).read().strip() + kimchi_log.info('Md5 file content: "%s"', md5) + os.remove(md5_report_file) cb('OK', True) - return
except OSError:
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
participants (2)
-
Aline Manera
-
Rodrigo Trujillo