From 7da303334ebeb86144a241c09d82c0eb1ec27da0 Mon Sep 17 00:00:00 2001 From: Scott Wittenburg Date: Thu, 13 Oct 2022 16:35:07 -0600 Subject: gitlab ci: Print better information about broken specs (#33124) When a pipeline generation job is automatically failed because it generated jobs for specs known to be broken on develop, print better information about the broken specs that were encountered. Include at a minimum the hash and the url of the job whose failure caused it to be put on the broken specs list in the first place. --- lib/spack/spack/ci.py | 85 +++++++++++++++++++++++++++++++++++++----- lib/spack/spack/cmd/ci.py | 39 +++++-------------- lib/spack/spack/test/cmd/ci.py | 23 +++++++----- 3 files changed, 98 insertions(+), 49 deletions(-) diff --git a/lib/spack/spack/ci.py b/lib/spack/spack/ci.py index 198e787dea..33724e7ac8 100644 --- a/lib/spack/spack/ci.py +++ b/lib/spack/spack/ci.py @@ -4,6 +4,7 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) import base64 +import codecs import copy import json import os @@ -1021,9 +1022,7 @@ def generate_gitlab_ci_yaml( continue if broken_spec_urls is not None and release_spec_dag_hash in broken_spec_urls: - known_broken_specs_encountered.append( - "{0} ({1})".format(release_spec, release_spec_dag_hash) - ) + known_broken_specs_encountered.append(release_spec_dag_hash) # Only keep track of these if we are copying rebuilt cache entries if spack_buildcache_copy: @@ -1286,6 +1285,7 @@ def generate_gitlab_ci_yaml( "SPACK_JOB_TEST_DIR": rel_job_test_dir, "SPACK_LOCAL_MIRROR_DIR": rel_local_mirror_dir, "SPACK_PIPELINE_TYPE": str(spack_pipeline_type), + "SPACK_CI_STACK_NAME": os.environ.get("SPACK_CI_STACK_NAME", "None"), } if remote_mirror_override: @@ -1343,13 +1343,9 @@ def generate_gitlab_ci_yaml( sorted_output = {"no-specs-to-rebuild": noop_job} if known_broken_specs_encountered: - error_msg = ( - "Pipeline generation failed due to the presence of the " - "following specs that are known to be broken in develop:\n" - ) - for broken_spec in known_broken_specs_encountered: - error_msg += "* {0}\n".format(broken_spec) - tty.die(error_msg) + tty.error("This pipeline generated hashes known to be broken on develop:") + display_broken_spec_messages(broken_specs_url, known_broken_specs_encountered) + tty.die() with open(output_file, "w") as outf: outf.write(syaml.dump_config(sorted_output, default_flow_style=True)) @@ -2060,6 +2056,75 @@ def create_buildcache(**kwargs): push_mirror_contents(env, json_path, pipeline_mirror_url, sign_binaries) +def write_broken_spec(url, pkg_name, stack_name, job_url, pipeline_url, spec_dict): + """Given a url to write to and the details of the failed job, write an entry + in the broken specs list. + """ + tmpdir = tempfile.mkdtemp() + file_path = os.path.join(tmpdir, "broken.txt") + + broken_spec_details = { + "broken-spec": { + "job-name": pkg_name, + "job-stack": stack_name, + "job-url": job_url, + "pipeline-url": pipeline_url, + "concrete-spec-dict": spec_dict, + } + } + + try: + with open(file_path, "w") as fd: + fd.write(syaml.dump(broken_spec_details)) + web_util.push_to_url( + file_path, + url, + keep_original=False, + extra_args={"ContentType": "text/plain"}, + ) + except Exception as err: + # If there is an S3 error (e.g., access denied or connection + # error), the first non boto-specific class in the exception + # hierarchy is Exception. Just print a warning and return + msg = "Error writing to broken specs list {0}: {1}".format(url, err) + tty.warn(msg) + finally: + shutil.rmtree(tmpdir) + + +def read_broken_spec(broken_spec_url): + """Read data from broken specs file located at the url, return as a yaml + object. + """ + try: + _, _, fs = web_util.read_from_url(broken_spec_url) + except (URLError, web_util.SpackWebError, HTTPError): + tty.warn("Unable to read broken spec from {0}".format(broken_spec_url)) + return None + + broken_spec_contents = codecs.getreader("utf-8")(fs).read() + return syaml.load(broken_spec_contents) + + +def display_broken_spec_messages(base_url, hashes): + """Fetch the broken spec file for each of the hashes under the base_url and + print a message with some details about each one. + """ + broken_specs = [(h, read_broken_spec(url_util.join(base_url, h))) for h in hashes] + for spec_hash, broken_spec in [tup for tup in broken_specs if tup[1]]: + details = broken_spec["broken-spec"] + if "job-name" in details: + item_name = "{0}/{1}".format(details["job-name"], spec_hash[:7]) + else: + item_name = spec_hash + + if "job-stack" in details: + item_name = "{0} (in stack {1})".format(item_name, details["job-stack"]) + + msg = " {0} was reported broken here: {1}".format(item_name, details["job-url"]) + tty.msg(msg) + + def run_standalone_tests(**kwargs): """Run stand-alone tests on the current spec. diff --git a/lib/spack/spack/cmd/ci.py b/lib/spack/spack/cmd/ci.py index 7bb0497c81..cd7a0bb767 100644 --- a/lib/spack/spack/cmd/ci.py +++ b/lib/spack/spack/cmd/ci.py @@ -7,7 +7,6 @@ import json import os import shutil import sys -import tempfile import llnl.util.filesystem as fs import llnl.util.tty as tty @@ -19,7 +18,6 @@ import spack.config as cfg import spack.environment as ev import spack.hash_types as ht import spack.mirror -import spack.util.spack_yaml as syaml import spack.util.url as url_util import spack.util.web as web_util @@ -285,6 +283,7 @@ def ci_rebuild(args): spack_pipeline_type = get_env_var("SPACK_PIPELINE_TYPE") remote_mirror_override = get_env_var("SPACK_REMOTE_MIRROR_OVERRIDE") remote_mirror_url = get_env_var("SPACK_REMOTE_MIRROR_URL") + spack_ci_stack_name = get_env_var("SPACK_CI_STACK_NAME") # Construct absolute paths relative to current $CI_PROJECT_DIR ci_project_dir = get_env_var("CI_PROJECT_DIR") @@ -547,34 +546,14 @@ def ci_rebuild(args): dev_fail_hash = job_spec.dag_hash() broken_spec_path = url_util.join(broken_specs_url, dev_fail_hash) tty.msg("Reporting broken develop build as: {0}".format(broken_spec_path)) - tmpdir = tempfile.mkdtemp() - empty_file_path = os.path.join(tmpdir, "empty.txt") - - broken_spec_details = { - "broken-spec": { - "job-url": get_env_var("CI_JOB_URL"), - "pipeline-url": get_env_var("CI_PIPELINE_URL"), - "concrete-spec-dict": job_spec.to_dict(hash=ht.dag_hash), - } - } - - try: - with open(empty_file_path, "w") as efd: - efd.write(syaml.dump(broken_spec_details)) - web_util.push_to_url( - empty_file_path, - broken_spec_path, - keep_original=False, - extra_args={"ContentType": "text/plain"}, - ) - except Exception as err: - # If there is an S3 error (e.g., access denied or connection - # error), the first non boto-specific class in the exception - # hierarchy is Exception. Just print a warning and return - msg = "Error writing to broken specs list {0}: {1}".format(broken_spec_path, err) - tty.warn(msg) - finally: - shutil.rmtree(tmpdir) + spack_ci.write_broken_spec( + broken_spec_path, + job_spec_pkg_name, + spack_ci_stack_name, + get_env_var("CI_JOB_URL"), + get_env_var("CI_PIPELINE_URL"), + job_spec.to_dict(hash=ht.dag_hash), + ) # We generated the "spack install ..." command to "--keep-stage", copy # any logs from the staging directory to artifacts now diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py index a8eaa2c631..37189af73e 100644 --- a/lib/spack/spack/test/cmd/ci.py +++ b/lib/spack/spack/test/cmd/ci.py @@ -1959,13 +1959,16 @@ def test_ci_generate_read_broken_specs_url( spec_flattendeps.concretize() flattendeps_dag_hash = spec_flattendeps.dag_hash() - # Mark 'a' as broken (but not 'flatten-deps') - broken_spec_a_path = str(tmpdir.join(a_dag_hash)) - with open(broken_spec_a_path, "w") as bsf: - bsf.write("") - broken_specs_url = "file://{0}".format(tmpdir.strpath) + # Mark 'a' as broken (but not 'flatten-deps') + broken_spec_a_url = "{0}/{1}".format(broken_specs_url, a_dag_hash) + job_stack = "job_stack" + a_job_url = "a_job_url" + ci.write_broken_spec( + broken_spec_a_url, spec_a.name, job_stack, a_job_url, "pipeline_url", spec_a.to_dict() + ) + # Test that `spack ci generate` notices this broken spec and fails. filename = str(tmpdir.join("spack.yaml")) with open(filename, "w") as f: @@ -2001,11 +2004,13 @@ spack: output = ci_cmd("generate", output=str, fail_on_error=False) assert "known to be broken" in output - ex = "({0})".format(a_dag_hash) - assert ex in output + expected = "{0}/{1} (in stack {2}) was reported broken here: {3}".format( + spec_a.name, a_dag_hash[:7], job_stack, a_job_url + ) + assert expected in output - ex = "({0})".format(flattendeps_dag_hash) - assert ex not in output + not_expected = "flatten-deps/{0} (in stack".format(flattendeps_dag_hash[:7]) + assert not_expected not in output def test_ci_generate_external_signing_job( -- cgit v1.2.3-60-g2f50