diff --git a/.changelog/18607.txt b/.changelog/18607.txt new file mode 100644 index 000000000..90733578b --- /dev/null +++ b/.changelog/18607.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: change the State filter on clients page to split out eligibility and drain status +``` diff --git a/ui/app/components/client-node-row.js b/ui/app/components/client-node-row.js index 137513629..fca7a9a4d 100644 --- a/ui/app/components/client-node-row.js +++ b/ui/app/components/client-node-row.js @@ -57,37 +57,31 @@ export default class ClientNodeRow extends Component.extend( @watchRelationship('allocations') watch; - @computed('node.compositeStatus') + @computed('node.status') get nodeStatusColor() { - let compositeStatus = this.get('node.compositeStatus'); - - if (compositeStatus === 'draining') { - return 'neutral'; - } else if (compositeStatus === 'ineligible') { + let status = this.get('node.status'); + if (status === 'disconnected') { return 'warning'; - } else if (compositeStatus === 'down') { + } else if (status === 'down') { return 'critical'; - } else if (compositeStatus === 'ready') { + } else if (status === 'ready') { return 'success'; - } else if (compositeStatus === 'initializing') { + } else if (status === 'initializing') { return 'neutral'; } else { return 'neutral'; } } - @computed('node.compositeStatus') + @computed('node.status') get nodeStatusIcon() { - let compositeStatus = this.get('node.compositeStatus'); - - if (compositeStatus === 'draining') { - return 'minus-circle'; - } else if (compositeStatus === 'ineligible') { + let status = this.get('node.status'); + if (status === 'disconnected') { return 'skip'; - } else if (compositeStatus === 'down') { + } else if (status === 'down') { return 'x-circle'; - } else if (compositeStatus === 'ready') { + } else if (status === 'ready') { return 'check-circle'; - } else if (compositeStatus === 'initializing') { + } else if (status === 'initializing') { return 'entry-point'; } else { return ''; diff --git a/ui/app/controllers/clients/index.js b/ui/app/controllers/clients/index.js index b7f38375f..ea2bc84bd 100644 --- a/ui/app/controllers/clients/index.js +++ b/ui/app/controllers/clients/index.js @@ -3,6 +3,8 @@ * SPDX-License-Identifier: BUSL-1.1 */ +// @ts-check + /* eslint-disable ember/no-incorrect-calls-with-inline-anonymous-functions */ import { alias, readOnly } from '@ember/object/computed'; import { inject as service } from '@ember/service'; @@ -45,9 +47,6 @@ export default class IndexController extends Controller.extend( { qpClass: 'class', }, - { - qpState: 'state', - }, { qpDatacenter: 'dc', }, @@ -62,6 +61,110 @@ export default class IndexController extends Controller.extend( }, ]; + filterFunc = (node) => { + return node.isEligible; + }; + + clientFilterToggles = { + state: [ + { + label: 'initializing', + qp: 'state_initializing', + default: true, + filter: (node) => node.status === 'initializing', + }, + { + label: 'ready', + qp: 'state_ready', + default: true, + filter: (node) => node.status === 'ready', + }, + { + label: 'down', + qp: 'state_down', + default: true, + filter: (node) => node.status === 'down', + }, + { + label: 'disconnected', + qp: 'state_disconnected', + default: true, + filter: (node) => node.status === 'disconnected', + }, + ], + eligibility: [ + { + label: 'eligible', + qp: 'eligibility_eligible', + default: true, + filter: (node) => node.isEligible, + }, + { + label: 'ineligible', + qp: 'eligibility_ineligible', + default: true, + filter: (node) => !node.isEligible, + }, + ], + drainStatus: [ + { + label: 'draining', + qp: 'drain_status_draining', + default: true, + filter: (node) => node.isDraining, + }, + { + label: 'not draining', + qp: 'drain_status_not_draining', + default: true, + filter: (node) => !node.isDraining, + }, + ], + }; + + @computed( + 'state_initializing', + 'state_ready', + 'state_down', + 'state_disconnected', + 'eligibility_eligible', + 'eligibility_ineligible', + 'drain_status_draining', + 'drain_status_not_draining', + 'allToggles.[]' + ) + get activeToggles() { + return this.allToggles.filter((t) => this[t.qp]); + } + + get allToggles() { + return Object.values(this.clientFilterToggles).reduce( + (acc, filters) => acc.concat(filters), + [] + ); + } + + // eslint-disable-next-line ember/classic-decorator-hooks + constructor() { + super(...arguments); + this.addDynamicQueryParams(); + } + + addDynamicQueryParams() { + this.clientFilterToggles.state.forEach((filter) => { + this.queryParams.push({ [filter.qp]: filter.qp }); + this.set(filter.qp, filter.default); + }); + this.clientFilterToggles.eligibility.forEach((filter) => { + this.queryParams.push({ [filter.qp]: filter.qp }); + this.set(filter.qp, filter.default); + }); + this.clientFilterToggles.drainStatus.forEach((filter) => { + this.queryParams.push({ [filter.qp]: filter.qp }); + this.set(filter.qp, filter.default); + }); + } + currentPage = 1; @readOnly('userSettings.pageSize') pageSize; @@ -74,14 +177,12 @@ export default class IndexController extends Controller.extend( } qpClass = ''; - qpState = ''; qpDatacenter = ''; qpVersion = ''; qpVolume = ''; qpNodePool = ''; @selection('qpClass') selectionClass; - @selection('qpState') selectionState; @selection('qpDatacenter') selectionDatacenter; @selection('qpVersion') selectionVersion; @selection('qpVolume') selectionVolume; @@ -105,18 +206,6 @@ export default class IndexController extends Controller.extend( return classes.sort().map((dc) => ({ key: dc, label: dc })); } - @computed - get optionsState() { - return [ - { key: 'initializing', label: 'Initializing' }, - { key: 'ready', label: 'Ready' }, - { key: 'down', label: 'Down' }, - { key: 'ineligible', label: 'Ineligible' }, - { key: 'draining', label: 'Draining' }, - { key: 'disconnected', label: 'Disconnected' }, - ]; - } - @computed('nodes.[]', 'selectionDatacenter') get optionsDatacenter() { const datacenters = Array.from( @@ -195,35 +284,50 @@ export default class IndexController extends Controller.extend( } @computed( + 'clientFilterToggles', + 'drain_status_draining', + 'drain_status_not_draining', + 'eligibility_eligible', + 'eligibility_ineligible', 'nodes.[]', 'selectionClass', - 'selectionState', 'selectionDatacenter', 'selectionNodePool', 'selectionVersion', - 'selectionVolume' + 'selectionVolume', + 'state_disconnected', + 'state_down', + 'state_initializing', + 'state_ready' ) get filteredNodes() { const { selectionClass: classes, - selectionState: states, selectionDatacenter: datacenters, selectionNodePool: nodePools, selectionVersion: versions, selectionVolume: volumes, } = this; - const onlyIneligible = states.includes('ineligible'); - const onlyDraining = states.includes('draining'); + let nodes = this.nodes; - // states is a composite of node status and other node states - const statuses = states.without('ineligible').without('draining'); + // new QP style filtering + for (let category in this.clientFilterToggles) { + nodes = nodes.filter((node) => { + let includeNode = false; + for (let filter of this.clientFilterToggles[category]) { + if (this[filter.qp] && filter.filter(node)) { + includeNode = true; + break; + } + } + return includeNode; + }); + } - return this.nodes.filter((node) => { + return nodes.filter((node) => { if (classes.length && !classes.includes(node.get('nodeClass'))) return false; - if (statuses.length && !statuses.includes(node.get('status'))) - return false; if (datacenters.length && !datacenters.includes(node.get('datacenter'))) return false; if (versions.length && !versions.includes(node.get('version'))) @@ -237,9 +341,6 @@ export default class IndexController extends Controller.extend( return false; } - if (onlyIneligible && node.get('isEligible')) return false; - if (onlyDraining && !node.get('isDraining')) return false; - return true; }); } @@ -254,6 +355,16 @@ export default class IndexController extends Controller.extend( this.set(queryParam, serialize(selection)); } + @action + handleFilterChange(queryParamValue, option, queryParamLabel) { + if (queryParamValue.includes(option)) { + queryParamValue.removeObject(option); + } else { + queryParamValue.addObject(option); + } + this.set(queryParamLabel, serialize(queryParamValue)); + } + @action gotoNode(node) { this.transitionToRoute('clients.client', node); diff --git a/ui/app/styles/core/table.scss b/ui/app/styles/core/table.scss index 1b70c3c0f..4af22275c 100644 --- a/ui/app/styles/core/table.scss +++ b/ui/app/styles/core/table.scss @@ -110,6 +110,12 @@ white-space: nowrap; } + &.node-status-badges { + .hds-badge__text { + white-space: nowrap; + } + } + &.is-narrow { padding: 1.25em 0 1.25em 0.5em; diff --git a/ui/app/templates/clients/index.hbs b/ui/app/templates/clients/index.hbs index ba61341d9..5394ff2af 100644 --- a/ui/app/templates/clients/index.hbs +++ b/ui/app/templates/clients/index.hbs @@ -18,52 +18,190 @@ /> {{/if}} -
-
- + + - + {{#each this.clientFilterToggles.state as |option|}} + + {{capitalize option.label}} + + {{/each}} + + + {{#each this.clientFilterToggles.eligibility as |option|}} + + {{capitalize option.label}} + + {{/each}} + + + {{#each this.clientFilterToggles.drainStatus as |option|}} + + {{capitalize option.label}} + + {{/each}} + + + + - + {{option.label}} + + {{else}} + + No Node Pool filters + + {{/each}} + + + + - + {{option.label}} + + {{else}} + + No Class filters + + {{/each}} + + + + - + {{option.label}} + + {{else}} + + No Datacenter filters + + {{/each}} + + + + + - + {{option.label}} + + {{else}} + + No Version filters + + {{/each}} + + + + -
-
+ {{#each this.optionsVolume key="label" as |option|}} + + {{option.label}} + + {{else}} + + No Volume filters + + {{/each}} + + {{#if this.sortedNodes}} Name - State + State Address Node Pool Datacenter diff --git a/ui/app/templates/components/client-node-row.hbs b/ui/app/templates/components/client-node-row.hbs index 66780e7bc..09e2142fd 100644 --- a/ui/app/templates/components/client-node-row.hbs +++ b/ui/app/templates/components/client-node-row.hbs @@ -12,15 +12,41 @@ {{this.node.shortId}} {{this.node.name}} - - + + + + {{#if this.node.isEligible}} - + {{else}} + + {{/if}} + + {{#if this.node.isDraining}} + + {{else}} + + {{/if}} {{this.node.httpAddr}} diff --git a/ui/tests/acceptance/clients-list-test.js b/ui/tests/acceptance/clients-list-test.js index 82fd597c7..49c53da54 100644 --- a/ui/tests/acceptance/clients-list-test.js +++ b/ui/tests/acceptance/clients-list-test.js @@ -77,7 +77,7 @@ module('Acceptance | clients list', function (hooks) { assert.equal(nodeRow.nodePool, node.nodePool, 'Node Pool'); assert.equal( nodeRow.compositeStatus.text, - 'Draining', + 'Ready Ineligible Draining', 'Combined status, draining, and eligbility' ); assert.equal(nodeRow.address, node.httpAddr); @@ -111,13 +111,13 @@ module('Acceptance | clients list', function (hooks) { assert.equal(nodeRow.id, node.id.split('-')[0], 'ID'); assert.equal( nodeRow.compositeStatus.text, - 'Ready', + 'Ready Eligible Not Draining', 'Combined status, draining, and eligbility' ); assert.equal(nodeRow.allocations, running.length, '# Allocations'); }); - test('client status, draining, and eligibility are collapsed into one column that stays sorted', async function (assert) { + test('client status, draining, and eligibility are combined into one column that stays sorted on status', async function (assert) { server.createList('agent', 1); server.create('node', { @@ -151,47 +151,71 @@ module('Acceptance | clients list', function (hooks) { drain: false, }); server.create('node', 'draining', { + schedulingEligibility: 'eligible', modifyIndex: 0, status: 'ready', }); await ClientsList.visit(); - ClientsList.nodes[0].compositeStatus.as((readyClient) => { - assert.equal(readyClient.text, 'Ready'); - console.log('readyClient', readyClient.text); - assert.equal(readyClient.tooltip, 'ready / not draining / eligible'); - }); - - assert.equal(ClientsList.nodes[1].compositeStatus.text, 'Initializing'); - assert.equal(ClientsList.nodes[2].compositeStatus.text, 'Down'); + assert.equal( + ClientsList.nodes[0].compositeStatus.text, + 'Ready Eligible Not Draining' + ); + assert.equal( + ClientsList.nodes[1].compositeStatus.text, + 'Initializing Eligible Not Draining' + ); assert.equal( ClientsList.nodes[2].compositeStatus.text, - 'Down', - 'down takes priority over ineligible' + 'Down Eligible Not Draining' + ); + assert.equal( + ClientsList.nodes[3].compositeStatus.text, + 'Down Ineligible Not Draining' + ); + assert.equal( + ClientsList.nodes[4].compositeStatus.text, + 'Ready Ineligible Not Draining' + ); + assert.equal( + ClientsList.nodes[5].compositeStatus.text, + 'Ready Eligible Draining' ); - assert.equal(ClientsList.nodes[4].compositeStatus.text, 'Ineligible'); - assert.equal(ClientsList.nodes[5].compositeStatus.text, 'Draining'); - - await ClientsList.sortBy('compositeStatus'); + await ClientsList.sortBy('status'); assert.deepEqual( ClientsList.nodes.map((n) => n.compositeStatus.text), - ['Ready', 'Initializing', 'Ineligible', 'Draining', 'Down', 'Down'] + [ + 'Ready Eligible Draining', + 'Ready Ineligible Not Draining', + 'Ready Eligible Not Draining', + 'Initializing Eligible Not Draining', + 'Down Ineligible Not Draining', + 'Down Eligible Not Draining', + ], + 'Nodes are sorted only by status, and otherwise default to modifyIndex' ); // Simulate a client state change arriving through polling - let readyClient = this.owner + let discoClient = this.owner .lookup('service:store') .peekAll('node') .findBy('modifyIndex', 5); - readyClient.set('schedulingEligibility', 'ineligible'); + discoClient.set('status', 'disconnected'); await settled(); assert.deepEqual( ClientsList.nodes.map((n) => n.compositeStatus.text), - ['Initializing', 'Ineligible', 'Ineligible', 'Draining', 'Down', 'Down'] + [ + 'Ready Eligible Draining', + 'Ready Ineligible Not Draining', + 'Disconnected Eligible Not Draining', + 'Initializing Eligible Not Draining', + 'Down Ineligible Not Draining', + 'Down Eligible Not Draining', + ] ); }); @@ -275,12 +299,14 @@ module('Acceptance | clients list', function (hooks) { facet: ClientsList.facets.state, paramName: 'state', expectedOptions: [ - 'Initializing', - 'Ready', - 'Down', - 'Ineligible', - 'Draining', - 'Disconnected', + 'initializing', + 'ready', + 'down', + 'disconnected', + 'eligible', + 'ineligible', + 'draining', + 'not draining', ], async beforeEach() { server.create('agent'); @@ -312,7 +338,7 @@ module('Acceptance | clients list', function (hooks) { ) return false; - return selection.includes(node.status); + return !selection.includes(node.status); }, }); @@ -400,9 +426,8 @@ module('Acceptance | clients list', function (hooks) { server.createList('node', 2, { status: 'ready' }); await ClientsList.visit(); - await ClientsList.facets.state.toggle(); - await ClientsList.facets.state.options.objectAt(0).toggle(); + await ClientsList.facets.state.options.objectAt(1).toggle(); assert.ok(ClientsList.isEmpty, 'There is an empty message'); assert.equal( ClientsList.empty.headline, @@ -441,7 +466,9 @@ module('Acceptance | clients list', function (hooks) { } assert.deepEqual( - facet.options.map((option) => option.label.trim()), + facet.options.map((option) => { + return option.key.trim(); + }), expectation, 'Options for facet are as expected' ); @@ -511,11 +538,20 @@ module('Acceptance | clients list', function (hooks) { await option2.toggle(); selection.push(option2.key); + // State is different from the other facets, in that it is an "exclusive" filter, whete others are "inclusive". + // Because of this, it doesn't pass "state" as a stringified-array query param; rather, exclusion is indicated + // for each option with a "${optionName}=false" query param. + + const stateString = `/clients?${selection + .map((option) => `state_${option}=false`) + .join('&')}`; + const nonStateString = `/clients?${paramName}=${encodeURIComponent( + JSON.stringify(selection) + )}`; + assert.equal( currentURL(), - `/clients?${paramName}=${encodeURIComponent( - JSON.stringify(selection) - )}`, + paramName === 'state' ? stateString : nonStateString, 'URL has the correct query param key and value' ); }); diff --git a/ui/tests/pages/clients/list.js b/ui/tests/pages/clients/list.js index c5249ee6b..13e567e79 100644 --- a/ui/tests/pages/clients/list.js +++ b/ui/tests/pages/clients/list.js @@ -16,9 +16,24 @@ import { visitable, } from 'ember-cli-page-object'; -import { multiFacet } from 'nomad-ui/tests/pages/components/facet'; import pageSizeSelect from 'nomad-ui/tests/pages/components/page-size-select'; +const heliosFacet = (scope) => ({ + scope, + toggle: clickable('button'), + options: collection( + '.hds-menu-primitive__content .hds-dropdown__content .hds-dropdown__list .hds-dropdown-list-item--variant-checkbox', + { + toggle: clickable('label'), + count: text('label .hds-dropdown-list-item__count'), + key: attribute( + 'data-test-dropdown-option', + '[data-test-dropdown-option]' + ), + } + ), +}); + export default create({ pageSize: 25, @@ -77,11 +92,11 @@ export default create({ }, facets: { - nodePools: multiFacet('[data-test-node-pool-facet]'), - class: multiFacet('[data-test-class-facet]'), - state: multiFacet('[data-test-state-facet]'), - datacenter: multiFacet('[data-test-datacenter-facet]'), - version: multiFacet('[data-test-version-facet]'), - volume: multiFacet('[data-test-volume-facet]'), + nodePools: heliosFacet('[data-test-node-pool-facet]'), + class: heliosFacet('[data-test-class-facet]'), + state: heliosFacet('[data-test-state-facet]'), + datacenter: heliosFacet('[data-test-datacenter-facet]'), + version: heliosFacet('[data-test-version-facet]'), + volume: heliosFacet('[data-test-volume-facet]'), }, });