improvements to our CDash reporter (#11168)

* Make a separate CDash report for each package installed

Previously, we generated a single CDash report ("build") for the complete results
of running a `spack install` command. Now we create a separate CDash build for
each package that was installed.

This commit also changes some of the tests related to CDash reporting.
Now only one of the tests exercises the code path of uploading to a
(nonexistent) CDash server. The rest of the related tests write their reports
to disk without trying to upload them.

* Don't report errors to CDash for successful packages

Convert errors detected by our log scraper into warnings when the package
being installed reports that it was successful.

* Report a maximum of 50 errors/warnings to CDash

This is in line with what CTest does. The idea is that if you have more than
50 errors/warnings you probably aren't going to read through them all anyway.
This change reduces the amount of data that we need to transfer and store.
This commit is contained in:
Zack Galbreath 2019-04-18 12:39:35 -04:00 committed by Scott Wittenburg
parent e64ee7e1be
commit 7febb88c2a
2 changed files with 127 additions and 84 deletions

View File

@ -12,11 +12,12 @@
import socket import socket
import time import time
import xml.sax.saxutils import xml.sax.saxutils
from six import text_type from six import iteritems, text_type
from six.moves.urllib.request import build_opener, HTTPHandler, Request from six.moves.urllib.request import build_opener, HTTPHandler, Request
from six.moves.urllib.parse import urlencode from six.moves.urllib.parse import urlencode
from llnl.util.filesystem import working_dir from llnl.util.filesystem import working_dir
from ordereddict_backport import OrderedDict
import spack.build_environment import spack.build_environment
import spack.fetch_strategy import spack.fetch_strategy
import spack.package import spack.package
@ -59,6 +60,11 @@ def __init__(self, args):
Reporter.__init__(self, args) Reporter.__init__(self, args)
self.template_dir = os.path.join('reports', 'cdash') self.template_dir = os.path.join('reports', 'cdash')
self.cdash_upload_url = args.cdash_upload_url self.cdash_upload_url = args.cdash_upload_url
if self.cdash_upload_url:
self.buildid_regexp = re.compile("<buildId>([0-9]+)</buildId>")
self.phase_regexp = re.compile(r"Executing phase: '(.*)'")
if args.package: if args.package:
packages = args.package packages = args.package
else: else:
@ -68,7 +74,7 @@ def __init__(self, args):
s = spack.spec.Spec.from_yaml(f) s = spack.spec.Spec.from_yaml(f)
packages.append(s.format()) packages.append(s.format())
self.install_command = ' '.join(packages) self.install_command = ' '.join(packages)
self.buildname = args.cdash_build or self.install_command self.base_buildname = args.cdash_build or self.install_command
self.site = args.cdash_site or socket.gethostname() self.site = args.cdash_site or socket.gethostname()
self.osname = platform.system() self.osname = platform.system()
self.endtime = int(time.time()) self.endtime = int(time.time())
@ -78,15 +84,25 @@ def __init__(self, args):
buildstamp_format = "%Y%m%d-%H%M-{0}".format(args.cdash_track) buildstamp_format = "%Y%m%d-%H%M-{0}".format(args.cdash_track)
self.buildstamp = time.strftime(buildstamp_format, self.buildstamp = time.strftime(buildstamp_format,
time.localtime(self.endtime)) time.localtime(self.endtime))
self.buildId = None self.buildIds = OrderedDict()
self.revision = '' self.revision = ''
git = which('git') git = which('git')
with working_dir(spack.paths.spack_root): with working_dir(spack.paths.spack_root):
self.revision = git('rev-parse', 'HEAD', output=str).strip() self.revision = git('rev-parse', 'HEAD', output=str).strip()
self.multiple_packages = False
def build_report(self, filename, report_data): def report_for_package(self, directory_name, package, duration):
self.initialize_report(filename, report_data) if 'stdout' not in package:
# Skip reporting on packages that did not generate any output.
return
self.current_package_name = package['name']
if self.multiple_packages:
self.buildname = "{0} - {1}".format(
self.base_buildname, package['name'])
else:
self.buildname = self.base_buildname
report_data = self.initialize_report(directory_name)
for phase in cdash_phases: for phase in cdash_phases:
report_data[phase] = {} report_data[phase] = {}
report_data[phase]['loglines'] = [] report_data[phase]['loglines'] = []
@ -94,40 +110,32 @@ def build_report(self, filename, report_data):
report_data[phase]['endtime'] = self.endtime report_data[phase]['endtime'] = self.endtime
# Track the phases we perform so we know what reports to create. # Track the phases we perform so we know what reports to create.
phases_encountered = [] # We always report the update step because this is how we tell CDash
total_duration = 0 # what revision of Spack we are using.
phases_encountered = ['update']
# Parse output phase-by-phase. # Generate a report for this package.
phase_regexp = re.compile(r"Executing phase: '(.*)'") current_phase = ''
cdash_phase = '' cdash_phase = ''
for spec in report_data['specs']: for line in package['stdout'].splitlines():
if 'time' in spec: match = None
total_duration += int(spec['time']) if line.find("Executing phase: '") != -1:
for package in spec['packages']: match = self.phase_regexp.search(line)
if 'stdout' in package: if match:
current_phase = match.group(1)
if current_phase not in map_phases_to_cdash:
current_phase = '' current_phase = ''
cdash_phase = '' continue
for line in package['stdout'].splitlines(): cdash_phase = \
match = None map_phases_to_cdash[current_phase]
if line.find("Executing phase: '") != -1: if cdash_phase not in phases_encountered:
match = phase_regexp.search(line) phases_encountered.append(cdash_phase)
if match: report_data[cdash_phase]['loglines'].append(
current_phase = match.group(1) text_type("{0} output for {1}:".format(
if current_phase not in map_phases_to_cdash: cdash_phase, package['name'])))
current_phase = '' elif cdash_phase:
continue report_data[cdash_phase]['loglines'].append(
cdash_phase = \ xml.sax.saxutils.escape(line))
map_phases_to_cdash[current_phase]
if cdash_phase not in phases_encountered:
phases_encountered.append(cdash_phase)
report_data[cdash_phase]['loglines'].append(
text_type("{0} output for {1}:".format(
cdash_phase, package['name'])))
elif cdash_phase:
report_data[cdash_phase]['loglines'].append(
xml.sax.saxutils.escape(line))
phases_encountered.append('update')
# Move the build phase to the front of the list if it occurred. # Move the build phase to the front of the list if it occurred.
# This supports older versions of CDash that expect this phase # This supports older versions of CDash that expect this phase
@ -136,12 +144,15 @@ def build_report(self, filename, report_data):
build_pos = phases_encountered.index("build") build_pos = phases_encountered.index("build")
phases_encountered.insert(0, phases_encountered.pop(build_pos)) phases_encountered.insert(0, phases_encountered.pop(build_pos))
self.starttime = self.endtime - total_duration self.starttime = self.endtime - duration
for phase in phases_encountered: for phase in phases_encountered:
report_data[phase]['starttime'] = self.starttime report_data[phase]['starttime'] = self.starttime
report_data[phase]['log'] = \ report_data[phase]['log'] = \
'\n'.join(report_data[phase]['loglines']) '\n'.join(report_data[phase]['loglines'])
errors, warnings = parse_log_events(report_data[phase]['loglines']) errors, warnings = parse_log_events(report_data[phase]['loglines'])
# Cap the number of errors and warnings at 50 each.
errors = errors[0:49]
warnings = warnings[0:49]
nerrors = len(errors) nerrors = len(errors)
if phase == 'configure' and nerrors > 0: if phase == 'configure' and nerrors > 0:
@ -166,6 +177,11 @@ def clean_log_event(event):
event['source_file']) event['source_file'])
return event return event
# Convert errors to warnings if the package reported success.
if package['result'] == 'success':
warnings = errors + warnings
errors = []
report_data[phase]['errors'] = [] report_data[phase]['errors'] = []
report_data[phase]['warnings'] = [] report_data[phase]['warnings'] = []
for error in errors: for error in errors:
@ -179,7 +195,11 @@ def clean_log_event(event):
# Write the report. # Write the report.
report_name = phase.capitalize() + ".xml" report_name = phase.capitalize() + ".xml"
phase_report = os.path.join(filename, report_name) if self.multiple_packages:
report_file_name = package['name'] + "_" + report_name
else:
report_file_name = report_name
phase_report = os.path.join(directory_name, report_file_name)
with codecs.open(phase_report, 'w', 'utf-8') as f: with codecs.open(phase_report, 'w', 'utf-8') as f:
env = spack.tengine.make_environment() env = spack.tengine.make_environment()
@ -194,11 +214,34 @@ def clean_log_event(event):
t = env.get_template(phase_template) t = env.get_template(phase_template)
f.write(t.render(report_data)) f.write(t.render(report_data))
self.upload(phase_report) self.upload(phase_report)
def build_report(self, directory_name, input_data):
# Do an initial scan to determine if we are generating reports for more
# than one package. When we're only reporting on a single package we
# do not explicitly include the package's name in the CDash build name.
num_packages = 0
for spec in input_data['specs']:
for package in spec['packages']:
if 'stdout' in package:
num_packages += 1
if num_packages > 1:
self.multiple_packages = True
break
if self.multiple_packages:
break
# Generate reports for each package in each spec.
for spec in input_data['specs']:
duration = 0
if 'time' in spec:
duration = int(spec['time'])
for package in spec['packages']:
self.report_for_package(directory_name, package, duration)
self.print_cdash_link() self.print_cdash_link()
def concretization_report(self, filename, msg): def concretization_report(self, directory_name, msg):
report_data = {} self.buildname = self.base_buildname
self.initialize_report(filename, report_data) report_data = self.initialize_report(directory_name)
report_data['update'] = {} report_data['update'] = {}
report_data['update']['starttime'] = self.endtime report_data['update']['starttime'] = self.endtime
report_data['update']['endtime'] = self.endtime report_data['update']['endtime'] = self.endtime
@ -208,20 +251,25 @@ def concretization_report(self, filename, msg):
env = spack.tengine.make_environment() env = spack.tengine.make_environment()
update_template = os.path.join(self.template_dir, 'Update.xml') update_template = os.path.join(self.template_dir, 'Update.xml')
t = env.get_template(update_template) t = env.get_template(update_template)
output_filename = os.path.join(filename, 'Update.xml') output_filename = os.path.join(directory_name, 'Update.xml')
with open(output_filename, 'w') as f: with open(output_filename, 'w') as f:
f.write(t.render(report_data)) f.write(t.render(report_data))
# We don't have a current package when reporting on concretization
# errors so refer to this report with the base buildname instead.
self.current_package_name = self.base_buildname
self.upload(output_filename) self.upload(output_filename)
self.print_cdash_link() self.print_cdash_link()
def initialize_report(self, filename, report_data): def initialize_report(self, directory_name):
if not os.path.exists(filename): if not os.path.exists(directory_name):
os.mkdir(filename) os.mkdir(directory_name)
report_data = {}
report_data['buildname'] = self.buildname report_data['buildname'] = self.buildname
report_data['buildstamp'] = self.buildstamp report_data['buildstamp'] = self.buildstamp
report_data['install_command'] = self.install_command report_data['install_command'] = self.install_command
report_data['osname'] = self.osname report_data['osname'] = self.osname
report_data['site'] = self.site report_data['site'] = self.site
return report_data
def upload(self, filename): def upload(self, filename):
if not self.cdash_upload_url: if not self.cdash_upload_url:
@ -230,7 +278,6 @@ def upload(self, filename):
# Compute md5 checksum for the contents of this file. # Compute md5 checksum for the contents of this file.
md5sum = checksum(hashlib.md5, filename, block_size=8192) md5sum = checksum(hashlib.md5, filename, block_size=8192)
buildid_regexp = re.compile("<buildId>([0-9]+)</buildId>")
opener = build_opener(HTTPHandler) opener = build_opener(HTTPHandler)
with open(filename, 'rb') as f: with open(filename, 'rb') as f:
params_dict = { params_dict = {
@ -248,16 +295,19 @@ def upload(self, filename):
# CDash needs expects this file to be uploaded via PUT. # CDash needs expects this file to be uploaded via PUT.
request.get_method = lambda: 'PUT' request.get_method = lambda: 'PUT'
response = opener.open(request) response = opener.open(request)
if not self.buildId: if self.current_package_name not in self.buildIds:
match = buildid_regexp.search(response.read()) match = self.buildid_regexp.search(response.read())
if match: if match:
self.buildId = match.group(1) buildid = match.group(1)
self.buildIds[self.current_package_name] = buildid
def print_cdash_link(self): def print_cdash_link(self):
if self.buildId: if self.buildIds:
# Construct and display a helpful link if CDash responded with print("View your build results here:")
# a buildId. for package_name, buildid in iteritems(self.buildIds):
build_url = self.cdash_upload_url # Construct and display a helpful link if CDash responded with
build_url = build_url[0:build_url.find("submit.php")] # a buildId.
build_url += "buildSummary.php?buildid={0}".format(self.buildId) build_url = self.cdash_upload_url
print("View your build results here:\n {0}\n".format(build_url)) build_url = build_url[0:build_url.find("submit.php")]
build_url += "buildSummary.php?buildid={0}".format(buildid)
print("{0}: {1}".format(package_name, build_url))

View File

@ -469,14 +469,13 @@ def test_cdash_upload_clean_build(tmpdir, mock_fetch, install_mockery,
# capfd interferes with Spack's capturing # capfd interferes with Spack's capturing
with capfd.disabled(): with capfd.disabled():
with tmpdir.as_cwd(): with tmpdir.as_cwd():
with pytest.raises((HTTPError, URLError)): install(
install( '--log-file=cdash_reports',
'--log-file=cdash_reports', '--log-format=cdash',
'--cdash-upload-url=http://localhost/fakeurl/submit.php?project=Spack', 'a')
'a')
report_dir = tmpdir.join('cdash_reports') report_dir = tmpdir.join('cdash_reports')
assert report_dir in tmpdir.listdir() assert report_dir in tmpdir.listdir()
report_file = report_dir.join('Build.xml') report_file = report_dir.join('a_Build.xml')
assert report_file in report_dir.listdir() assert report_file in report_dir.listdir()
content = report_file.open().read() content = report_file.open().read()
assert '</Build>' in content assert '</Build>' in content
@ -488,20 +487,19 @@ def test_cdash_upload_extra_params(tmpdir, mock_fetch, install_mockery, capfd):
# capfd interferes with Spack's capturing # capfd interferes with Spack's capturing
with capfd.disabled(): with capfd.disabled():
with tmpdir.as_cwd(): with tmpdir.as_cwd():
with pytest.raises((HTTPError, URLError)): install(
install( '--log-file=cdash_reports',
'--log-file=cdash_reports', '--log-format=cdash',
'--cdash-build=my_custom_build', '--cdash-build=my_custom_build',
'--cdash-site=my_custom_site', '--cdash-site=my_custom_site',
'--cdash-track=my_custom_track', '--cdash-track=my_custom_track',
'--cdash-upload-url=http://localhost/fakeurl/submit.php?project=Spack', 'a')
'a')
report_dir = tmpdir.join('cdash_reports') report_dir = tmpdir.join('cdash_reports')
assert report_dir in tmpdir.listdir() assert report_dir in tmpdir.listdir()
report_file = report_dir.join('Build.xml') report_file = report_dir.join('a_Build.xml')
assert report_file in report_dir.listdir() assert report_file in report_dir.listdir()
content = report_file.open().read() content = report_file.open().read()
assert 'Site BuildName="my_custom_build"' in content assert 'Site BuildName="my_custom_build - a"' in content
assert 'Name="my_custom_site"' in content assert 'Name="my_custom_site"' in content
assert '-my_custom_track' in content assert '-my_custom_track' in content
@ -515,21 +513,16 @@ def test_cdash_buildstamp_param(tmpdir, mock_fetch, install_mockery, capfd):
buildstamp_format = "%Y%m%d-%H%M-{0}".format(cdash_track) buildstamp_format = "%Y%m%d-%H%M-{0}".format(cdash_track)
buildstamp = time.strftime(buildstamp_format, buildstamp = time.strftime(buildstamp_format,
time.localtime(int(time.time()))) time.localtime(int(time.time())))
with pytest.raises((HTTPError, URLError)): install(
install( '--log-file=cdash_reports',
'--log-file=cdash_reports', '--log-format=cdash',
'--cdash-build=my_custom_build', '--cdash-buildstamp={0}'.format(buildstamp),
'--cdash-site=my_custom_site', 'a')
'--cdash-buildstamp={0}'.format(buildstamp),
'--cdash-upload-url=http://localhost/fakeurl/submit.php?project=Spack',
'a')
report_dir = tmpdir.join('cdash_reports') report_dir = tmpdir.join('cdash_reports')
assert report_dir in tmpdir.listdir() assert report_dir in tmpdir.listdir()
report_file = report_dir.join('Build.xml') report_file = report_dir.join('a_Build.xml')
assert report_file in report_dir.listdir() assert report_file in report_dir.listdir()
content = report_file.open().read() content = report_file.open().read()
assert 'Site BuildName="my_custom_build"' in content
assert 'Name="my_custom_site"' in content
assert buildstamp in content assert buildstamp in content
@ -559,7 +552,7 @@ def test_cdash_install_from_spec_yaml(tmpdir, mock_fetch, install_mockery,
report_dir = tmpdir.join('cdash_reports') report_dir = tmpdir.join('cdash_reports')
assert report_dir in tmpdir.listdir() assert report_dir in tmpdir.listdir()
report_file = report_dir.join('Configure.xml') report_file = report_dir.join('a_Configure.xml')
assert report_file in report_dir.listdir() assert report_file in report_dir.listdir()
content = report_file.open().read() content = report_file.open().read()
import re import re