Skip to content

refactor: standardize container runtime interface to crictl across playbooks (Issue #10907) #13172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AdeshDeshmukh wants to merge 2 commits into
base: master
Choose a base branch
from
Open

refactor: standardize container runtime interface to crictl across playbooks (Issue #10907) #13172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd2343d
refactor: replace ctr with crictl for image listing in containerd han…
AdeshDeshmukh Apr 14, 2026
a5f195f
refactor: consolidate image pull/info commands to crictl (Phases 2-4 …
AdeshDeshmukh Apr 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion roles/container-engine/containerd/handlers/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
listen: Restart containerd

- name: Containerd | wait for containerd
command: "{{ containerd_bin_dir }}/ctr images ls -q"
command: "{{ bin_dir }}/crictl images"
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wait handler now runs crictl images which produces a full table and can be quite large; since the output isn’t used (only the rc is), prefer the quiet form (e.g., crictl images -q) or otherwise suppress output to reduce handler payload and log/memory overhead during retries.

Suggested change
command: "{{ bin_dir }}/crictl images"
command: "{{ bin_dir }}/crictl images -q"

Copilot uses AI. Check for mistakes.
register: containerd_ready
retries: 8
delay: 4
Expand Down
4 changes: 3 additions & 1 deletion roles/download/tasks/set_container_facts.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
image_save_command: "{{ bin_dir }}/nerdctl -n k8s.io image save -o {{ image_path_final }} {{ image_reponame }}"
image_load_command: "{{ bin_dir }}/nerdctl -n k8s.io image load < {{ image_path_final }}"
when: container_manager == 'containerd'
# Note: Using nerdctl for save/load as crictl doesn't support image save/load operations (not in CRI spec)

- name: Set image save/load command for crio
set_fact:
Expand All @@ -46,8 +47,9 @@

- name: Set image save/load command for containerd on localhost
set_fact:
image_save_command_on_localhost: "{{ containerd_bin_dir }}/ctr -n k8s.io image export --platform linux/{{ image_arch }} {{ image_path_cached }} {{ image_reponame }}"
image_save_command_on_localhost: "{{ bin_dir }}/nerdctl -n k8s.io image save -o {{ image_path_cached }} {{ image_reponame }}"
when: container_manager_on_localhost == 'containerd'
# Changed from ctr to nerdctl for consistency with main image save commands
Comment on lines +50 to +52
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching the localhost containerd image save from ctr image export --platform linux/{{ image_arch }} to nerdctl image save drops the explicit platform selection. This changes behavior for multi-arch images and for cases where image_arch is overridden, potentially saving the wrong architecture to the cache. Consider preserving platform selection (e.g., keep the ctr ... --platform path, or add an equivalent --platform option if supported by the chosen tool) so the cached artifact matches image_arch deterministically.

Suggested change
image_save_command_on_localhost: "{{ bin_dir }}/nerdctl -n k8s.io image save -o {{ image_path_cached }} {{ image_reponame }}"
when: container_manager_on_localhost == 'containerd'
# Changed from ctr to nerdctl for consistency with main image save commands
image_save_command_on_localhost: "{{ bin_dir }}/ctr -n k8s.io images export --platform linux/{{ image_arch }} {{ image_path_cached }} {{ image_reponame }}"
when: container_manager_on_localhost == 'containerd'
# Use ctr here to preserve explicit platform selection so the cached image matches image_arch deterministically

Copilot uses AI. Check for mistakes.
Comment on lines +50 to +52
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image_save_command_on_localhost for containerd now uses {{ bin_dir }}/nerdctl ..., but in download_localhost mode these commands run on the Ansible controller (localhost), where bin_dir/nerdctl is not installed by Kubespray roles. Previously this path used {{ containerd_bin_dir }}/ctr ..., which aligns with a plain containerd install. To avoid breaking download_localhost with container_manager_on_localhost == 'containerd', either keep using ctr for localhost, or add logic to ensure nerdctl is present on localhost (or make the tool configurable separately for localhost).

Suggested change
image_save_command_on_localhost: "{{ bin_dir }}/nerdctl -n k8s.io image save -o {{ image_path_cached }} {{ image_reponame }}"
when: container_manager_on_localhost == 'containerd'
# Changed from ctr to nerdctl for consistency with main image save commands
image_save_command_on_localhost: "{{ containerd_bin_dir }}/ctr -n k8s.io images export {{ image_path_cached }} {{ image_reponame }}"
when: container_manager_on_localhost == 'containerd'
# Use ctr on localhost because download_localhost commands run on the Ansible controller,
# where Kubespray does not ensure nerdctl is installed in {{ bin_dir }}

Copilot uses AI. Check for mistakes.

- name: Set image save/load command for crio on localhost
set_fact:
Expand Down
2 changes: 1 addition & 1 deletion roles/kubespray_defaults/defaults/main/download.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ nerdctl_image_pull_command: "{{ bin_dir }}/nerdctl -n k8s.io pull --quiet"
crictl_image_info_command: "{{ bin_dir }}/crictl images --verbose | awk -F ': ' '/RepoTags|RepoDigests/ {print $2}' | tr '\n' ','"
crictl_image_pull_command: "{{ bin_dir }}/crictl pull"

image_command_tool: "{%- if container_manager == 'containerd' -%}nerdctl{%- elif container_manager == 'crio' -%}crictl{%- else -%}{{ container_manager }}{%- endif -%}"
image_command_tool: "{%- if container_manager == 'containerd' -%}crictl{%- elif container_manager == 'crio' -%}crictl{%- else -%}{{ container_manager }}{%- endif -%}"
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image_command_tool now hard-codes both containerd and crio to crictl via two separate branches. Since they resolve to the same value, consider collapsing this into a single condition (e.g., if container_manager in ['containerd','crio']) to reduce duplication and make future extensions less error-prone.

Suggested change
image_command_tool: "{%- if container_manager == 'containerd' -%}crictl{%- elif container_manager == 'crio' -%}crictl{%- else -%}{{ container_manager }}{%- endif -%}"
image_command_tool: "{%- if container_manager in ['containerd', 'crio'] -%}crictl{%- else -%}{{ container_manager }}{%- endif -%}"

Copilot uses AI. Check for mistakes.
image_command_tool_on_localhost: "{{ image_command_tool }}"

image_pull_command: "{{ lookup('vars', image_command_tool + '_image_pull_command') }}"
Expand Down
Loading