From 9526f9f4f5b1353aee1175d373b10b430c05d2aa Mon Sep 17 00:00:00 2001 From: wenincode Date: Thu, 13 Oct 2022 15:47:47 -0600 Subject: [PATCH 1/5] Filter healthchecks for synthetic-nodes --- .../dc/services/instance/healthchecks.js | 16 ++ .../dc/services/instance/healthchecks.hbs | 158 +++++++++--------- 2 files changed, 98 insertions(+), 76 deletions(-) create mode 100644 ui/packages/consul-ui/app/controllers/dc/services/instance/healthchecks.js diff --git a/ui/packages/consul-ui/app/controllers/dc/services/instance/healthchecks.js b/ui/packages/consul-ui/app/controllers/dc/services/instance/healthchecks.js new file mode 100644 index 000000000..8d6270e82 --- /dev/null +++ b/ui/packages/consul-ui/app/controllers/dc/services/instance/healthchecks.js @@ -0,0 +1,16 @@ +import Controller from '@ember/controller'; +import { action } from '@ember/object'; + +export default class HealthChecksController extends Controller { + @action + syntheticNodeSearchPropertyFilter(item, searchProperty) { + return !(item.Node.Meta?.['synthetic-node'] && searchProperty === 'Node'); + } + + @action + syntheticNodeHealthCheckFilter(item, healthcheck, index, list) { + console.log('List to be filtered: ', list); + console.log(healthcheck.Kind); + return !(item.Node.Meta?.['synthetic-node'] && healthcheck?.Kind === 'node'); + } +} diff --git a/ui/packages/consul-ui/app/templates/dc/services/instance/healthchecks.hbs b/ui/packages/consul-ui/app/templates/dc/services/instance/healthchecks.hbs index 454e92f1a..80f621db5 100644 --- a/ui/packages/consul-ui/app/templates/dc/services/instance/healthchecks.hbs +++ b/ui/packages/consul-ui/app/templates/dc/services/instance/healthchecks.hbs @@ -2,92 +2,98 @@ @name={{routeName}} as |route|> {{#let + (filter (action 'syntheticNodeSearchPropertyFilter' route.model.item) searchProperties) as |filteredSearchProperties|}} + {{#let - (hash - value=(or sortBy "Status:asc") - change=(action (mut sortBy) value="target.selected") - ) + (hash + value=(or sortBy "Status:asc") + change=(action (mut sortBy) value="target.selected") + ) - (hash - status=(hash - value=(if status (split status ',') undefined) - change=(action (mut status) value="target.selectedItems") - ) - check=(hash - value=(if check (split check ',') undefined) - change=(action (mut check) value="target.selectedItems") - ) - searchproperty=(hash - value=(if (not-eq searchproperty undefined) - (split searchproperty ',') - searchProperties + (hash + status=(hash + value=(if status (split status ',') undefined) + change=(action (mut status) value="target.selectedItems") + ) + check=(hash + value=(if check (split check ',') undefined) + change=(action (mut check) value="target.selectedItems") + ) + searchproperty=(hash + value=(if (not-eq searchproperty undefined) + (split searchproperty ',') + filteredSearchProperties + ) + change=(action (mut searchproperty) value="target.selectedItems") + default=filteredSearchProperties ) - change=(action (mut searchproperty) value="target.selectedItems") - default=searchProperties ) - ) - (merge-checks (array route.model.item.Checks route.model.proxy.Checks) route.model.proxy.ServiceProxy.Expose.Checks) + (filter + (action 'syntheticNodeHealthCheckFilter' route.model.item) + (merge-checks (array route.model.item.Checks route.model.proxy.Checks) route.model.proxy.ServiceProxy.Expose.Checks) + ) - as |sort filters items|}} -
+ as |sort filters items|}} +
- {{#if (gt items.length 0) }} - - + - {{/if}} - - {{#let (find-by "Type" "serf" items) as |serf|}} - {{#if (and serf (eq serf.Status "critical"))}} - - -

- {{t "routes.dc.services.instance.healthchecks.critical-serf-notice.header"}} -

- - - {{t - "routes.dc.services.instance.healthchecks.critical-serf-notice.body" - htmlSafe=true - }} - - - {{/if}} - {{/let}} - - - - - - - - {{t "routes.dc.services.instance.healthchecks.empty" - items=items.length - htmlSafe=true - }} - - - - + {{/if}} -
+ {{#let (find-by "Type" "serf" items) as |serf|}} + {{#if (and serf (eq serf.Status "critical"))}} + + +

+ {{t "routes.dc.services.instance.healthchecks.critical-serf-notice.header"}} +

+ + + {{t + "routes.dc.services.instance.healthchecks.critical-serf-notice.body" + htmlSafe=true + }} + + + {{/if}} + {{/let}} + + + + + + + + {{t "routes.dc.services.instance.healthchecks.empty" + items=items.length + htmlSafe=true + }} + + + + + +
+ {{/let}} {{/let}} From c27cc1799131ac3bba13c987991960161350f486 Mon Sep 17 00:00:00 2001 From: wenincode Date: Thu, 13 Oct 2022 15:48:18 -0600 Subject: [PATCH 2/5] Format healthchecks template --- .../dc/services/instance/healthchecks.hbs | 81 +++++++++---------- 1 file changed, 36 insertions(+), 45 deletions(-) diff --git a/ui/packages/consul-ui/app/templates/dc/services/instance/healthchecks.hbs b/ui/packages/consul-ui/app/templates/dc/services/instance/healthchecks.hbs index 80f621db5..f9630f2d3 100644 --- a/ui/packages/consul-ui/app/templates/dc/services/instance/healthchecks.hbs +++ b/ui/packages/consul-ui/app/templates/dc/services/instance/healthchecks.hbs @@ -1,68 +1,59 @@ - + {{#let - (filter (action 'syntheticNodeSearchPropertyFilter' route.model.item) searchProperties) as |filteredSearchProperties|}} + (filter (action 'syntheticNodeSearchPropertyFilter' route.model.item) searchProperties) + as |filteredSearchProperties| + }} {{#let - - (hash - value=(or sortBy "Status:asc") - change=(action (mut sortBy) value="target.selected") - ) - + (hash value=(or sortBy 'Status:asc') change=(action (mut sortBy) value='target.selected')) (hash status=(hash value=(if status (split status ',') undefined) - change=(action (mut status) value="target.selectedItems") + change=(action (mut status) value='target.selectedItems') ) check=(hash value=(if check (split check ',') undefined) - change=(action (mut check) value="target.selectedItems") + change=(action (mut check) value='target.selectedItems') ) searchproperty=(hash - value=(if (not-eq searchproperty undefined) - (split searchproperty ',') - filteredSearchProperties + value=(if + (not-eq searchproperty undefined) (split searchproperty ',') filteredSearchProperties ) - change=(action (mut searchproperty) value="target.selectedItems") + change=(action (mut searchproperty) value='target.selectedItems') default=filteredSearchProperties ) ) - - (filter + (filter (action 'syntheticNodeHealthCheckFilter' route.model.item) - (merge-checks (array route.model.item.Checks route.model.proxy.Checks) route.model.proxy.ServiceProxy.Expose.Checks) + (merge-checks + (array route.model.item.Checks route.model.proxy.Checks) + route.model.proxy.ServiceProxy.Expose.Checks + ) ) + as |sort filters items| + }} +
- as |sort filters items|}} -
- - {{#if (gt items.length 0) }} - + {{#if (gt items.length 0)}} + {{/if}} - {{#let (find-by "Type" "serf" items) as |serf|}} - {{#if (and serf (eq serf.Status "critical"))}} - + {{#let (find-by 'Type' 'serf' items) as |serf|}} + {{#if (and serf (eq serf.Status 'critical'))}} +

- {{t "routes.dc.services.instance.healthchecks.critical-serf-notice.header"}} + {{t 'routes.dc.services.instance.healthchecks.critical-serf-notice.header'}}

{{t - "routes.dc.services.instance.healthchecks.critical-serf-notice.body" + 'routes.dc.services.instance.healthchecks.critical-serf-notice.body' htmlSafe=true }} @@ -70,30 +61,30 @@ as |route|> {{/if}} {{/let}} + as |collection| + > - + - - {{t "routes.dc.services.instance.healthchecks.empty" + + {{t + 'routes.dc.services.instance.healthchecks.empty' items=items.length htmlSafe=true }} - - + + -
+
{{/let}} {{/let}} From e36848111a211e511ee24f3f0f0cd4a9c06eaa57 Mon Sep 17 00:00:00 2001 From: wenincode Date: Thu, 13 Oct 2022 18:45:15 -0600 Subject: [PATCH 3/5] Add tests for filtering node health checks --- .../consul/health-check/list/pageobject.js | 17 ++--- .../dc/services/instance/healthchecks.js | 2 - .../services/instances/health-checks.feature | 65 +++++++++++++++++++ .../dc/services/instances/show.feature | 2 +- .../consul-ui/tests/steps/assertions/model.js | 30 ++++++--- 5 files changed, 97 insertions(+), 19 deletions(-) diff --git a/ui/packages/consul-ui/app/components/consul/health-check/list/pageobject.js b/ui/packages/consul-ui/app/components/consul/health-check/list/pageobject.js index 286933d4c..d69dee72e 100644 --- a/ui/packages/consul-ui/app/components/consul/health-check/list/pageobject.js +++ b/ui/packages/consul-ui/app/components/consul/health-check/list/pageobject.js @@ -1,11 +1,12 @@ export default (collection, text) => (scope = '.consul-health-check-list') => { - return { - scope, - item: collection('li', { - name: text('header h3'), - type: text('[data-health-check-type]'), - exposed: text('[data-test-exposed]'), - }), - }; + return collection({ + scope, + itemScope: 'li', + item: { + name: text('header h2'), + type: text('[data-health-check-type]'), + exposed: text('[data-test-exposed]'), + } + }); }; diff --git a/ui/packages/consul-ui/app/controllers/dc/services/instance/healthchecks.js b/ui/packages/consul-ui/app/controllers/dc/services/instance/healthchecks.js index 8d6270e82..3e80cc0ce 100644 --- a/ui/packages/consul-ui/app/controllers/dc/services/instance/healthchecks.js +++ b/ui/packages/consul-ui/app/controllers/dc/services/instance/healthchecks.js @@ -9,8 +9,6 @@ export default class HealthChecksController extends Controller { @action syntheticNodeHealthCheckFilter(item, healthcheck, index, list) { - console.log('List to be filtered: ', list); - console.log(healthcheck.Kind); return !(item.Node.Meta?.['synthetic-node'] && healthcheck?.Kind === 'node'); } } diff --git a/ui/packages/consul-ui/tests/acceptance/dc/services/instances/health-checks.feature b/ui/packages/consul-ui/tests/acceptance/dc/services/instances/health-checks.feature index ed1e2c923..92f927a62 100644 --- a/ui/packages/consul-ui/tests/acceptance/dc/services/instances/health-checks.feature +++ b/ui/packages/consul-ui/tests/acceptance/dc/services/instances/health-checks.feature @@ -64,3 +64,68 @@ Feature: dc / services / instances / health-checks Then the url should be /dc1/services/service-0/instances/another-node/service-1-with-id/health-checks And I see healthChecksIsSelected on the tabs And I don't see criticalSerfNotice on the tabs.healthChecksTab + Scenario: Node health check should be hidden on agentless service instances + Given 1 instance model from yaml + --- + - Service: + ID: service-0-with-id + Node: + Node: another-node + Meta: + synthetic-node: true + Checks: + - Type: '' + Name: Node Health Check + CheckID: serfHealth + ServiceID: '' + Status: passing + Output: Agent alive and reachable + - Type: '' + Name: Serf Health Status + CheckID: serfHealth + Status: critical + Output: ouch + --- + When I visit the instance page for yaml + --- + dc: dc1 + service: service-0 + node: another-node + id: service-0-with-id + --- + Then the url should be /dc1/services/service-0/instances/another-node/service-0-with-id/health-checks + And I see healthChecksIsSelected on the tabs + And I see 1 healthCheck model on the tabs.healthChecksTab component + And I see 1 healthCheck model with the name "Serf Health Status" + Scenario: Node health checks should be visible on non-agentless service instances + Given 1 instance model from yaml + --- + - Service: + ID: service-0-with-id + Node: + Node: another-node + Checks: + - Type: '' + Name: Node Health Check + CheckID: serfHealth + ServiceID: '' + Status: passing + Output: Agent alive and reachable + - Type: '' + Name: Serf Health Status + CheckID: serfHealth + Status: critical + Output: ouch + --- + When I visit the instance page for yaml + --- + dc: dc1 + service: service-0 + node: another-node + id: service-0-with-id + --- + Then the url should be /dc1/services/service-0/instances/another-node/service-0-with-id/health-checks + And I see healthChecksIsSelected on the tabs + And I see 2 healthCheck models on the tabs.healthChecksTab component + And I see 1 healthCheck model with the name "Serf Health Status" + And I see 1 healthCheck model with the name "Node Health Check" diff --git a/ui/packages/consul-ui/tests/acceptance/dc/services/instances/show.feature b/ui/packages/consul-ui/tests/acceptance/dc/services/instances/show.feature index 55b87cedb..87f8d4b49 100644 --- a/ui/packages/consul-ui/tests/acceptance/dc/services/instances/show.feature +++ b/ui/packages/consul-ui/tests/acceptance/dc/services/instances/show.feature @@ -75,7 +75,7 @@ Feature: dc / services / instances / show: Show Service Instance And I don't see upstreams on the tabs And I see healthChecksIsSelected on the tabs - And I see 6 of the checks object + And I see 6 healthCheck models When I click tags&Meta on the tabs And I see tags&MetaIsSelected on the tabs diff --git a/ui/packages/consul-ui/tests/steps/assertions/model.js b/ui/packages/consul-ui/tests/steps/assertions/model.js index abe0c35cf..bc733d649 100644 --- a/ui/packages/consul-ui/tests/steps/assertions/model.js +++ b/ui/packages/consul-ui/tests/steps/assertions/model.js @@ -1,8 +1,25 @@ export default function (scenario, assert, find, currentPage, pauseUntil, pluralize) { + function getModelItems(model, component) { + let obj; + if (component) { + obj = find(component); + } else { + obj = currentPage(); + } + + let found = obj[pluralize(model)]; + + if (typeof found === 'function') { + found = found(); + } + + return found; + } + scenario .then('pause until I see $number $model model[s]?', function (num, model) { return pauseUntil(function (resolve, reject, retry) { - const len = currentPage()[pluralize(model)].filter(function (item) { + const len = getModelItems(model).filter(function (item) { return item.isVisible; }).length; if (len === num) { @@ -15,8 +32,7 @@ export default function (scenario, assert, find, currentPage, pauseUntil, plural 'pause until I see $number $model model[s]? on the $component component', function (num, model, component) { return pauseUntil(function (resolve, reject, retry) { - const obj = find(component); - const len = obj[pluralize(model)].filter(function (item) { + const len = getModelItems(model, component).filter(function (item) { return item.isVisible; }).length; if (len === num) { @@ -27,7 +43,7 @@ export default function (scenario, assert, find, currentPage, pauseUntil, plural } ) .then(['I see $num $model model[s]?'], function (num, model) { - const len = currentPage()[pluralize(model)].filter(function (item) { + const len = getModelItems(model).filter(function (item) { return item.isVisible; }).length; assert.equal(len, num, `Expected ${num} ${pluralize(model)}, saw ${len}`); @@ -35,15 +51,13 @@ export default function (scenario, assert, find, currentPage, pauseUntil, plural .then( ['I see $num $model model[s]? on the $component component'], function (num, model, component) { - const obj = find(component); - const len = obj[pluralize(model)].filter(function (item) { + const len = getModelItems(model, component).filter(function (item) { return item.isVisible; }).length; assert.equal(len, num, `Expected ${num} ${pluralize(model)}, saw ${len}`); } ) - // TODO: I${ dont } see .then( [`I see $num $model model[s]? with the $property "$value"`], function ( @@ -53,7 +67,7 @@ export default function (scenario, assert, find, currentPage, pauseUntil, plural property, value ) { - const len = currentPage()[pluralize(model)].filter(function (item) { + const len = getModelItems(model).filter(function (item) { if (item.isVisible) { let prop = item[property]; // cope with pageObjects that can have a multiple: true From 229a97967a1d6ce789db0e481da36b5eae55f870 Mon Sep 17 00:00:00 2001 From: wenincode Date: Thu, 13 Oct 2022 18:54:39 -0600 Subject: [PATCH 4/5] Add changelog entry --- .changelog/14986.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/14986.txt diff --git a/.changelog/14986.txt b/.changelog/14986.txt new file mode 100644 index 000000000..c9d20f4b0 --- /dev/null +++ b/.changelog/14986.txt @@ -0,0 +1,3 @@ +```release-note:feature +ui: Filter out node health checks on agentless service instances +``` From b761f583a83e0bac8c72db86ca9234bd77cc3870 Mon Sep 17 00:00:00 2001 From: wenincode Date: Thu, 13 Oct 2022 19:04:36 -0600 Subject: [PATCH 5/5] Address linting errors --- .../consul/health-check/list/pageobject.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ui/packages/consul-ui/app/components/consul/health-check/list/pageobject.js b/ui/packages/consul-ui/app/components/consul/health-check/list/pageobject.js index d69dee72e..63d9438a5 100644 --- a/ui/packages/consul-ui/app/components/consul/health-check/list/pageobject.js +++ b/ui/packages/consul-ui/app/components/consul/health-check/list/pageobject.js @@ -1,12 +1,12 @@ export default (collection, text) => (scope = '.consul-health-check-list') => { return collection({ - scope, - itemScope: 'li', - item: { - name: text('header h2'), - type: text('[data-health-check-type]'), - exposed: text('[data-test-exposed]'), - } - }); + scope, + itemScope: 'li', + item: { + name: text('header h2'), + type: text('[data-health-check-type]'), + exposed: text('[data-test-exposed]'), + }, + }); };