-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
endpoints: Use the public Destination APIs #2885
Labels
Comments
alpeb
added a commit
that referenced
this issue
Jun 13, 2019
The plan for #2885 is to have `linkerd endpoints` hit the `Destination.Get` gRPC endpoint. Given this will probably be leveraged from the dashboard as well, it made sense to do like with the other endpoints and serve it through the public api http server and from there forward to the Destination service. The easy way would have been to just add the Destination endpoints into /controller/api/public/client.go and complete the plumbing. But I wanted to experiment using a separate new client /controller/api/destination/client.go to try keeping things separate. This implied extracting out of /controller/api/public some things that I moved up one level into /controller/api to be reused by any client under that directory. Now both the public API and the Destination clients use a new `portforward.Init()` method that captures the common logic for building the port-forwarding, and then each client builds a separate `grpcOverHTTPClient` struct that only implements the methods relevant to each client. I also created /controller/api/stream_client.go with the struct and common methods needed for our gRPC streaming endpoints: Tap and Destination.Get. On the gRPC server side of things, one could make a similar refactoring to avoid grouping together all the endpoints, but I skipped that for this PR, and just did the normal plumbing. /cli/cmd/endpoints.go changed `requestEndpointsFromAPI()` just to be able to test the new Destination endpoint for a particular emoji authority (you can test with just `linkerd endpoints`). No processing of the output has been done yet. Let me know what you think. Was this too much? Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>
grampelberg
added
priority/P1
Planned for Release
and removed
priority/P0
Release Blocker
labels
Jun 17, 2019
alpeb
added a commit
that referenced
this issue
Jun 24, 2019
Fixes #2885 (By popular demand, this supersedes #2937 and refactors `linkerd endpoints` without creating a separate client for the Destination gRPC methods) To recapitulate #2885: we're refactoring `linkerd endpoints` so it hits directly the `Destination.Get` endpoint, instead of relying on the Discovery service. For that, I've created a new `client.go` for Destination and added it to the `APIClient` interface. I've also added a `destinationClient` struct that mimics `tapClient`, and whose common logic has been moved into `stream_client.go`. Analogously, I added a `destinationServer` struct that mimics `tapServer`. Still to do: - Remove everything related to the Discovery service? - Fix tests Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>
alpeb
added a commit
that referenced
this issue
Jul 3, 2019
* Have `linkerd endpoints` use `Destination.Get` Fixes #2885 We're refactoring `linkerd endpoints` so it hits directly the `Destination.Get` endpoint, instead of relying on the Discovery service. For that, I've created a new `client.go` for Destination and added it to the `APIClient` interface. I've also added a `destinationClient` struct that mimics `tapClient`, and whose common logic has been moved into `stream_client.go`. Analogously, I added a `destinationServer` struct that mimics `tapServer`. Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The
linkerd endpoints
is tightly coupled to the internal representation of the destination service, without much concrete benefit.I propose the following:
linkerd endpoints
CLI takes a list of (fully qualified) authorities, e.g. app.ns.svc.cluster.local:1234The text was updated successfully, but these errors were encountered: