luxolus/open-consul
2
0
Fork 0
Code Issues 1 Pull requests Packages Releases Wiki Activity

UI: Add support for blocking queries on the service instance detail page (#5487)

Browse source 
This commit includes several pieces of functionality to enable services
to be removed and the page to present information that this has happened
but also keep the deleted information on the page. Along with the more
usual blocking query based listing.

To enable this:

1. Implements `meta` on the model (only available on collections in
ember)
2. Adds new `catchable` ComputedProperty alongside a `listen` helper for
working with specific errors that can be thrown from EventSources in an
ember-like way. Briefly, normal computed properties update when a
property changes, EventSources can additionally throw errors so we can
catch them and show different visuals based on that.

Also:

Add support for blocking queries on the service instance detail page

1. Previous we could return  undefined when a service instance has no
proxy, but this means we have nothing to attach `meta` to. We've changed
this to return an almost empty object, so with only a meta property.
At first glance there doesn't seem to be any way to provide a proxy
object to templates and be able to detect whether it is actually null
or not so we instead change some conditional logic in the templates to
detect the property we are using to generate the anchor.
2. Made a `pauseUntil` test helper function for steps where we wait for
things. This helps for DRYness but also means if we can move away from
setInterval to something else later, we can do it in one place
3. Whilst running into point 1 here, we managed to make the blocking
queries eternally loop. Whilst this is due to an error in the code and
shouldn't ever happen whilst in actual use, we've added an extra check
so that we only recur/loop the blocking query if the previous response has a
`meta.cursor`

Adds support for blocking queries on the node detail page (#5489)

1. Moves data re-shaping for the templates variables into a repository
so they are easily covered by blocking queries (into coordinatesRepo)
2. The node API returns a 404 as signal for deregistration, we also
close the sessions and coordinates blocking queries when this happens
This commit is contained in:
John Cowen 2019-03-22 17:24:40 +00:00 committed by John Cowen
parent 1625a09372
commit 006b6000a8
23 changed files with 206 additions and 91 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
ui-v2/app/controllers/dc/nodes/show.js
View file

@ -1,9 +1,14 @@
import Controller from '@ember/controller';
import { inject as service } from '@ember/service';
import { get, set, computed } from '@ember/object';
import { alias } from '@ember/object/computed';
import WithSearching from 'consul-ui/mixins/with-searching';
export default Controller.extend(WithSearching, {
import WithEventSource, { listen } from 'consul-ui/mixins/with-event-source';
export default Controller.extend(WithEventSource, WithSearching, {
dom: service('dom'),
notify: service('flashMessages'),
items: alias('item.Services'),
queryParams: {
s: {
as: 'filter',
@ -16,6 +21,21 @@ export default Controller.extend(WithSearching, {
};
this._super(...arguments);
},
item: listen('item').catch(function(e) {
if (e.target.readyState === 1) {
// OPEN
if (get(e, 'error.errors.firstObject.status') === '404') {
get(this, 'notify').add({
destroyOnClick: false,
sticky: true,
type: 'warning',
action: 'update',
});
get(this, 'tomography').close();
get(this, 'sessions').close();
}
}
}),
searchable: computed('items', function() {
return get(this, 'searchables.nodeservice')
.add(get(this, 'items'))
@ -28,7 +48,7 @@ export default Controller.extend(WithSearching, {
// This method is called immediately after `Route::setupController`, and done here rather than there
// as this is a variable used purely for view level things, if the view was different we might not
// need this variable
set(this, 'selectedTab', get(this.item, 'Checks.length') > 0 ? 'health-checks' : 'services');
set(this, 'selectedTab', get(this, 'item.Checks.length') > 0 ? 'health-checks' : 'services');
},
actions: {
change: function(e) {

20
ui-v2/app/controllers/dc/services/instance.js
View file

@ -1,7 +1,10 @@
import Controller from '@ember/controller';
import { set } from '@ember/object';
import { get, set } from '@ember/object';
import { inject as service } from '@ember/service';
import WithEventSource, { listen } from 'consul-ui/mixins/with-event-source';
export default Controller.extend({
export default Controller.extend(WithEventSource, {
notify: service('flashMessages'),
setProperties: function() {
this._super(...arguments);
// This method is called immediately after `Route::setupController`, and done here rather than there
@ -9,6 +12,19 @@ export default Controller.extend({
// need this variable
set(this, 'selectedTab', 'service-checks');
},
item: listen('item').catch(function(e) {
if (e.target.readyState === 1) {
// OPEN
if (get(e, 'error.errors.firstObject.status') === '404') {
get(this, 'notify').add({
destroyOnClick: false,
sticky: true,
type: 'warning',
action: 'update',
});
}
}
}),
actions: {
change: function(e) {
set(this, 'selectedTab', e.target.value);

19
ui-v2/app/instance-initializers/event-source.js
View file

@ -5,7 +5,7 @@ export function initialize(container) {
if (config[enabled] || window.localStorage.getItem(enabled) !== null) {
return;
}
['node', 'service']
['node', 'coordinate', 'session', 'service', 'proxy']
.map(function(item) {
// create repositories that return a promise resolving to an EventSource
return {
@ -20,7 +20,7 @@ export function initialize(container) {
})
.concat([
// These are the routes where we overwrite the 'default'
// repo service. Default repos are repos that return a promise resovlving to
// repo service. Default repos are repos that return a promise resolving to
// an ember-data record or recordset
{
route: 'dc/nodes/index',
@ -28,6 +28,14 @@ export function initialize(container) {
repo: 'repository/node/event-source',
},
},
{
route: 'dc/nodes/show',
services: {
repo: 'repository/node/event-source',
coordinateRepo: 'repository/coordinate/event-source',
sessionRepo: 'repository/session/event-source',
},
},
{
route: 'dc/services/index',
services: {
@ -40,6 +48,13 @@ export function initialize(container) {
repo: 'repository/service/event-source',
},
},
{
route: 'dc/services/instance',
services: {
repo: 'repository/service/event-source',
proxyRepo: 'repository/proxy/event-source',
},
},
])
.forEach(function(definition) {
if (typeof definition.extend !== 'undefined') {

1
ui-v2/app/models/node.js
View file

@ -21,6 +21,7 @@ export default Model.extend({
Datacenter: attr('string'),
Segment: attr(),
Coord: attr(),
meta: attr(),
hasStatus: function(status) {
return hasStatus(get(this, 'Checks'), status);
},

32
ui-v2/app/routes/dc/nodes/show.js
View file

@ -1,17 +1,14 @@
import Route from '@ember/routing/route';
import { inject as service } from '@ember/service';
import { hash } from 'rsvp';
import { get, set } from '@ember/object';
import { get } from '@ember/object';
import distance from 'consul-ui/utils/distance';
import tomographyFactory from 'consul-ui/utils/tomography';
import WithBlockingActions from 'consul-ui/mixins/with-blocking-actions';
const tomography = tomographyFactory(distance);
export default Route.extend(WithBlockingActions, {
repo: service('repository/node'),
sessionRepo: service('repository/session'),
coordinateRepo: service('repository/coordinate'),
queryParams: {
s: {
as: 'filter',
@ -20,24 +17,11 @@ export default Route.extend(WithBlockingActions, {
},
model: function(params) {
const dc = this.modelFor('dc').dc.Name;
const repo = get(this, 'repo');
const sessionRepo = get(this, 'sessionRepo');
const name = params.name;
return hash({
item: repo.findBySlug(params.name, dc),
}).then(function(model) {
// TODO: Consider loading this after initial page load
const coordinates = get(model.item, 'Coordinates');
return hash({
...model,
...{
tomography:
get(coordinates, 'length') > 1
? tomography(params.name, coordinates.map(item => get(item, 'data')))
: null,
items: get(model.item, 'Services'),
sessions: sessionRepo.findByNode(get(model.item, 'Node'), dc),
},
});
item: get(this, 'repo').findBySlug(name, dc),
tomography: get(this, 'coordinateRepo').findAllByNode(name, dc),
sessions: get(this, 'sessionRepo').findByNode(name, dc),
});
},
setupController: function(controller, model) {
@ -52,7 +36,9 @@ export default Route.extend(WithBlockingActions, {
const node = get(item, 'Node');
return repo.remove(item).then(() => {
return repo.findByNode(node, dc).then(function(sessions) {
set(controller, 'sessions', sessions);
controller.setProperties({
sessions: sessions,
});
});
});
}, 'delete');

3
ui-v2/app/routes/dc/services/instance.js
View file

@ -13,6 +13,9 @@ export default Route.extend({
return hash({
item: repo.findInstanceBySlug(params.id, params.name, dc),
}).then(function(model) {
// this will not be run in a blocking loop, but this is ok as
// its highly unlikely that a service will suddenly change to being a
// connect-proxy or vice versa so leave as is for now
return hash({
proxy:
get(model.item, 'Kind') === 'connect-proxy'

15
ui-v2/app/services/repository/coordinate.js
View file

@ -1,8 +1,23 @@
import { get } from '@ember/object';
import RepositoryService from 'consul-ui/services/repository';
import tomographyFactory from 'consul-ui/utils/tomography';
import distance from 'consul-ui/utils/distance';
const tomography = tomographyFactory(distance);
const modelName = 'coordinate';
export default RepositoryService.extend({
getModelName: function() {
return modelName;
},
findAllByNode: function(node, dc, configuration) {
return this.findAllByDatacenter(dc, configuration).then(function(coordinates) {
let results = {};
if (get(coordinates, 'length') > 1) {
results = tomography(node, coordinates.map(item => get(item, 'data')));
}
results.meta = get(coordinates, 'meta');
return results;
});
},
});

11
ui-v2/app/services/repository/node.js
View file

@ -1,20 +1,9 @@
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;
},
findBySlug: function(slug, dc) {
return this._super(...arguments).then(node => {
return get(this, 'coordinates')
.findAllByDatacenter(dc)
.then(function(res) {
node.Coordinates = res;
return node;
});
});
},
});

17
ui-v2/app/services/repository/proxy.js
View file

@ -1,6 +1,6 @@
import RepositoryService from 'consul-ui/services/repository';
import { PRIMARY_KEY } from 'consul-ui/models/proxy';
import { get } from '@ember/object';
import { get, set } from '@ember/object';
const modelName = 'proxy';
export default RepositoryService.extend({
getModelName: function() {
@ -21,17 +21,20 @@ export default RepositoryService.extend({
},
findInstanceBySlug: function(id, slug, dc, configuration) {
return this.findAllBySlug(slug, dc, configuration).then(function(items) {
let res = {};
if (get(items, 'length') > 0) {
let instance = items.findBy('ServiceProxy.DestinationServiceID', id);
if (instance) {
return instance;
}
instance = items.findBy('ServiceProxy.DestinationServiceName', slug);
if (instance) {
return instance;
res = instance;
} else {
instance = items.findBy('ServiceProxy.DestinationServiceName', slug);
if (instance) {
res = instance;
}
}
}
return;
set(res, 'meta', get(items, 'meta'));
return res;
});
},
});

1
ui-v2/app/services/repository/service.js
View file

@ -46,6 +46,7 @@ export default RepositoryService.extend({
service.NodeChecks = item.Nodes[i].Checks.filter(function(item) {
return item.ServiceID == '';
});
set(service, 'meta', get(item, 'meta'));
return service;
}
// TODO: Add an store.error("404", "message") or similar

6
ui-v2/app/templates/dc/nodes/-notifications.hbs
View file

@ -4,5 +4,9 @@
{{else}}
There was an error invalidating the session.
{{/if}}
{{else if (eq type 'update')}}
{{#if (eq status 'warning') }}
This node no longer exists in the catalog.
{{else}}
{{/if}}
{{/if}}

10
ui-v2/app/templates/dc/nodes/show.hbs
View file

@ -18,7 +18,7 @@
(array
'Health Checks'
'Services'
(if tomography 'Round Trip Time' '')
(if tomography.distances 'Round Trip Time' '')
'Lock Sessions'
)
)
@ -48,10 +48,10 @@
{{#each
(compact
(array
(hash id=(slugify 'Health Checks') partial='dc/nodes/healthchecks')
(hash id=(slugify 'Services') partial='dc/nodes/services')
(if tomography (hash id=(slugify 'Round Trip Time') partial='dc/nodes/rtt') '')
(hash id=(slugify 'Lock Sessions') partial='dc/nodes/sessions')
(hash id=(slugify 'Health Checks') partial='dc/nodes/healthchecks')
(hash id=(slugify 'Services') partial='dc/nodes/services')
(if tomography.distances (hash id=(slugify 'Round Trip Time') partial='dc/nodes/rtt') '')
(hash id=(slugify 'Lock Sessions') partial='dc/nodes/sessions')
)
) as |panel|
}}

5
ui-v2/app/templates/dc/services/instance.hbs
View file

@ -1,4 +1,7 @@
{{#app-view class="instance show"}}
{{#block-slot 'notification' as |status type|}}
{{partial 'dc/services/notifications'}}
{{/block-slot}}
{{#block-slot 'breadcrumbs'}}
<ol>
<li><a data-test-back href={{href-to 'dc.services'}}>All Services</a></li>
@ -28,7 +31,7 @@
<dt>Node Name</dt>
<dd><a href="{{href-to 'dc.nodes.show' item.Node.Node}}">{{item.Node.Node}}</a></dd>
</dl>
{{#if proxy}}
{{#if proxy.ServiceName}}
<dl>
<dt data-test-proxy-type="{{if proxy.ServiceProxy.DestinationServiceID "sidecar-proxy" "proxy"}}">{{if proxy.ServiceProxy.DestinationServiceID "Sidecar " ""}}Proxy</dt>
<dd><a href="{{href-to 'dc.services.instance' proxy.ServiceName proxy.ServiceID}}">{{proxy.ServiceID}}</a></dd>

10
ui-v2/tests/acceptance/dc/list-blocking.feature
View file

@ -46,11 +46,9 @@ Feature: dc / list-blocking
And an external edit results in 1 [Model] model
And pause until I see 1 [Model] model
And an external edit results in 0 [Model] models
And pause for 300
And I see the text "deregistered" in "[data-notification]"
And pause until I see the text "deregistered" in "[data-notification]"
Where:
------------------------------------------------
| Page | Model | Url |
-------------------------------------------------
| Page | Model | Url |
| service | instance | services/service-0 |
------------------------------------------------
-------------------------------------------------

32
ui-v2/tests/acceptance/dc/nodes/show.feature
View file

@ -1,8 +1,9 @@
@setupApplicationTest
Feature: dc / nodes / show: Show node
Scenario: Given 2 nodes all the tabs are visible and clickable
Background:
Given 1 datacenter model with the value "dc1"
And 2 node models from yaml
Scenario: Given 2 nodes all the tabs are visible and clickable
Given 2 node models from yaml
When I visit the node page for yaml
---
dc: dc1
@ -19,8 +20,7 @@ Feature: dc / nodes / show: Show node
When I click lockSessions on the tabs
And I see lockSessionsIsSelected on the tabs
Scenario: Given 1 node all the tabs are visible and clickable and the RTT one isn't there
Given 1 datacenter model with the value "dc1"
And 1 node models from yaml
Given 1 node models from yaml
---
ID: node-0
---
@ -39,8 +39,7 @@ Feature: dc / nodes / show: Show node
When I click lockSessions on the tabs
And I see lockSessionsIsSelected on the tabs
Scenario: Given 1 node with no checks all the tabs are visible but the Services tab is selected
Given 1 datacenter model with the value "dc1"
And 1 node models from yaml
Given 1 node models from yaml
---
ID: node-0
Checks: []
@ -55,3 +54,24 @@ Feature: dc / nodes / show: Show node
And I see roundTripTime on the tabs
And I see lockSessions on the tabs
And I see servicesIsSelected on the tabs
Scenario: A node warns when deregistered whilst blocking
Given 1 node model from yaml
---
ID: node-0
---
And settings from yaml
---
consul:client:
blocking: 1
throttle: 200
---
And a network latency of 100
When I visit the node page for yaml
---
dc: dc1
node: node-0
---
Then the url should be /dc1/nodes/node-0
And the url "/v1/internal/ui/node/node-0" responds with a 404 status
And pause until I see the text "no longer exists" in "[data-notification]"

21
ui-v2/tests/acceptance/dc/services/instances/show.feature
View file

@ -1,6 +1,6 @@
@setupApplicationTest
Feature: dc / services / instances / show: Show Service Instance
Scenario: A Service instance has no Proxy
Background:
Given 1 datacenter model with the value "dc1"
And 1 service model from yaml
---
@ -41,6 +41,7 @@ Feature: dc / services / instances / show: Show Service Instance
DestinationServiceName: service-1
DestinationServiceID: ~
---
Scenario: A Service instance has no Proxy
When I visit the instance page for yaml
---
dc: dc1
@ -49,7 +50,6 @@ Feature: dc / services / instances / show: Show Service Instance
---
Then the url should be /dc1/services/service-0/service-0-with-id
Then I don't see type on the proxy
Then I see externalSource like "nomad"
And I don't see upstreams on the tabs
@ -65,4 +65,21 @@ Feature: dc / services / instances / show: Show Service Instance
Then I see the text "Tag1" in "[data-test-tags] span:nth-child(1)"
Then I see the text "Tag2" in "[data-test-tags] span:nth-child(2)"
Scenario: A Service instance warns when deregistered whilst blocking
Given settings from yaml
---
consul:client:
blocking: 1
throttle: 200
---
And a network latency of 100
When I visit the instance page for yaml
---
dc: dc1
service: service-0
id: service-0-with-id
---
Then the url should be /dc1/services/service-0/service-0-with-id
And an external edit results in 0 instance models
And pause until I see the text "deregistered" in "[data-notification]"

4
ui-v2/tests/integration/services/repository/node-test.js
View file

@ -55,6 +55,10 @@ test('findBySlug returns the correct data for item endpoint', function(assert) {
return Object.assign({}, item, {
Datacenter: dc,
uid: `["${dc}","${item.ID}"]`,
meta: {
date: undefined,
cursor: undefined,
},
});
})
);

20
ui-v2/tests/steps.js
View file

@ -21,6 +21,22 @@ export default function(assert, library, pages, utils) {
return page;
};
const pauseUntil = function(cb) {
return new Promise(function(resolve, reject) {
let count = 0;
const interval = setInterval(function() {
if (++count >= 50) {
clearInterval(interval);
assert.ok(false);
reject();
}
cb(function() {
clearInterval(interval);
resolve();
});
}, 100);
});
};
models(library, utils.create);
http(library, utils.respondWith, utils.set);
visit(library, pages, setCurrentPage);
@ -28,9 +44,9 @@ export default function(assert, library, pages, utils) {
form(library, utils.fillIn, utils.triggerKeyEvent, getCurrentPage);
debug(library, assert, utils.currentURL);
assertHttp(library, assert, utils.lastNthRequest);
assertModel(library, assert, getCurrentPage, utils.pluralize);
assertModel(library, assert, getCurrentPage, pauseUntil, utils.pluralize);
assertPage(library, assert, getCurrentPage);
assertDom(library, assert, utils.find, utils.currentURL);
assertDom(library, assert, pauseUntil, utils.find, utils.currentURL);
return library.given(["I'm using a legacy token"], function(number, model, data) {
window.localStorage['consul:token'] = JSON.stringify({ AccessorID: null, SecretID: 'id' });

14
ui-v2/tests/steps/assertions/dom.js
View file

@ -1,5 +1,17 @@
export default function(scenario, assert, find, currentURL) {
export default function(scenario, assert, pauseUntil, find, currentURL) {
scenario
.then('pause until I see the text "$text" in "$selector"', function(text, selector) {
return pauseUntil(function(resolve) {
const $el = find(selector);
if ($el) {
const hasText = $el.textContent.indexOf(text) !== -1;
if (hasText) {
assert.ok(hasText, `Expected to see "${text}" in "${selector}"`);
resolve();
}
}
});
})
.then(['I see the text "$text" in "$selector"'], function(text, selector) {
assert.ok(
find(selector).textContent.indexOf(text) !== -1,

27
ui-v2/tests/steps/assertions/model.js
View file

@ -1,23 +1,14 @@
export default function(scenario, assert, currentPage, pluralize) {
export default function(scenario, assert, currentPage, pauseUntil, pluralize) {
scenario
.then('pause until I see $number $model model[s]?', function(num, model) {
return new Promise(function(resolve) {
let count = 0;
const interval = setInterval(function() {
if (++count >= 50) {
clearInterval(interval);
assert.ok(false);
resolve();
}
const len = currentPage()[pluralize(model)].filter(function(item) {
return item.isVisible;
}).length;
if (len === num) {
clearInterval(interval);
assert.equal(len, num, `Expected ${num} ${model}s, saw ${len}`);
resolve();
}
}, 100);
return pauseUntil(function(resolve) {
const len = currentPage()[pluralize(model)].filter(function(item) {
return item.isVisible;
}).length;
if (len === num) {
assert.equal(len, num, `Expected ${num} ${model}s, saw ${len}`);
resolve();
}
});
})
.then(['I see $num $model model[s]?'], function(num, model) {

2
ui-v2/tests/unit/controllers/dc/nodes/show-test.js
View file

@ -2,7 +2,7 @@ import { moduleFor, test } from 'ember-qunit';
moduleFor('controller:dc/nodes/show', 'Unit | Controller | dc/nodes/show', {
// Specify the other units that are required for this test.
needs: ['service:search', 'service:dom'],
needs: ['service:search', 'service:dom', 'service:flashMessages'],
});
// Replace this with your real tests.

2
ui-v2/tests/unit/controllers/dc/services/instance-test.js
View file

@ -2,7 +2,7 @@ import { moduleFor, test } from 'ember-qunit';
moduleFor('controller:dc/services/instance', 'Unit | Controller | dc/services/instance', {
// Specify the other units that are required for this test.
// needs: ['controller:foo']
needs: ['service:dom', 'service:flashMessages'],
});
// Replace this with your real tests.

1
ui-v2/tests/unit/routes/dc/nodes/show-test.js
View file

@ -4,6 +4,7 @@ moduleFor('route:dc/nodes/show', 'Unit | Route | dc/nodes/show', {
// Specify the other units that are required for this test.
needs: [
'service:repository/node',
'service:repository/coordinate',
'service:repository/session',
'service:feedback',
'service:logger',