Skip to content

Commit

Permalink
split build logs by platform
Browse files Browse the repository at this point in the history 
platform is identified from task name
returned by osbs-client in tuple form
(task_run_name, log_line) and checked against
platforms specified by the user

* CLOUDBLD-8139

Signed-off-by: Harsh Modi <hmodi@redhat.com>
  • Loading branch information
@hjmodi
hjmodi committed Feb 24, 2022
1 parent 0667b30 commit 61ef204
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 53 deletions.
70 changes: 45 additions & 25 deletions koji_containerbuild/plugins/builder_containerbuild.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Acts as a wrapper between Koji and OpenShift builsystem via osbs for building
containers."""

# Copyright (C) 2015 Red Hat, Inc.
# Copyright (C) 2015-2022 Red Hat, Inc.
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
Expand Down Expand Up @@ -390,8 +390,9 @@ def _upload_logs_once(self):
except koji.ActionNotAllowed:
pass

def _write_logs(self, build_id, logs_dir):
log_filename = os.path.join(logs_dir, self.incremental_log_basename)
def _write_logs(self, build_id, logs_dir, platforms: list = None):
logfiles = {'noarch': open(os.path.join(logs_dir, self.incremental_log_basename),
'wb')}
self.logger.info("Will write follow log: %s", self.incremental_log_basename)
try:
logs = self.osbs().get_build_logs(build_id, follow=True, wait=True)
Expand All @@ -401,25 +402,43 @@ def _write_logs(self, build_id, logs_dir):

user_warnings = UserWarningsStore()

with open(log_filename, 'wb') as outfile:
for line in logs:
if METADATA_TAG in line:
_, meta_file = line.rsplit(' ', 1)
source_file = os.path.join(koji.pathinfo.work(), meta_file)
uploadpath = os.path.join(logs_dir, os.path.basename(meta_file))
shutil.copy(source_file, uploadpath)
continue
for task_run_name, line in logs:
if METADATA_TAG in line:
_, meta_file = line.rsplit(' ', 1)
source_file = os.path.join(koji.pathinfo.work(), meta_file)
uploadpath = os.path.join(logs_dir, os.path.basename(meta_file))
shutil.copy(source_file, uploadpath)
continue

if user_warnings.is_user_warning(line):
user_warnings.store(line)
continue

if platforms:
task_platform = next(
(platform for platform in platforms if
platform.replace('_', '-') in task_run_name),
'noarch'
)
else:
task_platform = 'noarch'

if user_warnings.is_user_warning(line):
user_warnings.store(line)
continue
if task_platform not in logfiles:
log_filename = f'{task_platform}.log'
logfiles[task_platform] = open(os.path.join(logs_dir, log_filename),
'wb')

try:
outfile.write(("%s\n" % line).encode('utf-8'))
outfile.flush()
except Exception as error:
msg = "Exception (%s) while writing build logs: %s" % (type(error), error)
raise ContainerError(msg)
outfile = logfiles[task_platform]

try:
outfile.write(("%s\n" % line).encode('utf-8'))
outfile.flush()
except Exception as error:
msg = "Exception (%s) while writing build logs: %s" % (type(error), error)
raise ContainerError(msg)

for logfile in logfiles.values():
logfile.close()

if user_warnings:
try:
Expand All @@ -434,8 +453,8 @@ def _write_logs(self, build_id, logs_dir):

self.logger.info("%s written", self.incremental_log_basename)

def _write_incremental_logs(self, build_id, logs_dir):
self._write_logs(build_id, logs_dir)
def _write_incremental_logs(self, build_id, logs_dir, platforms: list = None):
self._write_logs(build_id, logs_dir, platforms=platforms)

if self.osbs().build_not_finished(build_id):
raise ContainerError("Build log finished but build still has not "
Expand Down Expand Up @@ -483,7 +502,7 @@ def upload_build_results_as_annotations(self, build_results):
incremental_upload(self.session, ANNOTATIONS_FILENAME, f, self.getUploadPath(),
logger=self.logger)

def handle_build_response(self, build_id):
def handle_build_response(self, build_id, platforms: list = None):
self.logger.debug("OSBS build id: %r", build_id)

# When builds are cancelled the builder plugin process gets SIGINT and SIGKILL
Expand All @@ -509,7 +528,7 @@ def sigint_handler(*args, **kwargs):
self._osbs = None

try:
self._write_incremental_logs(build_id, osbs_logs_dir)
self._write_incremental_logs(build_id, osbs_logs_dir, platforms=platforms)
except Exception as error:
self.logger.info("Error while saving incremental logs: %s", error)
os._exit(1)
Expand Down Expand Up @@ -765,7 +784,8 @@ def createContainer(self, src=None, target_info=None, arches=None,
except OsbsValidationException as exc:
raise ContainerError('OSBS validation exception: {0}'.format(exc))

return self.handle_build_response(self.osbs().get_build_name(build_response))
return self.handle_build_response(self.osbs().get_build_name(build_response),
platforms=arches)

def getArchList(self, build_tag, extra=None):
"""Copied from build task"""
Expand Down
82 changes: 54 additions & 28 deletions tests/test_builder_containerbuild.py
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
Copyright (C) 2019 Red Hat, Inc.
Copyright (C) 2019-2022 Red Hat, Inc.
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
Expand Down Expand Up @@ -95,7 +95,7 @@ def test_osbs(self, task_method, method, scratch):
assert isinstance(osbs_obj, osbs.api.OSBS)
assert osbs_obj.os_conf.conf_section == expected_conf_section

def _check_logfiles(self, log_entries, logs_dir):
def _check_logfiles(self, log_entries, logs_dir, platforms: list = None):
def check_meta_entry(filename):
source_file = os.path.join(koji.pathinfo.work(), filename)
target_file = os.path.join(logs_dir, filename)
Expand All @@ -106,16 +106,29 @@ def check_meta_entry(filename):
user_warnings = UserWarningsStore()
log_contents = {}
log_name = 'osbs-build'
for line in log_entries:
for task_run_name, line in log_entries:

if platforms:
task_platform = next(
(platform for platform in platforms if
platform.replace('_', '-') in task_run_name),
'noarch'
)
else:
task_platform = 'noarch'

if builder_containerbuild.METADATA_TAG in line:
_, meta_file = line.rsplit(' ', 1)
check_meta_entry(meta_file)
elif user_warnings.is_user_warning(line):
user_warnings.store(line)
else:
log_contents[log_name] = '{old}{new}\n'.format(
old=log_contents.get(log_name, ''),
if task_platform != 'noarch':
log_filename = f'{task_platform}'
else:
log_filename = log_name
log_contents[log_filename] = '{old}{new}\n'.format(
old=log_contents.get(log_filename, ''),
new=line
)

Expand All @@ -138,14 +151,26 @@ def test_write_logs(self, tmpdir, get_logs_exc, build_not_finished):
workdir='workdir')

log_entries = [
'line 1',
'line 2',
'log - USER_WARNING - {"message": "message"}',
'x86_64 line 1',
'x86_64 line 2',
'x86_64 log - USER_WARNING - {"message": "message"}',
'x86_64 log - USER_WARNING - {"message": "another_message"}',
builder_containerbuild.METADATA_TAG + ' x.log'
('task_run', 'line 1'),
('task_run', 'line 2'),
('task_run', 'log - USER_WARNING - {"message": "message"}'),
('task_run_x86-64', 'x86_64 line 1'),
('task_run_x86-64', 'x86_64 line 2'),
('task_run_x86-64', 'x86_64 log - USER_WARNING - {"message": "message"}'),
('task_run_x86-64', 'x86_64 log - USER_WARNING - {"message": "another_message"}'),
('task_run_s390x', 's390x line 1'),
('task_run_s390x', 's390x line 2'),
('task_run_s390x', 's390x log - USER_WARNING - {"message": "message"}'),
('task_run_s390x', 's390x log - USER_WARNING - {"message": "another_message"}'),
('task_run_ppc64le', 'ppc64le line 1'),
('task_run_ppc64le', 'ppc64le line 2'),
('task_run_ppc64le', 'ppc64le log - USER_WARNING - {"message": "message"}'),
('task_run_ppc64le', 'ppc64le log - USER_WARNING - {"message": "another_message"}'),
('task_run_aarch64', 'aarch64 line 1'),
('task_run_aarch64', 'aarch64 line 2'),
('task_run_aarch64', 'aarch64 log - USER_WARNING - {"message": "message"}'),
('task_run_aarch64', 'aarch64 log - USER_WARNING - {"message": "another_message"}'),
('task_run', builder_containerbuild.METADATA_TAG + ' x.log')
]

koji_tmpdir = tmpdir.mkdir('koji')
Expand Down Expand Up @@ -180,15 +205,16 @@ def test_write_logs(self, tmpdir, get_logs_exc, build_not_finished):
else:
exc_type = exc_msg = None

platforms = ['x86_64', 's390x', 'ppc64le', 'aarch64']
if exc_type is not None:
with pytest.raises(exc_type) as exc_info:
cct._write_incremental_logs('id', str(tmpdir))
cct._write_incremental_logs('id', str(tmpdir), platforms=platforms)
assert str(exc_info.value) == exc_msg
else:
cct._write_incremental_logs('id', str(tmpdir))
cct._write_incremental_logs('id', str(tmpdir), platforms=platforms)

if get_logs_exc is None:
self._check_logfiles(log_entries, str(tmpdir))
self._check_logfiles(log_entries, str(tmpdir), platforms=platforms)

@pytest.mark.parametrize('get_logs_exc', [None, Exception('error')])
@pytest.mark.parametrize('build_not_finished', [False, True])
Expand All @@ -199,8 +225,8 @@ def test_write_logs_source(self, tmpdir, get_logs_exc, build_not_finished):
session='session',
options='options',
workdir='workdir')
log_entries = ['line 1',
'line 2']
log_entries = [('task_run', 'line 1'),
('task_run', 'line 2')]

(flexmock(osbs.api.OSBS)
.should_receive('build_not_finished').and_return(build_not_finished))
Expand Down Expand Up @@ -1680,11 +1706,11 @@ def test_raise_OsbsValidationException(self, tmpdir):

def test_user_warnings(self, tmpdir):
log_entries = [
'normal log',
'log - USER_WARNING - {"message": "message"}',
'log - USER_WARNING - {"message": "message"}',
'log - USER_WARNING - {"message": "another_message"}',
'another log',
('task_run', 'normal log'),
('task_run', 'log - USER_WARNING - {"message": "message"}'),
('task_run', 'log - USER_WARNING - {"message": "message"}'),
('task_run', 'log - USER_WARNING - {"message": "another_message"}'),
('task_run', 'another log'),
]

koji_task_id = 123
Expand Down Expand Up @@ -1729,11 +1755,11 @@ def test_user_warnings(self, tmpdir):

def test_user_warnings_source(self, tmpdir):
log_entries = [
'normal log',
'log - USER_WARNING - {"message": "message"}',
'log - USER_WARNING - {"message": "message"}',
'log - USER_WARNING - {"message": "another_message"}',
'another log',
('task_run', 'normal log'),
('task_run', 'log - USER_WARNING - {"message": "message"}'),
('task_run', 'log - USER_WARNING - {"message": "message"}'),
('task_run', 'log - USER_WARNING - {"message": "another_message"}'),
('task_run', 'another log'),
]

koji_task_id = 123
Expand Down

0 comments on commit 61ef204

Please sign in to comment.