image scp: add --compression-format and --compression-level - #29395
Open
scallaway wants to merge 6 commits into
Open
image scp: add --compression-format and --compression-level#29395scallaway wants to merge 6 commits into
scallaway wants to merge 6 commits into
Conversation
Three small things in one place, all groundwork rather than behaviour: The path SaveToRemote gets back from the remote mktemp still carries the trailing newline ssh.Exec hands over with the rest of the raw output. That is harmless while the path is only ever the last thing on a command line, but it is a trap for anything that appends to it. Trim it. The host, identity, port and user were restated in full for every command. State them once and let each command copy the value and add its own argv. Removing a file on the far end had one caller and was about to have more, so give it a name. It also gains -f, since a caller cleaning up after a failure cannot know which of the paths it is removing were created. Signed-off-by: Scott Callaway <github@scottcallaway.co.uk>
Groundwork for compressing the transfer archive: the options themselves, the set of algorithms that may be requested, and the check ExecuteTransfer runs before it does anything else. Nothing acts on them yet. The set of formats is deliberately narrower than what c/image knows. Every entry has to satisfy three things: podman load has to detect and decompress it from the stream alone, c/image has to be able to compress it (it only decompresses bzip2 and xz), and a command line compressor of the same name has to exist for the case where the archive is produced on a remote host. gzip and zstd qualify, and they match the vocabulary --compression-format already uses on podman push. One table drives the accepted formats, their level ranges, and the list offered on the command line, so there is nothing to keep in sync. A transfer between two users on the same machine never crosses a network, so a requested format is reported as ignored there rather than refused. The validation is worded without flag names because it also runs on the API path, where the caller never passed a flag. Signed-off-by: Scott Callaway <github@scottcallaway.co.uk>
podman save writes docker-archive layers uncompressed, so podman image scp puts the whole archive on the wire as is. Compressing it first takes a docker-archive to around half its size or less, which is why people work around this today with podman save | zstd, a manual copy and podman load on the far side. An oci-archive keeps whatever compression its layers already have, so there is little to gain there; the man page records the difference. When the archive is produced locally it can be compressed on the way out: the c/image compression package wraps the file as it is streamed into the ssh connection feeding the remote podman image load. No second temporary file, nothing buffered in full. Nothing is needed on the destination. podman load detects the compression from the stream and decompresses it itself, for docker-archive via c/image's AutoDecompress and for oci-archive via c/storage's DecompressStream. Both are covered, since which one runs depends on --format. Signed-off-by: Scott Callaway <github@scottcallaway.co.uk>
When the source is a remote host the archive is produced there, so it has to be compressed there too: compressing after copying it down would mean the uncompressed archive had already crossed the network, which is the cost this is meant to avoid. The only thing we can do on that host is run a command, so the matching compressor is invoked over ssh between the save and the copy. That is also why the set of formats is limited to algorithms available as a command of the same name. Two details worth stating. A shell reports 127 when it cannot find the command, which is worth reporting plainly as a host without the compressor installed; anything else, a failure to connect included, must not be reported that way, and a probe beforehand cannot make that distinction without also costing an extra connection. And the compressor removes its input only once it succeeds and may have written part of its output before giving up, so a failure cleans up both paths. Signed-off-by: Scott Callaway <github@scottcallaway.co.uk>
Expose the compression the transfer already knows how to do, and document what each option means on each path. --compression-format takes gzip or zstd, matching the vocabulary --compression-format already uses on podman push, minus the algorithms this cannot produce or detect. --compression-level takes the level, and is rejected without a format to apply it to rather than being silently ignored. The level needs one caveat spelling out in the man page. A remote source passes it to the command line compressor, where every value is distinct. A local source compresses through c/image, which groups zstd levels into four bands, so 10 and above are the same there. The accepted zstd range also stops at 19 rather than podman push's 20, because the command line compressor needs --ultra past that. The flags are validated before the engine is reached, so podman --remote reports a bad combination without a round trip; the transfer validates again for callers arriving over the API. Fixes: podman-container-tools#23192 Signed-off-by: Scott Callaway <github@scottcallaway.co.uk>
The tunnel engine builds its own ScpOptions, so without this the flags parse fine under podman --remote and are then dropped, transferring uncompressed with no indication that anything was ignored. Carry both options through the bindings to the libpod ImageScp handler, which hands them to ExecuteTransfer the same way the local path does, and document them on the endpoint. This is also the point at which the transfer's own validation becomes reachable over HTTP, so map it accordingly: a rejected format or level is the caller's mistake and answers 400, not the 500 every error from the transfer used to produce. Signed-off-by: Scott Callaway <github@scottcallaway.co.uk>
github-actions
Bot
added
the
kind/api-change
Contributor
|
Can you add a release note along the lines of “podman image scp now supports --compression-format and --compression-level” |
Contributor
|
Changes LGTM overall, but definitely want other eyeballs on this one. |
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
2 participants
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.
podman savewrites docker-archive layers uncompressed, sopodman image scpputs the whole archive on the wire as is. Today people work around that withpodman save | zstd, a manual copy andpodman loadon the far side.This adds
--compression-format(gzip, zstd) and--compression-levelso the transfer does it itself. Nothing is needed on the destination:podman loaddetects the compression from the stream and decompresses it.Compression happens on whichever host produces the archive, so only compressed bytes cross the network:
podman image load, via the c/image compression package. No second temporary file, nothing buffered in full.Measured on
quay.io/libpod/alpine, docker-archive goes from 5.85 MB to 2.72 MB with zstd. An oci-archive keeps whatever compression its layers already have, so there the gain is negligible (~98%); the man page records the difference.Testing
Unit tests in
pkg/domain/utils, 4 e2e specs, 3 apiv2 cases. Also exercised manually over ssh in all three directions with both--formatvalues: with the compressor removed from the source host the transfer fails withrequires the "zstd" command on the remote host, confirming it is genuinely invoked there.Fixes: #23192