github
/
consul
mirror of https://github.com/hashicorp/consul
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

ui: Keep track of previous node checks and avoid adding them twice (#9661) 

* Keep track of previous node checks and avoid adding them twice
pull/9668/head
John Cowen 4 years ago committed by GitHub
parent
19fd9a87d1
commit
3477b1de77
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 219 additions and 23 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 33
      ui/packages/consul-ui/app/models/service-instance.js
  2. 62
      ui/packages/consul-ui/app/utils/merge-checks.js
  3. 147
      ui/packages/consul-ui/tests/unit/utils/merge-checks-test.js

33
ui/packages/consul-ui/app/models/service-instance.js
Unescape View File

@ -1,8 +1,9 @@
import Model, { attr, belongsTo } from '@ember-data/model';
import { fragmentArray } from 'ember-data-model-fragments/attributes';
import { computed, get, set } from '@ember/object';
import { computed, get } from '@ember/object';
import { or, filter, alias } from '@ember/object/computed';
import { tracked } from '@glimmer/tracking';
import mergeChecks from 'consul-ui/utils/merge-checks';
export const PRIMARY_KEY = 'uid';
export const SLUG_KEY = 'Node.Node,Service.ID';
@ -52,29 +53,15 @@ export default class ServiceInstance extends Model {
@filter('Checks.@each.Kind', (item, i, arr) => item.Kind === 'node') NodeChecks;
// MeshChecks are a concatenation of Checks for the Instance and Checks for
// the ProxyInstance. Checks is an ember-data-model-fragment, so we can't just
// concat it, we have to loop through all the items in order to merge
@computed('Checks', 'ProxyInstance{Checks,ServiceProxy.Expose.Checks}')
// the ProxyInstance.
@computed('Checks.[]', 'ProxyInstance.{Checks.[],ServiceProxy.Expose.Checks}')
get MeshChecks() {
return (get(this, 'Checks') || [])
.map(item => {
set(
item,
'Exposed',
get(this, 'ProxyInstance.ServiceProxy.Expose.Checks') && get(item, 'Exposable')
);
return item;
})
.concat(
(get(this, 'ProxyInstance.Checks') || []).map(item => {
set(
item,
'Exposed',
get(this, 'ProxyInstance.ServiceProxy.Expose.Checks') && get(item, 'Exposable')
);
return item;
})
);
// merge the instance and proxy checks together, avoiding duplicate node
// checks and additionally setting any checks to exposed if required
return mergeChecks(
[get(this, 'Checks'), get(this, 'ProxyInstance.Checks')],
get(this, 'ProxyInstance.ServiceProxy.Expose.Checks')
);
}
@computed('Service.Meta')

62
ui/packages/consul-ui/app/utils/merge-checks.js
Unescape View File

@ -0,0 +1,62 @@
import { get, set } from '@ember/object';
import MultiMap from 'mnemonist/multi-map';
/**
* Checks are ember-data-model-fragments, so we can't just
* concat it, we have to loop through all the items in order to merge
* We also need to avoid repeating Node checks here as the service and the
* proxy is likely to be on the same node, without adding something extra here
* the node check will likely end up in the list twice.
*
* @param {array} checks - Multiple lists of healthchecks to merge each one of the items in this array should be a further array of healthchecks
* @param {boolean} exposed - Whether the checks should be marked as exposed via the proxy or not
* @param {class} MMap - A MultiMap class. This is only exposed to allow for an easier interface but still allow an innjectable MultiMap if we choose to do that during testing
* @returns {array} - The final array of all of the healthchecks with any duplicate node checks removed, and also marked as exposed if required
*/
export default (checks = [], exposed = false, MMap = MultiMap) => {
const ids = new MMap();
const a = checks.shift();
const result = a
.map(item => {
// its a Node check (ServiceName === ""), record this one so we
// don't end up with duplicates of it
if (item.ServiceName === '') {
ids.set(item.Node, item.CheckID);
}
return item;
})
// go through all remaining lists of checks adding each check to the
// list if its not a node check that has been already added
.concat(
checks.reduce((prev, items) => {
if (typeof items === 'undefined') {
return prev;
}
return prev.concat(
items.reduce((prev, item) => {
if (item.ServiceName === '') {
if ((ids.get(item.Node) || []).includes(item.CheckID)) {
return prev;
}
// if the node check hasn't been added yet, record this one
// so we don't end up with duplicates of it
ids.set(item.Node, item.CheckID);
}
prev.push(item);
return prev;
}, [])
);
}, [])
);
// if checks are exposed via the proxy, find the ones that are exposable
// (ones of a certain type) and set them as exposed
// TODO: consider moving this out of here so we aren't doing too much in one util
if (exposed) {
result
.filter(item => get(item, 'Exposable'))
.forEach(item => {
set(item, 'Exposed', exposed);
});
}
return result;
};

147
ui/packages/consul-ui/tests/unit/utils/merge-checks-test.js
Unescape View File

@ -0,0 +1,147 @@
import mergeChecks from 'consul-ui/utils/merge-checks';
import { module, test } from 'qunit';
module('Unit | Utility | merge-checks', function() {
test('it works', function(assert) {
[
{
desc: 'One list of checks, not exposed',
exposed: false,
checks: [
[
{
ServiceName: 'service-0',
CheckID: 'service-0-check-0',
Node: 'node-0',
Exposable: false,
},
],
],
expected: [
{
ServiceName: 'service-0',
CheckID: 'service-0-check-0',
Node: 'node-0',
Exposable: false,
},
],
},
{
desc: 'One list of checks, exposed',
exposed: true,
checks: [
[
{
ServiceName: 'service-0',
CheckID: 'service-0-check-0',
Node: 'node-0',
Exposable: true,
},
],
],
expected: [
{
ServiceName: 'service-0',
CheckID: 'service-0-check-0',
Node: 'node-0',
Exposable: true,
Exposed: true,
},
],
},
{
desc: 'Two lists of checks, not exposed',
exposed: false,
checks: [
[
{
ServiceName: 'service-0',
CheckID: 'service-0-check-0',
Node: 'node-0',
Exposable: true,
},
],
[
{
ServiceName: 'service-0-proxy',
CheckID: 'service-0-proxy-check-0',
Node: 'node-0',
Exposable: true,
},
],
],
expected: [
{
ServiceName: 'service-0',
CheckID: 'service-0-check-0',
Node: 'node-0',
Exposable: true,
},
{
ServiceName: 'service-0-proxy',
CheckID: 'service-0-proxy-check-0',
Node: 'node-0',
Exposable: true,
},
],
},
{
desc: 'Two lists of checks, with one duplicate node checks, not exposed',
exposed: false,
checks: [
[
{
ServiceName: 'service-0',
CheckID: 'service-0-check-0',
Node: 'node-0',
Exposable: true,
},
{
ServiceName: '',
CheckID: 'service-0-check-1',
Node: 'node-0',
Exposable: true,
},
],
[
{
ServiceName: 'service-0-proxy',
CheckID: 'service-0-proxy-check-0',
Node: 'node-0',
Exposable: true,
},
{
ServiceName: '',
CheckID: 'service-0-check-1',
Node: 'node-0',
Exposable: true,
},
],
],
expected: [
{
ServiceName: 'service-0',
CheckID: 'service-0-check-0',
Node: 'node-0',
Exposable: true,
},
{
ServiceName: '',
CheckID: 'service-0-check-1',
Node: 'node-0',
Exposable: true,
},
{
ServiceName: 'service-0-proxy',
CheckID: 'service-0-proxy-check-0',
Node: 'node-0',
Exposable: true,
},
],
},
].forEach(spec => {
const actual = mergeChecks(spec.checks, spec.exposed);
assert.deepEqual(actual, spec.expected);
});
});
});
Write Preview
Loading…
Cancel
Save