Update client list to combine statuses (#5789)

The draining, eligibility, and status fields now all show under a combined
state column. Draining takes precedence, then (in)eligibility; if neither of
those is true, the status displays.

CHANGELOG.md

@@ -4,6 +4,7 @@ IMPROVEMENTS:
 
 * api: use region from job hcl when not provided as query parameter in job registration and plan endpoints [[GH-5664](https://github.com/hashicorp/nomad/pull/5664)]
 * metrics: add namespace label as appropriate to metrics [[GH-5847](https://github.com/hashicorp/nomad/issues/5847)]
+* ui: Moved client status, draining, and eligibility fields into single state column [[GH-5789](https://github.com/hashicorp/nomad/pull/5789)]
 
 BUG FIXES:
 

ui/app/controllers/clients/index.js

@@ -19,9 +19,8 @@ export default Controller.extend(Sortable, Searchable, {
     sortProperty: 'sort',
     sortDescending: 'desc',
     qpClass: 'class',
-    qpStatus: 'status',
+    qpState: 'state',
     qpDatacenter: 'dc',
-    qpFlags: 'flags',
   },
 
   currentPage: 1,
@@ -33,14 +32,12 @@ export default Controller.extend(Sortable, Searchable, {
   searchProps: computed(() => ['id', 'name', 'datacenter']),
 
   qpClass: '',
-  qpStatus: '',
+  qpState: '',
   qpDatacenter: '',
-  qpFlags: '',
 
   selectionClass: selection('qpClass'),
-  selectionStatus: selection('qpStatus'),
+  selectionState: selection('qpState'),
   selectionDatacenter: selection('qpDatacenter'),
-  selectionFlags: selection('qpFlags'),
 
   optionsClass: computed('nodes.[]', function() {
     const classes = Array.from(new Set(this.nodes.mapBy('nodeClass'))).compact();
@@ -53,47 +50,42 @@ export default Controller.extend(Sortable, Searchable, {
     return classes.sort().map(dc => ({ key: dc, label: dc }));
   }),
 
-  optionsStatus: computed(() => [
+  optionsState: computed(() => [
     { key: 'initializing', label: 'Initializing' },
     { key: 'ready', label: 'Ready' },
     { key: 'down', label: 'Down' },
+    { key: 'ineligible', label: 'Ineligible' },
+    { key: 'draining', label: 'Draining' },
   ]),
 
   optionsDatacenter: computed('nodes.[]', function() {
     const datacenters = Array.from(new Set(this.nodes.mapBy('datacenter'))).compact();
 
     // Remove any invalid datacenters from the query param/selection
     scheduleOnce('actions', () => {
-      this.set(
-        'qpDatacenter',
-        serialize(intersection(datacenters, this.selectionDatacenter))
-      );
+      this.set('qpDatacenter', serialize(intersection(datacenters, this.selectionDatacenter)));
     });
 
     return datacenters.sort().map(dc => ({ key: dc, label: dc }));
   }),
 
-  optionsFlags: computed(() => [
-    { key: 'ineligible', label: 'Ineligible' },
-    { key: 'draining', label: 'Draining' },
-  ]),
-
   filteredNodes: computed(
     'nodes.[]',
     'selectionClass',
-    'selectionStatus',
+    'selectionState',
     'selectionDatacenter',
-    'selectionFlags',
     function() {
       const {
         selectionClass: classes,
-        selectionStatus: statuses,
+        selectionState: states,
         selectionDatacenter: datacenters,
-        selectionFlags: flags,
       } = this;
 
-      const onlyIneligible = flags.includes('ineligible');
-      const onlyDraining = flags.includes('draining');
+      const onlyIneligible = states.includes('ineligible');
+      const onlyDraining = states.includes('draining');
+
+      // states is a composite of node status and other node states
+      const statuses = states.without('ineligible').without('draining');
 
       return this.nodes.filter(node => {
         if (classes.length && !classes.includes(node.get('nodeClass'))) return false;

ui/app/templates/clients/index.hbs

@@ -20,23 +20,17 @@
             selection=selectionClass
             onSelect=(action setFacetQueryParam "qpClass")}}
           {{multi-select-dropdown
-            data-test-status-facet
-            label="Status"
-            options=optionsStatus
-            selection=selectionStatus
-            onSelect=(action setFacetQueryParam "qpStatus")}}
+            data-test-state-facet
+            label="State"
+            options=optionsState
+            selection=selectionState
+            onSelect=(action setFacetQueryParam "qpState")}}
           {{multi-select-dropdown
             data-test-datacenter-facet
             label="Datacenter"
             options=optionsDatacenter
             selection=selectionDatacenter
             onSelect=(action setFacetQueryParam "qpDatacenter")}}
-          {{multi-select-dropdown
-            data-test-flags-facet
-            label="Flags"
-            options=optionsFlags
-            selection=selectionFlags
-            onSelect=(action setFacetQueryParam "qpFlags")}}
         </div>
       </div>
     </div>
@@ -53,9 +47,7 @@
           <th class="is-narrow"></th>
           {{#t.sort-by prop="id"}}ID{{/t.sort-by}}
           {{#t.sort-by class="is-200px is-truncatable" prop="name"}}Name{{/t.sort-by}}
-          {{#t.sort-by prop="status"}}Status{{/t.sort-by}}
-          {{#t.sort-by prop="isDraining"}}Drain{{/t.sort-by}}
-          {{#t.sort-by prop="schedulingEligibility"}}Eligibility{{/t.sort-by}}
+          {{#t.sort-by prop="status"}}State{{/t.sort-by}}
           <th>Address</th>
           {{#t.sort-by prop="datacenter"}}Datacenter{{/t.sort-by}}
           <th># Allocs</th>

ui/app/templates/components/client-node-row.hbs

@@ -7,20 +7,16 @@
 </td>
 <td data-test-client-id>{{#link-to "clients.client" node.id class="is-primary"}}{{node.shortId}}{{/link-to}}</td>
 <td data-test-client-name class="is-200px is-truncatable" title="{{node.name}}">{{node.name}}</td>
-<td data-test-client-status>{{node.status}}</td>
-<td data-test-client-drain>
-  {{#if node.isDraining}}
-    <span class="status-text is-info">true</span>
-  {{else}}
-    false
-  {{/if}}
-</td>
-<td data-test-client-eligibility>
-  {{#if node.isEligible}}
-    {{node.schedulingEligibility}}
-  {{else}}
-    <span class="status-text is-warning">{{node.schedulingEligibility}}</span>
-  {{/if}}
+<td data-test-client-state>
+  <span class="tooltip" aria-label="{{node.status}} / {{if node.isDraining "draining" "not draining"}} / {{if node.isEligible "eligible" "not eligible"}}">
+    {{#if node.isDraining}}
+      <span class="status-text is-info">draining</span>
+    {{else if (not node.isEligible)}}
+      <span class="status-text is-warning">ineligible</span>
+    {{else}}
+      {{node.status}}
+    {{/if}}
+  </span>
 </td>
 <td data-test-client-address>{{node.httpAddr}}</td>
 <td data-test-client-datacenter>{{node.datacenter}}</td>

ui/tests/acceptance/clients-list-test.js

@@ -4,11 +4,6 @@ import { setupApplicationTest } from 'ember-qunit';
 import setupMirage from 'ember-cli-mirage/test-support/setup-mirage';
 import ClientsList from 'nomad-ui/tests/pages/clients/list';
 
-function minimumSetup() {
-  server.createList('node', 1);
-  server.createList('agent', 1);
-}
-
 module('Acceptance | clients list', function(hooks) {
   setupApplicationTest(hooks);
   setupMirage(hooks);
@@ -34,8 +29,8 @@ module('Acceptance | clients list', function(hooks) {
   });
 
   test('each client record should show high-level info of the client', async function(assert) {
-    minimumSetup();
-    const node = server.db.nodes[0];
+    const node = server.create('node', 'draining');
+    server.createList('agent', 1);
 
     await ClientsList.visit();
 
@@ -44,16 +39,66 @@ module('Acceptance | clients list', function(hooks) {
 
     assert.equal(nodeRow.id, node.id.split('-')[0], 'ID');
     assert.equal(nodeRow.name, node.name, 'Name');
-    assert.equal(nodeRow.status, node.status, 'Status');
-    assert.equal(nodeRow.drain, node.drain + '', 'Draining');
-    assert.equal(nodeRow.eligibility, node.schedulingEligibility, 'Eligibility');
+    assert.equal(nodeRow.state.text, 'draining', 'Combined status, draining, and eligbility');
     assert.equal(nodeRow.address, node.httpAddr);
     assert.equal(nodeRow.datacenter, node.datacenter, 'Datacenter');
     assert.equal(nodeRow.allocations, allocations.length, '# Allocations');
   });
 
+  test('client status, draining, and eligibility are collapsed into one column', async function(assert) {
+    server.createList('agent', 1);
+
+    server.create('node', {
+      modifyIndex: 4,
+      status: 'ready',
+      schedulingEligibility: 'eligible',
+      drain: false,
+    });
+    server.create('node', {
+      modifyIndex: 3,
+      status: 'initializing',
+      schedulingEligibility: 'eligible',
+      drain: false,
+    });
+    server.create('node', {
+      modifyIndex: 2,
+      status: 'down',
+      schedulingEligibility: 'eligible',
+      drain: false,
+    });
+    server.create('node', {
+      modifyIndex: 1,
+      status: 'ready',
+      schedulingEligibility: 'ineligible',
+      drain: false,
+    });
+    server.create('node', 'draining', {
+      modifyIndex: 0,
+      status: 'ready',
+    });
+
+    await ClientsList.visit();
+
+    ClientsList.nodes[0].state.as(readyClient => {
+      assert.equal(readyClient.text, 'ready');
+      assert.ok(readyClient.isUnformatted, 'expected no status class');
+      assert.equal(readyClient.tooltip, 'ready / not draining / eligible');
+    });
+
+    assert.equal(ClientsList.nodes[1].state.text, 'initializing');
+    assert.equal(ClientsList.nodes[2].state.text, 'down');
+
+    assert.equal(ClientsList.nodes[3].state.text, 'ineligible');
+    assert.ok(ClientsList.nodes[3].state.isWarning, 'expected warning class');
+
+    assert.equal(ClientsList.nodes[4].state.text, 'draining');
+    assert.ok(ClientsList.nodes[4].state.isInfo, 'expected info class');
+  });
+
   test('each client should link to the client detail page', async function(assert) {
-    minimumSetup();
+    server.createList('node', 1);
+    server.createList('agent', 1);
+
     const node = server.db.nodes[0];
 
     await ClientsList.visit();
@@ -112,18 +157,30 @@ module('Acceptance | clients list', function(hooks) {
     filter: (node, selection) => selection.includes(node.nodeClass),
   });
 
-  testFacet('Status', {
-    facet: ClientsList.facets.status,
-    paramName: 'status',
-    expectedOptions: ['Initializing', 'Ready', 'Down'],
+  testFacet('State', {
+    facet: ClientsList.facets.state,
+    paramName: 'state',
+    expectedOptions: ['Initializing', 'Ready', 'Down', 'Ineligible', 'Draining'],
     async beforeEach() {
       server.create('agent');
+
       server.createList('node', 2, { status: 'initializing' });
       server.createList('node', 2, { status: 'ready' });
       server.createList('node', 2, { status: 'down' });
+
+      server.createList('node', 2, { schedulingEligibility: 'eligible', drain: false });
+      server.createList('node', 2, { schedulingEligibility: 'ineligible', drain: false });
+      server.createList('node', 2, { schedulingEligibility: 'ineligible', drain: true });
+
       await ClientsList.visit();
     },
-    filter: (node, selection) => selection.includes(node.status),
+    filter: (node, selection) => {
+      if (selection.includes('draining') && !node.drain) return false;
+      if (selection.includes('ineligible') && node.schedulingEligibility === 'eligible')
+        return false;
+
+      return selection.includes(node.status);
+    },
   });
 
   testFacet('Datacenters', {
@@ -142,33 +199,14 @@ module('Acceptance | clients list', function(hooks) {
     filter: (node, selection) => selection.includes(node.datacenter),
   });
 
-  testFacet('Flags', {
-    facet: ClientsList.facets.flags,
-    paramName: 'flags',
-    expectedOptions: ['Ineligible', 'Draining'],
-    async beforeEach() {
-      server.create('agent');
-      server.createList('node', 2, { schedulingEligibility: 'eligible', drain: false });
-      server.createList('node', 2, { schedulingEligibility: 'ineligible', drain: false });
-      server.createList('node', 2, { schedulingEligibility: 'ineligible', drain: true });
-      await ClientsList.visit();
-    },
-    filter: (node, selection) => {
-      if (selection.includes('draining') && !node.drain) return false;
-      if (selection.includes('ineligible') && node.schedulingEligibility === 'eligible')
-        return false;
-      return true;
-    },
-  });
-
   test('when the facet selections result in no matches, the empty state states why', async function(assert) {
     server.create('agent');
     server.createList('node', 2, { status: 'ready' });
 
     await ClientsList.visit();
 
-    await ClientsList.facets.status.toggle();
-    await ClientsList.facets.status.options.objectAt(0).toggle();
+    await ClientsList.facets.state.toggle();
+    await ClientsList.facets.state.options.objectAt(0).toggle();
     assert.ok(ClientsList.isEmpty, 'There is an empty message');
     assert.equal(ClientsList.empty.headline, 'No Matches', 'The message is appropriate');
   });

ui/tests/pages/clients/list.js

@@ -1,8 +1,11 @@
 import {
+  attribute,
   create,
   collection,
   clickable,
   fillable,
+  hasClass,
+  isHidden,
   isPresent,
   text,
   visitable,
@@ -18,9 +21,17 @@ export default create({
   nodes: collection('[data-test-client-node-row]', {
     id: text('[data-test-client-id]'),
     name: text('[data-test-client-name]'),
-    status: text('[data-test-client-status]'),
-    drain: text('[data-test-client-drain]'),
-    eligibility: text('[data-test-client-eligibility]'),
+
+    state: {
+      scope: '[data-test-client-state]',
+
+      tooltip: attribute('aria-label', '.tooltip'),
+
+      isInfo: hasClass('is-info', '.status-text'),
+      isWarning: hasClass('is-warning', '.status-text'),
+      isUnformatted: isHidden('.status-text'),
+    },
+
     address: text('[data-test-client-address]'),
     datacenter: text('[data-test-client-datacenter]'),
     allocations: text('[data-test-client-allocations]'),
@@ -45,8 +56,7 @@ export default create({
 
   facets: {
     class: facet('[data-test-class-facet]'),
-    status: facet('[data-test-status-facet]'),
+    state: facet('[data-test-state-facet]'),
     datacenter: facet('[data-test-datacenter-facet]'),
-    flags: facet('[data-test-flags-facet]'),
   },
 });