Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Jobs requested with the wrong namespace under the right conditions #4475

Merged
merged 6 commits into from
Jul 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 17 additions & 35 deletions ui/app/adapters/job.js
Original file line number Diff line number Diff line change
@@ -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 => {
Expand All @@ -24,53 +14,38 @@ 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 } : {};

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) {
Expand All @@ -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}`;
Expand Down
11 changes: 1 addition & 10 deletions ui/app/adapters/node.js
Original file line number Diff line number Diff line change
@@ -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();
9 changes: 6 additions & 3 deletions ui/app/utils/properties/watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand All @@ -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')
Expand All @@ -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 } }),
Expand Down
3 changes: 1 addition & 2 deletions ui/config/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};

Expand Down
2 changes: 2 additions & 0 deletions ui/mirage/factories/allocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -147,6 +148,7 @@ export default Factory.extend({
);

allocation.update({
namespace,
jobId: job.id,
nodeId: node.id,
taskStateIds: states.mapBy('id'),
Expand Down
42 changes: 42 additions & 0 deletions ui/tests/acceptance/client-detail-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
});
});
Loading