Update linkerd top to use Tap APIService#3186
Merged
Conversation
PR #3167 introduced a Tap APIService, and migrated `linkerd tap` to it. This change migrates `linkerd top` to the new Tap APIService. It also addresses a `panic: close of closed channel` issue, where two go routines could both call `close(done)` on exit. Fixes #3168 Signed-off-by: Andrew Seigner <[email protected]>
Collaborator
|
Integration test results for dc05096: success 🎉 |
siggy
added a commit
that referenced
this pull request
Aug 2, 2019
PR #3167 introduced a Tap APIService, and migrated linkerd tap to it. This change migrates `linkerd profile --tap` to the new Tap APIService. Depends on #3186 Fixes #3169 Signed-off-by: Andrew Seigner <[email protected]>
kleimkuhler
reviewed
Aug 2, 2019
Signed-off-by: Andrew Seigner <[email protected]>
b8ea6d7 to
c7c3f87
Compare
siggy
added a commit
that referenced
this pull request
Aug 2, 2019
PR #3167 introduced a Tap APIService, and migrated linkerd tap to it. This change migrates `linkerd profile --tap` to the new Tap APIService. Depends on #3186 Fixes #3169 Signed-off-by: Andrew Seigner <[email protected]>
kleimkuhler
approved these changes
Aug 2, 2019
Contributor
kleimkuhler
left a comment
There was a problem hiding this comment.
Awesome--looks good!
siggy
added a commit
that referenced
this pull request
Aug 2, 2019
PR #3167 introduced a Tap APIService, and migrated linkerd tap to it. This change migrates `linkerd profile --tap` to the new Tap APIService. Depends on #3186 Fixes #3169 Signed-off-by: Andrew Seigner <[email protected]>
Collaborator
|
Integration test results for c7c3f87: success 🎉 |
siggy
added a commit
that referenced
this pull request
Aug 2, 2019
PR #3167 introduced a Tap APIService, and migrated linkerd tap to it. This change migrates `linkerd profile --tap` to the new Tap APIService. Depends on #3186 Fixes #3169 Signed-off-by: Andrew Seigner <[email protected]>
cpretzer
pushed a commit
that referenced
this pull request
Aug 6, 2019
PR #3167 introduced a Tap APIService, and migrated `linkerd tap` to it. This change migrates `linkerd top` to the new Tap APIService. It also addresses a `panic: close of closed channel` issue, where two go routines could both call `close(done)` on exit. Fixes #3168 Signed-off-by: Andrew Seigner <[email protected]>
cpretzer
pushed a commit
that referenced
this pull request
Aug 6, 2019
PR #3167 introduced a Tap APIService, and migrated linkerd tap to it. This change migrates `linkerd profile --tap` to the new Tap APIService. Depends on #3186 Fixes #3169 Signed-off-by: Andrew Seigner <[email protected]>
kleimkuhler
added a commit
that referenced
this pull request
Aug 8, 2019
### Motivation PR #3167 introduced the tap APIService and migrated `linkerd tap` to use it. Subsequent PRs (#3186 and #3187) updated `linkerd top` and `linkerd profile --tap` to use the tap APIService. This PR moves the web's Go server to now also use the tap APIService instead of the public API. It also ensures an error banner is shown to the user when unauthorized taps fail via `linkerd top` command in *Overview* and *Top*, and `linkerd tap` command in *Tap*. ### Details The majority of these changes are focused around piping through the HTTP error that occurs and making sure the error banner generated displays the error message explaining to view the tap RBAC docs. `httpError` is now public (`HTTPError`) and the error message generated is short enough to fit in a control frame (explained [here](https://github.com/linkerd/linkerd2/blob/kleimkuhler%2Fweb-tap-apiserver/web/srv/api_handlers.go#L173-L175)). ### Testing The error we are testing for only occurs when the linkerd-web service account is not authorzied to tap resources. Unforutnately that is not the case on Docker For Mac (assuming that is what you use locally), so you'll need to test on a different cluster. I chose a GKE cluster made through the GKE console--not made through cluster-utils because it adds cluster-admin. Checkout the branch locally and `bin/docker-build` or `ares-build` if you have it setup. It should produce a linkerd with the version `git-04e61786`. I have already pushed the dependent components, so you won't need to `bin/docker-push git-04e61786`. Install linkerd on this GKE cluster and try to run `tap` or `top` commands via the web. You should see the following errors: ### Tap  ### Top  Signed-off-by: Kevin Leimkuhler <[email protected]>
This file contains hidden or 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
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.
PR #3167 introduced a Tap APIService, and migrated
linkerd tapto it.This change migrates
linkerd topto the new Tap APIService. It alsoaddresses a
panic: close of closed channelissue, where two goroutines could both call
close(done)on exit.Fixes #3168
Signed-off-by: Andrew Seigner [email protected]