mantle/kola: copy basic tests into formal kola workflow

[prestist]

cmd-kola has non-conformed basic kola tests. These tests do not benefit from the supporting functionality around kola test's such as denylisting or other functionality. reduce cmd-kola and conform the basic kola tests.

fixes #3418

This is a small rework on an already existing; but stale pr. Cherry-picked and made a few touches.

[prestist]

hmm, I might have missed it but I did not see what triggers the CI to run kola -p qemu --build latest run --rerun --allow-rerun-success=tags=needs-internet --on-warn-failure-exit-77 --arch=x86_64 --basic-qemu-scenarios

If someone could point me to it I can make a pr and remove it.

[ravanelli]

hmm, I might have missed it but I did not see what triggers the CI to run kola -p qemu --build latest run --rerun --allow-rerun-success=tags=needs-internet --on-warn-failure-exit-77 --arch=x86_64 --basic-qemu-scenarios

If someone could point me to it I can make a pr and remove it.

We also need to touch coreos-ci-lib

[ravanelli]

Only a small comment, overall LGTM

[prestist]

hmm, I might have missed it but I did not see what triggers the CI to run kola -p qemu --build latest run --rerun --allow-rerun-success=tags=needs-internet --on-warn-failure-exit-77 --arch=x86_64 --basic-qemu-scenarios
If someone could point me to it I can make a pr and remove it.

We also need to touch coreos-ci-lib

awesome; made a pr. thank you

[marmijo]

This LGTM generally. I'm curious if we need 6 kola tests though. I believe the original logic produced only 4 basic tests:

• x86_64
  • basic (nvme)
  • uefi
  • uefi-secure
• multi-arch
  • basic (not-nvme)

[travier]

Trying to summarize what we'll run:

	x86_64	aarch64	ppc64le	s390x
boot type	bios/uefi/uefi-secure	bios	bios	bios
nvme	yes	no	no	no
Number of combination	6	1	1	1

So the question is: Should we really run all 6 combination for the test matrix on x86_64 or just a subset of those for nvme? For now we were only running basic (nvme), uefi, uefi-secure as Michael said.

Shouldn't we run uefi & uefi-secure boot on aarch64 as well? Feels like that should be working.

EDIT: Re-reading the PR, this runs all 6 ones for x86_64 so should be OK.

[marmijo]

Re-reading the PR, this runs all 6 ones for x86_64 so should be OK

I'm good with that. It makes sense that we would test bios, uefi, and uefi-secure with both nvme and non-nvme conditions on x86_64.

So we just need to make sure that the nvme and uefi/uefi-secure tests dont run on other arches.

Shouldn't we run uefi & uefi-secure boot on aarch64 as well? Feels like that should be working.

Unless we want these ones to run on aarch64.

[prestist]

So we just need to make sure that the nvme and uefi/uefi-secure tests dont run on other arches.

If thats the case It sounds like I need to update the field Architectures to only run x86 on all but bios?

[marmijo]

If thats the case It sounds like I need to update the field Architectures to only run x86 on all but bios?

Yes, I believe that should do it!

[jlebon]

So overall I think there are at least two main ways I see we can approach this:

1. Have knobs as part of register.Test to say e.g. which firmware to use for the test.
2. Use ClusterSize: 0 and manually bring up the VMs ourselves. This is done in a bunch of QEMU tests already where we need more control over the VM settings. (Do a git grep 'ClusterSize: \+0' to see them.)

Leaning towards the latter to avoid having to add a bunch more knobs in register.Test. You can have a single function that sets up the tests and takes arguments for the variable knobs and each test calls that one function with the different combinations we want (see the luks.* tests for an example of this pattern).

Re. the combinations, my vote is to match the current coverage and then we can extend it in the future. But not strongly against either!

[prestist]

So overall I think there are at least two main ways I see we can approach this:

1. Have knobs as part of `register.Test` to say e.g. which firmware to use for the test.

2. Use `ClusterSize: 0` and manually bring up the VMs ourselves. This is done in a bunch of QEMU tests already where we need more control over the VM settings. (Do a `git grep 'ClusterSize: \+0'` to see them.)

Leaning towards the latter to avoid having to add a bunch more knobs in register.Test. You can have a single function that sets up the tests and takes arguments for the variable knobs and each test calls that one function with the different combinations we want (see the luks.* tests for an example of this pattern).

Re. the combinations, my vote is to match the current coverage and then we can extend it in the future. But not strongly against either!

Ok thank you for the pointer. I am looking into doing this.

[prestist]

/retest

[jlebon]

+ cosa kola --basic-qemu-scenarios --skip-secure-boot --output-dir /logs/artifacts/kola-basic
kola -p qemu run --basic-qemu-scenarios --skip-secure-boot --output-dir /logs/artifacts/kola-basic
Error: unknown flag: --basic-qemu-scenarios

Hmm, the conditional at https://github.com/openshift/os/blob/59526550b3a4a466c6354cbf2455eeabe1122d1c/ci/prow-entrypoint.sh#L129 is failing for some reason. I wonder if somehow this isn't running with the changes from this PR.

I guess we can easily test that.
Added a test commit.

[jlebon]

OK, finally got to the bottom of the CI issue. Should be fixed by openshift/os#1485.

[prestist]

✅ 👀! I see green!

[prestist]

Thank you, holy crap. Talk about entangled.

[prestist]

Also, I noticed that you changed it from uefi.secure to uefi-secure in the latest push https://github.com/coreos/coreos-assembler/compare/9905a54445f384b124a8e83fe4f12e0dff2f8bb1..a6ad56783a796f4baa698e8d4c842532765b6482

Thank you for catching that.

[jlebon]

OK, finally green CI here! 🎉

I think we can merge this now, but to be safe let's wait until coreos/fedora-coreos-streams#886 is past the build and test stages.

[dustymabe]

I think we can merge this now, but to be safe let's wait until coreos/fedora-coreos-streams#886 is past the build and test stages.

we're past that point now! merging

[jlebon]

Next steps on this:

• Evaluate backportability to older cosa up to rhcos-4.12. If it applies cleanly/with minimal effort, then let's do that. Then we'll be able to clean up the logic added in vars/kola: drop --basic-qemu-scenarios coreos-ci-lib#154.
• Clean up the logic in openshift/os around Secure Boot to instead just use a denylist entry.

[travier]

Thanks Steven and Jonathan for persisting with this PR!

[travier]

openshift/os: openshift/os#1489