fold run-proxy.sh funtionality into proxy-dentity #6228
Conversation
There was a problem hiding this comment.
I tested this out by building the images locally and running a fresh install of the control plane + emoji. Everything works as expected.
Logic (and code wise) I have nothing to say; this looks good as is and I wouldn't necessarily make any other changes. Thanks for your time!
$ git rev-parse HEAD
49cce23337922d8344e46947ca9923402612925d
$ bin/linkerd version
Client version: git-49cce233
Server version: git-49cce233
# verify pods are not erroring out
---
$ kubectl get pods -n linkerd
NAME READY STATUS RESTARTS AGE
linkerd-identity-7dc9bfdcf5-8j5gm 2/2 Running 0 27m
linkerd-proxy-injector-59df4c8b4-sbmcd 2/2 Running 0 27m
linkerd-destination-7b47bd7448-f2vqr 3/3 Running 0 27m
$ kubectl get pods -n emojivoto
NAME READY STATUS RESTARTS AGE
voting-ff4c54b8d-g5nrr 2/2 Running 0 21m
emoji-696d9d8f95-r2r7d 2/2 Running 0 21m
vote-bot-6d7677bb68-bmb9t 2/2 Running 0 21m
web-5f86686c4d-rzhbg 2/2 Running 0 21m
---
# verify TLS for a random deploy
---
$ bin/linkerd viz tap deploy/emoji -n emojivoto
rsp id=0:25 proxy=in src=10.42.0.25:35422 dst=10.42.0.23:8080 tls=true :status=200 latency=215µs
end id=0:25 proxy=in src=10.42.0.25:35422 dst=10.42.0.23:8080 tls=true grpc-status=OK duration=22µs response-length=31B
proxy-identity/main.go
Outdated
| name := os.Getenv("LINKERD2_PROXY_IDENTITY_LOCAL_NAME") | ||
| dir := os.Getenv("LINKERD2_PROXY_IDENTITY_DIR") |
There was a problem hiding this comment.
nit: could we move these below the check for envDisabled? If identity is disabled we won't need these variables and they most likely will not even be set.
proxy-identity/main.go
Outdated
| func runProxy() { | ||
| cmd := exec.Command("/usr/lib/linkerd/linkerd2-proxy") | ||
| cmd.Stdout, cmd.Stderr = os.Stdout, os.Stderr | ||
| err := cmd.Run() |
There was a problem hiding this comment.
This isn't really the same as the shell script. Specifically, I think we need to use syscall.Exec so that the proxy can actually receive signals
There was a problem hiding this comment.
I think you are technically right about them not being the same but I think by default both processes would receive signals so for the most part it would behave the same as the shell script. I guess it is pretty easy to test the signal aspect. Anyways I'm happy to make the change if that is what we want to do.
A docker image with a shell is required to run the identity helper The logic for the identity helper shell script docker entry point has been moved into proxy-identity/main.go and the docker file has been updated to reflect the removal of the run-proxy.sh script none Fixes linkerd#6172 Signed-off-by: Taylor Skinner <tskinn12@gmail.com>
|
I've made a few updates:
|
There was a problem hiding this comment.
Thanks @tskinn for making those changes.
I've tested things again and everything works as expected.
A docker image with a shell is required to run the identity helper which is undesirable.
The logic for the identity helper shell script docker entry point has been moved into proxy-identity/main.go and the docker file has been updated to reflect the removal of the run-proxy.sh script
none
Fixes #6172
Signed-off-by: Taylor Skinner tskinn12@gmail.com