diff --git a/ui/app/adapters/token.js b/ui/app/adapters/token.js index 5ac2d9cce92..c6c768511f2 100644 --- a/ui/app/adapters/token.js +++ b/ui/app/adapters/token.js @@ -47,13 +47,18 @@ export default class TokenAdapter extends ApplicationAdapter { return `${this.buildURL()}/${singularize(modelName)}/${identifier}`; } - async findSelf() { + async findSelf(regionOverride = null) { // the application adapter automatically adds the region parameter to all requests, // but only if the /regions endpoint has been resolved first. Since this request is async, // we can ensure that the regions are loaded before making the token/self request. await this.system.regions; - const response = await this.ajax(`${this.buildURL()}/token/self`, 'GET'); + const options = regionOverride ? { regionOverride } : {}; + const response = await this.ajax( + `${this.buildURL()}/token/self`, + 'GET', + options + ); const normalized = this.store.normalize('token', response); const tokenRecord = this.store.push(normalized); return tokenRecord; diff --git a/ui/app/components/region-switcher.js b/ui/app/components/region-switcher.js index 7df4e002244..5e81bacb645 100644 --- a/ui/app/components/region-switcher.js +++ b/ui/app/components/region-switcher.js @@ -22,10 +22,8 @@ export default class RegionSwitcher extends Component { @action async gotoRegion(region) { - // Note: redundant but as long as we're using PowerSelect, the implicit set('activeRegion') - // is not something we can await, so we do it explicitly here. - this.system.set('activeRegion', region); - await this.get('token.fetchSelfTokenAndPolicies').perform().catch(); + // Fetch token for the new region before transitioning + await this.get('token.fetchSelfTokenAndPolicies').perform(region).catch(); this.router.transitionTo({ queryParams: { region } }); } diff --git a/ui/app/models/task.js b/ui/app/models/task.js index e804b9f2ab7..ca9268fc115 100644 --- a/ui/app/models/task.js +++ b/ui/app/models/task.js @@ -10,7 +10,7 @@ import { fragmentArray, fragmentOwner, } from 'ember-data-model-fragments/attributes'; -import { computed } from '@ember/object'; +import { computed, notifyPropertyChange } from '@ember/object'; export default class Task extends Fragment { @fragmentOwner() taskGroup; @@ -61,6 +61,19 @@ export default class Task extends Fragment { @fragmentArray('volume-mount', { defaultValue: () => [] }) volumeMounts; + _pathLinkedVariableJobID() { + let jobID = this._job.plainId; + const parentID = this._job.belongsTo('parent').id() + ? JSON.parse(this._job.belongsTo('parent').id())[0] + : null; + + if (parentID) { + jobID = parentID; + } + + return jobID; + } + async _fetchParentJob() { let job = this.store.peekRecord('job', this.taskGroup.job.id); if (!job) { @@ -69,6 +82,9 @@ export default class Task extends Fragment { }); } this._job = job; + await this._job.variables; + // pathLinkedVariable is consumed by a sync template condition, so notify after async load. + notifyPropertyChange(this, 'pathLinkedVariable'); } get pathLinkedVariable() { @@ -76,10 +92,7 @@ export default class Task extends Fragment { this._fetchParentJob(); return null; } else { - let jobID = this._job.plainId; - if (this._job.parent.get('plainId')) { - jobID = this._job.parent.get('plainId'); - } + let jobID = this._pathLinkedVariableJobID(); return this._job.variables?.findBy( 'path', `nomad/jobs/${jobID}/${this.taskGroup.name}/${this.name}`, @@ -93,14 +106,7 @@ export default class Task extends Fragment { await this._fetchParentJob(); } await this._job.variables; - let jobID = this._job.plainId; - // not getting plainID because we dont know the resolution status of the task's job's parent yet - let parentID = this._job.belongsTo('parent').id() - ? JSON.parse(this._job.belongsTo('parent').id())[0] - : null; - if (parentID) { - jobID = parentID; - } + let jobID = this._pathLinkedVariableJobID(); return await this._job.variables?.findBy( 'path', `nomad/jobs/${jobID}/${this.taskGroup.name}/${this.name}`, diff --git a/ui/app/routes/allocations/allocation.js b/ui/app/routes/allocations/allocation.js index d165da34cdd..c6cf62eb14f 100644 --- a/ui/app/routes/allocations/allocation.js +++ b/ui/app/routes/allocations/allocation.js @@ -54,7 +54,7 @@ export default class AllocationRoute extends Route.extend(WithWatchers) { await allocation.reload(); } const jobId = allocation.belongsTo('job').id(); - await this.store.findRecord('job', jobId); + await this.store.findRecord('job', jobId, { reload: true }); // Force fragment-array materialization before first render so Ember does // not lazily write `services` during template computation. diff --git a/ui/app/routes/application.js b/ui/app/routes/application.js index 0a1f45883f7..b0b4ad04e3c 100644 --- a/ui/app/routes/application.js +++ b/ui/app/routes/application.js @@ -17,7 +17,6 @@ import { handleRouteRedirects } from '../utils/route-redirector'; export default class ApplicationRoute extends Route { @service config; @service system; - @service store; @service token; @service router; @@ -89,16 +88,18 @@ export default class ApplicationRoute extends Route { const queryParam = transition.to.queryParams.region; const defaultRegion = this.get('system.defaultRegion.region'); const currentRegion = this.get('system.activeRegion') || defaultRegion; + const nextRegion = queryParam || defaultRegion; + const regionChanged = nextRegion !== currentRegion; - // Only reset the store if the region actually changed - if ( - (queryParam && queryParam !== currentRegion) || - (!queryParam && currentRegion !== defaultRegion) - ) { - this.store.unloadAll(); - } + // Update the active region - the adapter will use this for all API requests + this.set('system.activeRegion', nextRegion); - this.set('system.activeRegion', queryParam || defaultRegion); + // Refresh for child routes if region changes and only if query-param-only transitions + if (regionChanged && transition.queryParamsOnly) { + later(() => { + this.refresh(); + }, 0); + } return promises; } diff --git a/ui/app/routes/jobs/job.js b/ui/app/routes/jobs/job.js index 8db3acfb600..0126aa9fd98 100644 --- a/ui/app/routes/jobs/job.js +++ b/ui/app/routes/jobs/job.js @@ -42,8 +42,8 @@ export default class JobRoute extends Route.extend(WithWatchers) { .findRecord('job', fullId, { reload: true }) .then((job) => { const relatedModelsQueries = [ - job.get('allocations'), - job.get('evaluations'), + job.hasMany('allocations').reload(), + job.hasMany('evaluations').reload(), this.store.findAll('namespace'), ]; diff --git a/ui/app/services/token.js b/ui/app/services/token.js index 8c9337c66bb..26aa81d1d40 100644 --- a/ui/app/services/token.js +++ b/ui/app/services/token.js @@ -60,10 +60,10 @@ export default class TokenService extends Service { } } - @task(function* () { + @task(function* (regionOverride = null) { const TokenAdapter = getOwner(this).lookup('adapter:token'); try { - var token = yield TokenAdapter.findSelf(); + var token = yield TokenAdapter.findSelf(regionOverride); if (token.accessor === 'acls-disabled') { this.set('aclEnabled', false); return null; @@ -122,8 +122,8 @@ export default class TokenService extends Service { @alias('fetchSelfTokenPolicies.lastSuccessful.value') selfTokenPolicies; - @task(function* () { - yield this.fetchSelfToken.perform(); + @task(function* (regionOverride = null) { + yield this.fetchSelfToken.perform(regionOverride); this.kickoffTokenTTLMonitoring(); if (this.aclEnabled) { yield this.fetchSelfTokenPolicies.perform(); diff --git a/ui/tests/acceptance/job-definition-test.js b/ui/tests/acceptance/job-definition-test.js index 0199f1f2cb7..c101e3bb79a 100644 --- a/ui/tests/acceptance/job-definition-test.js +++ b/ui/tests/acceptance/job-definition-test.js @@ -13,7 +13,6 @@ import { setupMirage } from 'ember-cli-mirage/test-support'; import a11yAudit from 'nomad-ui/tests/helpers/a11y-audit'; import setupCodeMirror from 'nomad-ui/tests/helpers/codemirror'; import Definition from 'nomad-ui/tests/pages/jobs/job/definition'; -import { JOB_JSON } from 'nomad-ui/tests/utils/generate-raw-json-job'; let job; @@ -152,6 +151,7 @@ module('Acceptance | job definition | full specification', function (hooks) { }); test('it allows users to select between full specification and JSON definition', async function (assert) { + assert.expect(7); const specification_response = { Format: 'hcl2', JobID: 'example', @@ -163,7 +163,7 @@ module('Acceptance | job definition | full specification', function (hooks) { Variables: '', Version: 0, }; - this.server.get('/job/:id', () => JOB_JSON); + this.server.get('/job/:id/submission', () => specification_response); await Definition.visit({ id: job.id }); @@ -171,9 +171,9 @@ module('Acceptance | job definition | full specification', function (hooks) { assert .dom('[data-test-select="job-spec"]') - .exists('A select button exists and defaults to full definition'); + .exists('A select button exists and defaults to job spec'); let codeMirror = this.getCodeMirrorInstance(); - assert.deepEqual( + assert.equal( codeMirror.getValue(), specification_response.Source, 'Shows the full definition as written by the user', @@ -181,6 +181,31 @@ module('Acceptance | job definition | full specification', function (hooks) { await click('[data-test-select-full]'); codeMirror = this.getCodeMirrorInstance(); - assert.propContains(JSON.parse(codeMirror.getValue()), JOB_JSON); + const fullDefinition = JSON.parse(codeMirror.getValue()); + assert + .dom('[data-test-select="full-definition"]') + .exists('View switches to full definition mode'); + assert.equal( + fullDefinition.ID, + job.id, + 'Full definition shows the correct job ID', + ); + assert.equal( + fullDefinition.Name, + job.name, + 'Full definition shows the correct job name', + ); + assert.ok( + fullDefinition.TaskGroups, + 'Full definition includes task groups', + ); + + await click('[data-test-select="full-definition"] button:first-child'); + codeMirror = this.getCodeMirrorInstance(); + assert.equal( + codeMirror.getValue(), + specification_response.Source, + 'Switching back to job spec restores the original specification source', + ); }); }); diff --git a/ui/tests/acceptance/regions-test.js b/ui/tests/acceptance/regions-test.js index 416b1ed3d18..00cf5fe08cf 100644 --- a/ui/tests/acceptance/regions-test.js +++ b/ui/tests/acceptance/regions-test.js @@ -21,6 +21,7 @@ module('Acceptance | regions (only one)', function (hooks) { setupMirage(hooks); hooks.beforeEach(function () { + window.localStorage.clear(); this.server.create('agent'); this.server.create('node-pool'); this.server.create('node'); @@ -105,7 +106,11 @@ module('Acceptance | regions (many)', function (hooks) { }); test('the region switcher is rendered in the nav bar and the region is in the page title', async function (assert) { + let managementToken = this.server.create('token'); + window.localStorage.nomadTokenSecret = managementToken.secretId; + await JobsList.visit(); + await settled(); assert.ok( Layout.navbar.regionSwitcher.isPresent, @@ -146,6 +151,114 @@ module('Acceptance | regions (many)', function (hooks) { ); }); + test('switching regions on a query-param-only transition refreshes the active route model', async function (assert) { + const newRegion = this.server.db.regions[1].id; + + await JobsList.visit(); + + const jobsRequestsBeforeSwitch = + this.server.pretender.handledRequests.filter((request) => + request.url.includes('/v1/jobs'), + ).length; + + await selectChoose('[data-test-region-switcher-parent]', newRegion); + await settled(); + + const jobsRequestsAfterSwitch = + this.server.pretender.handledRequests.filter((request) => + request.url.includes('/v1/jobs'), + ); + + assert.ok( + jobsRequestsAfterSwitch.length > jobsRequestsBeforeSwitch, + 'Jobs model request is issued again after region query-param switch' + ); + + assert.ok( + jobsRequestsAfterSwitch + .slice(jobsRequestsBeforeSwitch) + .some((request) => request.url.includes(`region=${newRegion}`)), + 'Refreshed jobs request uses the selected region' + ); + }); + + test('switching regions on job detail reloads job, allocations, and evaluations', async function (assert) { + const newRegion = this.server.db.regions[1].id; + + await JobsList.visit(); + const jobId = JobsList.jobs.objectAt(0).id; + await JobsList.jobs.objectAt(0).clickRow(); + await settled(); + + const isForJobPath = (request, path) => { + const url = new URL(request.url, window.location.origin); + return url.pathname === `/v1/job/${jobId}${path}`; + }; + + const jobRequestsBeforeSwitch = + this.server.pretender.handledRequests.filter((request) => + isForJobPath(request, ''), + ).length; + + const allocationRequestsBeforeSwitch = + this.server.pretender.handledRequests.filter((request) => + isForJobPath(request, '/allocations'), + ).length; + + const evaluationRequestsBeforeSwitch = + this.server.pretender.handledRequests.filter((request) => + isForJobPath(request, '/evaluations'), + ).length; + + await selectChoose('[data-test-region-switcher-parent]', newRegion); + await settled(); + + const jobRequestsAfterSwitch = + this.server.pretender.handledRequests.filter((request) => + isForJobPath(request, ''), + ); + const allocationRequestsAfterSwitch = + this.server.pretender.handledRequests.filter((request) => + isForJobPath(request, '/allocations'), + ); + const evaluationRequestsAfterSwitch = + this.server.pretender.handledRequests.filter((request) => + isForJobPath(request, '/evaluations'), + ); + + assert.ok( + jobRequestsAfterSwitch.length > jobRequestsBeforeSwitch, + 'Job record is fetched again after switching regions on job detail' + ); + assert.ok( + allocationRequestsAfterSwitch.length > allocationRequestsBeforeSwitch, + 'Job allocations are fetched again after switching regions on job detail' + ); + assert.ok( + evaluationRequestsAfterSwitch.length > evaluationRequestsBeforeSwitch, + 'Job evaluations are fetched again after switching regions on job detail' + ); + + assert.ok( + jobRequestsAfterSwitch + .slice(jobRequestsBeforeSwitch) + .some((request) => request.url.includes(`region=${newRegion}`)), + 'Refetched job request includes selected region' + ); + assert.ok( + allocationRequestsAfterSwitch + .slice(allocationRequestsBeforeSwitch) + .some((request) => request.url.includes(`region=${newRegion}`)), + 'Refetched allocations request includes selected region' + ); + assert.ok( + evaluationRequestsAfterSwitch + .slice(evaluationRequestsBeforeSwitch) + .some((request) => request.url.includes(`region=${newRegion}`)), + 'Refetched evaluations request includes selected region' + ); + }); + test('switching regions to the default region, unsets the region query param', async function (assert) { let managementToken = this.server.create('token'); window.localStorage.nomadTokenSecret = managementToken.secretId;