kemko/nomad
1
0
Fork 0
mirror of https://github.com/kemko/nomad.git synced 2026-01-01 16:05:42 +03:00
Code Issues Packages Projects Releases Wiki Activity

[ui] "Clone and Edit" functionality for versions (#24168)

Browse Source 
* Edit from Version functionality

* Reworked as Clone and Revert

* Change name warning for cloning as new job, version 0 checking, and erroring on sourceless clone

* If you try to plan a new version of a job with a different name of the one you're editing, suggest they might want to run a new one instead

* A few code comments and log cleanup

* Scaffolding new acceptance tests

* A whack of fun new tests

* Unit test for version number url passing on fetchRawDef

* Bit of cleanup

* fetchRawDefinition gets version support at adapter layer

* Handle spec-not-available-but-definition-is for clone-as-new-job
This commit is contained in:
Phil Renaud
2024-11-29 16:12:56 -05:00
committed by GitHub
parent 261359fba7
commit de96c3498b
12 changed files with 446 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
ui/app/adapters/job.js
View File

@@ -20,16 +20,48 @@ export default class JobAdapter extends WatchableNamespaceIDs {
summary: '/summary',
};
fetchRawDefinition(job) {
const url = this.urlForFindRecord(job.get('id'), 'job');
return this.ajax(url, 'GET');
/**
* Gets the JSON definition of a job.
* Prior to Nomad 1.6, this was the only way to get job definition data.
* Now, this is included as a stringified JSON object when fetching raw specification (under .Source).
* This method is still important for backwards compatibility with older job versions, as well as a fallback
* for when fetching raw specification fails.
* @param {import('../models/job').default} job
* @param {number} version
*/
async fetchRawDefinition(job, version) {
if (version == null) {
const url = this.urlForFindRecord(job.get('id'), 'job');
return this.ajax(url, 'GET');
}
// For specific versions, we need to fetch from versions endpoint,
// and then find the specified version info from the response.
const versionsUrl = addToPath(
this.urlForFindRecord(job.get('id'), 'job', null, 'versions')
);
const response = await this.ajax(versionsUrl, 'GET');
const versionInfo = response.Versions.find((v) => v.Version === version);
if (!versionInfo) {
throw new Error(`Version ${version} not found`);
}
return versionInfo;
}
fetchRawSpecification(job) {
/**
* Gets submission info for a job, including (if available) the raw HCL or JSON spec used to run it,
* including variable flags and literals.
* @param {import('../models/job').default} job
* @param {number} version
*/
fetchRawSpecification(job, version) {
const url = addToPath(
this.urlForFindRecord(job.get('id'), 'job', null, 'submission'),
'',
'version=' + job.get('version')
'version=' + (version || job.get('version'))
);
return this.ajax(url, 'GET');
}

59
ui/app/components/job-version.js
View File

@@ -3,6 +3,8 @@
* SPDX-License-Identifier: BUSL-1.1
*/
// @ts-check
import Component from '@glimmer/component';
import { action, computed } from '@ember/object';
import { alias } from '@ember/object/computed';
@@ -80,6 +82,11 @@ export default class JobVersion extends Component {
this.isOpen = !this.isOpen;
}
/**
* @type {'idle' | 'confirming'}
*/
@tracked cloneButtonStatus = 'idle';
@task(function* () {
try {
const versionBeforeReversion = this.version.get('job.version');
@@ -108,6 +115,58 @@ export default class JobVersion extends Component {
})
revertTo;
@action async cloneAsNewVersion() {
try {
this.router.transitionTo(
'jobs.job.definition',
this.version.get('job.idWithNamespace'),
{
queryParams: {
isEditing: true,
version: this.version.number,
},
}
);
} catch (e) {
this.args.handleError({
level: 'danger',
title: 'Could Not Edit from Version',
});
}
}
@action async cloneAsNewJob() {
const job = await this.version.get('job');
try {
const specification = await job.fetchRawSpecification(
this.version.number
);
this.router.transitionTo('jobs.run', {
queryParams: {
sourceString: specification.Source,
},
});
return;
} catch (specError) {
try {
// If submission info is not available, try to fetch the raw definition
const definition = await job.fetchRawDefinition(this.version.number);
this.router.transitionTo('jobs.run', {
queryParams: {
sourceString: JSON.stringify(definition, null, 2),
},
});
} catch (defError) {
// Both methods failed, show error
this.args.handleError({
level: 'danger',
title: 'Could Not Clone as New Job',
description: messageForError(defError),
});
}
}
}
@action
handleKeydown(event) {
if (event.key === 'Escape') {

2
ui/app/controllers/jobs/run/index.js
View File

@@ -11,7 +11,7 @@ import { inject as service } from '@ember/service';
export default class RunController extends Controller {
@service router;
queryParams = ['template'];
queryParams = ['template', 'sourceString', 'disregardNameWarning'];
@action
handleSaveAsTemplate() {

8
ui/app/models/job.js
View File

@@ -537,12 +537,12 @@ export default class Job extends Model {
return undefined;
}
fetchRawDefinition() {
return this.store.adapterFor('job').fetchRawDefinition(this);
fetchRawDefinition(version) {
return this.store.adapterFor('job').fetchRawDefinition(this, version);
}
fetchRawSpecification() {
return this.store.adapterFor('job').fetchRawSpecification(this);
fetchRawSpecification(version) {
return this.store.adapterFor('job').fetchRawSpecification(this, version);
}
forcePeriodic() {

34
ui/app/routes/jobs/job/definition.js
View File

@@ -5,29 +5,55 @@
// @ts-check
import Route from '@ember/routing/route';
import { inject as service } from '@ember/service';
/**
* Route for fetching and displaying a job's definition and specification.
*/
export default class DefinitionRoute extends Route {
@service notifications;
queryParams = {
version: {
refreshModel: true,
},
};
/**
* Fetch the job's definition, specification, and variables from the API.
*
* @returns {Promise<Object>} A promise that resolves to an object containing the job, definition, format,
* specification, variableFlags, and variableLiteral.
*/
async model() {
async model({ version }) {
version = version ? +version : undefined; // query parameter is a string; convert to number
/** @type {import('../../../models/job').default} */
const job = this.modelFor('jobs.job');
if (!job) return;
const definition = await job.fetchRawDefinition();
let definition;
if (version !== undefined) {
// Not (!version) because version can be 0
try {
definition = await job.fetchRawDefinition(version);
} catch (e) {
console.error('error fetching job version definition', e);
this.notifications.add({
title: 'Error Fetching Job Version Definition',
message: `There was an error fetching the versions for this job: ${e.message}`,
color: 'critical',
});
}
} else {
definition = await job.fetchRawDefinition();
}
let format = 'json'; // default to json in network request errors
let specification;
let variableFlags;
let variableLiteral;
try {
const specificationResponse = await job.fetchRawSpecification();
const specificationResponse = await job.fetchRawSpecification(version);
specification = specificationResponse?.Source ?? null;
variableFlags = specificationResponse?.VariableFlags ?? null;
variableLiteral = specificationResponse?.Variables ?? null;

11
ui/app/routes/jobs/run/index.js
View File

@@ -21,6 +21,9 @@ export default class JobsRunIndexRoute extends Route {
template: {
refreshModel: true,
},
sourceString: {
refreshModel: true,
},
};
beforeModel(transition) {
@@ -33,7 +36,7 @@ export default class JobsRunIndexRoute extends Route {
}
}
async model({ template }) {
async model({ template, sourceString }) {
try {
// When jobs are created with a namespace attribute, it is verified against
// available namespaces to prevent redirecting to a non-existent namespace.
@@ -45,6 +48,10 @@ export default class JobsRunIndexRoute extends Route {
return this.store.createRecord('job', {
_newDefinition: templateRecord.items.template,
});
} else if (sourceString) {
return this.store.createRecord('job', {
_newDefinition: sourceString,
});
} else {
return this.store.createRecord('job');
}
@@ -72,6 +79,8 @@ export default class JobsRunIndexRoute extends Route {
if (isExiting) {
controller.model?.deleteRecord();
controller.set('template', null);
controller.set('sourceString', null);
controller.set('disregardNameWarning', null);
}
}
}

4
ui/app/templates/components/job-editor/alert.hbs
View File

@@ -8,6 +8,9 @@
<Hds::Alert @type="inline" @color="critical" data-test-error={{@data.error.type}} as |A|>
<A.Title data-test-error-title>{{conditionally-capitalize @data.error.type true}}</A.Title>
<A.Description data-test-error-message>{{@data.error.message}}</A.Description>
{{#if (eq @data.error.message "Job ID does not match")}}
<A.Button @text="Run as a new job instead" @color="primary" @route="jobs.run" @query={{hash sourceString=@data.job._newDefinition disregardNameWarning=true}} />
{{/if}}
</Hds::Alert>
{{/if}}
{{#if (and (eq @data.stage "read") @data.hasVariables (eq @data.view "job-spec"))}}
@@ -31,4 +34,3 @@
</Hds::Alert>
{{/if}}
</div>

67
ui/app/templates/components/job-version.hbs
View File

@@ -4,7 +4,7 @@
~}}
{{did-update this.versionsDidUpdate this.diff}}
<section class="job-version">
<section class="job-version" data-test-job-version={{this.version.number}}>
<div class="boxed-section {{if this.version.versionTag "tagged"}}" data-test-tagged-version={{if this.version.versionTag "true" "false"}}>
<header class="boxed-section-head is-light inline-definitions">
Version #{{this.version.number}}
@@ -81,20 +81,57 @@
<div class="version-options">
{{#unless this.isCurrent}}
{{#if (can "run job" namespace=this.version.job.namespace)}}
<TwoStepButton
data-test-revert-to
@classes={{hash
idleButton="is-warning is-outlined"
confirmButton="is-warning"}}
@fadingBackground={{true}}
@idleText="Revert Version"
@cancelText="Cancel"
@confirmText="Yes, Revert Version"
@confirmationMessage="Are you sure you want to revert to this version?"
@inlineText={{true}}
@size="small"
@awaitingConfirmation={{this.revertTo.isRunning}}
@onConfirm={{perform this.revertTo}} />
{{#if (eq this.cloneButtonStatus 'idle')}}
<Hds::Button
data-test-clone-and-edit
@text="Clone and Edit"
@color="secondary"
@size="small"
@isInline={{true}}
{{on "click" (action (mut this.cloneButtonStatus) "confirming")}}
/>
<TwoStepButton
data-test-revert-to
@classes={{hash
idleButton="is-warning is-outlined"
confirmButton="is-warning"}}
@fadingBackground={{true}}
@idleText="Revert Version"
@cancelText="Cancel"
@confirmText="Yes, Revert Version"
@confirmationMessage="Are you sure you want to revert to this version?"
@inlineText={{true}}
@size="small"
@awaitingConfirmation={{this.revertTo.isRunning}}
@onConfirm={{perform this.revertTo}}
/>
{{else if (eq this.cloneButtonStatus 'confirming')}}
<Hds::Button
data-test-cancel-clone
@text="Cancel"
@color="secondary"
@size="small"
@isInline={{true}}
{{on "click" (action (mut this.cloneButtonStatus) "idle")}}
/>
<Hds::Button
data-test-clone-as-new-version
@text="Clone as New Version of {{this.version.job.name}}"
@color="secondary"
@size="small"
@isInline={{true}}
{{on "click" (action this.cloneAsNewVersion)}}
/>
<Hds::Button
data-test-clone-as-new-job
@text="Clone as New Job"
@color="secondary"
@size="small"
@isInline={{true}}
{{on "click" (action this.cloneAsNewJob)}}
/>
{{/if}}
{{else}}
<Hds::Button
data-test-revert-to

8
ui/app/templates/jobs/run/index.hbs
View File

@@ -6,5 +6,11 @@
<Breadcrumb @crumb={{hash label="Run" args=(array "jobs.run")}} />
{{page-title "Run a job"}}
<section class="section">
{{#if (and this.sourceString (not this.disregardNameWarning))}}
<Hds::Alert @type="inline" @color="warning" data-test-job-name-warning as |A|>
<A.Title>Don't forget to change the job name!</A.Title>
<A.Description>Since you're cloning a job version's source as a new job, you'll want to change the job name. Otherwise, this will appear as a new version of the original job, rather than a new job.</A.Description>
</Hds::Alert>
{{/if}}
<JobEditor @job={{this.model}} @context="new" @onSubmit={{action this.onSubmit}} @handleSaveAsTemplate={{this.handleSaveAsTemplate}} data-test-job-editor />
</section>
</section>

160
ui/tests/acceptance/job-versions-test.js
View File

@@ -322,6 +322,166 @@ module('Acceptance | job versions', function (hooks) {
});
});
// Module for Clone and Edit
module('Acceptance | job versions (clone and edit)', function (hooks) {
setupApplicationTest(hooks);
setupMirage(hooks);
hooks.beforeEach(async function () {
server.create('node-pool');
namespace = server.create('namespace');
const managementToken = server.create('token');
window.localStorage.nomadTokenSecret = managementToken.secretId;
job = server.create('job', {
createAllocations: false,
version: 99,
namespaceId: namespace.id,
});
// remove auto-created versions and create 3 of them, one with a tag
server.db.jobVersions.remove();
server.create('job-version', {
job,
version: 99,
submitTime: 1731101785761339000,
});
server.create('job-version', {
job,
version: 98,
submitTime: 1731101685761339000,
versionTag: {
Name: 'test-tag',
Description: 'A tag with a brief description',
},
});
server.create('job-version', {
job,
version: 0,
submitTime: 1731101585761339000,
});
await Versions.visit({ id: job.id });
});
test('Clone and edit buttons are shown', async function (assert) {
assert.dom('.job-version').exists({ count: 3 });
assert
.dom('[data-test-clone-and-edit]')
.exists(
{ count: 2 },
'Current job version doesnt have clone or revert buttons'
);
const versionBlock = '[data-test-job-version="98"]';
assert
.dom(`${versionBlock} [data-test-clone-as-new-version]`)
.doesNotExist(
'Confirmation-stage clone-as-new-version button doesnt exist on initial load'
);
assert
.dom(`${versionBlock} [data-test-clone-as-new-job]`)
.doesNotExist(
'Confirmation-stage clone-as-new-job button doesnt exist on initial load'
);
await click(`${versionBlock} [data-test-clone-and-edit]`);
assert
.dom(`${versionBlock} [data-test-clone-as-new-version]`)
.exists(
'Confirmation-stage clone-as-new-version button exists after clicking clone and edit'
);
assert
.dom(`${versionBlock} [data-test-clone-as-new-job]`)
.exists(
'Confirmation-stage clone-as-new-job button exists after clicking clone and edit'
);
assert
.dom(`${versionBlock} [data-test-revert-to]`)
.doesNotExist('Revert button is hidden when Clone buttons are expanded');
assert
.dom('[data-test-job-version="0"] [data-test-revert-to]')
.exists('Revert button is visible for other versions');
await click(`${versionBlock} [data-test-cancel-clone]`);
assert
.dom(`${versionBlock} [data-test-clone-as-new-version]`)
.doesNotExist(
'Confirmation-stage clone-as-new-version button doesnt exist after clicking cancel'
);
});
test('Clone as new version', async function (assert) {
const versionBlock = '[data-test-job-version="98"]';
await click(`${versionBlock} [data-test-clone-and-edit]`);
await click(`${versionBlock} [data-test-clone-as-new-version]`);
assert.equal(
currentURL(),
`/jobs/${job.id}@${namespace.id}/definition?isEditing=true&version=98&view=job-spec`,
'Taken to the definition page in edit mode'
);
await percySnapshot(assert);
});
test('Clone as new version when version is 0', async function (assert) {
const versionBlock = '[data-test-job-version="0"]';
await click(`${versionBlock} [data-test-clone-and-edit]`);
await click(`${versionBlock} [data-test-clone-as-new-version]`);
assert.equal(
currentURL(),
`/jobs/${job.id}@${namespace.id}/definition?isEditing=true&version=0&view=job-spec`,
'Taken to the definition page in edit mode'
);
});
test('Clone as a new version when no submission info is available', async function (assert) {
server.pretender.get('/v1/job/:id/submission', () => [500, {}, '']);
const versionBlock = '[data-test-job-version="98"]';
await click(`${versionBlock} [data-test-clone-and-edit]`);
await click(`${versionBlock} [data-test-clone-as-new-version]`);
assert.equal(
currentURL(),
`/jobs/${job.id}@${namespace.id}/definition?isEditing=true&version=98&view=full-definition`,
'Taken to the definition page in edit mode'
);
assert.dom('[data-test-json-warning]').exists();
await percySnapshot(assert);
});
test('Clone as a new job', async function (assert) {
const testString =
'Test string that should appear in my sourceString url param';
server.pretender.get('/v1/job/:id/submission', () => [
200,
{},
JSON.stringify({
Source: testString,
}),
]);
const versionBlock = '[data-test-job-version="98"]';
await click(`${versionBlock} [data-test-clone-and-edit]`);
await click(`${versionBlock} [data-test-clone-as-new-job]`);
assert.equal(
currentURL(),
`/jobs/run?sourceString=${encodeURIComponent(testString)}`,
'Taken to the new job page'
);
assert.dom('[data-test-job-name-warning]').exists();
});
});
module('Acceptance | job versions (with client token)', function (hooks) {
setupApplicationTest(hooks);
setupMirage(hooks);

80
ui/tests/unit/adapters/job-test.js
View File

@@ -501,6 +501,65 @@ module('Unit | Adapter | Job', function (hooks) {
assert.equal(request.method, 'GET');
});
test('fetchRawDefinition handles version requests', async function (assert) {
assert.expect(5);
const adapter = this.owner.lookup('adapter:job');
const job = {
get: sinon.stub(),
};
job.get.withArgs('id').returns('["job-id"]');
const mockVersionResponse = {
Versions: [
{ Version: 1, JobID: 'job-id', JobModifyIndex: 100 },
{ Version: 2, JobID: 'job-id', JobModifyIndex: 200 },
],
};
// Stub ajax to return mock version data
const ajaxStub = sinon.stub(adapter, 'ajax');
ajaxStub
.withArgs('/v1/job/job-id/versions', 'GET')
.resolves(mockVersionResponse);
// Test fetching specific version
const result = await adapter.fetchRawDefinition(job, 2);
assert.equal(result.Version, 2, 'Returns correct version');
assert.equal(result.JobModifyIndex, 200, 'Returns full version info');
// Test version not found
try {
await adapter.fetchRawDefinition(job, 999);
assert.ok(false, 'Should have thrown error');
} catch (e) {
assert.equal(
e.message,
'Version 999 not found',
'Throws appropriate error'
);
}
// Test no version specified (current version)
ajaxStub
.withArgs('/v1/job/job-id', 'GET')
.resolves({ ID: 'job-id', Version: 2 });
const currentResult = await adapter.fetchRawDefinition(job);
assert.equal(
ajaxStub.lastCall.args[0],
'/v1/job/job-id',
'URL has no version query param'
);
assert.equal(
currentResult.Version,
2,
'Returns current version when no version specified'
);
});
test('forcePeriodic requests include the activeRegion', async function (assert) {
const region = 'region-2';
const job = await this.initializeWithJob({ region });
@@ -681,6 +740,27 @@ module('Unit | Adapter | Job', function (hooks) {
'/v1/job/job-id/submission?namespace=zoey&version=job-version'
);
});
test('Requests for specific versions include the queryParam', async function (assert) {
const adapter = this.owner.lookup('adapter:job');
const job = {
get: sinon.stub(),
};
job.get.withArgs('id').returns('["job-id"]');
job.get.withArgs('version').returns(100);
// Stub the ajax method to avoid making real API calls
sinon.stub(adapter, 'ajax').callsFake(() => resolve({}));
await adapter.fetchRawSpecification(job, 99);
assert.ok(adapter.ajax.calledOnce, 'The ajax method is called once');
assert.equal(
adapter.ajax.args[0][0],
'/v1/job/job-id/submission?version=99',
'it includes the version query param'
);
assert.equal(adapter.ajax.args[0][1], 'GET');
});
});
});