From a22a952b385815ecd6f97b4851b81e98e4b0f74a Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 25 Jan 2021 11:29:01 -0800 Subject: [PATCH 1/3] Only count the scheduled allocs on the topo viz node stats bar --- ui/app/components/topo-viz/node.js | 2 +- .../components/topo-viz/node-test.js | 29 ++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/ui/app/components/topo-viz/node.js b/ui/app/components/topo-viz/node.js index 5b8d98d0da19..1030f321a3a0 100644 --- a/ui/app/components/topo-viz/node.js +++ b/ui/app/components/topo-viz/node.js @@ -51,7 +51,7 @@ export default class TopoVizNode extends Component { } get count() { - return this.args.node.allocations.length; + return this.allocations.length; } get allocations() { diff --git a/ui/tests/integration/components/topo-viz/node-test.js b/ui/tests/integration/components/topo-viz/node-test.js index a249074d55d0..3d1cd2aa85b6 100644 --- a/ui/tests/integration/components/topo-viz/node-test.js +++ b/ui/tests/integration/components/topo-viz/node-test.js @@ -23,15 +23,15 @@ const nodeGen = (name, datacenter, memory, cpu, flags = {}) => ({ }, }); -const allocGen = (node, memory, cpu, isSelected) => ({ +const allocGen = (node, memory, cpu, isScheduled = true) => ({ memory, cpu, - isSelected, + isSelected: false, memoryPercent: memory / node.memory, cpuPercent: cpu / node.cpu, allocation: { id: faker.random.uuid(), - isScheduled: true, + isScheduled, }, }); @@ -66,7 +66,11 @@ module('Integration | Component | TopoViz::Node', function(hooks) { props({ node: { ...node, - allocations: [allocGen(node, 100, 100), allocGen(node, 250, 250)], + allocations: [ + allocGen(node, 100, 100), + allocGen(node, 250, 250), + allocGen(node, 300, 300, false), + ], }, }) ); @@ -74,7 +78,10 @@ module('Integration | Component | TopoViz::Node', function(hooks) { await this.render(commonTemplate); assert.ok(TopoVizNode.isPresent); - assert.ok(TopoVizNode.memoryRects.length); + assert.equal( + TopoVizNode.memoryRects.length, + this.node.allocations.filterBy('allocation.isScheduled').length + ); assert.ok(TopoVizNode.cpuRects.length); await componentA11yAudit(this.element, assert); @@ -86,7 +93,11 @@ module('Integration | Component | TopoViz::Node', function(hooks) { props({ node: { ...node, - allocations: [allocGen(node, 100, 100), allocGen(node, 250, 250)], + allocations: [ + allocGen(node, 100, 100), + allocGen(node, 250, 250), + allocGen(node, 300, 300, false), + ], }, }) ); @@ -94,7 +105,11 @@ module('Integration | Component | TopoViz::Node', function(hooks) { await this.render(commonTemplate); assert.ok(TopoVizNode.label.includes(node.node.name)); - assert.ok(TopoVizNode.label.includes(`${this.node.allocations.length} Allocs`)); + assert.ok( + TopoVizNode.label.includes( + `${this.node.allocations.filterBy('allocation.isScheduled').length} Allocs` + ) + ); assert.ok(TopoVizNode.label.includes(`${this.node.memory} MiB`)); assert.ok(TopoVizNode.label.includes(`${this.node.cpu} Mhz`)); }); From 792559e72e08145e87940075f97c6a3e60948e38 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 25 Jan 2021 18:59:55 -0800 Subject: [PATCH 2/3] Clamp widths at zero to prevent negative width warnings This would only ever realistically happen with fixture data, but still good to not have these warnings. --- ui/app/components/topo-viz/node.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ui/app/components/topo-viz/node.js b/ui/app/components/topo-viz/node.js index 1030f321a3a0..43ed0da90bc0 100644 --- a/ui/app/components/topo-viz/node.js +++ b/ui/app/components/topo-viz/node.js @@ -150,7 +150,7 @@ export default class TopoVizNode extends Component { allocation, offset: cpuOffset * 100, percent: cpuPercent * 100, - width: cpuWidth, + width: Math.max(cpuWidth, 0), x: cpuOffset * width + (isFirst ? 0 : 0.5) + (isSelected ? 0.5 : 0), className: allocation.allocation.clientStatus, }); @@ -158,7 +158,7 @@ export default class TopoVizNode extends Component { allocation, offset: memoryOffset * 100, percent: memoryPercent * 100, - width: memoryWidth, + width: Math.max(memoryWidth, 0), x: memoryOffset * width + (isFirst ? 0 : 0.5) + (isSelected ? 0.5 : 0), className: allocation.allocation.clientStatus, }); @@ -169,11 +169,11 @@ export default class TopoVizNode extends Component { const cpuRemainder = { x: cpuOffset * width + 0.5, - width: width - cpuOffset * width, + width: Math.max(width - cpuOffset * width, 0), }; const memoryRemainder = { x: memoryOffset * width + 0.5, - width: width - memoryOffset * width, + width: Math.max(width - memoryOffset * width, 0), }; return { From d3dd3f5b08a4ae0cbebadc6ee4fea737a5f986ea Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 25 Jan 2021 19:01:11 -0800 Subject: [PATCH 3/3] Test coverage for the topology info panel. This fixes a couple bugs 1. Overreporting resources reserved due to counting terminal allocs 2. Overreporting unique client placements due to uniquing on object refs instead of on client ID. --- ui/app/controllers/topology.js | 16 +-- ui/app/templates/topology.hbs | 49 ++++---- ui/tests/acceptance/topology-test.js | 166 ++++++++++++++++++++++++--- ui/tests/pages/topology.js | 52 ++++++++- 4 files changed, 240 insertions(+), 43 deletions(-) diff --git a/ui/app/controllers/topology.js b/ui/app/controllers/topology.js index 9bb0bec43e84..19c3e27a9ccf 100644 --- a/ui/app/controllers/topology.js +++ b/ui/app/controllers/topology.js @@ -47,15 +47,17 @@ export default class TopologyControllers extends Controller { return reduceToLargestUnit(this.totalMemory)[1]; } - @computed('model.allocations.@each.allocatedResources') + @computed('scheduledAllocations.@each.allocatedResources') get totalReservedMemory() { - const mibs = this.model.allocations.mapBy('allocatedResources.memory').reduce(sumAggregator, 0); + const mibs = this.scheduledAllocations + .mapBy('allocatedResources.memory') + .reduce(sumAggregator, 0); return mibs * 1024 * 1024; } - @computed('model.allocations.@each.allocatedResources') + @computed('scheduledAllocations.@each.allocatedResources') get totalReservedCPU() { - return this.model.allocations.mapBy('allocatedResources.cpu').reduce(sumAggregator, 0); + return this.scheduledAllocations.mapBy('allocatedResources.cpu').reduce(sumAggregator, 0); } @computed('totalMemory', 'totalReservedMemory') @@ -70,13 +72,13 @@ export default class TopologyControllers extends Controller { return this.totalReservedCPU / this.totalCPU; } - @computed('activeAllocation', 'model.allocations.@each.{taskGroupName,job}') + @computed('activeAllocation', 'scheduledAllocations.@each.{taskGroupName,job}') get siblingAllocations() { if (!this.activeAllocation) return []; const taskGroup = this.activeAllocation.taskGroupName; const jobId = this.activeAllocation.belongsTo('job').id(); - return this.model.allocations.filter(allocation => { + return this.scheduledAllocations.filter(allocation => { return allocation.taskGroupName === taskGroup && allocation.belongsTo('job').id() === jobId; }); } @@ -104,7 +106,7 @@ export default class TopologyControllers extends Controller { @computed('siblingAllocations.@each.node') get uniqueActiveAllocationNodes() { - return this.siblingAllocations.mapBy('node').uniq(); + return this.siblingAllocations.mapBy('node.id').uniq(); } @action diff --git a/ui/app/templates/topology.hbs b/ui/app/templates/topology.hbs index c41b1de9c6b5..43f543ff6b2b 100644 --- a/ui/app/templates/topology.hbs +++ b/ui/app/templates/topology.hbs @@ -60,29 +60,29 @@
{{#if this.activeNode}}Client{{else if this.activeAllocation}}Allocation{{else}}Cluster{{/if}} Details
-
+
{{#if this.activeNode}} {{#let this.activeNode.node as |node|}}
-

{{this.activeNode.allocations.length}} Allocations

+

{{this.activeNode.allocations.length}} Allocations

Client: - + {{node.shortId}}

-

Name: {{node.name}}

-

Address: {{node.httpAddr}}

-

Status: {{node.status}}

+

Name: {{node.name}}

+

Address: {{node.httpAddr}}

+

Status: {{node.status}}

- Draining? {{if node.isDraining "Yes" "No"}} + Draining? {{if node.isDraining "Yes" "No"}}

- Eligible? {{if node.isEligible "Yes" "No"}} + Eligible? {{if node.isEligible "Yes" "No"}}

@@ -93,8 +93,9 @@

-
+
@@ -106,7 +107,7 @@ {{format-percentage this.nodeUtilization.reservedMemoryPercent total=1}}
-
+
{{format-bytes this.nodeUtilization.totalReservedMemory}} / {{format-bytes this.nodeUtilization.totalMemory}} reserved
@@ -116,6 +117,7 @@
@@ -127,7 +129,7 @@ {{format-percentage this.nodeUtilization.reservedCPUPercent total=1}}
-
+
{{this.nodeUtilization.totalReservedCPU}} Mhz / {{this.nodeUtilization.totalCPU}} Mhz reserved
@@ -136,18 +138,19 @@

Allocation: - {{this.activeAllocation.shortId}} + {{this.activeAllocation.shortId}}

-

Sibling Allocations: {{this.siblingAllocations.length}}

-

Unique Client Placements: {{this.uniqueActiveAllocationNodes.length}}

+

Sibling Allocations: {{this.siblingAllocations.length}}

+

Unique Client Placements: {{this.uniqueActiveAllocationNodes.length}}

Job: + @query={{hash jobNamespace=this.activeAllocation.job.namespace.id}}> {{this.activeAllocation.job.name}} / {{this.activeAllocation.taskGroupName}}

@@ -157,7 +160,7 @@

Client: - + {{this.activeAllocation.node.shortId}}

@@ -173,10 +176,10 @@ {{else}}
-

{{this.model.nodes.length}} Clients

+

{{this.model.nodes.length}} Clients

-

{{this.scheduledAllocations.length}} Allocations

+

{{this.scheduledAllocations.length}} Allocations

@@ -185,6 +188,7 @@
@@ -193,10 +197,10 @@
- {{format-percentage this.reservedMemoryPercent total=1}} + {{format-percentage this.reservedMemoryPercent total=1}}
-
+
{{format-bytes this.totalReservedMemory}} / {{format-bytes this.totalMemory}} reserved
@@ -206,6 +210,7 @@
@@ -214,10 +219,10 @@
- {{format-percentage this.reservedCPUPercent total=1}} + {{format-percentage this.reservedCPUPercent total=1}}
-
+
{{this.totalReservedCPU}} Mhz / {{this.totalCPU}} Mhz reserved
diff --git a/ui/tests/acceptance/topology-test.js b/ui/tests/acceptance/topology-test.js index 70e510fb8894..b3652c6eb3b6 100644 --- a/ui/tests/acceptance/topology-test.js +++ b/ui/tests/acceptance/topology-test.js @@ -1,12 +1,16 @@ +import { get } from '@ember/object'; +import { currentURL } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import { setupMirage } from 'ember-cli-mirage/test-support'; import a11yAudit from 'nomad-ui/tests/helpers/a11y-audit'; import Topology from 'nomad-ui/tests/pages/topology'; +import { reduceToLargestUnit } from 'nomad-ui/helpers/format-bytes'; import queryString from 'query-string'; -// TODO: Once we settle on the contents of the info panel, the contents -// should also get acceptance tests. +const sumResources = (list, dimension) => + list.reduce((agg, val) => agg + (get(val, dimension) || 0), 0); + module('Acceptance | topology', function(hooks) { setupApplicationTest(hooks); setupMirage(hooks); @@ -30,6 +34,44 @@ module('Acceptance | topology', function(hooks) { await Topology.visit(); assert.equal(Topology.infoPanelTitle, 'Cluster Details'); assert.notOk(Topology.filteredNodesWarning.isPresent); + + assert.equal( + Topology.clusterInfoPanel.nodeCount, + `${server.schema.nodes.all().length} Clients` + ); + + const allocs = server.schema.allocations.all().models; + const scheduledAllocs = allocs.filter(alloc => + ['pending', 'running'].includes(alloc.clientStatus) + ); + assert.equal(Topology.clusterInfoPanel.allocCount, `${scheduledAllocs.length} Allocations`); + + const nodeResources = server.schema.nodes.all().models.mapBy('nodeResources'); + const taskResources = scheduledAllocs + .mapBy('taskResources.models') + .flat() + .mapBy('resources'); + + const totalMem = sumResources(nodeResources, 'Memory.MemoryMB'); + const totalCPU = sumResources(nodeResources, 'Cpu.CpuShares'); + const reservedMem = sumResources(taskResources, 'Memory.MemoryMB'); + const reservedCPU = sumResources(taskResources, 'Cpu.CpuShares'); + + assert.equal(Topology.clusterInfoPanel.memoryProgressValue, reservedMem / totalMem); + assert.equal(Topology.clusterInfoPanel.cpuProgressValue, reservedCPU / totalCPU); + + const [rNum, rUnit] = reduceToLargestUnit(reservedMem * 1024 * 1024); + const [tNum, tUnit] = reduceToLargestUnit(totalMem * 1024 * 1024); + + assert.equal( + Topology.clusterInfoPanel.memoryAbsoluteValue, + `${Math.floor(rNum)} ${rUnit} / ${Math.floor(tNum)} ${tUnit} reserved` + ); + + assert.equal( + Topology.clusterInfoPanel.cpuAbsoluteValue, + `${reservedCPU} Mhz / ${totalCPU} Mhz reserved` + ); }); test('all allocations for all namespaces and all clients are queried on load', async function(assert) { @@ -52,28 +94,126 @@ module('Acceptance | topology', function(hooks) { }); test('when an allocation is selected, the info panel shows information on the allocation', async function(assert) { - server.createList('node', 1); - server.createList('allocation', 5); - - await Topology.visit(); + const nodes = server.createList('node', 5); + const job = server.create('job', { createAllocations: false }); + const taskGroup = server.schema.find('taskGroup', job.taskGroupIds[0]).name; + const allocs = server.createList('allocation', 5, { + forceRunningClientStatus: true, + jobId: job.id, + taskGroup, + }); - if (Topology.viz.datacenters[0].nodes[0].isEmpty) { - assert.expect(0); - } else { - await Topology.viz.datacenters[0].nodes[0].memoryRects[0].select(); - assert.equal(Topology.infoPanelTitle, 'Allocation Details'); + // Get the first alloc of the first node that has an alloc + const sortedNodes = nodes.sortBy('datacenter'); + let node, alloc; + for (let n of sortedNodes) { + alloc = allocs.find(a => a.nodeId === n.id); + if (alloc) { + node = n; + break; + } } + + const dcIndex = nodes + .mapBy('datacenter') + .uniq() + .sort() + .indexOf(node.datacenter); + const nodeIndex = nodes.filterBy('datacenter', node.datacenter).indexOf(node); + + const reset = async () => { + await Topology.visit(); + await Topology.viz.datacenters[dcIndex].nodes[nodeIndex].memoryRects[0].select(); + }; + + await reset(); + assert.equal(Topology.infoPanelTitle, 'Allocation Details'); + + assert.equal(Topology.allocInfoPanel.id, alloc.id.split('-')[0]); + + const uniqueClients = allocs.mapBy('nodeId').uniq(); + assert.equal(Topology.allocInfoPanel.siblingAllocs, `Sibling Allocations: ${allocs.length}`); + assert.equal( + Topology.allocInfoPanel.uniquePlacements, + `Unique Client Placements: ${uniqueClients.length}` + ); + + assert.equal(Topology.allocInfoPanel.job, job.name); + assert.ok(Topology.allocInfoPanel.taskGroup.endsWith(alloc.taskGroup)); + assert.equal(Topology.allocInfoPanel.client, node.id.split('-')[0]); + + await Topology.allocInfoPanel.visitAlloc(); + assert.equal(currentURL(), `/allocations/${alloc.id}`); + + await reset(); + + await Topology.allocInfoPanel.visitJob(); + assert.equal(currentURL(), `/jobs/${job.id}`); + + await reset(); + + await Topology.allocInfoPanel.visitClient(); + assert.equal(currentURL(), `/clients/${node.id}`); }); test('when a node is selected, the info panel shows information on the node', async function(assert) { // A high node count is required for node selection - server.createList('node', 51); - server.createList('allocation', 5); + const nodes = server.createList('node', 51); + const node = nodes.sortBy('datacenter')[0]; + server.createList('allocation', 5, { forceRunningClientStatus: true }); + + const allocs = server.schema.allocations.where({ nodeId: node.id }).models; await Topology.visit(); await Topology.viz.datacenters[0].nodes[0].selectNode(); assert.equal(Topology.infoPanelTitle, 'Client Details'); + + assert.equal(Topology.nodeInfoPanel.id, node.id.split('-')[0]); + assert.equal(Topology.nodeInfoPanel.name, `Name: ${node.name}`); + assert.equal(Topology.nodeInfoPanel.address, `Address: ${node.httpAddr}`); + assert.equal(Topology.nodeInfoPanel.status, `Status: ${node.status}`); + + assert.equal(Topology.nodeInfoPanel.drainingLabel, node.drain ? 'Yes' : 'No'); + assert.equal( + Topology.nodeInfoPanel.eligibleLabel, + node.schedulingEligibility === 'eligible' ? 'Yes' : 'No' + ); + + assert.equal(Topology.nodeInfoPanel.drainingIsAccented, node.drain); + assert.equal( + Topology.nodeInfoPanel.eligibleIsAccented, + node.schedulingEligibility !== 'eligible' + ); + + const taskResources = allocs + .mapBy('taskResources.models') + .flat() + .mapBy('resources'); + const reservedMem = sumResources(taskResources, 'Memory.MemoryMB'); + const reservedCPU = sumResources(taskResources, 'Cpu.CpuShares'); + + const totalMem = node.nodeResources.Memory.MemoryMB; + const totalCPU = node.nodeResources.Cpu.CpuShares; + + assert.equal(Topology.nodeInfoPanel.memoryProgressValue, reservedMem / totalMem); + assert.equal(Topology.nodeInfoPanel.cpuProgressValue, reservedCPU / totalCPU); + + const [rNum, rUnit] = reduceToLargestUnit(reservedMem * 1024 * 1024); + const [tNum, tUnit] = reduceToLargestUnit(totalMem * 1024 * 1024); + + assert.equal( + Topology.nodeInfoPanel.memoryAbsoluteValue, + `${Math.floor(rNum)} ${rUnit} / ${Math.floor(tNum)} ${tUnit} reserved` + ); + + assert.equal( + Topology.nodeInfoPanel.cpuAbsoluteValue, + `${reservedCPU} Mhz / ${totalCPU} Mhz reserved` + ); + + await Topology.nodeInfoPanel.visitNode(); + assert.equal(currentURL(), `/clients/${node.id}`); }); test('when one or more nodes lack the NodeResources property, a warning message is shown', async function(assert) { diff --git a/ui/tests/pages/topology.js b/ui/tests/pages/topology.js index 63e38de22033..74f5258bf3ef 100644 --- a/ui/tests/pages/topology.js +++ b/ui/tests/pages/topology.js @@ -1,4 +1,4 @@ -import { create, text, visitable } from 'ember-cli-page-object'; +import { attribute, clickable, create, hasClass, text, visitable } from 'ember-cli-page-object'; import TopoViz from 'nomad-ui/tests/pages/components/topo-viz'; import notification from 'nomad-ui/tests/pages/components/notification'; @@ -10,4 +10,54 @@ export default create({ filteredNodesWarning: notification('[data-test-filtered-nodes-warning]'), viz: TopoViz('[data-test-topo-viz]'), + + clusterInfoPanel: { + scope: '[data-test-info-panel]', + nodeCount: text('[data-test-node-count]'), + allocCount: text('[data-test-alloc-count]'), + + memoryProgressValue: attribute('value', '[data-test-memory-progress-bar]'), + memoryAbsoluteValue: text('[data-test-memory-absolute-value]'), + cpuProgressValue: attribute('value', '[data-test-cpu-progress-bar]'), + cpuAbsoluteValue: text('[data-test-cpu-absolute-value]'), + }, + + nodeInfoPanel: { + scope: '[data-test-info-panel]', + allocations: text('[data-test-allocaions]'), + + visitNode: clickable('[data-test-client-link]'), + + id: text('[data-test-client-link]'), + name: text('[data-test-name]'), + address: text('[data-test-address]'), + status: text('[data-test-status]'), + + drainingLabel: text('[data-test-draining]'), + drainingIsAccented: hasClass('is-info', '[data-test-draining]'), + + eligibleLabel: text('[data-test-eligible]'), + eligibleIsAccented: hasClass('is-warning', '[data-test-eligible]'), + + memoryProgressValue: attribute('value', '[data-test-memory-progress-bar]'), + memoryAbsoluteValue: text('[data-test-memory-absolute-value]'), + cpuProgressValue: attribute('value', '[data-test-cpu-progress-bar]'), + cpuAbsoluteValue: text('[data-test-cpu-absolute-value]'), + }, + + allocInfoPanel: { + scope: '[data-test-info-panel]', + id: text('[data-test-id]'), + visitAlloc: clickable('[data-test-id]'), + + siblingAllocs: text('[data-test-sibling-allocs]'), + uniquePlacements: text('[data-test-unique-placements]'), + + job: text('[data-test-job]'), + visitJob: clickable('[data-test-job]'), + taskGroup: text('[data-test-task-group]'), + + client: text('[data-test-client]'), + visitClient: clickable('[data-test-client]'), + }, });