ui: Serf Health Check warning notice (#10194)

When the Consul serf health check is failing, this means that the health checks registered with the agent may no longer be correct. Therefore we show a notice to the user when we detect that the serf health check is failing both for the health check listing for nodes and for service instances.

There were a few little things we fixed up whilst we were here:

- We use our @replace decorator to replace an empty Type with serf in the model.
- We noticed that ServiceTags can be null, so we replace that with an empty array.
- We added docs for both our Notice component and the Consul::HealthCheck::List component. Notice now defaults to @type=info.

.changelog/10194.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+ui: Show a message to explain that health checks may be out of date if the serf health check is in a critical state
+```

ui/packages/consul-ui/app/components/consul/health-check/list/README.mdx

@@ -0,0 +1,33 @@
+# Consul::HealthCheck::List
+
+A presentational component for rendering HealthChecks.
+
+```hbs preview-template
+<figure>
+  <figcaption>Grab some mock data...</figcaption>
+
+  <DataSource @src="/default/dc-1/node/my-node" as |source|>
+    <figure>
+      <figcaption>but only show a max of 2 items for docs purposes</figcaption>
+
+      <Consul::HealthCheck::List
+        @items={{slice 0 2 source.data.Checks}}
+      />
+
+    </figure>
+  </DataSource>
+
+</figure>
+```
+
+## Arguments
+
+| Argument/Attribute | Type | Default | Description |
+| --- | --- | --- | --- |
+| `items` | `array` |  | An array of HealthChecks |
+
+## See
+
+- [Template Source Code](./index.hbs)
+
+---

ui/packages/consul-ui/app/components/consul/health-check/list/index.hbs

@@ -25,7 +25,7 @@
         <dl>
           <dt>Type</dt>
           <dd data-health-check-type>
-            {{or item.Type 'serf'}}
+            {{item.Type}}
   {{#if item.Exposed}}
             <em
               data-test-exposed="true"

ui/packages/consul-ui/app/components/notice/README.mdx

@@ -0,0 +1,49 @@
+# Notice
+
+Presentational component for informational/warning/error banners/notices.
+
+
+```hbs preview-template
+<Notice
+  @type={{this.type}}
+as |notice|>
+  <notice.Header>
+    <h3>Header</h3>
+  </notice.Header>
+  <notice.Body>
+    <p>
+      Body
+    </p>
+  </notice.Body>
+  <notice.Footer>
+    <p>
+      <a href="">Footer link</a>
+    </p>
+  </notice.Footer>
+</Notice>
+
+<figure>
+  <figcaption>Provide a widget to change the <code>@type</code></figcaption>
+
+  <select
+    onchange={{action (mut this.type) value="target.value"}}
+  >
+    <option>info</option>
+    <option>warning</option>
+    <option>error</option>
+  </select>
+
+</figure>
+```
+
+## Arguments
+
+| Argument/Attribute | Type | Default | Description |
+| --- | --- | --- | --- |
+| `type` | `String` | `info` | Type of notice [info\|warning\|error] |
+
+## See
+
+- [Template Source Code](./index.hbs)
+
+---

ui/packages/consul-ui/app/components/notice/README.stories.mdx

@@ -1,46 +0,0 @@
-import { Meta, Story, Canvas } from '@storybook/addon-docs/blocks';
-import { hbs } from 'ember-cli-htmlbars';
-
-<Meta title="Components/Notice" component="Notice" />
-
-# Notice
-
-<Canvas>
-  <Story name="Basic"
-    argTypes={{
-      type: {
-        defaultValue: 'success',
-        control: {
-          type: 'select',
-          options: [
-            'success',
-            'warning',
-            'info',
-            'highlight',
-          ]
-        }
-      }
-    }}
-  >{(args) => ({
-    template: hbs`<Notice
-        @type={{type}}
-      as |notice|>
-        <notice.Header>
-          <h3>Header</h3>
-        </notice.Header>
-        <notice.Body>
-          <p>
-            Body
-          </p>
-        </notice.Body>
-        <notice.Footer>
-          <p>
-            Footer
-          </p>
-        </notice.Footer>
-      </Notice>`,
-    context: args
-  })}
-  </Story>
-</Canvas>
-

ui/packages/consul-ui/app/components/notice/index.hbs

@@ -1,5 +1,5 @@
 <div
-  class="notice {{@type}}"
+  class="notice {{or @type 'info'}}"
   ...attributes
 >
 {{yield (hash

ui/packages/consul-ui/app/decorators/replace.js

@@ -2,7 +2,7 @@
  * Simple replacing decorator, with the primary usecase for avoiding null API
  * errors by decorating model attributes: @replace(null, []) @attr() Tags;
  */
-const replace = (find, replace) => (target, propertyKey, desc) => {
+export const replace = (find, replace) => (target, propertyKey, desc) => {
   return {
     get: function() {
       const value = desc.get.apply(this, arguments);

ui/packages/consul-ui/app/filter/predicates/health-check.js

@@ -9,7 +9,7 @@ export default {
     node: (item, value) => item.Kind === value,
   },
   check: {
-    serf: (item, value) => item.Type === '',
+    serf: (item, value) => item.Type === value,
     script: (item, value) => item.Type === value,
     http: (item, value) => item.Type === value,
     tcp: (item, value) => item.Type === value,

ui/packages/consul-ui/app/models/health-check.js

@@ -2,27 +2,29 @@ import Fragment from 'ember-data-model-fragments/fragment';
 import { array } from 'ember-data-model-fragments/attributes';
 import { attr } from '@ember-data/model';
 import { computed } from '@ember/object';
+import { replace, nullValue } from 'consul-ui/decorators/replace';
 
 export const schema = {
   Status: {
     allowedValues: ['passing', 'warning', 'critical'],
   },
   Type: {
-    allowedValues: ['', 'script', 'http', 'tcp', 'ttl', 'docker', 'grpc', 'alias'],
+    allowedValues: ['serf', 'script', 'http', 'tcp', 'ttl', 'docker', 'grpc', 'alias'],
   },
 };
 
 export default class HealthCheck extends Fragment {
   @attr('string') Name;
   @attr('string') CheckID;
-  @attr('string') Type;
+  // an empty Type means its the Consul serf Check
+  @replace('', 'serf') @attr('string') Type;
   @attr('string') Status;
   @attr('string') Notes;
   @attr('string') Output;
   @attr('string') ServiceName;
   @attr('string') ServiceID;
   @attr('string') Node;
-  @array('string') ServiceTags;
+  @nullValue([]) @array('string') ServiceTags;
   @attr() Definition; // {}
 
   // Exposed is only set correct if this Check is accessed via instance.MeshChecks

ui/packages/consul-ui/app/search/predicates/health-check.js

@@ -9,5 +9,5 @@ export default {
   ID: item => item.Service.ID || '',
   Notes: item => item.Notes,
   Output: item => item.Output,
-  ServiceTags: item => asArray(item.ServiceTags || []),
+  ServiceTags: item => asArray(item.ServiceTags),
 };

ui/packages/consul-ui/app/styles/debug.scss

@@ -39,13 +39,13 @@ html.is-debug body > .brand-loader {
       background-color: white;
       margin-bottom: 2rem;
     }
-    ol,
+    > ol,
-    ul {
+    > ul {
       list-style-position: outside;
       margin-bottom: 1rem;
       margin-left: 2rem;
     }
-    ul {
+    > ul {
       list-style-type: disc;
     }
   }
@@ -74,11 +74,18 @@ html.is-debug body > .brand-loader {
         color: var(--gray-400);
         font-style: italic;
       }
+      figcaption code {
+        @extend %inline-code;
+      }
       figure > [type='text'] {
         border: 1px solid var(--gray-999);
         width: 100%;
         padding: 0.5rem;
       }
+      figure > select {
+        border: 1px solid var(--gray-999);
+        padding: 0.5rem;
+      }
     }
     // &__snippets__tabs__button {
     //   display: none;

ui/packages/consul-ui/app/templates/dc/nodes/show/healthchecks.hbs

@@ -47,6 +47,26 @@ as |route|>
               @filter={{filters}}
             />
           {{/if}}
+          {{#let (find-by "Type" "serf" items) as |serf|}}
+            {{#if (and serf (eq serf.Status "critical"))}}
+              <Notice
+                data-test-critical-serf-notice
+                @type="warning"
+              as |notice|>
+                <notice.Header>
+                  <h2>
+                    {{t "routes.dc.nodes.show.healthchecks.critical-serf-notice.header"}}
+                  </h2>
+                </notice.Header>
+                <notice.Body>
+                  {{t
+                    "routes.dc.nodes.show.healthchecks.critical-serf-notice.body"
+                    htmlSafe=true
+                  }}
+                </notice.Body>
+              </Notice>
+            {{/if}}
+          {{/let}}
           <DataCollection
             @type="health-check"
             @sort={{sort.value}}
@@ -62,9 +82,10 @@ as |route|>
             <collection.Empty>
               <EmptyState>
                 <BlockSlot @name="body">
-                  <p>
+                    {{t "routes.dc.nodes.show.healthchecks.empty"
-                    This node has no health checks{{#if (gt items.length 0)}} matching that search{{/if}}.
+                      items=items.length
-                  </p>
+                      htmlSafe=true
+                    }}
                 </BlockSlot>
               </EmptyState>
               </collection.Empty>

ui/packages/consul-ui/app/templates/dc/services/instance/healthchecks.hbs

@@ -44,6 +44,26 @@ as |route|>
         />
       {{/if}}
 
+      {{#let (find-by "Type" "serf" items) as |serf|}}
+        {{#if (and serf (eq serf.Status "critical"))}}
+          <Notice
+            data-test-critical-serf-notice
+            @type="warning"
+          as |notice|>
+            <notice.Header>
+              <h2>
+                {{t "routes.dc.services.instance.healthchecks.critical-serf-notice.header"}}
+              </h2>
+            </notice.Header>
+            <notice.Body>
+              {{t
+                "routes.dc.services.instance.healthchecks.critical-serf-notice.body"
+                htmlSafe=true
+              }}
+            </notice.Body>
+          </Notice>
+        {{/if}}
+      {{/let}}
       <DataCollection
         @type="health-check"
         @sort={{sort.value}}
@@ -59,9 +79,10 @@ as |route|>
         <collection.Empty>
           <EmptyState>
             <BlockSlot @name="body">
-              <p>
+              {{t "routes.dc.services.instance.healthchecks.empty"
-                This instance has no health checks{{#if (gt items.length 0)}} matching that search{{/if}}.
+                items=items.length
-              </p>
+                htmlSafe=true
+              }}
             </BlockSlot>
           </EmptyState>
           </collection.Empty>

ui/packages/consul-ui/tests/acceptance/dc/nodes/show.feature

@@ -2,8 +2,9 @@
 Feature: dc / nodes / show: Show node
   Background:
     Given 1 datacenter model with the value "dc1"
+  # 2 nodes are required for the RTT tab to be visible
   Scenario: Given 2 nodes all the tabs are visible and clickable
-    Given 2 node models from yaml
+    Given 2 node models
     When I visit the node page for yaml
     ---
       dc: dc1

ui/packages/consul-ui/tests/acceptance/dc/nodes/show/health-checks.feature

@@ -0,0 +1,40 @@
+@setupApplicationTest
+Feature: dc / nodes / show / health-checks
+  Background:
+    Given 1 datacenter model with the value "dc1"
+  Scenario: A failing serf check
+    Given 1 node model from yaml
+    ---
+    ID: node-0
+    Checks:
+      - Type: ''
+        Name: Serf Health Status
+        CheckID: serfHealth
+        Status: critical
+        Output: ouch
+    ---
+    When I visit the node page for yaml
+    ---
+      dc: dc1
+      node: node-0
+    ---
+    And I see healthChecksIsSelected on the tabs
+    And I see criticalSerfNotice on the tabs.healthChecksTab
+  Scenario: A passing serf check
+    Given 1 node model from yaml
+    ---
+    ID: node-0
+    Checks:
+      - Type: ''
+        Name: Serf Health Status
+        CheckID: serfHealth
+        Status: passing
+        Output: Agent alive and reachable
+    ---
+    When I visit the node page for yaml
+    ---
+      dc: dc1
+      node: node-0
+    ---
+    And I see healthChecksIsSelected on the tabs
+    And I don't see criticalSerfNotice on the tabs.healthChecksTab

ui/packages/consul-ui/tests/acceptance/dc/services/instances/health-checks.feature

@@ -0,0 +1,66 @@
+@setupApplicationTest
+Feature: dc / services / instances / health-checks
+  Background:
+    Given 1 datacenter model with the value "dc1"
+    And 1 proxy model from yaml
+    ---
+    - ServiceProxy:
+        DestinationServiceName: service-1
+        DestinationServiceID: ~
+    ---
+  Scenario: A failing serf check
+    Given 2 instance models from yaml
+    ---
+    - Service:
+        ID: service-0-with-id
+      Node:
+        Node: node-0
+    - Service:
+        ID: service-1-with-id
+      Node:
+        Node: another-node
+      Checks:
+        - 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-1-with-id
+    ---
+    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 see criticalSerfNotice on the tabs.healthChecksTab
+  Scenario: A passing serf check
+    Given 2 instance models from yaml
+    ---
+    - Service:
+        ID: service-0-with-id
+      Node:
+        Node: node-0
+    - Service:
+        ID: service-1-with-id
+      Node:
+        Node: another-node
+      Checks:
+        - Type: ''
+          Name: Serf Health Status
+          CheckID: serfHealth
+          Status: passing
+          Output: Agent alive and reachable
+    ---
+    When I visit the instance page for yaml
+    ---
+      dc: dc1
+      service: service-0
+      node: another-node
+      id: service-1-with-id
+    ---
+    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

ui/packages/consul-ui/tests/acceptance/steps/dc/nodes/show/health-checks-steps.js

@@ -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);
+  });
+}

ui/packages/consul-ui/tests/acceptance/steps/dc/services/instances/health-checks-steps.js

@@ -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);
+  });
+}

ui/packages/consul-ui/tests/pages.js

@@ -164,6 +164,7 @@ export default {
       visitable,
       alias,
       attribute,
+      isPresent,
       collection,
       text,
       tabgroup,
@@ -177,7 +178,9 @@ export default {
       visitable,
       deletable,
       clickable,
+      alias,
       attribute,
+      isPresent,
       collection,
       tabgroup,
       text,

ui/packages/consul-ui/tests/pages/dc/nodes/show.js

@@ -2,13 +2,15 @@ export default function(
   visitable,
   deletable,
   clickable,
+  alias,
   attribute,
+  present,
   collection,
   tabs,
   text,
   healthChecks
 ) {
-  return {
+  const page = {
     visit: visitable('/:dc/nodes/:node'),
     tabs: tabs('tab', [
       'health-checks',
@@ -17,7 +19,7 @@ export default function(
       'lock-sessions',
       'metadata',
     ]),
-    healthChecks: healthChecks(),
+    healthChecks: alias('tabs.healthChecksTab.healthChecks'),
     services: collection('.consul-service-instance-list > ul > li:not(:first-child)', {
       name: text('[data-test-service-name]'),
       port: attribute('data-test-service-port', '[data-test-service-port]'),
@@ -31,4 +33,9 @@ export default function(
     }),
     metadata: collection('.consul-metadata-list [data-test-tabular-row]', {}),
   };
+  page.tabs.healthChecksTab = {
+    criticalSerfNotice: present('[data-test-critical-serf-notice]'),
+    healthChecks: healthChecks(),
+  };
+  return page;
 }

ui/packages/consul-ui/tests/pages/dc/services/instance.js

@@ -2,13 +2,14 @@ export default function(
   visitable,
   alias,
   attribute,
+  present,
   collection,
   text,
   tabs,
   upstreams,
   healthChecks
 ) {
-  return {
+  const page = {
     visit: visitable('/:dc/services/:service/instances/:node/:id'),
     externalSource: attribute('data-test-external-source', '[data-test-external-source]', {
       scope: '.title',
@@ -26,4 +27,9 @@ export default function(
     }),
     metadata: collection('.metadata [data-test-tabular-row]', {}),
   };
+  page.tabs.healthChecksTab = {
+    criticalSerfNotice: present('[data-test-critical-serf-notice]'),
+    healthChecks: healthChecks(),
+  };
+  return page;
 }

ui/packages/consul-ui/translations/routes/en-us.yaml

@@ -1,4 +1,20 @@
 dc:
+  nodes:
+    show:
+      healthchecks:
+        empty: |
+          <p>
+            This node has no health checks{items, select,
+              0 {}
+              other { matching that search}
+            }.
+          </p>
+        critical-serf-notice:
+          header: Failing serf check
+          body: |
+            <p>
+              This node has a failing serf node check. The health statuses shown on this page are the statuses as they were known before the node became unreachable.
+            </p>
   services:
     show:
       upstreams:
@@ -7,6 +23,20 @@ dc:
             Upstreams are services that may receive traffic from this gateway. If you are not using Consul DNS, please make sure your <code>Host:</code> header uses the correct domain name for the gateway to correctly proxy to its upstreams. Learn more about configuring gateways in our <a href="{CONSUL_DOCS_URL}/connect/ingress-gateways" target="_blank" rel="noopener noreferrer">documentation</a>.
           </p>
     instance:
+      healthchecks:
+        empty: |
+          <p>
+            This instance has no health checks{items, select,
+              0 {}
+              other { matching that search}
+            }.
+          </p>
+        critical-serf-notice:
+          header: Failing serf check
+          body: |
+            <p>
+              This instance has a failing serf node check. The health statuses shown on this page are the statuses as they were known before the node became unreachable.
+            </p>
       upstreams:
         tproxy-mode:
           header: Transparent proxy mode