add support for "pid" key #617

gitlawr · 2017-05-24T10:57:46Z

solve #610
convert service.pid to Pod.Spec.HostPid
set Pod.Spec.HostPid to true if service.pid ="host"
update conversion.md on support for the key

cdrage · 2017-05-24T14:46:32Z

pkg/transformer/kubernetes/k8sutils.go

+ if service.Pid == "host" {
+ podSecurityContext.HostPID = true
+ } else {
+ log.Warningf("Ignoring pid key for service \"%v\". Invalid value \"%v\".", name, service.Pid)


May be better to capitalize pid to PID.

cdrage

Other than the one comment. LGTM!

cdrage · 2017-05-25T13:33:48Z

Awesome. Thanks for updating. LGTM! @kadel @surajssd wanna give a quick review?

surajnarwade · 2017-05-29T07:50:24Z

LGTM :)

surajssd · 2017-05-29T08:36:47Z

When the pid: "host" is set inside the docker-compose file, I could see all the processes on the host, but when the flag on pod is set to hostPID: true, I don't see all the processes on the host but only inside the pod.

Please pardon me on this, I should I done this test even before this reached the PR state and we decided we gonna implement this!

I am using this docker-compose file:

$ cat docker-compose.yml 
version: "2"

services:
  web:
    image: centos/httpd
    ports:
    - "80"
    pid: "host"

when I run it with docker-compose and exec into the container:

$ eval $(minikube docker-env)
$ docker-compose up
Creating network "multiple_default" with the default driver
Creating multiple_web_1
Attaching to multiple_web_1
...

inside minikube vm:

$ docker ps | grep httpd
4e78c8566748        centos/httpd                               "/run-httpd.sh"          22 seconds ago       Up 22 seconds       0.0.0.0:32768->80/tcp   multiple_web_1

$ docker exec -it multiple_web_1 ps aux
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root         1  2.5  0.2 120416  5784 ?        Ss   08:13   0:11 /sbin/init
root         2  0.0  0.0      0     0 ?        S    08:13   0:00 [kthreadd]
root         3  0.0  0.0      0     0 ?        S    08:13   0:00 [ksoftirqd/0]
root         4  0.0  0.0      0     0 ?        S    08:13   0:00 [kworker/0:0]
root         5  0.0  0.0      0     0 ?        S<   08:13   0:00 [kworker/0:0H]
root         7  0.0  0.0      0     0 ?        S    08:13   0:00 [rcu_sched]
root         8  0.0  0.0      0     0 ?        S    08:13   0:00 [rcu_bh]
root         9  0.0  0.0      0     0 ?        S    08:13   0:00 [migration/0]
root        10  0.0  0.0      0     0 ?        S<   08:13   0:00 [lru-add-drain]
root        11  0.0  0.0      0     0 ?        S    08:13   0:00 [cpuhp/0]
root        12  0.0  0.0      0     0 ?        S    08:13   0:00 [cpuhp/1]
root        13  0.0  0.0      0     0 ?        S    08:13   0:00 [migration/1]
root        14  0.0  0.0      0     0 ?        S    08:13   0:00 [ksoftirqd/1]
root        16  0.0  0.0      0     0 ?        S<   08:13   0:00 [kworker/1:0H]
...

while inside minikube vm:

$ ps aux
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root         1  3.6  0.2 120416  5784 ?        Ss   08:13   0:10 /sbin/init
root         2  0.0  0.0      0     0 ?        S    08:13   0:00 [kthreadd]
root         3  0.0  0.0      0     0 ?        S    08:13   0:00 [ksoftirqd/0]
root         4  0.0  0.0      0     0 ?        S    08:13   0:00 [kworker/0:0]
root         5  0.0  0.0      0     0 ?        S<   08:13   0:00 [kworker/0:0H]
root         6  0.0  0.0      0     0 ?        S    08:13   0:00 [kworker/u4:0]
root         7  0.0  0.0      0     0 ?        S    08:13   0:00 [rcu_sched]
root         8  0.0  0.0      0     0 ?        S    08:13   0:00 [rcu_bh]
root         9  0.0  0.0      0     0 ?        S    08:13   0:00 [migration/0]
root        10  0.0  0.0      0     0 ?        S<   08:13   0:00 [lru-add-drain]
root        11  0.0  0.0      0     0 ?        S    08:13   0:00 [cpuhp/0]
root        12  0.0  0.0      0     0 ?        S    08:13   0:00 [cpuhp/1]
root        13  0.0  0.0      0     0 ?        S    08:13   0:00 [migration/1]
root        14  0.0  0.0      0     0 ?        S    08:13   0:00 [ksoftirqd/1]
root        15  0.0  0.0      0     0 ?        S    08:13   0:00 [kworker/1:0]
...

Now when run with kompose on the cluster running on the vm.

$ kompose up
...
$ kubectl get pods
NAME                   READY     STATUS    RESTARTS   AGE
web-2967932665-g4kbm   1/1       Running   0          2m

$ kubectl exec -it web-2967932665-g4kbm ps aux
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root         1  0.0  0.1  11636  2388 ?        Ss   08:30   0:00 /bin/sh /usr/sb
root         8  0.0  0.3 221928  7468 ?        S    08:30   0:00 /usr/sbin/httpd
apache       9  0.0  0.2 221928  5968 ?        S    08:30   0:00 /usr/sbin/httpd
apache      10  0.0  0.2 221928  5968 ?        S    08:30   0:00 /usr/sbin/httpd
apache      11  0.0  0.2 221928  5968 ?        S    08:30   0:00 /usr/sbin/httpd
apache      12  0.0  0.2 221928  5968 ?        S    08:30   0:00 /usr/sbin/httpd
apache      13  0.0  0.2 221928  5968 ?        S    08:30   0:00 /usr/sbin/httpd
root        18  0.0  0.1  47448  3344 ?        Rs+  08:32   0:00 ps aux


$ kubectl get pod web-2967932665-g4kbm -o yaml | grep -i hostpid
  hostPID: true

While according to this PR this should have shown the processes on the host.

surajssd · 2017-05-29T08:37:26Z

I don't think these keys from docker-compose and kubernetes map to each other!

surajssd · 2017-05-29T08:39:15Z

@cdrage @surajnarwade @gitlawr I think before we start implementing any key we should see if they have similar behavior in both the worlds and then only map it! At least from now on!

surajssd

see the comments above!

cdrage · 2017-05-29T14:10:12Z

I think this is the best possible 1-1 conversion, perhaps we should simply add a note/warning to it when the key is used?

Yes, the concept of pods vs docker compose's containers is much different. But the k8s docs say: "Use the host's pid namespace" in terms of using hostPID.

surajssd · 2017-05-30T10:02:04Z

@cdrage what is the point of adding something that behaves differently in two environment? IMHO we should only map keys which are similar in behavior.

cdrage · 2017-05-30T12:35:44Z

@surajssd

Perhaps it behaves differently, but the concepts are the same. Kubernetes views "Pods" as the host, while Docker-Compose views the entire system (in k8s terms, "Node"). Containers in the same Pod will be accessible, but not the entire Node.

gitlawr · 2017-05-31T03:27:53Z

@surajssd
what is your k8s version and docker version? I wonder if it is related to some issues with Kubernetes
kubernetes/kubernetes#44041
Regarding to the concept of both keys I think the mapping is reasonable.

Following are my steps to reproduce, I am runing a 1.5 k8s on AWS.

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"5", GitVersion:"v1.5.3", GitCommit:"029c3a408176b55c30846f0faedf56aae5992e9b", GitTreeState:"clean", BuildDate:"2017-02-15T06:40:50Z", GoVersion:"go1.7.4", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"5", GitVersion:"v1.5.6", GitCommit:"114f8911f9597be669a747ab72787e0bd74c9359", GitTreeState:"clean", BuildDate:"2017-03-28T13:36:31Z", GoVersion:"go1.7.4", Compiler:"gc", Platform:"linux/amd64"}

The compose file to use:

$ cat docker-compose.yaml 
version: "2"

services:
  web:
    image: centos/httpd
    ports:
    - "80"
    pid: "host"

Deploy to k8s:

$ kompose up
INFO We are going to create Kubernetes Deployments, Services and PersistentVolumeClaims for your Dockerized application. If you need different kind of resources, use the 'kompose convert' and 'kubectl create -f' commands instead. 
 
INFO Deploying application in "default" namespace 
INFO Successfully created Service: web            
INFO Successfully created Deployment: web         

Your application has been deployed to Kubernetes. You can run 'kubectl get deployment,svc,pods,pvc' for details.

$ kubectl get po -o wide
NAME                   READY     STATUS    RESTARTS   AGE       IP            NODE
web-3376332856-6x0s1   1/1       Running   0          2m        10.244.2.41   ip-172-31-63-82.ec2.internal
$ kubectl exec -it web-3376332856-6x0s1 ps aux
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root         1  0.0  0.2  38020  6104 ?        Ss   02:00   0:02 /sbin/init
root         2  0.0  0.0      0     0 ?        S    02:00   0:00 [kthreadd]
root         3  0.0  0.0      0     0 ?        S    02:00   0:00 [ksoftirqd/0]
root         5  0.0  0.0      0     0 ?        S<   02:00   0:00 [kworker/0:0H]
root         6  0.0  0.0      0     0 ?        S    02:00   0:00 [kworker/u30:0]
root         7  0.0  0.0      0     0 ?        S    02:00   0:00 [rcu_sched]
root         8  0.0  0.0      0     0 ?        S    02:00   0:00 [rcu_bh]
root         9  0.0  0.0      0     0 ?        S    02:00   0:00 [migration/0]
root        10  0.0  0.0      0     0 ?        S    02:00   0:00 [watchdog/0]

And inside the node on which the pod runs:

ubuntu@ip-172-31-63-82:~$ ps aux
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root         1  0.0  0.2  38020  6104 ?        Ss   02:00   0:02 /sbin/init
root         2  0.0  0.0      0     0 ?        S    02:00   0:00 [kthreadd]
root         3  0.0  0.0      0     0 ?        S    02:00   0:00 [ksoftirqd/0]
root         5  0.0  0.0      0     0 ?        S<   02:00   0:00 [kworker/0:0H]
root         6  0.0  0.0      0     0 ?        S    02:00   0:00 [kworker/u30:0]
root         7  0.0  0.0      0     0 ?        S    02:00   0:00 [rcu_sched]
root         8  0.0  0.0      0     0 ?        S    02:00   0:00 [rcu_bh]
root         9  0.0  0.0      0     0 ?        S    02:00   0:00 [migration/0]
root        10  0.0  0.0      0     0 ?        S    02:00   0:00 [watchdog/0]

Verify pidMod of the container that k8s created:

root@ip-172-31-63-82:/home/ubuntu# docker ps |grep web
ba562a75a72a        centos/httpd                                          "/run-httpd.sh"          4 minutes ago       Up 4 minutes                            k8s_web.dc333111_web-3376332856-6x0s1_default_939512e9-45ac-11e7-bd4c-129b80d9a4d4_ef68db5f
26a72ad939ae        gcr.io/google_containers/pause-amd64:3.0              "/pause"                 4 minutes ago       Up 4 minutes                            k8s_POD.d8dbe16c_web-3376332856-6x0s1_default_939512e9-45ac-11e7-bd4c-129b80d9a4d4_590b466b
root@ip-172-31-63-82:/home/ubuntu# docker inspect ba562a75a72a|grep -i pid
            "Pid": 14082,
            "PidMode": "host",
            "PidsLimit": 0,

surajssd · 2017-05-31T14:09:46Z

@gitlawr yes came across this issue then realized that it was problem with the k8s I was using! It is problem of 1.6.0.

Rebase and I think we can go ahead with it!

solve kubernetes#610 convert service.pid to Pod.Spec.HostPid set Pod.Spec.HostPid to true if service.pid ="host", to false otherwise update conversion.md on support for the key

gitlawr · 2017-05-31T15:10:47Z

Rebased and resolved the conflict.

surajssd

LGTM! Thanks for this! :-)

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 24, 2017

cdrage reviewed May 24, 2017

View reviewed changes

cdrage approved these changes May 24, 2017

View reviewed changes

surajssd suggested changes May 29, 2017

View reviewed changes

add support for "pid" key

1485ed4

solve kubernetes#610 convert service.pid to Pod.Spec.HostPid set Pod.Spec.HostPid to true if service.pid ="host", to false otherwise update conversion.md on support for the key

gitlawr force-pushed the add_pid branch from e3bfb53 to 1485ed4 Compare May 31, 2017 14:41

surajssd approved these changes May 31, 2017

View reviewed changes

surajssd merged commit a35d596 into kubernetes:master May 31, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for "pid" key #617

add support for "pid" key #617

gitlawr commented May 24, 2017

cdrage May 24, 2017

cdrage left a comment

cdrage commented May 25, 2017

surajnarwade commented May 29, 2017

surajssd commented May 29, 2017

surajssd commented May 29, 2017

surajssd commented May 29, 2017

surajssd left a comment

cdrage commented May 29, 2017

surajssd commented May 30, 2017

cdrage commented May 30, 2017

gitlawr commented May 31, 2017 •

edited

Loading

surajssd commented May 31, 2017

gitlawr commented May 31, 2017

surajssd left a comment

add support for "pid" key #617

add support for "pid" key #617

Conversation

gitlawr commented May 24, 2017

cdrage May 24, 2017

Choose a reason for hiding this comment

cdrage left a comment

Choose a reason for hiding this comment

cdrage commented May 25, 2017

surajnarwade commented May 29, 2017

surajssd commented May 29, 2017

surajssd commented May 29, 2017

surajssd commented May 29, 2017

surajssd left a comment

Choose a reason for hiding this comment

cdrage commented May 29, 2017

surajssd commented May 30, 2017

cdrage commented May 30, 2017

gitlawr commented May 31, 2017 • edited Loading

surajssd commented May 31, 2017

gitlawr commented May 31, 2017

surajssd left a comment

Choose a reason for hiding this comment

gitlawr commented May 31, 2017 •

edited

Loading