Save cached container images with nerdctl instead of ctr on the control node - #13403
Save cached container images with nerdctl instead of ctr on the control node#13403Payback159 wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Payback159 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @Payback159. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Images cached on the Ansible control node are pulled with nerdctl (image_pull_command_on_localhost -> nerdctl_image_pull_command) but were exported with "ctr image export --platform linux/<arch>". nerdctl only fetches the blobs for the platform it pulled, while ctr export walks the manifest list and requires every descriptor of the requested platform to be present in the content store. With containerd 2.1+ the two paths no longer agree, so the export aborts with: ctr: failed to get reader: content digest sha256:<digest>: not found Use "nerdctl image save" instead, which is what the remote node path (image_save_command) already does, so the image is pulled and exported by the same tool.
9fca7e3 to
8c5e17e
Compare
|
/ok-to-test |
What type of PR is this?
/kind bug
What this PR does / why we need it:
When images are downloaded/cached on the Ansible control node (
download_run_once+download_localhost: true) withcontainer_manager_on_localhost == 'containerd', the image export step intermittently fails with:The root cause is that two different tools are used for the two halves of the same operation:
nerdctl:image_pull_command_on_localhostresolves tonerdctl_image_pull_command(
{{ bin_dir }}/nerdctl -n k8s.io pull --quiet) — seeroles/kubespray_defaults/defaults/main/download.yml.ctr:image_save_command_on_localhostwas{{ containerd_bin_dir }}/ctr -n k8s.io image export --platform linux/{{ image_arch }} ...— see
roles/download/tasks/set_container_facts.yml.nerdctl pullfetches only the blobs it needs for the requested platform and records that in the content store via the containerd transfer service.ctr image export --platform ...then walks the index/manifest list and demands every descriptor for the selected platform to be present as local content. With containerd 2.1+ the two paths no longer agree on which manifest/layer digests must be materialized for a multi-arch image, so the export aborts on the first descriptor that was never pulled — thecontent digest ...: not foundabove. This is exactly what is reported in the upstream issue.Kubespray already solves this correctly for remote nodes, where both the pull and the save are done with
nerdctl(image_save_commandusesnerdctl -n k8s.io image save). This PR simply applies the same, already-proven logic to the localhost path.Before
After
The image is now pulled and exported by the same tool, so the archive is written from exactly the content
nerdctlplaced in the store. The output path (image_path_cached), thek8s.ionamespace and the resulting Docker-archive format are unchanged, so the downstream copy to the nodes and thenerdctl image loadon the remote side keep working as before — the produced tarball is the same format the remoteimage_save_commandalready produces.Which issue(s) this PR fixes:
Fixes #13235
Special notes for your reviewer:
roles/download/tasks/set_container_facts.ymland only affectscontainer_manager_on_localhost == 'containerd'. Thedockerandcriolocalhost branches, and allremote-node branches, are untouched.
nerdctlis used because it is the tool Kubespray already installs and uses for containerd imagehandling; no new dependency is introduced.
{{ bin_dir }}is the same prefix used bynerdctl_image_pull_commandand by the remoteimage_save_command.--platform linux/{{ image_arch }}: the flag could not do what it appeared to do.The pull on the control node (
nerdctl pull, no--all-platforms/--platform) only ever fetches thecontrol node's own platform, so requesting a different platform at export time is precisely the
situation that triggers the reported error.
nerdctl image saveexports what was actually pulled, sothe common same-architecture case is byte-for-byte equivalent and the cross-architecture case goes from
"hard failure" to "exports the platform that was downloaded". Making the localhost cache genuinely
multi-arch (i.e.
nerdctl pull --all-platformscombined with a platform-aware save) is a separatefeature and intentionally out of scope here.
Testing done:
pre-commitpasses on the change (yamllint,misspell, collection build,ci-matrix, checksumordering, …). The
ansible-linthook reports ajinja[invalid]violation inroles/kubernetes/client/tasks/main.yml, but it reproduces identically on an unmodifiedmastercheckout, so it is pre-existing and unrelated to this change.
roles/download/tasks/set_container_facts.ymlin isolation withansible-playbookandasserted the resulting facts:
<bin_dir>/nerdctl -n k8s.io image save -o <download_cache_dir>/images/registry.k8s.io_pause_3.10.tar registry.k8s.io/pause:3.10repo@sha256:...) renders correctly for both the archive name and the image referenceimage_save_command(remote) andimage_save_command_on_localhostnow resolve to the same binarydockerandcriolocalhost branches are unchangedDoes this PR introduce a user-facing change?: