Skip to content
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

Closed
olix0r opened this issue Jun 3, 2019 · 0 comments · Fixed by #2990
Closed

endpoints: Use the public Destination APIs #2885

olix0r opened this issue Jun 3, 2019 · 0 comments · Fixed by #2990
Assignees
Labels
area/cli priority/P1 Planned for Release

Comments

@olix0r
Copy link
Member

olix0r commented Jun 3, 2019

The linkerd endpoints is tightly coupled to the internal representation of the destination service, without much concrete benefit.

I propose the following:

  • The linkerd endpoints CLI takes a list of (fully qualified) authorities, e.g. app.ns.svc.cluster.local:1234
  • The CLI uses the public Destination.Get method to describe the state of the service.
@olix0r olix0r added area/cli priority/P0 Release Blocker labels Jun 3, 2019
@alpeb alpeb self-assigned this Jun 11, 2019
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 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>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/cli priority/P1 Planned for Release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants