[Carry #2280] Add support for CDI devices to device flag #4170

djdongjin · 2025-04-27T01:15:50Z

Copilot

Pull Request Overview

This PR adds support for Container Device Interface (CDI) by introducing new functions to obtain CDI specification directories across supported platforms, updating configuration and CLI flags accordingly, and integrating CDI device injection into container creation.

Adds CDISpecDirs functions in the defaults package for Windows, Linux, FreeBSD, and Darwin.
Updates the config structure, CLI flags, and tests to support CDI device injection.
Integrates CDI devices handling into container run and create commands.

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pkg/defaults/defaults_windows.go	Adds a stub CDISpecDirs function returning an empty slice for Windows.
pkg/defaults/defaults_linux.go	Implements CDISpecDirs with distinct behavior based on rootless mode.
pkg/defaults/defaults_freebsd.go	Provides a CDISpecDirs function returning default CDI spec directories.
pkg/defaults/defaults_darwin.go	Provides a CDISpecDirs function returning default CDI spec directories.
pkg/config/config.go	Introduces a new field for CDI spec directories in the configuration.
pkg/cmd/container/run_cdi.go	Adds a function to configure CDI devices during container run.
pkg/cmd/container/create.go	Injects the CDI devices option into the container creation process.
pkg/api/types/container_types.go	Adds a CDIDevices field to the container create options structure.
docs/config.md	Documents the new --cdi-spec-dirs CLI flag and configuration.
cmd/nerdctl/testdata/cdi/vendor1.yaml	Provides an example CDI vendor file for testing purposes.
cmd/nerdctl/main.go	Introduces the CLI flag for CDI spec directories.
cmd/nerdctl/helpers/flagutil.go	Processes the new cdi-spec-dirs flag and integrates it into global options.
cmd/nerdctl/container/container_run_linux_test.go	Adds tests to verify the CDI device injection functionality.
cmd/nerdctl/container/container_create.go	Differentiates between standard devices and CDI devices during creation.
.github/workflows/test.yml	Temporarily enables CDI support in the Docker daemon for integration tests.

Files not reviewed (1)

go.mod: Language not supported

cmd/nerdctl/container/container_run_linux_test.go

pkg/defaults/defaults_linux.go

docs/config.md

AkihiroSuda · 2025-04-28T11:41:21Z

docs/config.md

@@ -47,6 +47,7 @@ experimental   = true
 | `host_gateway_ip`   | `--host-gateway-ip`                | `NERDCTL_HOST_GATEWAY_IP` | IP address that the special 'host-gateway' string in --add-host resolves to. Defaults to the IP address of the host. It has no effect without setting --add-host | Since 1.3.0      |
 | `bridge_ip`         | `--bridge-ip`                      | `NERDCTL_BRIDGE_IP`       | IP address for the default nerdctl bridge network, e.g., 10.1.100.1/24                                                                                           | Since 2.0.1      |
 | `kube_hide_dupe`    | `--kube-hide-dupe`                 |                           | Deduplicate images for Kubernetes with namespace k8s.io, no more redundant <none> ones are displayed    | Since 2.0.3      |
+| `cdi_spec_dirs`     | `--cdi-spec-dirs`                   |                          | The folders to use when searching for CDI specifications.    | Since 2.0.5 |


CDI needs explanation

apostasie

@djdongjin one minor nit on help

apostasie · 2025-04-28T16:36:23Z

cmd/nerdctl/main.go

@@ -186,6 +186,7 @@ func initRootCmdFlags(rootCmd *cobra.Command, tomlPath string) (*pflag.FlagSet,
 	helpers.AddPersistentStringFlag(rootCmd, "host-gateway-ip", nil, nil, nil, aliasToBeInherited, cfg.HostGatewayIP, "NERDCTL_HOST_GATEWAY_IP", "IP address that the special 'host-gateway' string in --add-host resolves to. Defaults to the IP address of the host. It has no effect without setting --add-host")
 	helpers.AddPersistentStringFlag(rootCmd, "bridge-ip", nil, nil, nil, aliasToBeInherited, cfg.BridgeIP, "NERDCTL_BRIDGE_IP", "IP address for the default nerdctl bridge network")
 	rootCmd.PersistentFlags().Bool("kube-hide-dupe", cfg.KubeHideDupe, "Deduplicate images for Kubernetes with namespace k8s.io")
+	rootCmd.PersistentFlags().StringSlice("cdi-spec-dirs", cfg.CDISpecDirs, "The directories to search for CDI spec files. Defaults to /etc/cdi,/var/run/cdi")


We do not mention the defaults for CNI for eg - maybe we can remove them here as well? (or alternatively, mention the rootless defaults as well)

perhaps another pr..

mikebrow · 2025-04-29T16:20:57Z

docs/config.md

@@ -47,6 +47,7 @@ experimental   = true
 | `host_gateway_ip`   | `--host-gateway-ip`                | `NERDCTL_HOST_GATEWAY_IP` | IP address that the special 'host-gateway' string in --add-host resolves to. Defaults to the IP address of the host. It has no effect without setting --add-host | Since 1.3.0      |
 | `bridge_ip`         | `--bridge-ip`                      | `NERDCTL_BRIDGE_IP`       | IP address for the default nerdctl bridge network, e.g., 10.1.100.1/24                                                                                           | Since 2.0.1      |
 | `kube_hide_dupe`    | `--kube-hide-dupe`                 |                           | Deduplicate images for Kubernetes with namespace k8s.io, no more redundant <none> ones are displayed    | Since 2.0.3      |
+| `cdi_spec_dirs`     | `--cdi-spec-dirs`                   |                          | The folders to use when searching for CDI ([container-device-interface](https://github.com/cncf-tags/container-device-interface)) specifications.    | Since 2.1.0 |


folders :-)

mikebrow

LGTM

AkihiroSuda

Thanks, will merge after releasing v2.0.5

AkihiroSuda · 2025-05-01T03:24:12Z

Needs rebase

This change adds support for specifying fully-qualified CDI device names in the --device flag. This allows the Container Device Interface (CDI) to be used to inject devices into container being run. Signed-off-by: Evan Lezar <elezar@nvidia.com> Enable cdi feature for Docker Signed-off-by: Evan Lezar <elezar@nvidia.com> Update go mod Signed-off-by: Evan Lezar <elezar@nvidia.com> Signed-off-by: Jin Dong <djdongjin95@gmail.com> use containerd cdi.WithCDIDevices Signed-off-by: Jin Dong <djdongjin95@gmail.com>

djdongjin force-pushed the carry-8525-add-cdi-support branch 4 times, most recently from 548458e to 449537e Compare April 27, 2025 02:47

djdongjin requested a review from Copilot April 27, 2025 02:48

Copilot AI reviewed Apr 27, 2025

View reviewed changes

djdongjin changed the title ~~[Carry 8525] add cdi support~~ [Carry 8525] Add support for CDI devices to device flag Apr 27, 2025

djdongjin changed the title ~~[Carry 8525] Add support for CDI devices to device flag~~ [Carry #8525] Add support for CDI devices to device flag Apr 27, 2025

djdongjin changed the title ~~[Carry #8525] Add support for CDI devices to device flag~~ [Carry #2280] Add support for CDI devices to device flag Apr 27, 2025

djdongjin requested review from AkihiroSuda and mikebrow April 27, 2025 02:51

djdongjin marked this pull request as ready for review April 27, 2025 02:58

djdongjin force-pushed the carry-8525-add-cdi-support branch 4 times, most recently from 9af8317 to efe0417 Compare April 27, 2025 16:06

apostasie reviewed Apr 27, 2025

View reviewed changes

cmd/nerdctl/container/container_run_linux_test.go Outdated Show resolved Hide resolved

apostasie reviewed Apr 27, 2025

View reviewed changes

cmd/nerdctl/container/container_run_linux_test.go Show resolved Hide resolved

apostasie reviewed Apr 27, 2025

View reviewed changes

cmd/nerdctl/container/container_run_linux_test.go Outdated Show resolved Hide resolved

djdongjin force-pushed the carry-8525-add-cdi-support branch from efe0417 to 8e649bd Compare April 27, 2025 19:07

djdongjin requested a review from apostasie April 27, 2025 19:58

AkihiroSuda reviewed Apr 28, 2025

View reviewed changes

pkg/defaults/defaults_linux.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed Apr 28, 2025

View reviewed changes

docs/config.md Outdated Show resolved Hide resolved

AkihiroSuda reviewed Apr 28, 2025

View reviewed changes

djdongjin force-pushed the carry-8525-add-cdi-support branch from 8e649bd to c34d651 Compare April 28, 2025 13:38

djdongjin requested a review from AkihiroSuda April 28, 2025 13:39

djdongjin added this to the v2.1.0 (tentative) milestone Apr 28, 2025

apostasie approved these changes Apr 28, 2025

View reviewed changes

mikebrow reviewed Apr 29, 2025

View reviewed changes

mikebrow approved these changes Apr 29, 2025

View reviewed changes

AkihiroSuda approved these changes Apr 29, 2025

View reviewed changes

djdongjin force-pushed the carry-8525-add-cdi-support branch from c34d651 to 71c646f Compare May 1, 2025 14:18

AkihiroSuda approved these changes May 1, 2025

View reviewed changes

AkihiroSuda merged commit 8d7539a into containerd:main May 1, 2025
83 of 87 checks passed

djdongjin mentioned this pull request May 1, 2025

Add support for CDI devices to device flag #2280

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Carry #2280] Add support for CDI devices to device flag #4170

[Carry #2280] Add support for CDI devices to device flag #4170

djdongjin commented Apr 27, 2025 •

edited

Loading

Copilot AI left a comment

AkihiroSuda Apr 28, 2025

djdongjin Apr 28, 2025

apostasie left a comment

apostasie Apr 28, 2025

mikebrow Apr 29, 2025

mikebrow Apr 29, 2025

mikebrow left a comment

AkihiroSuda left a comment

AkihiroSuda commented May 1, 2025

[Carry #2280] Add support for CDI devices to device flag #4170

[Carry #2280] Add support for CDI devices to device flag #4170

Conversation

djdongjin commented Apr 27, 2025 • edited Loading

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

AkihiroSuda Apr 28, 2025

Choose a reason for hiding this comment

djdongjin Apr 28, 2025

Choose a reason for hiding this comment

apostasie left a comment

Choose a reason for hiding this comment

apostasie Apr 28, 2025

Choose a reason for hiding this comment

mikebrow Apr 29, 2025

Choose a reason for hiding this comment

mikebrow Apr 29, 2025

Choose a reason for hiding this comment

mikebrow left a comment

Choose a reason for hiding this comment

AkihiroSuda left a comment

Choose a reason for hiding this comment

AkihiroSuda commented May 1, 2025

djdongjin commented Apr 27, 2025 •

edited

Loading