Project

General

Profile

Actions

action #130940

closed

coordination #58184: [saga][epic][use case] full version control awareness within openQA

coordination #48641: [epic] Trigger openQA tests in pull requests of any product github pull request

coordination #130850: [epic] Use openqa-clone-custom-git-refspec to parse github description+comments and trigger openQA tests as part of CI

Trigger openQA tests mentioned in github comments as part of automatic testing as well - trusted group "tests-maintainer" only size:M

Added by okurz over 1 year ago. Updated 7 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Feature requests
Target version:
Start date:
2023-06-15
Due date:
% Done:

0%

Estimated time:

Description

Motivation

Follow-up to #130934. After having openQA CI integration which reads github pull request description and triggers according openQA clone jobs github comments created or updated should be considered the same.

Acceptance criteria

  • AC1: openQA CI integration in any existing test distribution github project automatically runs openQA tests based on links to existing openQA jobs in the github comments
  • AC2: Ensure openQA documentation covers that also github comments are parsed and how to use that
  • AC3: An update to the PR code does not retrigger any openQA jobs mentioned in comments
  • AC4: Comments created/edited by anyone not in the group "tests-maintainer" are ignored

Suggestions

  • Wait for #130934
  • Read what was done originally in #63712 and the according pull request to openQA https://github.com/os-autoinst/openQA/pull/2618 and also #130934
  • Check if openqa-clone-custom-git-refspec with special comments also covers github comments or only the initial description
  • Extend the existing approach where necessary
  • Ensure that the same process is triggered also on comment updates
  • Only look at comments created/edited by members of the group "tests-maintainer" for security reasons
Actions #1

Updated by okurz over 1 year ago

  • Target version changed from Ready to future
Actions #2

Updated by okurz 10 months ago

  • Target version changed from future to Tools - Next
Actions #3

Updated by okurz 9 months ago

  • Target version changed from Tools - Next to Ready
Actions #4

Updated by okurz 9 months ago

  • Subject changed from Trigger openQA tests mentioned in github comments as part of CI as well to Trigger openQA tests mentioned in github comments as part of automatic testing as well
  • Description updated (diff)
  • Assignee set to okurz

we couldn't agree on this yet during estimation and I will follow-up with the team in more detail about the general idea if it makes sense like that or not at all.

Actions #5

Updated by okurz 9 months ago

  • Target version changed from Ready to Tools - Next
Actions #6

Updated by okurz 8 months ago

  • Subject changed from Trigger openQA tests mentioned in github comments as part of automatic testing as well to Trigger openQA tests mentioned in github comments as part of automatic testing as well - trusted group "tests-maintainer" only
  • Description updated (diff)
  • Assignee deleted (okurz)
  • Target version changed from Tools - Next to Ready
Actions #7

Updated by jbaier_cz 8 months ago

  • Subject changed from Trigger openQA tests mentioned in github comments as part of automatic testing as well - trusted group "tests-maintainer" only to Trigger openQA tests mentioned in github comments as part of automatic testing as well - trusted group "tests-maintainer" only size:M
Actions #8

Updated by jbaier_cz 8 months ago

  • Status changed from New to Workable
Actions #9

Updated by mkittler 7 months ago

  • Status changed from Workable to In Progress
  • Assignee set to mkittler
Actions #10

Updated by openqa_review 7 months ago

  • Due date set to 2024-05-29

Setting due date based on mean cycle time of SUSE QE Tools

Actions #11

Updated by mkittler 7 months ago

I tried creating a workflow that triggers when comments are created. The triggering generally works but there are a few problems:

  1. The workflow isn't shown as part of the related PR (e.g. https://github.com/Martchus/os-autoinst-distri-opensuse/pull/3) but only under the "Actions" tab (e.g. https://github.com/Martchus/os-autoinst-distri-opensuse/actions). So we probably need to use the statuses API to get some in-progress/result checkmarks within the PR.
  2. The event meta-data doesn't contain the PRs ref so we need to query that from our action.

Those are the changes I have so far for our GitHub action: https://github.com/os-autoinst/scripts/compare/master...Martchus:scripts:clone-on-comment

Actions #12

Updated by mkittler 7 months ago

  • Status changed from In Progress to Feedback

PR for the scripts repo covering 1 and 2: https://github.com/os-autoinst/scripts/pull/318

Actions #13

Updated by mkittler 7 months ago

The scripts PR was merged so now the next step would be to merge https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19305.

The workflow is not triggered before merging the changes so I guess we cannot easily test this on the main repo before merging. But it worked when I tested it on my fork (where I added the changes already on the master branch). In case the token GitHub provides to workflows by default is not sufficient I will have to create a new token just for that - but it won't be a big deal as I have already tested this on my fork.

Actions #14

Updated by mkittler 7 months ago

The PR was merged as well and I created https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19371 to supply a token with enough permissions. I hope that'll be the last step.

Actions #15

Updated by mkittler 7 months ago

Looks like it generally works now, e.g. jobs were cloned for https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19305#issuecomment-2127177246.

It failed to update the status and to clone the ref:

Unable to update status on GitHub via 'https://api.github.com/repos/Martchus/os-autoinst-distri-opensuse/statuses/acf5d851c6a6eead3605a324a14f3651117d54b0'.
{"message":"Not Found","documentation_url":"https://docs.github.com/rest/commits/statuses#create-a-commit-status"}
[2024-05-23T15:45:30.188302Z] [warn] [pid:74634] !!! main: Unable to clone Git repository 'https://github.com/https://github.com/Martchus/os-autoinst-distri-opensuse.git#acf5d851c6a6eead3605a324a14f3651117d54b0' specified via CASEDIR (see log for details) at /usr/lib/os-autoinst/OpenQA/Isotovideo/Utils.pm line 164.

Maybe the URL for updating the status needed to point to the repository under the original organization (os-autoinst) and not the fork (Martchus). And the URL of CASEDIR is just wrong in a very obvious way.


But there's another caveat: This will now spawn a GitHub action on every new/updated comment in PRs. This means setting up a container and doing some initial checks - even if no job is to be cloned. That's maybe a bit wasteful. Not sure whether this can be tweaked via the if:-condition in the workflow YAML.

Actions #16

Updated by mkittler 7 months ago

  • Status changed from Feedback to In Progress
Actions #17

Updated by livdywan 7 months ago

mkittler wrote in #note-15:

Looks like it generally works now, e.g. jobs were cloned for https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19305#issuecomment-2127177246.

Really cool. One thing I noticed, there is no way to see the PR from the job itself or at least not easily.
This is not in the AC so just a suggestion, but it would be nice to see the PR in the settings for example.

But there's another caveat: This will now spawn a GitHub action on every new/updated comment in PRs. This means setting up a container and doing some initial checks - even if no job is to be cloned. That's maybe a bit wasteful. Not sure whether this can be tweaked via the if:-condition in the workflow YAML.

Maybe https://github.com/orgs/community/discussions/25389 (although I'd say it is fine as-is, too, if this is not straightforward to do)

Actions #18

Updated by mkittler 7 months ago

I was actually able to find the contains()-syntax myself (in the official documentation). It works so I created https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19379 to use it.

Actions #19

Updated by mkittler 7 months ago

  • Status changed from In Progress to Feedback

Really cool. One thing I noticed, there is no way to see the PR from the job itself or at least not easily.

I added another commit to https://github.com/os-autoinst/scripts/pull/324 for that. It will add a job setting pointing to the specific PR comment the job was cloned by. I also created https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19380 to add a back reference to the relevant PR for jobs cloned via PR descriptions.

Actions #20

Updated by mkittler 7 months ago ยท Edited

All PRs merged, looks good so far: https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19305#issuecomment-2133002967


EDIT: Unfortunately reporting the status back didn't work when I tested it on the open PR https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19378#issuecomment-2133222287.

The log contains the following error:

Unable to update status on GitHub via 'https://api.github.com/repos/os-autoinst/os-autoinst-distri-opensuse/statuses/32731e00569b74a04f4269c101da9d181cef8513'.
{"message":"Not Found","documentation_url":"https://docs.github.com/rest/commits/statuses#create-a-commit-status"}

I'm not sure why that's the case. The URL looks correct and the same worked on my fork.

Actions #21

Updated by mkittler 7 months ago

The long build numbers are disturbing the dashboard. I think this should be handled better in general by the dashboard so I created https://github.com/os-autoinst/openQA/pull/5663 to limit the size of build labels on the dashboard.

Actions #22

Updated by okurz 7 months ago

mkittler wrote in #note-21:

The long build numbers are disturbing the dashboard. I think this should be handled better in general by the dashboard so I created https://github.com/os-autoinst/openQA/pull/5663 to limit the size of build labels on the dashboard.

That only happens though if builds are created as part of job groups which shouldn't happen for those jobs triggered as part of verification jobs. Maybe you just need to make those jobs groupless?

Actions #23

Updated by okurz 7 months ago

  • Due date changed from 2024-05-29 to 2024-06-05
Actions #24

Updated by mkittler 7 months ago

The jobs are intentionally in a specific job group.


Showing the status also works now after making the bot account an owner within the os-autoinst organization, see the green checkmark on https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19378. The back-reference also works, see CLONED_BY on https://openqa.opensuse.org/tests/4243617#settings.


PRs for documentation:

Actions #25

Updated by mkittler 7 months ago

  • Status changed from Feedback to Resolved

All PRs have been merged. So this feature is now documented. Maybe it makes sense to mention it on the next tools team workshop so people are aware it actually exists.

Actions #26

Updated by okurz 7 months ago

  • Due date deleted (2024-06-05)
Actions

Also available in: Atom PDF