Add a client_state.json admin endpoint to expose the client address set #1768
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…set of each client Signed-off-by: Alex Leong <alex@buoyant.io>
|
cc @obeattie |
olix0r
reviewed
Jan 3, 2018
| override def apply(request: Request): Future[Response] = { | ||
| val entries = ClientRegistry.registrants.map { entry => | ||
| val LoadBalancerFactory.Dest(va) = entry.params[LoadBalancerFactory.Dest] | ||
| va.changes.toFuture().map(entry.addr -> _) |
There was a problem hiding this comment.
can we limit this to entries where entry.addr is a Name or Path so that we don't encode weird things for clients that don't do name resolution?
| override def apply(request: Request): Future[Response] = { | ||
| val entries = ClientRegistry.registrants.map { entry => | ||
| val LoadBalancerFactory.Dest(va) = entry.params[LoadBalancerFactory.Dest] | ||
| va.changes.toFuture().map(entry.addr -> _) |
There was a problem hiding this comment.
Why va.changes.toFuture() and not va.sample()?
| case Addr.Failed(why) => s"Failed: ${why.getMessage}" | ||
| case a@Addr.Pending => a.toString | ||
| case a@Addr.Neg => a.toString | ||
| } |
There was a problem hiding this comment.
wait what is the type of state? This seems a little annoying to decode if the type is "a list or maybe a string".
Perhaps better to have a case class that encodes e.g. "state": "bound", "addrs": [] etc (i'd bet that we even have this somewhere already for namerd's http api)
Signed-off-by: Alex Leong <alex@buoyant.io>
olix0r
reviewed
Jan 4, 2018
| // only display clients with a Path name | ||
| Try(Path.read(entry.addr)).isReturn | ||
| }.map { entry => | ||
| val LoadBalancerFactory.Dest(va) = entry.params[LoadBalancerFactory.Dest] |
There was a problem hiding this comment.
Are we confident that LoadBalancerFactory.Dest's Var is reference-counted? Do we need to ensure this higher up in the stack when we get the result from an interpreter?
olix0r
approved these changes
Jan 5, 2018
Merged
hawkw
added a commit
that referenced
this pull request
Jan 18, 2018
## 1.3.5 2018-01-17 * 🎓 H2 router and `io.l5d.mesh` Namerd interface are no longer experimental ([#1782](#1782))! 🎓 * Add an experimental namer for Rancher service discovery ([#1740](#1740)). A huge thank you to [@fangel](https://github.com/fangel) for contributing this namer! * Kubernetes * Fix a bug that could cause the `io.l5d.k8s` namer to get "stuck" and fail to recieve updates from an endpoint ([#1755](#1755)). Contributed by [@obeattie](https://github.com/obeattie). * Admin UI * Add `/admin/client_state.json` endpoint to return the current address set of each client ([#1768](#1768)). * Fix an error when using the admin UI to perform delegations with a dtab stored in Namerd over the `io.l5d.thriftNameInterpreter` interface ([#1762](#1762)). Thanks to [@jackkleeman](https://github.com/jackkleeman)! * Render an error instead of a blank page for failures on Namerd's dtab playground ([#1770](#1770)). * Namerd * Errors parsing dtabs stored in Consul are now surfaced in log messages ([#1760](#1760)). * Fix an error where Linekrd could sometimes miss updates from Namerd when using the `io.l5d.thriftNameInterpreter` interface ([#1753](#1753)). Thanks to [@obeattie](https://github.com/obeattie)!
|
nit: this PR adds '/client_state.json', but in the 1.3.5 release info the endpoint is '/admin/client_state.json` |
|
@nikolay-pshenichny whoops! Will fix that. |
hawkw
added a commit
that referenced
this pull request
Jan 24, 2018
See #1768 (comment). Signed-off-by: Eliza Weisman <eliza@buoyant.io>
|
Thank you so much for this 🙌 |
hawkw
added a commit
that referenced
this pull request
Jan 24, 2018
See #1768 (comment). Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2 tasks
Tim-Brooks
pushed a commit
to Tim-Brooks/linkerd
that referenced
this pull request
Dec 20, 2018
Jest is faster, has more flexibility to run a subset of the tests, and will allow us to remove a bunch of our assertion libraries. Many thanks to @grampelberg for prior work on this (linkerd#1000) This PR: - changes the test runner from karma to jest - moves individual tests from /test/ to/js/components` where jest expects them
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add client_state.json admin endpoint which outputs the current address set for each client in the client registry.
Sample output:
Signed-off-by: Alex Leong alex@buoyant.io