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

ui: [BUGFIX] Ensure we show the correct count of instances for each node #9749

Merged
merged 4 commits into from
Feb 11, 2021

Conversation

johncowen
Copy link
Contributor

In our node listing page we show the a number for the amount of Service Instances on each node.

Screenshot 2021-02-10 at 15 22 28

Before this PR, this count included any proxies on the Node, essentially doubling the service count. This was probably correct in a previous version of the UI when we used to show all the proxies across the catalog as well as the services, but since we merged proxies into the services themselves, essentially making the an implementation detail and therefore invisible in the UI, this count should not include proxies.

This PR adds a MeshServiceInstances property to the node model, 'mesh' here meaning 'services that have a proxy assigned to them, and therefore not a proxy itself'. We then use this property to show the instance count, meaning we omit proxy instances from the final count.

We also noticed that we weren't showing an 'empty' icon for services where the check count for either service or node checks was zero. So fixed that here also 🐦 🐦 🥧

This is how consul looks in the node listing after this fix (the consul service only has node checks, no service checks)

Screenshot 2021-02-10 at 15 25 26

@johncowen johncowen added type/bug Feature does not function as expected theme/ui Anything related to the UI backport/1.9 labels Feb 10, 2021
@johncowen johncowen requested a review from kaxcode February 10, 2021 15:27
@hashicorp-ci
Copy link
Contributor

🤔 Double check that this PR does not require a changelog entry in the .changelog directory. Reference

@vercel vercel bot temporarily deployed to Preview – consul February 10, 2021 15:30 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging February 10, 2021 15:30 Inactive
@johncowen johncowen changed the title ui [BUGFIX] Ensure we show the correct count of instances for each node ui: [BUGFIX] Ensure we show the correct count of instances for each node Feb 10, 2021
Copy link
Contributor

@kaxcode kaxcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@johncowen johncowen merged commit 96204a2 into master Feb 11, 2021
@johncowen johncowen deleted the ui/bugfix/service-instance-checks-counts branch February 11, 2021 11:36
@hashicorp-ci
Copy link
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/326669.

@hashicorp-ci
Copy link
Contributor

🍒✅ Cherry pick of commit 96204a2 onto release/1.9.x succeeded!

hashicorp-ci pushed a commit that referenced this pull request Feb 11, 2021
…ode (#9749)

* Add MeshServiceInstances property to node model

* Use MeshServiceInstances property

* Make sure we show the 'No * checks' if Checks.length is zero
dizzyup pushed a commit that referenced this pull request Apr 21, 2021
…ode (#9749)

* Add MeshServiceInstances property to node model

* Use MeshServiceInstances property

* Make sure we show the 'No * checks' if Checks.length is zero
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI type/bug Feature does not function as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants