From 39265849a63a868afc013c173b66d84ec2a7a365 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Tue, 19 Apr 2022 23:40:52 -0400 Subject: [PATCH 1/8] Allocation page linkfix --- ui/app/components/task-row.js | 8 ++++++++ ui/app/models/volume-definition.js | 1 + ui/app/templates/components/task-row.hbs | 2 +- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/ui/app/components/task-row.js b/ui/app/components/task-row.js index 35343205d4d3..c0bb5533c14f 100644 --- a/ui/app/components/task-row.js +++ b/ui/app/components/task-row.js @@ -31,6 +31,14 @@ export default class TaskRow extends Component { return !Ember.testing; } + // 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('task.allocation.name') + get volumeExtension() { + let allocName = this.task.allocation.name; + return allocName.substring(allocName.lastIndexOf('[')); + } + // Since all tasks for an allocation share the same tracker, use the registry @computed('task.{allocation,isRunning}') get stats() { diff --git a/ui/app/models/volume-definition.js b/ui/app/models/volume-definition.js index 77e6c0819a9f..20624d79631e 100644 --- a/ui/app/models/volume-definition.js +++ b/ui/app/models/volume-definition.js @@ -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; diff --git a/ui/app/templates/components/task-row.hbs b/ui/app/templates/components/task-row.hbs index 521f0b088d07..f9daf70ccd12 100644 --- a/ui/app/templates/components/task-row.hbs +++ b/ui/app/templates/components/task-row.hbs @@ -47,7 +47,7 @@ {{#if volume.isCSI}} {{volume.source}} From f16fdc5275972104f1ecbb194fa47fa2a9f3e316 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Tue, 19 Apr 2022 23:58:33 -0400 Subject: [PATCH 2/8] fix added to task page and computed prop moved to allocation model --- ui/.ember-cli | 2 +- ui/app/components/task-row.js | 8 -------- ui/app/models/allocation.js | 7 +++++++ ui/app/templates/allocations/allocation/task/index.hbs | 5 +++-- ui/app/templates/components/task-row.hbs | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/ui/.ember-cli b/ui/.ember-cli index dec208fd9dd9..2e2f72a26775 100644 --- a/ui/.ember-cli +++ b/ui/.ember-cli @@ -6,5 +6,5 @@ Setting `disableAnalytics` to true will prevent any data from being sent. */ "disableAnalytics": false, - "proxy": "http://127.0.0.1:4646" + "proxy": "http://192.168.56.11:4646" } diff --git a/ui/app/components/task-row.js b/ui/app/components/task-row.js index c0bb5533c14f..35343205d4d3 100644 --- a/ui/app/components/task-row.js +++ b/ui/app/components/task-row.js @@ -31,14 +31,6 @@ export default class TaskRow extends Component { return !Ember.testing; } - // 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('task.allocation.name') - get volumeExtension() { - let allocName = this.task.allocation.name; - return allocName.substring(allocName.lastIndexOf('[')); - } - // Since all tasks for an allocation share the same tracker, use the registry @computed('task.{allocation,isRunning}') get stats() { diff --git a/ui/app/models/allocation.js b/ui/app/models/allocation.js index fb3aa8445210..9f9e1a05459d 100644 --- a/ui/app/models/allocation.js +++ b/ui/app/models/allocation.js @@ -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; diff --git a/ui/app/templates/allocations/allocation/task/index.hbs b/ui/app/templates/allocations/allocation/task/index.hbs index e6d62dc20953..477ebe757411 100644 --- a/ui/app/templates/allocations/allocation/task/index.hbs +++ b/ui/app/templates/allocations/allocation/task/index.hbs @@ -161,12 +161,13 @@ {{#if row.model.isCSI}} + {{log "ah heck" row.model}} - {{row.model.volume}} + {{row.model.source}} {{else}} {{row.model.source}} diff --git a/ui/app/templates/components/task-row.hbs b/ui/app/templates/components/task-row.hbs index f9daf70ccd12..44dd0f34691b 100644 --- a/ui/app/templates/components/task-row.hbs +++ b/ui/app/templates/components/task-row.hbs @@ -47,7 +47,7 @@ {{#if volume.isCSI}} {{volume.source}} From ffc2d4c21e1e58835632dc737278e1dab6026f31 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Wed, 20 Apr 2022 00:21:36 -0400 Subject: [PATCH 3/8] Fallback query added to task group when specific volume isnt knowable --- ui/.ember-cli | 2 +- ui/app/templates/jobs/job/task-group.hbs | 17 +++++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/ui/.ember-cli b/ui/.ember-cli index 2e2f72a26775..dec208fd9dd9 100644 --- a/ui/.ember-cli +++ b/ui/.ember-cli @@ -6,5 +6,5 @@ Setting `disableAnalytics` to true will prevent any data from being sent. */ "disableAnalytics": false, - "proxy": "http://192.168.56.11:4646" + "proxy": "http://127.0.0.1:4646" } diff --git a/ui/app/templates/jobs/job/task-group.hbs b/ui/app/templates/jobs/job/task-group.hbs index 2402b5df63a5..83160e68a054 100644 --- a/ui/app/templates/jobs/job/task-group.hbs +++ b/ui/app/templates/jobs/job/task-group.hbs @@ -293,12 +293,17 @@ {{#if row.model.isCSI}} - - {{row.model.name}} - + {{!-- 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}} + {{row.model.name}} + {{else}} + + {{row.model.name}} + + {{/if}} {{else}} {{row.model.name}} {{/if}} From 190370d775cb82a93133a3ad31e85e43f0730771 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Wed, 20 Apr 2022 00:30:19 -0400 Subject: [PATCH 4/8] Delog --- ui/app/templates/allocations/allocation/task/index.hbs | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/app/templates/allocations/allocation/task/index.hbs b/ui/app/templates/allocations/allocation/task/index.hbs index 477ebe757411..024d04f798ea 100644 --- a/ui/app/templates/allocations/allocation/task/index.hbs +++ b/ui/app/templates/allocations/allocation/task/index.hbs @@ -161,7 +161,6 @@ {{#if row.model.isCSI}} - {{log "ah heck" row.model}} Date: Wed, 20 Apr 2022 14:40:47 -0400 Subject: [PATCH 5/8] link text reflects alloc suffix --- ui/app/templates/allocations/allocation/task/index.hbs | 2 +- ui/app/templates/components/task-row.hbs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/app/templates/allocations/allocation/task/index.hbs b/ui/app/templates/allocations/allocation/task/index.hbs index 024d04f798ea..820038295265 100644 --- a/ui/app/templates/allocations/allocation/task/index.hbs +++ b/ui/app/templates/allocations/allocation/task/index.hbs @@ -166,7 +166,7 @@ @model={{concat row.model.source (if row.model.volumeDeclaration.perAlloc this.model.allocation.volumeExtension) "@" row.model.namespace.id }} > - {{row.model.source}} + {{row.model.source}}{{#if row.model.volumeDeclaration.perAlloc}}{{this.model.allocation.volumeExtension}}{{/if}} {{else}} {{row.model.source}} diff --git a/ui/app/templates/components/task-row.hbs b/ui/app/templates/components/task-row.hbs index 44dd0f34691b..9d40377c6a0d 100644 --- a/ui/app/templates/components/task-row.hbs +++ b/ui/app/templates/components/task-row.hbs @@ -49,7 +49,7 @@ @route="csi.volumes.volume" @model={{concat volume.source (if volume.volumeDeclaration.perAlloc this.task.allocation.volumeExtension) "@" volume.namespace.id}} > - {{volume.source}} + {{volume.source}}{{#if volume.volumeDeclaration.perAlloc}}{{this.task.allocation.volumeExtension}}{{/if}} {{else}} {{volume.source}} From 0ebdeb6919f0fb88e64c1b028c1da862f5288fd9 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Thu, 21 Apr 2022 11:28:21 -0400 Subject: [PATCH 6/8] Helper instead of in-template conditionals --- .changelog/12713.txt | 3 +++ jsconfig.json | 1 + ui/.template-lintrc.js | 1 + ui/app/components/task-row.js | 2 ++ ui/app/helpers/format-volume-name.js | 15 +++++++++++++++ .../allocations/allocation/task/index.hbs | 13 +++++++++++-- ui/app/templates/components/task-row.hbs | 14 ++++++++++++-- 7 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 .changelog/12713.txt create mode 100644 jsconfig.json create mode 100644 ui/app/helpers/format-volume-name.js diff --git a/.changelog/12713.txt b/.changelog/12713.txt new file mode 100644 index 000000000000..3094100bfc85 --- /dev/null +++ b/.changelog/12713.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: Fixed a bug where volumes were being incorrectly linked when per_alloc=true +``` \ No newline at end of file diff --git a/jsconfig.json b/jsconfig.json new file mode 100644 index 000000000000..f408cac8c29f --- /dev/null +++ b/jsconfig.json @@ -0,0 +1 @@ +{"compilerOptions":{"target":"es6","experimentalDecorators":true},"exclude":["node_modules","bower_components","tmp","vendor",".git","dist"]} \ No newline at end of file diff --git a/ui/.template-lintrc.js b/ui/.template-lintrc.js index fdfa6bd90c16..28057918d84a 100644 --- a/ui/.template-lintrc.js +++ b/ui/.template-lintrc.js @@ -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'] }, }, }; diff --git a/ui/app/components/task-row.js b/ui/app/components/task-row.js index 35343205d4d3..d5320fd2bfcd 100644 --- a/ui/app/components/task-row.js +++ b/ui/app/components/task-row.js @@ -5,6 +5,8 @@ import { computed } from '@ember/object'; import { alias } from '@ember/object/computed'; import { task, timeout } from 'ember-concurrency'; import { lazyClick } from '../helpers/lazy-click'; +import { formatVolumeName } from '../helpers/format-volume-name'; + import { classNames, tagName, diff --git a/ui/app/helpers/format-volume-name.js b/ui/app/helpers/format-volume-name.js new file mode 100644 index 000000000000..21f09dd47636 --- /dev/null +++ b/ui/app/helpers/format-volume-name.js @@ -0,0 +1,15 @@ +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) + */ +function formatVolumeName(_, { source, isPerAlloc, volumeExtension }, b, c) { + return `${source}${isPerAlloc ? volumeExtension : ''}`; +} + +export default Helper.helper(formatVolumeName); diff --git a/ui/app/templates/allocations/allocation/task/index.hbs b/ui/app/templates/allocations/allocation/task/index.hbs index 820038295265..3191a1601c37 100644 --- a/ui/app/templates/allocations/allocation/task/index.hbs +++ b/ui/app/templates/allocations/allocation/task/index.hbs @@ -163,10 +163,19 @@ {{#if row.model.isCSI}} - {{row.model.source}}{{#if row.model.volumeDeclaration.perAlloc}}{{this.model.allocation.volumeExtension}}{{/if}} + {{format-volume-name + source=row.model.source + isPerAlloc=row.model.volumeDeclaration.perAlloc + volumeExtension=this.model.allocation.volumeExtension}} {{else}} {{row.model.source}} diff --git a/ui/app/templates/components/task-row.hbs b/ui/app/templates/components/task-row.hbs index 9d40377c6a0d..f6e79314e65c 100644 --- a/ui/app/templates/components/task-row.hbs +++ b/ui/app/templates/components/task-row.hbs @@ -47,9 +47,19 @@ {{#if volume.isCSI}} - {{volume.source}}{{#if volume.volumeDeclaration.perAlloc}}{{this.task.allocation.volumeExtension}}{{/if}} + {{format-volume-name + source=volume.source + isPerAlloc=volume.volumeDeclaration.perAlloc + volumeExtension=this.task.allocation.volumeExtension}} {{else}} {{volume.source}} From f1ee4bdf41d1ce67638f4ab540ce084e7d0a6861 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Thu, 21 Apr 2022 12:17:54 -0400 Subject: [PATCH 7/8] formatVolumeName unit test --- ui/app/helpers/format-volume-name.js | 9 +++-- .../unit/helpers/format-volume-name-test.js | 39 +++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 ui/tests/unit/helpers/format-volume-name-test.js diff --git a/ui/app/helpers/format-volume-name.js b/ui/app/helpers/format-volume-name.js index 21f09dd47636..5ec96e28192a 100644 --- a/ui/app/helpers/format-volume-name.js +++ b/ui/app/helpers/format-volume-name.js @@ -1,4 +1,4 @@ -import Helper from '@ember/component/helper'; +import { helper } from '@ember/component/helper'; /** * Volume Name Formatter @@ -8,8 +8,11 @@ import Helper from '@ember/component/helper'; * Outputs a title/link for volumes that are per_alloc-aware. * (when a volume is per_alloc, its route location requires an additional extension) */ -function formatVolumeName(_, { source, isPerAlloc, volumeExtension }, b, c) { +export function formatVolumeName( + _, + { source = '', isPerAlloc, volumeExtension } +) { return `${source}${isPerAlloc ? volumeExtension : ''}`; } -export default Helper.helper(formatVolumeName); +export default helper(formatVolumeName); diff --git a/ui/tests/unit/helpers/format-volume-name-test.js b/ui/tests/unit/helpers/format-volume-name-test.js new file mode 100644 index 000000000000..4fafb7c8328b --- /dev/null +++ b/ui/tests/unit/helpers/format-volume-name-test.js @@ -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 + ); + }); +}); From 78c72828d725cb7b8025164de43c57ae905b24c4 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Thu, 21 Apr 2022 12:34:13 -0400 Subject: [PATCH 8/8] Removing unused helper import --- ui/app/components/task-row.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/app/components/task-row.js b/ui/app/components/task-row.js index d5320fd2bfcd..64bccf1a5f5a 100644 --- a/ui/app/components/task-row.js +++ b/ui/app/components/task-row.js @@ -5,7 +5,6 @@ import { computed } from '@ember/object'; import { alias } from '@ember/object/computed'; import { task, timeout } from 'ember-concurrency'; import { lazyClick } from '../helpers/lazy-click'; -import { formatVolumeName } from '../helpers/format-volume-name'; import { classNames,