pipelines: support testing PRs from forks (#19248)

This change makes improvements to the `spack ci rebuild` command
which supports running gitlab pipelines on PRs from forks.  Much
of this has to do with making sure we can run without the secrets
previously required for running gitlab pipelines (e.g signing key,
aws credentials, etc).  Specific improvements in this PR:

Check if spack has precisely one signing key, and use that information
as an additional constraint on whether or not we should attempt to sign
the binary package we create.

Also, if spack does not have at least one public key, add the install
option "--no-check-signature"

If we are running a pipeline without any profile or environment
variables allowing us to push to S3, the pipeline could still
successfully create a buildcache in the artifacts and move on.  So
just print a message and move on if pushing either the buildcache
entry or cdash id file to the remote mirror fails.

When we attempt to generate a pacakge or gpg key index on an S3
mirror, and there is nothing to index, just print a warning and
exit gracefully rather than throw an exception.

Support the use of PR-specific mirrors for temporary binary pkg
storage.  This will allow quality-of-life improvement for developers,
providing a place to store binaries over the lifetime of a PR, so
that they must only wait for packages to rebuild from source when
they push a new commit that causes it to be necessary.

Replace two-pass install with a single pass and the new option:
 --require-full-hash-match.  Doing this also removes the need to
save a copy of the spack.yaml to be copied over the one spack
rewrites in between the two spack install passes.

Work around a mirror configuration issue caused by using
spack.util.executable to do the package installation.

* Update pipeline trigger jobs for PRs from forks

Moving to PRs from forks relies on external synchronization script
pushing special branch names.  Also secrets will only live on the
spack mirror project, and must be propagated to the E4S project via
variables on the trigger jobs.

When this change is merged, pipelines will not run until we update
the "Custom CI configuration path" in the Gitlab CI Settings, as the
name of the file has changed to better reflect its purpose.

* Arg to MirrorCollection is used exclusively, so add main remote mirror to it

* Compute full hash less frequently

* Add tests covering index generation error handling code
This commit is contained in:
Scott Wittenburg 2020-11-16 16:16:24 -07:00 committed by GitHub
parent ee5ae14a3b
commit ef0a555ca2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 259 additions and 124 deletions

View File

@ -698,10 +698,23 @@ def generate_package_index(cache_prefix):
enable_transaction_locking=False,
record_fields=['spec', 'ref_count'])
try:
file_list = (
entry
for entry in web_util.list_url(cache_prefix)
if entry.endswith('.yaml'))
except KeyError as inst:
msg = 'No packages at {0}: {1}'.format(cache_prefix, inst)
tty.warn(msg)
return
except Exception as err:
# If we got some kind of S3 (access denied or other connection
# error), the first non boto-specific class in the exception
# hierarchy is Exception. Just print a warning and return
msg = 'Encountered problem listing packages at {0}: {1}'.format(
cache_prefix, err)
tty.warn(msg)
return
tty.debug('Retrieving spec.yaml files from {0} to build index'.format(
cache_prefix))
@ -763,10 +776,23 @@ def generate_key_index(key_prefix, tmpdir=None):
url_util.format(key_prefix),
'to build key index')))
try:
fingerprints = (
entry[:-4]
for entry in web_util.list_url(key_prefix, recursive=False)
if entry.endswith('.pub'))
except KeyError as inst:
msg = 'No keys at {0}: {1}'.format(key_prefix, inst)
tty.warn(msg)
return
except Exception as err:
# If we got some kind of S3 (access denied or other connection
# error), the first non boto-specific class in the exception
# hierarchy is Exception. Just print a warning and return
msg = 'Encountered problem listing keys at {0}: {1}'.format(
key_prefix, err)
tty.warn(msg)
return
remove_tmpdir = False
@ -1284,15 +1310,16 @@ def extract_tarball(spec, filename, allow_root=False, unsigned=False,
os.remove(filename)
def try_direct_fetch(spec, force=False, full_hash_match=False):
def try_direct_fetch(spec, force=False, full_hash_match=False, mirrors=None):
"""
Try to find the spec directly on the configured mirrors
"""
specfile_name = tarball_name(spec, '.spec.yaml')
lenient = not full_hash_match
found_specs = []
spec_full_hash = spec.full_hash()
for mirror in spack.mirror.MirrorCollection().values():
for mirror in spack.mirror.MirrorCollection(mirrors=mirrors).values():
buildcache_fetch_url = url_util.join(
mirror.fetch_url, _build_cache_relative_path, specfile_name)
@ -1312,7 +1339,7 @@ def try_direct_fetch(spec, force=False, full_hash_match=False):
# Do not recompute the full hash for the fetched spec, instead just
# read the property.
if lenient or fetched_spec._full_hash == spec.full_hash():
if lenient or fetched_spec._full_hash == spec_full_hash:
found_specs.append({
'mirror_url': mirror.fetch_url,
'spec': fetched_spec,
@ -1321,7 +1348,8 @@ def try_direct_fetch(spec, force=False, full_hash_match=False):
return found_specs
def get_mirrors_for_spec(spec=None, force=False, full_hash_match=False):
def get_mirrors_for_spec(spec=None, force=False, full_hash_match=False,
mirrors_to_check=None):
"""
Check if concrete spec exists on mirrors and return a list
indicating the mirrors on which it can be found
@ -1329,7 +1357,7 @@ def get_mirrors_for_spec(spec=None, force=False, full_hash_match=False):
if spec is None:
return []
if not spack.mirror.MirrorCollection():
if not spack.mirror.MirrorCollection(mirrors=mirrors_to_check):
tty.debug("No Spack mirrors are currently configured")
return {}
@ -1354,7 +1382,9 @@ def filter_candidates(candidate_list):
if not results:
results = try_direct_fetch(spec,
force=force,
full_hash_match=full_hash_match)
full_hash_match=full_hash_match,
mirrors=mirrors_to_check)
if results:
binary_index.update_spec(spec, results)

View File

@ -33,12 +33,16 @@
from spack.spec import Spec
import spack.util.spack_yaml as syaml
import spack.util.web as web_util
import spack.util.gpg as gpg_util
import spack.util.url as url_util
JOB_RETRY_CONDITIONS = [
'always',
]
SPACK_PR_MIRRORS_ROOT_URL = 's3://spack-pr-mirrors'
spack_gpg = spack.main.SpackCommand('gpg')
spack_compiler = spack.main.SpackCommand('compiler')
@ -529,6 +533,12 @@ def generate_gitlab_ci_yaml(env, print_summary, output_file,
os.environ.get('SPACK_IS_PR_PIPELINE', '').lower() == 'true'
)
spack_pr_branch = os.environ.get('SPACK_PR_BRANCH', None)
pr_mirror_url = None
if spack_pr_branch:
pr_mirror_url = url_util.join(SPACK_PR_MIRRORS_ROOT_URL,
spack_pr_branch)
ci_mirrors = yaml_root['mirrors']
mirror_urls = [url for url in ci_mirrors.values()]
@ -858,6 +868,9 @@ def generate_gitlab_ci_yaml(env, print_summary, output_file,
'SPACK_CHECKOUT_VERSION': version_to_clone,
}
if pr_mirror_url:
output_object['variables']['SPACK_PR_MIRROR_URL'] = pr_mirror_url
sorted_output = {}
for output_key, output_value in sorted(output_object.items()):
sorted_output[output_key] = output_value
@ -920,6 +933,14 @@ def import_signing_key(base64_signing_key):
tty.debug(signing_keys_output)
def can_sign_binaries():
return len(gpg_util.signing_keys()) == 1
def can_verify_binaries():
return len(gpg_util.public_keys()) >= 1
def configure_compilers(compiler_action, scope=None):
if compiler_action == 'INSTALL_MISSING':
tty.debug('Make sure bootstrapped compiler will be installed')
@ -1095,12 +1116,15 @@ def read_cdashid_from_mirror(spec, mirror_url):
return int(contents)
def push_mirror_contents(env, spec, yaml_path, mirror_url, build_id):
def push_mirror_contents(env, spec, yaml_path, mirror_url, build_id,
sign_binaries):
if mirror_url:
tty.debug('Creating buildcache')
unsigned = not sign_binaries
tty.debug('Creating buildcache ({0})'.format(
'unsigned' if unsigned else 'signed'))
buildcache._createtarball(env, spec_yaml=yaml_path, add_deps=False,
output_location=mirror_url, force=True,
allow_root=True)
allow_root=True, unsigned=unsigned)
if build_id:
tty.debug('Writing cdashid ({0}) to remote mirror: {1}'.format(
build_id, mirror_url))

View File

@ -138,6 +138,7 @@ def ci_rebuild(args):
cdash_build_name = get_env_var('SPACK_CDASH_BUILD_NAME')
related_builds = get_env_var('SPACK_RELATED_BUILDS_CDASH')
pr_env_var = get_env_var('SPACK_IS_PR_PIPELINE')
pr_mirror_url = get_env_var('SPACK_PR_MIRROR_URL')
gitlab_ci = None
if 'gitlab-ci' in yaml_root:
@ -180,8 +181,6 @@ def ci_rebuild(args):
tty.debug('job_spec_pkg_name = {0}'.format(job_spec_pkg_name))
tty.debug('compiler_action = {0}'.format(compiler_action))
spack_cmd = exe.which('spack')
cdash_report_dir = os.path.join(ci_artifact_dir, 'cdash_report')
temp_dir = os.path.join(ci_artifact_dir, 'jobs_scratch_dir')
job_log_dir = os.path.join(temp_dir, 'logs')
@ -235,20 +234,17 @@ def ci_rebuild(args):
for next_entry in directory_list:
tty.debug(' {0}'.format(next_entry))
# Make a copy of the environment file, so we can overwrite the changed
# version in between the two invocations of "spack install"
env_src_path = env.manifest_path
env_dirname = os.path.dirname(env_src_path)
env_filename = os.path.basename(env_src_path)
env_copyname = '{0}_BACKUP'.format(env_filename)
env_dst_path = os.path.join(env_dirname, env_copyname)
shutil.copyfile(env_src_path, env_dst_path)
tty.debug('job concrete spec path: {0}'.format(job_spec_yaml_path))
if signing_key:
spack_ci.import_signing_key(signing_key)
can_sign = spack_ci.can_sign_binaries()
sign_binaries = can_sign and spack_is_pr_pipeline is False
can_verify = spack_ci.can_verify_binaries()
verify_binaries = can_verify and spack_is_pr_pipeline is False
spack_ci.configure_compilers(compiler_action)
spec_map = spack_ci.get_concrete_specs(
@ -273,27 +269,76 @@ def ci_rebuild(args):
with open(root_spec_yaml_path, 'w') as fd:
fd.write(spec_map['root'].to_yaml(hash=ht.build_hash))
if bindist.needs_rebuild(job_spec, remote_mirror_url, True):
# Binary on remote mirror is not up to date, we need to rebuild
# it.
#
# FIXME: ensure mirror precedence causes this local mirror to
# be chosen ahead of the remote one when installing deps
# TODO: Refactor the spack install command so it's easier to use from
# python modules. Currently we use "exe.which('spack')" to make it
# easier to install packages from here, but it introduces some
# problems, e.g. if we want the spack command to have access to the
# mirrors we're configuring, then we have to use the "spack" command
# to add the mirrors too, which in turn means that any code here *not*
# using the spack command does *not* have access to the mirrors.
spack_cmd = exe.which('spack')
mirrors_to_check = {
'ci_remote_mirror': remote_mirror_url,
}
def add_mirror(mirror_name, mirror_url):
m_args = ['mirror', 'add', mirror_name, mirror_url]
tty.debug('Adding mirror: spack {0}'.format(m_args))
mirror_add_output = spack_cmd(*m_args)
# Workaround: Adding the mirrors above, using "spack_cmd" makes
# sure they're available later when we use "spack_cmd" to install
# the package. But then we also need to add them to this dict
# below, so they're available in this process (we end up having to
# pass them to "bindist.get_mirrors_for_spec()")
mirrors_to_check[mirror_name] = mirror_url
tty.debug('spack mirror add output: {0}'.format(mirror_add_output))
# Configure mirrors
if pr_mirror_url:
add_mirror('ci_pr_mirror', pr_mirror_url)
if enable_artifacts_mirror:
mirror_add_output = spack_cmd(
'mirror', 'add', 'local_mirror', artifact_mirror_url)
tty.debug('spack mirror add:')
tty.debug(mirror_add_output)
add_mirror('ci_artifact_mirror', artifact_mirror_url)
mirror_list_output = spack_cmd('mirror', 'list')
tty.debug('listing spack mirrors:')
tty.debug(mirror_list_output)
spack_cmd('mirror', 'list')
spack_cmd('config', 'blame', 'mirrors')
# 2) build up install arguments
install_args = ['-d', '-v', '-k', 'install', '--keep-stage']
# Checks all mirrors for a built spec with a matching full hash
matches = bindist.get_mirrors_for_spec(
job_spec, force=False, full_hash_match=True,
mirrors_to_check=mirrors_to_check)
# 3) create/register a new build on CDash (if enabled)
cdash_args = []
if matches:
# Got at full hash match on at least one configured mirror. All
# matches represent the fully up-to-date spec, so should all be
# equivalent. If artifacts mirror is enabled, we just pick one
# of the matches and download the buildcache files from there to
# the artifacts, so they're available to be used by dependent
# jobs in subsequent stages.
tty.debug('No need to rebuild {0}'.format(job_spec_pkg_name))
if enable_artifacts_mirror:
matching_mirror = matches[0]['mirror_url']
tty.debug('Getting {0} buildcache from {1}'.format(
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)
else:
# No full hash match anywhere means we need to rebuild spec
# Build up common install arguments
install_args = [
'-d', '-v', '-k', 'install',
'--keep-stage',
'--require-full-hash-match',
]
if not verify_binaries:
install_args.append('--no-check-signature')
# Add arguments to create + register a new build on CDash (if
# enabled)
if enable_cdash:
tty.debug('Registering build with CDash')
(cdash_build_id,
@ -304,82 +349,63 @@ def ci_rebuild(args):
cdash_upload_url = '{0}/submit.php?project={1}'.format(
cdash_base_url, cdash_project_enc)
cdash_args = [
install_args.extend([
'--cdash-upload-url', cdash_upload_url,
'--cdash-build', cdash_build_name,
'--cdash-site', cdash_site,
'--cdash-buildstamp', cdash_build_stamp,
]
])
spec_cli_arg = [job_spec_yaml_path]
install_args.append(job_spec_yaml_path)
tty.debug('Installing package')
tty.debug('Installing {0} from source'.format(job_spec.name))
try:
# Two-pass install is intended to avoid spack trying to
# install from buildcache even though the locally computed
# full hash is different than the one stored in the spec.yaml
# file on the remote mirror.
first_pass_args = install_args + [
'--cache-only',
'--only',
'dependencies',
]
first_pass_args.extend(spec_cli_arg)
tty.debug('First pass install arguments: {0}'.format(
first_pass_args))
spack_cmd(*first_pass_args)
# Overwrite the changed environment file so it doesn't break
# the next install invocation.
tty.debug('Copying {0} to {1}'.format(
env_dst_path, env_src_path))
shutil.copyfile(env_dst_path, env_src_path)
second_pass_args = install_args + [
'--no-cache',
'--only',
'package',
]
second_pass_args.extend(cdash_args)
second_pass_args.extend(spec_cli_arg)
tty.debug('Second pass install arguments: {0}'.format(
second_pass_args))
spack_cmd(*second_pass_args)
except Exception as inst:
tty.error('Caught exception during install:')
tty.error(inst)
tty.debug('spack install arguments: {0}'.format(
install_args))
spack_cmd(*install_args)
finally:
spack_ci.copy_stage_logs_to_artifacts(job_spec, job_log_dir)
# 4) create buildcache on remote mirror, but not if this is
# running to test a spack PR
if not spack_is_pr_pipeline:
# Create buildcache on remote mirror, either on pr-specific
# mirror or on mirror defined in spack environment
if spack_is_pr_pipeline:
buildcache_mirror_url = pr_mirror_url
else:
buildcache_mirror_url = remote_mirror_url
try:
spack_ci.push_mirror_contents(
env, job_spec, job_spec_yaml_path, remote_mirror_url,
cdash_build_id)
env, job_spec, job_spec_yaml_path, buildcache_mirror_url,
cdash_build_id, sign_binaries)
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 mirror')
tty.msg(err_msg)
# 5) create another copy of that buildcache on "local artifact
# mirror" (only done if cash reporting is enabled)
# Create another copy of that buildcache on "local artifact
# mirror" (only done if artifacts buildcache is enabled)
spack_ci.push_mirror_contents(env, job_spec, job_spec_yaml_path,
artifact_mirror_url, cdash_build_id)
artifact_mirror_url, cdash_build_id,
sign_binaries)
# 6) relate this build to its dependencies on CDash (if enabled)
# Relate this build to its dependencies on CDash (if enabled)
if enable_cdash:
spack_ci.relate_cdash_builds(
spec_map, cdash_base_url, cdash_build_id, cdash_project,
artifact_mirror_url or remote_mirror_url)
else:
# There is nothing to do here unless "local artifact mirror" is
# enabled, in which case, we need to download the buildcache to
# the local artifacts directory to be used by dependent jobs in
# subsequent stages
tty.debug('No need to rebuild {0}'.format(job_spec_pkg_name))
if enable_artifacts_mirror:
tty.debug('Getting {0} buildcache'.format(job_spec_pkg_name))
tty.debug('Downloading to {0}'.format(build_cache_dir))
buildcache.download_buildcache_files(
job_spec, build_cache_dir, True, remote_mirror_url)
artifact_mirror_url or pr_mirror_url or remote_mirror_url)
def ci(parser, args):

View File

@ -22,6 +22,7 @@
from spack.main import SpackCommand
import spack.mirror
import spack.util.gpg
import spack.util.web as web_util
from spack.directory_layout import YamlDirectoryLayout
from spack.spec import Spec
@ -622,3 +623,51 @@ def test_spec_needs_rebuild(install_mockery_mutable_config, mock_packages,
rebuild = bindist.needs_rebuild(s, mirror_url, rebuild_on_errors=True)
assert rebuild
def test_generate_indices_key_error(monkeypatch, capfd):
def mock_list_url(url, recursive=False):
print('mocked list_url({0}, {1})'.format(url, recursive))
raise KeyError('Test KeyError handling')
monkeypatch.setattr(web_util, 'list_url', mock_list_url)
test_url = 'file:///fake/keys/dir'
# Make sure generate_key_index handles the KeyError
bindist.generate_key_index(test_url)
err = capfd.readouterr()[1]
assert 'Warning: No keys at {0}'.format(test_url) in err
# Make sure generate_package_index handles the KeyError
bindist.generate_package_index(test_url)
err = capfd.readouterr()[1]
assert 'Warning: No packages at {0}'.format(test_url) in err
def test_generate_indices_exception(monkeypatch, capfd):
def mock_list_url(url, recursive=False):
print('mocked list_url({0}, {1})'.format(url, recursive))
raise Exception('Test Exception handling')
monkeypatch.setattr(web_util, 'list_url', mock_list_url)
test_url = 'file:///fake/keys/dir'
# Make sure generate_key_index handles the Exception
bindist.generate_key_index(test_url)
err = capfd.readouterr()[1]
expect = 'Encountered problem listing keys at {0}'.format(test_url)
assert expect in err
# Make sure generate_package_index handles the Exception
bindist.generate_package_index(test_url)
err = capfd.readouterr()[1]
expect = 'Encountered problem listing packages at {0}'.format(test_url)
assert expect in err

View File

@ -733,9 +733,9 @@ def test_push_mirror_contents(tmpdir, mutable_mock_env_path, env_deactivate,
install_cmd('--keep-stage', yaml_path)
# env, spec, yaml_path, mirror_url, build_id
# env, spec, yaml_path, mirror_url, build_id, sign_binaries
ci.push_mirror_contents(
env, concrete_spec, yaml_path, mirror_url, '42')
env, concrete_spec, yaml_path, mirror_url, '42', True)
buildcache_path = os.path.join(mirror_dir.strpath, 'build_cache')

View File

@ -0,0 +1,24 @@
pr_pipeline:
only:
- /^github\/pr[\d]+_.*$/
variables:
SPACK_REF: ${CI_COMMIT_SHA}
SPACK_PR_BRANCH: ${CI_COMMIT_REF_NAME}
SPACK_IS_PR_PIPELINE: "True"
AWS_ACCESS_KEY_ID: ${PR_MIRRORS_AWS_ACCESS_KEY_ID}
AWS_SECRET_ACCESS_KEY: ${PR_MIRRORS_AWS_SECRET_ACCESS_KEY}
trigger:
project: spack/e4s
strategy: depend
develop_pipeline:
only:
- /^github\/develop$/
variables:
SPACK_REF: ${CI_COMMIT_SHA}
AWS_ACCESS_KEY_ID: ${AWS_ACCESS_KEY_ID}
AWS_SECRET_ACCESS_KEY: ${AWS_SECRET_ACCESS_KEY}
SPACK_SIGNING_KEY: ${SPACK_SIGNING_KEY}
trigger:
project: spack/e4s
strategy: depend

View File

@ -1,18 +0,0 @@
pr_pipeline:
only:
- external_pull_requests
variables:
SPACK_REF: ${CI_EXTERNAL_PULL_REQUEST_SOURCE_BRANCH_NAME}
SPACK_IS_PR_PIPELINE: "True"
trigger:
project: spack/e4s
strategy: depend
merge_pipeline:
only:
- develop
variables:
SPACK_REF: ${CI_COMMIT_SHA}
trigger:
project: spack/e4s
strategy: depend