-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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: Leader icon for node listing view #6265
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
659ff86
ui: yarn upgrade consul-aoi-double which includes `status/leader`
ec751e0
ui: add all the ember-data things required to call a new endpoint
791e25d
ui: Pass the new leader variable through to the template
d961104
ui: use the new leader variable in the template to set a leader
2300903
ui: add acceptence testing to verify leaders are highlighted
a73307a
ui: Change testing navigation/api requests to status/leader
555103c
ui: Template whitespace commit (less indenting)
70f04bb
ui: adds a test to to assert no errors happen with an unelected leader
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,17 @@ | ||
import RepositoryService from 'consul-ui/services/repository'; | ||
import { inject as service } from '@ember/service'; | ||
import { get } from '@ember/object'; | ||
|
||
const modelName = 'node'; | ||
export default RepositoryService.extend({ | ||
coordinates: service('repository/coordinate'), | ||
getModelName: function() { | ||
return modelName; | ||
}, | ||
findByLeader: function(dc) { | ||
const query = { | ||
dc: dc, | ||
}; | ||
return get(this, 'store').queryLeader(this.getModelName(), query); | ||
}, | ||
}); |
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,72 +1,84 @@ | ||
{{#app-view class="node list"}} | ||
{{#block-slot 'header'}} | ||
<h1> | ||
Nodes <em>{{format-number items.length}} total</em> | ||
</h1> | ||
<label for="toolbar-toggle"></label> | ||
{{/block-slot}} | ||
{{#block-slot 'toolbar'}} | ||
{{#block-slot 'header'}} | ||
<h1> | ||
Nodes <em>{{format-number items.length}} total</em> | ||
</h1> | ||
<label for="toolbar-toggle"></label> | ||
{{/block-slot}} | ||
{{#block-slot 'toolbar'}} | ||
{{#if (gt items.length 0) }} | ||
{{catalog-filter searchable=(array searchableHealthy searchableUnhealthy) search=s status=filters.status onchange=(action 'filter')}} | ||
{{catalog-filter searchable=(array searchableHealthy searchableUnhealthy) search=s status=filters.status onchange=(action 'filter')}} | ||
{{/if}} | ||
{{/block-slot}} | ||
{{#block-slot 'content'}} | ||
{{/block-slot}} | ||
{{#block-slot 'content'}} | ||
{{#if (gt unhealthy.length 0) }} | ||
<div class="unhealthy"> | ||
<h2>Unhealthy Nodes</h2> | ||
<div> | ||
{{! think about 2 differing views here }} | ||
<ul> | ||
{{#changeable-set dispatcher=searchableUnhealthy}} | ||
{{#block-slot 'set' as |unhealthy|}} | ||
{{#each unhealthy as |item|}} | ||
{{healthchecked-resource | ||
tagName='li' | ||
data-test-node=item.Node | ||
href=(href-to 'dc.nodes.show' item.Node) | ||
name=item.Node | ||
address=item.Address | ||
checks=item.Checks | ||
}} | ||
{{/each}} | ||
<div class="unhealthy"> | ||
<h2>Unhealthy Nodes</h2> | ||
<div> | ||
{{! think about 2 differing views here }} | ||
<ul> | ||
{{#changeable-set dispatcher=searchableUnhealthy}} | ||
{{#block-slot 'set' as |unhealthy|}} | ||
{{#each unhealthy as |item|}} | ||
{{#healthchecked-resource | ||
tagName='li' | ||
data-test-node=item.Node | ||
href=(href-to 'dc.nodes.show' item.Node) | ||
name=item.Node | ||
address=item.Address | ||
checks=item.Checks | ||
}} | ||
{{#block-slot 'icon'}} | ||
{{#if (eq item.Address leader.Address)}} | ||
<span data-test-leader={{leader.Address}} data-tooltip="Leader">Leader</span> | ||
{{/if}} | ||
{{/block-slot}} | ||
{{#block-slot 'empty'}} | ||
<p> | ||
There are no unhealthy nodes for that search. | ||
</p> | ||
{{/block-slot}} | ||
{{/changeable-set}} | ||
</ul> | ||
</div> | ||
</div> | ||
{{/if}} | ||
{{#if (gt healthy.length 0) }} | ||
<div class="healthy"> | ||
<h2>Healthy Nodes</h2> | ||
{{#changeable-set dispatcher=searchableHealthy}} | ||
{{#block-slot 'set' as |healthy|}} | ||
{{#list-collection cellHeight=92 items=healthy as |item index|}} | ||
{{healthchecked-resource | ||
data-test-node=item.Node | ||
href=(href-to 'dc.nodes.show' item.Node) | ||
name=item.Node | ||
address=item.Address | ||
checks=item.Checks | ||
}} | ||
{{/list-collection}} | ||
{{/healthchecked-resource}} | ||
{{/each}} | ||
{{/block-slot}} | ||
{{#block-slot 'empty'}} | ||
<p> | ||
There are no healthy nodes for that search. | ||
There are no unhealthy nodes for that search. | ||
</p> | ||
{{/block-slot}} | ||
{{/changeable-set}} | ||
</ul> | ||
</div> | ||
</div> | ||
{{/if}} | ||
{{#if (gt healthy.length 0) }} | ||
<div class="healthy"> | ||
<h2>Healthy Nodes</h2> | ||
{{#changeable-set dispatcher=searchableHealthy}} | ||
{{#block-slot 'set' as |healthy|}} | ||
{{#list-collection cellHeight=92 items=healthy as |item index|}} | ||
{{#healthchecked-resource | ||
data-test-node=item.Node | ||
href=(href-to 'dc.nodes.show' item.Node) | ||
name=item.Node | ||
address=item.Address | ||
checks=item.Checks | ||
}} | ||
{{#block-slot 'icon'}} | ||
{{#if (eq item.Address leader.Address)}} | ||
<span data-test-leader={{leader.Address}} data-tooltip="Leader">Leader</span> | ||
{{/if}} | ||
{{/block-slot}} | ||
{{/healthchecked-resource}} | ||
{{/list-collection}} | ||
{{/block-slot}} | ||
{{#block-slot 'empty'}} | ||
<p> | ||
There are no healthy nodes for that search. | ||
</p> | ||
{{/block-slot}} | ||
{{/changeable-set}} | ||
</div> | ||
{{/if}} | ||
{{#if (and (eq healthy.length 0) (eq unhealthy.length 0)) }} | ||
<p> | ||
There are no nodes. | ||
</p> | ||
<p> | ||
There are no nodes. | ||
</p> | ||
{{/if}} | ||
{{/block-slot}} | ||
{{/block-slot}} | ||
{{/app-view}} |
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,51 @@ | ||
@setupApplicationTest | ||
Feature: Nodes | ||
Scenario: | ||
Feature: dc / nodes / index | ||
Background: | ||
Given 1 datacenter model with the value "dc-1" | ||
And 3 node models | ||
And the url "/v1/status/leader" responds with from yaml | ||
--- | ||
body: | | ||
"211.245.86.75:8500" | ||
--- | ||
Scenario: Viewing nodes in the listing | ||
Given 3 node models | ||
When I visit the nodes page for yaml | ||
--- | ||
dc: dc-1 | ||
--- | ||
Then the url should be /dc-1/nodes | ||
Then I see 3 node models | ||
Scenario: Seeing the leader in unhealthy listing | ||
Given 3 node models from yaml | ||
--- | ||
- Address: 211.245.86.75 | ||
Checks: | ||
- Status: warning | ||
Name: Warning check | ||
- Address: 10.0.0.1 | ||
- Address: 10.0.0.3 | ||
--- | ||
When I visit the nodes page for yaml | ||
--- | ||
dc: dc-1 | ||
--- | ||
Then the url should be /dc-1/nodes | ||
Then I see 3 node models | ||
And I see leader on the unHealthyNodes | ||
Scenario: Seeing the leader in healthy listing | ||
Given 3 node models from yaml | ||
--- | ||
- Address: 211.245.86.75 | ||
Checks: | ||
- Status: passing | ||
Name: Passing check | ||
- Address: 10.0.0.1 | ||
- Address: 10.0.0.3 | ||
--- | ||
When I visit the nodes page for yaml | ||
--- | ||
dc: dc-1 | ||
--- | ||
Then the url should be /dc-1/nodes | ||
Then I see 3 node models | ||
And I see leader on the healthyNodes |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
@setupApplicationTest | ||
Feature: dc / nodes / no-leader | ||
Scenario: Leader hasn't been elected | ||
Given 1 datacenter model with the value "dc-1" | ||
And 3 node models | ||
And the url "/v1/status/leader" responds with from yaml | ||
--- | ||
body: | | ||
"" | ||
--- | ||
When I visit the nodes page for yaml | ||
--- | ||
dc: dc-1 | ||
--- | ||
Then the url should be /dc-1/nodes | ||
Then I see 3 node models | ||
And I don't see leader on the nodes | ||
|
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import steps from '../../steps'; | ||
|
||
// step definitions that are shared between features should be moved to the | ||
// tests/acceptance/steps/steps.js file | ||
|
||
export default function(assert) { | ||
return steps(assert).then('I should find a file', function() { | ||
assert.ok(true, this.step); | ||
}); | ||
} |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @gregone ,
Thanks for looking at this! I was thinking of merging without doing this suggestion, reasons being:
Most of the Consul UI codebase is ES5-y (think ember 2.18 timescales here, 2-3 years back), we generally don't do this kind of assignment (I think the only ES6-y thing we use is the Object.assign-like
{...{}, ...{}}
which was only because we preferred that to having to use the ember providedassign
)Other examples specifically in Repositories are:
consul/ui-v2/app/services/repository/token.js
Lines 27 to 30 in d848637
consul/ui-v2/app/services/repository/session.js
Lines 12 to 15 in d848637
consul/ui-v2/app/services/repository/proxy.js
Lines 13 to 16 in d848637
consul/ui-v2/app/services/repository/kv.js
Lines 27 to 29 in d848637
..but there are a few more around and about.
I can completely see where you are coming from, right now it's one of those consistency/context things. Once we get past ember 3.1* we'll think about moving up a notch, and if we do we'll probably skim the entire codebase in one go so everything is consistent.
Anyway, let me know what you think, happy to add this before merging if you feel strongly about it.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @gregone
We are going to try and get this into our next release, so I'm going to merge this without the suggestion, hope that's ok, thanks for reviewing!