X Tutup
Skip to content

cdi: read XDG_CONFIG_HOME/cdi and XDG_RUNTIME_DIR/cdi for rootless#51624

Merged
thaJeztah merged 4 commits intomoby:masterfrom
AkihiroSuda:fix-51601
Dec 17, 2025
Merged

cdi: read XDG_CONFIG_HOME/cdi and XDG_RUNTIME_DIR/cdi for rootless#51624
thaJeztah merged 4 commits intomoby:masterfrom
AkihiroSuda:fix-51601

Conversation

@AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Dec 1, 2025 •
edited by vvoland
Loading

- What I did

Commit 1: cdi: read XDG_CONFIG_HOME/cdi and XDG_RUNTIME_DIR/cdi for rootless

Fix #51601

Commit 2: cdi: skip scanning non-readable dirs

This simplifies dockerd-rootless.sh by removing the workaround for CDI: Error associated with spec file /etc/cdi: failed to monitor for changes: permission denied.

- How I did it

See the commits

- How to verify it

For commit 1 cdi: read XDG_CONFIG_HOME/cdi and XDG_RUNTIME_DIR/cdi for rootless, create ~/.config/cdi/foo.yaml as follows, and run docker run --rm --device example.com/foo=foo alpine ls -l /dev/null2

cdiVersion: "0.6.0"
kind: "example.com/foo"
devices:
- name: foo
  containerEdits:
    mounts:
    - hostPath: /dev/null
      containerPath: /dev/null2
      options: [bind]

Try the same with /run/user/$UID/cdi, /etc/cdi, and /var/run/cdi too.

For commit 2 cdi: skip scanning non-readable dirs:

sudo mkdir -p -m /etc/cdi
dockerd-rootless-setuptool.sh install
docker info
# no error

- Human readable description for the release notes

Rootless: Consider `$XDG_CONFIG_HOME/cdi` and `$XDG_RUNTIME_DIR/cdi` when looking for CDI devices

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

vvoland
vvoland reviewed Dec 1, 2025
View reviewed changes
@vvoland vvoland added this to the 29.1.2 milestone Dec 1, 2025
@AkihiroSuda AkihiroSuda marked this pull request as draft December 1, 2025 10:48
@AkihiroSuda AkihiroSuda marked this pull request as ready for review December 1, 2025 10:56
AkihiroSuda
AkihiroSuda commented Dec 1, 2025
View reviewed changes
contrib/dockerd-rootless.sh
# the CDI paths need to exist before the Docker daemon is started in
# order for it to read the CDI specification files. Otherwise, a
# Docker daemon restart will be required for the daemon to discover
# them.
Copy link
Member Author

@AkihiroSuda AkihiroSuda Dec 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

NOTE: now the daemon has to be restarted when /etc/cdi (or /var/run/cdi) was created after starting the daemon

systemctl --user restart docker

This is not required for ~/.config/cdi and /run/user/$UID/cdi.

@AkihiroSuda
cdi: read XDG_CONFIG_HOME/cdi and XDG_RUNTIME_DIR/cdi for rootless
cd4397b 
Fix issue 51601

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda
cdi: skip scanning non-readable dirs
e29eaed 
This simplifies `dockerd-rootless.sh` by removing the workaround for
`CDI: Error associated with spec file /etc/cdi: failed to monitor for changes: permission denied`.

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda
cdi: resolve symlinks
a55f610 
Fix using /etc/cdi entries in rootless mode:

> level=warning msg="Refreshing the CDI registry generated errors"
> error="failed to monitor for changes: no such file or directory\nfailed to monitor for changes: no such file or directory"

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda
integration: cdi: add TestEtcCDI to verify /etc/cdi is used in rootle…
b9d18a1 
…ss mode

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
thaJeztah
thaJeztah reviewed Dec 2, 2025
View reviewed changes
daemon/command/daemon.go
Comment on lines +660 to +663
if rootless.RunningWithRootlessKit() {
// In rootless mode, we add the user-specific CDI spec directory.
xch, err := homedir.GetConfigHome()
if err != nil {
Copy link
Member

@thaJeztah thaJeztah Dec 2, 2025

Choose a reason for hiding this comment

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

Perhaps a silly question, but something I've been looking at a few times; is there any reason why we wouldn't handle XDG_xxx directories when running rootful?

I'm assuming here that when running as root, the XDG_xx directories would be owned by root, so if that's the case, it wouldn't risk a non-privileged user from installing CDI-related things that would be executed as root, but it would make the logic slightly more consistent without needing special branches for rootless / non-rootless sprinkled in various parts of the code.

Copy link
Member

@thaJeztah thaJeztah Dec 2, 2025

Choose a reason for hiding this comment

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

That said; what does CDI itself define for this? Having a quick glance at the README (https://github.com/cncf-tags/container-device-interface), it mentions;

CDI file containing updates for the OCI spec in JSON or YAML format (with a .json or .yaml file extension, respectively) should be present in a CDI spec directory. The default directories are /etc/cdi and /var/run/cdi, but may depend on your runtime configuration

So, should adding XDG directories be something that's ideally also defined in the CDI specification?

Copy link
Member Author

@AkihiroSuda AkihiroSuda Dec 3, 2025

Choose a reason for hiding this comment

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

Perhaps a silly question, but something I've been looking at a few times; is there any reason why we wouldn't handle XDG_xxx directories when running rootful?

Just because we have not been using XDG dirs for other components in rootful.
I think this can be discussed in a separate issue/PR.

@vvoland vvoland modified the milestones: 29.1.2, 29.1.3 Dec 2, 2025
@vvoland vvoland modified the milestones: 29.1.3, 29.2.0 Dec 11, 2025
vvoland
vvoland approved these changes Dec 16, 2025
View reviewed changes
Copy link
Contributor

@vvoland vvoland left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@vvoland vvoland requested a review from thaJeztah December 16, 2025 18:29
thaJeztah
thaJeztah approved these changes Dec 17, 2025
View reviewed changes
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

@thaJeztah thaJeztah merged commit 32190e9 into moby:master Dec 17, 2025
188 checks passed
@vvoland vvoland mentioned this pull request Jan 27, 2026
Open
@thaJeztah thaJeztah mentioned this pull request Jan 29, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

@vvoland vvoland vvoland approved these changes

Assignees

No one assigned

Labels

area/cdi area/contrib area/daemon Core Engine area/packaging area/rootless Rootless Mode impact/changelog kind/bugfix PR's that fix bugs

Projects

None yet

Milestone

29.2.0

Development

Successfully merging this pull request may close these issues.

CDI: read $XDG_CONFIG_HOME/cdi and XDG_RUNTIME_DIR/cdi when running in rootless

3 participants

@AkihiroSuda @thaJeztah @vvoland
X Tutup