fix(bench): float the tool and image versions, record what each run used - #118
Merged
Conversation
A benchmark run is a point in time, not a fixed rig. Pinning the competitor
tools measured ocync against versions nobody runs, and pinning the AMI did
the same for the OS. Both float again, and the run record carries what was
actually used, which is what makes two runs comparable.
The record was the missing half. bench/results/{registry}.json named neither
the image nor skopeo, which moves every byte dregsy is credited with, nor the
credential helper both Go tools authenticate through. It now carries all
three: the image id from instance metadata, and relay versions read from the
module stamped into each binary, which is the only version several of them
carry since go install sets none of a release build's ldflags.
ami is ForceNew and AWS advances the AL2023 parameter as it publishes, so
ignore_ami_changes keeps an unrelated apply from destroying a running
instance and its results. New images arrive on deliberate replacement. The
module resolves the same parameter from its own default, so the data sources
that did it by hand are gone.
Two resolution details survive from the pinning work because they were bugs
rather than pins. skopeo installs from go.podman.io/skopeo, since the
github.com path serves tags whose go.mod declares the new path and fails on a
mismatch. The version probe distinguishes "reported nothing" from the string
"unknown" so a tool that logs on a zero exit cannot have that line recorded
as its version.
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
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.
A benchmark run is a point in time, not a fixed rig. Pinning the competitor tools measured ocync against versions nobody runs, and pinning the AMI did the same for the OS. Both float again.
The record was the missing half, and it predates the pinning.
bench/results/{registry}.jsonnamed neither the image, nor skopeo, which moves every byte dregsy is credited with, nor the credential helper both Go tools authenticate to ECR through. It now carries all three: the image id from instance metadata, and relay versions read from the module stamped into each binary. That is the only version several of these carry, sincego installsets none of a release build's ldflags, so the credential helper reportsdevelopmentand dregsy has no version flag at all.amiis ForceNew and AWS advances the AL2023 parameter as it publishes, soignore_ami_changesis set. Without it the next apply for any unrelated reason destroys a running instance and its results, and re-readingdata.http.my_ipevery plan means an operator IP change alone is such a reason. New images arrive on deliberate replacement. The ec2 module resolves the same SSM parameter from its own default, so the data sources doing it by hand are gone, which also drops two API calls per plan.Two things survive from the pinning work because they were bugs rather than pins. skopeo installs from
go.podman.io/skopeo, since thegithub.compath serves tags whosego.moddeclares the new path and fails on a mismatch. And the version probe now distinguishes "reported nothing" from the literal stringunknown, so a tool that logs a line on a zero exit cannot have that line recorded as its version, which is the bug #116 fixed and a sentinel comparison would have re-opened.Worth knowing rather than assuming, and now written down in
bench/CLAUDE.md:@latestresolves once, at instance creation, because thego installcalls live in user-data andbench-remoteonly rebuilds ocync. An instance built in March runs March's dregsy in September. Benchmarking against newer competitor releases means replacing the instance, and the run record is what says which versions a given result came from. Two related traps are documented there too:@latestnever crosses a major version boundary under semantic import versioning, and dregsy tags without avprefix so it resolves to a pseudo-version off the default branch rather than a release.CI is the gate. Locally
cargo fmt,clippy -D warnings,check --no-default-features --features non-fips,cargo deny,terraform fmt,terraform validateand 147 xtask tests pass. The testcontainers-backed integration tests inocync-distributionfail on my machine withWaitContainer(StartupTimeout), which is container startup rather than an assertion, and this diff cannot reach that crate.