containerd: properly pull images with containerd specific tools #8245
Conversation
k8s-ci-robot
added
kind/bug
floryut
added
kind/container-managers
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cristicalin, floryut The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
cristicalin
force-pushed
the
fix_ctr_pull_export
branch
from
4290d38 to
f879155
Compare
k8s-ci-robot
added
size/S
cristicalin
force-pushed
the
fix_ctr_pull_export
branch
from
f879155 to
b554015
Compare
cristicalin
changed the title
cristicalin
force-pushed
the
fix_ctr_pull_export
branch
from
b554015 to
84d1ec9
Compare
| @@ -29,7 +29,7 @@ | |||
|
|
|||
| - name: Set image save/load command for containerd | |||
| set_fact: | |||
| image_save_command: "{{ containerd_bin_dir }}/ctr -n k8s.io image export {{ image_path_final }} {{ image_reponame }}" | |||
| image_save_command: "{{ containerd_bin_dir }}/ctr -n k8s.io image export --platform linux/{{ image_arch }} {{ image_path_final }} {{ image_reponame }}" | |||
There was a problem hiding this comment.
I was wondering we would need to support Windows and this additional option could block it.
But Kubespray README.md clearly says Linux distributions are supported.
So it is fine to pass this option.
There was a problem hiding this comment.
At that point we would need to switch to --all-platforms instead if we wanted to support multi-platform or move to something like an iterative approach to downloading images where we iterate through all supported platforms and arch'es.
For now kubespray is Linux only so it should get the job done. I'm more concerned about the situation where we would want to support heterogeneous clusters with x86 and arm nodes. But our code needs further refinement for that as well.
| - name: prep_download | Set image pull/info command for containerd on localhost | ||
| set_fact: | ||
| image_info_command_on_localhost: "{{ bin_dir }}/ctr -n k8s.io images ls | awk '/application/ {print $1}' | grep -v ^sha | tr '\n' ','" | ||
| image_pull_command_on_localhost: "{{ bin_dir }}/ctr -n k8s.io images pull --platform linux/{{ image_arch }}" |
There was a problem hiding this comment.
image_info_command_on_localhost and image_pull_command_on_localhost are the same as image_info_command and image_pull_command.
We could define
image_info_command_on_localhost: "{{ image_info_command }}"
image_pull_command_on_localhost" "{{ image_pull_command }}"
but Kubespray already contained such duplicated definitions for crio.
It is fine to go with current change at this time.
There was a problem hiding this comment.
The download_on_localhost seems to be broken currently for anything but docker so this would need further attention in a subsequent PR(s).
|
There was an outage of the bot as https://kubernetes.slack.com/archives/C01672LSZL0/p1638225907066400?thread_ts=1638224302.065800&cid=C01672LSZL0 /lgtm |
k8s-ci-robot
added
the
lgtm
|
/retest |
What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR ensures we are pulling, exporting and importing the images for the target platform where we are dealing with multi-platform images such as nginx upstream image. This PR fixes issues observed in CI like: https://gitlab.com/kargo-ci/kubernetes-sigs-kubespray/-/jobs/1826926575#L3504
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
This might require rebasing some active PRs after the merge to fix broken CI runs.
Does this PR introduce a user-facing change?: