-
Notifications
You must be signed in to change notification settings - Fork 770
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
add support for "pid" key #617
Conversation
if service.Pid == "host" { | ||
podSecurityContext.HostPID = true | ||
} else { | ||
log.Warningf("Ignoring pid key for service \"%v\". Invalid value \"%v\".", name, service.Pid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be better to capitalize pid to PID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the one comment. LGTM!
LGTM :) |
When the 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. |
I don't think these keys from docker-compose and kubernetes map to each other! |
@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! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see the comments above!
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. |
@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. |
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. |
@surajssd Following are my steps to reproduce, I am runing a 1.5 k8s on AWS.
The compose file to use:
Deploy to k8s:
And inside the node on which the pod runs:
Verify pidMod of the container that k8s created:
|
@gitlawr yes came across this issue then realized that it was problem with the k8s I was using! It is problem of 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
Rebased and resolved the conflict. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for this! :-)
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