Skip to content
Snippets Groups Projects
Commit 5d14ef05 authored by Jameson Graef Rollins's avatar Jameson Graef Rollins
Browse files

CI: add approval validation check for noise changing merge requests

This adds a CI special job that runs only on merge requests that update
the test git ref, indicating that a noise has been changed.  The MR will
show present a report of the changed noises, and will block on MR approval.
Once approval has been given, the check_approval job can be re-run, which
should now pass, allow the MR to proceed.

If the noises change but the reference has not been updated, then the CI
pipeline will fail as usual.
parent fe33fd1f
No related branches found
No related tags found
No related merge requests found
This commit is part of merge request !80. Comments created here will be created in the context of that merge request.
stages:
- dist
- test
- gen_cache
- update_cache
- review
- docs
- deploy
# have to specify this so that all jobs execute for all commits
# including merge requests
workflow:
rules:
- if: $CI_MERGE_REQUEST_ID
- if: $CI_COMMIT_BRANCH
variables:
GIT_STRATEGY: clone
# build the docker image we will use in all the jobs, with all
# dependencies pre-installed/configured.
gwinc/base:
......@@ -19,14 +28,16 @@ gwinc/base:
cat <<EOF > Dockerfile
FROM igwn/base:buster
RUN apt-get update -qq
RUN apt-get -y install --no-install-recommends git python3 python3-yaml python3-scipy python3-matplotlib python3-ipython lalsimulation-python3 python3-pypdf2 python3-h5py
RUN apt-get -y install --no-install-recommends git gitlab-cli python3 python3-yaml python3-scipy python3-matplotlib python3-ipython lalsimulation-python3 python3-pypdf2 python3-h5py
RUN git clone https://gitlab-ci-token:ci_token@git.ligo.org/gwinc/inspiral_range.git
EOF
- docker build -t $IMAGE_TAG .
- docker push $IMAGE_TAG
# run the tests and generate the test report on failure
test:
# validate that the noises haven't changed relative to the reference
# (the reference itself could have been updated, though, see
# check_approval job)
validate_noise:
stage: test
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
......@@ -37,13 +48,60 @@ test:
when: on_failure
paths:
- gwinc_test_report.pdf
expose_as: 'GWINC test failure report PDF'
expose_as: 'noise validation failure report'
# this is a special job intended to run only for merge requests where
# the test reference hash file has been updated, indicating that there
# has been a noise change. if the merge request has not yet been
# approved, generate a report of noise changes relative to the target
# branch and present that to the reviewers. if the merge request is
# approved, re-run this job, which will succeed if the MR is approved.
check_approval:
stage: review
rules:
# - if: '$CI_MERGE_REQUEST_TARGET_BRANCH_NAME == "master"'
- if: $CI_MERGE_REQUEST_ID
changes:
- gwinc/test/ref_hash
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
- echo "NOISE REFERENCE CHANGE, checking approval..."
- |
cat <<EOF > check_approved.py
import sys
import gitlab
project_id = sys.argv[1]
mr_iid = sys.argv[2]
# this only works for public repos, otherwise need to specify
# private_token=
gl = gitlab.Gitlab('https://git.ligo.org')
project = gl.projects.get(project_id)
mr = project.mergerequests.get(mr_iid)
approvals = mr.approvals.get()
print(approvals.approved)
EOF
- echo $CI_MERGE_REQUEST_PROJECT_ID, $CI_MERGE_REQUEST_IID, $CI_MERGE_REQUEST_TARGET_BRANCH_NAME,
- approved=$(python3 check_approved.py $CI_MERGE_REQUEST_PROJECT_ID $CI_MERGE_REQUEST_IID )
- if [[ $approved != True ]] ; then
- old_hash=$(git cat-file -p origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME:gwinc/test/ref_hash)
- if ! python3 -m gwinc.test --git-ref $old_hash -r gwinc_test_report.pdf ; then
- echo "APPROVAL REQUIRED TO MERGE THIS BRANCH."
- /bin/false
- else
- echo "Reference update did not cause appreciable noise change."
- fi
- else
- echo "Merge request approved, reference change accepted."
- fi
artifacts:
when: on_failure
paths:
- gwinc_test_report.pdf
expose_as: 'noise changes relative to target branch head APPROVAL REQUIRED TO MERGE'
# create plots for the canonical IFOs
ifo:
stage: docs
needs:
- test
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
- mkdir -p ifo
......@@ -63,8 +121,6 @@ html:
stage: docs
only:
- master
needs:
- test
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
- rm -rf public
......
......@@ -44,4 +44,23 @@ commit after the commits that change the noise calculations.*
Once you submit your merge request, an automated pipeline will run the
test script to validate your changes. If the tests fail but noise
changes are detected, then the merge request will be rejected.
changes are detected, then the merge request will be rejected. If
your merge request contains changes to the test reference an alert
will be generated in the merge request indicating that approval by
reviewers is required before merge.
## For admins: approving noise curve changes
As discussed above, merge requests that generate noise changes will
cause pipeline failures. If the MR properly includes a reference
update, then the failure will only be in waiting for proper approval
of the MR. A report will also be generated to compare all the noise
changes against the target branch (usually 'master'). See the 'View
exposed artifacts' menu item in the pipeline report. Once you have
reviewed the report and the code, and understand and accept the noise
changes, click the 'Approve' button in the MR. Once sufficient
approval has been given, re-run the `review:check_approval` job, which
should now pick up that approval has been given, and if everything
goes ok the pipeline will now succeed, allowing the code to be merged.
Click the 'Merge' button to finally merge the code.
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment