Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Guard against nodes running an old version of the Nomad agent #9733

Merged
merged 2 commits into from
Jan 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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]'),
});