Conversation
Signed-off-by: Alex Leong <alex@buoyant.io>
|
Should we call the destination container proxy-api? Perhaps as a followup? |
dadjeibaah
left a comment
dadjeibaah
left a comment
There was a problem hiding this comment.
This change looks good to me. Hooray for making things simpler! 📦
|
|
||
| func main() { | ||
| addr := flag.String("addr", "127.0.0.1:8089", "address to serve on") | ||
| addr := flag.String("addr", ":8086", "address to serve on") |
There was a problem hiding this comment.
Is there a reason we are no longer using 127.0.0.1? I am sure it's not a big deal but just curious
There was a problem hiding this comment.
Yeah, I didn't make this change at first and it took me hours to figure out what was wrong.
This causes the destination server to listen on all interfaces instead of just localhost. If it only listens on localhost then the proxy can't connect to it.
Signed-off-by: Alex Leong <alex@buoyant.io>
klingerf
left a comment
klingerf
left a comment
There was a problem hiding this comment.
Nice cleanup! I just had a few additional naming nits, but otherwise looks good to me.
controller/cmd/proxy-api/main.go
Outdated
| addr := flag.String("addr", ":8086", "address to serve on") | ||
| metricsAddr := flag.String("metrics-addr", ":9996", "address to serve scrapable metrics on") | ||
| destinationAddr := flag.String("destination-addr", "127.0.0.1:8089", "address of destination service") | ||
| metricsAddr := flag.String("metrics-addr", ":9999", "address to serve scrapable metrics on") |
There was a problem hiding this comment.
My (somewhat arbitrary) preference is to continue to run the proxy-api's admin server on 9996. The rest of our controller executables use that convention (e.g. public-api runs on 8085 / 9995; tap runs on 8088 / 9998, etc.).
controller/api/proxy/server.go
Outdated
| enableTLS bool | ||
| } | ||
|
|
||
| // The Destination service serves service discovery information to the proxy. |
There was a problem hiding this comment.
Should this be:
// The proxy-api services serves...
| rand.Seed(time.Now().UnixNano()) | ||
|
|
||
| addr := flag.String("addr", ":8089", "address of destination service") | ||
| addr := flag.String("addr", ":8086", "address of destination service") |
There was a problem hiding this comment.
"address of destination service" => "address of the proxy API" or something similar? On a related note, maybe we should rename this directory from destination-client to proxy-api-client?
There was a problem hiding this comment.
I've gone back and forth on this. You can think of this as a client of the the destination gRPC service, which is served by the proxy-api process. So I can see it either way. What do you think?
There was a problem hiding this comment.
Ah, yeah, I like the framing that you gave. Am fine leaving this as is.
Signed-off-by: Alex Leong <alex@buoyant.io>
klingerf
left a comment
klingerf
left a comment
There was a problem hiding this comment.
⭐️ Great, thanks for updating!
4 participants
A container called
proxy-apiruns in the Linkerd2 controller pod. This container listens on port 8086 and serves the proxy-api but does nothing other than forward gRPC requests to the destination container which listens on port 8089.We remove the proxy-api container altogether and change the destination container to listen on port 8086 instead of 8089. The result is that clients still use the proxy-api by connecting to
proxy-api.<ns>.svc.cluster.local:8086but the controller has one fewer containers. This results in a simpler system that is easier to reason about.Signed-off-by: Alex Leong alex@buoyant.io