tests: add distro tags to FCOS only tests #1322

miabbott · 2021-11-02T20:06:38Z

This is a first pass at categorizing the existing external tests using
the distros tag to indicate that the tests are only supported on
FCOS. This should allow better sharing of tests with RHCOS.

Note: some of these tests may be able to be adapted for RHCOS; will
investigate in a follow-up PR.

See: openshift/os#661
See: coreos/coreos-assembler#2521

This removes the individually symlinked tests in favor of a symlink to the entire FCOS external tests directory. With the external tests properly categorized with the `distros` tag, only tests that are applicable to both FCOS + RHCOS should run from the `shared` directory. Requires: coreos/fedora-coreos-config#1322

HuijingHei · 2021-11-04T13:08:37Z

Looks good to me

tests/kola/README.md

tests/kola/ignition/systemd-unmasking/test.sh

dustymabe

Thanks for doing this work @miabbott.

A few suggestions.. Mostly LGTM, but I'd like to advocate for adding reasons why tests only apply to FCOS even if it's blatantly obvious. I really do feel that stating the obvious is a good thing sometimes especially when new people stumble onto the code and might not know the history.

miabbott · 2021-11-04T14:03:15Z

Thanks for doing this work @miabbott.

A few suggestions.. Mostly LGTM, but I'd like to advocate for adding reasons why tests only apply to FCOS even if it's blatantly obvious. I really do feel that stating the obvious is a good thing sometimes especially when new people stumble onto the code and might not know the history.

Great suggestion, Dusty! I'll work that into the next revision.

saqibali-2k

I second Dusty's idea of adding a short description of why is test is limited to FCOS. Besides that changes look good overall!

dustymabe · 2021-11-08T15:27:19Z

tests/kola/toolbox/test.sh

@@ -13,7 +13,8 @@

 # Only run on QEMU to reduce CI costs as nothing is platform specific here.
 # Toolbox container is currently available only for x86_64 and aarch64 in Fedora
-# kola: { "tags": "needs-internet", "platforms": "qemu-unpriv", "architectures": "x86_64 aarch64" }
+# This test only runs on FCOS because RHCOS is missing the `machinectl` command.


I think this might be able to be improved. While technically accurate that RHCOS might be missing the machinectl command I think there are other reasons that we would only want this test to run on FCOS. i.e. I imagine the default container they pull should be different and the test we want to validate is probably different. So we'd either need to adapt the test to work on both or just state that this test is targeted at FCOS and we should create a RHCOS specific toolbox test in the future.

Side note: remember soon "RHCOS" will be ambiguous. It'd be more accurate to say "RHCOS8" for example, because perhaps we do choose to pull in machinectl in RHCOS9.

This is actually another reason why longer term these distro tags are going to be less sustainable than doing e.g. feature detection. IOW skipping the test if machinectl doesn't exist.

I think the current practice is probably most efficient. Basically what we have been doing typically is using feature detection if it's easy. Sometimes this is easily done by querying a program other times not. Sometimes it can also be done by querying a version of a particular software (i.e. systemd or NetworkManager). Other times it's not so easy and kind of complex. In those cases I think we should basically switch off RHCOS/FCOS and then do "version detection" (i.e. F34, F35, RHEL 8.X, RHEL 9.X) within the embedded logic.

dustymabe

LGTM - I just had one suggestion for improvement, but that's optional.

jlebon · 2021-11-10T20:43:53Z

Are we planning to hold this until #1325? Otherwise I think this will need an additional pass for all the tests split out of misc-ro.

miabbott · 2021-11-12T20:49:59Z

Rebased after #1325 landed, but now I see #1337, so might be another rebase in my future.

I tweaked three tests as separate commits to make them work seamlessly across FCOS/RHCOS. (@dustymabe interested in any feedback re: the networking test I changed)

There are other tests in there that could be modified for the same outcome, but require a little more investigation.

miabbott · 2021-11-15T21:22:30Z

Rebased again on top of #1337.

Added additional comments (grep for TODO-RHCOS) to indicate which tests may be able to be adapted for RHCOS.

tests/kola/files/rpmdb-sqlite

tests/kola/firewall/iptables-legacy

tests/kola/networking/stub-resolve.sh

tests/kola/root-reprovision/luks/test.sh

tests/kola/ignition/systemd-unmasking/test.sh

tests/kola/networking/default-network-behavior-change/test.sh

dustymabe · 2021-11-15T22:32:16Z

Thank you so much for working on this @miabbott!

miabbott · 2021-11-16T21:33:19Z

Rebased one more time and dropped the commit to change the network test that was covered by #1339

dustymabe

LGTM - one spelling fix suggestion

dustymabe · 2021-11-16T21:35:36Z

tests/kola/containers/cgroups-v2

@@ -1,5 +1,8 @@
 #!/bin/bash
-# kola: { "exclusive": false }
+# This test only runs on FCOS because RHCOS does not currently support cgroupsv2
+# by defaulr.


Suggested change

# by defaulr.

# by default.

dustymabe · 2021-11-16T21:40:08Z

Hurry up and merge before anything else lands!

This is a first pass at categorizing the existing external tests using the `distros` tag to indicate that the tests are only supported on FCOS. This should allow better sharing of tests with RHCOS. Note: some of these tests may be able to be adapted for RHCOS; will investigate in a follow-up PR. See: openshift/os#661 See: coreos/coreos-assembler#2521

Omitting the "." from the `jq` expression on RHCOS causes the command to fail. By including the ".", the test can run on FCOS + RHCOS.

The version of `find` on RHCOS does not accept a comma-separated list of file types to the `-type` argument. Splitting it into multiple args should allow the test to work with FCOS/RHCOS.

This removes the individually symlinked tests in favor of a symlink to the entire FCOS external tests directory. With the external tests properly categorized with the `distros` tag, only tests that are applicable to both FCOS + RHCOS should run from the `shared` directory. Requires: coreos/fedora-coreos-config#1322

The version of `systemd-udev` on RHCOS does not include the `systemd-repart` service. Missed this test as part of coreos#1322.

The version of `systemd-udev` on RHCOS does not include the `systemd-repart` service. Missed this test as part of #1322.

This removes the individually symlinked tests in favor of a symlink to the entire FCOS external tests directory. With the external tests properly categorized with the `distros` tag, only tests that are applicable to both FCOS + RHCOS should run from the `shared` directory. Requires: coreos/fedora-coreos-config#1322

With the work related to coreos/fedora-coreos-config#1322, we can now share the external tests from FCOS more sanely with RHCOS. This introduces a 'shared' directory under `tests/kola` that symlinks to the FOCS external tests. Those external tests are currently categorized to run only on the supported distros, so including the whole directory is safe to do.

With the work related to coreos/fedora-coreos-config#1322, we can now share the external tests from FCOS more sanely with RHCOS. This introduces a 'shared' directory under `tests/kola` that symlinks to the FOCS external tests. Those external tests are currently categorized to run only on the supported distros, so including the whole directory is safe to do. Additionally, any duplicated tests have been removed from this directory; we should default to writing tests that support both FCOS/RHCOS and only stray from that default when absolutely necessary.

The version of `systemd-udev` on RHCOS does not include the `systemd-repart` service. Missed this test as part of coreos#1322.

dustymabe reviewed Nov 4, 2021

View reviewed changes

tests/kola/README.md Outdated Show resolved Hide resolved

dustymabe reviewed Nov 4, 2021

View reviewed changes

tests/kola/README.md Outdated Show resolved Hide resolved

dustymabe reviewed Nov 4, 2021

View reviewed changes

tests/kola/ignition/systemd-unmasking/test.sh Outdated Show resolved Hide resolved

dustymabe reviewed Nov 4, 2021

View reviewed changes

dustymabe mentioned this pull request Nov 4, 2021

tests: separate out misc-ro test #1325

Merged

saqibali-2k previously approved these changes Nov 5, 2021

View reviewed changes

miabbott dismissed saqibali-2k’s stale review via d3b751f November 5, 2021 21:21

miabbott force-pushed the distro_tag_tests branch 2 times, most recently from d3b751f to dfde190 Compare November 5, 2021 21:26

dustymabe reviewed Nov 8, 2021

View reviewed changes

dustymabe previously approved these changes Nov 8, 2021

View reviewed changes

miabbott dismissed dustymabe’s stale review via c8b9364 November 8, 2021 18:47

miabbott force-pushed the distro_tag_tests branch from dfde190 to c8b9364 Compare November 8, 2021 18:47

miabbott force-pushed the distro_tag_tests branch from c8b9364 to 21643de Compare November 12, 2021 20:47

miabbott force-pushed the distro_tag_tests branch from 21643de to 7fa7a5d Compare November 15, 2021 21:21

dustymabe reviewed Nov 15, 2021

View reviewed changes

tests/kola/networking/default-network-behavior-change/test.sh Outdated Show resolved Hide resolved

miabbott mentioned this pull request Nov 16, 2021

tests/kdump: Use test definition from fedora-coreos-config openshift/os#659

Closed

miabbott force-pushed the distro_tag_tests branch from 7fa7a5d to a798715 Compare November 16, 2021 21:32

dustymabe previously approved these changes Nov 16, 2021

View reviewed changes

miabbott added 3 commits November 17, 2021 08:35

tests/aleph-version: change jq to work on RHCOS

5310f05

Omitting the "." from the `jq` expression on RHCOS causes the command to fail. By including the ".", the test can run on FCOS + RHCOS.

tests/files: split types into separate arguments

06bcd7d

The version of `find` on RHCOS does not accept a comma-separated list of file types to the `-type` argument. Splitting it into multiple args should allow the test to work with FCOS/RHCOS.

miabbott dismissed dustymabe’s stale review via 06bcd7d November 17, 2021 13:35

miabbott force-pushed the distro_tag_tests branch from a798715 to 06bcd7d Compare November 17, 2021 13:35

miabbott enabled auto-merge (rebase) November 17, 2021 13:35

dustymabe approved these changes Nov 17, 2021

View reviewed changes

miabbott merged commit dac1f68 into coreos:testing-devel Nov 17, 2021

miabbott added a commit to miabbott/fedora-coreos-config that referenced this pull request Nov 17, 2021

tests/systemd-repart-service: only run on FCOS

ae47daa

The version of `systemd-udev` on RHCOS does not include the `systemd-repart` service. Missed this test as part of coreos#1322.

miabbott mentioned this pull request Nov 17, 2021

tests/systemd-repart-service: only run on FCOS #1341

Merged

miabbott added a commit to miabbott/fedora-coreos-config that referenced this pull request Nov 18, 2021

tests/systemd-repart-service: only run on FCOS

9584554

The version of `systemd-udev` on RHCOS does not include the `systemd-repart` service. Missed this test as part of coreos#1322.

miabbott added a commit that referenced this pull request Nov 18, 2021

tests/systemd-repart-service: only run on FCOS

8ecedee

The version of `systemd-udev` on RHCOS does not include the `systemd-repart` service. Missed this test as part of #1322.

HuijingHei pushed a commit to HuijingHei/fedora-coreos-config that referenced this pull request Oct 10, 2023

tests/systemd-repart-service: only run on FCOS

d803402

The version of `systemd-udev` on RHCOS does not include the `systemd-repart` service. Missed this test as part of coreos#1322.

HuijingHei pushed a commit to HuijingHei/fedora-coreos-config that referenced this pull request Oct 10, 2023

tests/systemd-repart-service: only run on FCOS

72868a0

The version of `systemd-udev` on RHCOS does not include the `systemd-repart` service. Missed this test as part of coreos#1322.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: add distro tags to FCOS only tests #1322

tests: add distro tags to FCOS only tests #1322

miabbott commented Nov 2, 2021

HuijingHei commented Nov 4, 2021

dustymabe left a comment •

edited

Loading

miabbott commented Nov 4, 2021

saqibali-2k left a comment

dustymabe Nov 8, 2021 •

edited

Loading

cgwalters Nov 8, 2021

dustymabe Nov 15, 2021

dustymabe left a comment

jlebon commented Nov 10, 2021

miabbott commented Nov 12, 2021 •

edited

Loading

miabbott commented Nov 15, 2021

dustymabe commented Nov 15, 2021

miabbott commented Nov 16, 2021

dustymabe left a comment

dustymabe Nov 16, 2021

dustymabe commented Nov 16, 2021

tests: add distro tags to FCOS only tests #1322

tests: add distro tags to FCOS only tests #1322

Conversation

miabbott commented Nov 2, 2021

HuijingHei commented Nov 4, 2021

dustymabe left a comment • edited Loading

Choose a reason for hiding this comment

miabbott commented Nov 4, 2021

saqibali-2k left a comment

Choose a reason for hiding this comment

dustymabe Nov 8, 2021 • edited Loading

Choose a reason for hiding this comment

cgwalters Nov 8, 2021

Choose a reason for hiding this comment

dustymabe Nov 15, 2021

Choose a reason for hiding this comment

dustymabe left a comment

Choose a reason for hiding this comment

jlebon commented Nov 10, 2021

miabbott commented Nov 12, 2021 • edited Loading

miabbott commented Nov 15, 2021

dustymabe commented Nov 15, 2021

miabbott commented Nov 16, 2021

dustymabe left a comment

Choose a reason for hiding this comment

dustymabe Nov 16, 2021

Choose a reason for hiding this comment

dustymabe commented Nov 16, 2021

dustymabe left a comment •

edited

Loading

dustymabe Nov 8, 2021 •

edited

Loading

miabbott commented Nov 12, 2021 •

edited

Loading