Add basic TMT gating test structure

[c4rt0]

Adds TMT / FMF tests structure. Without this, broken RPM builds pass unchecked.

Follows the same pattern used by ignition and coreos-installer

Reference: work done by @aaradhak's in #127.

Resolve COS-2288: Add gating test for console-login-helper-messages.

[travier]

I think you need to had a workflow (maybe https://github.com/coreos/ignition/blob/main/.github/workflows/tmt-tests.yml) to trigger those.

[c4rt0]

I think you need to had a workflow (maybe https://github.com/coreos/ignition/blob/main/.github/workflows/tmt-tests.yml) to trigger those.

Thank you @travier, this should now work. 🤞🏻

[travier]

Did the test actually run in testing farm? I don't see any URL pointing to the results in the workflow log

[c4rt0]

Did the test actually run in testing farm? I don't see any URL pointing to the results in the workflow log

I think you're referring to the --debug -vvvv which is missing. Adding it now.

[travier]

I'm not sure I understand the goal here. This appear to be testing the RPM installation and then verifying its present. Shouldn't that test be in the distgit repo, testing the installation of the RPM package for PRs there? I think we have packit doing this now already?

If we don't rebuild the RPM from the sources here then we are not testing the content of the PR but just what is in the Fedora repos? What I am missing?

[c4rt0]

@travier : AFAIK the rpm -q test does make sense in the upstream repo. It's not meant to test the PR's code changes. It's there so dist-git can discover it and use it as a gating test against actual RPM builds.

If we don't rebuild the RPM from the sources here then we are not testing the content of the PR".

That's correct, from what I understand the GH Actions workflow doesn't test the PR content. It's just a sanity check which verifies if TMT structure is valid.

This follows the same pattern as ignition's dist-git, where the plans discovers tests from upstream via url, just like ignition's smoke.fmf on gitlab. Through your question I discovered that the dist-git PR for console-login-helper-messages is going to be my next step to have this complete (unless I'm missing something). It would be nice indeed to have some clarification from @tlbueno.

[aaradhak]

@c4rt0 By running 'rpm -q ', it tests almost nothing because it installs the existing package and not the code from the PR. Without rebuilding from source, we are not testing the PR changes. In order to address that packit will act as the bridge. It rebuilds the rpm from PR source code and runs TMT tests against the rebuilt package.

Huijing had worked on enabling packit for bootupd. May be you can use this PR (coreos/bootupd#1003) for reference to add the gating test for CLHM.

[c4rt0]

@aaradhak thank you for the above. I will follow Huijing's work and come back here with some questions if I still have any.

[c4rt0]

/retest

[c4rt0]

Did the test actually run in testing farm? I don't see any URL pointing to the results in the workflow log

@travier I added .packit which should now run the TMT tests on testing farm. If I'm not mistaken this isn't yet triggered as packit isn't activated for console-login-helper-messages. Could we enable it so that I can verify if this works, please?

[travier]

That seems to make more sense. I need to find the instructions to enable packit again.

[c4rt0]

/retest

[c4rt0]

/packit-ci help

[packit-as-a-service]

usage: /packit-ci {help,test,scratch-build} ...

positional arguments:
  {help,test,scratch-build}
                        commands available
    help                show this help message
    test                run tests in Testing Farm
    scratch-build       build package in Koji

Please note:

• /packit-ci commands interact with Fedora CI jobs (e.g. scratch-build, test rpminspect, etc.). If you are looking for help on the opt-in packit jobs (e.g. propose-downstream, pull-from-upstream), refer to /packit or /packit-stg. These assume a valid Packit configuration file is present in the repo.

Contact:

• Email: hello@packit.dev
• Matrix: #packit:fedora.im
• Mastodon: @packit@fosstodon.org

Documentation:

• https://packit.dev/fedora-ci

[joelcapitao]

/packit-ci help

[packit-as-a-service]

usage: /packit-ci {help,test,scratch-build} ...

positional arguments:
  {help,test,scratch-build}
                        commands available
    help                show this help message
    test                run tests in Testing Farm
    scratch-build       build package in Koji

Please note:

• /packit-ci commands interact with Fedora CI jobs (e.g. scratch-build, test rpminspect, etc.). If you are looking for help on the opt-in packit jobs (e.g. propose-downstream, pull-from-upstream), refer to /packit or /packit-stg. These assume a valid Packit configuration file is present in the repo.

Contact:

• Email: hello@packit.dev
• Matrix: #packit:fedora.im
• Mastodon: @packit@fosstodon.org

Documentation:

• https://packit.dev/fedora-ci

[c4rt0]

/packit-ci test

[c4rt0]

It was requested so here's a simple: How to review this PR

This commit adds three things: a TMT test structure, a Packit configuration, and a (copied from dist-git and very gently modified) spec file. In fact I only removed this since it was breaking the build, and if rhelstatement.

TMT test structure (tests/tmt/, .fmf/version)

I was asked why would we need a smoke test which only runs rpm -q, so I figured it out:
The smoke test is here intentionally, minimal. It only runs rpm -q console-login-helper-messages to verify if the package installs. It could fail due to broken dependencies, file conflicts and so on... an rpm can build but still fail to install. This matches the pattern used by ignition, afterburn, and coreos-installer for gating tests... so to review and compare we could look at what was already merged there.

In the TMT smoke plan, there are two .fmf plan files that work together:the smoke.fmf and main.fmf. Before any test runs, TMT installs the package into the test environment. When Packit's tests job runs on Testing Farm, the test VM is a stock Fedora machine (matching the fedora-rawhide-x86_64 in .packit.yaml). Testing Farm automatically adds the COPR repo that Packit's copr_build just built into as a package source on that VM. So when main.fmf says how: install, dnf sees two sources for console-login-helper-messages:

1. The official Fedora repo (stock package, v0.22.0)
2. The COPR repo (PR-built package, v0.22.0-1.pr136.xxx)

The COPR version has a higher release string, so dnf picks it. That's how the test ends up running against this PR's code instead of the stock Fedora package.

We don't configure any of this - it's a packit thing behind the scenes.

Packit configuration (.packit.yaml)

Two jobs: copr_build rebuilds the RPM from the PR source, tests runs the TMT smoke plan against that rebuilt RPM via Testing Farm. This makes the PR test their own content instead of testing it against the stock Fedora rpm. At some point I was wondering if I should divide the architectures, but it turns out that diferent arches would just run the same test, so here I left simply the x86_64.

In-tree spec file (console-login-helper-messages.spec)
As mentioned above this is the Fedora rawhide spec with two fixes.

This curl will show the diff between both:

diff <(curl -s https://src.fedoraproject.org/rpms/console-login-helper-messages/raw/rawhide/f/console-login-helper-messages.spec) <(curl -sL https://raw.githubusercontent.com/c4rt0/console-login-helper-messages/add-gating-test/console-login-helper-messages.spec)

Lastly there's the gh actions workflow (.github/workflows/tmt-tests.yml)
It installs tmt and podman on an Ubuntu runner, then runs tmt in a container. Its just a: "does the Fedora package still exist and does it pass the installation" check... now that I wrote that, I wonder if we need it, but since there's gh actions also in ignition and afterburn, I decided to leave it.

Hope this helps, I'm open to any sugestions!

[joelcapitao]

I left an in-line comment about the changelog.
Also, it would be really great to run tests against CentOS streams targets. In a previous comment, I pointed to the container-tools Packit config but there is also the bootc one which is interesting to take as reference c.f. https://github.com/bootc-dev/bootc/blob/main/.packit.yaml

[c4rt0]

... How to review this PR

I'm afraid this will have to be updated

[travier]

I can see the same RPM built by Packit and tested in TMT so this is looking good.

Can we avoid vendoring a copy of the spec file here? Maybe we can dynamically fetch it like we do it in https://github.com/coreos/ignition/blob/main/.copr/Makefile.

From your spec file change, I found https://src.fedoraproject.org/rpms/console-login-helper-messages/pull-request/18 which also looks like it needs a review so maybe we should fix the current specfile first or do a release of CLHM here and then retry all that?

[travier]

CC @jbtrystram for the patch pending review / update: https://src.fedoraproject.org/rpms/console-login-helper-messages/pull-request/18

[c4rt0]

@travier Thanks for the review!

Can we avoid vendoring a copy of the spec file here? Maybe we can dynamically fetch it like we do it in https://github.com/coreos/ignition/blob/main/.copr/Makefile.

I should flag that this has been previously implemented, and it now conflicts with @joelcapitao's suggestion above to keep the spec here and use propose_downstream to sync it to dist-git. I followed Joel's direction in the latest push.

I'm happy to go either way, but would need clarification which aproach is the one we agree here on:
a) in-tree spec + patch in rpm/, propose_downstream syncs to dist-git
or
b) fetch from dist-git, fix dist-git first.

From your spec file change, I found https://src.fedoraproject.org/rpms/console-login-helper-messages/pull-request/18 which also looks like it needs a review so maybe we should fix the current specfile first or do a release of CLHM here and then retry all that?

Agreed, the dist-git spec needs fixing regardless - I also had the PR opened for the issuegen.conf issue, but Joel pointed out that the scratch build fails because issuegen.conf is still in v.0.22, so I closed it.

Joel's approach is that the dist-git spec currently doesn't match the upstream source (the issuegen.conf issue), which caused the Packit builds to fail when fetching it. Hosting the spec here lets us fix it alongside the code.

[travier]

Ah indeed, this sounds like a better plan. I had missed Joel's comment. LGTM then.

[travier]

Looking good. But I think we should drop the patch from the RPM here.

Modified as requested. 👍🏻

[c4rt0]

Looking good. But I think we should drop the patch from the RPM here.

Done.

[travier]

Looks good to me. Thanks

[travier]

Let's do a release to exercise / validate the full flow?

[jbtrystram]

Let's do a release to exercise / validate the full flow?

don't tag anything in fedora until fedora-selinux/selinux-policy#3169

[joelcapitao]

Looking good. But I think we should drop the patch from the RPM here.

Hmm, but we need that patch downstream don't we ? Or maybe the plan is to add it back only in CentOS stream distgit ?

[c4rt0]

Let's do a release to exercise / validate the full flow?

don't tag anything in fedora until fedora-selinux/selinux-policy#3169

I went through the tagging exercise at CLHM straight away... it was not my intention to do anything at the dist-git side - I assumed that was a maintainer's job.