Usability improvements - #32
Draft
nmanthey wants to merge 16 commits into
Draft
Conversation
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>
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>
Add a pgbench-kernel-regression VM test that installs a base kernel, runs a PostgreSQL 16 pgbench read-only and read-write suite, reboots into a second (tip) kernel, re-runs the suite, and emits benchmark-base-*.csv and benchmark-tip-*.csv for the regression analyzer to compare (read-only/read-write TPS and average latency). Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
When many VMs are launched at once (min_count x tests), or several pipeline runs execute concurrently, the burst of RunInstances and SendCommand calls can hit EC2/SSM API throttling (RequestLimitExceeded, InsufficientInstanceCapacity, ...). boto3's adaptive retry mode does not always absorb these, so a single throttled call could fail an otherwise healthy VM — or, when it happened during spawn, take down the whole run. Add an application-level bounded retry with exponential backoff + full jitter (_call_with_retries) around run_instances and send_command, and stagger thread starts by a small configurable delay (PULLAB_VM_SPAWN_STAGGER_SEC, default 1s) so the initial API burst is spread out instead of arriving in the same instant. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
VMLauncher(full_vm_config) was constructed outside the per-VM try block. Its __init__ resolves the AMI via SSM and creates boto3 clients, so a transient error there (e.g. an SSM throttle while many threads construct at once) escaped the thread's handler: the VM was neither recorded as failed nor cleaned up, and in the worst case the container aborted with a partial result. Move construction inside the try so any per-VM error — construction, spawn, SSM command, or result check — is caught, logged with a traceback, and recorded as a single failed VM. Guard cleanup() for the case where construction never produced a launcher. One VM's failure now leaves the other VMs to finish and be reported normally. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
All Fargate orchestrator tasks logged into /ecs/<family> with the same hardcoded "ecs" stream prefix, so concurrent runs interleaved under indistinguishable stream names (only the task-id suffix differed). Make awslogs-stream-prefix configurable in the task definition (log_stream_prefix, default "ecs") and derive it from the run's test_id in aws_auth when building the task config. Concurrent runs now get human/tool-distinguishable stream prefixes like '<test_id>/<container>/<task-id>' in the shared log group. Add unit coverage for the default and a custom prefix. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
The analyzer only reported regressions; a significant, meaningful change in the *better* direction was silently lumped into 'no regressions'. Classify each metric symmetrically: a change that is both statistically significant (t-test or U-test p < 0.05) and meaningful (|Cohen's d| >= 0.5) is a regression if it moved the worse way and an improvement if it moved the better way (direction depends on more_is_better). Add is_improvement on MetricComparison, improvements/has_improvement on TestBenchmarkResult, and tests_with_improvement / improvement_test_names on the summary. The summary log now prints an 'IMPROVEMENTS DETECTED' section and an 'Improvements found' footer count. Add unit tests for regression/improvement/noise classification in both directions, and document the new fields in the README. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
The task wait loop tails the VM console log group to detect crashes and progress. If that CloudWatch call fails transiently — most commonly an ExpiredTokenException while the credential provider refreshes — the old code fell through to the 'no events' branch, so the hang timer kept advancing and could false-positive kill a healthy run (observed during a long run when the operator's credentials briefly expired). Catch ClientError around the log fetch: refresh the logs client and, for that cycle, treat it as activity (advance last_event_seen_at) so the hang timer is paused while retrieval is failing. Real hangs — where log retrieval succeeds but returns no new events — still trip the threshold. get_task_status already refreshes/handles ExpiredTokenException, so the status path was already covered. Add a test that a persistently failing log fetch does not trigger the hang timeout (the run instead stops on the overall timeout). Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
aws run only kept the orchestrator logs under logs/run_*/; the benchmark CSVs and per-VM outputs stayed in S3 (read into memory by the analyzer and discarded locally). Getting them on disk required a separate 'aws analyze --output-dir' pass. Add an optional --results-dir DIR to 'aws run'. After the pipeline finishes, download every object under the run's S3 prefix into DIR/<run_prefix>/, preserving the bucket key structure (benchmark CSVs, result.txt, console logs, summary/artifacts). Best-effort: logs and continues on error, and is a no-op when no --results-dir is given. Add unit tests for structure preservation, folder-placeholder skipping, and the incomplete-storage no-op, plus a README note. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
bhcopeland
self-requested a review
August 26, 2026 09:09
bhcopeland
marked this pull request as draft
August 26, 2026 09:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The changes in this branch help to better work with the results of the KernelCI runner interactively, and improve runner stability.