From 7238089fc7591d51f7a558e5f4b592764cd6322c Mon Sep 17 00:00:00 2001 From: John Cowen Date: Fri, 21 Aug 2020 08:53:22 +0100 Subject: [PATCH] ui: Add new Service.Mesh* properties for improved sorting (#8542) * ui: Serialize proxies into the model, add Mesh* model props Serializes the proxies associated with a service onto the Service model itself, then adds various Mesh* properties * ui: Uses the new Mesh* properties throughout the app --- .../components/consul-service-list/index.hbs | 14 +++---- .../components/consul-upstream-list/index.hbs | 8 ++-- ui-v2/app/controllers/dc/services/index.js | 18 --------- ui-v2/app/helpers/service/health-checks.js | 19 --------- ui-v2/app/models/service.js | 36 +++++++++++++++++ ui-v2/app/serializers/service.js | 35 ++++++++++++++++ ui-v2/app/sort/comparators/service.js | 12 +++--- ui-v2/app/templates/dc/services/index.hbs | 2 +- .../integration/serializers/service-test.js | 4 +- .../services/repository/service-test.js | 40 ------------------- .../unit/sort/comparators/service-test.js | 38 +++++++++--------- 11 files changed, 109 insertions(+), 117 deletions(-) delete mode 100644 ui-v2/app/helpers/service/health-checks.js diff --git a/ui-v2/app/components/consul-service-list/index.hbs b/ui-v2/app/components/consul-service-list/index.hbs index 0328a0e68..783f0b594 100644 --- a/ui-v2/app/components/consul-service-list/index.hbs +++ b/ui-v2/app/components/consul-service-list/index.hbs @@ -1,19 +1,17 @@ {{#if (gt items.length 0)}} -{{#let (get proxies item.Name) as |proxy|}} - {{#let (service/health-checks item proxy) as |health|}} -
+
Health
- {{#if (eq 'critical' health)}} + {{#if (eq 'critical' item.MeshStatus)}} At least one health check on one instance is failing. - {{else if (eq 'warning' health)}} + {{else if (eq 'warning' item.MeshStatus)}} At least one health check on one instance has a warning. - {{else if (eq 'passing' health)}} + {{else if (eq 'passing' item.MeshStatus)}} All health checks are passing. {{else}} There are no health checks. @@ -21,8 +19,6 @@
- {{/let}} -{{/let}} {{#if (gt item.InstanceCount 0)}} {{#if (eq item.Kind 'terminating-gateway')}} @@ -65,7 +61,7 @@ {{format-number item.InstanceCount}} {{pluralize item.InstanceCount 'Instance' without-count=true}} {{/if}} - {{#if (get proxies item.Name)}} + {{#if item.Proxy}}
diff --git a/ui-v2/app/components/consul-upstream-list/index.hbs b/ui-v2/app/components/consul-upstream-list/index.hbs index 72dc36bf1..5668c935b 100644 --- a/ui-v2/app/components/consul-upstream-list/index.hbs +++ b/ui-v2/app/components/consul-upstream-list/index.hbs @@ -1,17 +1,17 @@ {{#if (gt item.InstanceCount 0)}} -
+
Health
- {{#if (eq 'critical' (service/health-checks item))}} + {{#if (eq 'critical' item.MeshStatus)}} At least one health check on one instance is failing. - {{else if (eq 'warning' (service/health-checks item))}} + {{else if (eq 'warning' item.MeshStatus)}} At least one health check on one instance has a warning. - {{else if (eq 'passing' (service/health-checks item))}} + {{else if (eq 'passing' item.MeshStatus)}} All health checks are passing. {{else}} There are no health checks. diff --git a/ui-v2/app/controllers/dc/services/index.js b/ui-v2/app/controllers/dc/services/index.js index d3e8b11c6..deebfa8e0 100644 --- a/ui-v2/app/controllers/dc/services/index.js +++ b/ui-v2/app/controllers/dc/services/index.js @@ -13,22 +13,4 @@ export default Controller.extend({ return item.Kind !== 'connect-proxy'; }); }), - proxies: computed('items.[]', function() { - const proxies = {}; - this.items - .filter(function(item) { - return item.Kind === 'connect-proxy'; - }) - .forEach(item => { - // Iterating to cover the usecase of a proxy being - // used by more than one service - if (item.ProxyFor) { - item.ProxyFor.forEach(service => { - proxies[service] = item; - }); - } - }); - - return proxies; - }), }); diff --git a/ui-v2/app/helpers/service/health-checks.js b/ui-v2/app/helpers/service/health-checks.js deleted file mode 100644 index 635c2f8f0..000000000 --- a/ui-v2/app/helpers/service/health-checks.js +++ /dev/null @@ -1,19 +0,0 @@ -import { helper } from '@ember/component/helper'; - -export function healthChecks( - [item, proxy = { ChecksCritical: 0, ChecksWarning: 0, ChecksPassing: 0 }], - hash -) { - switch (true) { - case item.ChecksCritical !== 0 || proxy.ChecksCritical !== 0: - return 'critical'; - case item.ChecksWarning !== 0 || proxy.ChecksWarning !== 0: - return 'warning'; - case item.ChecksPassing !== 0 || proxy.ChecksPassing !== 0: - return 'passing'; - default: - return 'empty'; - } -} - -export default helper(healthChecks); diff --git a/ui-v2/app/models/service.js b/ui-v2/app/models/service.js index 18a739fa1..a6a30e1df 100644 --- a/ui-v2/app/models/service.js +++ b/ui-v2/app/models/service.js @@ -15,6 +15,7 @@ export default Model.extend({ }), InstanceCount: attr('number'), ProxyFor: attr(), + Proxy: attr(), Kind: attr('string'), ExternalSources: attr(), GatewayConfig: attr(), @@ -37,6 +38,41 @@ export default Model.extend({ Checks: attr(), SyncTime: attr('number'), meta: attr(), + /* Mesh properties involve both the service and the associated proxy */ + MeshStatus: computed('MeshChecksPassing', 'MeshChecksWarning', 'MeshChecksCritical', function() { + switch (true) { + case this.MeshChecksCritical !== 0: + return 'critical'; + case this.MeshChecksWarning !== 0: + return 'warning'; + case this.MeshChecksPassing !== 0: + return 'passing'; + default: + return 'empty'; + } + }), + MeshChecksPassing: computed('ChecksPassing', 'Proxy.ChecksPassing', function() { + let proxyCount = 0; + if (typeof this.Proxy !== 'undefined') { + proxyCount = this.Proxy.ChecksPassing; + } + return this.ChecksPassing + proxyCount; + }), + MeshChecksWarning: computed('ChecksWarning', 'Proxy.ChecksWarning', function() { + let proxyCount = 0; + if (typeof this.Proxy !== 'undefined') { + proxyCount = this.Proxy.ChecksWarning; + } + return this.ChecksWarning + proxyCount; + }), + MeshChecksCritical: computed('ChecksCritical', 'Proxy.ChecksCritical', function() { + let proxyCount = 0; + if (typeof this.Proxy !== 'undefined') { + proxyCount = this.Proxy.ChecksCritical; + } + return this.ChecksCritical + proxyCount; + }), + /**/ passing: computed('ChecksPassing', 'Checks', function() { let num = 0; // TODO: use typeof diff --git a/ui-v2/app/serializers/service.js b/ui-v2/app/serializers/service.js index 16a25a1e7..4edaa7ada 100644 --- a/ui-v2/app/serializers/service.js +++ b/ui-v2/app/serializers/service.js @@ -5,6 +5,41 @@ import { get } from '@ember/object'; export default Serializer.extend({ primaryKey: PRIMARY_KEY, slugKey: SLUG_KEY, + respondForQuery: function(respond, query) { + return this._super( + cb => + respond((headers, body) => { + // Services and proxies all come together in the same list + // Here we map the proxies to their related services on a Service.Proxy + // property for easy access later on + const services = {}; + body + .filter(function(item) { + return item.Kind !== 'connect-proxy'; + }) + .forEach(item => { + services[item.Name] = item; + }); + body + .filter(function(item) { + return item.Kind === 'connect-proxy'; + }) + .forEach(item => { + // Iterating to cover the usecase of a proxy being + // used by more than one service + if (item.ProxyFor) { + item.ProxyFor.forEach(service => { + if (typeof services[service] !== 'undefined') { + services[service].Proxy = item; + } + }); + } + }); + return cb(headers, body); + }), + query + ); + }, respondForQueryRecord: function(respond, query) { // Name is added here from the query, which is used to make the uid // Datacenter gets added in the ApplicationSerializer diff --git a/ui-v2/app/sort/comparators/service.js b/ui-v2/app/sort/comparators/service.js index a5c906d10..f353f74bf 100644 --- a/ui-v2/app/sort/comparators/service.js +++ b/ui-v2/app/sort/comparators/service.js @@ -11,21 +11,21 @@ export default () => key => { b = serviceB; } switch (true) { - case a.ChecksCritical > b.ChecksCritical: + case a.MeshChecksCritical > b.MeshChecksCritical: return 1; - case a.ChecksCritical < b.ChecksCritical: + case a.MeshChecksCritical < b.MeshChecksCritical: return -1; default: switch (true) { - case a.ChecksWarning > b.ChecksWarning: + case a.MeshChecksWarning > b.MeshChecksWarning: return 1; - case a.ChecksWarning < b.ChecksWarning: + case a.MeshChecksWarning < b.MeshChecksWarning: return -1; default: switch (true) { - case a.ChecksPassing < b.ChecksPassing: + case a.MeshChecksPassing < b.MeshChecksPassing: return 1; - case a.ChecksPassing > b.ChecksPassing: + case a.MeshChecksPassing > b.MeshChecksPassing: return -1; } } diff --git a/ui-v2/app/templates/dc/services/index.hbs b/ui-v2/app/templates/dc/services/index.hbs index e64c8e99f..b9395b31d 100644 --- a/ui-v2/app/templates/dc/services/index.hbs +++ b/ui-v2/app/templates/dc/services/index.hbs @@ -59,7 +59,7 @@ {{#let (sort-by (comparator 'service' sort) services) as |sorted|}} - + diff --git a/ui-v2/tests/integration/serializers/service-test.js b/ui-v2/tests/integration/serializers/service-test.js index 1d50230d7..636f64471 100644 --- a/ui-v2/tests/integration/serializers/service-test.js +++ b/ui-v2/tests/integration/serializers/service-test.js @@ -37,7 +37,9 @@ module('Integration | Serializer | service', function(hooks) { ns: nspace, } ); - assert.deepEqual(actual, expected); + assert.equal(actual[0].Namespace, expected[0].Namespace); + assert.equal(actual[0].Datacenter, expected[0].Datacenter); + assert.equal(actual[0].uid, expected[0].uid); }); }); test(`respondForQuery returns the correct data for list endpoint when gateway is set when nspace is ${nspace}`, function(assert) { diff --git a/ui-v2/tests/integration/services/repository/service-test.js b/ui-v2/tests/integration/services/repository/service-test.js index ace26d0bd..a3c87edbe 100644 --- a/ui-v2/tests/integration/services/repository/service-test.js +++ b/ui-v2/tests/integration/services/repository/service-test.js @@ -1,5 +1,4 @@ import { moduleFor, test } from 'ember-qunit'; -import { skip } from 'qunit'; import repo from 'consul-ui/tests/helpers/repo'; import { get } from '@ember/object'; const NAME = 'service'; @@ -7,7 +6,6 @@ moduleFor(`service:repository/${NAME}`, `Integration | Service | ${NAME}`, { // Specify the other units that are required for this test. integration: true, }); -skip('findBySlug returns a sane tree'); const dc = 'dc-1'; const id = 'token-name'; const now = new Date().getTime(); @@ -41,44 +39,6 @@ const undefinedNspace = 'default'; }); }); - test(`findByDatacenter returns the correct data for list endpoint when nspace is ${nspace}`, function(assert) { - get(this.subject(), 'store').serializerFor(NAME).timestamp = function() { - return now; - }; - return repo( - 'Service', - 'findAllByDatacenter', - this.subject(), - function retrieveStub(stub) { - return stub( - `/v1/internal/ui/services?dc=${dc}${ - typeof nspace !== 'undefined' ? `&ns=${nspace}` : `` - }`, - { - CONSUL_SERVICE_COUNT: '100', - } - ); - }, - function performTest(service) { - return service.findAllByDatacenter(dc, nspace || undefinedNspace); - }, - function performAssertion(actual, expected) { - assert.deepEqual( - actual, - expected(function(payload) { - return payload.map(item => - Object.assign({}, item, { - SyncTime: now, - Datacenter: dc, - Namespace: item.Namespace || undefinedNspace, - uid: `["${item.Namespace || undefinedNspace}","${dc}","${item.Name}"]`, - }) - ); - }) - ); - } - ); - }); test(`findBySlug returns the correct data for item endpoint when the nspace is ${nspace}`, function(assert) { return repo( 'Service', diff --git a/ui-v2/tests/unit/sort/comparators/service-test.js b/ui-v2/tests/unit/sort/comparators/service-test.js index 073ff39c9..d857ec366 100644 --- a/ui-v2/tests/unit/sort/comparators/service-test.js +++ b/ui-v2/tests/unit/sort/comparators/service-test.js @@ -8,22 +8,22 @@ module('Unit | Sort | Comparator | service', function() { const actual = comparator(expected); assert.equal(actual, expected); }); - test('items are sorted by a fake Status which uses Checks{Passing,Warning,Critical}', function(assert) { + test('items are sorted by a fake Status which uses MeshChecks{Passing,Warning,Critical}', function(assert) { const items = [ { - ChecksPassing: 1, - ChecksWarning: 1, - ChecksCritical: 1, + MeshChecksPassing: 1, + MeshChecksWarning: 1, + MeshChecksCritical: 1, }, { - ChecksPassing: 1, - ChecksWarning: 1, - ChecksCritical: 2, + MeshChecksPassing: 1, + MeshChecksWarning: 1, + MeshChecksCritical: 2, }, { - ChecksPassing: 1, - ChecksWarning: 1, - ChecksCritical: 3, + MeshChecksPassing: 1, + MeshChecksWarning: 1, + MeshChecksCritical: 3, }, ]; const comp = comparator('Status:asc'); @@ -31,19 +31,19 @@ module('Unit | Sort | Comparator | service', function() { const expected = [ { - ChecksPassing: 1, - ChecksWarning: 1, - ChecksCritical: 3, + MeshChecksPassing: 1, + MeshChecksWarning: 1, + MeshChecksCritical: 3, }, { - ChecksPassing: 1, - ChecksWarning: 1, - ChecksCritical: 2, + MeshChecksPassing: 1, + MeshChecksWarning: 1, + MeshChecksCritical: 2, }, { - ChecksPassing: 1, - ChecksWarning: 1, - ChecksCritical: 1, + MeshChecksPassing: 1, + MeshChecksWarning: 1, + MeshChecksCritical: 1, }, ]; let actual = items.sort(comp);