diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index eeba0652255d..e130478155af 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -1,19 +1,9 @@ import { inject as service } from '@ember/service'; -import { assign } from '@ember/polyfills'; import Watchable from './watchable'; export default Watchable.extend({ system: service(), - buildQuery() { - const namespace = this.get('system.activeNamespace.id'); - - if (namespace && namespace !== 'default') { - return { namespace }; - } - return {}; - }, - findAll() { const namespace = this.get('system.activeNamespace'); return this._super(...arguments).then(data => { @@ -24,12 +14,6 @@ export default Watchable.extend({ }); }, - findRecordSummary(modelName, name, snapshot, namespaceQuery) { - return this.ajax(`${this.buildURL(modelName, name, snapshot, 'findRecord')}/summary`, 'GET', { - data: assign(this.buildQuery() || {}, namespaceQuery), - }); - }, - findRecord(store, type, id, snapshot) { const [, namespace] = JSON.parse(id); const namespaceQuery = namespace && namespace !== 'default' ? { namespace } : {}; @@ -37,40 +21,31 @@ export default Watchable.extend({ return this._super(store, type, id, snapshot, namespaceQuery); }, + urlForFindAll() { + const url = this._super(...arguments); + const namespace = this.get('system.activeNamespace.id'); + return associateNamespace(url, namespace); + }, + urlForFindRecord(id, type, hash) { const [name, namespace] = JSON.parse(id); let url = this._super(name, type, hash); - if (namespace && namespace !== 'default') { - url += `?namespace=${namespace}`; - } - return url; + return associateNamespace(url, namespace); }, xhrKey(url, method, options = {}) { const plainKey = this._super(...arguments); const namespace = options.data && options.data.namespace; - if (namespace) { - return `${plainKey}?namespace=${namespace}`; - } - return plainKey; + return associateNamespace(plainKey, namespace); }, relationshipFallbackLinks: { summary: '/summary', }, - findAllocations(job) { - const url = `${this.buildURL('job', job.get('id'), job, 'findRecord')}/allocations`; - return this.ajax(url, 'GET', { data: this.buildQuery() }).then(allocs => { - return this.store.pushPayload('allocation', { - allocations: allocs, - }); - }); - }, - fetchRawDefinition(job) { - const url = this.buildURL('job', job.get('id'), job, 'findRecord'); - return this.ajax(url, 'GET', { data: this.buildQuery() }); + const url = this.urlForFindRecord(job.get('id'), 'job'); + return this.ajax(url, 'GET'); }, forcePeriodic(job) { @@ -86,6 +61,13 @@ export default Watchable.extend({ }, }); +function associateNamespace(url, namespace) { + if (namespace && namespace !== 'default') { + url += `?namespace=${namespace}`; + } + return url; +} + function addToPath(url, extension = '') { const [path, params] = url.split('?'); let newUrl = `${path}${extension}`; diff --git a/ui/app/adapters/node.js b/ui/app/adapters/node.js index 5d77bebd2688..8aa4b662fd5b 100644 --- a/ui/app/adapters/node.js +++ b/ui/app/adapters/node.js @@ -1,12 +1,3 @@ import Watchable from './watchable'; -export default Watchable.extend({ - findAllocations(node) { - const url = `${this.buildURL('node', node.get('id'), node, 'findRecord')}/allocations`; - return this.ajax(url, 'GET').then(allocs => { - return this.store.pushPayload('allocation', { - allocations: allocs, - }); - }); - }, -}); +export default Watchable.extend(); diff --git a/ui/app/utils/properties/watch.js b/ui/app/utils/properties/watch.js index a372d293d6d5..aec2c6921bb0 100644 --- a/ui/app/utils/properties/watch.js +++ b/ui/app/utils/properties/watch.js @@ -3,13 +3,16 @@ import { get } from '@ember/object'; import RSVP from 'rsvp'; import { task } from 'ember-concurrency'; import wait from 'nomad-ui/utils/wait'; +import config from 'nomad-ui/config/environment'; + +const isEnabled = config.APP.blockingQueries !== false; export function watchRecord(modelName) { return task(function*(id, throttle = 2000) { if (typeof id === 'object') { id = get(id, 'id'); } - while (!Ember.testing) { + while (isEnabled && !Ember.testing) { try { yield RSVP.all([ this.get('store').findRecord(modelName, id, { @@ -32,7 +35,7 @@ export function watchRecord(modelName) { export function watchRelationship(relationshipName) { return task(function*(model, throttle = 2000) { - while (!Ember.testing) { + while (isEnabled && !Ember.testing) { try { yield RSVP.all([ this.get('store') @@ -54,7 +57,7 @@ export function watchRelationship(relationshipName) { export function watchAll(modelName) { return task(function*(throttle = 2000) { - while (!Ember.testing) { + while (isEnabled && !Ember.testing) { try { yield RSVP.all([ this.get('store').findAll(modelName, { reload: true, adapterOptions: { watch: true } }), diff --git a/ui/config/environment.js b/ui/config/environment.js index fdceb4472747..b97c3f1836e4 100644 --- a/ui/config/environment.js +++ b/ui/config/environment.js @@ -19,8 +19,7 @@ module.exports = function(environment) { }, APP: { - // Here you can pass flags/options to your application instance - // when it is created + blockingQueries: true, }, }; diff --git a/ui/mirage/factories/allocation.js b/ui/mirage/factories/allocation.js index 6c1c2039310f..3336a9c1588a 100644 --- a/ui/mirage/factories/allocation.js +++ b/ui/mirage/factories/allocation.js @@ -125,6 +125,7 @@ export default Factory.extend({ ); const job = allocation.jobId ? server.db.jobs.find(allocation.jobId) : pickOne(server.db.jobs); + const namespace = allocation.namespace || job.namespace; const node = allocation.nodeId ? server.db.nodes.find(allocation.nodeId) : pickOne(server.db.nodes); @@ -147,6 +148,7 @@ export default Factory.extend({ ); allocation.update({ + namespace, jobId: job.id, nodeId: node.id, taskStateIds: states.mapBy('id'), diff --git a/ui/tests/acceptance/client-detail-test.js b/ui/tests/acceptance/client-detail-test.js index f1860c610332..72204b92b2d1 100644 --- a/ui/tests/acceptance/client-detail-test.js +++ b/ui/tests/acceptance/client-detail-test.js @@ -654,3 +654,45 @@ test('when the node has a drain stategy with a negative deadline, the drain stra ); }); }); + +moduleForAcceptance('Acceptance | client detail (multi-namespace)', { + beforeEach() { + server.create('node', 'forceIPv4', { schedulingEligibility: 'eligible' }); + node = server.db.nodes[0]; + + // Related models + server.create('namespace'); + server.create('namespace', { id: 'other-namespace' }); + + server.create('agent'); + + // Make a job for each namespace, but have both scheduled on the same node + server.create('job', { id: 'job-1', namespaceId: 'default', createAllocations: false }); + server.createList('allocation', 3, { nodeId: node.id }); + + server.create('job', { id: 'job-2', namespaceId: 'other-namespace', createAllocations: false }); + server.createList('allocation', 3, { nodeId: node.id, jobId: 'job-2' }); + }, +}); + +test('when the node has allocations on different namespaces, the associated jobs are fetched correctly', function(assert) { + window.localStorage.nomadActiveNamespace = 'other-namespace'; + + visit(`/clients/${node.id}`); + + andThen(() => { + assert.equal( + findAll('[data-test-allocation]').length, + server.db.allocations.length, + 'All allocations are scheduled on this node' + ); + assert.ok( + server.pretender.handledRequests.findBy('url', '/v1/job/job-1'), + 'Job One fetched correctly' + ); + assert.ok( + server.pretender.handledRequests.findBy('url', '/v1/job/job-2?namespace=other-namespace'), + 'Job Two fetched correctly' + ); + }); +}); diff --git a/ui/tests/unit/adapters/job-test.js b/ui/tests/unit/adapters/job-test.js index aa6827d5a773..1eca17e933ad 100644 --- a/ui/tests/unit/adapters/job-test.js +++ b/ui/tests/unit/adapters/job-test.js @@ -1,4 +1,5 @@ import EmberObject from '@ember/object'; +import { getOwner } from '@ember/application'; import { run } from '@ember/runloop'; import { assign } from '@ember/polyfills'; import { test } from 'ember-qunit'; @@ -8,32 +9,87 @@ import moduleForAdapter from '../../helpers/module-for-adapter'; moduleForAdapter('job', 'Unit | Adapter | Job', { needs: [ + 'adapter:application', 'adapter:job', - 'service:token', - 'service:system', + 'adapter:namespace', + 'model:task-group', 'model:allocation', 'model:deployment', 'model:evaluation', 'model:job-summary', 'model:job-version', 'model:namespace', - 'adapter:application', + 'model:task-group-summary', + 'serializer:namespace', + 'serializer:job', + 'serializer:job-summary', + 'service:token', + 'service:system', 'service:watchList', + 'transform:fragment', + 'transform:fragment-array', ], beforeEach() { window.sessionStorage.clear(); + window.localStorage.clear(); this.server = startMirage(); + this.server.create('namespace'); + this.server.create('namespace', { id: 'some-namespace' }); this.server.create('node'); - this.server.create('job', { id: 'job-1' }); + this.server.create('job', { id: 'job-1', namespaceId: 'default' }); this.server.create('job', { id: 'job-2', namespaceId: 'some-namespace' }); + + this.system = getOwner(this).lookup('service:system'); + this.system.get('namespaces'); + + // Reset the handledRequests array to avoid accounting for this + // namespaces request everywhere. + this.server.pretender.handledRequests.length = 0; }, afterEach() { this.server.shutdown(); }, }); -test('The job summary is stitched into the job request', function(assert) { +test('The job endpoint is the only required endpoint for fetching a job', function(assert) { + const { pretender } = this.server; + const jobName = 'job-1'; + const jobNamespace = 'default'; + const jobId = JSON.stringify([jobName, jobNamespace]); + + this.subject().findRecord(null, { modelName: 'job' }, jobId); + + assert.deepEqual( + pretender.handledRequests.mapBy('url'), + [`/v1/job/${jobName}`], + 'The only request made is /job/:id' + ); +}); + +test('When a namespace is set in localStorage but a job in the default namespace is requested, the namespace query param is not present', function(assert) { + window.localStorage.nomadActiveNamespace = 'some-namespace'; + + const { pretender } = this.server; + const jobName = 'job-1'; + const jobNamespace = 'default'; + const jobId = JSON.stringify([jobName, jobNamespace]); + + this.system.get('namespaces'); + return wait().then(() => { + this.subject().findRecord(null, { modelName: 'job' }, jobId); + + assert.deepEqual( + pretender.handledRequests.mapBy('url'), + [`/v1/job/${jobName}`], + 'The one request made is /job/:id with no namespace query param' + ); + }); +}); + +test('When a namespace is in localStorage and the requested job is in the default namespace, the namespace query param is left out', function(assert) { + window.localStorage.nomadActiveNamespace = 'red-herring'; + const { pretender } = this.server; const jobName = 'job-1'; const jobNamespace = 'default'; @@ -43,8 +99,8 @@ test('The job summary is stitched into the job request', function(assert) { assert.deepEqual( pretender.handledRequests.mapBy('url'), - ['/v1/namespaces', `/v1/job/${jobName}`], - 'The two requests made are /namespaces and /job/:id' + [`/v1/job/${jobName}`], + 'The request made is /job/:id with no namespace query param' ); }); @@ -58,8 +114,8 @@ test('When the job has a namespace other than default, it is in the URL', functi assert.deepEqual( pretender.handledRequests.mapBy('url'), - ['/v1/namespaces', `/v1/job/${jobName}?namespace=${jobNamespace}`], - 'The two requests made are /namespaces and /job/:id?namespace=:namespace' + [`/v1/job/${jobName}?namespace=${jobNamespace}`], + 'The only request made is /job/:id?namespace=:namespace' ); }); @@ -103,11 +159,6 @@ test('findAll can be watched', function(assert) { request(); assert.equal( pretender.handledRequests[0].url, - '/v1/namespaces', - 'First request is for namespaces' - ); - assert.equal( - pretender.handledRequests[1].url, '/v1/jobs?index=1', 'Second request is a blocking request for jobs' ); @@ -115,7 +166,7 @@ test('findAll can be watched', function(assert) { return wait().then(() => { request(); assert.equal( - pretender.handledRequests[2].url, + pretender.handledRequests[1].url, '/v1/jobs?index=2', 'Third request is a blocking request with an incremented index param' ); @@ -137,11 +188,6 @@ test('findRecord can be watched', function(assert) { request(); assert.equal( pretender.handledRequests[0].url, - '/v1/namespaces', - 'First request is for namespaces' - ); - assert.equal( - pretender.handledRequests[1].url, '/v1/job/job-1?index=1', 'Second request is a blocking request for job-1' ); @@ -149,7 +195,7 @@ test('findRecord can be watched', function(assert) { return wait().then(() => { request(); assert.equal( - pretender.handledRequests[2].url, + pretender.handledRequests[1].url, '/v1/job/job-1?index=2', 'Third request is a blocking request with an incremented index param' ); @@ -164,11 +210,13 @@ test('relationships can be reloaded', function(assert) { const mockModel = makeMockModel(plainId); this.subject().reloadRelationship(mockModel, 'summary'); - assert.equal( - pretender.handledRequests[0].url, - `/v1/job/${plainId}/summary`, - 'Relationship was reloaded' - ); + return wait().then(() => { + assert.equal( + pretender.handledRequests[0].url, + `/v1/job/${plainId}/summary`, + 'Relationship was reloaded' + ); + }); }); test('relationship reloads can be watched', function(assert) {