From 203b54b2464420b800431b21a4c10c90f6696eaf Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 15 May 2019 15:33:58 -0700 Subject: [PATCH 01/11] Move addToPath to its own util --- ui/app/adapters/job.js | 12 +--------- ui/app/utils/add-to-path.js | 11 +++++++++ ui/tests/unit/utils/add-to-path-test.js | 32 +++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 11 deletions(-) create mode 100644 ui/app/utils/add-to-path.js create mode 100644 ui/tests/unit/utils/add-to-path-test.js diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index a75da89d3016..9bb5a2485826 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -1,5 +1,6 @@ import { inject as service } from '@ember/service'; import Watchable from './watchable'; +import addToPath from 'nomad-ui/utils/add-to-path'; export default Watchable.extend({ system: service(), @@ -118,14 +119,3 @@ function associateNamespace(url, namespace) { } return url; } - -function addToPath(url, extension = '') { - const [path, params] = url.split('?'); - let newUrl = `${path}${extension}`; - - if (params) { - newUrl += `?${params}`; - } - - return newUrl; -} diff --git a/ui/app/utils/add-to-path.js b/ui/app/utils/add-to-path.js new file mode 100644 index 000000000000..7b76e263bcbd --- /dev/null +++ b/ui/app/utils/add-to-path.js @@ -0,0 +1,11 @@ +// Adds a string to the end of a URL path while being mindful of query params +export default function addToPath(url, extension = '') { + const [path, params] = url.split('?'); + let newUrl = `${path}${extension}`; + + if (params) { + newUrl += `?${params}`; + } + + return newUrl; +} diff --git a/ui/tests/unit/utils/add-to-path-test.js b/ui/tests/unit/utils/add-to-path-test.js new file mode 100644 index 000000000000..6317cfad0325 --- /dev/null +++ b/ui/tests/unit/utils/add-to-path-test.js @@ -0,0 +1,32 @@ +import { module, test } from 'qunit'; +import addToPath from 'nomad-ui/utils/add-to-path'; + +const testCases = [ + { + name: 'Only domain', + in: ['https://domain.com', '/path'], + out: 'https://domain.com/path', + }, + { + name: 'Deep path', + in: ['https://domain.com/a/path', '/to/nowhere'], + out: 'https://domain.com/a/path/to/nowhere', + }, + { + name: 'With Query Params', + in: ['https://domain.com?interesting=development', '/this-is-an'], + out: 'https://domain.com/this-is-an?interesting=development', + }, +]; + +module('Unit | Util | addToPath', function() { + testCases.forEach(testCase => { + test(testCase.name, function(assert) { + assert.equal( + addToPath.apply(null, testCase.in), + testCase.out, + `[${testCase.in.join(', ')}] => ${testCase.out}` + ); + }); + }); +}); From b41af1c67a307340c2ca20393aee1837d551d5b8 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 16 May 2019 17:41:40 -0700 Subject: [PATCH 02/11] Add cancel on click outside and disabled behaviors to two-step-button --- ui/app/components/two-step-button.js | 16 +++++ .../templates/components/two-step-button.hbs | 7 +- ui/tests/integration/two-step-button-test.js | 66 +++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/ui/app/components/two-step-button.js b/ui/app/components/two-step-button.js index 8e7bc05d57bd..1499edbf090d 100644 --- a/ui/app/components/two-step-button.js +++ b/ui/app/components/two-step-button.js @@ -1,5 +1,7 @@ import Component from '@ember/component'; +import { next } from '@ember/runloop'; import { equal } from '@ember/object/computed'; +import { task, waitForEvent } from 'ember-concurrency'; import RSVP from 'rsvp'; export default Component.extend({ @@ -10,6 +12,7 @@ export default Component.extend({ confirmText: '', confirmationMessage: '', awaitingConfirmation: false, + disabled: false, onConfirm() {}, onCancel() {}, @@ -17,12 +20,25 @@ export default Component.extend({ isIdle: equal('state', 'idle'), isPendingConfirmation: equal('state', 'prompt'), + cancelOnClickOutside: task(function*() { + while (true) { + let ev = yield waitForEvent(document.body, 'click'); + if (!this.element.contains(ev.target) && !this.awaitingConfirmation) { + this.send('setToIdle'); + } + } + }), + actions: { setToIdle() { this.set('state', 'idle'); + this.cancelOnClickOutside.cancelAll(); }, promptForConfirmation() { this.set('state', 'prompt'); + next(() => { + this.cancelOnClickOutside.perform(); + }); }, confirm() { RSVP.resolve(this.onConfirm()).then(() => { diff --git a/ui/app/templates/components/two-step-button.hbs b/ui/app/templates/components/two-step-button.hbs index 1b95184fea51..e29194e9f355 100644 --- a/ui/app/templates/components/two-step-button.hbs +++ b/ui/app/templates/components/two-step-button.hbs @@ -1,5 +1,10 @@ {{#if isIdle}} - {{else if isPendingConfirmation}} diff --git a/ui/tests/integration/two-step-button-test.js b/ui/tests/integration/two-step-button-test.js index fb7b30a0b80e..0256e42e91cc 100644 --- a/ui/tests/integration/two-step-button-test.js +++ b/ui/tests/integration/two-step-button-test.js @@ -14,6 +14,7 @@ module('Integration | Component | two step button', function(hooks) { confirmText: 'Confirm Action', confirmationMessage: 'Are you certain', awaitingConfirmation: false, + disabled: false, onConfirm: sinon.spy(), onCancel: sinon.spy(), }); @@ -25,6 +26,7 @@ module('Integration | Component | two step button', function(hooks) { confirmText=confirmText confirmationMessage=confirmationMessage awaitingConfirmation=awaitingConfirmation + disabled=disabled onConfirm=onConfirm onCancel=onCancel}} `; @@ -135,4 +137,68 @@ module('Integration | Component | two step button', function(hooks) { ); }); }); + + test('when in the prompt state, clicking outside will reset state back to idle', async function(assert) { + const props = commonProperties(); + this.setProperties(props); + await render(commonTemplate); + + click('[data-test-idle-button]'); + await settled(); + + assert.ok(find('[data-test-cancel-button]'), 'In the prompt state'); + + click(document.body); + await settled(); + + assert.ok(find('[data-test-idle-button]'), 'Back in the idle state'); + }); + + test('when in the prompt state, clicking inside will not reset state back to idle', async function(assert) { + const props = commonProperties(); + this.setProperties(props); + await render(commonTemplate); + + click('[data-test-idle-button]'); + await settled(); + + assert.ok(find('[data-test-cancel-button]'), 'In the prompt state'); + + click('[data-test-confirmation-message]'); + await settled(); + + assert.notOk(find('[data-test-idle-button]'), 'Still in the prompt state'); + }); + + test('when awaitingConfirmation is true, clicking outside does nothing', async function(assert) { + const props = commonProperties(); + props.awaitingConfirmation = true; + this.setProperties(props); + await render(commonTemplate); + + click('[data-test-idle-button]'); + await settled(); + + assert.ok(find('[data-test-cancel-button]'), 'In the prompt state'); + + click(document.body); + await settled(); + + assert.notOk(find('[data-test-idle-button]'), 'Still in the prompt state'); + }); + + test('when disabled is true, the idle button is disabled', async function(assert) { + const props = commonProperties(); + props.disabled = true; + this.setProperties(props); + await render(commonTemplate); + + assert.ok( + find('[data-test-idle-button]').hasAttribute('disabled'), + 'The idle button is disabled' + ); + + click('[data-test-idle-button]'); + assert.ok(find('[data-test-idle-button]'), 'Still in the idle state after clicking'); + }); }); From b33ff625dbc4a4d1d8bedd9a0c4eab50a78a15dd Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 16 May 2019 17:42:36 -0700 Subject: [PATCH 03/11] New with-headroom modifier for titles --- ui/app/styles/core/title.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/app/styles/core/title.scss b/ui/app/styles/core/title.scss index f6464e8e8d70..40de40ed8bcd 100644 --- a/ui/app/styles/core/title.scss +++ b/ui/app/styles/core/title.scss @@ -4,4 +4,8 @@ &.is-6 { margin-bottom: 0.5rem; } + + &.with-headroom { + margin-top: 1rem; + } } From 4038434603eaa5c242938e7fcc0642b286de2018 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 16 May 2019 17:43:00 -0700 Subject: [PATCH 04/11] Allocation methods for stopping and restarting --- ui/app/adapters/allocation.js | 21 ++++++++++++++++++++- ui/app/models/allocation.js | 8 ++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/ui/app/adapters/allocation.js b/ui/app/adapters/allocation.js index 8aa4b662fd5b..dd26ef5612c5 100644 --- a/ui/app/adapters/allocation.js +++ b/ui/app/adapters/allocation.js @@ -1,3 +1,22 @@ import Watchable from './watchable'; +import addToPath from 'nomad-ui/utils/add-to-path'; -export default Watchable.extend(); +export default Watchable.extend({ + stop: adapterAction('/stop'), + restart: adapterAction((adapter, allocation) => { + const prefix = `${adapter.host || '/'}${adapter.urlPrefix()}`; + return `${prefix}/client/allocation/${allocation.id}/restart`; + }, 'PUT'), +}); + +function adapterAction(path, verb = 'POST') { + return function(allocation) { + let url; + if (typeof path === 'function') { + url = path(this, allocation); + } else { + url = addToPath(this.urlForFindRecord(allocation.id, 'allocation'), path); + } + return this.ajax(url, verb); + }; +} diff --git a/ui/app/models/allocation.js b/ui/app/models/allocation.js index f93f19175ce3..2a8239bcfb20 100644 --- a/ui/app/models/allocation.js +++ b/ui/app/models/allocation.js @@ -99,4 +99,12 @@ export default Model.extend({ ); } ), + + stop() { + return this.store.adapterFor('allocation').stop(this); + }, + + restart() { + return this.store.adapterFor('allocation').restart(this); + }, }); From 64b4bf654645ed410c33231fb46f2192f13be92a Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 16 May 2019 17:43:12 -0700 Subject: [PATCH 05/11] Add stop and restart buttons to the allocation index page --- .../allocations/allocation/index.js | 36 +++++++++++++++++++ .../allocations/allocation/index.hbs | 36 ++++++++++++++++++- 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/ui/app/controllers/allocations/allocation/index.js b/ui/app/controllers/allocations/allocation/index.js index 7ccfd9b91b71..662aac437830 100644 --- a/ui/app/controllers/allocations/allocation/index.js +++ b/ui/app/controllers/allocations/allocation/index.js @@ -1,6 +1,8 @@ import Controller from '@ember/controller'; import { inject as service } from '@ember/service'; +import { computed } from '@ember/object'; import { alias } from '@ember/object/computed'; +import { task } from 'ember-concurrency'; import Sortable from 'nomad-ui/mixins/sortable'; import { lazyClick } from 'nomad-ui/helpers/lazy-click'; @@ -21,6 +23,40 @@ export default Controller.extend(Sortable, { // Set in the route preempter: null, + error: computed(() => { + // { title, description } + return null; + }), + + onDismiss() { + this.set('error', null); + }, + + stopAllocation: task(function*() { + try { + yield this.model.stop(); + // Eagerly update the allocation clientStatus to avoid flickering + this.model.set('clientStatus', 'complete'); + } catch (err) { + this.set('error', { + title: 'Could Not Stop Allocation', + description: 'Your ACL token does not grant allocation lifecyle permissions.', + }); + } + }), + + restartAllocation: task(function*() { + try { + yield this.model.restart(); + } catch (err) { + console.log('oops', err); + this.set('error', { + title: 'Could Not Stop Allocation', + description: 'Your ACL token does not grant allocation lifecyle permissions.', + }); + } + }), + actions: { gotoTask(allocation, task) { this.transitionToRoute('allocations.allocation.task', task); diff --git a/ui/app/templates/allocations/allocation/index.hbs b/ui/app/templates/allocations/allocation/index.hbs index 09df050e2c4f..921213bc060d 100644 --- a/ui/app/templates/allocations/allocation/index.hbs +++ b/ui/app/templates/allocations/allocation/index.hbs @@ -1,8 +1,42 @@
-

+ {{#if error}} +
+
+
+

{{error.title}}

+

{{error.description}}

+
+
+ +
+
+
+ {{/if}} + +

Allocation {{model.name}} {{model.clientStatus}} {{model.id}} + {{#if model.isRunning}} + {{two-step-button + data-test-stop + idleText="Stop" + cancelText="Cancel" + confirmText="Yes, Stop" + confirmationMessage="Are you sure? This will reschedule the allocation on a different client." + awaitingConfirmation=stopAllocation.isRunning + disabled=(or stopAllocation.isRunning restartAllocation.isRunning) + onConfirm=(perform stopAllocation)}} + {{two-step-button + data-test-restart + idleText="Restart" + cancelText="Cancel" + confirmText="Yes, Restart" + confirmationMessage="Are you sure? This will restart the allocation in-place." + awaitingConfirmation=restartAllocation.isRunning + disabled=(or stopAllocation.isRunning restartAllocation.isRunning) + onConfirm=(perform restartAllocation)}} + {{/if}}

From 69c6c34afac83fc5cd6ce0808ff965dc560e691b Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 20 May 2019 15:27:20 -0700 Subject: [PATCH 06/11] Support task restarting in the allocation adapter --- ui/app/adapters/allocation.js | 19 +++--- ui/app/models/allocation.js | 4 +- ui/app/models/task-state.js | 4 ++ ui/mirage/config.js | 8 +++ ui/tests/unit/adapters/allocation-test.js | 78 +++++++++++++++++++++++ 5 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 ui/tests/unit/adapters/allocation-test.js diff --git a/ui/app/adapters/allocation.js b/ui/app/adapters/allocation.js index dd26ef5612c5..a2c24a5fff19 100644 --- a/ui/app/adapters/allocation.js +++ b/ui/app/adapters/allocation.js @@ -3,20 +3,19 @@ import addToPath from 'nomad-ui/utils/add-to-path'; export default Watchable.extend({ stop: adapterAction('/stop'), - restart: adapterAction((adapter, allocation) => { - const prefix = `${adapter.host || '/'}${adapter.urlPrefix()}`; - return `${prefix}/client/allocation/${allocation.id}/restart`; - }, 'PUT'), + + restart(allocation, taskName) { + const prefix = `${this.host || '/'}${this.urlPrefix()}`; + const url = `${prefix}/client/allocation/${allocation.id}/restart`; + return this.ajax(url, 'PUT', { + data: taskName && { TaskName: taskName }, + }); + }, }); function adapterAction(path, verb = 'POST') { return function(allocation) { - let url; - if (typeof path === 'function') { - url = path(this, allocation); - } else { - url = addToPath(this.urlForFindRecord(allocation.id, 'allocation'), path); - } + const url = addToPath(this.urlForFindRecord(allocation.id, 'allocation'), path); return this.ajax(url, verb); }; } diff --git a/ui/app/models/allocation.js b/ui/app/models/allocation.js index 2a8239bcfb20..258d1a55879f 100644 --- a/ui/app/models/allocation.js +++ b/ui/app/models/allocation.js @@ -104,7 +104,7 @@ export default Model.extend({ return this.store.adapterFor('allocation').stop(this); }, - restart() { - return this.store.adapterFor('allocation').restart(this); + restart(taskName) { + return this.store.adapterFor('allocation').restart(this, taskName); }, }); diff --git a/ui/app/models/task-state.js b/ui/app/models/task-state.js index 9ee5f229683a..66ef8fbe3031 100644 --- a/ui/app/models/task-state.js +++ b/ui/app/models/task-state.js @@ -43,4 +43,8 @@ export default Fragment.extend({ return classMap[this.state] || 'is-dark'; }), + + restart() { + return this.allocation.restart(this.name); + }, }); diff --git a/ui/mirage/config.js b/ui/mirage/config.js index 5dbd947f201d..24ddf66618fd 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -200,6 +200,10 @@ export default function() { this.get('/allocation/:id'); + this.post('/allocation/:id/stop', function() { + return new Response(204, {}, ''); + }); + this.get('/namespaces', function({ namespaces }) { const records = namespaces.all(); @@ -301,6 +305,10 @@ export default function() { }; // Client requests are available on the server and the client + this.put('/client/allocation/:id/restart', function() { + return new Response(204, {}, ''); + }); + this.get('/client/allocation/:id/stats', clientAllocationStatsHandler); this.get('/client/fs/logs/:allocation_id', clientAllocationLog); diff --git a/ui/tests/unit/adapters/allocation-test.js b/ui/tests/unit/adapters/allocation-test.js new file mode 100644 index 000000000000..58c5fea3f4c6 --- /dev/null +++ b/ui/tests/unit/adapters/allocation-test.js @@ -0,0 +1,78 @@ +import { settled } from '@ember/test-helpers'; +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; +import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage'; + +module('Unit | Adapter | Allocation', function(hooks) { + setupTest(hooks); + + hooks.beforeEach(async function() { + this.store = this.owner.lookup('service:store'); + this.subject = () => this.store.adapterFor('allocation'); + + this.server = startMirage(); + + this.server.create('namespace'); + this.server.create('node'); + this.server.create('job', { createAllocations: false }); + this.server.create('allocation', { id: 'alloc-1' }); + }); + + hooks.afterEach(function() { + this.server.shutdown(); + }); + + test('`stop` makes the correct API call', async function(assert) { + const { pretender } = this.server; + const allocationId = 'alloc-1'; + + const allocation = await this.store.findRecord('allocation', allocationId); + pretender.handledRequests.length = 0; + + await this.subject().stop(allocation); + const req = pretender.handledRequests[0]; + assert.equal( + `${req.method} ${req.url}`, + `POST /v1/allocation/${allocationId}/stop`, + `POST /v1/allocation/${allocationId}/stop` + ); + }); + + test('`restart` makes the correct API call', async function(assert) { + const { pretender } = this.server; + const allocationId = 'alloc-1'; + + const allocation = await this.store.findRecord('allocation', allocationId); + pretender.handledRequests.length = 0; + + await this.subject().restart(allocation); + const req = pretender.handledRequests[0]; + assert.equal( + `${req.method} ${req.url}`, + `PUT /v1/client/allocation/${allocationId}/restart`, + `PUT /v1/client/allocation/${allocationId}/restart` + ); + }); + + test('`restart` takes an optional task name and makes the correct API call', async function(assert) { + const { pretender } = this.server; + const allocationId = 'alloc-1'; + const taskName = 'task-name'; + + const allocation = await this.store.findRecord('allocation', allocationId); + pretender.handledRequests.length = 0; + + await this.subject().restart(allocation, taskName); + const req = pretender.handledRequests[0]; + assert.equal( + `${req.method} ${req.url}`, + `PUT /v1/client/allocation/${allocationId}/restart`, + `PUT /v1/client/allocation/${allocationId}/restart` + ); + assert.deepEqual( + JSON.parse(req.requestBody), + { TaskName: taskName }, + 'Request body is correct' + ); + }); +}); From 3a8334e720fc788050a86306c2455128c8536656 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 20 May 2019 15:35:21 -0700 Subject: [PATCH 07/11] Watch the next allocation on the allocation index page --- .../controllers/allocations/allocation/index.js | 17 ++++++++++++++--- ui/app/routes/allocations/allocation/index.js | 6 ++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/ui/app/controllers/allocations/allocation/index.js b/ui/app/controllers/allocations/allocation/index.js index 662aac437830..e2eaf5223737 100644 --- a/ui/app/controllers/allocations/allocation/index.js +++ b/ui/app/controllers/allocations/allocation/index.js @@ -1,10 +1,11 @@ import Controller from '@ember/controller'; import { inject as service } from '@ember/service'; -import { computed } from '@ember/object'; +import { computed, observer } from '@ember/object'; import { alias } from '@ember/object/computed'; import { task } from 'ember-concurrency'; import Sortable from 'nomad-ui/mixins/sortable'; import { lazyClick } from 'nomad-ui/helpers/lazy-click'; +import { watchRecord } from 'nomad-ui/utils/properties/watch'; export default Controller.extend(Sortable, { token: service(), @@ -32,6 +33,17 @@ export default Controller.extend(Sortable, { this.set('error', null); }, + watchNext: watchRecord('allocation'), + + observeWatchNext: observer('model.nextAllocation.clientStatus', function() { + const nextAllocation = this.model.nextAllocation; + if (nextAllocation && nextAllocation.content) { + this.watchNext.perform(nextAllocation); + } else { + this.watchNext.cancelAll(); + } + }), + stopAllocation: task(function*() { try { yield this.model.stop(); @@ -49,9 +61,8 @@ export default Controller.extend(Sortable, { try { yield this.model.restart(); } catch (err) { - console.log('oops', err); this.set('error', { - title: 'Could Not Stop Allocation', + title: 'Could Not Restart Allocation', description: 'Your ACL token does not grant allocation lifecyle permissions.', }); } diff --git a/ui/app/routes/allocations/allocation/index.js b/ui/app/routes/allocations/allocation/index.js index ebe985da28f6..323af0e82b2a 100644 --- a/ui/app/routes/allocations/allocation/index.js +++ b/ui/app/routes/allocations/allocation/index.js @@ -10,4 +10,10 @@ export default Route.extend({ return this._super(...arguments); }, + + resetController(controller, isExiting) { + if (isExiting) { + controller.watchNext.cancelAll(); + } + }, }); From 01a3450c0851e0b20ce958853e893de0946af237 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 20 May 2019 15:35:59 -0700 Subject: [PATCH 08/11] Don't cancel watchers when transitioning to a sub-route --- ui/app/mixins/with-watchers.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ui/app/mixins/with-watchers.js b/ui/app/mixins/with-watchers.js index d96992b74f37..af964dfcabce 100644 --- a/ui/app/mixins/with-watchers.js +++ b/ui/app/mixins/with-watchers.js @@ -31,8 +31,11 @@ export default Mixin.create(WithVisibilityDetection, { }, actions: { - willTransition() { - this.cancelAllWatchers(); + willTransition(transition) { + // Don't cancel watchers if transitioning into a sub-route + if (!transition.intent.name.startsWith(this.routeName)) { + this.cancelAllWatchers(); + } // Bubble the action up to the application route return true; From a301e0f079116ee092bd7e584163c711bf1e19b5 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 20 May 2019 15:36:15 -0700 Subject: [PATCH 09/11] Restart a single task from the task detail page --- .../allocations/allocation/task/index.js | 22 ++++++++++++++++ .../allocations/allocation/task/index.hbs | 25 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/ui/app/controllers/allocations/allocation/task/index.js b/ui/app/controllers/allocations/allocation/task/index.js index c020bc4a2b61..186f6b258f87 100644 --- a/ui/app/controllers/allocations/allocation/task/index.js +++ b/ui/app/controllers/allocations/allocation/task/index.js @@ -1,6 +1,7 @@ import Controller from '@ember/controller'; import { computed } from '@ember/object'; import { alias } from '@ember/object/computed'; +import { task } from 'ember-concurrency'; export default Controller.extend({ network: alias('model.resources.networks.firstObject'), @@ -20,4 +21,25 @@ export default Controller.extend({ ) .sortBy('name'); }), + + error: computed(() => { + // { title, description } + return null; + }), + + onDismiss() { + this.set('error', null); + }, + + restartTask: task(function*() { + try { + yield this.model.restart(); + } catch (err) { + console.log('OOPs', err); + this.set('error', { + title: 'Could Not Restart Task', + description: 'Your ACL token does not grant allocation lifecyle permissions.', + }); + } + }), }); diff --git a/ui/app/templates/allocations/allocation/task/index.hbs b/ui/app/templates/allocations/allocation/task/index.hbs index 518e8e58dc45..9409934ff864 100644 --- a/ui/app/templates/allocations/allocation/task/index.hbs +++ b/ui/app/templates/allocations/allocation/task/index.hbs @@ -1,8 +1,33 @@ {{partial "allocations/allocation/task/subnav"}}
+ {{#if error}} +
+
+
+

{{error.title}}

+

{{error.description}}

+
+
+ +
+
+
+ {{/if}} +

{{model.name}} {{model.state}} + {{#if model.isRunning}} + {{two-step-button + data-test-restart + idleText="Restart" + cancelText="Cancel" + confirmText="Yes, Restart" + confirmationMessage="Are you sure? This will restart the task in-place." + awaitingConfirmation=restartTask.isRunning + disabled=restartTask.isRunning + onConfirm=(perform restartTask)}} + {{/if}}

From d33f4bfd49b8494d007205916231c842de6db831 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 20 May 2019 17:19:24 -0700 Subject: [PATCH 10/11] Acceptance testing for allocation lifecycle --- .../allocations/allocation/index.js | 4 +- .../allocations/allocation/task/index.js | 3 +- ui/app/mixins/with-watchers.js | 2 +- .../allocations/allocation/index.hbs | 8 +-- .../allocations/allocation/task/index.hbs | 8 +-- ui/tests/acceptance/allocation-detail-test.js | 58 +++++++++++++++++++ ui/tests/acceptance/task-detail-test.js | 36 ++++++++++++ ui/tests/pages/allocations/detail.js | 11 ++++ ui/tests/pages/allocations/task/detail.js | 11 ++++ ui/tests/pages/components/two-step-button.js | 14 +++++ ui/tests/unit/adapters/allocation-test.js | 1 - 11 files changed, 142 insertions(+), 14 deletions(-) create mode 100644 ui/tests/pages/components/two-step-button.js diff --git a/ui/app/controllers/allocations/allocation/index.js b/ui/app/controllers/allocations/allocation/index.js index e2eaf5223737..127718221e21 100644 --- a/ui/app/controllers/allocations/allocation/index.js +++ b/ui/app/controllers/allocations/allocation/index.js @@ -52,7 +52,7 @@ export default Controller.extend(Sortable, { } catch (err) { this.set('error', { title: 'Could Not Stop Allocation', - description: 'Your ACL token does not grant allocation lifecyle permissions.', + description: 'Your ACL token does not grant allocation lifecycle permissions.', }); } }), @@ -63,7 +63,7 @@ export default Controller.extend(Sortable, { } catch (err) { this.set('error', { title: 'Could Not Restart Allocation', - description: 'Your ACL token does not grant allocation lifecyle permissions.', + description: 'Your ACL token does not grant allocation lifecycle permissions.', }); } }), diff --git a/ui/app/controllers/allocations/allocation/task/index.js b/ui/app/controllers/allocations/allocation/task/index.js index 186f6b258f87..ce1d3f847f10 100644 --- a/ui/app/controllers/allocations/allocation/task/index.js +++ b/ui/app/controllers/allocations/allocation/task/index.js @@ -35,10 +35,9 @@ export default Controller.extend({ try { yield this.model.restart(); } catch (err) { - console.log('OOPs', err); this.set('error', { title: 'Could Not Restart Task', - description: 'Your ACL token does not grant allocation lifecyle permissions.', + description: 'Your ACL token does not grant allocation lifecycle permissions.', }); } }), diff --git a/ui/app/mixins/with-watchers.js b/ui/app/mixins/with-watchers.js index af964dfcabce..ce89ec526dfa 100644 --- a/ui/app/mixins/with-watchers.js +++ b/ui/app/mixins/with-watchers.js @@ -33,7 +33,7 @@ export default Mixin.create(WithVisibilityDetection, { actions: { willTransition(transition) { // Don't cancel watchers if transitioning into a sub-route - if (!transition.intent.name.startsWith(this.routeName)) { + if (!transition.intent.name || !transition.intent.name.startsWith(this.routeName)) { this.cancelAllWatchers(); } diff --git a/ui/app/templates/allocations/allocation/index.hbs b/ui/app/templates/allocations/allocation/index.hbs index 921213bc060d..280744b08bba 100644 --- a/ui/app/templates/allocations/allocation/index.hbs +++ b/ui/app/templates/allocations/allocation/index.hbs @@ -1,13 +1,13 @@
{{#if error}} -
+
-

{{error.title}}

-

{{error.description}}

+

{{error.title}}

+

{{error.description}}

- +
diff --git a/ui/app/templates/allocations/allocation/task/index.hbs b/ui/app/templates/allocations/allocation/task/index.hbs index 9409934ff864..2e1d56fb0fb2 100644 --- a/ui/app/templates/allocations/allocation/task/index.hbs +++ b/ui/app/templates/allocations/allocation/task/index.hbs @@ -1,14 +1,14 @@ {{partial "allocations/allocation/task/subnav"}}
{{#if error}} -
+
-

{{error.title}}

-

{{error.description}}

+

{{error.title}}

+

{{error.description}}

- +
diff --git a/ui/tests/acceptance/allocation-detail-test.js b/ui/tests/acceptance/allocation-detail-test.js index 3e5ca598066a..66bc2b71b769 100644 --- a/ui/tests/acceptance/allocation-detail-test.js +++ b/ui/tests/acceptance/allocation-detail-test.js @@ -1,3 +1,4 @@ +import { run } from '@ember/runloop'; import { currentURL } from '@ember/test-helpers'; import { assign } from '@ember/polyfills'; import { module, test } from 'qunit'; @@ -155,6 +156,63 @@ module('Acceptance | allocation detail', function(hooks) { assert.ok(Allocation.error.isShown, 'Error message is shown'); assert.equal(Allocation.error.title, 'Not Found', 'Error message is for 404'); }); + + test('allocation can be stopped', async function(assert) { + await Allocation.stop.idle(); + await Allocation.stop.confirm(); + + assert.equal( + server.pretender.handledRequests.findBy('method', 'POST').url, + `/v1/allocation/${allocation.id}/stop`, + 'Stop request is made for the allocation' + ); + }); + + test('allocation can be restarted', async function(assert) { + await Allocation.restart.idle(); + await Allocation.restart.confirm(); + + assert.equal( + server.pretender.handledRequests.findBy('method', 'PUT').url, + `/v1/client/allocation/${allocation.id}/restart`, + 'Restart request is made for the allocation' + ); + }); + + test('while an allocation is being restarted, the stop button is disabled', async function(assert) { + server.pretender.post('/v1/allocation/:id/stop', () => [204, {}, ''], true); + + await Allocation.stop.idle(); + + run.later(() => { + assert.ok(Allocation.stop.isRunning, 'Stop is loading'); + assert.ok(Allocation.restart.isDisabled, 'Restart is disabled'); + server.pretender.resolve(server.pretender.requestReferences[0].request); + }, 500); + + await Allocation.stop.confirm(); + }); + + test('if stopping or restarting fails, an error message is shown', async function(assert) { + server.pretender.post('/v1/allocation/:id/stop', () => [403, {}, '']); + + await Allocation.stop.idle(); + await Allocation.stop.confirm(); + + assert.ok(Allocation.inlineError.isShown, 'Inline error is shown'); + assert.ok( + Allocation.inlineError.title.includes('Could Not Stop Allocation'), + 'Title is descriptive' + ); + assert.ok( + /ACL token.+?allocation lifecycle/.test(Allocation.inlineError.message), + 'Message mentions ACLs and the appropriate permission' + ); + + await Allocation.inlineError.dismiss(); + + assert.notOk(Allocation.inlineError.isShown, 'Inline error is no longer shown'); + }); }); module('Acceptance | allocation detail (rescheduled)', function(hooks) { diff --git a/ui/tests/acceptance/task-detail-test.js b/ui/tests/acceptance/task-detail-test.js index 211f2cea853d..a19ce0d89ddc 100644 --- a/ui/tests/acceptance/task-detail-test.js +++ b/ui/tests/acceptance/task-detail-test.js @@ -174,6 +174,42 @@ module('Acceptance | task detail', function(hooks) { assert.ok(Task.error.isPresent, 'Error message is shown'); assert.equal(Task.error.title, 'Not Found', 'Error message is for 404'); }); + + test('task can be restarted', async function(assert) { + await Task.restart.idle(); + await Task.restart.confirm(); + + const request = server.pretender.handledRequests.findBy('method', 'PUT'); + assert.equal( + request.url, + `/v1/client/allocation/${allocation.id}/restart`, + 'Restart request is made for the allocation' + ); + + assert.deepEqual( + JSON.parse(request.requestBody), + { TaskName: task.name }, + 'Restart request is made for the correct task' + ); + }); + + test('when task restart fails, an error message is shown', async function(assert) { + server.pretender.put('/v1/client/allocation/:id/restart', () => [403, {}, '']); + + await Task.restart.idle(); + await Task.restart.confirm(); + + assert.ok(Task.inlineError.isShown, 'Inline error is shown'); + assert.ok(Task.inlineError.title.includes('Could Not Restart Task'), 'Title is descriptive'); + assert.ok( + /ACL token.+?allocation lifecycle/.test(Task.inlineError.message), + 'Message mentions ACLs and the appropriate permission' + ); + + await Task.inlineError.dismiss(); + + assert.notOk(Task.inlineError.isShown, 'Inline error is no longer shown'); + }); }); module('Acceptance | task detail (no addresses)', function(hooks) { diff --git a/ui/tests/pages/allocations/detail.js b/ui/tests/pages/allocations/detail.js index f1e5348f2539..82362e84f5c1 100644 --- a/ui/tests/pages/allocations/detail.js +++ b/ui/tests/pages/allocations/detail.js @@ -9,12 +9,16 @@ import { } from 'ember-cli-page-object'; import allocations from 'nomad-ui/tests/pages/components/allocations'; +import twoStepButton from 'nomad-ui/tests/pages/components/two-step-button'; export default create({ visit: visitable('/allocations/:id'), title: text('[data-test-title]'), + stop: twoStepButton('[data-test-stop]'), + restart: twoStepButton('[data-test-restart]'), + details: { scope: '[data-test-allocation-details]', @@ -77,4 +81,11 @@ export default create({ message: text('[data-test-error-message]'), seekHelp: clickable('[data-test-error-message] a'), }, + + inlineError: { + isShown: isPresent('[data-test-inline-error]'), + title: text('[data-test-inline-error-title]'), + message: text('[data-test-inline-error-body]'), + dismiss: clickable('[data-test-inline-error-close]'), + }, }); diff --git a/ui/tests/pages/allocations/task/detail.js b/ui/tests/pages/allocations/task/detail.js index 9e5c0a60be19..a7bb60ec7366 100644 --- a/ui/tests/pages/allocations/task/detail.js +++ b/ui/tests/pages/allocations/task/detail.js @@ -8,6 +8,8 @@ import { visitable, } from 'ember-cli-page-object'; +import twoStepButton from 'nomad-ui/tests/pages/components/two-step-button'; + export default create({ visit: visitable('/allocations/:id/:name'), @@ -15,6 +17,8 @@ export default create({ state: text('[data-test-state]'), startedAt: text('[data-test-started-at]'), + restart: twoStepButton('[data-test-restart]'), + breadcrumbs: collection('[data-test-breadcrumb]', { id: attribute('data-test-breadcrumb'), text: text(), @@ -51,4 +55,11 @@ export default create({ message: text('[data-test-error-message]'), seekHelp: clickable('[data-test-error-message] a'), }, + + inlineError: { + isShown: isPresent('[data-test-inline-error]'), + title: text('[data-test-inline-error-title]'), + message: text('[data-test-inline-error-body]'), + dismiss: clickable('[data-test-inline-error-close]'), + }, }); diff --git a/ui/tests/pages/components/two-step-button.js b/ui/tests/pages/components/two-step-button.js new file mode 100644 index 000000000000..142718839d3a --- /dev/null +++ b/ui/tests/pages/components/two-step-button.js @@ -0,0 +1,14 @@ +import { attribute, clickable, hasClass, isPresent } from 'ember-cli-page-object'; + +export default scope => ({ + scope, + + isPresent: isPresent(), + + idle: clickable('[data-test-idle-button]'), + confirm: clickable('[data-test-confirm-button]'), + cancel: clickable('[data-test-cancel-button]'), + + isRunning: hasClass('is-loading', '[data-test-confirm-button]'), + isDisabled: attribute('disabled', '[data-test-idle-button]'), +}); diff --git a/ui/tests/unit/adapters/allocation-test.js b/ui/tests/unit/adapters/allocation-test.js index 58c5fea3f4c6..c176e84535d7 100644 --- a/ui/tests/unit/adapters/allocation-test.js +++ b/ui/tests/unit/adapters/allocation-test.js @@ -1,4 +1,3 @@ -import { settled } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage'; From 20028361c13adf5afb532263d8f69ba18a271f0c Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 May 2019 09:23:47 -0700 Subject: [PATCH 11/11] Use the TwoStepButton page object for integration testing too --- ui/tests/integration/two-step-button-test.js | 64 +++++++------------- ui/tests/pages/components/two-step-button.js | 10 ++- 2 files changed, 31 insertions(+), 43 deletions(-) diff --git a/ui/tests/integration/two-step-button-test.js b/ui/tests/integration/two-step-button-test.js index 0256e42e91cc..370eec266dff 100644 --- a/ui/tests/integration/two-step-button-test.js +++ b/ui/tests/integration/two-step-button-test.js @@ -4,6 +4,10 @@ import { setupRenderingTest } from 'ember-qunit'; import { render, settled } from '@ember/test-helpers'; import hbs from 'htmlbars-inline-precompile'; import sinon from 'sinon'; +import { create } from 'ember-cli-page-object'; +import twoStepButton from 'nomad-ui/tests/pages/components/two-step-button'; + +const TwoStepButton = create(twoStepButton()); module('Integration | Component | two step button', function(hooks) { setupRenderingTest(hooks); @@ -37,11 +41,7 @@ module('Integration | Component | two step button', function(hooks) { await render(commonTemplate); assert.ok(find('[data-test-idle-button]'), 'Idle button is rendered'); - assert.equal( - find('[data-test-idle-button]').textContent.trim(), - props.idleText, - 'Button is labeled correctly' - ); + assert.equal(TwoStepButton.idleText, props.idleText, 'Button is labeled correctly'); assert.notOk(find('[data-test-cancel-button]'), 'No cancel button yet'); assert.notOk(find('[data-test-confirm-button]'), 'No confirm button yet'); @@ -53,25 +53,17 @@ module('Integration | Component | two step button', function(hooks) { this.setProperties(props); await render(commonTemplate); - click('[data-test-idle-button]'); + TwoStepButton.idle(); return settled().then(() => { assert.ok(find('[data-test-cancel-button]'), 'Cancel button is rendered'); - assert.equal( - find('[data-test-cancel-button]').textContent.trim(), - props.cancelText, - 'Button is labeled correctly' - ); + assert.equal(TwoStepButton.cancelText, props.cancelText, 'Button is labeled correctly'); assert.ok(find('[data-test-confirm-button]'), 'Confirm button is rendered'); - assert.equal( - find('[data-test-confirm-button]').textContent.trim(), - props.confirmText, - 'Button is labeled correctly' - ); + assert.equal(TwoStepButton.confirmText, props.confirmText, 'Button is labeled correctly'); assert.equal( - find('[data-test-confirmation-message]').textContent.trim(), + TwoStepButton.confirmationMessage, props.confirmationMessage, 'Confirmation message is shown' ); @@ -85,10 +77,10 @@ module('Integration | Component | two step button', function(hooks) { this.setProperties(props); await render(commonTemplate); - click('[data-test-idle-button]'); + TwoStepButton.idle(); return settled().then(() => { - click('[data-test-cancel-button]'); + TwoStepButton.cancel(); return settled().then(() => { assert.ok(props.onCancel.calledOnce, 'The onCancel hook fired'); @@ -102,10 +94,10 @@ module('Integration | Component | two step button', function(hooks) { this.setProperties(props); await render(commonTemplate); - click('[data-test-idle-button]'); + TwoStepButton.idle(); return settled().then(() => { - click('[data-test-confirm-button]'); + TwoStepButton.confirm(); return settled().then(() => { assert.ok(props.onConfirm.calledOnce, 'The onConfirm hook fired'); @@ -120,21 +112,12 @@ module('Integration | Component | two step button', function(hooks) { this.setProperties(props); await render(commonTemplate); - click('[data-test-idle-button]'); + TwoStepButton.idle(); return settled().then(() => { - assert.ok( - find('[data-test-cancel-button]').hasAttribute('disabled'), - 'The cancel button is disabled' - ); - assert.ok( - find('[data-test-confirm-button]').hasAttribute('disabled'), - 'The confirm button is disabled' - ); - assert.ok( - find('[data-test-confirm-button]').classList.contains('is-loading'), - 'The confirm button is in a loading state' - ); + assert.ok(TwoStepButton.cancelIsDisabled, 'The cancel button is disabled'); + assert.ok(TwoStepButton.confirmIsDisabled, 'The confirm button is disabled'); + assert.ok(TwoStepButton.isRunning, 'The confirm button is in a loading state'); }); }); @@ -143,7 +126,7 @@ module('Integration | Component | two step button', function(hooks) { this.setProperties(props); await render(commonTemplate); - click('[data-test-idle-button]'); + TwoStepButton.idle(); await settled(); assert.ok(find('[data-test-cancel-button]'), 'In the prompt state'); @@ -159,7 +142,7 @@ module('Integration | Component | two step button', function(hooks) { this.setProperties(props); await render(commonTemplate); - click('[data-test-idle-button]'); + TwoStepButton.idle(); await settled(); assert.ok(find('[data-test-cancel-button]'), 'In the prompt state'); @@ -176,7 +159,7 @@ module('Integration | Component | two step button', function(hooks) { this.setProperties(props); await render(commonTemplate); - click('[data-test-idle-button]'); + TwoStepButton.idle(); await settled(); assert.ok(find('[data-test-cancel-button]'), 'In the prompt state'); @@ -193,12 +176,9 @@ module('Integration | Component | two step button', function(hooks) { this.setProperties(props); await render(commonTemplate); - assert.ok( - find('[data-test-idle-button]').hasAttribute('disabled'), - 'The idle button is disabled' - ); + assert.ok(TwoStepButton.isDisabled, 'The idle button is disabled'); - click('[data-test-idle-button]'); + TwoStepButton.idle(); assert.ok(find('[data-test-idle-button]'), 'Still in the idle state after clicking'); }); }); diff --git a/ui/tests/pages/components/two-step-button.js b/ui/tests/pages/components/two-step-button.js index 142718839d3a..1c787b9bb7bb 100644 --- a/ui/tests/pages/components/two-step-button.js +++ b/ui/tests/pages/components/two-step-button.js @@ -1,4 +1,4 @@ -import { attribute, clickable, hasClass, isPresent } from 'ember-cli-page-object'; +import { attribute, clickable, hasClass, isPresent, text } from 'ember-cli-page-object'; export default scope => ({ scope, @@ -11,4 +11,12 @@ export default scope => ({ isRunning: hasClass('is-loading', '[data-test-confirm-button]'), isDisabled: attribute('disabled', '[data-test-idle-button]'), + + cancelIsDisabled: attribute('disabled', '[data-test-cancel-button]'), + confirmIsDisabled: attribute('disabled', '[data-test-confirm-button]'), + + idleText: text('[data-test-idle-button]'), + cancelText: text('[data-test-cancel-button]'), + confirmText: text('[data-test-confirm-button]'), + confirmationMessage: text('[data-test-confirmation-message]'), });