Skip to content

Commit

Permalink
Merge pull request #9733 from hashicorp/b-ui/topo-viz-old-agent
Browse files Browse the repository at this point in the history
UI: Guard against nodes running an old version of the Nomad agent
  • Loading branch information
DingoEatingFuzz committed Jan 7, 2021
2 parents 08af8eb + 37e1551 commit a53c8eb
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 3 deletions.
19 changes: 18 additions & 1 deletion ui/app/components/topo-viz.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,18 @@ export default class TopoViz extends Component {
const nodes = this.args.nodes;
const allocations = this.args.allocations;

// Nodes may not have a resources property due to having an old Nomad agent version.
const badNodes = [];

// Wrap nodes in a topo viz specific data structure and build an index to speed up allocation assignment
const nodeContainers = [];
const nodeIndex = {};
nodes.forEach(node => {
if (!node.resources) {
badNodes.push(node);
return;
}

const container = this.dataForNode(node);
nodeContainers.push(container);
nodeIndex[node.id] = container;
Expand All @@ -99,7 +107,7 @@ export default class TopoViz extends Component {
const nodeId = allocation.belongsTo('node').id();
const nodeContainer = nodeIndex[nodeId];

// Ignore orphaned allocations
// Ignore orphaned allocations and allocations on nodes with an old Nomad agent version.
if (!nodeContainer) return;

const allocationContainer = this.dataForAllocation(allocation, nodeContainer);
Expand Down Expand Up @@ -131,6 +139,15 @@ export default class TopoViz extends Component {
.domain(extent(nodeContainers.mapBy('memory'))),
};
this.topology = topology;

if (badNodes.length && this.args.onDataError) {
this.args.onDataError([
{
type: 'filtered-nodes',
context: badNodes,
},
]);
}
}

@action
Expand Down
11 changes: 11 additions & 0 deletions ui/app/controllers/topology.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import Controller from '@ember/controller';
import { computed, action } from '@ember/object';
import { alias } from '@ember/object/computed';
import { inject as service } from '@ember/service';
import { tracked } from '@glimmer/tracking';
import classic from 'ember-classic-decorator';
import { reduceToLargestUnit } from 'nomad-ui/helpers/format-bytes';

Expand All @@ -13,6 +14,8 @@ export default class TopologyControllers extends Controller {

@alias('userSettings.showTopoVizPollingNotice') showPollingNotice;

@tracked filteredNodes = null;

@computed('model.nodes.@each.datacenter')
get datacenters() {
return Array.from(new Set(this.model.nodes.mapBy('datacenter'))).compact();
Expand Down Expand Up @@ -117,4 +120,12 @@ export default class TopologyControllers extends Controller {
setNode(node) {
this.set('activeNode', node);
}

@action
handleTopoVizDataError(errors) {
const filteredNodesError = errors.findBy('type', 'filtered-nodes');
if (filteredNodesError) {
this.filteredNodes = filteredNodesError.context;
}
}
}
16 changes: 15 additions & 1 deletion ui/app/templates/topology.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@
{{#if this.isForbidden}}
<ForbiddenMessage />
{{else}}
{{#if this.filteredNodes}}
<div class="notification is-warning">
<div data-test-filtered-nodes-warning class="columns">
<div class="column">
<h3 data-test-title class="title is-4">Some Clients Were Filtered</h3>
<p data-test-message>{{this.filteredNodes.length}} {{if (eq this.filteredNodes.length 1) "client was" "clients were"}} filtered from the topology visualization. This is most likely due to the {{pluralize "client" this.filteredNodes.length}} running a version of Nomad &lt;0.9.0.</p>
</div>
<div class="column is-centered is-minimum">
<button data-test-dismiss class="button is-warning" onclick={{action (mut this.filteredNodes) null}} type="button">Okay</button>
</div>
</div>
</div>
{{/if}}
<div class="columns">
<div class="column is-narrow is-400">
{{#if this.showPollingNotice}}
Expand Down Expand Up @@ -217,7 +230,8 @@
@nodes={{this.model.nodes}}
@allocations={{this.model.allocations}}
@onAllocationSelect={{action this.setAllocation}}
@onNodeSelect={{action this.setNode}} />
@onNodeSelect={{action this.setNode}}
@onDataError={{action this.handleTopoVizDataError}}/>
</div>
</div>
{{/if}}
Expand Down
12 changes: 12 additions & 0 deletions ui/tests/acceptance/topology-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ module('Acceptance | topology', function(hooks) {

await Topology.visit();
assert.equal(Topology.infoPanelTitle, 'Cluster Details');
assert.notOk(Topology.filteredNodesWarning.isPresent);
});

test('all allocations for all namespaces and all clients are queried on load', async function(assert) {
Expand Down Expand Up @@ -74,4 +75,15 @@ module('Acceptance | topology', function(hooks) {
await Topology.viz.datacenters[0].nodes[0].selectNode();
assert.equal(Topology.infoPanelTitle, 'Client Details');
});

test('when one or more nodes lack the NodeResources property, a warning message is shown', async function(assert) {
server.createList('node', 3);
server.createList('allocation', 5);

server.schema.nodes.all().models[0].update({ nodeResources: null });

await Topology.visit();
assert.ok(Topology.filteredNodesWarning.isPresent);
assert.ok(Topology.filteredNodesWarning.message.startsWith('1'));
});
});
34 changes: 33 additions & 1 deletion ui/tests/integration/components/topo-viz-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ module('Integration | Component | TopoViz', function(hooks) {
@nodes={{this.nodes}}
@allocations={{this.allocations}}
@onAllocationSelect={{this.onAllocationSelect}}
@onNodeSelect={{this.onNodeSelect}} />
@onNodeSelect={{this.onNodeSelect}}
@onDataError={{this.onDataError}} />
`;

test('presents as a FlexMasonry of datacenters', async function(assert) {
Expand Down Expand Up @@ -167,4 +168,35 @@ module('Integration | Component | TopoViz', function(hooks) {
await TopoViz.datacenters[0].nodes[0].memoryRects[0].select();
assert.equal(TopoViz.allocationAssociations.length, 0);
});

test('when one or more nodes are missing the resources property, those nodes are filtered out of the topology view and onDataError is called', async function(assert) {
const badNode = node('dc1', 'node0', 1000, 500);
delete badNode.resources;

this.setProperties({
nodes: [badNode, node('dc1', 'node1', 1000, 500)],
allocations: [
alloc('node0', 'job1', 'group', 100, 100),
alloc('node0', 'job1', 'group', 100, 100),
alloc('node1', 'job1', 'group', 100, 100),
alloc('node1', 'job1', 'group', 100, 100),
alloc('node0', 'job1', 'groupTwo', 100, 100),
],
onNodeSelect: sinon.spy(),
onAllocationSelect: sinon.spy(),
onDataError: sinon.spy(),
});

await this.render(commonTemplate);

assert.ok(this.onDataError.calledOnce);
assert.deepEqual(this.onDataError.getCall(0).args[0], [
{
type: 'filtered-nodes',
context: [this.nodes[0]],
},
]);

assert.equal(TopoViz.datacenters[0].nodes.length, 1);
});
});
2 changes: 2 additions & 0 deletions ui/tests/pages/topology.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { create, 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';

export default create({
visit: visitable('/topology'),

infoPanelTitle: text('[data-test-info-panel-title]'),
filteredNodesWarning: notification('[data-test-filtered-nodes-warning]'),

viz: TopoViz('[data-test-topo-viz]'),
});

0 comments on commit a53c8eb

Please sign in to comment.