diff --git a/CHANGELOG.md b/CHANGELOG.md index 5805c5240ac..05c06f80f6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ IMPROVEMENTS: * networking: Added support for user-defined iptables rules on the NOMAD-ADMIN chain. [[GH-10181](https://github.com/hashicorp/nomad/issues/10181)] * networking: Added support for interpolating host network names with node attributes. [[GH-10196](https://github.com/hashicorp/nomad/issues/10196)] * nomad/structs: Removed deprecated Node.Drain field, added API extensions to restore it [[GH-10202](https://github.com/hashicorp/nomad/issues/10202)] + * ui: Added a job reversion button [[GH-10336](https://github.com/hashicorp/nomad/pull/10336)] BUG FIXES: * core (Enterprise): Update licensing library to v0.0.11 to include race condition fix. [[GH-10253](https://github.com/hashicorp/nomad/issues/10253)] diff --git a/ui/app/adapters/job-version.js b/ui/app/adapters/job-version.js new file mode 100644 index 00000000000..eb2393a9489 --- /dev/null +++ b/ui/app/adapters/job-version.js @@ -0,0 +1,18 @@ +import ApplicationAdapter from './application'; +import addToPath from 'nomad-ui/utils/add-to-path'; + +export default class JobVersionAdapter extends ApplicationAdapter { + revertTo(jobVersion) { + const jobAdapter = this.store.adapterFor('job'); + + const url = addToPath(jobAdapter.urlForFindRecord(jobVersion.get('job.id'), 'job'), '/revert'); + const [jobName] = JSON.parse(jobVersion.get('job.id')); + + return this.ajax(url, 'POST', { + data: { + JobID: jobName, + JobVersion: jobVersion.number, + }, + }); + } +} diff --git a/ui/app/components/job-version.js b/ui/app/components/job-version.js index 25499648823..d3ba75a7510 100644 --- a/ui/app/components/job-version.js +++ b/ui/app/components/job-version.js @@ -1,6 +1,9 @@ import Component from '@ember/component'; import { action, computed } from '@ember/object'; import { classNames } from '@ember-decorators/component'; +import { inject as service } from '@ember/service'; +import { task } from 'ember-concurrency'; +import messageForError from 'nomad-ui/utils/message-from-adapter-error'; import classic from 'ember-classic-decorator'; const changeTypes = ['Added', 'Deleted', 'Edited']; @@ -14,6 +17,8 @@ export default class JobVersion extends Component { // Passes through to the job-diff component verbose = true; + @service router; + @computed('version.diff') get changeCount() { const diff = this.get('version.diff'); @@ -30,10 +35,47 @@ export default class JobVersion extends Component { ); } + @computed('version.{number,job.version}') + get isCurrent() { + return this.get('version.number') === this.get('version.job.version'); + } + @action toggleDiff() { this.toggleProperty('isOpen'); } + + @task(function*() { + try { + const versionBeforeReversion = this.get('version.job.version'); + + yield this.version.revertTo(); + yield this.version.job.reload(); + + const versionAfterReversion = this.get('version.job.version'); + + if (versionBeforeReversion === versionAfterReversion) { + this.handleError({ + level: 'warn', + title: 'Reversion Had No Effect', + description: 'Reverting to an identical older version doesn’t produce a new version', + }); + } else { + const job = this.get('version.job'); + + this.router.transitionTo('jobs.job', job.get('plainId'), { + queryParams: { namespace: job.get('namespace.name') }, + }); + } + } catch (e) { + this.handleError({ + level: 'danger', + title: 'Could Not Revert', + description: messageForError(e, 'revert'), + }); + } + }) + revertTo; } const flatten = (accumulator, array) => accumulator.concat(array); diff --git a/ui/app/components/two-step-button.js b/ui/app/components/two-step-button.js index 4bf934eadb4..70ff3401062 100644 --- a/ui/app/components/two-step-button.js +++ b/ui/app/components/two-step-button.js @@ -9,7 +9,7 @@ import classic from 'ember-classic-decorator'; @classic @classNames('two-step-button') -@classNameBindings('inlineText:has-inline-text') +@classNameBindings('inlineText:has-inline-text', 'fadingBackground:has-fading-background') export default class TwoStepButton extends Component { idleText = ''; cancelText = ''; diff --git a/ui/app/controllers/jobs/job/versions.js b/ui/app/controllers/jobs/job/versions.js index ca198917709..087100ea039 100644 --- a/ui/app/controllers/jobs/job/versions.js +++ b/ui/app/controllers/jobs/job/versions.js @@ -1,9 +1,33 @@ import Controller from '@ember/controller'; import WithNamespaceResetting from 'nomad-ui/mixins/with-namespace-resetting'; import { alias } from '@ember/object/computed'; +import { action, computed } from '@ember/object'; import classic from 'ember-classic-decorator'; +const alertClassFallback = 'is-info'; + +const errorLevelToAlertClass = { + danger: 'is-danger', + warn: 'is-warning', +}; + @classic export default class VersionsController extends Controller.extend(WithNamespaceResetting) { + error = null; + @alias('model') job; + + @computed('error.level') + get errorLevelClass() { + return errorLevelToAlertClass[this.get('error.level')] || alertClassFallback; + } + + onDismiss() { + this.set('error', null); + } + + @action + handleError(errorObject) { + this.set('error', errorObject); + } } diff --git a/ui/app/models/job-version.js b/ui/app/models/job-version.js index 155fb85a1ef..cc37430073e 100644 --- a/ui/app/models/job-version.js +++ b/ui/app/models/job-version.js @@ -7,4 +7,8 @@ export default class JobVersion extends Model { @attr('date') submitTime; @attr('number') number; @attr() diff; + + revertTo() { + return this.store.adapterFor('job-version').revertTo(this); + } } diff --git a/ui/app/routes/jobs/job/versions.js b/ui/app/routes/jobs/job/versions.js index c6ab5684bde..2923c321c77 100644 --- a/ui/app/routes/jobs/job/versions.js +++ b/ui/app/routes/jobs/job/versions.js @@ -1,6 +1,6 @@ import Route from '@ember/routing/route'; import { collect } from '@ember/object/computed'; -import { watchRelationship } from 'nomad-ui/utils/properties/watch'; +import { watchRecord, watchRelationship } from 'nomad-ui/utils/properties/watch'; import WithWatchers from 'nomad-ui/mixins/with-watchers'; export default class VersionsRoute extends Route.extend(WithWatchers) { @@ -11,10 +11,13 @@ export default class VersionsRoute extends Route.extend(WithWatchers) { startWatchers(controller, model) { if (model) { - controller.set('watcher', this.watchVersions.perform(model)); + controller.set('watcher', this.watch.perform(model)); + controller.set('watchVersions', this.watchVersions.perform(model)); } } + @watchRecord('job') watch; @watchRelationship('versions') watchVersions; - @collect('watchVersions') watchers; + + @collect('watch', 'watchVersions') watchers; } diff --git a/ui/app/styles/components/boxed-section.scss b/ui/app/styles/components/boxed-section.scss index f960bac5a87..bcebd868f41 100644 --- a/ui/app/styles/components/boxed-section.scss +++ b/ui/app/styles/components/boxed-section.scss @@ -19,6 +19,11 @@ margin-left: auto; } + .is-fixed-width { + display: inline-block; + width: 8em; + } + &.is-compact { padding: 0.75em; } diff --git a/ui/app/styles/components/two-step-button.scss b/ui/app/styles/components/two-step-button.scss index c4dfcbb2665..3a1aeb9e47d 100644 --- a/ui/app/styles/components/two-step-button.scss +++ b/ui/app/styles/components/two-step-button.scss @@ -14,11 +14,19 @@ } .confirmation-text { - position: static; - margin-right: 0.5ch; + position: absolute; + left: auto; + right: 0; + top: auto; + margin-right: 100%; } } + &.has-fading-background .confirmation-text { + padding: 0.5rem 0 0.5rem 4rem; + background: linear-gradient(to left, white, white 90%, transparent 100%); + } + .confirmation-text { position: absolute; left: 0; diff --git a/ui/app/templates/components/job-version.hbs b/ui/app/templates/components/job-version.hbs index 6aa4647a46a..fc219e964cd 100644 --- a/ui/app/templates/components/job-version.hbs +++ b/ui/app/templates/components/job-version.hbs @@ -8,11 +8,42 @@ Submitted {{format-ts this.version.submitTime}} - {{#if this.version.diff}} - - {{else}} - No Changes - {{/if}} +
+ {{#unless this.isCurrent}} + {{#if (can "run job")}} + + {{else}} + + {{/if}} + {{/unless}} + + {{#if this.version.diff}} + + {{else}} +
No Changes
+ {{/if}} +
{{#if this.isOpen}}
diff --git a/ui/app/templates/components/job-versions-stream.hbs b/ui/app/templates/components/job-versions-stream.hbs index c05caa48a61..0528fc292c6 100644 --- a/ui/app/templates/components/job-versions-stream.hbs +++ b/ui/app/templates/components/job-versions-stream.hbs @@ -5,6 +5,6 @@ {{/if}}
  • - +
  • {{/each}} diff --git a/ui/app/templates/jobs/job/versions.hbs b/ui/app/templates/jobs/job/versions.hbs index dc643705345..d6829bd849c 100644 --- a/ui/app/templates/jobs/job/versions.hbs +++ b/ui/app/templates/jobs/job/versions.hbs @@ -1,5 +1,19 @@ {{page-title "Job " this.job.name " versions"}}
    - + {{#if this.error}} +
    +
    +
    +

    {{this.error.title}}

    +

    {{this.error.description}}

    +
    +
    + +
    +
    +
    + {{/if}} + +
    diff --git a/ui/mirage/config.js b/ui/mirage/config.js index f739a353b69..1641fca8c6c 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -180,6 +180,15 @@ export default function() { return okEmpty(); }); + this.post('/job/:id/revert', function({ jobs }, { requestBody }) { + const { JobID, JobVersion } = JSON.parse(requestBody); + const job = jobs.find(JobID); + job.version = JobVersion; + job.save(); + + return okEmpty(); + }); + this.post('/job/:id/scale', function({ jobs }, { params }) { return this.serialize(jobs.find(params.id)); }); diff --git a/ui/tests/acceptance/job-versions-test.js b/ui/tests/acceptance/job-versions-test.js index cd2c224d602..d9b973e49cd 100644 --- a/ui/tests/acceptance/job-versions-test.js +++ b/ui/tests/acceptance/job-versions-test.js @@ -4,9 +4,11 @@ import { setupApplicationTest } from 'ember-qunit'; import { setupMirage } from 'ember-cli-mirage/test-support'; import a11yAudit from 'nomad-ui/tests/helpers/a11y-audit'; import Versions from 'nomad-ui/tests/pages/jobs/job/versions'; +import Layout from 'nomad-ui/tests/pages/layout'; import moment from 'moment'; let job; +let namespace; let versions; module('Acceptance | job versions', function(hooks) { @@ -14,10 +16,16 @@ module('Acceptance | job versions', function(hooks) { setupMirage(hooks); hooks.beforeEach(async function() { - job = server.create('job', { createAllocations: false }); + server.create('namespace'); + namespace = server.create('namespace'); + + job = server.create('job', { namespaceId: namespace.id, createAllocations: false }); versions = server.db.jobVersions.where({ jobId: job.id }); - await Versions.visit({ id: job.id }); + const managementToken = server.create('token'); + window.localStorage.nomadTokenSecret = managementToken.secretId; + + await Versions.visit({ id: job.id, namespace: namespace.id }); }); test('it passes an accessibility audit', async function(assert) { @@ -41,6 +49,79 @@ module('Acceptance | job versions', function(hooks) { assert.equal(versionRow.submitTime, formattedSubmitTime, 'Submit time'); }); + test('all versions but the current one have a button to revert to that version', async function(assert) { + let versionRowToRevertTo; + + Versions.versions.forEach((versionRow) => { + if (versionRow.number === job.version) { + assert.ok(versionRow.revertToButton.isHidden); + } else { + assert.ok(versionRow.revertToButton.isPresent); + + versionRowToRevertTo = versionRow; + } + }); + + if (versionRowToRevertTo) { + const versionNumberRevertingTo = versionRowToRevertTo.number; + await versionRowToRevertTo.revertToButton.idle(); + await versionRowToRevertTo.revertToButton.confirm(); + + const revertRequest = this.server.pretender.handledRequests.find(request => request.url.includes('revert')); + + assert.equal(revertRequest.url, `/v1/job/${job.id}/revert?namespace=${namespace.id}`); + + assert.deepEqual(JSON.parse(revertRequest.requestBody), { + JobID: job.id, + JobVersion: versionNumberRevertingTo, + }); + + assert.equal(currentURL(), `/jobs/${job.id}?namespace=${namespace.id}`); + } + }); + + test('when reversion fails, the error message from the API is piped through to the alert', async function(assert) { + const versionRowToRevertTo = Versions.versions.filter(versionRow => versionRow.revertToButton.isPresent)[0]; + + if (versionRowToRevertTo) { + const message = 'A plaintext error message'; + server.pretender.post('/v1/job/:id/revert', () => [500, {}, message]); + + await versionRowToRevertTo.revertToButton.idle(); + await versionRowToRevertTo.revertToButton.confirm(); + + assert.ok(Layout.inlineError.isShown); + assert.ok(Layout.inlineError.isDanger); + assert.ok(Layout.inlineError.title.includes('Could Not Revert')); + assert.equal(Layout.inlineError.message, message); + + await Layout.inlineError.dismiss(); + + assert.notOk(Layout.inlineError.isShown); + } else { + assert.expect(0); + } + }); + + test('when reversion has no effect, the error message explains', async function(assert) { + const versionRowToRevertTo = Versions.versions.filter(versionRow => versionRow.revertToButton.isPresent)[0]; + + if (versionRowToRevertTo) { + // The default Mirage implementation updates the job version as passed in, this does nothing + server.pretender.post('/v1/job/:id/revert', () => [200, {}, '']); + + await versionRowToRevertTo.revertToButton.idle(); + await versionRowToRevertTo.revertToButton.confirm(); + + assert.ok(Layout.inlineError.isShown); + assert.ok(Layout.inlineError.isWarning); + assert.ok(Layout.inlineError.title.includes('Reversion Had No Effect')); + assert.equal(Layout.inlineError.message, 'Reverting to an identical older version doesn’t produce a new version'); + } else { + assert.expect(0); + } + }); + test('when the job for the versions is not found, an error message is shown, but the URL persists', async function(assert) { await Versions.visit({ id: 'not-a-real-job' }); @@ -56,3 +137,31 @@ module('Acceptance | job versions', function(hooks) { assert.equal(Versions.error.title, 'Not Found', 'Error message is for 404'); }); }); + +module('Acceptance | job versions (with client token)', function(hooks) { + setupApplicationTest(hooks); + setupMirage(hooks); + + hooks.beforeEach(async function() { + job = server.create('job', { createAllocations: false }); + versions = server.db.jobVersions.where({ jobId: job.id }); + + server.create('token'); + const clientToken = server.create('token'); + window.localStorage.nomadTokenSecret = clientToken.secretId; + + await Versions.visit({ id: job.id }); + }); + + test('reversion buttons are disabled when the token lacks permissions', async function(assert) { + const versionRowWithReversion = Versions.versions.filter(versionRow => versionRow.revertToButton.isPresent)[0]; + + if (versionRowWithReversion) { + assert.ok(versionRowWithReversion.revertToButtonIsDisabled); + } else { + assert.expect(0); + } + + window.localStorage.clear(); + }); +}); diff --git a/ui/tests/pages/jobs/job/versions.js b/ui/tests/pages/jobs/job/versions.js index 1b58b9557f7..416406f00e9 100644 --- a/ui/tests/pages/jobs/job/versions.js +++ b/ui/tests/pages/jobs/job/versions.js @@ -1,5 +1,7 @@ -import { create, collection, text, visitable } from 'ember-cli-page-object'; +import { attribute, create, collection, text, visitable } from 'ember-cli-page-object'; +import { getter } from 'ember-cli-page-object/macros'; +import twoStepButton from 'nomad-ui/tests/pages/components/two-step-button'; import error from 'nomad-ui/tests/pages/components/error'; export default create({ @@ -9,6 +11,13 @@ export default create({ text: text(), stability: text('[data-test-version-stability]'), submitTime: text('[data-test-version-submit-time]'), + + revertToButton: twoStepButton('[data-test-revert-to]'), + revertToButtonIsDisabled: attribute('disabled', '[data-test-revert-to]'), + + number: getter(function() { + return parseInt(this.text.match(/#(\d+)/)[1]); + }), }), error: error(), diff --git a/ui/tests/pages/layout.js b/ui/tests/pages/layout.js index cb1fcbbe8c2..c2a34c343f0 100644 --- a/ui/tests/pages/layout.js +++ b/ui/tests/pages/layout.js @@ -99,4 +99,14 @@ export default create({ title: text('[data-test-error-title]'), message: text('[data-test-error-message]'), }, + + inlineError: { + isShown: isPresent('[data-test-inline-error]'), + title: text('[data-test-inline-error-title]'), + message: text('[data-test-inline-error-body]'), + dismiss: clickable('[data-test-inline-error-close]'), + + isDanger: hasClass('is-danger', '[data-test-inline-error]'), + isWarning: hasClass('is-warning', '[data-test-inline-error]'), + }, });