{"action":"created","issue":{"url":"https://api.github.com/repos/kubernetes/kubernetes/issues/31169","repository_url":"https://api.github.com/repos/kubernetes/kubernetes","labels_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/31169/labels{/name}","comments_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/31169/comments","events_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/31169/events","html_url":"https://github.com/kubernetes/kubernetes/pull/31169","id":172566033,"number":31169,"title":"Default host user namespace via experimental flag","user":{"login":"pweil-","id":5230419,"avatar_url":"https://avatars.githubusercontent.com/u/5230419?v=3","gravatar_id":"","url":"https://api.github.com/users/pweil-","html_url":"https://github.com/pweil-","followers_url":"https://api.github.com/users/pweil-/followers","following_url":"https://api.github.com/users/pweil-/following{/other_user}","gists_url":"https://api.github.com/users/pweil-/gists{/gist_id}","starred_url":"https://api.github.com/users/pweil-/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pweil-/subscriptions","organizations_url":"https://api.github.com/users/pweil-/orgs","repos_url":"https://api.github.com/users/pweil-/repos","events_url":"https://api.github.com/users/pweil-/events{/privacy}","received_events_url":"https://api.github.com/users/pweil-/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/cla:%20yes","name":"cla: yes","color":"bfe5bf"},{"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/kind/api-change","name":"kind/api-change","color":"c7def8"},{"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/release-note","name":"release-note","color":"c2e0c6"},{"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/size/L","name":"size/L","color":"ee9900"}],"state":"open","locked":false,"assignee":{"login":"vishh","id":7063592,"avatar_url":"https://avatars.githubusercontent.com/u/7063592?v=3","gravatar_id":"","url":"https://api.github.com/users/vishh","html_url":"https://github.com/vishh","followers_url":"https://api.github.com/users/vishh/followers","following_url":"https://api.github.com/users/vishh/following{/other_user}","gists_url":"https://api.github.com/users/vishh/gists{/gist_id}","starred_url":"https://api.github.com/users/vishh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vishh/subscriptions","organizations_url":"https://api.github.com/users/vishh/orgs","repos_url":"https://api.github.com/users/vishh/repos","events_url":"https://api.github.com/users/vishh/events{/privacy}","received_events_url":"https://api.github.com/users/vishh/received_events","type":"User","site_admin":false},"assignees":[{"login":"vishh","id":7063592,"avatar_url":"https://avatars.githubusercontent.com/u/7063592?v=3","gravatar_id":"","url":"https://api.github.com/users/vishh","html_url":"https://github.com/vishh","followers_url":"https://api.github.com/users/vishh/followers","following_url":"https://api.github.com/users/vishh/following{/other_user}","gists_url":"https://api.github.com/users/vishh/gists{/gist_id}","starred_url":"https://api.github.com/users/vishh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vishh/subscriptions","organizations_url":"https://api.github.com/users/vishh/orgs","repos_url":"https://api.github.com/users/vishh/repos","events_url":"https://api.github.com/users/vishh/events{/privacy}","received_events_url":"https://api.github.com/users/vishh/received_events","type":"User","site_admin":false}],"milestone":null,"comments":4,"created_at":"2016-08-22T22:03:10Z","updated_at":"2016-08-23T18:17:42Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/kubernetes/kubernetes/pulls/31169","html_url":"https://github.com/kubernetes/kubernetes/pull/31169","diff_url":"https://github.com/kubernetes/kubernetes/pull/31169.diff","patch_url":"https://github.com/kubernetes/kubernetes/pull/31169.patch"},"body":"@vishh @ncdc @pmorie @smarterclayton @thockin \r\n\r\nInitial thought on the implementation https://github.com/kubernetes/kubernetes/pull/30684#issuecomment-241523425 wasn't quite right.  Since we need to dereference a PVC in some cases the defaulting code didn't fit nicely in the docker manager code (would've coupled it with a kube client and would've been messy).  I think passing this in via the runtime config turned out cleaner.  PTAL\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.kubernetes.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.kubernetes.io/reviews/kubernetes/kubernetes/31169)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/kubernetes/kubernetes/issues/comments/241825356","html_url":"https://github.com/kubernetes/kubernetes/pull/31169#issuecomment-241825356","issue_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/31169","id":241825356,"user":{"login":"vishh","id":7063592,"avatar_url":"https://avatars.githubusercontent.com/u/7063592?v=3","gravatar_id":"","url":"https://api.github.com/users/vishh","html_url":"https://github.com/vishh","followers_url":"https://api.github.com/users/vishh/followers","following_url":"https://api.github.com/users/vishh/following{/other_user}","gists_url":"https://api.github.com/users/vishh/gists{/gist_id}","starred_url":"https://api.github.com/users/vishh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vishh/subscriptions","organizations_url":"https://api.github.com/users/vishh/orgs","repos_url":"https://api.github.com/users/vishh/repos","events_url":"https://api.github.com/users/vishh/events{/privacy}","received_events_url":"https://api.github.com/users/vishh/received_events","type":"User","site_admin":false},"created_at":"2016-08-23T18:17:42Z","updated_at":"2016-08-23T18:17:42Z","body":"\n\n\n\nReview status: 0 of 14 files reviewed at latest revision, 13 unresolved discussions.\n\n---\n\n*[cmd/kubelet/app/options/options.go, line 216 \\[r1\\]](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/31169#-KPsZBtQrM1qG0lpL3Mm:-KPsZBtQrM1qG0lpL3Mn:bkacd4e) ([raw file](https://github.com/kubernetes/kubernetes/blob/d6650fd437632eeef40a78dde590f8f479ab2f71/cmd/kubelet/app/options/options.go#L216)):*\n> ```Go\n> \n> \t// experimental flags\n> \tfs.BoolVar(&s.ExperimentalHostUserNamespaceDefaulting, \"experimental-host-user-namespace-defaulting\", s.ExperimentalHostUserNamespaceDefaulting, \"Default the value for UsernsMode for containers that are using other host namespaces, host mounts, or specific non-namespaced capabilities (MKNOD, SYS_MODULE, SYS_TIME). This should only be enabled is user namespace remapping is enabled in the docker daemon. [default=false]\")\n> ```\n\nI'd prefer using `--feature-gates` flag to gate this feature instead of adding a special flag to it. \n\n---\n\n*[pkg/apis/componentconfig/v1alpha1/types.go, line 475 \\[r1\\]](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/31169#-KPsZwQJHkchT3KankWM:-KPsZwQJHkchT3KankWN:bdivllh) ([raw file](https://github.com/kubernetes/kubernetes/blob/d6650fd437632eeef40a78dde590f8f479ab2f71/pkg/apis/componentconfig/v1alpha1/types.go#L475)):*\n> ```Go\n> \t// experimentalHostUserNamespaceDefaulting is an experimental flag to enable defaulting of the UsernsMode for\n> \t// a container.\n> \tExperimentalHostUserNamespaceDefaulting bool `json:\"experimentalHostUserNamespaceDefaulting\"`\n> ```\n\nThis feature should ideally be passed in via a `FeatureConfigs` field. @mtaufen is working on adding `FeatureConfigs` field to `kubeletConfiguration`.\n\n---\n\n*[pkg/kubelet/kubelet.go, line 253 \\[r1\\]](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/31169#-KPs__CPDlfFvLg81bxN:-KPs__CPDlfFvLg81bxO:bqxdzix) ([raw file](https://github.com/kubernetes/kubernetes/blob/d6650fd437632eeef40a78dde590f8f479ab2f71/pkg/kubelet/kubelet.go#L253)):*\n> ```Go\n> \tiptablesMasqueradeBit int,\n> \tiptablesDropBit int,\n> \texperimentalHostUserNamespaceDefaulting bool,\n> ```\n\nIf you rebase, Kubelet factory now takes in a `kubeletConfiguration` object. You don't have to pass in this feature explicitly. \n\n---\n\n*[pkg/kubelet/kubelet.go, line 1323 \\[r1\\]](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/31169#-KPsa8CmhYCAV0Fkcigk:-KPsa8CmhYCAV0Fkcigl:bt1tuoc) ([raw file](https://github.com/kubernetes/kubernetes/blob/d6650fd437632eeef40a78dde590f8f479ab2f71/pkg/kubelet/kubelet.go#L1323)):*\n> ```Go\n> \n> \t// only do this check if the experimental behavior is enabled, otherwise allow it to default to false\n> \tif kl.experimentalHostUserNamespaceDefaulting {\n> ```\n\nWhy not pass this feature down to the runtime and let the runtime shims take care of the defaulting?\n\n---\n\n*[pkg/kubelet/kubelet_pods.go, line 17 \\[r6\\]](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/31169#-KPrRyiaPOHCnKYA147U-r6-17:-KPsaIFHlPcMwPXm4TN0:b-1z8qcu) ([raw file](https://github.com/kubernetes/kubernetes/blob/8dfdec5d9de07c74a414e048f43eddfa386650a1/pkg/kubelet/kubelet_pods.go#L17)):*\n<details><summary><i>Previously, pmorie (Paul Morie) wrote…</i></summary>\n> note to other reviewers: I asked @pweil- to name this file as such, going to put a ton of other stuff into it in a planned refactor.\n</details>\nIsn't this specific to docker runtime? Why keep this logic in the `kubelet` package?\n\n---\n\n*[pkg/kubelet/container/runtime.go, line 395 \\[r2\\]](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/31169#-KPoGsKER7jV7guPuBuT-r2-395:-KPsbiC-M84MyR8B-EbQ:bqxdjmj) ([raw file](https://github.com/kubernetes/kubernetes/blob/11725e4acd3837afd74603781cf7501015bb0ca1/pkg/kubelet/container/runtime.go#L395)):*\n<details><summary><i>Previously, smarterclayton (Clayton Coleman) wrote…</i></summary>\n> Comment should be: \"When users request other host namespaces, automatically set userns=host for Docker containers\" or similar.\n</details>\nI'd prefer making this change local to docker shim\n\n---\n\n*[pkg/kubelet/kubelet_hostusernamespace.go, line 38 \\[r1\\]](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/31169#-KPsbF3fI0ax0YaiF4-v:-KPsbF3fI0ax0YaiF4-w:bcppu26) ([raw file](https://github.com/kubernetes/kubernetes/blob/d6650fd437632eeef40a78dde590f8f479ab2f71/pkg/kubelet/kubelet_hostusernamespace.go#L38)):*\n> ```Go\n> func hasNonNamespacedCapability(pod *api.Pod) bool {\n> \tfor _, c := range pod.Spec.Containers {\n> \t\tif c.SecurityContext != nil && c.SecurityContext.Capabilities != nil {\n> ```\n\nWhat about privileged pods?\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/31169)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"vishh"}}