ci: Don't raise an exception if we fail to relate CDash builds (#24299)

ci: Do not allow cdash-related errors to cause pipeline failure

4 files changed, 96 insertions, 73 deletions

diff --git a/lib/spack/spack/ci.py b/lib/spack/spack/ci.py
index 3e5e134e05..7afc074bca 100644
--- a/lib/spack/spack/ci.py
+++ b/lib/spack/spack/ci.py
@@ -81,7 +81,8 @@ def _create_buildgroup(opener, headers, url, project, group_name, group_type):
     if response_code != 200 and response_code != 201:
         msg = 'Creating buildgroup failed (response code = {0}'.format(
             response_code)
-        raise SpackError(msg)
+        tty.warn(msg)
+        return None
 
     response_text = response.read()
     response_json = json.loads(response_text)
@@ -110,7 +111,8 @@ def populate_buildgroup(job_names, group_name, project, site,
     if not parent_group_id or not group_id:
         msg = 'Failed to create or retrieve buildgroups for {0}'.format(
             group_name)
-        raise SpackError(msg)
+        tty.warn(msg)
+        return
 
     data = {
         'project': project,
@@ -133,7 +135,7 @@ def populate_buildgroup(job_names, group_name, project, site,
     if response_code != 200:
         msg = 'Error response code ({0}) in populate_buildgroup'.format(
             response_code)
-        raise SpackError(msg)
+        tty.warn(msg)
 
 
 def is_main_phase(phase_name):
@@ -1259,7 +1261,8 @@ def register_cdash_build(build_name, base_url, project, site, track):
 
     if response_code != 200 and response_code != 201:
         msg = 'Adding build failed (response code = {0}'.format(response_code)
-        raise SpackError(msg)
+        tty.warn(msg)
+        return (None, None)
 
     response_text = response.read()
     response_json = json.loads(response_text)
@@ -1296,8 +1299,9 @@ def relate_cdash_builds(spec_map, cdash_base_url, job_build_id, cdash_project,
                 tty.debug('Did not find cdashid for {0} on {1}'.format(
                     dep_pkg_name, url))
         else:
-            raise SpackError('Did not find cdashid for {0} anywhere'.format(
+            tty.warn('Did not find cdashid for {0} anywhere'.format(
                 dep_pkg_name))
+            return
 
         payload = {
             "project": cdash_project,
@@ -1318,7 +1322,8 @@ def relate_cdash_builds(spec_map, cdash_base_url, job_build_id, cdash_project,
         if response_code != 200 and response_code != 201:
             msg = 'Relate builds ({0} -> {1}) failed (resp code = {2})'.format(
                 job_build_id, dep_build_id, response_code)
-            raise SpackError(msg)
+            tty.warn(msg)
+            return
 
         response_text = response.read()
         tty.debug('Relate builds response: {0}'.format(response_text))
@@ -1341,7 +1346,16 @@ def write_cdashid_to_mirror(cdashid, spec, mirror_url):
         tty.debug('pushing cdashid to url')
         tty.debug('  local file path: {0}'.format(local_cdash_path))
         tty.debug('  remote url: {0}'.format(remote_url))
-        web_util.push_to_url(local_cdash_path, remote_url)
+
+        try:
+            web_util.push_to_url(local_cdash_path, remote_url)
+        except Exception as inst:
+            # No matter what went wrong here, don't allow the pipeline to fail
+            # just because there was an issue storing the cdashid on the mirror
+            msg = 'Failed to write cdashid {0} to mirror {1}'.format(
+                cdashid, mirror_url)
+            tty.warn(inst)
+            tty.warn(msg)
 
 
 def read_cdashid_from_mirror(spec, mirror_url):
@@ -1359,40 +1373,34 @@ def read_cdashid_from_mirror(spec, mirror_url):
     return int(contents)
 
 
-def push_mirror_contents(env, spec, yaml_path, mirror_url, build_id,
-                         sign_binaries):
-    if mirror_url:
-        try:
-            unsigned = not sign_binaries
-            tty.debug('Creating buildcache ({0})'.format(
-                'unsigned' if unsigned else 'signed'))
-            spack.cmd.buildcache._createtarball(
-                env, spec_yaml=yaml_path, add_deps=False,
-                output_location=mirror_url, force=True, allow_root=True,
-                unsigned=unsigned)
-            if build_id:
-                tty.debug('Writing cdashid ({0}) to remote mirror: {1}'.format(
-                    build_id, mirror_url))
-                write_cdashid_to_mirror(build_id, spec, mirror_url)
-        except Exception as inst:
-            # If the mirror we're pushing to is on S3 and there's some
-            # permissions problem, for example, we can't just target
-            # that exception type here, since users of the
-            # `spack ci rebuild' may not need or want any dependency
-            # on boto3.  So we use the first non-boto exception type
-            # in the heirarchy:
-            #     boto3.exceptions.S3UploadFailedError
-            #     boto3.exceptions.Boto3Error
-            #     Exception
-            #     BaseException
-            #     object
-            err_msg = 'Error msg: {0}'.format(inst)
-            if 'Access Denied' in err_msg:
-                tty.msg('Permission problem writing to {0}'.format(
-                    mirror_url))
-                tty.msg(err_msg)
-            else:
-                raise inst
+def push_mirror_contents(env, spec, yaml_path, mirror_url, sign_binaries):
+    try:
+        unsigned = not sign_binaries
+        tty.debug('Creating buildcache ({0})'.format(
+            'unsigned' if unsigned else 'signed'))
+        spack.cmd.buildcache._createtarball(
+            env, spec_yaml=yaml_path, add_deps=False,
+            output_location=mirror_url, force=True, allow_root=True,
+            unsigned=unsigned)
+    except Exception as inst:
+        # If the mirror we're pushing to is on S3 and there's some
+        # permissions problem, for example, we can't just target
+        # that exception type here, since users of the
+        # `spack ci rebuild' may not need or want any dependency
+        # on boto3.  So we use the first non-boto exception type
+        # in the heirarchy:
+        #     boto3.exceptions.S3UploadFailedError
+        #     boto3.exceptions.Boto3Error
+        #     Exception
+        #     BaseException
+        #     object
+        err_msg = 'Error msg: {0}'.format(inst)
+        if 'Access Denied' in err_msg:
+            tty.msg('Permission problem writing to {0}'.format(
+                mirror_url))
+            tty.msg(err_msg)
+        else:
+            raise inst
 
 
 def copy_stage_logs_to_artifacts(job_spec, job_log_dir):
diff --git a/lib/spack/spack/cmd/ci.py b/lib/spack/spack/cmd/ci.py
index 1f87924538..bc0c1685ae 100644
--- a/lib/spack/spack/cmd/ci.py
+++ b/lib/spack/spack/cmd/ci.py
@@ -230,7 +230,6 @@ def ci_rebuild(args):
         eq_idx = proj_enc.find('=') + 1
         cdash_project_enc = proj_enc[eq_idx:]
         cdash_site = ci_cdash['site']
-        cdash_id_path = os.path.join(repro_dir, 'cdash_id.txt')
         tty.debug('cdash_base_url = {0}'.format(cdash_base_url))
         tty.debug('cdash_project = {0}'.format(cdash_project))
         tty.debug('cdash_project_enc = {0}'.format(cdash_project_enc))
@@ -398,7 +397,7 @@ def ci_rebuild(args):
                 job_spec_pkg_name, matching_mirror))
             tty.debug('Downloading to {0}'.format(build_cache_dir))
             buildcache.download_buildcache_files(
-                job_spec, build_cache_dir, True, matching_mirror)
+                job_spec, build_cache_dir, False, matching_mirror)
 
         # Now we are done and successful
         sys.exit(0)
@@ -433,24 +432,21 @@ def ci_rebuild(args):
             cdash_build_name, cdash_base_url, cdash_project,
             cdash_site, job_spec_buildgroup)
 
-        cdash_upload_url = '{0}/submit.php?project={1}'.format(
-            cdash_base_url, cdash_project_enc)
+        if cdash_build_id is not None:
+            cdash_upload_url = '{0}/submit.php?project={1}'.format(
+                cdash_base_url, cdash_project_enc)
 
-        install_args.extend([
-            '--cdash-upload-url', cdash_upload_url,
-            '--cdash-build', cdash_build_name,
-            '--cdash-site', cdash_site,
-            '--cdash-buildstamp', cdash_build_stamp,
-        ])
+            install_args.extend([
+                '--cdash-upload-url', cdash_upload_url,
+                '--cdash-build', cdash_build_name,
+                '--cdash-site', cdash_site,
+                '--cdash-buildstamp', cdash_build_stamp,
+            ])
 
-        tty.debug('CDash: Relating build with dependency builds')
-        spack_ci.relate_cdash_builds(
-            spec_map, cdash_base_url, cdash_build_id, cdash_project,
-            [pipeline_mirror_url, pr_mirror_url, remote_mirror_url])
-
-        # store the cdash build id on disk for later
-        with open(cdash_id_path, 'w') as fd:
-            fd.write(cdash_build_id)
+            tty.debug('CDash: Relating build with dependency builds')
+            spack_ci.relate_cdash_builds(
+                spec_map, cdash_base_url, cdash_build_id, cdash_project,
+                [pipeline_mirror_url, pr_mirror_url, remote_mirror_url])
 
     # A compiler action of 'FIND_ANY' means we are building a bootstrap
     # compiler or one of its deps.
@@ -546,17 +542,31 @@ def ci_rebuild(args):
 
         # Create buildcache in either the main remote mirror, or in the
         # per-PR mirror, if this is a PR pipeline
-        spack_ci.push_mirror_contents(
-            env, job_spec, job_spec_yaml_path, buildcache_mirror_url,
-            cdash_build_id, sign_binaries)
+        if buildcache_mirror_url:
+            spack_ci.push_mirror_contents(
+                env, job_spec, job_spec_yaml_path, buildcache_mirror_url,
+                sign_binaries)
+
+            if cdash_build_id:
+                tty.debug('Writing cdashid ({0}) to remote mirror: {1}'.format(
+                    cdash_build_id, buildcache_mirror_url))
+                spack_ci.write_cdashid_to_mirror(
+                    cdash_build_id, job_spec, buildcache_mirror_url)
 
         # Create another copy of that buildcache in the per-pipeline
         # temporary storage mirror (this is only done if either
         # artifacts buildcache is enabled or a temporary storage url
         # prefix is set)
-        spack_ci.push_mirror_contents(
-            env, job_spec, job_spec_yaml_path, pipeline_mirror_url,
-            cdash_build_id, sign_binaries)
+        if pipeline_mirror_url:
+            spack_ci.push_mirror_contents(
+                env, job_spec, job_spec_yaml_path, pipeline_mirror_url,
+                sign_binaries)
+
+            if cdash_build_id:
+                tty.debug('Writing cdashid ({0}) to remote mirror: {1}'.format(
+                    cdash_build_id, pipeline_mirror_url))
+                spack_ci.write_cdashid_to_mirror(
+                    cdash_build_id, job_spec, pipeline_mirror_url)
     else:
         tty.debug('spack install exited non-zero, will not create buildcache')
 
diff --git a/lib/spack/spack/test/ci.py b/lib/spack/spack/test/ci.py
index d9c337e4f8..bdf1af6849 100644
--- a/lib/spack/spack/test/ci.py
+++ b/lib/spack/spack/test/ci.py
@@ -179,7 +179,7 @@ def test_register_cdash_build(monkeypatch):
 
 
 def test_relate_cdash_builds(config, mutable_mock_env_path, mock_packages,
-                             monkeypatch):
+                             monkeypatch, capfd):
     e = ev.create('test1')
     e.add('dyninst')
     e.concretize()
@@ -228,9 +228,11 @@ def test_relate_cdash_builds(config, mutable_mock_env_path, mock_packages,
         }
 
         fake_responder._resp_code = 400
-        with pytest.raises(spack.error.SpackError):
-            ci.relate_cdash_builds(spec_map, cdash_api_url, job_build_id,
-                                   cdash_project, [cdashids_mirror_url])
+        ci.relate_cdash_builds(spec_map, cdash_api_url, job_build_id,
+                               cdash_project, [cdashids_mirror_url])
+        out, err = capfd.readouterr()
+        assert('Warning: Relate builds' in err)
+        assert('failed' in err)
 
         dep_cdash_ids = {}
 
diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py
index b267596a72..702597b07f 100644
--- a/lib/spack/spack/test/cmd/ci.py
+++ b/lib/spack/spack/test/cmd/ci.py
@@ -894,8 +894,9 @@ spack:
 
 @pytest.mark.disable_clean_stage_check
 def test_push_mirror_contents(tmpdir, mutable_mock_env_path, env_deactivate,
-                              install_mockery, mock_packages, mock_fetch,
-                              mock_stage, mock_gnupghome, project_dir_env):
+                              install_mockery_mutable_config, mock_packages,
+                              mock_fetch, mock_stage, mock_gnupghome,
+                              project_dir_env):
     project_dir_env(tmpdir.strpath)
     working_dir = tmpdir.join('working_dir')
 
@@ -951,7 +952,9 @@ spack:
 
             # env, spec, yaml_path, mirror_url, build_id, sign_binaries
             ci.push_mirror_contents(
-                env, concrete_spec, yaml_path, mirror_url, '42', True)
+                env, concrete_spec, yaml_path, mirror_url, True)
+
+            ci.write_cdashid_to_mirror('42', concrete_spec, mirror_url)
 
             buildcache_path = os.path.join(mirror_dir.strpath, 'build_cache')
 
@@ -1054,7 +1057,7 @@ def test_push_mirror_contents_exceptions(monkeypatch, capsys):
     monkeypatch.setattr(buildcache, '_createtarball', faked)
 
     url = 'fakejunk'
-    ci.push_mirror_contents(None, None, None, url, None, None)
+    ci.push_mirror_contents(None, None, None, url, None)
 
     captured = capsys.readouterr()
     std_out = captured[0]