luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

[ui] Fix: Wildcard-datacenter system/sysbatch jobs stopped showing client links/chart (#16274) 

* Fix for wildcard DC sys/sysbatch jobs

* A few extra modules for wildcard DC in systemish jobs

* doesMatchPattern moved to its own util as match-glob

* DC glob lookup using matchGlob

* PR feedback
This commit is contained in:
Phil Renaud 2023-03-06 10:06:31 -05:00 committed by GitHub
parent 2a1a790820
commit edf59597d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 378 additions and 340 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/16274.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
ui: fixed an issue where system/sysbatch jobs with wildcard datacenters (like ["dc*"]) were not showing client status charts
```

39
ui/app/abilities/variable.js
View File

@ -2,6 +2,7 @@
import { computed, get } from '@ember/object';
import { or } from '@ember/object/computed';
import AbstractAbility from './abstract';
import doesMatchPattern from 'nomad-ui/utils/match-glob';
const WILDCARD_GLOB = '*';
const WILDCARD_PATTERN = '/';
@ -188,7 +189,7 @@ export default class Variable extends AbstractAbility {
const matches = [];
for (const pathName of pathNames) {
if (this._doesMatchPattern(pathName, path)) matches.push(pathName);
if (doesMatchPattern(pathName, path)) matches.push(pathName);
}
return matches;
@ -207,42 +208,6 @@ export default class Variable extends AbstractAbility {
return this._smallestDifference(allMatchingPaths, path);
}
_doesMatchPattern(pattern, path) {
const parts = pattern?.split(WILDCARD_GLOB);
const hasLeadingGlob = pattern?.startsWith(WILDCARD_GLOB);
const hasTrailingGlob = pattern?.endsWith(WILDCARD_GLOB);
const lastPartOfPattern = parts[parts.length - 1];
const isPatternWithoutGlob = parts.length === 1 && !hasLeadingGlob;
if (!pattern || !path || isPatternWithoutGlob) {
return pattern === path;
}
if (pattern === WILDCARD_GLOB) {
return true;
}
let subPathToMatchOn = path;
for (let i = 0; i < parts.length; i++) {
const part = parts[i];
const idx = subPathToMatchOn?.indexOf(part);
const doesPathIncludeSubPattern = idx > -1;
const doesMatchOnFirstSubpattern = idx === 0;
if (i === 0 && !hasLeadingGlob && !doesMatchOnFirstSubpattern) {
return false;
}
if (!doesPathIncludeSubPattern) {
return false;
}
subPathToMatchOn = subPathToMatchOn.slice(0, idx + path.length);
}
return hasTrailingGlob || path.endsWith(lastPartOfPattern);
}
_computeLengthDiff(pattern, path) {
const countGlobsInPattern = pattern
?.split('')

52
ui/app/utils/match-glob.js Normal file
View File

@ -0,0 +1,52 @@
// @ts-check
const WILDCARD_GLOB = '*';
/**
*
* @param {string} pattern
* @param {string} comparable
* @example matchGlob('foo', 'foo') // true
* @example matchGlob('foo', 'bar') // false
* @example matchGlob('foo*', 'footbar') // true
* @example matchGlob('foo*', 'bar') // false
* @example matchGlob('*foo', 'foobar') // false
* @example matchGlob('*foo', 'barfoo') // true
* @example matchGlob('foo*bar', 'footbar') // true
* @returns {boolean}
*/
export default function matchGlob(pattern, comparable) {
const parts = pattern?.split(WILDCARD_GLOB);
const hasLeadingGlob = pattern?.startsWith(WILDCARD_GLOB);
const hasTrailingGlob = pattern?.endsWith(WILDCARD_GLOB);
const lastPartOfPattern = parts[parts.length - 1];
const isPatternWithoutGlob = parts.length === 1 && !hasLeadingGlob;
if (!pattern || !comparable || isPatternWithoutGlob) {
return pattern === comparable;
}
if (pattern === WILDCARD_GLOB) {
return true;
}
let subStringToMatchOn = comparable;
for (let i = 0; i < parts.length; i++) {
const part = parts[i];
const idx = subStringToMatchOn?.indexOf(part);
const doesStringIncludeSubPattern = idx > -1;
const doesMatchOnFirstSubpattern = idx === 0;
if (i === 0 && !hasLeadingGlob && !doesMatchOnFirstSubpattern) {
return false;
}
if (!doesStringIncludeSubPattern) {
return false;
}
subStringToMatchOn = subStringToMatchOn.slice(0, idx + comparable.length);
}
return hasTrailingGlob || comparable.endsWith(lastPartOfPattern);
}

5
ui/app/utils/properties/job-client-status.js
View File

@ -1,4 +1,5 @@
import { computed } from '@ember/object';
import matchGlob from '../match-glob';
const STATUS = [
'queued',
@ -26,7 +27,9 @@ export default function jobClientStatus(nodesKey, jobKey) {
// Filter nodes by the datacenters defined in the job.
const filteredNodes = nodes.filter((n) => {
return job.datacenters.indexOf(n.datacenter) >= 0;
return job.datacenters.find((dc) => {
return !!matchGlob(dc, n.datacenter);
});
});
if (job.status === 'pending') {

40
ui/tests/acceptance/job-detail-test.js
View File

@ -30,6 +30,18 @@ moduleForJobWithClientStatus(
})
);
moduleForJobWithClientStatus(
'Acceptance | job detail with client status (system with wildcard dc)',
() =>
server.create('job', {
id: 'system-wildcard-dc',
status: 'running',
datacenters: ['canada-*-1'],
type: 'system',
createAllocations: false,
})
);
moduleForJob('Acceptance | job detail (sysbatch)', 'allocations', () =>
server.create('job', { type: 'sysbatch', shallow: true })
);
@ -59,6 +71,20 @@ moduleForJobWithClientStatus(
}
);
moduleForJobWithClientStatus(
'Acceptance | job detail with client status (sysbatch with namespace and wildcard dc)',
() => {
const namespace = server.create('namespace', { id: 'test' });
return server.create('job', {
status: 'running',
datacenters: ['*'],
type: 'sysbatch',
namespaceId: namespace.name,
createAllocations: false,
});
}
);
moduleForJob('Acceptance | job detail (sysbatch child)', 'allocations', () => {
const parent = server.create('job', 'periodicSysbatch', {
childrenCount: 1,
@ -94,6 +120,20 @@ moduleForJobWithClientStatus(
}
);
moduleForJobWithClientStatus(
'Acceptance | job detail with client status (sysbatch child with namespace and wildcard dc)',
() => {
const namespace = server.create('namespace', { id: 'test' });
const parent = server.create('job', 'periodicSysbatch', {
childrenCount: 1,
shallow: true,
namespaceId: namespace.name,
datacenters: ['*'],
});
return server.db.jobs.where({ parentId: parent.id })[0];
}
);
moduleForJob(
'Acceptance | job detail (periodic)',
'children',

10
ui/tests/helpers/module-for-job.js
View File

@ -239,6 +239,16 @@ export function moduleForJobWithClientStatus(
datacenter: 'dc1',
status: 'ready',
});
clients.push(
server.create('node', { datacenter: 'dc2', status: 'ready' })
);
clients.push(
server.create('node', { datacenter: 'dc3', status: 'ready' })
);
clients.push(
server.create('node', { datacenter: 'canada-west-1', status: 'ready' })
);
job = jobFactory();
clients.forEach((c) => {
server.create('allocation', { jobId: job.id, nodeId: c.id });

302
ui/tests/unit/abilities/variable-test.js
View File

@ -715,308 +715,6 @@ module('Unit | Ability | variable', function (hooks) {
});
});
module('#_doesMatchPattern', function () {
const edgeCaseTest = 'this is a ϗѾ test';
module('base cases', function () {
test('it handles an empty pattern', function (assert) {
// arrange
const pattern = '';
const emptyPath = '';
const nonEmptyPath = 'a';
// act
const matchingResult = this.ability._doesMatchPattern(
pattern,
emptyPath
);
const nonMatchingResult = this.ability._doesMatchPattern(
pattern,
nonEmptyPath
);
// assert
assert.ok(matchingResult, 'Empty pattern should match empty path');
assert.notOk(
nonMatchingResult,
'Empty pattern should not match non-empty path'
);
});
test('it handles an empty path', function (assert) {
// arrange
const emptyPath = '';
const emptyPattern = '';
const nonEmptyPattern = 'a';
// act
const matchingResult = this.ability._doesMatchPattern(
emptyPattern,
emptyPath
);
const nonMatchingResult = this.ability._doesMatchPattern(
nonEmptyPattern,
emptyPath
);
// assert
assert.ok(matchingResult, 'Empty path should match empty pattern');
assert.notOk(
nonMatchingResult,
'Empty path should not match non-empty pattern'
);
});
test('it handles a pattern without a glob', function (assert) {
// arrange
const path = '/foo';
const matchingPattern = '/foo';
const nonMatchingPattern = '/bar';
// act
const matchingResult = this.ability._doesMatchPattern(
matchingPattern,
path
);
const nonMatchingResult = this.ability._doesMatchPattern(
nonMatchingPattern,
path
);
// assert
assert.ok(matchingResult, 'Matches path correctly.');
assert.notOk(nonMatchingResult, 'Does not match non-matching path.');
});
test('it handles a pattern that is a lone glob', function (assert) {
// arrange
const path = '/foo';
const glob = '*';
// act
const matchingResult = this.ability._doesMatchPattern(glob, path);
// assert
assert.ok(matchingResult, 'Matches glob.');
});
test('it matches on leading glob', function (assert) {
// arrange
const pattern = '*bar';
const matchingPath = 'footbar';
const nonMatchingPath = 'rockthecasba';
// act
const matchingResult = this.ability._doesMatchPattern(
pattern,
matchingPath
);
const nonMatchingResult = this.ability._doesMatchPattern(
pattern,
nonMatchingPath
);
// assert
assert.ok(
matchingResult,
'Correctly matches when leading glob and matching path.'
);
assert.notOk(
nonMatchingResult,
'Does not match when leading glob and non-matching path.'
);
});
test('it matches on trailing glob', function (assert) {
// arrange
const pattern = 'foo*';
const matchingPath = 'footbar';
const nonMatchingPath = 'bar';
// act
const matchingResult = this.ability._doesMatchPattern(
pattern,
matchingPath
);
const nonMatchingResult = this.ability._doesMatchPattern(
pattern,
nonMatchingPath
);
// assert
assert.ok(matchingResult, 'Correctly matches on trailing glob.');
assert.notOk(
nonMatchingResult,
'Does not match on trailing glob if pattern does not match.'
);
});
test('it matches when glob is in middle', function (assert) {
// arrange
const pattern = 'foo*bar';
const matchingPath = 'footbar';
const nonMatchingPath = 'footba';
// act
const matchingResult = this.ability._doesMatchPattern(
pattern,
matchingPath
);
const nonMatchingResult = this.ability._doesMatchPattern(
pattern,
nonMatchingPath
);
// assert
assert.ok(
matchingResult,
'Correctly matches on glob in middle of path.'
);
assert.notOk(
nonMatchingResult,
'Does not match on glob in middle of path if not full pattern match.'
);
});
});
module('matching edge cases', function () {
test('it matches when string is between globs', function (assert) {
// arrange
const pattern = '*is *';
// act
const result = this.ability._doesMatchPattern(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles many non-consective globs', function (assert) {
// arrange
const pattern = '*is*a*';
// act
const result = this.ability._doesMatchPattern(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles double globs', function (assert) {
// arrange
const pattern = '**test**';
// act
const result = this.ability._doesMatchPattern(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles many consecutive globs', function (assert) {
// arrange
const pattern = '**is**a***test*';
// act
const result = this.ability._doesMatchPattern(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles white space between globs', function (assert) {
// arrange
const pattern = '* *';
// act
const result = this.ability._doesMatchPattern(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles a pattern of only globs', function (assert) {
// arrange
const pattern = '**********';
// act
const result = this.ability._doesMatchPattern(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles unicode characters', function (assert) {
// arrange
const pattern = `*Ѿ*`;
// act
const result = this.ability._doesMatchPattern(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles mixed ASCII codes', function (assert) {
// arrange
const pattern = `*is a ϗѾ *`;
// act
const result = this.ability._doesMatchPattern(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
});
module('non-matching edge cases', function () {
const failingCases = [
{
case: 'test*',
message: 'Implicit substring match',
},
{
case: '*is',
message: 'Parial match',
},
{
case: '*no*',
message: 'Globs without match between them',
},
{
case: ' ',
message: 'Plain white space',
},
{
case: '* ',
message: 'Trailing white space',
},
{
case: ' *',
message: 'Leading white space',
},
{
case: '*ʤ*',
message: 'Non-matching unicode',
},
{
case: 'this*this is a test',
message: 'Repeated prefix',
},
];
failingCases.forEach(({ case: failingPattern, message }) => {
test('should fail the specified cases', function (assert) {
const result = this.ability._doesMatchPattern(
failingPattern,
edgeCaseTest
);
assert.notOk(result, `${message} should not match.`);
});
});
});
});
module('#_computeLengthDiff', function () {
test('should return the difference in length between a path and a pattern', function (assert) {
// arrange

267
ui/tests/unit/utils/match-glob-test.js Normal file
View File

@ -0,0 +1,267 @@
import matchGlob from 'nomad-ui/utils/match-glob';
import { module, test } from 'qunit';
module('Unit | Utility | match-glob', function () {
module('#_doesMatchPattern', function () {
const edgeCaseTest = 'this is a ϗѾ test';
module('base cases', function () {
test('it handles an empty pattern', function (assert) {
// arrange
const pattern = '';
const emptyPath = '';
const nonEmptyPath = 'a';
// act
const matchingResult = matchGlob(pattern, emptyPath);
const nonMatchingResult = matchGlob(pattern, nonEmptyPath);
// assert
assert.ok(matchingResult, 'Empty pattern should match empty path');
assert.notOk(
nonMatchingResult,
'Empty pattern should not match non-empty path'
);
});
test('it handles an empty path', function (assert) {
// arrange
const emptyPath = '';
const emptyPattern = '';
const nonEmptyPattern = 'a';
// act
const matchingResult = matchGlob(emptyPattern, emptyPath);
const nonMatchingResult = matchGlob(nonEmptyPattern, emptyPath);
// assert
assert.ok(matchingResult, 'Empty path should match empty pattern');
assert.notOk(
nonMatchingResult,
'Empty path should not match non-empty pattern'
);
});
test('it handles a pattern without a glob', function (assert) {
// arrange
const path = '/foo';
const matchingPattern = '/foo';
const nonMatchingPattern = '/bar';
// act
const matchingResult = matchGlob(matchingPattern, path);
const nonMatchingResult = matchGlob(nonMatchingPattern, path);
// assert
assert.ok(matchingResult, 'Matches path correctly.');
assert.notOk(nonMatchingResult, 'Does not match non-matching path.');
});
test('it handles a pattern that is a lone glob', function (assert) {
// arrange
const path = '/foo';
const glob = '*';
// act
const matchingResult = matchGlob(glob, path);
// assert
assert.ok(matchingResult, 'Matches glob.');
});
test('it matches on leading glob', function (assert) {
// arrange
const pattern = '*bar';
const matchingPath = 'footbar';
const nonMatchingPath = 'rockthecasba';
// act
const matchingResult = matchGlob(pattern, matchingPath);
const nonMatchingResult = matchGlob(pattern, nonMatchingPath);
// assert
assert.ok(
matchingResult,
'Correctly matches when leading glob and matching path.'
);
assert.notOk(
nonMatchingResult,
'Does not match when leading glob and non-matching path.'
);
});
test('it matches on trailing glob', function (assert) {
// arrange
const pattern = 'foo*';
const matchingPath = 'footbar';
const nonMatchingPath = 'bar';
// act
const matchingResult = matchGlob(pattern, matchingPath);
const nonMatchingResult = matchGlob(pattern, nonMatchingPath);
// assert
assert.ok(matchingResult, 'Correctly matches on trailing glob.');
assert.notOk(
nonMatchingResult,
'Does not match on trailing glob if pattern does not match.'
);
});
test('it matches when glob is in middle', function (assert) {
// arrange
const pattern = 'foo*bar';
const matchingPath = 'footbar';
const nonMatchingPath = 'footba';
// act
const matchingResult = matchGlob(pattern, matchingPath);
const nonMatchingResult = matchGlob(pattern, nonMatchingPath);
// assert
assert.ok(
matchingResult,
'Correctly matches on glob in middle of path.'
);
assert.notOk(
nonMatchingResult,
'Does not match on glob in middle of path if not full pattern match.'
);
});
});
module('matching edge cases', function () {
test('it matches when string is between globs', function (assert) {
// arrange
const pattern = '*is *';
// act
const result = matchGlob(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles many non-consective globs', function (assert) {
// arrange
const pattern = '*is*a*';
// act
const result = matchGlob(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles double globs', function (assert) {
// arrange
const pattern = '**test**';
// act
const result = matchGlob(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles many consecutive globs', function (assert) {
// arrange
const pattern = '**is**a***test*';
// act
const result = matchGlob(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles white space between globs', function (assert) {
// arrange
const pattern = '* *';
// act
const result = matchGlob(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles a pattern of only globs', function (assert) {
// arrange
const pattern = '**********';
// act
const result = matchGlob(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles unicode characters', function (assert) {
// arrange
const pattern = `*Ѿ*`;
// act
const result = matchGlob(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
test('it handles mixed ASCII codes', function (assert) {
// arrange
const pattern = `*is a ϗѾ *`;
// act
const result = matchGlob(pattern, edgeCaseTest);
// assert
assert.ok(result);
});
});
module('non-matching edge cases', function () {
const failingCases = [
{
case: 'test*',
message: 'Implicit substring match',
},
{
case: '*is',
message: 'Parial match',
},
{
case: '*no*',
message: 'Globs without match between them',
},
{
case: ' ',
message: 'Plain white space',
},
{
case: '* ',
message: 'Trailing white space',
},
{
case: ' *',
message: 'Leading white space',
},
{
case: '*ʤ*',
message: 'Non-matching unicode',
},
{
case: 'this*this is a test',
message: 'Repeated prefix',
},
];
failingCases.forEach(({ case: failingPattern, message }) => {
test('should fail the specified cases', function (assert) {
const result = matchGlob(failingPattern, edgeCaseTest);
assert.notOk(result, `${message} should not match.`);
});
});
});
});
});