kola/harness: Adjust stream-based denylist filtering for --qemu-image and unknown streams#4563
kola/harness: Adjust stream-based denylist filtering for --qemu-image and unknown streams#4563Rolv-Apneseth wants to merge 3 commits into
Conversation
…can't be determined When a denylist entry has defined streams, the default when the current stream can't be determined is to skip the test anyway. This change inverts the behaviour to avoid unintentionally skipping tests. The example I ran into before making these changes was running kola against a qcow2 image.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a DiskImageIsUserProvided flag to track whether a QEMU disk image was explicitly provided. This flag is used in the test harness to skip reading stream metadata from meta.json when a custom image is used, and the logic for applying stream-scoped denylist entries has been updated accordingly. Review feedback suggested refining the warning message for undetermined streams to be more accurate and concise.
Rolv-Apneseth
force-pushed
the
stream_skip
branch
from
8e07b08 to
ef3751a
Compare
|
Described logic sounds good to me. You have some misc whitespace changes mixed in the middle. Can you split those into a distinct commit? |
Sorry - formatter. I'll split those out |
… disk image The disk image being tested may belong to a different stream so it doesn't make sense to use the local build's stream. Prefer using --denylist-stream.
Rolv-Apneseth
force-pushed
the
stream_skip
branch
from
ef3751a to
7c18687
Compare
|
/retest |
|
@Rolv-Apneseth: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/retest |
3 participants
This one may be a bit opinionated so I'm very open to differing opinions and/or feedback.
I ran into a scenario I found confusing with
kola. I tried runningkola run '*-rt-*' --qemu-image <rhel-10.2 image>and found that denylist entries for c10s were getting applied, which I found very unintuitive. This was because even when--qemu-imageis passed by the user, the stream is still taken from the local build'smeta.json.While changing this behaviour, I also realised that when the stream can't be determined, the default is to apply stream-scoped entries anyway (skip the tests).
With these changes:
--qemu-image, the stream is not read from the local build'smeta.jsonI think 2. makes more sense in most scenarios, not just when running a
--qemu-image. If the stream can't be determined, the issue should probably be investigated (or if using a QEMU image,--denylist-streamshould be used). The lone log line can easily be missed when the tests are just skipped by default, but is a lot more likely to get seen when tests are failing.Also worth noting that this shouldn't affect the pipelines since we don't use
--qemu-imageand we always pass--build(always has stream frommeta.json): https://github.com/coreos/coreos-ci-lib/blob/e570b12f44b28e69f18eac336c93b45e59466af3/vars/kola.groovy#L57