action #138416
closedopenQA Tests - coordination #96596: [qe-core][CI] CI/CD and Coding style improvements
Unify GitHub Actions for QA Projects size:M
0%
Description
Motivation¶
There are repeated actions across GitHub repositories.
https://github.com/search?q=org%3Aos-autoinst+gsactions%2Fcommit-message-checker&type=code
os-autoinst-common should be the base for all checks and they must be inherited in all repos.
The gitsubrepo flow is materializing the file tree in each repo already. It should be as trivial as symlinking the workflows from os-autoinst-common to each project.
Acceptance criteria¶
- AC1: At least https://github.com/os-autoinst/os-autoinst-distri-example inherits static checks from os-autoinst-common
- AC2: Checks in openQA and os-autoinst still run successfully on the master branch
Suggestions¶
- Follow the existing pull request
- Ensure that os-autoinst as an example works with the updated os-autoinst-common changes
- Focus on only the parts that are really common with most if not all inheriting projects, so e.g. excluding updating dependencies.yaml
- Begin with https://github.com/os-autoinst/os-autoinst-distri-example and then test the rest.
Out of scope¶
- All checks in all os-autoinst repos, see #155065
Files
Updated by tinita about 1 year ago
I think we tried symlinks in the past for that and found out that github actions do not support them :-/
Do you have a working setup with symlinks?
Updated by okurz about 1 year ago
- Category set to Feature requests
- Target version set to future
Updated by josegomezr about 1 year ago
I haven't tried symlinks. But GitHub docs offers "Reusable Workflows" 0
I'll set up a simple check on os-autoinst-commons (the commit message one) with what they say in the docs and try to "inherit it" on os-autoinst.
Updated by josegomezr about 1 year ago
Here's a working minimal example of a reusable workflow:
https://github.com/os-autoinst/os-autoinst-common/pull/28
The limitation is that reusable workflows must be in: /.github/workflows/, no nesting is allowed as per docs.
Other repositories can make references to the reusable workflow as long as they have access to.
Updated by josegomezr about 1 year ago
Steps forward:
- [x] Cleanup the Reusable workflow PR 0 to have only the commit messages check.
- [x] Have it merged.
[] Substitute checks in:
with the reusable checks from os-autoinst-common.
Updated by szarate about 1 year ago
- Tags set to platform-team
- Parent task set to #96596
Updated by szarate about 1 year ago
- Subject changed from Unify GitHub Actions for all QA Projects to [qe-core] Unify GitHub Actions for all QA Projects
- Status changed from New to In Progress
- Assignee set to josegomezr
- Target version changed from future to QE-Core: Ready
Updated by okurz about 1 year ago
If you like you can also create PRs to use the inherited workflow in
- https://github.com/os-autoinst/os-autoinst-distri-openQA/blob/master/.github/workflows/commit_message_checker.yml
- https://github.com/os-autoinst/openqa-logwarn/blob/master/.github/workflows/commit_message_checker.yml
but somebody else can also do that.
Updated by tinita about 1 year ago
We are now getting an error in our nightly dependencies update job:
https://app.circleci.com/pipelines/github/os-autoinst/openQA/12400/workflows/7bc41274-bbd1-4145-9a0f-0715f4e9e37f/jobs/115757
[dependency_2023-10-27 d20ce8677] Dependency cron 2023-10-27
2 files changed, 2 insertions(+), 2 deletions(-)
To https://github.com/os-autoinst-bot/openQA.git
! [remote rejected] dependency_2023-10-27 -> dependency_2023-10-27 (refusing to allow a Personal Access Token to create or update workflow `.github/workflows/commit-message-checker.yml` without `workflow` scope)
error: failed to push some refs to 'https://github.com/os-autoinst-bot/openQA.git'
Exited with code exit status 1
Updated by tinita about 1 year ago
I looked up the token for the os-autoinst-bot and it does already have the "workflow" scope. So maybe the way the reusable workflow is using the token is wrong...
Updated by josegomezr about 1 year ago
tinita wrote in #note-9:
https://github.com/os-autoinst/openqa-logwarn/pull/48
https://github.com/os-autoinst/os-autoinst-distri-openQA/pull/152
Thanks for taking care @tinita!
tinita wrote in #note-10:
We are now getting an error in our nightly dependencies update job:
https://app.circleci.com/pipelines/github/os-autoinst/openQA/12400/workflows/7bc41274-bbd1-4145-9a0f-0715f4e9e37f/jobs/115757[dependency_2023-10-27 d20ce8677] Dependency cron 2023-10-27 2 files changed, 2 insertions(+), 2 deletions(-) To https://github.com/os-autoinst-bot/openQA.git ! [remote rejected] dependency_2023-10-27 -> dependency_2023-10-27 (refusing to allow a Personal Access Token to create or update workflow `.github/workflows/commit-message-checker.yml` without `workflow` scope) error: failed to push some refs to 'https://github.com/os-autoinst-bot/openQA.git' Exited with code exit status 1
I think it prohibited for automation to create .github workflow files, I'll investigate.
Updated by tinita about 1 year ago
- Copied to action #138674: Reusable github workflow in openQA is causing a problem for os-autoinst-bot dependency PR added
Updated by tinita about 1 year ago
I was looking at the wrong token. I now added the "workflow scope" to the right one and reran the job. All fine now :)
Updated by okurz 12 months ago
- Related to coordination #58184: [saga][epic][use case] full version control awareness within openQA added
Updated by okurz 12 months ago
- Related to action #115013: os-autoinst-plugin as a wheel helper tool added
Updated by okurz 12 months ago
- Related to coordination #117097: Evaluate GitHub template repositories for wheels added
Updated by tinita 12 months ago
- Related to action #151219: [bug][os-autoinst-distri-opensuse] No .perltidyrc added
Updated by openqa_review 12 months ago
- Due date set to 2023-12-07
Setting due date based on mean cycle time of SUSE QE Tools
Updated by josegomezr 12 months ago
Following through I've pulled the subrepo commons into a personal fork of os-autoinst-distri-example
It raises a couple of warnings about missing strict/warnings:
And the base pipeline seems not to be fully working, isotovideo tests are all red.
I believe is in a good-enough shape to be adopted and start enforcing the style in the downstream repositories.
However there's a couple of small discussions:
- does testapi bring warnings/strict? (answer seems to be no).
- if it's true, this module can be configured in the settings of the linter so it doesn't raise missing strict/warnings.
- if it's false: adding the missing lines is trivial.
- Can we bump the minimum perl version? there seems to be a Jira PED for this.
Updated by livdywan 11 months ago
- Target version changed from Ready to future
The ticket doesn't look to me like it's actively being worked on or would be done this week. Since someone outside the Tools team is assigned to it, though, we should not have it on the backlog as we can't require others to meet our expectations.
Updated by szarate 11 months ago
- Target version changed from future to QE-Core: Ready
livdywan wrote in #note-24:
The ticket doesn't look to me like it's actively being worked on or would be done this week. Since someone outside the Tools team is assigned to it, though, we should not have it on the backlog as we can't require others to meet our expectations.
This ticket was never meant to be in the tools team's queue.
Updated by josegomezr 11 months ago
livdywan wrote in #note-24:
The ticket doesn't look to me like it's actively being worked on or would be done this week. Since someone outside the Tools team is assigned to it, though, we should not have it on the backlog as we can't require others to meet our expectations.
Is not clear to me where to drive the discussion. Here there are unanswered questions and in the PR's there are also unanswered questions. Please advise for an appropriate place and I'll follow through.
As per feedback from @okurz, I've splitted os-autoinst/os-autoinst-commons#30
in two isolated PR's #31 & #32. Hopefully that can make reviews easier.
Additionally, the example distribution also gets some love for the simplification area:
- os-autoinst/os-autoinst-distri-example#28 makes the workflow definition easier to read and brings pipeline artifacts for later inspection.
Updated by josegomezr 11 months ago ยท Edited
Following the discussion on standup today:
- I'll add a PR in os-autoinst to either:
- provide the suggested
scripts/validate-test-results
from os-autoinst/os-autoinst-distri-example#28 - allow
isotoviodeo
to reflect the test result in the exit code. Something along the lines ofisotovideo [...] --exit-status-from-error
.
- provide the suggested
whatever of the two is less effort.
Updated by josegomezr 11 months ago
Updated by okurz 10 months ago
- Subject changed from [qe-core] Unify GitHub Actions for QA Projects size:M to Unify GitHub Actions for QA Projects size:M
- Due date deleted (
2023-12-14) - Status changed from In Progress to Workable
- Assignee deleted (
josegomezr) - Target version changed from QE-Core: Ready to Ready
Next steps:
- Rebase https://github.com/os-autoinst/os-autoinst-common/pull/30
- See what is missing
- Check for other open related PRs
Updated by ybonatakis 10 months ago
- Status changed from Workable to In Progress
- Assignee set to ybonatakis
Updated by ybonatakis 10 months ago
I am a bit confused with it and looks more complicated than I initially thought.
I started with what looks to work immediately, starting with os-autoinst
[0] replace the .perlcriticrc
and .yamllint
diff of .perlcriticrc[1] is significant but seems to work
[0] https://github.com/os-autoinst/os-autoinst/pull/2451
[1] https://gist.github.com/b10n1k/5825121257899cb3f85cfd0463266dae
Updated by openqa_review 10 months ago
- Due date set to 2024-02-15
Setting due date based on mean cycle time of SUSE QE Tools
Updated by szarate 10 months ago
- Related to action #137489: [qe-core] Perl Tidy updates should announce when package is available on Factory added
Updated by ybonatakis 10 months ago
Before link the .perlcriticrc from os-autoinst-common is better to apply the changes sequentially per rule.
Some rules are under discussion
- https://suse.slack.com/archives/C02AJ1E568M/p1706805133571569
- https://suse.slack.com/archives/C02AJ1E568M/p1706805214484479
- https://suse.slack.com/archives/C02AJ1E568M/p1706805314954719
- https://suse.slack.com/archives/C02AJ1E568M/p1706805376502689
The following PR is all about interpolation strings(Useless interpolation of literal string
) and some Hash key with quotes
https://github.com/os-autoinst/openQA/pull/5448
quite big but really simple
Updated by ybonatakis 10 months ago
Another one for no critic
annotation https://github.com/os-autoinst/openQA/pull/5449
Updated by ybonatakis 10 months ago
Created https://github.com/os-autoinst/openQA/pull/5450 as followup refactoring to fix deeply nested loops. I would like to review this before peer reviews as i think it can be a little better.
Updated by ybonatakis 10 months ago
https://github.com/os-autoinst/openQA/pull/5453 was last touch on OpenQA.
All merged so ..last part before switch to the perlcritic from the os-autoinst-common is one small adjustment regarding useless quotes in hashes
https://github.com/os-autoinst/os-autoinst-common/pull/40
Updated by okurz 10 months ago
- Copied to action #155062: Unify GitHub Actions for QA Projects - perltidy in os-autoinst size:M added
Updated by okurz 10 months ago
- Copied to action #155065: Unify GitHub Actions for QA Projects - use as many common static checks within all os-autoinst repos as possible added
Updated by ybonatakis 10 months ago
https://github.com/os-autoinst/openQA/pull/5459 last PR for openQA. https://github.com/os-autoinst/openQA/pull/5457 has to merged first after https://github.com/os-autoinst/os-autoinst-common/pull/45/files :(
turn focus to os-autoinst-distri-example
Updated by ybonatakis 9 months ago
As we agree, we want just to enable checks from common repo on os-autoinst-distri-example.
https://github.com/os-autoinst/os-autoinst-distri-example/pull/31 enables yamllint, tidy and perlcritic.
overall, openQA uses the common perlcritic, os-autoinst yamllint and perlcritic and finally yamllint, tidy and perlcritic for os-autoinst-distri-example
Updated by ybonatakis 9 months ago
- Status changed from In Progress to Feedback
https://github.com/os-autoinst/os-autoinst-distri-example/pull/31 contains commit which fixes also isotovideo tests
Updated by ybonatakis 9 months ago
ybonatakis wrote in #note-46:
https://github.com/os-autoinst/os-autoinst-distri-example/pull/31 contains commit which fixes also isotovideo tests
Seems I have to split all the commits. here is the first https://progress.opensuse.org/issues/138416#note-46 cloning the common repo
Updated by ybonatakis 9 months ago
ybonatakis wrote in #note-47:
ybonatakis wrote in #note-46:
https://github.com/os-autoinst/os-autoinst-distri-example/pull/31 contains commit which fixes also isotovideo tests
Seems I have to split all the commits. here is the first https://progress.opensuse.org/issues/138416#note-46 cloning the common repo
fixing the pipeline https://github.com/os-autoinst/os-autoinst-distri-example/pull/33
Updated by ybonatakis 9 months ago
pipeline fixed by another PR by Oliver.
this ticket is almost ready. problem is that we split the initial commit in smaller parts. Give it a couple of days
Updated by ybonatakis 9 months ago
- Status changed from In Progress to Feedback