Skip to content

Commit

Permalink
[ui, bugfix] Link fix for volumes where per_alloc=true (#12713)
Browse files Browse the repository at this point in the history
* Allocation page linkfix

* fix added to task page and computed prop moved to allocation model

* Fallback query added to task group when specific volume isnt knowable

* Delog

* link text reflects alloc suffix

* Helper instead of in-template conditionals

* formatVolumeName unit test

* Removing unused helper import
  • Loading branch information
philrenaud committed Apr 21, 2022
1 parent f1fcd50 commit a977577
Show file tree
Hide file tree
Showing 11 changed files with 105 additions and 10 deletions.
3 changes: 3 additions & 0 deletions .changelog/12713.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: Fixed a bug where volumes were being incorrectly linked when per_alloc=true
```
1 change: 1 addition & 0 deletions jsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"compilerOptions":{"target":"es6","experimentalDecorators":true},"exclude":["node_modules","bower_components","tmp","vendor",".git","dist"]}
1 change: 1 addition & 0 deletions ui/.template-lintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ module.exports = {
'no-action': 'off',
'no-invalid-interactive': 'off',
'no-inline-styles': 'off',
'no-curly-component-invocation': { allow: ['format-volume-name'] },
},
};
1 change: 1 addition & 0 deletions ui/app/components/task-row.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { computed } from '@ember/object';
import { alias } from '@ember/object/computed';
import { task, timeout } from 'ember-concurrency';
import { lazyClick } from '../helpers/lazy-click';

import {
classNames,
tagName,
Expand Down
18 changes: 18 additions & 0 deletions ui/app/helpers/format-volume-name.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { helper } from '@ember/component/helper';

/**
* Volume Name Formatter
*
* Usage: {{format-volume-name source=string isPerAlloc=boolean volumeExtension=string}}
*
* Outputs a title/link for volumes that are per_alloc-aware.
* (when a volume is per_alloc, its route location requires an additional extension)
*/
export function formatVolumeName(
_,
{ source = '', isPerAlloc, volumeExtension }
) {
return `${source}${isPerAlloc ? volumeExtension : ''}`;
}

export default helper(formatVolumeName);
7 changes: 7 additions & 0 deletions ui/app/models/allocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@ export default class Allocation extends Model {
return [];
}

// When per_alloc is set to true on a volume, the volumes are duplicated between active allocations.
// We differentiate them with a [#] suffix, inferred from a volume's allocation's name property.
@computed('name')
get volumeExtension() {
return this.name.substring(this.name.lastIndexOf('['));
}

@fragmentArray('task-state') states;
@fragmentArray('reschedule-event') rescheduleEvents;

Expand Down
1 change: 1 addition & 0 deletions ui/app/models/volume-definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export default class VolumeDefinition extends Fragment {
@attr('string') source;
@attr('string') type;
@attr('boolean') readOnly;
@attr('boolean') perAlloc;

@equal('type', 'csi') isCSI;
@alias('taskGroup.job.namespace') namespace;
Expand Down
13 changes: 11 additions & 2 deletions ui/app/templates/allocations/allocation/task/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,19 @@
{{#if row.model.isCSI}}
<LinkTo
@route="csi.volumes.volume"
@model={{concat row.model.volume "@" row.model.namespace.id
@model={{concat
(format-volume-name
source=row.model.source
isPerAlloc=row.model.volumeDeclaration.perAlloc
volumeExtension=this.model.allocation.volumeExtension)
"@"
row.model.namespace.id
}}
>
{{row.model.volume}}
{{format-volume-name
source=row.model.source
isPerAlloc=row.model.volumeDeclaration.perAlloc
volumeExtension=this.model.allocation.volumeExtension}}
</LinkTo>
{{else}}
{{row.model.source}}
Expand Down
14 changes: 12 additions & 2 deletions ui/app/templates/components/task-row.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,19 @@
{{#if volume.isCSI}}
<LinkTo
@route="csi.volumes.volume"
@model={{concat volume.source "@" volume.namespace.id}}
@model={{concat
(format-volume-name
source=volume.source
isPerAlloc=volume.volumeDeclaration.perAlloc
volumeExtension=this.task.allocation.volumeExtension)
"@"
volume.namespace.id
}}
>
{{volume.source}}
{{format-volume-name
source=volume.source
isPerAlloc=volume.volumeDeclaration.perAlloc
volumeExtension=this.task.allocation.volumeExtension}}
</LinkTo>
{{else}}
{{volume.source}}
Expand Down
17 changes: 11 additions & 6 deletions ui/app/templates/jobs/job/task-group.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -293,12 +293,17 @@
<tr data-test-volume>
<td data-test-volume-name>
{{#if row.model.isCSI}}
<LinkTo
@route="csi.volumes.volume"
@model={{concat row.model.source "@" row.model.namespace.id}}
>
{{row.model.name}}
</LinkTo>
{{!-- if volume is per_alloc=true, there's no one specific volume. So, link to the volumes index with an active query --}}
{{#if row.model.perAlloc}}
<LinkTo @route="csi.volumes.index" @query={{hash search=row.model.source}}>{{row.model.name}}</LinkTo>
{{else}}
<LinkTo
@route="csi.volumes.volume"
@model={{concat row.model.source "@" row.model.namespace.id}}
>
{{row.model.name}}
</LinkTo>
{{/if}}
{{else}}
{{row.model.name}}
{{/if}}
Expand Down
39 changes: 39 additions & 0 deletions ui/tests/unit/helpers/format-volume-name-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { module, test } from 'qunit';
import { formatVolumeName } from 'nomad-ui/helpers/format-volume-name';

module('Unit | Helper | formatVolumeName', function () {
test('Returns source as string when isPerAlloc is false', function (assert) {
const expectation = 'my-volume-source';
assert.equal(
formatVolumeName(null, {
source: 'my-volume-source',
isPerAlloc: false,
volumeExtension: '[arbitrary]',
}),
expectation,
'false perAlloc'
);
assert.equal(
formatVolumeName(null, {
source: 'my-volume-source',
isPerAlloc: null,
volumeExtension: '[arbitrary]',
}),
expectation,
'null perAlloc'
);
});

test('Returns concatonated name when isPerAlloc is true', function (assert) {
const expectation = 'my-volume-source[1]';
assert.equal(
formatVolumeName(null, {
source: 'my-volume-source',
isPerAlloc: true,
volumeExtension: '[1]',
}),
expectation,
expectation
);
});
});

0 comments on commit a977577

Please sign in to comment.