Pull missing Docker images, and explain the file_hash check set - #108
Open
bgrewell wants to merge 3 commits into
Open
Pull missing Docker images, and explain the file_hash check set#108bgrewell wants to merge 3 commits into
bgrewell wants to merge 3 commits into
Conversation
A docker node referencing an image the daemon did not hold failed at container creation with the daemon's "No such image", which describes the symptom rather than the cause. The image is now fetched during node setup. Two cases are deliberately left alone: an image already present is not re-fetched, so a suite keeps the copy it has instead of silently moving to a newer build behind the same tag; and an image the suite builds through docker.images is never pulled, having no registry to pull from. Pull progress is discarded rather than printed — node setup renders through a spinner that raw daemon output would overwrite — but a failed pull names the image it could not fetch. Separately, file_hash rejected every check outside md5/sha1/sha256 while calling them unknown hash algorithms, which misdescribes the cause. The restriction is right: the check runs sha256sum and friends, so its stdout is the checksum line rather than the file's contents, and a contains: there would read like a content assertion while matching "<digest> <path>". The message now says that, and points at file_content.
A matching digest already proves the file's contents byte for byte, so a further check on the same file can only be redundant or contradictory. That is a plainer justification than the one about what stdout happens to contain, which is now the secondary point rather than the whole argument.
The message still reasoned from what stdout contains while the code comment and docs had moved to the plainer point: a matching digest already proves the contents, so a second check on the same file cannot add anything.
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.
Closes #94. Also removes the last "documented wart" from the docs sweep.
Docker image pull
A docker node referencing an image the daemon did not hold failed at container creation with the daemon's
No such image— the symptom, not the cause. The image is now fetched during node setup.Two cases are deliberately left alone:
docker.imagesis never pulled. It has no registry to pull from, and attempting one would fail on an image that is about to exist.Pull progress is discarded rather than printed: node setup renders through a spinner that raw daemon output would scribble over. A failed pull names the image (
could not pull image <ref>: ...) instead of surfacing later as a creation failure.#94 — file_hash's check set
You asked what the logic was, so to state it plainly:
file_hashrunsmd5sum/sha1sum/sha256sumon the node and compares the digest in the output against the one you give.filenameplus one or more ofmd5/sha1/sha256— which is exactly the "specify an algorithm and the expected value" model you'd expect. That part was never in question.The oddity was what happened when you added anything else to
evaluate. It was rejected as an "unknown hash algorithm", which is wrong for a name likecontainsthat is a perfectly real check elsewhere.I kept the restriction and fixed the message, because the restriction turns out to be protective rather than arbitrary: the result's stdout is the checksum line, not the file's contents. So
contains: "hello"would look like a content assertion while actually matching against"<digest> <path>"— a false green of exactly the kind this series has been removing. Refusing it is better than letting it mean something the reader would not expect.If you'd rather the generic checks were available, the honest way is to make
file_hashexpose the file's contents as stdout instead of the digest line — a larger change, and it would break the existing digest comparison. Happy to do it if you disagree with the call.Verification
Same caveat as #104: no Docker daemon is reachable from this machine or the datacenter hosts, so the pull path is tested against a recording client rather than a live registry — present/absent inspection, that a pull happens only when absent, that suite-built images are skipped (both
name:tagand barenameforms), and that a pull failure is reported against the image.What I would want confirmed on a real daemon: that a first run with
image: nginx:alpineand no local copy now succeeds, and thatexamples/docker/docker.yamlstill works given its images are suite-built and must not be pulled.Build, vet, gofmt, and the full suite are green; the docs site builds. The
Note: DART does not pull Docker imagesparagraph is gone, replaced by what it does now.