From 751b6e2fd678b9a05953bdb608c422f6837e0512 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 14 Aug 2018 12:46:55 -0700 Subject: [PATCH 01/49] Enforce a min-height for the code editor component --- ui/app/styles/components/codemirror.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/app/styles/components/codemirror.scss b/ui/app/styles/components/codemirror.scss index 59f4d755b..16b54f23b 100644 --- a/ui/app/styles/components/codemirror.scss +++ b/ui/app/styles/components/codemirror.scss @@ -4,6 +4,10 @@ $dark-bright: lighten($dark, 15%); height: auto; } +.CodeMirror-scroll { + min-height: 500px; +} + .cm-s-hashi, .cm-s-hashi-read-only { &.CodeMirror { From 53f2ca31279470b78c4b01fbb033f83be8fcf56f Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 14 Aug 2018 12:47:28 -0700 Subject: [PATCH 02/49] New layout helper for associating two elements vertically By default, blocks have a margin of 1.5em to create a consistent vertical rhythm. However, sometimes elements need to be associated with the element above them. In this cases, the gap between elements needs to be tighter. There are many ways to do this, but this approach asks the latter content to be marked as associative. The implication is that the association is with the previous block. --- ui/app/styles/core.scss | 51 +++++++++++++++++---------------- ui/app/styles/utils/layout.scss | 3 ++ 2 files changed, 29 insertions(+), 25 deletions(-) create mode 100644 ui/app/styles/utils/layout.scss diff --git a/ui/app/styles/core.scss b/ui/app/styles/core.scss index e4391415b..a92c17ade 100644 --- a/ui/app/styles/core.scss +++ b/ui/app/styles/core.scss @@ -1,34 +1,35 @@ // Utils -@import "./utils/reset.scss"; -@import "./utils/z-indices"; -@import "./utils/product-colors"; -@import "./utils/bumper"; +@import './utils/reset.scss'; +@import './utils/z-indices'; +@import './utils/product-colors'; +@import './utils/bumper'; +@import './utils/layout'; // Start with Bulma variables as a foundation -@import "bulma/sass/utilities/initial-variables"; +@import 'bulma/sass/utilities/initial-variables'; // Override variables where appropriate -@import "./core/variables.scss"; +@import './core/variables.scss'; // Bring in the rest of Bulma -@import "bulma/bulma"; +@import 'bulma/bulma'; // Override Bulma details where appropriate -@import "./core/buttons"; -@import "./core/breadcrumb"; -@import "./core/columns"; -@import "./core/forms"; -@import "./core/icon"; -@import "./core/level"; -@import "./core/menu"; -@import "./core/message"; -@import "./core/navbar"; -@import "./core/notification"; -@import "./core/pagination"; -@import "./core/progress"; -@import "./core/section"; -@import "./core/table"; -@import "./core/tabs"; -@import "./core/tag"; -@import "./core/title"; -@import "./core/typography"; +@import './core/buttons'; +@import './core/breadcrumb'; +@import './core/columns'; +@import './core/forms'; +@import './core/icon'; +@import './core/level'; +@import './core/menu'; +@import './core/message'; +@import './core/navbar'; +@import './core/notification'; +@import './core/pagination'; +@import './core/progress'; +@import './core/section'; +@import './core/table'; +@import './core/tabs'; +@import './core/tag'; +@import './core/title'; +@import './core/typography'; diff --git a/ui/app/styles/utils/layout.scss b/ui/app/styles/utils/layout.scss new file mode 100644 index 000000000..6504a68cf --- /dev/null +++ b/ui/app/styles/utils/layout.scss @@ -0,0 +1,3 @@ +.is-associative { + margin-top: -0.75em; +} From 33956a69dae92ea6925c879ecf62f2b55a550b7c Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 14 Aug 2018 12:54:54 -0700 Subject: [PATCH 03/49] New job run page and navigation to get there. --- ui/app/router.js | 1 + ui/app/templates/jobs/index.hbs | 13 +++++++++---- ui/app/templates/jobs/run.hbs | 31 +++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 ui/app/templates/jobs/run.hbs diff --git a/ui/app/router.js b/ui/app/router.js index 39446ffa9..7a4015c28 100644 --- a/ui/app/router.js +++ b/ui/app/router.js @@ -8,6 +8,7 @@ const Router = EmberRouter.extend({ Router.map(function() { this.route('jobs', function() { + this.route('run'); this.route('job', { path: '/:job_name' }, function() { this.route('task-group', { path: '/:name' }); this.route('definition'); diff --git a/ui/app/templates/jobs/index.hbs b/ui/app/templates/jobs/index.hbs index a306f5106..8b792046d 100644 --- a/ui/app/templates/jobs/index.hbs +++ b/ui/app/templates/jobs/index.hbs @@ -2,11 +2,16 @@ {{#if isForbidden}} {{partial "partials/forbidden-message"}} {{else}} - {{#if filteredJobs.length}} -
-
{{search-box data-test-jobs-search searchTerm=(mut searchTerm) placeholder="Search jobs..."}}
+
+ {{#if filteredJobs.length}} +
+ {{search-box data-test-jobs-search searchTerm=(mut searchTerm) placeholder="Search jobs..."}} +
+ {{/if}} +
+ {{#link-to "jobs.run" class="button is-primary is-pulled-right"}}Run Job{{/link-to}}
- {{/if}} +
{{#list-pagination source=sortedJobs size=pageSize diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs new file mode 100644 index 000000000..d142248aa --- /dev/null +++ b/ui/app/templates/jobs/run.hbs @@ -0,0 +1,31 @@ +
+
+
+
+

Run a Job

+

Paste or author HCL or JSON to submit to your cluster. A plan will be requested before the job is submitted.

+
+
+ +
+
+
+
+
+ Job Definition +
+
+ {{ivy-codemirror + value=jobSpec + options=(hash + mode="javascript" + theme="hashi" + tabSize=2 + lineNumbers=true + )}} +
+
+
+ +
+
From 302401d45c241e6767fa1e22391414e2860040c7 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 14 Aug 2018 13:06:26 -0700 Subject: [PATCH 04/49] Add breadcrumb to the run job page --- ui/app/routes/jobs/run.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 ui/app/routes/jobs/run.js diff --git a/ui/app/routes/jobs/run.js b/ui/app/routes/jobs/run.js new file mode 100644 index 000000000..ac6c654b1 --- /dev/null +++ b/ui/app/routes/jobs/run.js @@ -0,0 +1,13 @@ +import Route from '@ember/routing/route'; +import { inject as service } from '@ember/service'; + +export default Route.extend({ + store: service(), + + breadcrumbs: [ + { + label: 'Run', + args: ['jobs.run'], + }, + ], +}); From da1e179704b41cdfa9e969058e3faa3bff3bbb28 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 14 Aug 2018 17:29:51 -0700 Subject: [PATCH 05/49] Parse and Plan API and UI workflows --- ui/app/adapters/job.js | 20 ++++++ ui/app/controllers/jobs/run.js | 36 +++++++++++ ui/app/models/job.js | 46 ++++++++++++++ ui/app/routes/jobs/run.js | 13 ++++ ui/app/styles/components/codemirror.scss | 2 +- ui/app/templates/jobs/run.hbs | 77 ++++++++++++++++-------- 6 files changed, 168 insertions(+), 26 deletions(-) create mode 100644 ui/app/controllers/jobs/run.js diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index e13047815..af8b05aaf 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -59,6 +59,26 @@ export default Watchable.extend({ const url = this.urlForFindRecord(job.get('id'), 'job'); return this.ajax(url, 'DELETE'); }, + + parse(spec) { + const url = addToPath(this.urlForFindAll('job'), '/parse'); + return this.ajax(url, 'POST', { + data: { + JobHCL: spec, + Canonicalize: true, + }, + }); + }, + + plan(job) { + const url = addToPath(this.urlForFindRecord(job.get('id'), 'job'), '/plan'); + return this.ajax(url, 'POST', { + data: { + Job: job.get('_newDefinitionJSON'), + Diff: true, + }, + }); + }, }); function associateNamespace(url, namespace) { diff --git a/ui/app/controllers/jobs/run.js b/ui/app/controllers/jobs/run.js new file mode 100644 index 000000000..916a0c8e2 --- /dev/null +++ b/ui/app/controllers/jobs/run.js @@ -0,0 +1,36 @@ +import Controller from '@ember/controller'; +import { computed } from '@ember/object'; +import { task } from 'ember-concurrency'; + +export default Controller.extend({ + stage: computed('planOutput', function() { + return this.get('planOutput') ? 'plan' : 'editor'; + }), + + plan: task(function*() { + this.cancel(); + + try { + yield this.get('model').parse(); + } catch (err) { + this.set('parseError', err); + } + + try { + const planOutput = yield this.get('model').plan(); + console.log('Heyo!', planOutput); + this.set('planOutput', planOutput); + } catch (err) { + this.set('planError', err); + console.log('Uhoh', err); + } + }).drop(), + + submit: task(function*() {}), + + cancel() { + this.set('planOutput', null); + this.set('planError', null); + this.set('parseError', null); + }, +}); diff --git a/ui/app/models/job.js b/ui/app/models/job.js index b69040eaa..5de6977ae 100644 --- a/ui/app/models/job.js +++ b/ui/app/models/job.js @@ -4,6 +4,8 @@ import Model from 'ember-data/model'; import attr from 'ember-data/attr'; import { belongsTo, hasMany } from 'ember-data/relationships'; import { fragmentArray } from 'ember-data-model-fragments/attributes'; +import RSVP from 'rsvp'; +import { assert } from '@ember/debug'; const JOB_TYPES = ['service', 'batch', 'system']; @@ -191,6 +193,41 @@ export default Model.extend({ return this.store.adapterFor('job').stop(this); }, + plan() { + assert('A job must be parsed before planned', this.get('_newDefinitionJSON')); + return this.store.adapterFor('job').plan(this); + }, + + parse() { + const definition = this.get('_newDefinition'); + let promise; + + try { + // If the definition is already JSON then it doesn't need to be parsed. + const json = JSON.parse(definition); + this.set('_newDefinitionJSON', definition); + this.setIDByPayload(json); + promise = RSVP.Resolve(definition); + } catch (err) { + // If the definition is invalid JSON, assume it is HCL. If it is invalid + // in anyway, the parse endpoint will throw an error. + promise = this.store + .adapterFor('job') + .parse(this.get('_newDefinition')) + .then(response => { + this.set('_newDefinitionJSON', response); + this.setIDByPayload(response); + }); + } + + return promise; + }, + + setIDByPayload(payload) { + this.set('plainId', payload.Name); + this.set('id', JSON.stringify([payload.Name, payload.Namespace || 'default'])); + }, + statusClass: computed('status', function() { const classMap = { pending: 'is-pending', @@ -206,4 +243,13 @@ export default Model.extend({ // Lazily decode the base64 encoded payload return window.atob(this.get('payload') || ''); }), + + // An arbitrary HCL or JSON string that is used by the serializer to plan + // and run this job. Used for both new job models and saved job models. + _newDefinition: attr('string'), + + // The new definition may be HCL, in which case the API will need to parse the + // spec first. In order to preserve both the original HCL and the parsed response + // that will be submitted to the create job endpoint, another prop is necessary. + _newDefinitionJSON: attr('string'), }); diff --git a/ui/app/routes/jobs/run.js b/ui/app/routes/jobs/run.js index ac6c654b1..121e9b4ce 100644 --- a/ui/app/routes/jobs/run.js +++ b/ui/app/routes/jobs/run.js @@ -3,6 +3,7 @@ import { inject as service } from '@ember/service'; export default Route.extend({ store: service(), + system: service(), breadcrumbs: [ { @@ -10,4 +11,16 @@ export default Route.extend({ args: ['jobs.run'], }, ], + + model() { + return this.get('store').createRecord('job', { + namespace: this.get('system.activeNamespace'), + }); + }, + + resetController(controller, isExiting) { + if (isExiting) { + controller.get('model').deleteRecord(); + } + }, }); diff --git a/ui/app/styles/components/codemirror.scss b/ui/app/styles/components/codemirror.scss index 16b54f23b..81c08356e 100644 --- a/ui/app/styles/components/codemirror.scss +++ b/ui/app/styles/components/codemirror.scss @@ -43,7 +43,7 @@ $dark-bright: lighten($dark, 15%); } span.cm-comment { - color: $grey-light; + color: $grey; } span.cm-string, diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index d142248aa..953ea855f 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -1,31 +1,58 @@
-
-
-
-

Run a Job

-

Paste or author HCL or JSON to submit to your cluster. A plan will be requested before the job is submitted.

-
-
- + {{#if (eq stage "editor")}} +
+
+
+

Run a Job

+

Paste or author HCL or JSON to submit to your cluster. A plan will be requested before the job is submitted.

+
+
+ +
-
-
-
- Job Definition +
+
+ Job Definition +
+
+ {{ivy-codemirror + value=(or model._newDefinition jobSpec) + valueUpdated=(action (mut model._newDefinition)) + options=(hash + mode="javascript" + theme="hashi" + tabSize=2 + lineNumbers=true + )}} +
-
- {{ivy-codemirror - value=jobSpec - options=(hash - mode="javascript" - theme="hashi" - tabSize=2 - lineNumbers=true - )}} +
+
-
-
- -
+ {{/if}} + + {{#if (eq stage "plan")}} +
+
+
+

Job Plan

+

This is the impact running this job will have on your cluster.

+
+
+ +
+
+
+
+
Job Plan
+
+ {{job-diff diff=planOutput.Diff}} +
+
+
+ + +
+ {{/if}}
From 27f4a59259891a5c76e878b8e5e391a689595a28 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 14 Aug 2018 17:37:15 -0700 Subject: [PATCH 06/49] Fix no allocations error message layout for the recent allocations component --- .../templates/components/job-page/parts/recent-allocations.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/templates/components/job-page/parts/recent-allocations.hbs b/ui/app/templates/components/job-page/parts/recent-allocations.hbs index 4a0c22965..062f06cc2 100644 --- a/ui/app/templates/components/job-page/parts/recent-allocations.hbs +++ b/ui/app/templates/components/job-page/parts/recent-allocations.hbs @@ -2,7 +2,7 @@
Recent Allocations
-
+
{{#if job.allocations.length}} {{#list-table source=sortedAllocations From 21da150b9308dc5cdee1acf92c90edb2c2a2b76c Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 14 Aug 2018 17:37:44 -0700 Subject: [PATCH 07/49] Remove unused solarized theme configuration --- ui/ember-cli-build.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/ember-cli-build.js b/ui/ember-cli-build.js index 447069309..87dfc3f0f 100644 --- a/ui/ember-cli-build.js +++ b/ui/ember-cli-build.js @@ -13,7 +13,6 @@ module.exports = function(defaults) { paths: ['public/images/icons'], }, codemirror: { - themes: ['solarized'], modes: ['javascript'], }, funnel: { From f2128872cec003e1c111f29e4ea4e1f056b0bc9b Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 14 Aug 2018 18:26:26 -0700 Subject: [PATCH 08/49] Run job UI and API workflows --- ui/app/adapters/job.js | 10 ++++++++++ ui/app/controllers/jobs/run.js | 18 +++++++++++++++--- ui/app/models/job.js | 17 +++++++++++++++-- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index af8b05aaf..2999988fa 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -79,6 +79,16 @@ export default Watchable.extend({ }, }); }, + + // Running a job doesn't follow REST create semantics so it's easier to + // treat it as an action. + run(job) { + return this.ajax(this.urlForCreateRecord('job'), 'POST', { + data: { + Job: job.get('_newDefinitionJSON'), + }, + }); + }, }); function associateNamespace(url, namespace) { diff --git a/ui/app/controllers/jobs/run.js b/ui/app/controllers/jobs/run.js index 916a0c8e2..871f7ed4f 100644 --- a/ui/app/controllers/jobs/run.js +++ b/ui/app/controllers/jobs/run.js @@ -1,6 +1,8 @@ import Controller from '@ember/controller'; import { computed } from '@ember/object'; import { task } from 'ember-concurrency'; +import { qpBuilder } from 'nomad-ui/utils/classes/query-params'; +import { next } from '@ember/runloop'; export default Controller.extend({ stage: computed('planOutput', function() { @@ -18,15 +20,25 @@ export default Controller.extend({ try { const planOutput = yield this.get('model').plan(); - console.log('Heyo!', planOutput); this.set('planOutput', planOutput); } catch (err) { this.set('planError', err); - console.log('Uhoh', err); } }).drop(), - submit: task(function*() {}), + submit: task(function*() { + try { + yield this.get('model').run(); + const id = this.get('model.plainId'); + const namespace = this.get('model.namespace.name') || 'default'; + // navigate to the new job page + this.transitionToRoute('jobs.job', id, { + queryParams: { jobNamespace: namespace }, + }); + } catch (err) { + this.set('runError', err); + } + }), cancel() { this.set('planOutput', null); diff --git a/ui/app/models/job.js b/ui/app/models/job.js index 5de6977ae..1ccbf305e 100644 --- a/ui/app/models/job.js +++ b/ui/app/models/job.js @@ -198,6 +198,11 @@ export default Model.extend({ return this.store.adapterFor('job').plan(this); }, + run() { + assert('A job must be parsed before ran', this.get('_newDefinitionJSON')); + return this.store.adapterFor('job').run(this); + }, + parse() { const definition = this.get('_newDefinition'); let promise; @@ -224,8 +229,16 @@ export default Model.extend({ }, setIDByPayload(payload) { - this.set('plainId', payload.Name); - this.set('id', JSON.stringify([payload.Name, payload.Namespace || 'default'])); + const namespace = payload.Namespace || 'default'; + const id = payload.Name; + + this.set('plainId', id); + this.set('id', JSON.stringify([id, namespace])); + + const namespaceRecord = this.store.peekRecord('namespace', namespace); + if (namespaceRecord) { + this.set('namespace', namespaceRecord); + } }, statusClass: computed('status', function() { From f29f4351f1ca7728ab3746e4487d520ddbb36004 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 15 Aug 2018 15:18:38 -0700 Subject: [PATCH 09/49] Error messages for job submit --- ui/app/controllers/jobs/run.js | 29 +++++++--- ui/app/templates/jobs/run.hbs | 61 +++++++++++++++------- ui/app/utils/message-from-adapter-error.js | 6 +++ ui/app/utils/properties/local-storage.js | 19 +++++++ 4 files changed, 89 insertions(+), 26 deletions(-) create mode 100644 ui/app/utils/message-from-adapter-error.js create mode 100644 ui/app/utils/properties/local-storage.js diff --git a/ui/app/controllers/jobs/run.js b/ui/app/controllers/jobs/run.js index 871f7ed4f..7e0d01f5e 100644 --- a/ui/app/controllers/jobs/run.js +++ b/ui/app/controllers/jobs/run.js @@ -1,46 +1,61 @@ import Controller from '@ember/controller'; import { computed } from '@ember/object'; import { task } from 'ember-concurrency'; -import { qpBuilder } from 'nomad-ui/utils/classes/query-params'; -import { next } from '@ember/runloop'; +import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; +import localStorageProperty from 'nomad-ui/utils/properties/local-storage'; export default Controller.extend({ + parseError: null, + planError: null, + runError: null, + + showPlanMessage: localStorageProperty('nomadMessageJobPlan', true), + showEditorMessage: localStorageProperty('nomadMessageJobEditor', true), + stage: computed('planOutput', function() { return this.get('planOutput') ? 'plan' : 'editor'; }), plan: task(function*() { - this.cancel(); + this.reset(); try { yield this.get('model').parse(); } catch (err) { - this.set('parseError', err); + const error = messageFromAdapterError(err) || 'Could not parse input'; + this.set('parseError', error); + return; } try { const planOutput = yield this.get('model').plan(); this.set('planOutput', planOutput); } catch (err) { - this.set('planError', err); + const error = messageFromAdapterError(err) || 'Could not plan job'; + this.set('planError', error); } }).drop(), submit: task(function*() { try { yield this.get('model').run(); + const id = this.get('model.plainId'); const namespace = this.get('model.namespace.name') || 'default'; + + this.reset(); + // navigate to the new job page this.transitionToRoute('jobs.job', id, { queryParams: { jobNamespace: namespace }, }); } catch (err) { - this.set('runError', err); + const error = messageFromAdapterError(err) || 'Could not submit job'; + this.set('runError', error); } }), - cancel() { + reset() { this.set('planOutput', null); this.set('planError', null); this.set('parseError', null); diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index 953ea855f..d27ee5f07 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -1,16 +1,37 @@
+ {{#if parseError}} +
+

Parse Error

+

{{parseError}}

+
+ {{/if}} + {{#if planError}} +
+

Plan Error

+

{{planError}}

+
+ {{/if}} + {{#if runError}} +
+

Run Error

+

{{runError}}

+
+ {{/if}} + {{#if (eq stage "editor")}} -
-
-
-

Run a Job

-

Paste or author HCL or JSON to submit to your cluster. A plan will be requested before the job is submitted.

-
-
- + {{#if showEditorMessage}} +
+
+
+

Run a Job

+

Paste or author HCL or JSON to submit to your cluster. A plan will be requested before the job is submitted.

+
+
+ +
-
+ {{/if}}
Job Definition @@ -33,17 +54,19 @@ {{/if}} {{#if (eq stage "plan")}} -
-
-
-

Job Plan

-

This is the impact running this job will have on your cluster.

-
-
- + {{#if showPlanMessage}} +
+
+
+

Job Plan

+

This is the impact running this job will have on your cluster.

+
+
+ +
-
+ {{/if}}
Job Plan
@@ -52,7 +75,7 @@
- +
{{/if}}
diff --git a/ui/app/utils/message-from-adapter-error.js b/ui/app/utils/message-from-adapter-error.js new file mode 100644 index 000000000..2b1ce864b --- /dev/null +++ b/ui/app/utils/message-from-adapter-error.js @@ -0,0 +1,6 @@ +// Returns a single string based on the response the adapter received +export default function messageFromAdapterError(error) { + if (error.errors) { + return error.errors.mapBy('detail').join('\n\n'); + } +} diff --git a/ui/app/utils/properties/local-storage.js b/ui/app/utils/properties/local-storage.js new file mode 100644 index 000000000..5049ed27c --- /dev/null +++ b/ui/app/utils/properties/local-storage.js @@ -0,0 +1,19 @@ +import { computed } from '@ember/object'; + +// An Ember.Computed property that persists set values in localStorage +// and will attempt to get its initial value from localStorage before +// falling back to a default. +// +// ex. showTutorial: localStorageProperty('nomadTutorial', true), +export default function localStorageProperty(localStorageKey, defaultValue) { + return computed({ + get() { + const persistedValue = window.localStorage.getItem(localStorageKey); + return persistedValue ? JSON.parse(persistedValue) : defaultValue; + }, + set(key, value) { + window.localStorage.setItem(localStorageKey, JSON.stringify(value)); + return value; + }, + }); +} From a9707411980851035fe4083f32057fe42315fbec Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 15 Aug 2018 16:58:54 -0700 Subject: [PATCH 10/49] Move the Diff property read out of the template --- ui/app/controllers/jobs/run.js | 4 +++- ui/app/templates/jobs/run.hbs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ui/app/controllers/jobs/run.js b/ui/app/controllers/jobs/run.js index 7e0d01f5e..2c1eca1fe 100644 --- a/ui/app/controllers/jobs/run.js +++ b/ui/app/controllers/jobs/run.js @@ -9,6 +9,8 @@ export default Controller.extend({ planError: null, runError: null, + planOutput: null, + showPlanMessage: localStorageProperty('nomadMessageJobPlan', true), showEditorMessage: localStorageProperty('nomadMessageJobEditor', true), @@ -29,7 +31,7 @@ export default Controller.extend({ try { const planOutput = yield this.get('model').plan(); - this.set('planOutput', planOutput); + this.set('planOutput', planOutput.Diff); } catch (err) { const error = messageFromAdapterError(err) || 'Could not plan job'; this.set('planError', error); diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index d27ee5f07..2d37df19c 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -70,7 +70,7 @@
Job Plan
- {{job-diff diff=planOutput.Diff}} + {{job-diff diff=planOutput}}
From 09e6432e3811d8b742ee8f0c6122f00aab16182a Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 15 Aug 2018 16:59:42 -0700 Subject: [PATCH 11/49] Support parse, plan, and run endpoints in mirage --- ui/mirage/config.js | 37 ++++++++++++++++++++++++++++-- ui/mirage/factories/job-version.js | 6 ++--- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/ui/mirage/config.js b/ui/mirage/config.js index 0311a8d94..ad236ae2f 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -2,6 +2,7 @@ import Ember from 'ember'; import Response from 'ember-cli-mirage/response'; import { HOSTS } from './common'; import { logFrames, logEncode } from './data/logs'; +import { generateDiff } from './factories/job-version'; const { copy } = Ember; @@ -55,6 +56,33 @@ export default function() { }) ); + this.post('/jobs', function({ jobs }, req) { + const body = JSON.parse(req.requestBody); + + if (!body.Job) return new Response(400, {}, 'Job is a required field on the request payload'); + + return okEmpty(); + }); + + this.post('/jobs/parse', function({ jobs }, req) { + const body = JSON.parse(req.requestBody); + + if (!body.JobHCL) + return new Response(400, {}, 'JobHCL is a required field on the request payload'); + if (!body.Canonicalize) return new Response(400, {}, 'Expected Canonicalize to be true'); + + return new Response(200, {}, this.serialize(jobs.first())); + }); + + this.post('/job/:id/plan', function({ jobs }, req) { + const body = JSON.parse(req.requestBody); + + if (!body.Job) return new Response(400, {}, 'Job is a required field on the request payload'); + if (!body.Diff) return new Response(400, {}, 'Expected Diff to be true'); + + return new Response(200, {}, JSON.stringify({ Diff: generateDiff(req.params.id) })); + }); + this.get( '/job/:id', withBlockingSupport(function({ jobs }, { params, queryParams }) { @@ -107,8 +135,7 @@ export default function() { createAllocations: parent.createAllocations, }); - // Return bogus, since the response is normally just eval information - return new Response(200, {}, '{}'); + return okEmpty(); }); this.delete('/job/:id', function(schema, { params }) { @@ -276,3 +303,9 @@ function filterKeys(object, ...keys) { return clone; } + +// An empty response but not a 204 No Content. This is still a valid JSON +// response that represents a payload with no worthwhile data. +function okEmpty() { + return new Response(200, {}, '{}'); +} diff --git a/ui/mirage/factories/job-version.js b/ui/mirage/factories/job-version.js index 3e1db492d..fe3404450 100644 --- a/ui/mirage/factories/job-version.js +++ b/ui/mirage/factories/job-version.js @@ -6,7 +6,7 @@ export default Factory.extend({ stable: faker.random.boolean, submitTime: () => faker.date.past(2 / 365, REF_TIME) * 1000000, diff() { - return generateDiff(this); + return generateDiff(this.jobId); }, jobId: null, @@ -39,10 +39,10 @@ export default Factory.extend({ }, }); -function generateDiff(version) { +export function generateDiff(id) { return { Fields: null, - ID: version.jobId, + ID: id, Objects: null, TaskGroups: [ { From 4b12c069f6227dc0c2ad9906fac73ddf5b6c291b Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 15 Aug 2018 17:00:08 -0700 Subject: [PATCH 12/49] Use the job name as the job id This has bit me more than once. It's best just to make Mirage consistent with the API even if it currently means indeterminate job ids --- ui/mirage/factories/job.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ui/mirage/factories/job.js b/ui/mirage/factories/job.js index e6380106e..096c66228 100644 --- a/ui/mirage/factories/job.js +++ b/ui/mirage/factories/job.js @@ -8,8 +8,12 @@ const JOB_TYPES = ['service', 'batch', 'system']; const JOB_STATUSES = ['pending', 'running', 'dead']; export default Factory.extend({ - id: i => `job-${i}`, - name: i => `${faker.list.random(...JOB_PREFIXES)()}-${faker.hacker.noun().dasherize()}-${i}`, + id: i => + `${faker.list.random(...JOB_PREFIXES)()}-${faker.hacker.noun().dasherize()}-${i}`.toLowerCase(), + + name() { + return this.id; + }, groupsCount: () => faker.random.number({ min: 1, max: 5 }), From 3b8b894ffd1fc31946fcbf51d224adac13d57b3d Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 15 Aug 2018 17:12:18 -0700 Subject: [PATCH 13/49] Acceptance test for the jobs list page --- ui/app/templates/jobs/index.hbs | 2 +- ui/tests/acceptance/jobs-list-test.js | 12 ++++++++++++ ui/tests/pages/jobs/list.js | 2 ++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/ui/app/templates/jobs/index.hbs b/ui/app/templates/jobs/index.hbs index 8b792046d..10a04f422 100644 --- a/ui/app/templates/jobs/index.hbs +++ b/ui/app/templates/jobs/index.hbs @@ -9,7 +9,7 @@
{{/if}}
- {{#link-to "jobs.run" class="button is-primary is-pulled-right"}}Run Job{{/link-to}} + {{#link-to "jobs.run" data-test-run-job class="button is-primary is-pulled-right"}}Run Job{{/link-to}}
{{#list-pagination diff --git a/ui/tests/acceptance/jobs-list-test.js b/ui/tests/acceptance/jobs-list-test.js index 0437a7d95..69c9eed71 100644 --- a/ui/tests/acceptance/jobs-list-test.js +++ b/ui/tests/acceptance/jobs-list-test.js @@ -69,6 +69,18 @@ test('each job row should link to the corresponding job', function(assert) { }); }); +test('the new job button transitions to the new job page', function(assert) { + JobsList.visit(); + + andThen(() => { + JobsList.runJob(); + }); + + andThen(() => { + assert.equal(currentURL(), '/jobs/run'); + }); +}); + test('when there are no jobs, there is an empty message', function(assert) { JobsList.visit(); diff --git a/ui/tests/pages/jobs/list.js b/ui/tests/pages/jobs/list.js index cd59ac7ef..252b92efc 100644 --- a/ui/tests/pages/jobs/list.js +++ b/ui/tests/pages/jobs/list.js @@ -16,6 +16,8 @@ export default create({ search: fillable('[data-test-jobs-search] input'), + runJob: clickable('[data-test-run-job]'), + jobs: collection('[data-test-job-row]', { id: attribute('data-test-job-row'), name: text('[data-test-job-name]'), From 875ba9971e11790e0f1b2ad41a63b33ac94047d9 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 16 Aug 2018 10:56:37 -0700 Subject: [PATCH 14/49] New test helper for getting the underlying CodeMirror instance from a dom selector --- ui/tests/.eslintrc.js | 1 + ui/tests/helpers/codemirror.js | 19 +++++++++++++++++++ ui/tests/helpers/start-app.js | 2 ++ 3 files changed, 22 insertions(+) create mode 100644 ui/tests/helpers/codemirror.js diff --git a/ui/tests/.eslintrc.js b/ui/tests/.eslintrc.js index 7cf1e81db..0d333ae8e 100644 --- a/ui/tests/.eslintrc.js +++ b/ui/tests/.eslintrc.js @@ -5,6 +5,7 @@ module.exports = { selectSearch: true, removeMultipleOption: true, clearSelected: true, + getCodeMirrorInstance: true, }, env: { embertest: true, diff --git a/ui/tests/helpers/codemirror.js b/ui/tests/helpers/codemirror.js new file mode 100644 index 000000000..87db0c814 --- /dev/null +++ b/ui/tests/helpers/codemirror.js @@ -0,0 +1,19 @@ +import { registerHelper } from '@ember/test'; + +const invariant = (truthy, error) => { + if (!truthy) throw new Error(error); +}; + +export default function registerCodeMirrorHelpers() { + registerHelper('getCodeMirrorInstance', function(app, selector) { + const cmService = app.__container__.lookup('service:code-mirror'); + + const element = document.querySelector(selector); + invariant(element, `Selector ${selector} matched no elements`); + + const cm = cmService.instanceFor(element.id); + invariant(cm, `No registered CodeMirror instance for ${selector}`); + + return cm; + }); +} diff --git a/ui/tests/helpers/start-app.js b/ui/tests/helpers/start-app.js index 304c6e377..496f7190a 100644 --- a/ui/tests/helpers/start-app.js +++ b/ui/tests/helpers/start-app.js @@ -3,8 +3,10 @@ import { merge } from '@ember/polyfills'; import Application from '../../app'; import config from '../../config/environment'; import registerPowerSelectHelpers from 'ember-power-select/test-support/helpers'; +import registerCodeMirrorHelpers from 'nomad-ui/tests/helpers/codemirror'; registerPowerSelectHelpers(); +registerCodeMirrorHelpers(); export default function startApp(attrs) { let attributes = merge({}, config.APP); From 3b5d96b234809e9ebe83ab5804c797d799a84ba1 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 16 Aug 2018 10:57:13 -0700 Subject: [PATCH 15/49] New PageObject helper for getting and setting CodeMirror values --- ui/tests/pages/helpers/codemirror.js | 32 ++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 ui/tests/pages/helpers/codemirror.js diff --git a/ui/tests/pages/helpers/codemirror.js b/ui/tests/pages/helpers/codemirror.js new file mode 100644 index 000000000..8a64b6e5b --- /dev/null +++ b/ui/tests/pages/helpers/codemirror.js @@ -0,0 +1,32 @@ +// Like fillable, but for the CodeMirror editor +// +// Usage: fillIn: codeFillable('[data-test-editor]') +// Page.fillIn(code); +export function codeFillable(selector) { + return { + isDescriptor: true, + + get() { + return function(code) { + const cm = getCodeMirrorInstance(selector); + cm.setValue(code); + return this; + }; + }, + }; +} + +// Like text, but for the CodeMirror editor +// +// Usage: content: code('[data-test-editor]') +// Page.code(); // some = [ 'string', 'of', 'code' ] +export function code(selector) { + return { + isDescriptor: true, + + get() { + const cm = getCodeMirrorInstance(selector); + return cm.getValue(); + }, + }; +} From c6fa7575d10b4578b61cd6e824bdc8257a1993ba Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 16 Aug 2018 10:57:56 -0700 Subject: [PATCH 16/49] New Page Object component for common error formatting --- ui/tests/pages/components/error.js | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 ui/tests/pages/components/error.js diff --git a/ui/tests/pages/components/error.js b/ui/tests/pages/components/error.js new file mode 100644 index 000000000..066ef08fc --- /dev/null +++ b/ui/tests/pages/components/error.js @@ -0,0 +1,11 @@ +import { clickable, isPresent, text } from 'ember-cli-page-object'; + +export default function(selectorBase = 'data-test-error') { + return { + scope: `[${selectorBase}]`, + isPresent: isPresent(), + title: text(`[${selectorBase}-title]`), + message: text(`[${selectorBase}-message]`), + seekHelp: clickable(`[${selectorBase}-message] a`), + }; +} From 635411f7502878865561597dca3154dd6c6a4def Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 16 Aug 2018 17:21:44 -0700 Subject: [PATCH 17/49] Rework job parse mirage request to get the job ID out of the payload --- ui/mirage/config.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ui/mirage/config.js b/ui/mirage/config.js index ad236ae2f..21e4c07e6 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -71,7 +71,17 @@ export default function() { return new Response(400, {}, 'JobHCL is a required field on the request payload'); if (!body.Canonicalize) return new Response(400, {}, 'Expected Canonicalize to be true'); - return new Response(200, {}, this.serialize(jobs.first())); + // Parse the name out of the first real line of HCL to match IDs in the new job record + // Regex expectation: + // in: job "job-name" { + // out: job-name + const nameFromHCLBlock = /.+?"(.+?)"/; + const jobName = body.JobHCL.trim() + .split('\n')[0] + .match(nameFromHCLBlock)[1]; + + const job = server.create('job', { id: jobName }); + return new Response(200, {}, this.serialize(job)); }); this.post('/job/:id/plan', function({ jobs }, req) { From a5d6790eba5555730f17e27fb8e62e3226d193af Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 16 Aug 2018 17:22:58 -0700 Subject: [PATCH 18/49] Acceptance tests for job run page --- ui/app/models/job.js | 2 +- ui/app/templates/jobs/run.hbs | 33 +-- ui/tests/acceptance/job-run-test.js | 315 ++++++++++++++++++++++++++++ ui/tests/pages/jobs/run.js | 38 ++++ 4 files changed, 371 insertions(+), 17 deletions(-) create mode 100644 ui/tests/acceptance/job-run-test.js create mode 100644 ui/tests/pages/jobs/run.js diff --git a/ui/app/models/job.js b/ui/app/models/job.js index 1ccbf305e..6bb5959c0 100644 --- a/ui/app/models/job.js +++ b/ui/app/models/job.js @@ -212,7 +212,7 @@ export default Model.extend({ const json = JSON.parse(definition); this.set('_newDefinitionJSON', definition); this.setIDByPayload(json); - promise = RSVP.Resolve(definition); + promise = RSVP.resolve(definition); } catch (err) { // If the definition is invalid JSON, assume it is HCL. If it is invalid // in anyway, the parse endpoint will throw an error. diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index 2d37df19c..172487f37 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -1,20 +1,20 @@
{{#if parseError}}
-

Parse Error

-

{{parseError}}

+

Parse Error

+

{{parseError}}

{{/if}} {{#if planError}}
-

Plan Error

-

{{planError}}

+

Plan Error

+

{{planError}}

{{/if}} {{#if runError}}
-

Run Error

-

{{runError}}

+

Run Error

+

{{runError}}

{{/if}} @@ -23,11 +23,11 @@
-

Run a Job

-

Paste or author HCL or JSON to submit to your cluster. A plan will be requested before the job is submitted.

+

Run a Job

+

Paste or author HCL or JSON to submit to your cluster. A plan will be requested before the job is submitted.

- +
@@ -38,6 +38,7 @@
{{ivy-codemirror + data-test-editor value=(or model._newDefinition jobSpec) valueUpdated=(action (mut model._newDefinition)) options=(hash @@ -49,7 +50,7 @@
- +
{{/if}} @@ -58,11 +59,11 @@
-

Job Plan

-

This is the impact running this job will have on your cluster.

+

Job Plan

+

This is the impact running this job will have on your cluster.

- +
@@ -70,12 +71,12 @@
Job Plan
- {{job-diff diff=planOutput}} + {{job-diff data-test-plan-output diff=planOutput}}
- - + +
{{/if}} diff --git a/ui/tests/acceptance/job-run-test.js b/ui/tests/acceptance/job-run-test.js new file mode 100644 index 000000000..e168f8ec1 --- /dev/null +++ b/ui/tests/acceptance/job-run-test.js @@ -0,0 +1,315 @@ +import { assign } from '@ember/polyfills'; +import { currentURL } from 'ember-native-dom-helpers'; +import { test } from 'qunit'; +import moduleForAcceptance from 'nomad-ui/tests/helpers/module-for-acceptance'; +import JobRun from 'nomad-ui/tests/pages/jobs/run'; + +const newJobName = 'new-job'; + +const jsonJob = overrides => { + return JSON.stringify( + assign( + {}, + { + Name: newJobName, + Namespace: 'default', + Datacenters: ['dc1'], + Priority: 50, + TaskGroups: { + redis: { + Tasks: { + redis: { + Driver: 'docker', + }, + }, + }, + }, + }, + overrides + ), + null, + 2 + ); +}; + +const hclJob = () => ` +job "${newJobName}" { + namespace = "default" + datacenters = ["dc1"] + + task "redis" { + driver = "docker" + } +} +`; + +moduleForAcceptance('Acceptance | job run', { + beforeEach() { + // Required for placing allocations (a result of creating jobs) + server.create('node'); + }, +}); + +test('visiting /jobs/run', function(assert) { + JobRun.visit(); + + andThen(() => { + assert.equal(currentURL(), '/jobs/run'); + }); +}); + +test('the page has an editor and an explanation popup', function(assert) { + JobRun.visit(); + + andThen(() => { + assert.ok(JobRun.editorHelp.isPresent, 'Editor explanation popup is present'); + assert.ok(JobRun.editor.isPresent, 'Editor is present'); + }); +}); + +test('the explanation popup can be dismissed', function(assert) { + JobRun.visit(); + + andThen(() => { + JobRun.editorHelp.dismiss(); + }); + + andThen(() => { + assert.notOk(JobRun.editorHelp.isPresent, 'Editor explanation popup is gone'); + assert.equal( + window.localStorage.nomadMessageJobEditor, + 'false', + 'Dismissal is persisted in localStorage' + ); + }); +}); + +test('the explanation popup is not shown once the dismissal state is set in localStorage', function(assert) { + window.localStorage.nomadMessageJobEditor = 'false'; + + JobRun.visit(); + + andThen(() => { + assert.notOk(JobRun.editorHelp.isPresent, 'Editor explanation popup is gone'); + }); +}); + +test('submitting a json job skips the parse endpoint', function(assert) { + const spec = jsonJob(); + + JobRun.visit(); + + andThen(() => { + JobRun.editor.fillIn(spec); + JobRun.plan(); + }); + + andThen(() => { + const requests = server.pretender.handledRequests.mapBy('url'); + assert.notOk(requests.includes('/v1/jobs/parse'), 'JSON job spec is not parsed'); + assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'JSON job spec is still planned'); + }); +}); + +test('submitting an hcl job requires the parse endpoint', function(assert) { + const spec = hclJob(); + + JobRun.visit(); + + andThen(() => { + JobRun.editor.fillIn(spec); + JobRun.plan(); + }); + + andThen(() => { + const requests = server.pretender.handledRequests.mapBy('url'); + assert.ok(requests.includes('/v1/jobs/parse'), 'HCL job spec is parsed first'); + assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'HCL job spec is planned'); + assert.ok( + requests.indexOf('/v1/jobs/parse') < requests.indexOf(`/v1/job/${newJobName}/plan`), + 'Parse comes before Plan' + ); + }); +}); + +test('when a job is successfully parsed and planned, the plan is shown to the user', function(assert) { + const spec = hclJob(); + + JobRun.visit(); + + andThen(() => { + JobRun.editor.fillIn(spec); + JobRun.plan(); + }); + + andThen(() => { + assert.ok(JobRun.planOutput, 'The plan is outputted'); + assert.notOk(JobRun.editor.isPresent, 'The editor is replaced with the plan output'); + assert.ok(JobRun.planHelp.isPresent, 'The plan explanation popup is shown'); + }); +}); + +test('from the plan screen, the cancel button goes back to the editor with the job still in tact', function(assert) { + const spec = hclJob(); + + JobRun.visit(); + + andThen(() => { + JobRun.editor.fillIn(spec); + JobRun.plan(); + }); + + andThen(() => { + JobRun.cancel(); + }); + + andThen(() => { + assert.ok(JobRun.editor.isPresent, 'The editor is shown again'); + assert.notOk(JobRun.planOutpu, 'The plan is gone'); + assert.equal(JobRun.editor.contents, spec, 'The spec that was planned is still in the editor'); + }); +}); + +test('from the plan screen, the submit button submits the job and redirects to the job overview page', function(assert) { + const spec = hclJob(); + + JobRun.visit(); + + andThen(() => { + JobRun.editor.fillIn(spec); + JobRun.plan(); + }); + + andThen(() => { + JobRun.run(); + }); + + andThen(() => { + assert.equal( + currentURL(), + `/jobs/${newJobName}`, + `Redirected to the job overview page for ${newJobName}` + ); + + const runRequest = server.pretender.handledRequests.find( + req => req.method === 'POST' && req.url === '/v1/jobs' + ); + const planRequest = server.pretender.handledRequests.find( + req => req.method === 'POST' && req.url === '/v1/jobs/parse' + ); + + assert.ok(runRequest, 'A POST request was made to run the new job'); + assert.deepEqual( + JSON.parse(runRequest.requestBody).Job, + JSON.parse(planRequest.responseText), + 'The Job payload parameter is equivalent to the result of the parse request' + ); + }); +}); + +test('when parse fails, the parse error message is shown', function(assert) { + const spec = hclJob(); + + const errorMessage = 'Parse Failed!! :o'; + server.pretender.post('/v1/jobs/parse', () => [400, {}, errorMessage]); + + JobRun.visit(); + + andThen(() => { + JobRun.editor.fillIn(spec); + JobRun.plan(); + }); + + andThen(() => { + assert.notOk(JobRun.planError.isPresent, 'Plan error is not shown'); + assert.notOk(JobRun.runError.isPresent, 'Run error is not shown'); + + assert.ok(JobRun.parseError.isPresent, 'Parse error is shown'); + assert.equal( + JobRun.parseError.message, + errorMessage, + 'The error message from the server is shown in the error in the UI' + ); + }); +}); + +test('when plan fails, the plan error message is shown', function(assert) { + const spec = hclJob(); + + const errorMessage = 'Parse Failed!! :o'; + server.pretender.post(`/v1/job/${newJobName}/plan`, () => [400, {}, errorMessage]); + + JobRun.visit(); + + andThen(() => { + JobRun.editor.fillIn(spec); + JobRun.plan(); + }); + + andThen(() => { + assert.notOk(JobRun.parseError.isPresent, 'Parse error is not shown'); + assert.notOk(JobRun.runError.isPresent, 'Run error is not shown'); + + assert.ok(JobRun.planError.isPresent, 'Plan error is shown'); + assert.equal( + JobRun.planError.message, + errorMessage, + 'The error message from the server is shown in the error in the UI' + ); + }); +}); + +test('when run fails, the run error message is shown', function(assert) { + const spec = hclJob(); + + const errorMessage = 'Parse Failed!! :o'; + server.pretender.post('/v1/jobs', () => [400, {}, errorMessage]); + + JobRun.visit(); + + andThen(() => { + JobRun.editor.fillIn(spec); + JobRun.plan(); + }); + + andThen(() => { + JobRun.run(); + }); + + andThen(() => { + assert.notOk(JobRun.planError.isPresent, 'Plan error is not shown'); + assert.notOk(JobRun.parseError.isPresent, 'Parse error is not shown'); + + assert.ok(JobRun.runError.isPresent, 'Run error is shown'); + assert.equal( + JobRun.runError.message, + errorMessage, + 'The error message from the server is shown in the error in the UI' + ); + }); +}); + +test('when submitting a job to a different namespace, the redirect to the job overview page takes namespace into account', function(assert) { + const newNamespace = 'second-namespace'; + + server.create('namespace', { id: newNamespace }); + const spec = jsonJob({ Namespace: newNamespace }); + + JobRun.visit(); + + andThen(() => { + JobRun.editor.fillIn(spec); + JobRun.plan(); + }); + + andThen(() => { + JobRun.run(); + }); + andThen(() => { + assert.equal( + currentURL(), + `/jobs/${newJobName}?namespace=${newNamespace}`, + `Redirected to the job overview page for ${newJobName} and switched the namespace to ${newNamespace}` + ); + }); +}); diff --git a/ui/tests/pages/jobs/run.js b/ui/tests/pages/jobs/run.js new file mode 100644 index 000000000..9192b03c1 --- /dev/null +++ b/ui/tests/pages/jobs/run.js @@ -0,0 +1,38 @@ +import { clickable, create, isPresent, text, visitable } from 'ember-cli-page-object'; +import { codeFillable, code } from 'nomad-ui/tests/pages/helpers/codemirror'; + +import error from 'nomad-ui/tests/pages/components/error'; + +export default create({ + visit: visitable('/jobs/run'), + + planError: error('data-test-plan-error'), + parseError: error('data-test-parse-error'), + runError: error('data-test-run-error'), + + plan: clickable('[data-test-plan]'), + cancel: clickable('[data-test-cancel]'), + run: clickable('[data-test-run]'), + + planOutput: text('[data-test-plan-output]'), + + planHelp: { + isPresent: isPresent('[data-test-plan-help-title]'), + title: text('[data-test-plan-help-title]'), + message: text('[data-test-plan-help-message]'), + dismiss: clickable('[data-test-plan-help-dismiss]'), + }, + + editorHelp: { + isPresent: isPresent('[data-test-editor-help-title]'), + title: text('[data-test-editor-help-title]'), + message: text('[data-test-editor-help-message]'), + dismiss: clickable('[data-test-editor-help-dismiss]'), + }, + + editor: { + isPresent: isPresent('[data-test-editor]'), + contents: code('[data-test-editor]'), + fillIn: codeFillable('[data-test-editor]'), + }, +}); From da8a6e4f2b2a71fc0badddce957dafe4eae880c1 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 17 Aug 2018 18:20:15 -0700 Subject: [PATCH 19/49] Spiff up the form buttons with type and disabled attributes --- ui/app/templates/jobs/run.hbs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index 172487f37..e6bd40573 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -50,7 +50,7 @@
- +
{{/if}} @@ -75,8 +75,8 @@
- - + +
{{/if}} From 1c94fdb1755c9db40e7adff8b79e235cb65570b6 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 17 Aug 2018 10:31:23 -0700 Subject: [PATCH 20/49] Don't use the verbose diff for job run plan --- ui/app/templates/components/job-diff.hbs | 6 +++--- ui/app/templates/jobs/run.hbs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ui/app/templates/components/job-diff.hbs b/ui/app/templates/components/job-diff.hbs index 543311238..d0908e6e4 100644 --- a/ui/app/templates/components/job-diff.hbs +++ b/ui/app/templates/components/job-diff.hbs @@ -80,10 +80,10 @@ Task: "{{task.Name}}" {{#if task.Annotations}} - ({{#each task.Annotations as |annotation index|}} + ({{~#each task.Annotations as |annotation index|}} {{annotation}} - {{#unless (eq index (dec annotations.length))}},{{/unless}} - {{/each}}) + {{#unless (eq index (dec task.Annotations.length))}},{{/unless}} + {{/each~}}) {{/if}} {{#if (or verbose (eq (lowercase task.Type "edited")))}} {{job-diff-fields-and-objects fields=task.Fields objects=task.Objects}} diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index e6bd40573..44b979ec2 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -71,7 +71,7 @@
Job Plan
- {{job-diff data-test-plan-output diff=planOutput}} + {{job-diff data-test-plan-output diff=planOutput verbose=false}}
From 9b7b465a66b1a5ba62f07e7275b2ad5b6c04db2b Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 17 Aug 2018 11:22:26 -0700 Subject: [PATCH 21/49] Show the scheduler dry-run output on the plan page --- ui/app/adapters/job.js | 3 +++ ui/app/components/placement-failure.js | 10 ++++++++++ ui/app/controllers/jobs/run.js | 8 ++++++-- ui/app/models/job-plan.js | 8 ++++++++ ui/app/serializers/job-plan.js | 12 ++++++++++++ ui/app/templates/components/placement-failure.hbs | 7 +++---- ui/app/templates/jobs/run.hbs | 14 +++++++++++++- ui/tests/integration/placement-failure-test.js | 1 + 8 files changed, 56 insertions(+), 7 deletions(-) create mode 100644 ui/app/components/placement-failure.js create mode 100644 ui/app/models/job-plan.js create mode 100644 ui/app/serializers/job-plan.js diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index 2999988fa..a8c09ce09 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -77,6 +77,9 @@ export default Watchable.extend({ Job: job.get('_newDefinitionJSON'), Diff: true, }, + }).then(json => { + json.ID = job.get('id'); + this.get('store').pushPayload('job-plan', { jobPlans: [json] }); }); }, diff --git a/ui/app/components/placement-failure.js b/ui/app/components/placement-failure.js new file mode 100644 index 000000000..b8052235f --- /dev/null +++ b/ui/app/components/placement-failure.js @@ -0,0 +1,10 @@ +import Component from '@ember/component'; +import { or } from '@ember/object/computed'; + +export default Component.extend({ + // Either provide a taskGroup or a failedTGAlloc + taskGroup: null, + failedTGAlloc: null, + + placementFailures: or('taskGroup.placementFailures', 'failedTGAlloc'), +}); diff --git a/ui/app/controllers/jobs/run.js b/ui/app/controllers/jobs/run.js index 2c1eca1fe..79525e460 100644 --- a/ui/app/controllers/jobs/run.js +++ b/ui/app/controllers/jobs/run.js @@ -1,10 +1,13 @@ import Controller from '@ember/controller'; +import { inject as service } from '@ember/service'; import { computed } from '@ember/object'; import { task } from 'ember-concurrency'; import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; import localStorageProperty from 'nomad-ui/utils/properties/local-storage'; export default Controller.extend({ + store: service(), + parseError: null, planError: null, runError: null, @@ -30,8 +33,9 @@ export default Controller.extend({ } try { - const planOutput = yield this.get('model').plan(); - this.set('planOutput', planOutput.Diff); + yield this.get('model').plan(); + const plan = this.get('store').peekRecord('job-plan', this.get('model.id')); + this.set('planOutput', plan); } catch (err) { const error = messageFromAdapterError(err) || 'Could not plan job'; this.set('planError', error); diff --git a/ui/app/models/job-plan.js b/ui/app/models/job-plan.js new file mode 100644 index 000000000..8f9c10345 --- /dev/null +++ b/ui/app/models/job-plan.js @@ -0,0 +1,8 @@ +import Model from 'ember-data/model'; +import attr from 'ember-data/attr'; +import { fragmentArray } from 'ember-data-model-fragments/attributes'; + +export default Model.extend({ + diff: attr(), + failedTGAllocs: fragmentArray('placement-failure', { defaultValue: () => [] }), +}); diff --git a/ui/app/serializers/job-plan.js b/ui/app/serializers/job-plan.js new file mode 100644 index 000000000..028073438 --- /dev/null +++ b/ui/app/serializers/job-plan.js @@ -0,0 +1,12 @@ +import { get } from '@ember/object'; +import { assign } from '@ember/polyfills'; +import ApplicationSerializer from './application'; + +export default ApplicationSerializer.extend({ + normalize(typeHash, hash) { + hash.FailedTGAllocs = Object.keys(hash.FailedTGAllocs || {}).map(key => { + return assign({ Name: key }, get(hash, `FailedTGAllocs.${key}`) || {}); + }); + return this._super(...arguments); + }, +}); diff --git a/ui/app/templates/components/placement-failure.hbs b/ui/app/templates/components/placement-failure.hbs index 63f49a826..9cf7089ec 100644 --- a/ui/app/templates/components/placement-failure.hbs +++ b/ui/app/templates/components/placement-failure.hbs @@ -1,7 +1,7 @@ -{{#if taskGroup.placementFailures}} - {{#with taskGroup.placementFailures as |failures|}} +{{#if placementFailures}} + {{#with placementFailures as |failures|}}

- {{taskGroup.name}} + {{placementFailures.name}} {{inc failures.coalescedFailures}} unplaced

    @@ -37,4 +37,3 @@
{{/with}} {{/if}} - diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index 44b979ec2..2e3856030 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -71,7 +71,19 @@
Job Plan
- {{job-diff data-test-plan-output diff=planOutput verbose=false}} + {{job-diff data-test-plan-output diff=planOutput.diff verbose=false}} +
+
+
+
Scheduler dry-run
+
+ {{#if planOutput.failedTGAllocs}} + {{#each planOutput.failedTGAllocs as |placementFailure|}} + {{placement-failure failedTGAlloc=placementFailure}} + {{/each}} + {{else}} + All tasks successfully allocated. + {{/if}}
diff --git a/ui/tests/integration/placement-failure-test.js b/ui/tests/integration/placement-failure-test.js index b5bf79800..97d8b688e 100644 --- a/ui/tests/integration/placement-failure-test.js +++ b/ui/tests/integration/placement-failure-test.js @@ -108,6 +108,7 @@ function createFixture(obj = {}, name = 'Placement Failure') { name: name, placementFailures: assign( { + name: name, coalescedFailures: 10, nodesEvaluated: 0, nodesAvailable: { From 223dae27a0614d9b2504cbb864b159dd472ea742 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 17 Aug 2018 18:13:09 -0700 Subject: [PATCH 22/49] Fixed bug that prevented non verbose job diffs from printing changed leaf nodes --- ui/app/templates/components/job-diff.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/templates/components/job-diff.hbs b/ui/app/templates/components/job-diff.hbs index d0908e6e4..e87a91e73 100644 --- a/ui/app/templates/components/job-diff.hbs +++ b/ui/app/templates/components/job-diff.hbs @@ -85,7 +85,7 @@ {{#unless (eq index (dec task.Annotations.length))}},{{/unless}} {{/each~}}) {{/if}} - {{#if (or verbose (eq (lowercase task.Type "edited")))}} + {{#if (or verbose (eq (lowercase task.Type) "edited"))}} {{job-diff-fields-and-objects fields=task.Fields objects=task.Objects}} {{/if}}
From 2d0805c4530b9436d363bb04519f20d432fcd39e Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 20 Aug 2018 15:04:33 -0700 Subject: [PATCH 23/49] Test coverage for scheduler dry-run addition to the plan page --- ui/app/models/job.js | 2 +- ui/app/templates/jobs/run.hbs | 6 +-- ui/mirage/config.js | 28 ++++++++++-- ui/mirage/factories/evaluation.js | 30 +++++++------ ui/tests/acceptance/job-run-test.js | 66 ++++++++++++++++++++++++++--- ui/tests/pages/jobs/run.js | 10 ++++- 6 files changed, 113 insertions(+), 29 deletions(-) diff --git a/ui/app/models/job.js b/ui/app/models/job.js index 6bb5959c0..fc9dfd555 100644 --- a/ui/app/models/job.js +++ b/ui/app/models/job.js @@ -210,7 +210,7 @@ export default Model.extend({ try { // If the definition is already JSON then it doesn't need to be parsed. const json = JSON.parse(definition); - this.set('_newDefinitionJSON', definition); + this.set('_newDefinitionJSON', json); this.setIDByPayload(json); promise = RSVP.resolve(definition); } catch (err) { diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index 2e3856030..9039cae7f 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -74,9 +74,9 @@ {{job-diff data-test-plan-output diff=planOutput.diff verbose=false}}
-
-
Scheduler dry-run
-
+
+
Scheduler dry-run
+
{{#if planOutput.failedTGAllocs}} {{#each planOutput.failedTGAllocs as |placementFailure|}} {{placement-failure failedTGAlloc=placementFailure}} diff --git a/ui/mirage/config.js b/ui/mirage/config.js index 21e4c07e6..d3b2a9778 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -3,6 +3,7 @@ import Response from 'ember-cli-mirage/response'; import { HOSTS } from './common'; import { logFrames, logEncode } from './data/logs'; import { generateDiff } from './factories/job-version'; +import { generateTaskGroupFailures } from './factories/evaluation'; const { copy } = Ember; @@ -56,7 +57,7 @@ export default function() { }) ); - this.post('/jobs', function({ jobs }, req) { + this.post('/jobs', function(schema, req) { const body = JSON.parse(req.requestBody); if (!body.Job) return new Response(400, {}, 'Job is a required field on the request payload'); @@ -64,7 +65,7 @@ export default function() { return okEmpty(); }); - this.post('/jobs/parse', function({ jobs }, req) { + this.post('/jobs/parse', function(schema, req) { const body = JSON.parse(req.requestBody); if (!body.JobHCL) @@ -84,13 +85,19 @@ export default function() { return new Response(200, {}, this.serialize(job)); }); - this.post('/job/:id/plan', function({ jobs }, req) { + this.post('/job/:id/plan', function(schema, req) { const body = JSON.parse(req.requestBody); if (!body.Job) return new Response(400, {}, 'Job is a required field on the request payload'); if (!body.Diff) return new Response(400, {}, 'Expected Diff to be true'); - return new Response(200, {}, JSON.stringify({ Diff: generateDiff(req.params.id) })); + const FailedTGAllocs = body.Job.Unschedulable && generateFailedTGAllocs(body.Job); + + return new Response( + 200, + {}, + JSON.stringify({ FailedTGAllocs, Diff: generateDiff(req.params.id) }) + ); }); this.get( @@ -319,3 +326,16 @@ function filterKeys(object, ...keys) { function okEmpty() { return new Response(200, {}, '{}'); } + +function generateFailedTGAllocs(job, taskGroups) { + const taskGroupsFromSpec = job.TaskGroups && job.TaskGroups.mapBy('Name'); + + let tgNames = ['tg-one', 'tg-two']; + if (taskGroupsFromSpec && taskGroupsFromSpec.length) tgNames = taskGroupsFromSpec; + if (taskGroups && taskGroups.length) tgNames = taskGroups; + + return tgNames.reduce((hash, tgName) => { + hash[tgName] = generateTaskGroupFailures(); + return hash; + }, {}); +} diff --git a/ui/mirage/factories/evaluation.js b/ui/mirage/factories/evaluation.js index ebf01e217..0dbfa25dc 100644 --- a/ui/mirage/factories/evaluation.js +++ b/ui/mirage/factories/evaluation.js @@ -72,19 +72,7 @@ export default Factory.extend({ } const placementFailures = failedTaskGroupNames.reduce((hash, name) => { - hash[name] = { - CoalescedFailures: faker.random.number({ min: 1, max: 20 }), - NodesEvaluated: faker.random.number({ min: 1, max: 100 }), - NodesExhausted: faker.random.number({ min: 1, max: 100 }), - - NodesAvailable: Math.random() > 0.7 ? generateNodesAvailable() : null, - ClassFiltered: Math.random() > 0.7 ? generateClassFiltered() : null, - ConstraintFiltered: Math.random() > 0.7 ? generateConstraintFiltered() : null, - ClassExhausted: Math.random() > 0.7 ? generateClassExhausted() : null, - DimensionExhausted: Math.random() > 0.7 ? generateDimensionExhausted() : null, - QuotaExhausted: Math.random() > 0.7 ? generateQuotaExhausted() : null, - Scores: Math.random() > 0.7 ? generateScores() : null, - }; + hash[name] = generateTaskGroupFailures(); return hash; }, {}); @@ -111,3 +99,19 @@ function assignJob(evaluation, server) { job_id: job.id, }); } + +export function generateTaskGroupFailures() { + return { + CoalescedFailures: faker.random.number({ min: 1, max: 20 }), + NodesEvaluated: faker.random.number({ min: 1, max: 100 }), + NodesExhausted: faker.random.number({ min: 1, max: 100 }), + + NodesAvailable: Math.random() > 0.7 ? generateNodesAvailable() : null, + ClassFiltered: Math.random() > 0.7 ? generateClassFiltered() : null, + ConstraintFiltered: Math.random() > 0.7 ? generateConstraintFiltered() : null, + ClassExhausted: Math.random() > 0.7 ? generateClassExhausted() : null, + DimensionExhausted: Math.random() > 0.7 ? generateDimensionExhausted() : null, + QuotaExhausted: Math.random() > 0.7 ? generateQuotaExhausted() : null, + Scores: Math.random() > 0.7 ? generateScores() : null, + }; +} diff --git a/ui/tests/acceptance/job-run-test.js b/ui/tests/acceptance/job-run-test.js index e168f8ec1..5b56eafcb 100644 --- a/ui/tests/acceptance/job-run-test.js +++ b/ui/tests/acceptance/job-run-test.js @@ -5,6 +5,7 @@ import moduleForAcceptance from 'nomad-ui/tests/helpers/module-for-acceptance'; import JobRun from 'nomad-ui/tests/pages/jobs/run'; const newJobName = 'new-job'; +const newJobTaskGroupName = 'redis'; const jsonJob = overrides => { return JSON.stringify( @@ -15,15 +16,17 @@ const jsonJob = overrides => { Namespace: 'default', Datacenters: ['dc1'], Priority: 50, - TaskGroups: { - redis: { - Tasks: { - redis: { + TaskGroups: [ + { + Name: newJobTaskGroupName, + Tasks: [ + { + Name: 'redis', Driver: 'docker', }, - }, + ], }, - }, + ], }, overrides ), @@ -37,7 +40,7 @@ job "${newJobName}" { namespace = "default" datacenters = ["dc1"] - task "redis" { + task "${newJobTaskGroupName}" { driver = "docker" } } @@ -313,3 +316,52 @@ test('when submitting a job to a different namespace, the redirect to the job ov ); }); }); + +test('when the scheduler dry-run has warnings, the warnings are shown to the user', function(assert) { + // Unschedulable is a hint to Mirage to respond with warnings from the plan endpoint + const spec = jsonJob({ Unschedulable: true }); + + JobRun.visit(); + + andThen(() => { + JobRun.editor.fillIn(spec); + JobRun.plan(); + }); + + andThen(() => { + assert.ok( + JobRun.dryRunMessage.errored, + 'The scheduler dry-run message is in the warning state' + ); + assert.notOk( + JobRun.dryRunMessage.succeeded, + 'The success message is not shown in addition to the warning message' + ); + assert.ok( + JobRun.dryRunMessage.body.includes(newJobTaskGroupName), + 'The scheduler dry-run message includes the warning from send back by the API' + ); + }); +}); + +test('when the scheduler dry-run has no warnings, a success message is shown to the user', function(assert) { + const spec = hclJob(); + + JobRun.visit(); + + andThen(() => { + JobRun.editor.fillIn(spec); + JobRun.plan(); + }); + + andThen(() => { + assert.ok( + JobRun.dryRunMessage.succeeded, + 'The scheduler dry-run message is in the success state' + ); + assert.notOk( + JobRun.dryRunMessage.errored, + 'The warning message is not shown in addition to the success message' + ); + }); +}); diff --git a/ui/tests/pages/jobs/run.js b/ui/tests/pages/jobs/run.js index 9192b03c1..08c1bcb7c 100644 --- a/ui/tests/pages/jobs/run.js +++ b/ui/tests/pages/jobs/run.js @@ -1,4 +1,4 @@ -import { clickable, create, isPresent, text, visitable } from 'ember-cli-page-object'; +import { clickable, create, hasClass, isPresent, text, visitable } from 'ember-cli-page-object'; import { codeFillable, code } from 'nomad-ui/tests/pages/helpers/codemirror'; import error from 'nomad-ui/tests/pages/components/error'; @@ -35,4 +35,12 @@ export default create({ contents: code('[data-test-editor]'), fillIn: codeFillable('[data-test-editor]'), }, + + dryRunMessage: { + scope: '[data-test-dry-run-message]', + title: text('[data-test-dry-run-title]'), + body: text('[data-test-dry-run-body]'), + errored: hasClass('is-warning'), + succeeded: hasClass('is-primary'), + }, }); From d64491b22fc9c878f7a1fc54b287b198d49e69ed Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 12:06:43 -0700 Subject: [PATCH 24/49] New job edit page --- ui/app/router.js | 1 + ui/app/templates/jobs/job/definition.hbs | 1 + 2 files changed, 2 insertions(+) diff --git a/ui/app/router.js b/ui/app/router.js index 7a4015c28..2c298fa71 100644 --- a/ui/app/router.js +++ b/ui/app/router.js @@ -16,6 +16,7 @@ Router.map(function() { this.route('deployments'); this.route('evaluations'); this.route('allocations'); + this.route('edit'); }); }); diff --git a/ui/app/templates/jobs/job/definition.hbs b/ui/app/templates/jobs/job/definition.hbs index 2e4280af2..44b3515aa 100644 --- a/ui/app/templates/jobs/job/definition.hbs +++ b/ui/app/templates/jobs/job/definition.hbs @@ -1,5 +1,6 @@ {{partial "jobs/job/subnav"}}
+ {{#link-to "jobs.job.edit" job class="button is-primary"}}Edit{{/link-to}}
{{json-viewer data-test-definition-view json=model.definition}} From a2c12b91e3b8d720e388820e4fee53142422fa0b Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 12:07:45 -0700 Subject: [PATCH 25/49] Move the bulk of the new job page into a new job editor component --- ui/app/components/job-editor.js | 70 ++++++++++++++++ ui/app/controllers/jobs/run.js | 68 +--------------- ui/app/templates/components/job-editor.hbs | 92 +++++++++++++++++++++ ui/app/templates/jobs/run.hbs | 93 +--------------------- 4 files changed, 167 insertions(+), 156 deletions(-) create mode 100644 ui/app/components/job-editor.js create mode 100644 ui/app/templates/components/job-editor.hbs diff --git a/ui/app/components/job-editor.js b/ui/app/components/job-editor.js new file mode 100644 index 000000000..7464742cf --- /dev/null +++ b/ui/app/components/job-editor.js @@ -0,0 +1,70 @@ +import Component from '@ember/component'; +import { inject as service } from '@ember/service'; +import { computed } from '@ember/object'; +import { task } from 'ember-concurrency'; +import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; +import localStorageProperty from 'nomad-ui/utils/properties/local-storage'; + +export default Component.extend({ + store: service(), + + job: null, + onSubmit() {}, + + parseError: null, + planError: null, + runError: null, + + planOutput: null, + + showPlanMessage: localStorageProperty('nomadMessageJobPlan', true), + showEditorMessage: localStorageProperty('nomadMessageJobEditor', true), + + stage: computed('planOutput', function() { + return this.get('planOutput') ? 'plan' : 'editor'; + }), + + plan: task(function*() { + this.reset(); + + try { + yield this.get('job').parse(); + } catch (err) { + const error = messageFromAdapterError(err) || 'Could not parse input'; + this.set('parseError', error); + return; + } + + try { + yield this.get('job').plan(); + const plan = this.get('store').peekRecord('job-plan', this.get('job.id')); + this.set('planOutput', plan); + } catch (err) { + const error = messageFromAdapterError(err) || 'Could not plan job'; + this.set('planError', error); + } + }).drop(), + + submit: task(function*() { + try { + yield this.get('job').run(); + + const id = this.get('job.plainId'); + const namespace = this.get('job.namespace.name') || 'default'; + + this.reset(); + + // Treat the job as ephemeral and only provide ID parts. + this.get('onSubmit')(id, namespace); + } catch (err) { + const error = messageFromAdapterError(err) || 'Could not submit job'; + this.set('runError', error); + } + }), + + reset() { + this.set('planOutput', null); + this.set('planError', null); + this.set('parseError', null); + }, +}); diff --git a/ui/app/controllers/jobs/run.js b/ui/app/controllers/jobs/run.js index 79525e460..baaf84183 100644 --- a/ui/app/controllers/jobs/run.js +++ b/ui/app/controllers/jobs/run.js @@ -1,69 +1,9 @@ import Controller from '@ember/controller'; -import { inject as service } from '@ember/service'; -import { computed } from '@ember/object'; -import { task } from 'ember-concurrency'; -import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; -import localStorageProperty from 'nomad-ui/utils/properties/local-storage'; export default Controller.extend({ - store: service(), - - parseError: null, - planError: null, - runError: null, - - planOutput: null, - - showPlanMessage: localStorageProperty('nomadMessageJobPlan', true), - showEditorMessage: localStorageProperty('nomadMessageJobEditor', true), - - stage: computed('planOutput', function() { - return this.get('planOutput') ? 'plan' : 'editor'; - }), - - plan: task(function*() { - this.reset(); - - try { - yield this.get('model').parse(); - } catch (err) { - const error = messageFromAdapterError(err) || 'Could not parse input'; - this.set('parseError', error); - return; - } - - try { - yield this.get('model').plan(); - const plan = this.get('store').peekRecord('job-plan', this.get('model.id')); - this.set('planOutput', plan); - } catch (err) { - const error = messageFromAdapterError(err) || 'Could not plan job'; - this.set('planError', error); - } - }).drop(), - - submit: task(function*() { - try { - yield this.get('model').run(); - - const id = this.get('model.plainId'); - const namespace = this.get('model.namespace.name') || 'default'; - - this.reset(); - - // navigate to the new job page - this.transitionToRoute('jobs.job', id, { - queryParams: { jobNamespace: namespace }, - }); - } catch (err) { - const error = messageFromAdapterError(err) || 'Could not submit job'; - this.set('runError', error); - } - }), - - reset() { - this.set('planOutput', null); - this.set('planError', null); - this.set('parseError', null); + onSubmit(id, namespace) { + this.transitionToRoute('jobs.job', id, { + queryParams: { jobNamespace: namespace }, + }); }, }); diff --git a/ui/app/templates/components/job-editor.hbs b/ui/app/templates/components/job-editor.hbs new file mode 100644 index 000000000..250934bc7 --- /dev/null +++ b/ui/app/templates/components/job-editor.hbs @@ -0,0 +1,92 @@ +{{#if parseError}} +
+

Parse Error

+

{{parseError}}

+
+{{/if}} +{{#if planError}} +
+

Plan Error

+

{{planError}}

+
+{{/if}} +{{#if runError}} +
+

Run Error

+

{{runError}}

+
+{{/if}} + +{{#if (eq stage "editor")}} + {{#if showEditorMessage}} +
+
+
+

Run a Job

+

Paste or author HCL or JSON to submit to your cluster. A plan will be requested before the job is submitted.

+
+
+ +
+
+
+ {{/if}} +
+
+ Job Definition +
+
+ {{ivy-codemirror + data-test-editor + value=(or job._newDefinition jobSpec) + valueUpdated=(action (mut job._newDefinition)) + options=(hash + mode="javascript" + theme="hashi" + tabSize=2 + lineNumbers=true + )}} +
+
+
+ +
+{{/if}} + +{{#if (eq stage "plan")}} + {{#if showPlanMessage}} +
+
+
+

Job Plan

+

This is the impact running this job will have on your cluster.

+
+
+ +
+
+
+ {{/if}} +
+
Job Plan
+
+ {{job-diff data-test-plan-output diff=planOutput.diff verbose=false}} +
+
+
+
Scheduler dry-run
+
+ {{#if planOutput.failedTGAllocs}} + {{#each planOutput.failedTGAllocs as |placementFailure|}} + {{placement-failure failedTGAlloc=placementFailure}} + {{/each}} + {{else}} + All tasks successfully allocated. + {{/if}} +
+
+
+ + +
+{{/if}} diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index 9039cae7f..52400b413 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -1,94 +1,3 @@
- {{#if parseError}} -
-

Parse Error

-

{{parseError}}

-
- {{/if}} - {{#if planError}} -
-

Plan Error

-

{{planError}}

-
- {{/if}} - {{#if runError}} -
-

Run Error

-

{{runError}}

-
- {{/if}} - - {{#if (eq stage "editor")}} - {{#if showEditorMessage}} -
-
-
-

Run a Job

-

Paste or author HCL or JSON to submit to your cluster. A plan will be requested before the job is submitted.

-
-
- -
-
-
- {{/if}} -
-
- Job Definition -
-
- {{ivy-codemirror - data-test-editor - value=(or model._newDefinition jobSpec) - valueUpdated=(action (mut model._newDefinition)) - options=(hash - mode="javascript" - theme="hashi" - tabSize=2 - lineNumbers=true - )}} -
-
-
- -
- {{/if}} - - {{#if (eq stage "plan")}} - {{#if showPlanMessage}} -
-
-
-

Job Plan

-

This is the impact running this job will have on your cluster.

-
-
- -
-
-
- {{/if}} -
-
Job Plan
-
- {{job-diff data-test-plan-output diff=planOutput.diff verbose=false}} -
-
-
-
Scheduler dry-run
-
- {{#if planOutput.failedTGAllocs}} - {{#each planOutput.failedTGAllocs as |placementFailure|}} - {{placement-failure failedTGAlloc=placementFailure}} - {{/each}} - {{else}} - All tasks successfully allocated. - {{/if}} -
-
-
- - -
- {{/if}} + {{job-editor job=model onSubmit=(action onSubmit)}}
From e9190f49bab1d420c999dc9ce5dd4b82d395cf76 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 13:47:01 -0700 Subject: [PATCH 26/49] Fix a blocking queries bug The lowest valid blocking query index is 1, but the API will return 0 if there has yet to be an index set (no response). This in conjunction with that 0 being stored as a string made the "fallback to 1" guard not work. --- ui/app/services/watch-list.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/services/watch-list.js b/ui/app/services/watch-list.js index 51154d6bd..4387c40c4 100644 --- a/ui/app/services/watch-list.js +++ b/ui/app/services/watch-list.js @@ -18,6 +18,6 @@ export default Service.extend({ }, setIndexFor(url, value) { - list[url] = value; + list[url] = +value; }, }); From 8031a0d35dd99285561389451d1b2f6e2c198251 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:37:36 -0700 Subject: [PATCH 27/49] Fix multiple highlight bug in the distribution-bar component Caused by the re-indexing that occurs to remove zero-value bars. --- ui/app/templates/components/distribution-bar.hbs | 2 +- ui/app/templates/components/job-page/parts/summary.hbs | 2 +- ui/app/templates/jobs/job/task-group.hbs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/app/templates/components/distribution-bar.hbs b/ui/app/templates/components/distribution-bar.hbs index 4db55477f..c5a5a546a 100644 --- a/ui/app/templates/components/distribution-bar.hbs +++ b/ui/app/templates/components/distribution-bar.hbs @@ -15,7 +15,7 @@
    {{#each _data as |datum index|}} -
  1. +
  2. {{datum.label}} diff --git a/ui/app/templates/components/job-page/parts/summary.hbs b/ui/app/templates/components/job-page/parts/summary.hbs index 38e9a1b68..0bacf5c52 100644 --- a/ui/app/templates/components/job-page/parts/summary.hbs +++ b/ui/app/templates/components/job-page/parts/summary.hbs @@ -39,7 +39,7 @@ class="split-view" as |chart|}}
      {{#each chart.data as |datum index|}} -
    1. +
    2. {{datum.value}} diff --git a/ui/app/templates/jobs/job/task-group.hbs b/ui/app/templates/jobs/job/task-group.hbs index 1c20a03e7..b9ead72d8 100644 --- a/ui/app/templates/jobs/job/task-group.hbs +++ b/ui/app/templates/jobs/job/task-group.hbs @@ -27,7 +27,7 @@ {{#allocation-status-bar allocationContainer=model.summary class="split-view" as |chart|}}
        {{#each chart.data as |datum index|}} -
      1. +
      2. {{datum.value}} From 1e4ca096b2e80f205d5c6b96e81493c5835e9dec Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:38:35 -0700 Subject: [PATCH 28/49] Use the same urlForFindRecord logic for urlForUpdateRecord --- ui/app/adapters/application.js | 59 ++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/ui/app/adapters/application.js b/ui/app/adapters/application.js index b1504ff03..13a24d0f2 100644 --- a/ui/app/adapters/application.js +++ b/ui/app/adapters/application.js @@ -73,32 +73,35 @@ export default RESTAdapter.extend({ // // This is the original implementation of _buildURL // without the pluralization of modelName - urlForFindRecord(id, modelName) { - let path; - let url = []; - let host = get(this, 'host'); - let prefix = this.urlPrefix(); - - if (modelName) { - path = modelName.camelize(); - if (path) { - url.push(path); - } - } - - if (id) { - url.push(encodeURIComponent(id)); - } - - if (prefix) { - url.unshift(prefix); - } - - url = url.join('/'); - if (!host && url && url.charAt(0) !== '/') { - url = '/' + url; - } - - return url; - }, + urlForFindRecord: urlForRecord, + urlForUpdateRecord: urlForRecord, }); + +function urlForRecord(id, modelName) { + let path; + let url = []; + let host = get(this, 'host'); + let prefix = this.urlPrefix(); + + if (modelName) { + path = modelName.camelize(); + if (path) { + url.push(path); + } + } + + if (id) { + url.push(encodeURIComponent(id)); + } + + if (prefix) { + url.unshift(prefix); + } + + url = url.join('/'); + if (!host && url && url.charAt(0) !== '/') { + url = '/' + url; + } + + return url; +} From 522f6d783bc25e81aad61b457d7921f2c4a7e4ad Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:39:47 -0700 Subject: [PATCH 29/49] Support job update in the adapter --- ui/app/adapters/job.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index a8c09ce09..8ad0e5df2 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -33,6 +33,12 @@ export default Watchable.extend({ return associateNamespace(url, namespace); }, + urlForUpdateRecord(id, type, hash) { + const [name, namespace] = JSON.parse(id); + let url = this._super(name, type, hash); + return associateNamespace(url, namespace); + }, + xhrKey(url, method, options = {}) { const plainKey = this._super(...arguments); const namespace = options.data && options.data.namespace; @@ -92,6 +98,15 @@ export default Watchable.extend({ }, }); }, + + update(job) { + const url = this.urlForUpdateRecord(job.get('id'), 'job'); + return this.ajax(this.urlForUpdateRecord(job.get('id'), 'job'), 'POST', { + data: { + Job: job.get('_newDefinitionJSON'), + }, + }); + }, }); function associateNamespace(url, namespace) { From cdc37ec36c8cfc6fcf058704425cad59a3ddd0f3 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:41:05 -0700 Subject: [PATCH 30/49] Support different contexts for the job editor --- ui/app/components/job-editor.js | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/ui/app/components/job-editor.js b/ui/app/components/job-editor.js index 7464742cf..1e240dfd1 100644 --- a/ui/app/components/job-editor.js +++ b/ui/app/components/job-editor.js @@ -7,10 +7,25 @@ import localStorageProperty from 'nomad-ui/utils/properties/local-storage'; export default Component.extend({ store: service(), + config: service(), job: null, onSubmit() {}, + context: computed({ + get() { + return this.get('_context'); + }, + set(key, value) { + const allowedValues = ['new', 'edit']; + if (!allowedValues.includes(value)) { + throw new Error(`context must be one of: ${allowedValues.join(', ')}`); + } + this.set('_context', value); + return value; + }, + }), + _context: null, parseError: null, planError: null, runError: null, @@ -32,6 +47,7 @@ export default Component.extend({ } catch (err) { const error = messageFromAdapterError(err) || 'Could not parse input'; this.set('parseError', error); + this.scrollToError(); return; } @@ -42,12 +58,17 @@ export default Component.extend({ } catch (err) { const error = messageFromAdapterError(err) || 'Could not plan job'; this.set('planError', error); + this.scrollToError(); } }).drop(), submit: task(function*() { try { - yield this.get('job').run(); + if (this.get('context') === 'new') { + yield this.get('job').run(); + } else { + yield this.get('job').update(); + } const id = this.get('job.plainId'); const namespace = this.get('job.namespace.name') || 'default'; @@ -59,6 +80,8 @@ export default Component.extend({ } catch (err) { const error = messageFromAdapterError(err) || 'Could not submit job'; this.set('runError', error); + this.set('planOutput', null); + this.scrollToError(); } }), @@ -66,5 +89,12 @@ export default Component.extend({ this.set('planOutput', null); this.set('planError', null); this.set('parseError', null); + this.set('runError', null); + }, + + scrollToError() { + if (!this.get('config.isTest')) { + window.scrollTo(0, 0); + } }, }); From 91f8c1550ce6c21c36e00839bfe25f8ccd1bd40c Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:43:30 -0700 Subject: [PATCH 31/49] fixup-adapter --- ui/app/adapters/job.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index 8ad0e5df2..b9dc7aa09 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -100,7 +100,6 @@ export default Watchable.extend({ }, update(job) { - const url = this.urlForUpdateRecord(job.get('id'), 'job'); return this.ajax(this.urlForUpdateRecord(job.get('id'), 'job'), 'POST', { data: { Job: job.get('_newDefinitionJSON'), From d36270b0dd8af37572dff8e4fd7b886e494e5e0f Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:43:46 -0700 Subject: [PATCH 32/49] fixup-job-editor --- ui/app/components/job-editor.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ui/app/components/job-editor.js b/ui/app/components/job-editor.js index 1e240dfd1..7cb6ff201 100644 --- a/ui/app/components/job-editor.js +++ b/ui/app/components/job-editor.js @@ -1,4 +1,5 @@ import Component from '@ember/component'; +import { assert } from '@ember/debug'; import { inject as service } from '@ember/service'; import { computed } from '@ember/object'; import { task } from 'ember-concurrency'; @@ -17,9 +18,9 @@ export default Component.extend({ }, set(key, value) { const allowedValues = ['new', 'edit']; - if (!allowedValues.includes(value)) { - throw new Error(`context must be one of: ${allowedValues.join(', ')}`); - } + + assert(`context must be one of: ${allowedValues.join(', ')}`, allowedValues.includes(value)); + this.set('_context', value); return value; }, From 69d28de528ee816e62c8e8d479a317db10ff622a Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:44:31 -0700 Subject: [PATCH 33/49] Handle update job in the model --- ui/app/models/job.js | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/ui/app/models/job.js b/ui/app/models/job.js index fc9dfd555..6db6ef676 100644 --- a/ui/app/models/job.js +++ b/ui/app/models/job.js @@ -203,6 +203,11 @@ export default Model.extend({ return this.store.adapterFor('job').run(this); }, + update() { + assert('A job must be parsed before updated', this.get('_newDefinitionJSON')); + return this.store.adapterFor('job').update(this); + }, + parse() { const definition = this.get('_newDefinition'); let promise; @@ -211,7 +216,12 @@ export default Model.extend({ // If the definition is already JSON then it doesn't need to be parsed. const json = JSON.parse(definition); this.set('_newDefinitionJSON', json); - this.setIDByPayload(json); + + // You can't set the ID of a record that already exists + if (this.get('isNew')) { + this.setIdByPayload(json); + } + promise = RSVP.resolve(definition); } catch (err) { // If the definition is invalid JSON, assume it is HCL. If it is invalid @@ -221,14 +231,14 @@ export default Model.extend({ .parse(this.get('_newDefinition')) .then(response => { this.set('_newDefinitionJSON', response); - this.setIDByPayload(response); + this.setIdByPayload(response); }); } return promise; }, - setIDByPayload(payload) { + setIdByPayload(payload) { const namespace = payload.Namespace || 'default'; const id = payload.Name; @@ -241,6 +251,10 @@ export default Model.extend({ } }, + resetId() { + this.set('id', JSON.stringify([this.get('plainId'), this.get('namespace.name') || 'default'])); + }, + statusClass: computed('status', function() { const classMap = { pending: 'is-pending', From b68ba6110587553fe8e71d31c0affe17fbbdc7e0 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:44:59 -0700 Subject: [PATCH 34/49] Fix bug where scrolling wasn't using the document Instead it was using the page-layout is-right div --- ui/app/styles/components/page-layout.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/styles/components/page-layout.scss b/ui/app/styles/components/page-layout.scss index 294f041ff..26bcf732e 100644 --- a/ui/app/styles/components/page-layout.scss +++ b/ui/app/styles/components/page-layout.scss @@ -1,5 +1,5 @@ .page-layout { - height: 100%; + min-height: 100%; display: flex; flex-direction: column; From b478d71a7b6915dbee13d5258361e2c247dc4b80 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:45:53 -0700 Subject: [PATCH 35/49] Support cancellation of editing in the job-editor --- ui/app/templates/components/job-editor.hbs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/app/templates/components/job-editor.hbs b/ui/app/templates/components/job-editor.hbs index 250934bc7..c0c2df862 100644 --- a/ui/app/templates/components/job-editor.hbs +++ b/ui/app/templates/components/job-editor.hbs @@ -34,6 +34,9 @@
        Job Definition + {{#if cancelable}} + + {{/if}}
        {{ivy-codemirror From 684f45c2854bb87e06d946609c60bbd8e3a300eb Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 21 Aug 2018 16:46:24 -0700 Subject: [PATCH 36/49] Introduce job editing to the job definition page --- ui/app/controllers/jobs/job/definition.js | 18 ++++++++++++++++++ ui/app/routes/jobs/job/definition.js | 9 +++++++++ ui/app/templates/jobs/job/definition.hbs | 22 +++++++++++++++++----- ui/app/templates/jobs/run.hbs | 5 ++++- 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/ui/app/controllers/jobs/job/definition.js b/ui/app/controllers/jobs/job/definition.js index 7efede0aa..ff03ba643 100644 --- a/ui/app/controllers/jobs/job/definition.js +++ b/ui/app/controllers/jobs/job/definition.js @@ -4,4 +4,22 @@ import { alias } from '@ember/object/computed'; export default Controller.extend(WithNamespaceResetting, { job: alias('model.job'), + definition: alias('model.definition'), + + isEditing: false, + + edit() { + this.get('job').set('_newDefinition', JSON.stringify(this.get('definition'), null, 2)); + this.set('isEditing', true); + }, + + onCancel() { + this.set('isEditing', false); + }, + + onSubmit(id, namespace) { + this.transitionToRoute('jobs.job', id, { + queryParams: { jobNamespace: namespace }, + }); + }, }); diff --git a/ui/app/routes/jobs/job/definition.js b/ui/app/routes/jobs/job/definition.js index 8730f83b8..f6294ebf5 100644 --- a/ui/app/routes/jobs/job/definition.js +++ b/ui/app/routes/jobs/job/definition.js @@ -8,4 +8,13 @@ export default Route.extend({ definition, })); }, + + resetController(controller, isExiting) { + if (isExiting) { + const job = controller.get('job'); + job.rollbackAttributes(); + job.resetId(); + controller.set('isEditing', false); + } + }, }); diff --git a/ui/app/templates/jobs/job/definition.hbs b/ui/app/templates/jobs/job/definition.hbs index 44b3515aa..5d3813fb2 100644 --- a/ui/app/templates/jobs/job/definition.hbs +++ b/ui/app/templates/jobs/job/definition.hbs @@ -1,9 +1,21 @@ {{partial "jobs/job/subnav"}}
        - {{#link-to "jobs.job.edit" job class="button is-primary"}}Edit{{/link-to}} -
        -
        - {{json-viewer data-test-definition-view json=model.definition}} + {{#unless isEditing}} +
        +
        + Job Definition + +
        +
        + {{json-viewer data-test-definition-view json=definition}} +
        -
        + {{else}} + {{job-editor + job=job + cancelable=true + context="edit" + onCancel=(action onCancel) + onSubmit=(action onSubmit)}} + {{/unless}}
        diff --git a/ui/app/templates/jobs/run.hbs b/ui/app/templates/jobs/run.hbs index 52400b413..2fa8f850f 100644 --- a/ui/app/templates/jobs/run.hbs +++ b/ui/app/templates/jobs/run.hbs @@ -1,3 +1,6 @@
        - {{job-editor job=model onSubmit=(action onSubmit)}} + {{job-editor + job=model + context="new" + onSubmit=(action onSubmit)}}
        From 4ae15be35edb2b0a432cc8311e1da0cc96736446 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 22 Aug 2018 17:34:25 -0700 Subject: [PATCH 37/49] Since registerHelper doesn't work in integration tests a new way is needed This exports a function that returns the pertinent function when given a container. This way it works in registerHelper and in integration tests beforeEach hooks --- ui/tests/helpers/codemirror.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ui/tests/helpers/codemirror.js b/ui/tests/helpers/codemirror.js index 87db0c814..247eb7699 100644 --- a/ui/tests/helpers/codemirror.js +++ b/ui/tests/helpers/codemirror.js @@ -4,9 +4,9 @@ const invariant = (truthy, error) => { if (!truthy) throw new Error(error); }; -export default function registerCodeMirrorHelpers() { - registerHelper('getCodeMirrorInstance', function(app, selector) { - const cmService = app.__container__.lookup('service:code-mirror'); +export function getCodeMirrorInstance(container) { + return function(selector) { + const cmService = container.lookup('service:code-mirror'); const element = document.querySelector(selector); invariant(element, `Selector ${selector} matched no elements`); @@ -15,5 +15,12 @@ export default function registerCodeMirrorHelpers() { invariant(cm, `No registered CodeMirror instance for ${selector}`); return cm; + }; +} + +export default function registerCodeMirrorHelpers() { + registerHelper('getCodeMirrorInstance', function(app, selector) { + const helper = getCodeMirrorInstance(app.__container__); + return helper(selector); }); } From 6463efe2ceef0d3a5c217d5592fe01e0cd07a7e5 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 22 Aug 2018 17:36:04 -0700 Subject: [PATCH 38/49] Test coverage for the job-editor component Most of this was ported over from the existing job run acceptance tests --- ui/app/components/job-editor.js | 2 + ui/app/templates/components/job-editor.hbs | 4 +- ui/mirage/config.js | 8 + ui/tests/integration/job-editor-test.js | 492 +++++++++++++++++++++ ui/tests/pages/components/job-editor.js | 49 ++ ui/tests/pages/jobs/run.js | 44 +- 6 files changed, 556 insertions(+), 43 deletions(-) create mode 100644 ui/tests/integration/job-editor-test.js create mode 100644 ui/tests/pages/components/job-editor.js diff --git a/ui/app/components/job-editor.js b/ui/app/components/job-editor.js index 7cb6ff201..d72c97a8a 100644 --- a/ui/app/components/job-editor.js +++ b/ui/app/components/job-editor.js @@ -10,6 +10,8 @@ export default Component.extend({ store: service(), config: service(), + 'data-test-job-editor': true, + job: null, onSubmit() {}, context: computed({ diff --git a/ui/app/templates/components/job-editor.hbs b/ui/app/templates/components/job-editor.hbs index c0c2df862..8f85ebc58 100644 --- a/ui/app/templates/components/job-editor.hbs +++ b/ui/app/templates/components/job-editor.hbs @@ -18,7 +18,7 @@ {{/if}} {{#if (eq stage "editor")}} - {{#if showEditorMessage}} + {{#if (and showEditorMessage (eq context "new"))}}
        @@ -35,7 +35,7 @@
        Job Definition {{#if cancelable}} - + {{/if}}
        diff --git a/ui/mirage/config.js b/ui/mirage/config.js index d3b2a9778..d573ed6de 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -116,6 +116,14 @@ export default function() { }) ); + this.post('/job/:id', function(schema, req) { + const body = JSON.parse(req.requestBody); + + if (!body.Job) return new Response(400, {}, 'Job is a required field on the request payload'); + + return okEmpty(); + }); + this.get( '/job/:id/summary', withBlockingSupport(function({ jobSummaries }, { params }) { diff --git a/ui/tests/integration/job-editor-test.js b/ui/tests/integration/job-editor-test.js new file mode 100644 index 000000000..9513a650e --- /dev/null +++ b/ui/tests/integration/job-editor-test.js @@ -0,0 +1,492 @@ +import { getOwner } from '@ember/application'; +import { assign } from '@ember/polyfills'; +import { run } from '@ember/runloop'; +import { test, moduleForComponent } from 'ember-qunit'; +import wait from 'ember-test-helpers/wait'; +import hbs from 'htmlbars-inline-precompile'; +import { create } from 'ember-cli-page-object'; +import sinon from 'sinon'; +import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage'; +import { getCodeMirrorInstance } from 'nomad-ui/tests/helpers/codemirror'; +import jobEditor from 'nomad-ui/tests/pages/components/job-editor'; +import { initialize as fragmentSerializerInitializer } from 'nomad-ui/initializers/fragment-serializer'; + +const Editor = create(jobEditor()); + +moduleForComponent('job-editor', 'Integration | Component | job-editor', { + integration: true, + beforeEach() { + window.localStorage.clear(); + + fragmentSerializerInitializer(getOwner(this)); + + // Normally getCodeMirrorInstance is a registered test helper, + // but those registered test helpers only work in acceptance tests. + window._getCodeMirrorInstance = window.getCodeMirrorInstance; + window.getCodeMirrorInstance = getCodeMirrorInstance(getOwner(this)); + + this.store = getOwner(this).lookup('service:store'); + this.server = startMirage(); + + // Required for placing allocations (a result of creating jobs) + this.server.create('node'); + + Editor.setContext(this); + }, + afterEach() { + this.server.shutdown(); + Editor.removeContext(); + window.getCodeMirrorInstance = window._getCodeMirrorInstance; + delete window._getCodeMirrorInstance; + }, +}); + +const newJobName = 'new-job'; +const newJobTaskGroupName = 'redis'; +const jsonJob = overrides => { + return JSON.stringify( + assign( + {}, + { + Name: newJobName, + Namespace: 'default', + Datacenters: ['dc1'], + Priority: 50, + TaskGroups: [ + { + Name: newJobTaskGroupName, + Tasks: [ + { + Name: 'redis', + Driver: 'docker', + }, + ], + }, + ], + }, + overrides + ), + null, + 2 + ); +}; + +const hclJob = () => ` +job "${newJobName}" { + namespace = "default" + datacenters = ["dc1"] + + task "${newJobTaskGroupName}" { + driver = "docker" + } +} +`; + +const commonTemplate = hbs` + {{job-editor + job=job + context=context + onSubmit=onSubmit}} +`; + +const cancelableTemplate = hbs` + {{job-editor + job=job + context=context + cancelable=true + onSubmit=onSubmit + onCancel=onCancel}} +`; + +const renderNewJob = (component, job) => () => { + component.setProperties({ job, onSubmit: sinon.spy(), context: 'new' }); + component.render(commonTemplate); + return wait(); +}; + +const renderEditJob = (component, job) => () => { + component.setProperties({ job, onSubmit: sinon.spy(), onCancel: sinon.spy(), context: 'edit' }); + component.render(cancelableTemplate); +}; + +const planJob = spec => () => { + Editor.editor.fillIn(spec); + return wait().then(() => { + Editor.plan(); + return wait(); + }); +}; + +test('the default state is an editor with an explanation popup', function(assert) { + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(() => { + assert.ok(Editor.editorHelp.isPresent, 'Editor explanation popup is present'); + assert.ok(Editor.editor.isPresent, 'Editor is present'); + }); +}); + +test('the explanation popup can be dismissed', function(assert) { + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(() => { + Editor.editorHelp.dismiss(); + return wait(); + }) + .then(() => { + assert.notOk(Editor.editorHelp.isPresent, 'Editor explanation popup is gone'); + assert.equal( + window.localStorage.nomadMessageJobEditor, + 'false', + 'Dismissal is persisted in localStorage' + ); + }); +}); + +test('the explanation popup is not shown once the dismissal state is set in localStorage', function(assert) { + window.localStorage.nomadMessageJobEditor = 'false'; + + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(() => { + assert.notOk(Editor.editorHelp.isPresent, 'Editor explanation popup is gone'); + }); +}); + +test('submitting a json job skips the parse endpoint', function(assert) { + const spec = jsonJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + const requests = this.server.pretender.handledRequests.mapBy('url'); + assert.notOk(requests.includes('/v1/jobs/parse'), 'JSON job spec is not parsed'); + assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'JSON job spec is still planned'); + }); +}); + +test('submitting an hcl job requires the parse endpoint', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + const requests = this.server.pretender.handledRequests.mapBy('url'); + assert.ok(requests.includes('/v1/jobs/parse'), 'HCL job spec is parsed first'); + assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'HCL job spec is planned'); + assert.ok( + requests.indexOf('/v1/jobs/parse') < requests.indexOf(`/v1/job/${newJobName}/plan`), + 'Parse comes before Plan' + ); + }); +}); + +test('when a job is successfully parsed and planned, the plan is shown to the user', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + assert.ok(Editor.planOutput, 'The plan is outputted'); + assert.notOk(Editor.editor.isPresent, 'The editor is replaced with the plan output'); + assert.ok(Editor.planHelp.isPresent, 'The plan explanation popup is shown'); + }); +}); + +test('from the plan screen, the cancel button goes back to the editor with the job still in tact', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + Editor.cancel(); + return wait(); + }) + .then(() => { + assert.ok(Editor.editor.isPresent, 'The editor is shown again'); + assert.equal( + Editor.editor.contents, + spec, + 'The spec that was planned is still in the editor' + ); + }); +}); + +test('when parse fails, the parse error message is shown', function(assert) { + const spec = hclJob(); + const errorMessage = 'Parse Failed!! :o'; + + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + this.server.pretender.post('/v1/jobs/parse', () => [400, {}, errorMessage]); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + assert.notOk(Editor.planError.isPresent, 'Plan error is not shown'); + assert.notOk(Editor.runError.isPresent, 'Run error is not shown'); + + assert.ok(Editor.parseError.isPresent, 'Parse error is shown'); + assert.equal( + Editor.parseError.message, + errorMessage, + 'The error message from the server is shown in the error in the UI' + ); + }); +}); + +test('when plan fails, the plan error message is shown', function(assert) { + const spec = hclJob(); + const errorMessage = 'Plan Failed!! :o'; + + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + this.server.pretender.post(`/v1/job/${newJobName}/plan`, () => [400, {}, errorMessage]); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + assert.notOk(Editor.parseError.isPresent, 'Parse error is not shown'); + assert.notOk(Editor.runError.isPresent, 'Run error is not shown'); + + assert.ok(Editor.planError.isPresent, 'Plan error is shown'); + assert.equal( + Editor.planError.message, + errorMessage, + 'The error message from the server is shown in the error in the UI' + ); + }); +}); + +test('when run fails, the run error message is shown', function(assert) { + const spec = hclJob(); + const errorMessage = 'Run Failed!! :o'; + + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + this.server.pretender.post('/v1/jobs', () => [400, {}, errorMessage]); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + Editor.run(); + return wait(); + }) + .then(() => { + assert.notOk(Editor.planError.isPresent, 'Plan error is not shown'); + assert.notOk(Editor.parseError.isPresent, 'Parse error is not shown'); + + assert.ok(Editor.runError.isPresent, 'Run error is shown'); + assert.equal( + Editor.runError.message, + errorMessage, + 'The error message from the server is shown in the error in the UI' + ); + }); +}); + +test('when the scheduler dry-run has warnings, the warnings are shown to the user', function(assert) { + const spec = jsonJob({ Unschedulable: true }); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + assert.ok( + Editor.dryRunMessage.errored, + 'The scheduler dry-run message is in the warning state' + ); + assert.notOk( + Editor.dryRunMessage.succeeded, + 'The success message is not shown in addition to the warning message' + ); + assert.ok( + Editor.dryRunMessage.body.includes(newJobTaskGroupName), + 'The scheduler dry-run message includes the warning from send back by the API' + ); + }); +}); + +test('when the scheduler dry-run has no warnings, a success message is shown to the user', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + assert.ok( + Editor.dryRunMessage.succeeded, + 'The scheduler dry-run message is in the success state' + ); + assert.notOk( + Editor.dryRunMessage.errored, + 'The warning message is not shown in addition to the success message' + ); + }); +}); + +test('when a job is submitted in the edit context, a POST request is made to the update job endpoint', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderEditJob(this, job)) + .then(planJob(spec)) + .then(() => { + Editor.run(); + }) + .then(() => { + const requests = this.server.pretender.handledRequests + .filterBy('method', 'POST') + .mapBy('url'); + assert.ok(requests.includes(`/v1/job/${newJobName}`), 'A request was made to job update'); + assert.notOk(requests.includes('/v1/jobs'), 'A request was not made to job create'); + }); +}); + +test('when a job is submitted in the new context, a POST request is made to the create job endpoint', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + Editor.run(); + }) + .then(() => { + const requests = this.server.pretender.handledRequests + .filterBy('method', 'POST') + .mapBy('url'); + assert.ok(requests.includes('/v1/jobs'), 'A request was made to job create'); + assert.notOk( + requests.includes(`/v1/job/${newJobName}`), + 'A request was not made to job update' + ); + }); +}); + +test('when a job is successfully submitted, the onSubmit hook is called', function(assert) { + const spec = hclJob(); + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(planJob(spec)) + .then(() => { + Editor.run(); + return wait(); + }) + .then(() => { + assert.ok( + this.get('onSubmit').calledWith(newJobName, 'default'), + 'The onSubmit hook was called with the correct arguments' + ); + }); +}); + +test('when the job-editor cancelable flag is false, there is no cancel button in the header', function(assert) { + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderNewJob(this, job)) + .then(() => { + assert.notOk(Editor.cancelEditingIsAvailable, 'No way to cancel editing'); + }); +}); + +test('when the job-editor cancelable flag is true, there is a cancel button in the header', function(assert) { + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderEditJob(this, job)) + .then(() => { + assert.ok(Editor.cancelEditingIsAvailable, 'Cancel editing button exists'); + }); +}); + +test('when the job-editor cancel button is clicked, the onCancel hook is called', function(assert) { + let job; + run(() => { + job = this.store.createRecord('job'); + }); + + return wait() + .then(renderEditJob(this, job)) + .then(() => { + Editor.cancelEditing(); + }) + .then(() => { + assert.ok(this.get('onCancel').calledOnce, 'The onCancel hook was called'); + }); +}); diff --git a/ui/tests/pages/components/job-editor.js b/ui/tests/pages/components/job-editor.js new file mode 100644 index 000000000..c9de367c5 --- /dev/null +++ b/ui/tests/pages/components/job-editor.js @@ -0,0 +1,49 @@ +import { clickable, hasClass, isPresent, text } from 'ember-cli-page-object'; +import { codeFillable, code } from 'nomad-ui/tests/pages/helpers/codemirror'; + +import error from 'nomad-ui/tests/pages/components/error'; + +export default () => ({ + scope: '[data-test-job-editor]', + + planError: error('data-test-plan-error'), + parseError: error('data-test-parse-error'), + runError: error('data-test-run-error'), + + plan: clickable('[data-test-plan]'), + cancel: clickable('[data-test-cancel]'), + run: clickable('[data-test-run]'), + + cancelEditing: clickable('[data-test-cancel-editing]'), + cancelEditingIsAvailable: isPresent('[data-test-cancel-editing]'), + + planOutput: text('[data-test-plan-output]'), + + planHelp: { + isPresent: isPresent('[data-test-plan-help-title]'), + title: text('[data-test-plan-help-title]'), + message: text('[data-test-plan-help-message]'), + dismiss: clickable('[data-test-plan-help-dismiss]'), + }, + + editorHelp: { + isPresent: isPresent('[data-test-editor-help-title]'), + title: text('[data-test-editor-help-title]'), + message: text('[data-test-editor-help-message]'), + dismiss: clickable('[data-test-editor-help-dismiss]'), + }, + + editor: { + isPresent: isPresent('[data-test-editor]'), + contents: code('[data-test-editor]'), + fillIn: codeFillable('[data-test-editor]'), + }, + + dryRunMessage: { + scope: '[data-test-dry-run-message]', + title: text('[data-test-dry-run-title]'), + body: text('[data-test-dry-run-body]'), + errored: hasClass('is-warning'), + succeeded: hasClass('is-primary'), + }, +}); diff --git a/ui/tests/pages/jobs/run.js b/ui/tests/pages/jobs/run.js index 08c1bcb7c..03c24ef14 100644 --- a/ui/tests/pages/jobs/run.js +++ b/ui/tests/pages/jobs/run.js @@ -1,46 +1,8 @@ -import { clickable, create, hasClass, isPresent, text, visitable } from 'ember-cli-page-object'; -import { codeFillable, code } from 'nomad-ui/tests/pages/helpers/codemirror'; +import { create, visitable } from 'ember-cli-page-object'; -import error from 'nomad-ui/tests/pages/components/error'; +import jobEditor from 'nomad-ui/tests/pages/components/job-editor'; export default create({ visit: visitable('/jobs/run'), - - planError: error('data-test-plan-error'), - parseError: error('data-test-parse-error'), - runError: error('data-test-run-error'), - - plan: clickable('[data-test-plan]'), - cancel: clickable('[data-test-cancel]'), - run: clickable('[data-test-run]'), - - planOutput: text('[data-test-plan-output]'), - - planHelp: { - isPresent: isPresent('[data-test-plan-help-title]'), - title: text('[data-test-plan-help-title]'), - message: text('[data-test-plan-help-message]'), - dismiss: clickable('[data-test-plan-help-dismiss]'), - }, - - editorHelp: { - isPresent: isPresent('[data-test-editor-help-title]'), - title: text('[data-test-editor-help-title]'), - message: text('[data-test-editor-help-message]'), - dismiss: clickable('[data-test-editor-help-dismiss]'), - }, - - editor: { - isPresent: isPresent('[data-test-editor]'), - contents: code('[data-test-editor]'), - fillIn: codeFillable('[data-test-editor]'), - }, - - dryRunMessage: { - scope: '[data-test-dry-run-message]', - title: text('[data-test-dry-run-title]'), - body: text('[data-test-dry-run-body]'), - errored: hasClass('is-warning'), - succeeded: hasClass('is-primary'), - }, + editor: jobEditor(), }); From c1d44fb0590c4dd4d6255ae0e0487e57016878c4 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 23 Aug 2018 09:00:47 -0700 Subject: [PATCH 39/49] Rewrite the job run acceptance tests to be about routing --- ui/tests/acceptance/job-run-test.js | 283 +--------------------------- 1 file changed, 7 insertions(+), 276 deletions(-) diff --git a/ui/tests/acceptance/job-run-test.js b/ui/tests/acceptance/job-run-test.js index 5b56eafcb..8d9f99047 100644 --- a/ui/tests/acceptance/job-run-test.js +++ b/ui/tests/acceptance/job-run-test.js @@ -35,17 +35,6 @@ const jsonJob = overrides => { ); }; -const hclJob = () => ` -job "${newJobName}" { - namespace = "default" - datacenters = ["dc1"] - - task "${newJobTaskGroupName}" { - driver = "docker" - } -} -`; - moduleForAcceptance('Acceptance | job run', { beforeEach() { // Required for placing allocations (a result of creating jobs) @@ -61,234 +50,25 @@ test('visiting /jobs/run', function(assert) { }); }); -test('the page has an editor and an explanation popup', function(assert) { - JobRun.visit(); - - andThen(() => { - assert.ok(JobRun.editorHelp.isPresent, 'Editor explanation popup is present'); - assert.ok(JobRun.editor.isPresent, 'Editor is present'); - }); -}); - -test('the explanation popup can be dismissed', function(assert) { - JobRun.visit(); - - andThen(() => { - JobRun.editorHelp.dismiss(); - }); - - andThen(() => { - assert.notOk(JobRun.editorHelp.isPresent, 'Editor explanation popup is gone'); - assert.equal( - window.localStorage.nomadMessageJobEditor, - 'false', - 'Dismissal is persisted in localStorage' - ); - }); -}); - -test('the explanation popup is not shown once the dismissal state is set in localStorage', function(assert) { - window.localStorage.nomadMessageJobEditor = 'false'; - - JobRun.visit(); - - andThen(() => { - assert.notOk(JobRun.editorHelp.isPresent, 'Editor explanation popup is gone'); - }); -}); - -test('submitting a json job skips the parse endpoint', function(assert) { +test('when submitting a job, the site redirects to the new job overview page', function(assert) { const spec = jsonJob(); JobRun.visit(); andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); + JobRun.editor.editor.fillIn(spec); + JobRun.editor.plan(); }); andThen(() => { - const requests = server.pretender.handledRequests.mapBy('url'); - assert.notOk(requests.includes('/v1/jobs/parse'), 'JSON job spec is not parsed'); - assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'JSON job spec is still planned'); + JobRun.editor.run(); }); -}); - -test('submitting an hcl job requires the parse endpoint', function(assert) { - const spec = hclJob(); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - const requests = server.pretender.handledRequests.mapBy('url'); - assert.ok(requests.includes('/v1/jobs/parse'), 'HCL job spec is parsed first'); - assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'HCL job spec is planned'); - assert.ok( - requests.indexOf('/v1/jobs/parse') < requests.indexOf(`/v1/job/${newJobName}/plan`), - 'Parse comes before Plan' - ); - }); -}); - -test('when a job is successfully parsed and planned, the plan is shown to the user', function(assert) { - const spec = hclJob(); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - assert.ok(JobRun.planOutput, 'The plan is outputted'); - assert.notOk(JobRun.editor.isPresent, 'The editor is replaced with the plan output'); - assert.ok(JobRun.planHelp.isPresent, 'The plan explanation popup is shown'); - }); -}); - -test('from the plan screen, the cancel button goes back to the editor with the job still in tact', function(assert) { - const spec = hclJob(); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - JobRun.cancel(); - }); - - andThen(() => { - assert.ok(JobRun.editor.isPresent, 'The editor is shown again'); - assert.notOk(JobRun.planOutpu, 'The plan is gone'); - assert.equal(JobRun.editor.contents, spec, 'The spec that was planned is still in the editor'); - }); -}); - -test('from the plan screen, the submit button submits the job and redirects to the job overview page', function(assert) { - const spec = hclJob(); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - JobRun.run(); - }); - andThen(() => { assert.equal( currentURL(), `/jobs/${newJobName}`, `Redirected to the job overview page for ${newJobName}` ); - - const runRequest = server.pretender.handledRequests.find( - req => req.method === 'POST' && req.url === '/v1/jobs' - ); - const planRequest = server.pretender.handledRequests.find( - req => req.method === 'POST' && req.url === '/v1/jobs/parse' - ); - - assert.ok(runRequest, 'A POST request was made to run the new job'); - assert.deepEqual( - JSON.parse(runRequest.requestBody).Job, - JSON.parse(planRequest.responseText), - 'The Job payload parameter is equivalent to the result of the parse request' - ); - }); -}); - -test('when parse fails, the parse error message is shown', function(assert) { - const spec = hclJob(); - - const errorMessage = 'Parse Failed!! :o'; - server.pretender.post('/v1/jobs/parse', () => [400, {}, errorMessage]); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - assert.notOk(JobRun.planError.isPresent, 'Plan error is not shown'); - assert.notOk(JobRun.runError.isPresent, 'Run error is not shown'); - - assert.ok(JobRun.parseError.isPresent, 'Parse error is shown'); - assert.equal( - JobRun.parseError.message, - errorMessage, - 'The error message from the server is shown in the error in the UI' - ); - }); -}); - -test('when plan fails, the plan error message is shown', function(assert) { - const spec = hclJob(); - - const errorMessage = 'Parse Failed!! :o'; - server.pretender.post(`/v1/job/${newJobName}/plan`, () => [400, {}, errorMessage]); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - assert.notOk(JobRun.parseError.isPresent, 'Parse error is not shown'); - assert.notOk(JobRun.runError.isPresent, 'Run error is not shown'); - - assert.ok(JobRun.planError.isPresent, 'Plan error is shown'); - assert.equal( - JobRun.planError.message, - errorMessage, - 'The error message from the server is shown in the error in the UI' - ); - }); -}); - -test('when run fails, the run error message is shown', function(assert) { - const spec = hclJob(); - - const errorMessage = 'Parse Failed!! :o'; - server.pretender.post('/v1/jobs', () => [400, {}, errorMessage]); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - JobRun.run(); - }); - - andThen(() => { - assert.notOk(JobRun.planError.isPresent, 'Plan error is not shown'); - assert.notOk(JobRun.parseError.isPresent, 'Parse error is not shown'); - - assert.ok(JobRun.runError.isPresent, 'Run error is shown'); - assert.equal( - JobRun.runError.message, - errorMessage, - 'The error message from the server is shown in the error in the UI' - ); }); }); @@ -301,12 +81,12 @@ test('when submitting a job to a different namespace, the redirect to the job ov JobRun.visit(); andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); + JobRun.editor.editor.fillIn(spec); + JobRun.editor.plan(); }); andThen(() => { - JobRun.run(); + JobRun.editor.run(); }); andThen(() => { assert.equal( @@ -316,52 +96,3 @@ test('when submitting a job to a different namespace, the redirect to the job ov ); }); }); - -test('when the scheduler dry-run has warnings, the warnings are shown to the user', function(assert) { - // Unschedulable is a hint to Mirage to respond with warnings from the plan endpoint - const spec = jsonJob({ Unschedulable: true }); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - assert.ok( - JobRun.dryRunMessage.errored, - 'The scheduler dry-run message is in the warning state' - ); - assert.notOk( - JobRun.dryRunMessage.succeeded, - 'The success message is not shown in addition to the warning message' - ); - assert.ok( - JobRun.dryRunMessage.body.includes(newJobTaskGroupName), - 'The scheduler dry-run message includes the warning from send back by the API' - ); - }); -}); - -test('when the scheduler dry-run has no warnings, a success message is shown to the user', function(assert) { - const spec = hclJob(); - - JobRun.visit(); - - andThen(() => { - JobRun.editor.fillIn(spec); - JobRun.plan(); - }); - - andThen(() => { - assert.ok( - JobRun.dryRunMessage.succeeded, - 'The scheduler dry-run message is in the success state' - ); - assert.notOk( - JobRun.dryRunMessage.errored, - 'The warning message is not shown in addition to the success message' - ); - }); -}); From e8db71a06515bb68b1c3dccd2cb525b2d70150e3 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 23 Aug 2018 10:26:20 -0700 Subject: [PATCH 40/49] Acceptance tests for the edit behaviors on the job definition page --- ui/app/templates/jobs/job/definition.hbs | 2 +- ui/tests/acceptance/job-definition-test.js | 53 ++++++++++++++++++++++ ui/tests/pages/components/job-editor.js | 2 + ui/tests/pages/jobs/job/definition.js | 7 ++- 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/ui/app/templates/jobs/job/definition.hbs b/ui/app/templates/jobs/job/definition.hbs index 5d3813fb2..4f30522b6 100644 --- a/ui/app/templates/jobs/job/definition.hbs +++ b/ui/app/templates/jobs/job/definition.hbs @@ -4,7 +4,7 @@
        Job Definition - +
        {{json-viewer data-test-definition-view json=definition}} diff --git a/ui/tests/acceptance/job-definition-test.js b/ui/tests/acceptance/job-definition-test.js index 65736df28..78f65541b 100644 --- a/ui/tests/acceptance/job-definition-test.js +++ b/ui/tests/acceptance/job-definition-test.js @@ -29,3 +29,56 @@ test('the job definition page requests the job to display in an unmutated form', .filter(url => url === jobURL); assert.ok(jobRequests.length === 2, 'Two requests for the job were made'); }); + +test('the job definition can be edited', function(assert) { + assert.notOk(Definition.editor.isPresent, 'Editor is not shown on load'); + + Definition.edit(); + + andThen(() => { + assert.ok(Definition.editor.isPresent, 'Editor is shown after clicking edit'); + assert.notOk(Definition.jsonViewer, 'Editor replaces the JSON viewer'); + }); +}); + +test('when in editing mode, the action can be canceled, showing the read-only definition again', function(assert) { + Definition.edit(); + + andThen(() => { + Definition.editor.cancelEditing(); + }); + + andThen(() => { + assert.ok(Definition.jsonViewer, 'The JSON Viewer is back'); + assert.notOk(Definition.editor.isPresent, 'The editor is gone'); + }); +}); + +test('when in editing mode, the editor is prepopulated with the job definition', function(assert) { + const requests = server.pretender.handledRequests; + const jobDefinition = requests.findBy('url', `/v1/job/${job.id}`).responseText; + const formattedJobDefinition = JSON.stringify(JSON.parse(jobDefinition), null, 2); + + Definition.edit(); + + andThen(() => { + assert.equal( + Definition.editor.editor.contents, + formattedJobDefinition, + 'The editor already has the job definition in it' + ); + }); +}); + +test('when changes are submitted, the site redirects to the job overview page', function(assert) { + Definition.edit(); + + andThen(() => { + Definition.editor.plan(); + Definition.editor.run(); + }); + + andThen(() => { + assert.equal(currentURL(), `/jobs/${job.id}`, 'Now on the job overview page'); + }); +}); diff --git a/ui/tests/pages/components/job-editor.js b/ui/tests/pages/components/job-editor.js index c9de367c5..387cd95a1 100644 --- a/ui/tests/pages/components/job-editor.js +++ b/ui/tests/pages/components/job-editor.js @@ -6,6 +6,8 @@ import error from 'nomad-ui/tests/pages/components/error'; export default () => ({ scope: '[data-test-job-editor]', + isPresent: isPresent(), + planError: error('data-test-plan-error'), parseError: error('data-test-parse-error'), runError: error('data-test-run-error'), diff --git a/ui/tests/pages/jobs/job/definition.js b/ui/tests/pages/jobs/job/definition.js index 789d0cabc..b015019b7 100644 --- a/ui/tests/pages/jobs/job/definition.js +++ b/ui/tests/pages/jobs/job/definition.js @@ -1,7 +1,12 @@ -import { create, isPresent, visitable } from 'ember-cli-page-object'; +import { create, isPresent, visitable, clickable } from 'ember-cli-page-object'; + +import jobEditor from 'nomad-ui/tests/pages/components/job-editor'; export default create({ visit: visitable('/jobs/:id/definition'), jsonViewer: isPresent('[data-test-definition-view]'), + editor: jobEditor(), + + edit: clickable('[data-test-edit-job]'), }); From 85aaa53fd58f1c2ca4f821157dbf05535b5e9a06 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 23 Aug 2018 15:40:42 -0700 Subject: [PATCH 41/49] Simplify the data control flow around job.plan() --- ui/app/adapters/job.js | 10 +++++++--- ui/app/components/job-editor.js | 3 +-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index b9dc7aa09..efc18efea 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -77,15 +77,19 @@ export default Watchable.extend({ }, plan(job) { - const url = addToPath(this.urlForFindRecord(job.get('id'), 'job'), '/plan'); + const jobId = job.get('id'); + const store = this.get('store'); + const url = addToPath(this.urlForFindRecord(jobId, 'job'), '/plan'); + return this.ajax(url, 'POST', { data: { Job: job.get('_newDefinitionJSON'), Diff: true, }, }).then(json => { - json.ID = job.get('id'); - this.get('store').pushPayload('job-plan', { jobPlans: [json] }); + json.ID = jobId; + store.pushPayload('job-plan', { jobPlans: [json] }); + return store.peekRecord('job-plan', jobId); }); }, diff --git a/ui/app/components/job-editor.js b/ui/app/components/job-editor.js index d72c97a8a..2620954dc 100644 --- a/ui/app/components/job-editor.js +++ b/ui/app/components/job-editor.js @@ -55,8 +55,7 @@ export default Component.extend({ } try { - yield this.get('job').plan(); - const plan = this.get('store').peekRecord('job-plan', this.get('job.id')); + const plan = yield this.get('job').plan(); this.set('planOutput', plan); } catch (err) { const error = messageFromAdapterError(err) || 'Could not plan job'; From 78b9d32eb46698ccf0ba8889dd07cd9337cdb11a Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 24 Aug 2018 15:33:04 -0700 Subject: [PATCH 42/49] Support the promote deployment api action --- ui/app/adapters/deployment.js | 29 +++++++++++++++++++++++++++++ ui/app/models/deployment.js | 6 ++++++ 2 files changed, 35 insertions(+) create mode 100644 ui/app/adapters/deployment.js diff --git a/ui/app/adapters/deployment.js b/ui/app/adapters/deployment.js new file mode 100644 index 000000000..6dbc20dbd --- /dev/null +++ b/ui/app/adapters/deployment.js @@ -0,0 +1,29 @@ +import Watchable from './watchable'; + +export default Watchable.extend({ + promote(deployment) { + const id = deployment.get('id'); + const url = urlForAction(this.urlForFindRecord(id, 'deployment'), '/promote'); + return this.ajax(url, 'POST', { + data: { + DeploymentId: id, + All: true, + }, + }); + }, +}); + +// The deployment action API endpoints all end with the ID +// /deployment/:action/:deployment_id instead of /deployment/:deployment_id/:action +function urlForAction(url, extension = '') { + const [path, params] = url.split('?'); + const pathParts = path.split('/'); + const idPart = pathParts.pop(); + let newUrl = `${pathParts.join('/')}${extension}/${idPart}`; + + if (params) { + newUrl += `?${params}`; + } + + return newUrl; +} diff --git a/ui/app/models/deployment.js b/ui/app/models/deployment.js index 5d86e9296..e3fefb94f 100644 --- a/ui/app/models/deployment.js +++ b/ui/app/models/deployment.js @@ -1,5 +1,6 @@ import { alias, equal } from '@ember/object/computed'; import { computed } from '@ember/object'; +import { assert } from '@ember/debug'; import Model from 'ember-data/model'; import attr from 'ember-data/attr'; import { belongsTo, hasMany } from 'ember-data/relationships'; @@ -58,4 +59,9 @@ export default Model.extend({ return classMap[this.get('status')] || 'is-dark'; }), + + promote() { + assert('A deployment needs to requirePromotion to be promoted', this.get('requiresPromotion')); + return this.store.adapterFor('deployment').promote(this); + }, }); From c510060c128c8511f787f37ac85792e9e03cc399 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 24 Aug 2018 15:34:07 -0700 Subject: [PATCH 43/49] Change the latest deployment component to include a Promote Canary button Before it would say the deployment required promotion, now it has a button that triggers the promotion. --- .../job-page/parts/latest-deployment.js | 19 +++++++++++++++++++ .../job-page/parts/latest-deployment.hbs | 6 +++++- .../templates/components/job-page/service.hbs | 2 +- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/ui/app/components/job-page/parts/latest-deployment.js b/ui/app/components/job-page/parts/latest-deployment.js index 72ea12f8e..b68978520 100644 --- a/ui/app/components/job-page/parts/latest-deployment.js +++ b/ui/app/components/job-page/parts/latest-deployment.js @@ -1,8 +1,27 @@ import Component from '@ember/component'; +import { task } from 'ember-concurrency'; +import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; export default Component.extend({ job: null, tagName: '', + handleError() {}, + isShowingDeploymentDetails: false, + + promote: task(function*() { + try { + yield this.get('job.latestDeployment.content').promote(); + } catch (err) { + let message = messageFromAdapterError(err); + if (!message || message === 'Forbidden') { + message = 'Your ACL token does not grant permission to promote deployments.'; + } + this.get('handleError')({ + title: 'Could Not Promote Deployment', + description: message, + }); + } + }), }); diff --git a/ui/app/templates/components/job-page/parts/latest-deployment.hbs b/ui/app/templates/components/job-page/parts/latest-deployment.hbs index b67f0bd0b..04bb69caa 100644 --- a/ui/app/templates/components/job-page/parts/latest-deployment.hbs +++ b/ui/app/templates/components/job-page/parts/latest-deployment.hbs @@ -13,7 +13,11 @@ {{job.latestDeployment.status}} {{#if job.latestDeployment.requiresPromotion}} - Deployment is running but requires promotion + {{/if}}
        diff --git a/ui/app/templates/components/job-page/service.hbs b/ui/app/templates/components/job-page/service.hbs index 6a921beb8..cbef54c1f 100644 --- a/ui/app/templates/components/job-page/service.hbs +++ b/ui/app/templates/components/job-page/service.hbs @@ -17,7 +17,7 @@ {{job-page/parts/placement-failures job=job}} - {{job-page/parts/latest-deployment job=job}} + {{job-page/parts/latest-deployment job=job handleError=(action "handleError")}} {{job-page/parts/task-groups job=job From 619376c3b85a4a82e3c795e590afa7f663a0e8eb Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 24 Aug 2018 13:17:45 -0700 Subject: [PATCH 44/49] Add Start Job action on the job overview page for when a job is dead --- ui/app/components/job-page/parts/title.js | 31 +++++++++++++++++++ .../components/job-page/parts/title.hbs | 8 +++++ 2 files changed, 39 insertions(+) diff --git a/ui/app/components/job-page/parts/title.js b/ui/app/components/job-page/parts/title.js index fb8e3232c..a7f079705 100644 --- a/ui/app/components/job-page/parts/title.js +++ b/ui/app/components/job-page/parts/title.js @@ -1,4 +1,5 @@ import Component from '@ember/component'; +import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; export default Component.extend({ tagName: '', @@ -19,5 +20,35 @@ export default Component.extend({ }); }); }, + + startJob() { + const job = this.get('job'); + job + .fetchRawDefinition() + .then(definition => { + // A stopped job will have this Stop = true metadata + // If Stop is true when submitted to the cluster, the job + // won't transition from the Dead to Running state. + delete definition.Stop; + job.set('_newDefinition', JSON.stringify(definition)); + }) + .then(() => { + return job.parse(); + }) + .then(() => { + return job.update(); + }) + .catch(err => { + let message = messageFromAdapterError(err); + if (!message || message === 'Forbidden') { + message = 'Your ACL token does not grant permission to stop jobs.'; + } + + this.get('handleError')({ + title: 'Could Not Start Job', + description: message, + }); + }); + }, }, }); diff --git a/ui/app/templates/components/job-page/parts/title.hbs b/ui/app/templates/components/job-page/parts/title.hbs index 445640d76..6bbd9f9b4 100644 --- a/ui/app/templates/components/job-page/parts/title.hbs +++ b/ui/app/templates/components/job-page/parts/title.hbs @@ -10,5 +10,13 @@ confirmText="Yes, Stop" confirmationMessage="Are you sure you want to stop this job?" onConfirm=(action "stopJob")}} + {{else}} + {{two-step-button + data-test-start + idleText="Start" + cancelText="Cancel" + confirmText="Yes, Start" + confirmationMessage="Are you sure you want to start this job?" + onConfirm=(action "startJob")}} {{/if}} From 1b481a3b72f6b1b6873d0185bd735d9530a583ae Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 24 Aug 2018 13:18:10 -0700 Subject: [PATCH 45/49] Test coverage for the Start Job behavior --- ui/tests/integration/job-page/helpers.js | 24 +++++++- .../integration/job-page/periodic-test.js | 57 ++++++++++++++++++- ui/tests/integration/job-page/service-test.js | 42 +++++++++++++- 3 files changed, 117 insertions(+), 6 deletions(-) diff --git a/ui/tests/integration/job-page/helpers.js b/ui/tests/integration/job-page/helpers.js index ef64e86af..21deb51b1 100644 --- a/ui/tests/integration/job-page/helpers.js +++ b/ui/tests/integration/job-page/helpers.js @@ -19,11 +19,31 @@ export function stopJob() { }); } -export function expectStopError(assert) { +export function startJob() { + click('[data-test-start] [data-test-idle-button]'); + return wait().then(() => { + click('[data-test-start] [data-test-confirm-button]'); + return wait(); + }); +} + +export function expectStartRequest(assert, server, job) { + const expectedURL = jobURL(job); + const request = server.pretender.handledRequests + .filterBy('method', 'POST') + .find(req => req.url === expectedURL); + + const requestPayload = JSON.parse(request.requestBody).Job; + + assert.ok(request, 'POST URL was made correctly'); + assert.ok(requestPayload.Stop == null, 'The Stop signal is not sent in the POST request'); +} + +export function expectError(assert, title) { return () => { assert.equal( find('[data-test-job-error-title]').textContent, - 'Could Not Stop Job', + title, 'Appropriate error is shown' ); assert.ok( diff --git a/ui/tests/integration/job-page/periodic-test.js b/ui/tests/integration/job-page/periodic-test.js index ba93d1a51..0908447f9 100644 --- a/ui/tests/integration/job-page/periodic-test.js +++ b/ui/tests/integration/job-page/periodic-test.js @@ -4,7 +4,14 @@ import { click, find, findAll } from 'ember-native-dom-helpers'; import wait from 'ember-test-helpers/wait'; import hbs from 'htmlbars-inline-precompile'; import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage'; -import { jobURL, stopJob, expectStopError, expectDeleteRequest } from './helpers'; +import { + jobURL, + stopJob, + startJob, + expectError, + expectDeleteRequest, + expectStartRequest, +} from './helpers'; moduleForComponent('job-page/periodic', 'Integration | Component | job-page/periodic', { integration: true, @@ -167,5 +174,51 @@ test('Stopping a job without proper permissions shows an error message', functio return wait(); }) .then(stopJob) - .then(expectStopError(assert)); + .then(expectError(assert, 'Could Not Stop Job')); +}); + +test('Starting a job sends a post request for the job using the current definition', function(assert) { + let job; + + const mirageJob = this.server.create('job', 'periodic', { + childrenCount: 0, + createAllocations: false, + status: 'dead', + }); + this.store.findAll('job'); + + return wait() + .then(() => { + job = this.store.peekAll('job').findBy('plainId', mirageJob.id); + + this.setProperties(commonProperties(job)); + this.render(commonTemplate); + + return wait(); + }) + .then(startJob) + .then(() => expectStartRequest(assert, this.server, job)); +}); + +test('Starting a job without proper permissions shows an error message', function(assert) { + this.server.pretender.post('/v1/job/:id', () => [403, {}, null]); + + const mirageJob = this.server.create('job', 'periodic', { + childrenCount: 0, + createAllocations: false, + status: 'dead', + }); + this.store.findAll('job'); + + return wait() + .then(() => { + const job = this.store.peekAll('job').findBy('plainId', mirageJob.id); + + this.setProperties(commonProperties(job)); + this.render(commonTemplate); + + return wait(); + }) + .then(startJob) + .then(expectError(assert, 'Could Not Start Job')); }); diff --git a/ui/tests/integration/job-page/service-test.js b/ui/tests/integration/job-page/service-test.js index aef698a14..ff29b3da9 100644 --- a/ui/tests/integration/job-page/service-test.js +++ b/ui/tests/integration/job-page/service-test.js @@ -4,7 +4,7 @@ import { test, moduleForComponent } from 'ember-qunit'; import wait from 'ember-test-helpers/wait'; import hbs from 'htmlbars-inline-precompile'; import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage'; -import { stopJob, expectStopError, expectDeleteRequest } from './helpers'; +import { startJob, stopJob, expectError, expectDeleteRequest, expectStartRequest } from './helpers'; import Job from 'nomad-ui/tests/pages/jobs/detail'; moduleForComponent('job-page/service', 'Integration | Component | job-page/service', { @@ -88,7 +88,45 @@ test('Stopping a job without proper permissions shows an error message', functio return wait(); }) .then(stopJob) - .then(expectStopError(assert)); + .then(expectError(assert, 'Could Not Stop Job')); +}); + +test('Starting a job sends a post request for the job using the current definition', function(assert) { + let job; + + const mirageJob = makeMirageJob(this.server, { status: 'dead' }); + this.store.findAll('job'); + + return wait() + .then(() => { + job = this.store.peekAll('job').findBy('plainId', mirageJob.id); + + this.setProperties(commonProperties(job)); + this.render(commonTemplate); + + return wait(); + }) + .then(startJob) + .then(() => expectStartRequest(assert, this.server, job)); +}); + +test('Starting a job without proper permissions shows an error message', function(assert) { + this.server.pretender.post('/v1/job/:id', () => [403, {}, null]); + + const mirageJob = makeMirageJob(this.server, { status: 'dead' }); + this.store.findAll('job'); + + return wait() + .then(() => { + const job = this.store.peekAll('job').findBy('plainId', mirageJob.id); + + this.setProperties(commonProperties(job)); + this.render(commonTemplate); + + return wait(); + }) + .then(startJob) + .then(expectError(assert, 'Could Not Start Job')); }); test('Recent allocations shows allocations in the job context', function(assert) { From f09e9a41bc2baa102bbb227f22b5eaf7822cf1b4 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 24 Aug 2018 13:20:29 -0700 Subject: [PATCH 46/49] Switch stop/run job actions to EC tasks --- ui/app/components/job-page/parts/title.js | 72 +++++++++---------- .../components/job-page/parts/title.hbs | 4 +- 2 files changed, 34 insertions(+), 42 deletions(-) diff --git a/ui/app/components/job-page/parts/title.js b/ui/app/components/job-page/parts/title.js index a7f079705..d6b4dcbf5 100644 --- a/ui/app/components/job-page/parts/title.js +++ b/ui/app/components/job-page/parts/title.js @@ -1,4 +1,5 @@ import Component from '@ember/component'; +import { task } from 'ember-concurrency'; import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error'; export default Component.extend({ @@ -9,46 +10,37 @@ export default Component.extend({ handleError() {}, - actions: { - stopJob() { - this.get('job') - .stop() - .catch(() => { - this.get('handleError')({ - title: 'Could Not Stop Job', - description: 'Your ACL token does not grant permission to stop jobs.', - }); - }); - }, + stopJob: task(function*() { + try { + yield this.get('job').stop(); + } catch (err) { + this.get('handleError')({ + title: 'Could Not Stop Job', + description: 'Your ACL token does not grant permission to stop jobs.', + }); + } + }), - startJob() { - const job = this.get('job'); - job - .fetchRawDefinition() - .then(definition => { - // A stopped job will have this Stop = true metadata - // If Stop is true when submitted to the cluster, the job - // won't transition from the Dead to Running state. - delete definition.Stop; - job.set('_newDefinition', JSON.stringify(definition)); - }) - .then(() => { - return job.parse(); - }) - .then(() => { - return job.update(); - }) - .catch(err => { - let message = messageFromAdapterError(err); - if (!message || message === 'Forbidden') { - message = 'Your ACL token does not grant permission to stop jobs.'; - } + startJob: task(function*() { + const job = this.get('job'); + const definition = yield job.fetchRawDefinition(); - this.get('handleError')({ - title: 'Could Not Start Job', - description: message, - }); - }); - }, - }, + delete definition.Stop; + job.set('_newDefinition', JSON.stringify(definition)); + + try { + yield job.parse(); + yield job.update(); + } catch (err) { + let message = messageFromAdapterError(err); + if (!message || message === 'Forbidden') { + message = 'Your ACL token does not grant permission to stop jobs.'; + } + + this.get('handleError')({ + title: 'Could Not Start Job', + description: message, + }); + } + }), }); diff --git a/ui/app/templates/components/job-page/parts/title.hbs b/ui/app/templates/components/job-page/parts/title.hbs index 6bbd9f9b4..a7cf01876 100644 --- a/ui/app/templates/components/job-page/parts/title.hbs +++ b/ui/app/templates/components/job-page/parts/title.hbs @@ -9,7 +9,7 @@ cancelText="Cancel" confirmText="Yes, Stop" confirmationMessage="Are you sure you want to stop this job?" - onConfirm=(action "stopJob")}} + onConfirm=(perform stopJob)}} {{else}} {{two-step-button data-test-start @@ -17,6 +17,6 @@ cancelText="Cancel" confirmText="Yes, Start" confirmationMessage="Are you sure you want to start this job?" - onConfirm=(action "startJob")}} + onConfirm=(perform startJob)}} {{/if}} From f79f0370968dd4e16c5742caba52904335f7c305 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 24 Aug 2018 13:41:14 -0700 Subject: [PATCH 47/49] Add a confirmation loading state to the two-step-button component --- ui/app/components/two-step-button.js | 1 + .../freestyle/sg-two-step-button.hbs | 18 +++++++++++++ .../templates/components/two-step-button.hbs | 25 ++++++++++++------ ui/tests/integration/two-step-button-test.js | 26 +++++++++++++++++++ 4 files changed, 62 insertions(+), 8 deletions(-) diff --git a/ui/app/components/two-step-button.js b/ui/app/components/two-step-button.js index 7016e96ec..431526841 100644 --- a/ui/app/components/two-step-button.js +++ b/ui/app/components/two-step-button.js @@ -8,6 +8,7 @@ export default Component.extend({ cancelText: '', confirmText: '', confirmationMessage: '', + awaitingConfirmation: false, onConfirm() {}, onCancel() {}, diff --git a/ui/app/templates/components/freestyle/sg-two-step-button.hbs b/ui/app/templates/components/freestyle/sg-two-step-button.hbs index f39096b0e..74f43919f 100644 --- a/ui/app/templates/components/freestyle/sg-two-step-button.hbs +++ b/ui/app/templates/components/freestyle/sg-two-step-button.hbs @@ -20,3 +20,21 @@
        {{/freestyle-usage}} + +{{#freestyle-usage "two-step-button-loading" title="Two Step Button Loading State"}} +
        +

        + This is a page title + {{two-step-button + idleText="Scary Action" + cancelText="Nvm" + confirmText="Yep" + confirmationMessage="Wait, really? Like...seriously?" + awaitingConfirmation=true + state="prompt"}} +

        +
        +{{/freestyle-usage}} +{{#freestyle-annotation}} + Note: the state property is internal state and only used here to bypass the idle state for demonstration purposes. +{{/freestyle-annotation}} diff --git a/ui/app/templates/components/two-step-button.hbs b/ui/app/templates/components/two-step-button.hbs index e9fe906b5..cffa9dd7b 100644 --- a/ui/app/templates/components/two-step-button.hbs +++ b/ui/app/templates/components/two-step-button.hbs @@ -4,16 +4,25 @@ {{else if isPendingConfirmation}} {{confirmationMessage}} - - {{/if}} diff --git a/ui/tests/integration/two-step-button-test.js b/ui/tests/integration/two-step-button-test.js index 41eff1e6e..12d23c1d7 100644 --- a/ui/tests/integration/two-step-button-test.js +++ b/ui/tests/integration/two-step-button-test.js @@ -13,6 +13,7 @@ const commonProperties = () => ({ cancelText: 'Cancel Action', confirmText: 'Confirm Action', confirmationMessage: 'Are you certain', + awaitingConfirmation: false, onConfirm: sinon.spy(), onCancel: sinon.spy(), }); @@ -23,6 +24,7 @@ const commonTemplate = hbs` cancelText=cancelText confirmText=confirmText confirmationMessage=confirmationMessage + awaitingConfirmation=awaitingConfirmation onConfirm=onConfirm onCancel=onCancel}} `; @@ -109,3 +111,27 @@ test('confirming the promptForConfirmation state calls the onConfirm hook and re }); }); }); + +test('when awaitingConfirmation is true, the cancel and submit buttons are disabled and the submit button is loading', function(assert) { + const props = commonProperties(); + props.awaitingConfirmation = true; + this.setProperties(props); + this.render(commonTemplate); + + click('[data-test-idle-button]'); + + return wait().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' + ); + }); +}); From f4ceb2264c272daa46128d6bf757fe6efecd44ca Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 24 Aug 2018 14:02:13 -0700 Subject: [PATCH 48/49] Fix the flickering issue with start/stop job When the server doesn't respond immediately, there is a visible window of time between the action being submitted and the blocking query coming back with the new job status. --- ui/app/components/job-page/parts/title.js | 7 ++++++- ui/app/components/two-step-button.js | 6 ++++++ ui/app/templates/components/job-page/parts/title.hbs | 2 ++ ui/app/templates/components/two-step-button.hbs | 5 +---- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/ui/app/components/job-page/parts/title.js b/ui/app/components/job-page/parts/title.js index d6b4dcbf5..ba3284627 100644 --- a/ui/app/components/job-page/parts/title.js +++ b/ui/app/components/job-page/parts/title.js @@ -12,7 +12,10 @@ export default Component.extend({ stopJob: task(function*() { try { - yield this.get('job').stop(); + const job = this.get('job'); + yield job.stop(); + // Eagerly update the job status to avoid flickering + this.job.set('status', 'dead'); } catch (err) { this.get('handleError')({ title: 'Could Not Stop Job', @@ -31,6 +34,8 @@ export default Component.extend({ try { yield job.parse(); yield job.update(); + // Eagerly update the job status to avoid flickering + job.set('status', 'running'); } catch (err) { let message = messageFromAdapterError(err); if (!message || message === 'Forbidden') { diff --git a/ui/app/components/two-step-button.js b/ui/app/components/two-step-button.js index 431526841..fe40c16f2 100644 --- a/ui/app/components/two-step-button.js +++ b/ui/app/components/two-step-button.js @@ -1,5 +1,6 @@ import Component from '@ember/component'; import { equal } from '@ember/object/computed'; +import RSVP from 'rsvp'; export default Component.extend({ classNames: ['two-step-button'], @@ -23,5 +24,10 @@ export default Component.extend({ promptForConfirmation() { this.set('state', 'prompt'); }, + confirm() { + RSVP.resolve(this.get('onConfirm')()).then(() => { + this.send('setToIdle'); + }); + }, }, }); diff --git a/ui/app/templates/components/job-page/parts/title.hbs b/ui/app/templates/components/job-page/parts/title.hbs index a7cf01876..131b06e56 100644 --- a/ui/app/templates/components/job-page/parts/title.hbs +++ b/ui/app/templates/components/job-page/parts/title.hbs @@ -9,6 +9,7 @@ cancelText="Cancel" confirmText="Yes, Stop" confirmationMessage="Are you sure you want to stop this job?" + awaitingConfirmation=stopJob.isRunning onConfirm=(perform stopJob)}} {{else}} {{two-step-button @@ -17,6 +18,7 @@ cancelText="Cancel" confirmText="Yes, Start" confirmationMessage="Are you sure you want to start this job?" + awaitingConfirmation=startJob.isRunning onConfirm=(perform startJob)}} {{/if}} diff --git a/ui/app/templates/components/two-step-button.hbs b/ui/app/templates/components/two-step-button.hbs index cffa9dd7b..1b95184fe 100644 --- a/ui/app/templates/components/two-step-button.hbs +++ b/ui/app/templates/components/two-step-button.hbs @@ -19,10 +19,7 @@ data-test-confirm-button class="button is-danger is-small {{if awaitingConfirmation "is-loading"}}" disabled={{awaitingConfirmation}} - onclick={{action (queue - (action "setToIdle") - (action onConfirm) - )}}> + onclick={{action "confirm"}}> {{confirmText}} {{/if}} From c96c99aa37edc585e22daa83deaf927c8141ef8b Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 24 Aug 2018 16:32:08 -0700 Subject: [PATCH 49/49] Test coverage for the promote canary feature --- .../job-page/parts/latest-deployment.hbs | 1 + ui/mirage/config.js | 3 + .../deployment-task-group-summary.js | 2 + ui/mirage/factories/deployment.js | 2 + ui/tests/integration/job-page/service-test.js | 77 +++++++++++++++++++ 5 files changed, 85 insertions(+) diff --git a/ui/app/templates/components/job-page/parts/latest-deployment.hbs b/ui/app/templates/components/job-page/parts/latest-deployment.hbs index 04bb69caa..a2a3aef3e 100644 --- a/ui/app/templates/components/job-page/parts/latest-deployment.hbs +++ b/ui/app/templates/components/job-page/parts/latest-deployment.hbs @@ -14,6 +14,7 @@ {{#if job.latestDeployment.requiresPromotion}}