Kola: update to handle torcx deprecation / docker and containerd sysext #465

t-lo · 2023-10-13T16:10:05Z

This PR prepares kola for torcx deprecation. It is a prerequisite for flatcar/scripts#1216 .

A new "sysext" test module has been introduced; the existing sysext.go has been moved there from the systemd module.
- two additional tests for disabling the docker and containerd sysexts have been added there, too.
The "torcx" test module has been removed entirely - it contained a single test (explicitly enabling docker) which needed to be extended to cover sysext docker. That test has been moved to docker/enable_docker.go.
Torcx tests are made to run conditionally and will run only for major version below a certain threshold (currently up to version 3745 but this will need to be updated depending on when Contrib: Deprecate torcx, ship containerd / docker as sysexts scripts#1216 gets merged).
New sysext tests run for major versions greater than 3746 (same conditions apply)
A number of tests have been amended to better handle sysext environments (some implicit assumptions don't hold true anymore): kola/tests/bpf/bpf.go and kola/tests/systemd/journald.go.
kola/cluster/cluster.go has been extended to print SSH commands before execution (log level INFO) and command output after execution (log level DEBUG).

How to use

build the container: docker build -t my-mantle-container .
Check out the torcx deprecation WIP branch https://github.com/flatcar/scripts/tree/contrib/torcx-deprecation-docker-sysext
build packages, image, and qemu_uefi VM
edit sdk_container/.repo/manifests/mantle-container and put in my-mantle-container
Run ./run_local_test.sh

Note that the sysext.custom-oem test may still fail - it tries to download a devcontainer which might not exist for your particular image version.

Testing done

Built a mantle container and ran the full suite of tests locally (see "How to use" above) on an AMD64 OS image built from the latest torcx-deprecation-docker-sysext branch.

This change updates kola to handle torcx deprecation. Newer releases of Flatcar won't ship torcx anymore, while current stable and LTS will still use it. Kola tests have been updated to conditionally run torcx tests based on the OS release version. Also, first basic sysext tests have been added in a separate kola module. Currently, tests only focus on disabling sysexts. Sysext tests formerly residing in the systemd module have been moved to the new sysext module. Furthermore, the docker enable test has been generalised to also work with syysext and moved from the torcx module to the docker module.

This change adds a logger to kola/cluster which is used to log SSH commands (at INFO level) and stdout and stderr of the command execution (at DEBUG level) to the console kola has been started on. This is useful for understanding test failures from console output alone. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

This change adapts the bpf execsnoop test to work with sysext - the bcc container logs differ a bit when run on sysext docker. Also, the test is made more robust as docker log line parsing is now wrapped into a retry loop instead of being one-shot. Lastly, the change removes the additional c.run() wrapper around the test function. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

Journal filtering for user units appears to be racy / flakey in Flatcar; sometimes, journalctl --user --unit=... works, and sometimes it does not. This change updates the user unit journal test to not filter by unit but instead read the full user journal. Additionally, a retry loop is added around log retrieval to avoid potential race conditions. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

kola/tests/docker/torcx_manifest_pkgs.go

kola/tests/sysext/disable_containerd.go

pothos · 2023-10-13T17:21:30Z

kola/tests/sysext/disable_containerd.go

+	output := c.MustSSH(m,
+		`test ! -f /usr/bin/containerd || echo "ERROR"`)
+	if string(output) == "ERROR" {
+		c.Errorf("/usr/bin/containerd exists even when sysext is disabled!")
+	}


The test would be more robust by asserting what we expect instead of what we don't expect.
I think the easiest way would be:

Suggested change

output := c.MustSSH(m,

`test ! -f /usr/bin/containerd || echo "ERROR"`)

if string(output) == "ERROR" {

c.Errorf("/usr/bin/containerd exists even when sysext is disabled!")

}

# Assert that the containerd binary does not exist

_ = c.MustSSH(m, `test ! -f /usr/bin/containerd`)

I disagree - the suggestion makes the command fail instead of creating a meaningful test outcome. This would imply the test itself is broken.

In general, tests should end in one of 3 ways:

The test succeeded, signalling the build is healthy (containerd is not present).

The test failed, signalling an issue with the build (containerd is present).

The test is broken, signalling an issue with the test suite (either "test" or "echo" are broken, or SSH failed.)

I want to communicate 2., not 3.

Yes, it makes sense to distinguish that. The test is still not robust because it checks for errors explicitly. What you can change is to use && echo SUCCESS and if string(output) != "SUCCESS" { to handle more things that can get wrong.

I'll reverse the check and catch test returning 1 (if the binary is not present).

Done, please have another look.

What I meant was that == "ERROR" is too narrow and the test will silently break if it returns ERROR\n or the empty string. Therefore it's better to use something like != "SUCCESS".

kola/tests/sysext/disable_docker.go

t-lo · 2023-10-16T09:29:35Z

Addressed all feedback, thank you for your review @pothos !

Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

kola/tests/systemd/journald.go

kola/tests/bpf/bpf.go

kola/tests/sysext/disable_containerd.go

Co-authored-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>

Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

kola/tests/docker/enable_docker.go

tormath1

LGTM - a last nit-pick. Feel free to rebase or not your commits. Thanks for this work!

Co-authored-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>

This change bumps the image ref of the mantle container to ghcr.io/flatcar/mantle:git-20a2f8ffee8c8a1a042b1da99f0f59312110f285. This version includes 2 PRs (flatcar/mantle#465 and flatcar/mantle#466) which add support for sysext docker / torcx removal in the OS image. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

t-lo added 5 commits October 12, 2023 17:41

kola/tests/sysext: Use official Flatcar sysext names

1c39182

Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

t-lo requested a review from a team October 13, 2023 16:10

t-lo mentioned this pull request Oct 13, 2023

Contrib: Deprecate torcx, ship containerd / docker as sysexts flatcar/scripts#1216

Merged

6 tasks

kola torcx deprecation: fix gofmt errors

c48fb8b

Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

pothos reviewed Oct 13, 2023

View reviewed changes

kola/tests/docker/torcx_manifest_pkgs.go Outdated Show resolved Hide resolved

pothos reviewed Oct 13, 2023

View reviewed changes

kola/tests/sysext/disable_containerd.go Show resolved Hide resolved

pothos reviewed Oct 13, 2023

View reviewed changes

kola/tests/sysext/disable_docker.go Outdated Show resolved Hide resolved

t-lo force-pushed the t-lo/torcx-deprecation-tests-updates branch from 7deb834 to 2f40955 Compare October 16, 2023 09:29

t-lo requested a review from pothos October 16, 2023 09:29

kola torcx replacement: fix sysext.custom-docker, address PR feedback

e8d9164

Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

t-lo force-pushed the t-lo/torcx-deprecation-tests-updates branch from 2f40955 to e8d9164 Compare October 16, 2023 10:09

t-lo requested review from tormath1 and a team October 16, 2023 11:13

tormath1 reviewed Oct 16, 2023

View reviewed changes

kola/tests/systemd/journald.go Outdated Show resolved Hide resolved

kola/tests/systemd/journald.go Outdated Show resolved Hide resolved

kola/tests/bpf/bpf.go Outdated Show resolved Hide resolved

kola/tests/sysext/disable_containerd.go Outdated Show resolved Hide resolved

Apply suggestions from code review

16b5ee1

Co-authored-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>

t-lo requested a review from tormath1 October 16, 2023 13:52

t-lo force-pushed the t-lo/torcx-deprecation-tests-updates branch 2 times, most recently from ac393eb to f7a20fb Compare October 16, 2023 15:19

kola/ torcx removal: address feedback from PR review

1a30986

Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>

t-lo force-pushed the t-lo/torcx-deprecation-tests-updates branch from f7a20fb to 1a30986 Compare October 17, 2023 06:24

tormath1 reviewed Oct 17, 2023

View reviewed changes

kola/tests/docker/enable_docker.go Outdated Show resolved Hide resolved

tormath1 reviewed Oct 17, 2023

View reviewed changes

kola/tests/docker/enable_docker.go: Use cmd output SSH wrapper

6136eb8

Co-authored-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>

t-lo merged commit 313aa7e into flatcar-master Oct 17, 2023

t-lo deleted the t-lo/torcx-deprecation-tests-updates branch October 17, 2023 10:23

t-lo mentioned this pull request Oct 17, 2023

kola: bump torcx deprecation major release number #466

Merged

github-actions bot mentioned this pull request Nov 9, 2023

Monthly contributions report 2023-09-22 - 2023-10-21 flatcar/Flatcar#1244

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kola: update to handle torcx deprecation / docker and containerd sysext #465

Kola: update to handle torcx deprecation / docker and containerd sysext #465

t-lo commented Oct 13, 2023

pothos Oct 13, 2023

t-lo Oct 16, 2023

pothos Oct 16, 2023

t-lo Oct 16, 2023

t-lo Oct 16, 2023

pothos Oct 19, 2023

t-lo commented Oct 16, 2023

tormath1 left a comment

Kola: update to handle torcx deprecation / docker and containerd sysext #465

Kola: update to handle torcx deprecation / docker and containerd sysext #465

Conversation

t-lo commented Oct 13, 2023

How to use

Testing done

pothos Oct 13, 2023

Choose a reason for hiding this comment

t-lo Oct 16, 2023

Choose a reason for hiding this comment

pothos Oct 16, 2023

Choose a reason for hiding this comment

t-lo Oct 16, 2023

Choose a reason for hiding this comment

t-lo Oct 16, 2023

Choose a reason for hiding this comment

pothos Oct 19, 2023

Choose a reason for hiding this comment

t-lo commented Oct 16, 2023

tormath1 left a comment

Choose a reason for hiding this comment