CI: Add testing based on Centos containers (COMPOSER-2125)

[elkoniu]

In the release loop upstream changes are merged to Centos every two weeks. This creates a delay in error detection when new tests being added upstream.

Running tests in Centos based containers for every pull request should solve the issue.

https://issues.redhat.com/browse/COMPOSER-2125

[achilleas-k]

In the gitlab CI, we build CS9 manifests (on CS9 in AWS) already. https://gitlab.com/redhat/services/products/image-builder/ci/osbuild/-/jobs/6155161065
They might not be as comprehensive as the unit tests (we might not be covering all supported stages in those manifests), but the stage unit tests are all actually Fedora manifests. Which is also fine, we're testing if osbuild runs properly on CS9 I suppose, but I think the existing tests are both more realistic (building CS9 on CS9) and less likely to break due to unsupported features (trying to build a Fedora manifest with a btrfs disk on CS9).

I think we would be better served by having a job that updates the content in the manifests more frequently than running the Fedora-based unit tests.

[elkoniu]

What I am doing here precisely is:

1. Spawn c8s and c9s centos images prepared by @thozza here:Add osbuild-ci images based on c8s and c9s (COMPOSER-2125) containers#63
2. In the action I am reusing testing scripts directly from the centos repo: https://gitlab.com/redhat/centos-stream/rpms/osbuild/-/tree/c9s/tests/unit-tests?ref_type=heads
3. Normally those scripts are used by ZuluCI when upstream release is pushed to centos repo by centos-bot every 2 weeks.
4. Result is donsteam-like testing of every upstream PR to check if it will not break the centos stream (and rhel stream as result)

From time to time we have a situation when centos testing fails due to upstream changes and we detect this with the 2 weeks delay. I know it will delay PR review as running centos tests takes more than 2h. I think we should mark this as optional from the review point of view or run nightly. Still, we would be able detect "potential" centos / rhel issue way faster than in 2 weeks from PR being merged.

[achilleas-k]

I know it will delay PR review as running centos tests takes more than 2h. I think we should mark this as optional from the review point of view or run nightly.

Right, that makes sense. Thanks for the explanation!

[achilleas-k]

I'm fine to merge now but maybe we can think about doing these nightly.

[elkoniu]

Lets get an opionion from @thozza and @ochosi \o/

[ochosi]

The code and approach are both fine - thanks @elkoniu for figuring this out! ❤️

At the same time, >2h sounds like a hefty price to pay on every PR.
Plus, I'm not a strong believer in non-blocking CI, although sometimes it makes sense and maybe this is one of those times.

Combining the two points above I wonder if a nightly test wouldn't be a good compromise. We would still get the benefit of early notifications when something sneaks into main that breaks downstream, but it wouldn't slow down development too much.

All it would take is changing the on: condition to a cron schedule and upon failure we can notify ourselves via the #osbuild-release Slack channel.
WDYT?

Happy to wait for @thozza to be back to get to the final decision.

[elkoniu]

@achilleas-k and @ochosi - thank you both for the review. I will update the code tomorrow with:

• running nightly
• removing duplicated jobs by change to matrix strategy

I think we should wait for @thozza final verdict as I am using some centos downstream scripts bit "nasty" way instead of copying them to repo (as usually our upstream feeds downstream, not the other way around:) )

[thozza]

Thanks for your work on this Pawel, much appreciated.

Unfortunately, I do not think that this approach is correct and it would IMO create problems in the future. I added a longer comment inline, but in short, the test should not depend on any downstream code from CentOS Stream. The whole idea is to run the unit tests in a pre-built CS containers (in addition to the Fedora container that is used now).

[elkoniu]

Centos based tests unwraps to different pytest settings.
Centos8 settings:

sudo python3 -m pytest
--rootdir /osb/workdir/osbuild/osbuild-111
--ignore /osb/workdir/osbuild/osbuild-111/test/src
--unsupported-fs btrfs
-k 'not (TestBoot and boot) and not (test_qemu[ext4-vdi] or test_qemu[xfs-vdi]) and not (TestStages and test_qemu)'
-v /osb/workdir/osbuild/osbuild-111/test/

Centos9 settings:

sudo python3 -m pytest
--rootdir /osb/workdir/osbuild/osbuild-111
--ignore /osb/workdir/osbuild/osbuild-111/test/src
--unsupported-fs btrfs
-k 'not (TestBoot and boot)'
-v /osb/workdir/osbuild/osbuild-111/test/

I have encapsulated this final pytest parameters into the new CI jobs and removed dependency from downstream code.

[elkoniu]

@thozza @ochosi I think this one is ready to go.

[thozza]

Thanks for the changes. I like this version!

I've added a few comments in-line.

Could you please try testing the action as a PR in your own repo against main? Because from what I can tell, the action was not executed as part of this PR, so I can't tell if it works.

[elkoniu]

@thozza thanks for the review \o/

Could you please try testing the action as a PR in your own repo against main? Because from what I can tell, the action was not executed as part of this PR, so I can't tell if it works.

As @ochosi suggested I switched from calling this action on every PR to a nightly schema. I will add run result from my fork in comment.

[thozza]

Thanks a lot for the changes. I like how it all turned out and how simple is the action. 🥳

Happy to approve once we resolve the remaining discussions and after you can provide an example run of the action.

[ochosi]

This is fine by me!

Not sure about osbuild testing... We touched upon this part of the CI a bit in the osbuild CI hackathon, but I'm not sure if this would be helpful here, too.

[achilleas-k]

Clicked the rebase button.

[thozza]

FTR, this PR is good to go from my PoV, I'm waiting only on a proof of a successful run of the pipeline (on a fork).

not relevant any more

[github-actions]

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

[thozza]

@elkoniu what is the status of this PR?