ui: Ensure Service Instance pages account for nodes #5933
Merged
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
backspace
approved these changes
Jun 10, 2019
johncowen
pushed a commit
that referenced
this pull request
Jun 11, 2019
Folllowing on from #5933 we noticed that 'normal' proxies should link to the service, rather than the service instance. Additionally sideproxy 'searching' within the repository should take into account the name of the node that the originating service is on (sidecar proxies are generally co-located) Added an additiojnal test here to prove that a sidecar-proxy with the same service id but on a different node does not show the sidecar proxy link.
johncowen
pushed a commit
that referenced
this pull request
Jun 17, 2019
Folllowing on from #5933 we noticed that 'normal' proxies should link to the service, rather than the service instance. Additionally sideproxy 'searching' within the repository should take into account the name of the node that the originating service is on (sidecar proxies are generally co-located) Added an additiojnal test here to prove that a sidecar-proxy with the same service id but on a different node does not show the sidecar proxy link.
johncowen
added a commit
that referenced
this pull request
Jun 20, 2019
* ui: Normal proxies line to services, sidecars to instances Following on from #5933 we noticed that 'normal' proxies should link to the service, rather than the service instance. Additionally proxy 'searching' within the repository should take into account the name of the node that the originating service is on (sidecar proxies are generally co-located) Added an additional test here to prove that a sidecar-proxy with the same service id but on a different node does not show the sidecar proxy link.
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.
Previously we were using ServiceIds as a unique identifier/slug for linking to Service Instance Detail pages (
/:dc/services/:servicename/:serviceid). ServiceIds are not unique per datacenter, only per Node.This PR includes the node name into the unique identifier/slug (
/:dc/services/:servicename/:nodename/:serviceid) and then instead of just looking for the first piece of data in the API response that has a matching ServiceId, we also check for a matching Node name.This means we then show the correct information on the Service Detail page
Fixes #5853
There is similar work to come that applies a similar fix when linking to Sidecar Proxies (but not 'normal' proxies)