Project

General

Profile

Actions

action #138416

closed

openQA Tests (public) - coordination #96596: [qe-core][CI] CI/CD and Coding style improvements

Unify GitHub Actions for QA Projects size:M

Added by josegomezr about 1 year ago. Updated 10 months ago.

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

0%

Estimated time:

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

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


Related issues 8 (5 open3 closed)

Related to openQA Project (public) - coordination #58184: [saga][epic][use case] full version control awareness within openQABlockedokurz2019-08-01

Actions
Related to openQA Project (public) - action #115013: os-autoinst-plugin as a wheel helper toolNew2022-08-04

Actions
Related to openQA Project (public) - coordination #117097: Evaluate GitHub template repositories for wheelsNew2022-09-23

Actions
Related to openQA Project (public) - action #151219: [bug][os-autoinst-distri-opensuse] No .perltidyrcResolvedtinita2023-11-21

Actions
Related to openQA Tests (public) - action #137489: [qe-core] Perl Tidy updates should announce when package is available on FactoryWorkable2023-10-05

Actions
Copied to openQA Project (public) - action #138674: Reusable github workflow in openQA is causing a problem for os-autoinst-bot dependency PRResolvedtinita2023-10-24

Actions
Copied to openQA Project (public) - action #155062: Unify GitHub Actions for QA Projects - perltidy in os-autoinst size:MResolvedtinita2023-10-24

Actions
Copied to openQA Project (public) - action #155065: Unify GitHub Actions for QA Projects - use as many common static checks within all os-autoinst repos as possibleNew

Actions
Actions #1

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?

Actions #2

Updated by okurz about 1 year ago

  • Category set to Feature requests
  • Target version set to future
Actions #3

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.

Actions #4

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.

Actions #5

Updated by josegomezr about 1 year ago

Steps forward:

  1. [x] Cleanup the Reusable workflow PR 0 to have only the commit messages check.
  2. [x] Have it merged.
  3. [] Substitute checks in:

    with the reusable checks from os-autoinst-common.

Actions #6

Updated by szarate about 1 year ago

  • Tags set to platform-team
  • Parent task set to #96596
Actions #7

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
Actions #10

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
Actions #11

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

Actions #12

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.

Actions #13

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
Actions #14

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 :)

Actions #15

Updated by livdywan about 1 year ago

  • Target version changed from QE-Core: Ready to Ready
Actions #16

Updated by okurz about 1 year ago

  • Related to coordination #58184: [saga][epic][use case] full version control awareness within openQA added
Actions #17

Updated by okurz about 1 year ago

  • Related to action #115013: os-autoinst-plugin as a wheel helper tool added
Actions #18

Updated by okurz about 1 year ago

Actions #19

Updated by okurz about 1 year ago

  • Subject changed from [qe-core] Unify GitHub Actions for all QA Projects to [qe-core] Unify GitHub Actions for QA Projects size:M
  • Description updated (diff)
Actions #20

Updated by tinita about 1 year ago

  • Related to action #151219: [bug][os-autoinst-distri-opensuse] No .perltidyrc added
Actions #21

Updated by openqa_review about 1 year ago

  • Due date set to 2023-12-07

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

Actions #22

Updated by josegomezr about 1 year 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.
Actions #23

Updated by tinita about 1 year ago

  • Due date changed from 2023-12-07 to 2023-12-14
Actions #24

Updated by livdywan about 1 year 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.

Actions #25

Updated by szarate about 1 year 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.

Actions #26

Updated by josegomezr about 1 year 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:

Actions #27

Updated by okurz about 1 year ago

Let's focus on the discussions in the open pull requests

Actions #28

Updated by josegomezr 12 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 of isotovideo [...] --exit-status-from-error.

whatever of the two is less effort.

Actions #31

Updated by okurz 11 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:

Actions #32

Updated by tinita 11 months ago

Currently making several subrepo update PRs, using the new common config files where possible

Actions #33

Updated by ybonatakis 11 months ago

  • Status changed from Workable to In Progress
  • Assignee set to ybonatakis
Actions #34

Updated by ybonatakis 11 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

Actions #35

Updated by openqa_review 11 months ago

  • Due date set to 2024-02-15

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

Actions #36

Updated by szarate 11 months ago

  • Related to action #137489: [qe-core] Perl Tidy updates should announce when package is available on Factory added
Actions #37

Updated by ybonatakis 11 months ago

Before link the .perlcriticrc from os-autoinst-common is better to apply the changes sequentially per rule.
Some rules are under discussion

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

Actions #38

Updated by ybonatakis 11 months ago

Another one for no critic annotation https://github.com/os-autoinst/openQA/pull/5449

Actions #39

Updated by ybonatakis 11 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.

Actions #40

Updated by ybonatakis 11 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

Actions #41

Updated by okurz 11 months ago

  • Copied to action #155062: Unify GitHub Actions for QA Projects - perltidy in os-autoinst size:M added
Actions #42

Updated by okurz 11 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
Actions #43

Updated by okurz 11 months ago

  • Description updated (diff)
Actions #45

Updated by ybonatakis 10 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

Actions #46

Updated by ybonatakis 10 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

Actions #47

Updated by ybonatakis 10 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

Actions #48

Updated by ybonatakis 10 months ago

  • Status changed from Feedback to In Progress
Actions #49

Updated by ybonatakis 10 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

Actions #50

Updated by ybonatakis 10 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

Actions #52

Updated by ybonatakis 10 months ago

  • Status changed from Feedback to Resolved
Actions #53

Updated by okurz 10 months ago

  • Due date deleted (2024-02-15)
Actions

Also available in: Atom PDF