kola: Replace run-ext-bin with `kola run -E/--exttest` #1252

cgwalters · 2020-03-16T17:54:24Z

As I tried to port the ostree tests, I hit various issues
with the run-ext-bin model; for example, kola run already
supports --parallel and cleanly separates tests into
output directories etc.

This reworks things so that one can do e.g.:

cosa kola run -- --parallel 4 --output-dir tmp/kola -E ~/src/github/ostreedev/ostree/tests/installed/nondestructive/ 'ext.nondestructive.*'

to run the ostree test suite. Or of course, run both the
ostree exttests plus builtin tests basic or whatever.

And another thing I'd like to do soon is teach cosa how to
automatically add src/config/tests so that one can seamlessly
e.g. add a change to the manifest and also add a test for it
as one atomic commit.

There's still a lot more to do here; among other things ostree
cleanly separates destructive/nondestructive tests and having
each test run as a new VM is a hit. But this is a good start
I think.

cgwalters · 2020-03-16T18:00:04Z

I updated ostreedev/ostree#2025 to use this as a demo.

ashcrow

Couple items but I really like this design!

mantle/kola/harness.go

mantle/kola/README-kola-ext.md

Pairs with coreos/coreos-assembler#1252 The motivation here is to get the ball rolling for (where it makes sense) to have a place to add tests corresponding to changes in this repository.

arithx

Didn't test it but few stylistic comments, otherwise LGTM. Will let someone else take a review stab as well.

/approve

mantle/kola/harness.go

mantle/kola/register/register.go

cgwalters · 2020-03-16T23:23:13Z

New demonstration: coreos/fedora-coreos-config#305

cgwalters · 2020-03-17T14:54:10Z

I discovered the rebooting support was even more aspirational rather than functional.

arithx · 2020-03-17T18:09:43Z

I discovered the rebooting support was even more aspirational rather than functional.

What specifically are you seeing? I haven't tried it with NativeFuncs but it works fine for normal tests.

cgwalters · 2020-03-17T18:17:53Z

What specifically are you seeing? I haven't tried it with NativeFuncs but it works fine for normal tests.

This code is trying to support having the test code run /usr/sbin/reboot. Remember a huge difference between these external tests and most of the rest of kola is the idea that the test is a binary running on the target, we just poll its status via ssh.

This avoids a whole lot synchronous round trips with ssh and allows writing e.g. a test as a 3 line shell script instead of needing to write Go. (And yes, I do think shell scripts under 10 lines that don't involve functions can be more expressive and clear than Go code invoking SSH)

But the current kola code around reboots is much more oriented around having kola drive/monitor the reboot status so I'm reworking things to have the test code request a reboot rather than invoke it directly.

cgwalters · 2020-03-17T19:25:16Z

OK cool, I fixed up the rebooting support (not in the most elegant way, but eh it works) and also rewrote the rhcos.luks.tpm test using this: https://gitlab.cee.redhat.com/coreos/redhat-coreos/merge_requests/871

cgwalters · 2020-03-17T19:26:18Z

Oh and this now supports a kola.json that can be in the test directory, e.g. for that one:

{
    "architectures": "!s390x ppc64le",
    "platforms": "qemu-unpriv"
}

Pairs with coreos/coreos-assembler#1252 The motivation here is to get the ball rolling for (where it makes sense) to have a place to add tests corresponding to changes in this repository.

cgwalters · 2020-03-17T22:14:18Z

OK, updated the docs and lifting WIP on this!

cgwalters · 2020-03-17T22:24:05Z

mantle/cmd/kolet/kolet.go

@@ -138,9 +139,16 @@ func dispatchRunExtUnit(unitname string, sdconn *systemddbus.Conn) (bool, error)
 					}
 				case CLD_KILLED:
 					// SIGTERM; we explicitly allow that, expecting we're rebooting.
-					if mainstatus == 15 {
+					if mainstatus == int32(15) {


Oh yeah this...SO EVIL.

ashcrow · 2020-03-18T13:05:35Z

mantle/cmd/kolet/kolet.go

+							Pid: os.Getpid(),
+						}
+						selfproc.Signal(syscall.SIGTERM)
+						time.Sleep(time.Hour)


It feels odd to wait wait up to an hour, but then again it won't hurt to wait that long since it should be interrupted.

cgwalters · 2020-03-18T13:30:08Z

Another big topic here is that I think we need to standardize a bit more of the flow for "build project X, overlay it on fcos, run just the tests for that project (plus possibly kola basic or others) as external tests". A bit more discussion of related topics in coreos/rpm-ostree#1958

Previously we made an effort to use the [Fedora Standard Test Interface](https://docs.fedoraproject.org/en-US/ci/standard-test-interface/). This effort was not very successful; the primary thing that it really died on is Ansible just didn't support rebooting very well. I think that's since gotten better, but even then, Ansible wasn't the best thing for a test framework for us anyways. In the meantime Fedora CoreOS happened emphasizing Ignition and not "post-hoc reconciliation" models like Ansible over ssh. And, [coreos-assembler](https://github.com/coreos/coreos-assembler) happened too. Furthermore, we really need to test OSTree's interaction with Ignition as we've invented several special things there. Then most recently, I've been working on having cosa/kola support running externally defined tests: coreos/coreos-assembler#1215 There's a lot of things to clean up after this but at least this works for me: ``` $ cd /srv/fcos $ cosa kola run -- --parallel 4 --output-dir tmp/kola -E ~/src/github/ostreedev/ostree/ 'ext.ostree.*' ``` NOTE: This *does not* drop ostree binaries into the target. See: coreos/coreos-assembler#1252 (comment) This drops our dependency on Python in the installed tests, and also fixes a few bugs that came up. I disabled the `itest-bare-user-root.sh` one because it's entangled with the shell script infrastructure for the unit tests.

So we can use `-E` in CI, see coreos/coreos-assembler#1252

Previously we made an effort to use the [Fedora Standard Test Interface](https://docs.fedoraproject.org/en-US/ci/standard-test-interface/). This effort was not very successful; the primary thing that it really died on is Ansible just didn't support rebooting very well. I think that's since gotten better, but even then, Ansible wasn't the best thing for a test framework for us anyways. In the meantime Fedora CoreOS happened emphasizing Ignition and not "post-hoc reconciliation" models like Ansible over ssh. And, [coreos-assembler](https://github.com/coreos/coreos-assembler) happened too. Furthermore, we really need to test OSTree's interaction with Ignition as we've invented several special things there. Then most recently, I've been working on having cosa/kola support running externally defined tests: coreos/coreos-assembler#1215 There's a lot of things to clean up after this but at least this works for me: ``` $ cd /srv/fcos $ cosa kola run -- --parallel 4 --output-dir tmp/kola -E ~/src/github/ostreedev/ostree/ 'ext.ostree.*' ``` NOTE: This *does not* drop ostree binaries into the target. See: coreos/coreos-assembler#1252 (comment) This drops our dependency on Python in the installed tests, and also fixes a few bugs that came up. I disabled the `itest-bare-user-root.sh` one because it's entangled with the shell script infrastructure for the unit tests.

So we can use `-E` in CI, see coreos/coreos-assembler#1252

Pairs with coreos/coreos-assembler#1252 The motivation here is to get the ball rolling for (where it makes sense) to have a place to add tests corresponding to changes in this repository.

There's a lot going on here. First, this is intended to run nicely as part of the new [cosa/kola ext-tests](coreos/coreos-assembler#1252). With Rust we can get one big static binary that we can upload, and include a webserver as part of the binary. This way we don't need to do the hack of running a container with Python or whatever. Now, what's even better about Rust for this is that it has macros, and specifically we are using [commandspec](https://github.com/tcr/commandspec/) which allows us to "inline" shell script. I think the macros could be even better, but this shows how we can intermix pure Rust code along with using shell safely enough. We're using my fork of commandspec because the upstream hasn't merged [a few PRs](https://github.com/tcr/commandspec/pulls?q=is%3Apr+author%3Acgwalters+). This model is intended to replace *both* some of our `make check` tests as well. Oh, and this takes the obvious step of using the Rust OSTree bindings as part of our tests. Currently the "commandspec tests" and "API tests" are separate, but nothing stops us from intermixing them if we wanted. I haven't yet tried to write destructive tests with this but I think it will go well.

Pairs with coreos/coreos-assembler#1252 The motivation here is to get the ball rolling for (where it makes sense) to have a place to add tests corresponding to changes in this repository.

There's a lot going on here. First, this is intended to run nicely as part of the new [cosa/kola ext-tests](coreos/coreos-assembler#1252). With Rust we can get one big static binary that we can upload, and include a webserver as part of the binary. This way we don't need to do the hack of running a container with Python or whatever. Now, what's even better about Rust for this is that it has macros, and specifically we are using [commandspec](https://github.com/tcr/commandspec/) which allows us to "inline" shell script. I think the macros could be even better, but this shows how we can intermix pure Rust code along with using shell safely enough. We're using my fork of commandspec because the upstream hasn't merged [a few PRs](https://github.com/tcr/commandspec/pulls?q=is%3Apr+author%3Acgwalters+). This model is intended to replace *both* some of our `make check` tests as well. Oh, and this takes the obvious step of using the Rust OSTree bindings as part of our tests. Currently the "commandspec tests" and "API tests" are separate, but nothing stops us from intermixing them if we wanted. I haven't yet tried to write destructive tests with this but I think it will go well.

openshift-ci-robot requested review from ashcrow and mike-nguyen March 16, 2020 17:54

openshift-ci-robot added the approved label Mar 16, 2020

ashcrow reviewed Mar 16, 2020

View reviewed changes

mantle/kola/harness.go Show resolved Hide resolved

mantle/kola/harness.go Show resolved Hide resolved

mantle/kola/README-kola-ext.md Show resolved Hide resolved

ashcrow requested a review from arithx March 16, 2020 18:38

cgwalters force-pushed the external-test-dirs branch from 5e6f856 to 3c790da Compare March 16, 2020 21:15

cgwalters mentioned this pull request Mar 16, 2020

tests/kola: New basic test using kola external tests coreos/fedora-coreos-config#305

Merged

arithx approved these changes Mar 16, 2020

View reviewed changes

mantle/kola/harness.go Outdated Show resolved Hide resolved

mantle/kola/register/register.go Outdated Show resolved Hide resolved

cgwalters changed the title ~~kola: Replace run-ext-bin with kola run -E/--exttest~~ WIP: kola: Replace run-ext-bin with kola run -E/--exttest Mar 17, 2020

openshift-ci-robot added the do-not-merge/work-in-progress label Mar 17, 2020

cgwalters force-pushed the external-test-dirs branch from 334da8c to f8bb1b3 Compare March 17, 2020 19:24

cgwalters changed the title ~~WIP: kola: Replace run-ext-bin with kola run -E/--exttest~~ kola: Replace run-ext-bin with kola run -E/--exttest Mar 17, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress label Mar 17, 2020

cgwalters force-pushed the external-test-dirs branch from f8bb1b3 to be271a4 Compare March 17, 2020 22:13

cgwalters commented Mar 17, 2020

View reviewed changes

ashcrow reviewed Mar 18, 2020

View reviewed changes

ashcrow approved these changes Mar 18, 2020

View reviewed changes

cgwalters mentioned this pull request Mar 18, 2020

tests: Rework tests/installed → tests/kola ostreedev/ostree#2025

Merged

cgwalters added a commit to cgwalters/coreos-ci-lib that referenced this pull request Mar 19, 2020

fcosKola: Add argv parameter

cff7c77

So we can use `-E` in CI, see coreos/coreos-assembler#1252

cgwalters mentioned this pull request Mar 19, 2020

fcosKola: Automatically pick up tests/kola coreos/coreos-ci-lib#16

Merged

cgwalters added a commit to cgwalters/coreos-ci-lib that referenced this pull request Mar 19, 2020

fcosKola: Add argv parameter

a34f1de

So we can use `-E` in CI, see coreos/coreos-assembler#1252

cgwalters added a commit to cgwalters/coreos-ci-lib that referenced this pull request Mar 19, 2020

fcosKola: Automatically pick up tests/kola

ea398d4

So we can use `-E` in CI, see coreos/coreos-assembler#1252

cgwalters added a commit to cgwalters/coreos-ci-lib that referenced this pull request Mar 19, 2020

fcosKola: Automatically pick up tests/kola

e94afa5

So we can use `-E` in CI, see coreos/coreos-assembler#1252

cgwalters mentioned this pull request Mar 20, 2020

kola/tests/ignition: add test for systemd instantiated units #1266

Merged

jlebon pushed a commit to cgwalters/coreos-ci-lib that referenced this pull request Mar 20, 2020

fcosKola: Automatically pick up tests/kola

bda07b8

So we can use `-E` in CI, see coreos/coreos-assembler#1252

jlebon pushed a commit to coreos/coreos-ci-lib that referenced this pull request Mar 20, 2020

fcosKola: Automatically pick up tests/kola

be2b202

So we can use `-E` in CI, see coreos/coreos-assembler#1252

cgwalters mentioned this pull request Mar 23, 2020

running upstream project tests via kola? #1176

Closed

cgwalters mentioned this pull request Mar 30, 2020

Add new Rust-based tests ostreedev/ostree#2048

Merged

cgwalters mentioned this pull request Apr 5, 2020

reworking test suite: dropping gnome-installed-tests ostreedev/ostree#2056

Open

cgwalters mentioned this pull request Apr 30, 2020

tests/manual: update the net test script to support RHCOS coreos/fedora-coreos-config#369

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kola: Replace run-ext-bin with `kola run -E/--exttest` #1252

kola: Replace run-ext-bin with `kola run -E/--exttest` #1252

cgwalters commented Mar 16, 2020

cgwalters commented Mar 16, 2020

ashcrow left a comment

arithx left a comment

cgwalters commented Mar 16, 2020

cgwalters commented Mar 17, 2020

arithx commented Mar 17, 2020

cgwalters commented Mar 17, 2020

cgwalters commented Mar 17, 2020

cgwalters commented Mar 17, 2020

cgwalters commented Mar 17, 2020

cgwalters Mar 17, 2020

ashcrow Mar 18, 2020

cgwalters commented Mar 18, 2020

kola: Replace run-ext-bin with kola run -E/--exttest #1252

kola: Replace run-ext-bin with kola run -E/--exttest #1252

Conversation

cgwalters commented Mar 16, 2020

cgwalters commented Mar 16, 2020

ashcrow left a comment

Choose a reason for hiding this comment

arithx left a comment

Choose a reason for hiding this comment

cgwalters commented Mar 16, 2020

cgwalters commented Mar 17, 2020

arithx commented Mar 17, 2020

cgwalters commented Mar 17, 2020

cgwalters commented Mar 17, 2020

cgwalters commented Mar 17, 2020

cgwalters commented Mar 17, 2020

cgwalters Mar 17, 2020

Choose a reason for hiding this comment

ashcrow Mar 18, 2020

Choose a reason for hiding this comment

cgwalters commented Mar 18, 2020

kola: Replace run-ext-bin with `kola run -E/--exttest` #1252

kola: Replace run-ext-bin with `kola run -E/--exttest` #1252