Skip to content

Commit

Permalink
Jenkins: Allow full custom config override (#135)
Browse files Browse the repository at this point in the history
# Overview

Add a new text box `CONFIG_OVERRIDE_SCRIPT_URL` so we can specify a URL
to a script that can be sourced to alter any configs before starting the
test run.

A sample script is provided to filter the notebooks for testing
GeoServer only. This greatly improve the turn over time by avoiding to
run unrelated notebooks. Was useful to test this PR
bird-house/birdhouse-deploy#348.

This custom script can be coming from any URL and change other configs
than just the list of notebooks to run.

Default is no custom script URL, same as the current behavior.

@fmigneault excellent question:

I'm curious to better understand the use case to see if it is the right
(and only) solution.

The current (CRIM) birdhouse-deploy CI sources the `env.local` file,
starts a test instance with it, and then forwards the env values to the
children Jenkins job for E2E workflow tests. In other words, if a
variable needs to be overridden for tests, it can be set directly in it,
or a custom component can be defined and listed at the end of
`EXTRA_CONF_DIRS`. There is also a `EXTRA_TEST_ENV_VAR` variable for yet
even more additional variables to pass between the test instance
creation and the E2E workflow tests. This is notably useful for manual
test trigger or quick tweaks against some existing branch/config
combination.

Is there no way to use those approaches or accomplish similar behavior
in case another CI (or local test) are used? More specifically, is there
a need for yet another method to override variables?

@tlvu answer:

2 reasons:

1. The existing way via `env.local` only works for CRIM pipeline where
each PAVICS deployment is meant to run only one test run, then the
deployment is discarded.

My test servers are meant for multiple tests configs so I'd rather not
have to change my `env.local` each time and having to restart the stack,
wait for all the compoments to be ready, then launch Jenkins, it's too
slow.

Furthermore, if I want to test against the production instance, I won't
be able to touch the `env.local`.

1. The existing way can only impact "input" configs, not the "resulting"
configs.

The list of notebooks to run is not part of the input, it is calculated
so there is no other way to modify it than to intercept it the way I
did.

I had thought of exposing a big text box to manually specify the list of
notebooks but that is way too cumbersome. Filtering it programmatically
is way more flexible and repeatable across multiples runs.

Talking about flexibility, this "hook" can do way more than just
filtering the list of notebooks. Any extra oneoff pre-processing steps
for some edge-case scenario just before launching `py.test` can be done
there. Those are edge-cases processing so can not be committed as
"regular" steps.

@fmigneault Just to be clear, this new way to customize the config is
not meant to replace the existing way. It just gives us new
possibilities not possible before. It is 100% backward-compatible with
the existing way so there are no changes required on CRIM pipeline side.
If whatever you are doing already works, keep it, no need to change
anything.
  • Loading branch information
tlvu authored Apr 15, 2024
2 parents a4592ea + f1cfda9 commit 81987bf
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 0 deletions.
2 changes: 2 additions & 0 deletions Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ Requires 'weaver' component to be active on the target 'PAVICS_HOST' server
description: 'Extra options to pass to pytest, ex: --nbval-lax --dist=loadscope --numprocesses=0', trim: true)
string(name: 'EXTRA_TEST_ENV_VAR', defaultValue: '',
description: 'Extra environment variables for the various tests, ex: "TEST_RUNS=50 TEST_WPS_BIRDS=finch,raven,flyingpigeon TEST_NO_USE_PROD_DATA=1"', trim: true)
string(name: 'CONFIG_OVERRIDE_SCRIPT_URL', defaultValue: '',
description: 'Url to a script that will be sourced, allowing to programmatically override ALL configs. Ex: https://raw.githubusercontent.com/Ouranosinc/PAVICS-e2e-workflow-tests/master/test-override/geoserver-nb-only.include.sh', trim: true)
booleanParam(name: 'TEST_LOCAL_NOTEBOOKS', defaultValue: true,
description: 'Check the box to test notebooks in this repo (./notebooks/*.ipynb).')
booleanParam(name: 'VERIFY_SSL', defaultValue: true,
Expand Down
12 changes: 12 additions & 0 deletions runtest
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ fi
# so tests still pass when user want to disable ssl cert verification
export PYTHONWARNINGS="ignore:Unverified HTTPS request"

# Allow full override of ALL configs before running test suite.
if [ -n "$CONFIG_OVERRIDE_SCRIPT_URL" ]; then
TMP_CONF_OVERRIDE="/tmp/conf_override"
rm -vf "$TMP_CONF_OVERRIDE"

# Use tee to log content of override script for traceability.
curl --silent "$CONFIG_OVERRIDE_SCRIPT_URL" | tee "$TMP_CONF_OVERRIDE"

# Source script so it can alter ALL existing variables in the current context.
. "$TMP_CONF_OVERRIDE"
fi

py.test --nbval $NOTEBOOKS --sanitize-with notebooks/output-sanitize.cfg $PYTEST_EXTRA_OPTS
EXIT_CODE="$?"

Expand Down
16 changes: 16 additions & 0 deletions test-override/geoserver-nb-only.include.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/bin/sh
# Sample config override script to only run notebooks that hits GeoServer.

# Have to be used together with TEST_NO_USE_PROD_DATA=1.

NEW_NB_LIST=""
for nb in $NOTEBOOKS; do
if echo "$nb" | grep -i raven && grep -i -e 'import birdy' -e 'from birdy' $nb; then
# Any Raven nb which import birdy will hit GeoServer.
NEW_NB_LIST="$NEW_NB_LIST $nb"
elif grep -i geoserver $nb; then
# Any mention of GeoServer, 'geoserver/wfs'.
NEW_NB_LIST="$NEW_NB_LIST $nb"
fi
done
NOTEBOOKS="$NEW_NB_LIST"

0 comments on commit 81987bf

Please sign in to comment.