Skip to content

daemon: Enable CDI by default#49963

Merged
thaJeztah merged 1 commit intomoby:masterfrom
vvoland:cdi-ga
May 15, 2025
Merged

daemon: Enable CDI by default#49963
thaJeztah merged 1 commit intomoby:masterfrom
vvoland:cdi-ga

Conversation

@vvoland
Copy link
Contributor

@vvoland vvoland commented May 13, 2025

CDI support has been available as opt-in starting from v25.0.0. Considering that there are no known blockers I believe it's about time to make it enabled by default to encourage wider usage.

CDI is now enabled by default

- A picture of a cute animal (not mandatory but encouraged)

@vvoland vvoland added this to the 28.2.0 milestone May 13, 2025
@vvoland vvoland self-assigned this May 13, 2025
@vvoland vvoland added status/2-code-review kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny impact/changelog area/daemon Core Engine area/cdi labels May 13, 2025
@vvoland vvoland requested a review from a team May 13, 2025 09:09
@thaJeztah
Copy link
Member

Wondering if we should keep that ticket still open to consider working on the follow-ups (so "addresses / relates-to" instead of "fixes")?

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@dmcgowan PTAL

@tonistiigi
Copy link
Member

If this is enabled by default, I would expect docker info to report what devices are available. (Ideally also if --gpus NVIDIA device has been registered).

@thaJeztah
Copy link
Member

Not 100% against, but also slightly concerned that we'd be overloading "docker info" even more ; we don't do the same currently for (e.g.) containerd runtimes.

Given that these could be installed out of bound, I guess we can't memoize the results, so we'd have to re-read the list on every call to the /info endpoints; possibly parse each CDI spec we find to enumerate what's in them?

@tonistiigi
Copy link
Member

Given that these could be installed out of bound, I guess we can't memoize the results, so we'd have to re-read the list on every call to the /info endpoints; possibly parse each CDI spec we find to enumerate what's in them?

No, https://github.com/moby/moby/blob/master/daemon/cdi.go#L63 by default does auto-refresh.

@vvoland
Copy link
Contributor Author

vvoland commented May 14, 2025

Opened CDI device discovery as a separate PR (in case we decide to postpone it): #49980

@thaJeztah
Copy link
Member

Thanks both! Yes, probably it's fine to review / merge separately.

As to my concerns about putting it in /info - I could see the information useful (which could be as part of, say, tab-completion), but knowing that the /info endpoint is already quite loaded, I wondered if we should also consider alternative for that purpose, before it's something we cannot back out of.

@vvoland
Copy link
Contributor Author

vvoland commented May 15, 2025

Any objections against merging it now, so we get this in rc1? @tonistiigi

@vvoland vvoland requested a review from tonistiigi May 15, 2025 11:03
@thaJeztah
Copy link
Member

Needs a minor rebase after #49990 was merged

@thompson-shaun thompson-shaun added the release-blocker PRs we want to block a release on label May 15, 2025
@tonistiigi
Copy link
Member

Ok with getting this in but would like #49980 as well.

@dmcgowan
Copy link
Member

Does this need another rebase to pass validation? It seems a vendor change around the same time as the rebase was pushed caused CI to fail.

@thompson-shaun thompson-shaun moved this from Up next to Complete in 🔦 Maintainer spotlight May 15, 2025
CDI will now be enabled by default unless opted-out by setting `cdi`
feature to `false`.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@thaJeztah
Copy link
Member

I did a quick rebase

@thaJeztah
Copy link
Member

All green; bringing this in

@thaJeztah thaJeztah merged commit 994d280 into moby:master May 15, 2025
141 checks passed
@vvoland vvoland mentioned this pull request Jun 26, 2025
1 task
vvoland added a commit to vvoland/container-device-interface that referenced this pull request Jun 26, 2025
Docker 28.2 no longer requires an explicit opt-in to use CDI: moby/moby#49963
Reflect that information in the README.

Also, since the `--device` behavior is the same for both Docker and
Podman, combine th
This commit also combines the Docker and Podman section to share the
`--device` option usage.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
vvoland added a commit to vvoland/container-device-interface that referenced this pull request Jun 26, 2025
Docker 28.2 no longer requires an explicit opt-in to use CDI: moby/moby#49963
Reflect that information in the README.

Also, since the `--device` behavior is the same for both Docker and
Podman, combine them into one section.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cdi area/daemon Core Engine impact/changelog kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny release-blocker PRs we want to block a release on status/2-code-review

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants