Skip to content

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

Merged
tianon merged 1 commit into
moby: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 into
moby:masterfrom
AkihiroSuda:rootless-new-shim-socket-path

Conversation

@AkihiroSuda

@AkihiroSuda AkihiroSuda commented Jun 26, 2020

Copy link
Copy Markdown
Member

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 <[email protected]>
@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.

@thaJeztah thaJeztah left a comment

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.

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.

@tianon tianon left a comment

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.

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