search

Ouranosinc / PAVICS-e2e-workflow-tests

Test user-level workflow.
Apache License 2.0
0 stars 2 forks source link

Jenkins: Allow full custom config override #135

Closed tlvu closed 5 months ago

tlvu commented 5 months ago

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 https://github.com/bird-house/birdhouse-deploy/pull/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.

  2. 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.

tlvu commented 5 months ago

FYI @mishaschwartz

tlvu commented 5 months ago

@fmigneault Oh, excellent question !

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.

  2. 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.

I will post your excellent question and this answer to the PR description as well so it is preserved in the commit on the merge.

tlvu commented 5 months ago

@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.

fmigneault commented 5 months ago

Ok. Seems reasonable.

Can you add a #!/bin/sh at the start of the example file?

And, should the curl command use the docker curl image as in the case of birdhouse-deploy?

tlvu commented 5 months ago

And, should the curl command use the docker curl image as in the case of birdhouse-deploy?

No need, it runs inside the gigantic Jupyter env image so curl already exists. For birdhouse-deploy, it was the minimalistic docker-compose image that was used, that's why curl was missing.