New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use shareable IPC for sandbox container #70826

Merged
merged 1 commit into from Jan 1, 2019

Conversation

@kolyshkin
Copy link
Contributor

kolyshkin commented Nov 8, 2018

Currently, Docker make IPC of every container shareable by default,
which means other containers can join it's IPC namespace. This is
implemented by creating a tmpfs mount on the host, and then
bind-mounting it to a container's /dev/shm. Other containers
that want to share the same IPC (and the same /dev/shm) can also
bind-mount the very same host's mount.

Now, since moby/moby@7120976
(moby/moby#34087) there is a possiblity
to have per-daemon default of having "private" IPC mode,
meaning all the containers created will have non-shareable
/dev/shm.

For shared IPC to work in the above scenario, we need to
explicitly make the "pause" container's IPC mode as "shareable",
which is what this commit does.

To test: add "default-ipc-mode: private" to /etc/docker/daemon.json,
try using kube as usual, there should be no errors.

Fix inability to use k8s with dockerd having default IPC mode set to private.

/kind bug

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Nov 8, 2018

Hi @kolyshkin. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Nov 8, 2018

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@kolyshkin

This comment has been minimized.

Copy link
Contributor

kolyshkin commented Nov 8, 2018

/check-cla

@ddebroy

This comment has been minimized.

Copy link
Member

ddebroy commented Nov 9, 2018

/ok-to-test

@ddebroy

This comment has been minimized.

Copy link
Member

ddebroy commented Nov 9, 2018

/assign @vishh

@kolyshkin

This comment has been minimized.

Copy link
Contributor

kolyshkin commented Nov 9, 2018

CI failure:

W1109 02:55:20.724] 2018/11/09 02:55:20 process.go:153: Running: kubectl get nodes -ojson
W1109 02:55:21.148] Unable to connect to the server: dial tcp: lookup api.e2e-113032-dba53.test-cncf-aws.k8s.io on 10.63.240.10:53: no such host
W1109 02:55:21.151] 2018/11/09 02:55:21 process.go:155: Step 'kubectl get nodes -ojson' finished in 524.399862ms
etc

looks more like a glitch in the test env (although I'm not an expert here)

@kolyshkin

This comment has been minimized.

Copy link
Contributor

kolyshkin commented Nov 28, 2018

@vishh PTAL

@kolyshkin

This comment has been minimized.

Copy link
Contributor

kolyshkin commented Nov 28, 2018

/retest

Use shareable IPC for sandbox container
Currently, Docker make IPC of every container shareable by default,
which means other containers can join it's IPC namespace. This is
implemented by creating a tmpfs mount on the host, and then
bind-mounting it to a container's /dev/shm. Other containers
that want to share the same IPC (and the same /dev/shm) can also
bind-mount the very same host's mount.

Now, since moby/moby@7120976
(moby/moby#34087) there is a possiblity
to have per-daemon default of having "private" IPC mode,
meaning all the containers created will have non-shareable
/dev/shm.

For shared IPC to work in the above scenario, we need to
explicitly make the "pause" container's IPC mode as "shareable",
which is what this commit does.

To test: add "default-ipc-mode: private" to /etc/docker/daemon.json,
try using kube as usual, there should be no errors.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

@kolyshkin kolyshkin force-pushed the kolyshkin:shareable-ipc-sandbox branch from 17ae115 to 1dca64f Nov 28, 2018

@kolyshkin

This comment has been minimized.

Copy link
Contributor

kolyshkin commented Nov 28, 2018

rebased to master HEAD (clean rebase)

@kolyshkin

This comment has been minimized.

Copy link
Contributor

kolyshkin commented Nov 28, 2018

apparently CI failure was caused by the broken master -- rebase has fixed this.

@kolyshkin

This comment has been minimized.

Copy link
Contributor

kolyshkin commented Dec 11, 2018

How do we move this forward? Anything I can clarify/fix/address? @vishh @ddebroy

@kad

This comment has been minimized.

Copy link
Member

kad commented Dec 13, 2018

What would help with older versions of the docker? Maybe there is need for some docker version check?

@mtaufen

This comment has been minimized.

Copy link
Contributor

mtaufen commented Dec 14, 2018

/assign @yujuhong

@yujuhong

This comment has been minimized.

Copy link
Contributor

yujuhong commented Dec 15, 2018

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Dec 15, 2018

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 15, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kolyshkin, yujuhong

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ash2k

This comment has been minimized.

Copy link
Member

ash2k commented Jan 1, 2019

/test pull-kubernetes-godeps

@k8s-ci-robot k8s-ci-robot merged commit e76322e into kubernetes:master Jan 1, 2019

19 checks passed

cla/linuxfoundation kolyshkin authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-godeps Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment