Fixup vm tests by nmanthey · Pull Request #30 · kernelci/pullab_cloud

nmanthey · 2026-08-18T12:03:49Z

I have been preparing adding a new benchmark test. While working on this, and testing existing benchmarks as reference, I came across a few problems with the current steps. This commit series refactors common test scripts into a library, and then improves the common functions to cover more use case for kernel names and similar.

Testing Done

I executed benchmarks and functional tests in my AWS account.

Ran the kernel A/B tests (unixbench-kernel-regression) end-to-end on 2 and 5 VMs, base 6.1.141 vs tip 6.1.150 on c8i.4xlarge
Verified the --allowerasing install path on a live 6.18 AMI.
make lint clean; 234 unit tests pass.

On a system with multiple python environments, we might want to run with a different version. Therefore, allow the script to select a python version. This change also helps when adding support to new python versions. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

bhcopeland · 2026-08-26T10:06:07Z

+# List available kernel RPMs from the shared S3 area.
+list_kernels_from_s3()
+{
+    S3_PATH="s3://${RESULTS_BUCKET}/${RUN_PREFIX}/shared/kernel-rpms/binary/${ARCH}/"


I see this is hardcoded to /binary/, but simple-source-reboot needs src/. I recommend some sort of subpath arg here.

simple-source-reboot/external_requirements.json has "kernel-rpms/binary": false, so that prefix is never staged for the run and the listing comes back empty. And download_kernel_rpm just below has the same hardcoded path, so parameterising only list_kernels_from_s3 won't be enough.

Yes. I will iterate. I did not test all tests with this change. Will do that.

bhcopeland · 2026-08-26T10:07:52Z

+        # syscall round-trips per second (lps), not a time. Do NOT invert it.
+        more_is_better = "true"

        printf "%s.%s,%s,%s,%s,%s,%s,%s,%s\n", benchmark_version, metric, unit, value, more_is_better, kernel_version, instance_id, instance_type, arch, arch


arch is here twice? (although not a changed line). Looks like simple-unixbench has it right in PR though

Good catch, will drop it.

bhcopeland · 2026-08-26T10:08:38Z

-        install_package gcc make tar || return 1
-    fi
-}
+# Extract unixbench


Duplicate comment

bhcopeland · 2026-08-26T10:12:06Z

(GitHub has anchored this to the top of the file. It's about the /^System Benchmarks Partial Index/ guard around line 88, and the identical one in simple-unixbench/common_lib.sh.)

I assume this only works because speed fs never runs dhry2reg, so the index is always partial? If the test list ever changes, the header becomes System Benchmarks Index Values and this stops matching. It won't error, it'll just silently start emitting garbage rows: in_results never resets, so the index table gets parsed by the same rule as the results section.

Worth matching both headers, in both copies, something like /^System Benchmarks (Partial Index|Index Values)/.

I will fix the documentation, and make the change more consistent. thanks.

bhcopeland · 2026-08-26T10:14:10Z

+            [ -n "$pkg" ] && { install_package "$pkg" || return 1; }
+        done <"$deps_file"
+    else
+        echo "ERROR: dependencies.txt not found" >&2


This used to fall back to installing gcc make tar. Fine if that's deliberate, since all three tests ship a dependencies.txt, just flagging it as a behaviour change for anything out of tree.

I will add the "default install" logic back.

bhcopeland · 2026-08-26T10:15:18Z

+    else
+        sudo dnf install -y "$kernel_rpm" 2>/dev/null \
+            || sudo yum localinstall -y "$kernel_rpm" 2>/dev/null \
+            || sudo dnf install -y --allowerasing "$kernel_rpm" 2>/dev/null \


--allowerasing could it take out the running kernel? If the new one then doesn't boot there's no fallback and we sit here until the timeout.

The running kernel is protected from this, so this extension should not cause a problem. In general, you are right: if something goes wrong during boot, we wait there until timeout.

bhcopeland · 2026-08-26T10:16:13Z

+FIPS-disable-before-reboot logic, and the `--allowerasing` cross-series install
+live only in the shared lib.
+
+## Status — migration complete


This says migration complete, but only unixbench-kernel-regression actually uses get_running_kernel_id. example-kernel-reboot-test and simple-source-reboot still call get_running_kernel, so it's 1 of 3. Not a problem while those two use kernels with different NVRs, but the doc is ahead of the code.

Also, is a TODO doc something we want tracked in the repo long term, or is it scratch for this series?

I will drop the doc from the git repository.

bhcopeland · 2026-08-26T10:18:00Z

        poll_interval = float(os.getenv("PULLAB_TASK_POLL_INTERVAL_SEC") or 30)
        log_interval = float(os.getenv("PULLAB_TASK_PROGRESS_LOG_SEC") or 120)
-        hang_threshold = float(os.getenv("PULLAB_TASK_HANG_THRESHOLD_SEC") or 600)
+        hang_threshold = float(os.getenv("PULLAB_TASK_HANG_THRESHOLD_SEC") or 1200)


Worth looking at PULLAB_TASK_WAIT_TIMEOUT_SEC alongside this, it's still 3600. If a benchmark can now legitimately go quiet for 20 minutes at a time, the overall hour starts to look tight. Separate fix rather than this PR though.

Let's monitor and decide which tests we really want to execute. If pgbench is quicker, we might be able to reduce this again.

bhcopeland · 2026-08-26T10:33:56Z

Nice cleanup, the shared lib is the right move and it makes the boot-hook work easier later.

One to fix before merge: the shared S3 helpers break simple-source-reboot (inline). Rest is minor.

Could you also update docs/08-analysis-regression.md? It still says System_Call_Overhead is the only metric forced to false, which is what this PR changes.

summarize_unixbench_log built the metric name from fields 1..NF-4, which pulled the numeric value, unit and part of the timing info into the metric name — producing malformed rows like 'Arithmetic_Test_(double)_385400605.9_lps' with a single sample each, instead of one 'Arithmetic_Test_(double)' metric aggregated across VMs. Use NF-6 for the metric name (matching unixbench-kernel-regression) so value=$(NF-5) and unit=$(NF-4) line up, and drop the index-section parsing that emitted duplicate/derived score metrics. Verified against sample UnixBench output: clean metric names, correct value/unit. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

UnixBench (and other CPU-bound benchmarks) run for many minutes with no new console output during the benchmark phase. The 600s hang-detection default tripped mid-run and killed all VMs as a false-positive stall. Raise the PULLAB_TASK_HANG_THRESHOLD_SEC default from 600 to 1200s so these benchmarks complete, while still catching genuine hangs within a reasonable window. The value remains env-overridable for lighter workloads that want faster detection. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

The kernel A/B tests each carried their own copy of the kernel install/reboot helpers, so any change had to be made in every test. Introduce vm-tests/lib/kernel_helpers.sh as the single home for that logic (environment validation, kernel RPM download/selection, install_kernel_rpm with grubby boot-entry management, and the get_running_kernel / assert_kernel_changed helpers). Each kernel test includes it via a kernel_helpers.sh symlink and sources it, keeping only its test-specific functions: - example-kernel-reboot-test: none (pure kernel install/reboot). - simple-source-reboot: source-RPM build helpers. - unixbench-kernel-regression: UnixBench prepare/run/summarize. The symlink is stored by the payload zip as real content, so the VM sees a normal file; no pipeline change is needed. Subsequent fixes to the kernel logic now land once in the shared lib. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

A kernel built with make binrpm-pkg and LOCALVERSION=-nogup has an RPM VERSION of 6.18.41_nogup (underscore) but installs vmlinuz-6.18.41-nogup (dash). The vmlinuz lookup missed the file because it used the RPM VERSION verbatim. Compute an alternate version string with underscores replaced by dashes, try both in the grubby --info and vmlinuz globs, and derive the kernel version for dracut/initramfs from the actual vmlinuz filename. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

AL2023 enables FIPS by default. A custom kernel built with make binrpm-pkg carries unsigned modules (e.g. ghash_clmulni_intel) that fail FIPS signature verification, causing a kernel panic reboot loop. Add fips=0 to the grubby boot-entry args and run fips-mode-setup --disable after installing the kernel, before the reboot, so unsigned modules load without panic. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

On an AL2023 AMI whose default kernel is a different series than the RPM under test (e.g. a 6.18 AMI installing a 6.1 kernel), the distro kernel<N>-tools package declares 'conflicts with kernel-uname-r < <N>', so a plain dnf/yum install is refused with 'conflicting requests'. Verified on a live 6.18 AMI: plain install fails, but 'dnf install --allowerasing' removes the conflicting kernel<N>-tools package and installs the requested kernel; both vmlinuz files remain in /boot so the target kernel boots normally. Add --allowerasing as the final fallback in install_kernel_rpm, making the kernel A/B tests robust to base-AMI kernel-series drift. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

UnixBench's 'System Call Overhead' result is reported in lps — the count of syscall iterations completed in a fixed time window — like every other first-section UnixBench metric. Despite the name 'Overhead', a higher value means more syscalls/sec, i.e. faster. Confirmed in the bundled UnixBench 6.0.0 source: * src/syscall.c: the test loops calling syscalls and increments a counter until a timer fires, then reports the count: iter = 0; wake_me(duration, report); while (1) { close(dup(fd)); syscall(SYS_getpid); getuid(); umask(022); iter++; } void report() { fprintf(stderr,"COUNT|%ld|1|lps\n", iter); } So 'iter' is iterations-per-run — higher is faster. * UnixBench/Run: the syscall test is a plain count metric ("repeat" => 'long', "options" => "10"), scored via the count-based branch 'product += log(count)' (Run:1194), not the time-inverted branch — bigger count yields a bigger index. The tests marked it more_is_better=false, which inverted its meaning: a genuine syscall-throughput improvement was flagged as a regression (and a real slowdown would have been mislabeled an improvement). Set more_is_better=true for all first-section UnixBench metrics in both unixbench-kernel-regression and simple-unixbench. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

Same-NVR kernel RPMs (e.g. two builds sharing 6.18.41-94.142.amzn2023.x86_64 but differing in compiler/toolchain) were skipped by dnf as "already installed", so the second kernel was never actually written and a kernel-regression test compared a kernel against itself. Add get_running_kernel_id() to the shared kernel_helpers.sh library: a composite build identity (uname -r | uname -v | vmlinuz sha256) that detects a real kernel switch even when two builds share an NVR. Make install_kernel_rpm force 'dnf reinstall' when the NVR is already present, and have assert_kernel_changed compare identities with a clearer message. Wire the unixbench-kernel-regression run scripts to use the new helper, and name their benchmark CSVs by $(uname -r) (clean/human-readable) rather than the composite identity, which would embed '|' and spaces. The logic lives once in vm-tests/lib/kernel_helpers.sh; tests only call the shared helper. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>

nmanthey · 2026-08-31T14:59:08Z

I improved the code and fixed a few more things.

I re-tested with a pair of kernels. I'll update the other PRs once this one is done.

bhcopeland self-requested a review August 26, 2026 09:34

bhcopeland reviewed Aug 26, 2026

View reviewed changes

nmanthey added 8 commits August 26, 2026 14:29

nmanthey force-pushed the fixup-vm-tests branch from 014a591 to 4321ae0 Compare August 31, 2026 14:56

Uh oh!

Conversation

nmanthey commented Aug 18, 2026

Testing Done

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bhcopeland commented Aug 26, 2026

Uh oh!

nmanthey commented Aug 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants