Add verification run scheduler #5647

ByteOtter · 2024-05-17T14:12:06Z

What?

Add a script which wraps the standard use cases for openqa-clone-custom-git-refspec into a more usable form. Instead of requiring the user to type out the URLs of the openqa.opensuse.org test they want to use it injects all information - like the PR number, base test ID, test modules to load, etc. - into a openqa-clone-custom-git-refspec call.

This script itself does not implement new functionality or behaviour, but is simply a different "front-end" - with a more descriptive name - to use for quickly scheduling verification runs for PRs against e.g os-autoinst.

This is just a thought experiment and could theoretically be integrated into other components without the need for a separate script.

How?

Several parameters are mandatory for the script to function:

$SOURCE - Which specifies whether to use openqa.opensuse.org or openqa.suse.de (with custom targets potentially supported via config file)
$PR_ID - The ID the GitHub PR
$BASE_TEST_ID - The ID of the test the user wants to use as a base
$[test/module,...] - A comma sperated list of test modules to load
$TEST_NAME - The TEST parameter for the call

The GitHub access token is provided via a config file at ~/.config/gh.conf (name temporary).

Why?

This PR aims to lower the barrier of entry for new contributors and make general test writing easier by having a easy-to-use, descriptively named script a user can call to provide a run for their PR. Instead of going around trying to find the right command.

foursixnine · 2024-05-17T14:24:34Z

Following the discussion slack:

This could be integrated inside clone-custom-git-refspec directly. That’s more due to how we already work, and I fear introducing a new script might not be 100% the best idea, but its a start already
The handling of github tokens is a great adition!
on the spec file, perhaps once we figure out what to do, it could be aliased to openqa-verify-pr or a nice short name :D (because openqa-clone-custom-git-refspec is... well, a bit long

kalikiana · 2024-05-21T11:30:08Z

script/openqa-verify_pr

+elif [ "$source" = "suse" ]; then
+  echo "Dispatching verification run for suse..."
+  openqa-clonse-custom-git-refspec "https://github.com/os-autoinst-distri-opensuse/pull/$pr_id" \
+    "https://openqa.suse.de/tests/$test_id" \


I wonder, are you aware of the existing support for job URL's within the PR description? See https://github.com/os-autoinst/openQA/blob/master/script/openqa-clone-custom-git-refspec#L52

I actually wasn't aware of this, thanks for making me aware. That'd make integrating parts of the ideas into openqa-clone-custom-git-refspec even more alluring to take advantage of this feature. @foursixnine, what do you think?

I'd like to mention that we came to the conclusion that it is better to avoid the @-syntax as one probably don't want to mention the @openqa organization on GitHub here. Of course this makes the mentioned feature of the refspec script less interesting - unless one would change it.

However, I suggest to use https://github.com/os-autoinst/scripts/blob/master/openqa-clone-and-monitor-job-from-pr instead (which is already setup for the openSUSE test distribution and uses the openqa: Clone … syntax). It will soon also support creating clones by creating/editing comments but it is so far limited to the PR description.

(You can also read https://open.qa/docs/#_create_and_monitor_openqa_jobs_from_within_the_ci_runner for more information on the broader topic.)

I actually wasn't aware of this, thanks for making me aware. That'd make integrating parts of the ideas into openqa-clone-custom-git-refspec even more alluring to take advantage of this feature. @foursixnine, what do you think?

Yep its the first point here: #5647 (comment)

I wonder, are you aware of the existing support for job URL's within the PR description?

the extract URLs from description sounds nice, but only useful if it would edit the description and add the badges; I'd say lets keep that out of scope :)

support custom domains of openqa instances. Configurable in the config file.

Martchus · 2024-05-21T16:09:41Z

Please capitalize the first letter of the commit message subject lines. It would also be much nicer if openqa-verify_pr was openqa-verify-pr. The formatting of the help text and general coding style is unfortunately not in-line with what we generally use in our scripts. I would at least add a set -e or perhaps even set -eux -o pipefail at the beginning.

Note that additional scripts could also be added to https://github.com/os-autoinst/scripts - although if it is supposed to be distributed via an RPM package keeping it here might be the better option.

ByteOtter · 2024-05-22T08:03:12Z

Thanks for your feedback, I have changed the name and added the set -o pipefail line. Could you elaborate a bit on what you mean with "general coding style"? What do I need to do differently?
Thanks :)

Martchus

Generally we use a more compact coding style. I gave a few concrete suggestions in inline-comments.

script/openqa-verify-pr

script/openqa-clone-custom-git-refspec

Co-authored-by: Santiago Zarate <[email protected]>

foursixnine · 2024-05-29T08:11:17Z

script/openqa-verify-pr

+  echo "Dispatching verification run for opensuse..."
+  openqa-clone-custom-git-refspec "https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/$pr_id" \
+    "https://openqa.opensuse.org/tests/$test_id" \
+    SCHEDULE="tests/installation/bootloader_start,tests/boot/boot_to_desktop,$modules" \


Remember to move this to the openqa-clone-custom-git-refspec behind a switch i.e --modules a,b,c so it gets appended via the for loop, before calling cone job.

and then there's the consideration that if the caller is passing:

YAML_SCHEDULE + SCHEDULE, die (as one of them has to be overwritten.

if the switch is passed, and so is either (yaml_schedule or schedule) die, as options aren't compatible

Yep, currently looking into that :)

foursixnine · 2024-05-29T09:59:14Z

script/openqa-clone-custom-git-refspec

@@ -157,6 +182,12 @@ Please try 'curl $json_url' or select another job, e.g. in the same scenario: $h
    local scriptdir
    scriptdir=$(dirname "${BASH_SOURCE[0]}")
    local cmd="$dry_run $scriptdir/openqa-clone-job $clone_args \"$host\" \"$job\" _GROUP=\"$GROUP\" TEST+=\"$test_suffix\" BUILD=\"$build\" CASEDIR=\"$casedir\" PRODUCTDIR=\"$productdir\" NEEDLES_DIR=\"$needles_dir\""
+    if [[ $cmd =~ (SCHEDULE|YAML_SCHEDULE) ]]; then


is this an and? or an or?

it's an or, my bad. Though changing it to

if [[ $cmd =~ SCHEDULE ]] && [[ $cmd =~ YAML_SCHEDULE ]]; then ...

doesnt seem to change anythin rn so my problem may be that I am using cmd?

okurz

As suggested in chat originally I appreciate that you try to bring in new ideas with pull requests but we should be careful about introducing new scripts. And the same probably goes for the modules command line parameter which is redundant considering the support for generic openQA test variables. Before investing more effort into that I recommend you look into this variables concept in general. Maybe for now you want to focus only on the aspect of reading the github token from a config file which seems independent of the other changes

script/openqa-clone-custom-git-refspec

mergify · 2024-06-10T12:25:54Z

This pull request is now in conflicts. Could you fix it? 🙏

foursixnine self-assigned this May 17, 2024

foursixnine self-requested a review May 17, 2024 14:46

foursixnine assigned ByteOtter May 17, 2024

add scheduler script

23b2be0

ByteOtter force-pushed the add-verification-scheduler branch from 4d0130e to 23b2be0 Compare May 21, 2024 09:08

ByteOtter added 2 commits May 21, 2024 11:18

update script naming to fit convention

42fb4e7

fix help text

33a384b

kalikiana reviewed May 21, 2024

View reviewed changes

support custom target domains

6c9dc7e

support custom domains of openqa instances. Configurable in the config file.

Fix style of help text

c7f23ee

Martchus reviewed May 22, 2024

View reviewed changes

ByteOtter added 2 commits May 22, 2024 14:31

Apply coding style feedback

6bd62d6

Fix text formatting

b7737ac

ByteOtter force-pushed the add-verification-scheduler branch from 01deb94 to b7737ac Compare May 22, 2024 12:38

Remove unnecessary quotations

dbac7fb

ByteOtter force-pushed the add-verification-scheduler branch from d26812f to dbac7fb Compare May 23, 2024 12:39

Merge token retrieval into clone-custom-git-refspec script

b113b10

foursixnine reviewed May 29, 2024

View reviewed changes

script/openqa-clone-custom-git-refspec Outdated Show resolved Hide resolved

foursixnine reviewed May 29, 2024

View reviewed changes

script/openqa-clone-custom-git-refspec Outdated Show resolved Hide resolved

Change name of configuration file

f8fc47b

Co-authored-by: Santiago Zarate <[email protected]>

foursixnine reviewed May 29, 2024

View reviewed changes

ByteOtter added 2 commits May 29, 2024 11:47

Implement --modules flag

d103077

Fix typo in example

95c234e

foursixnine reviewed May 29, 2024

View reviewed changes

Move token collection after function declaration

1c3a1e2

okurz requested changes May 29, 2024

View reviewed changes

script/openqa-clone-custom-git-refspec Show resolved Hide resolved

Update scripts (tmp)

f1ab36f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add verification run scheduler #5647

Add verification run scheduler #5647

ByteOtter commented May 17, 2024

foursixnine commented May 17, 2024

kalikiana May 21, 2024

ByteOtter May 21, 2024

Martchus May 21, 2024

foursixnine May 21, 2024 •

edited

Loading

Martchus commented May 21, 2024

ByteOtter commented May 22, 2024

Martchus left a comment

foursixnine May 29, 2024

ByteOtter May 29, 2024

foursixnine May 29, 2024

ByteOtter May 29, 2024

okurz left a comment

mergify bot commented Jun 10, 2024

Add verification run scheduler #5647

Are you sure you want to change the base?

Add verification run scheduler #5647

Conversation

ByteOtter commented May 17, 2024

What?

How?

Why?

foursixnine commented May 17, 2024

kalikiana May 21, 2024

Choose a reason for hiding this comment

ByteOtter May 21, 2024

Choose a reason for hiding this comment

Martchus May 21, 2024

Choose a reason for hiding this comment

foursixnine May 21, 2024 • edited Loading

Choose a reason for hiding this comment

Martchus commented May 21, 2024

ByteOtter commented May 22, 2024

Martchus left a comment

Choose a reason for hiding this comment

foursixnine May 29, 2024

Choose a reason for hiding this comment

ByteOtter May 29, 2024

Choose a reason for hiding this comment

foursixnine May 29, 2024

Choose a reason for hiding this comment

ByteOtter May 29, 2024

Choose a reason for hiding this comment

okurz left a comment

Choose a reason for hiding this comment

mergify bot commented Jun 10, 2024

foursixnine May 21, 2024 •

edited

Loading