Skip to content

Commit

Permalink
ui: fix Topology node state filter (#17940)
Browse files Browse the repository at this point in the history
"Ineligible" and "Draining" are not determined by the node status, but
are rather inferred from other fields.
  • Loading branch information
lgfa29 authored and jrasell committed Jul 26, 2023
1 parent 476c2aa commit 9df3822
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 3 deletions.
3 changes: 3 additions & 0 deletions .changelog/17940.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: Fixed a bug that prevented nodes from being filtered by the "Ineligible" and "Draining" state filters
```
7 changes: 6 additions & 1 deletion ui/app/controllers/topology.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,13 @@ export default class TopologyControllers extends Controller.extend(Searchable) {
selectionClass,
selectionNodePool,
} = this;
const matchState =
selectionState.includes(node.status) ||
(selectionState.includes('ineligible') && !node.isEligible) ||
(selectionState.includes('draining') && node.isDraining);

return (
(selectionState.length ? selectionState.includes(node.status) : true) &&
(selectionState.length ? matchState : true) &&
(selectionVersion.length
? selectionVersion.includes(node.version)
: true) &&
Expand Down
49 changes: 47 additions & 2 deletions ui/tests/acceptance/topology-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,12 @@ module('Acceptance | topology', function (hooks) {
nodeClass: 'foo-bar-baz',
});

// Make sure we have at least one node draining and one ineligible.
server.create('node', {
schedulingEligibility: 'ineligible',
});
server.create('node', 'draining');

// Create node pool exclusive for these nodes.
server.create('node-pool', { name: 'test-node-pool' });
server.createList('node', 3, {
Expand All @@ -342,16 +348,32 @@ module('Acceptance | topology', function (hooks) {

server.createList('allocation', 5);

// Count draining and ineligible nodes.
const counts = {
ineligible: 0,
draining: 0,
};
server.db.nodes.forEach((n) => {
if (n.schedulingEligibility === 'ineligible') {
counts['ineligible'] += 1;
}
if (n.drain) {
counts['draining'] += 1;
}
});

await Topology.visit();
assert.dom('[data-test-topo-viz-node]').exists({ count: 15 });
assert.dom('[data-test-topo-viz-node]').exists({ count: 17 });

// Test search.
await typeIn('input.node-search', server.schema.nodes.first().name);
assert.dom('[data-test-topo-viz-node]').exists({ count: 1 });
await typeIn('input.node-search', server.schema.nodes.first().name);
assert.dom('[data-test-topo-viz-node]').doesNotExist();
await click('[title="Clear search"]');
assert.dom('[data-test-topo-viz-node]').exists({ count: 15 });
assert.dom('[data-test-topo-viz-node]').exists({ count: 17 });

// Test node class filter.
await Topology.facets.class.toggle();
await Topology.facets.class.options
.findOneBy('label', 'foo-bar-baz')
Expand All @@ -361,6 +383,29 @@ module('Acceptance | topology', function (hooks) {
.findOneBy('label', 'foo-bar-baz')
.toggle();

// Test ineligible state filter.
await Topology.facets.state.toggle();
await Topology.facets.state.options
.findOneBy('label', 'Ineligible')
.toggle();
assert
.dom('[data-test-topo-viz-node]')
.exists({ count: counts['ineligible'] });
await Topology.facets.state.options
.findOneBy('label', 'Ineligible')
.toggle();
await Topology.facets.state.toggle();

// Test draining state filter.
await Topology.facets.state.toggle();
await Topology.facets.state.options.findOneBy('label', 'Draining').toggle();
assert
.dom('[data-test-topo-viz-node]')
.exists({ count: counts['draining'] });
await Topology.facets.state.options.findOneBy('label', 'Draining').toggle();
await Topology.facets.state.toggle();

// Test node pool filter.
await Topology.facets.nodePool.toggle();
await Topology.facets.nodePool.options
.findOneBy('label', 'test-node-pool')
Expand Down

0 comments on commit 9df3822

Please sign in to comment.