Skip to content
Snippets Groups Projects
Commit 33204de4 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 d7322e67
No related branches found
No related tags found
1 merge request!82MR approval check for noise changes
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
......
......@@ -42,7 +42,25 @@ commit after the commits that change the noise calculations* (git
commits can't have foreknowledge of their own commit hash).
Once you submit your merge request, an automated pipeline will run the
test command to validate your changes. If budgets differences are
detected the pipeline will fail, alerting reviewers that a more
detailed review is required. Test issues will need to be resolved
before code can be merged.
test command to validate your changes. If there are budgets
differences but the reference has not been updated, the pipeline will
fail and indicate that a reference update is required. Updates to the
reference will also cause a validation pipeline failure, but these
failures can be resolved through reviewer approval of the changes (see
below).
## 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 should only be in the approval check. A
report will be generated comparing all 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` pipeline job, which should now pick
up that approval has been given and allow the pipeline to succeed.
Once the pipeline succeeds the merge can be enacted. 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