Skip to content

dockerd-rootless.sh: support new containerd shim socket path convention#41156

Merged
tianon merged 1 commit intomoby:masterfrom
AkihiroSuda:rootless-new-shim-socket-path
Oct 15, 2020
Merged

dockerd-rootless.sh: support new containerd shim socket path convention#41156
tianon merged 1 commit intomoby:masterfrom
AkihiroSuda:rootless-new-shim-socket-path

Conversation

@AkihiroSuda
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda commented Jun 26, 2020

The new shim socket path convention hardcodes /run/containerd: containerd/containerd#4343

dockerd-rootless.sh is updated to hide the rootful /run/containerd from the mount namespace of the rootless dockerd.

The new shim socket path convention hardcodes `/run/containerd`:
containerd/containerd#4343

`dockerd-rootless.sh` is updated to hide the rootful `/run/containerd`
from the mount namespace of the rootless dockerd.

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda AkihiroSuda added this to the 20.03.0 milestone Jun 26, 2020
@AkihiroSuda AkihiroSuda changed the title dockerd-rootless.sh: support containerd v1.4 shim socket path convention dockerd-rootless.sh: support new containerd shim socket path convention Jul 28, 2020
@thaJeztah
Copy link
Copy Markdown
Member

@AkihiroSuda still draft?

@AkihiroSuda AkihiroSuda marked this pull request as ready for review October 7, 2020 06:35
@AkihiroSuda
Copy link
Copy Markdown
Member Author

The containerd PR isn't merged yet, but this PR can be merged safely now. PTAL.

Copy link
Copy Markdown
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

# remove the symlinks for the existing files in the parent namespace if any,
# so that we can create our own files in our mount namespace.
rm -f /run/docker /run/xtables.lock
rm -f /run/docker /run/containerd /run/xtables.lock
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This I assume is in the new mount namespace, and therefore the tmpfs is not from the host?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, and it's not rm -r, so the potential impact if this happens to have a more "real" /run/containerd is low (it'll probably fail).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, it is a new mount namespace. /run/containerd being removed is a symlink to the root-owned /run/containerd in the parent namespace. So we can safely remove it without privileges.

Copy link
Copy Markdown
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda
Copy link
Copy Markdown
Member Author

Cherry-pick PR: #41557

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants