Revert to using isolated PID namespaces in Docker #51634

verb · 2017-08-30T16:09:38Z

What this PR does / why we need it: Reverts to the previous docker default of using isolated PID namespaces for containers in a pod. There exist container images that expect always to be PID 1 which we want to support unmodified in 1.8.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #48937

Special notes for your reviewer:

Release note:

Sharing a PID namespace between containers in a pod is disabled by default in 1.8. To enable for a node, use the --docker-disable-shared-pid=false kubelet flag. Note that PID namespace sharing requires docker >= 1.13.1.

verb · 2017-08-30T17:04:02Z

looks like maybe the e2e node test doesn't set the flag. Will look into it tomorrow.

derekwaynecarr · 2017-08-31T01:47:12Z

poke when tests pass, thanks for making the update.

dims · 2017-08-31T11:30:12Z

@verb do we want to specify the docker versions needed for the shared pid flag to work in the release note?

verb · 2017-08-31T13:24:04Z

@dims oh yes, great catch. Updated the release note.

verb · 2017-08-31T19:02:50Z

@yguo0905 can you offer any advice about how to best to update the node e2e test for this? It looks like the kubelet is only run one time for the test, so I can't test with both the flag enabled and disabled.

Should I test the default behavior or test the feature even though it's disabled by default?

I assume the former but I want to make sure I'm not missing something.

yguo0905 · 2017-08-31T19:58:19Z

I think we want to test this behavior because user can enable it if they want. We can set the flag at

kubernetes/test/e2e_node/jenkins/jenkins-pull.properties

Line 7 in 55cb667

KUBELET_ARGS='--cgroups-per-qos=true --cgroup-root=/'

verb · 2017-09-01T14:08:33Z

/retest

verb · 2017-09-01T15:33:11Z

/retest

yguo0905 · 2017-09-01T15:44:24Z

test/e2e_node/services/kubelet.go

@@ -155,6 +155,7 @@ func (e *E2EServices) startKubelet() (*server, error) {
 		"--serialize-image-pulls", "false",
 		"--pod-manifest-path", manifestPath,
 		"--file-check-frequency", "10s", // Check file frequently so tests won't wait too long
+		"--docker-disable-shared-pid=false",


If we set it here we don't need the change in test/e2e_node/jenkins/jenkins-pull.properties?

Actually, if we don't set it here there are other properties files in test/e2e_node/jenkins/ need to be changed.

Ok, thanks. Based on this advice (and because it made the e2e node test pass) I've removed the jenkins config and set the flag here.

A shared PID namespace were enabled by default in the 1.7 when running with a supported Docker runtime, but a Docker version that supports a shared namespace was not qualified for use. Release 1.8 will qualify a docker version supporting shared PID, but we don't want to cause disruption for container images which expect always to have PID 1.

This also renames isSharedPIDNamespaceEnabled() to isSharedPIDNamespaceSupported() to be more accurate.

verb · 2017-09-02T05:12:45Z

/retest

…

verb · 2017-09-02T14:13:46Z

/retest

verb · 2017-09-04T05:33:02Z

/retest

verb · 2017-09-05T09:41:24Z

This PR is ready for review. According to sig-testing, pull-kubernetes-e2e-kops-aws is broken and has been disabled for the merge queue.

This PR should be added to the 1.8 milestone to resolve critical-urgent issue #48937.

dims · 2017-09-05T13:41:43Z

@kubernetes/sig-release-members Can you please apply milestone v1.8?

derekwaynecarr · 2017-09-05T15:35:44Z

/lgtm
/approve

k8s-github-robot · 2017-09-05T15:36:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, verb

Associated issue: 48937

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

Needs approval from an approver in each of these OWNERS Files:

~~cmd/kubelet/OWNERS~~ [derekwaynecarr]
~~test/e2e_node/OWNERS~~ [derekwaynecarr]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

verb · 2017-09-05T18:54:42Z

/retest

fejta-bot · 2017-09-05T23:06:51Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

k8s-ci-robot · 2017-09-05T23:59:21Z

@verb: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-kops-aws	`765374c`	link	`/test pull-kubernetes-e2e-kops-aws`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

k8s-github-robot · 2017-09-06T01:40:32Z

Automatic merge from submit-queue (batch tested with PRs 51984, 51351, 51873, 51795, 51634)

jdumars · 2017-09-06T19:41:00Z

@verb could you please assign a SIG to this?

verb · 2017-09-07T07:33:24Z

/sig node

In 1.7 release the shared namespace is enabled by default. This is causing a problem when running pbench container which runs systemd. Related issue in upstream kubernetes: kubernetes/kubernetes#48937 According to my understanding this will be fixed from 1.8 with the following patch: kubernetes/kubernetes#51634

In 1.7 release the shared PID namespaces is enabled by default. This is causing a problem when running pbench container which runs systemd. Related issue in upstream kubernetes: kubernetes/kubernetes#48937 According to my understanding this will be fixed from 1.8 with the following patch: kubernetes/kubernetes#51634

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 30, 2017

k8s-github-robot assigned tmrts and derekwaynecarr Aug 30, 2017

k8s-github-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Aug 30, 2017

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 1, 2017

yguo0905 reviewed Sep 1, 2017

View reviewed changes

verb added 2 commits September 1, 2017 23:50

Explicitly enable docker shared-pid for e2e_node

765374c

This also renames isSharedPIDNamespaceEnabled() to isSharedPIDNamespaceSupported() to be more accurate.

verb force-pushed the sharedpid-default-off branch from 48f203e to 765374c Compare September 1, 2017 21:51

verb mentioned this pull request Sep 5, 2017

Cannot deploy GlusterFS through Kubernetes - "Couldn't find an alternative telinit implementation to spawn" #48937

Closed

ericchiang added this to the v1.8 milestone Sep 5, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 5, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 5, 2017

k8s-github-robot merged commit eb86cc5 into kubernetes:master Sep 6, 2017

k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Sep 7, 2017

verb mentioned this pull request Sep 7, 2017

Shared PID and UTS namespaces #1615

Closed

jarrpa mentioned this pull request Sep 8, 2017

glusterfs pods will not deploy gluster/gluster-kubernetes#341

Closed

bradbeam mentioned this pull request Sep 27, 2017

Set default value for disable_shared_pid kubernetes-sigs/kubespray#1710

Merged

weiwei04 mentioned this pull request Nov 9, 2017

why not share pid namespace cri-o/cri-o#1135

Closed

chaitanyaenr mentioned this pull request Nov 28, 2017

Modify template to work with kubernetes 1.7 openshift/svt#345

Merged

dustymabe mentioned this pull request Dec 11, 2017

glusterfs containers can't start on 3.7 with docker 1.13+ openshift/openshift-ansible#6431

Closed

sjenning mentioned this pull request Dec 12, 2017

[3.7] UPSTREAM: 51634: Revert to using isolated PID namespaces in Docker openshift/origin#17722

Merged

briansmith mentioned this pull request Mar 16, 2018

proxy: add SIGTERM and SIGQUIT handlers linkerd/linkerd2#581

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert to using isolated PID namespaces in Docker #51634

Revert to using isolated PID namespaces in Docker #51634

verb commented Aug 30, 2017 •

edited

Loading

verb commented Aug 30, 2017

derekwaynecarr commented Aug 31, 2017

dims commented Aug 31, 2017

verb commented Aug 31, 2017

verb commented Aug 31, 2017

yguo0905 commented Aug 31, 2017

verb commented Sep 1, 2017

verb commented Sep 1, 2017

yguo0905 Sep 1, 2017

verb Sep 4, 2017

verb commented Sep 2, 2017 via email

verb commented Sep 2, 2017

verb commented Sep 4, 2017

verb commented Sep 5, 2017

dims commented Sep 5, 2017

derekwaynecarr commented Sep 5, 2017

k8s-github-robot commented Sep 5, 2017

verb commented Sep 5, 2017

fejta-bot commented Sep 5, 2017

k8s-ci-robot commented Sep 5, 2017

k8s-github-robot commented Sep 6, 2017

jdumars commented Sep 6, 2017

verb commented Sep 7, 2017

Revert to using isolated PID namespaces in Docker #51634

Revert to using isolated PID namespaces in Docker #51634

Conversation

verb commented Aug 30, 2017 • edited Loading

verb commented Aug 30, 2017

derekwaynecarr commented Aug 31, 2017

dims commented Aug 31, 2017

verb commented Aug 31, 2017

verb commented Aug 31, 2017

yguo0905 commented Aug 31, 2017

verb commented Sep 1, 2017

verb commented Sep 1, 2017

yguo0905 Sep 1, 2017

Choose a reason for hiding this comment

verb Sep 4, 2017

Choose a reason for hiding this comment

verb commented Sep 2, 2017 via email

verb commented Sep 2, 2017

verb commented Sep 4, 2017

verb commented Sep 5, 2017

dims commented Sep 5, 2017

derekwaynecarr commented Sep 5, 2017

k8s-github-robot commented Sep 5, 2017

verb commented Sep 5, 2017

fejta-bot commented Sep 5, 2017

k8s-ci-robot commented Sep 5, 2017

k8s-github-robot commented Sep 6, 2017

jdumars commented Sep 6, 2017

verb commented Sep 7, 2017

verb commented Aug 30, 2017 •

edited

Loading