Skip to content

Commit

Permalink
Merge pull request #7855 from hashicorp/b-ui/alloc-wrong-reserved-res…
Browse files Browse the repository at this point in the history
…ources

UI: Make allocation reference own task group instead of job's task group when job versions don't match
  • Loading branch information
DingoEatingFuzz authored May 6, 2020
2 parents 16d076d + 59636ad commit d9aaaa0
Show file tree
Hide file tree
Showing 8 changed files with 218 additions and 2 deletions.
3 changes: 3 additions & 0 deletions ui/app/components/allocation-row.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ export default Component.extend({

function qualifyAllocation() {
const allocation = this.allocation;

// Make sure the allocation is a complete record and not a partial so we
// can show information such as preemptions and rescheduled allocation.
return allocation.reload().then(() => {
this.fetchStats.perform();

Expand Down
13 changes: 12 additions & 1 deletion ui/app/models/allocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,22 @@ export default Model.extend({
return classMap[this.clientStatus] || 'is-dark';
}),

taskGroup: computed('taskGroupName', 'job.taskGroups.[]', function() {
isOld: computed('jobVersion', 'job.version', function() {
return this.jobVersion !== this.get('job.version');
}),

taskGroup: computed('isOld', 'jobTaskGroup', 'allocationTaskGroup', function() {
if (!this.isOld) return this.jobTaskGroup;
return this.allocationTaskGroup;
}),

jobTaskGroup: computed('taskGroupName', 'job.taskGroups.[]', function() {
const taskGroups = this.get('job.taskGroups');
return taskGroups && taskGroups.findBy('name', this.taskGroupName);
}),

allocationTaskGroup: fragment('task-group', { defaultValue: null }),

unhealthyDrivers: computed('taskGroup.drivers.[]', 'node.unhealthyDriverNames.[]', function() {
const taskGroupUnhealthyDrivers = this.get('taskGroup.drivers');
const nodeUnhealthyDrivers = this.get('node.unhealthyDriverNames');
Expand Down
9 changes: 9 additions & 0 deletions ui/app/serializers/allocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ import { inject as service } from '@ember/service';
import { get } from '@ember/object';
import ApplicationSerializer from './application';

const taskGroupFromJob = (job, taskGroupName) => {
const taskGroups = job && job.TaskGroups;
const taskGroup = taskGroups && taskGroups.find(group => group.Name === taskGroupName);
return taskGroup ? taskGroup : null;
};

export default ApplicationSerializer.extend({
system: service(),

Expand Down Expand Up @@ -54,6 +60,9 @@ export default ApplicationSerializer.extend({
// When present, the resources are nested under AllocatedResources.Shared
hash.AllocatedResources = hash.AllocatedResources && hash.AllocatedResources.Shared;

// The Job definition for an allocation is only included in findRecord responses.
hash.AllocationTaskGroup = !hash.Job ? null : taskGroupFromJob(hash.Job, hash.TaskGroup);

return this._super(typeHash, hash);
},
});
2 changes: 1 addition & 1 deletion ui/mirage/factories/allocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const REF_TIME = new Date();
export default Factory.extend({
id: i => (i >= 100 ? `${UUIDS[i % 100]}-${i}` : UUIDS[i]),

jobVersion: () => faker.random.number(10),
jobVersion: 1,

modifyIndex: () => faker.random.number({ min: 10, max: 2000 }),
modifyTime: () => faker.date.past(2 / 365, REF_TIME) * 1000000,
Expand Down
2 changes: 2 additions & 0 deletions ui/mirage/factories/job.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export default Factory.extend({
return this.id;
},

version: 1,

groupsCount: () => faker.random.number({ min: 1, max: 2 }),

region: () => 'global',
Expand Down
98 changes: 98 additions & 0 deletions ui/tests/unit/models/allocation-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { run } from '@ember/runloop';
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';

module('Unit | Model | allocation', function(hooks) {
setupTest(hooks);
hooks.beforeEach(function() {
this.store = this.owner.lookup('service:store');
});

test("When the allocation's job version matches the job's version, the task group comes from the job.", function(assert) {
const job = run(() =>
this.store.createRecord('job', {
name: 'this-job',
version: 1,
taskGroups: [
{
name: 'from-job',
count: 1,
task: [],
},
],
})
);

const allocation = run(() =>
this.store.createRecord('allocation', {
job,
jobVersion: 1,
taskGroupName: 'from-job',
allocationTaskGroup: {
name: 'from-allocation',
count: 1,
task: [],
},
})
);

assert.equal(allocation.get('taskGroup.name'), 'from-job');
});

test("When the allocation's job version does not match the job's version, the task group comes from the alloc.", function(assert) {
const job = run(() =>
this.store.createRecord('job', {
name: 'this-job',
version: 1,
taskGroups: [
{
name: 'from-job',
count: 1,
task: [],
},
],
})
);

const allocation = run(() =>
this.store.createRecord('allocation', {
job,
jobVersion: 2,
taskGroupName: 'from-job',
allocationTaskGroup: {
name: 'from-allocation',
count: 1,
task: [],
},
})
);

assert.equal(allocation.get('taskGroup.name'), 'from-allocation');
});

test("When the allocation's job version does not match the job's version and the allocation has no task group, then task group is null", async function(assert) {
const job = run(() =>
this.store.createRecord('job', {
name: 'this-job',
version: 1,
taskGroups: [
{
name: 'from-job',
count: 1,
task: [],
},
],
})
);

const allocation = run(() =>
this.store.createRecord('allocation', {
job,
jobVersion: 2,
taskGroupName: 'from-job',
})
);

assert.equal(allocation.get('taskGroup.name'), null);
});
});
90 changes: 90 additions & 0 deletions ui/tests/unit/serializers/allocation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ module('Unit | Serializer | Allocation', function(hooks) {
},
],
wasPreempted: false,
allocationTaskGroup: null,
},
relationships: {
followUpEvaluation: {
Expand Down Expand Up @@ -116,6 +117,7 @@ module('Unit | Serializer | Allocation', function(hooks) {
},
],
wasPreempted: false,
allocationTaskGroup: null,
},
relationships: {
followUpEvaluation: {
Expand Down Expand Up @@ -180,6 +182,7 @@ module('Unit | Serializer | Allocation', function(hooks) {
},
],
wasPreempted: true,
allocationTaskGroup: null,
},
relationships: {
followUpEvaluation: {
Expand Down Expand Up @@ -213,6 +216,93 @@ module('Unit | Serializer | Allocation', function(hooks) {
},
},
},

{
name: 'Derives task group from embedded job when available',
in: {
ID: 'test-allocation',
JobID: 'test-summary',
Name: 'test-summary[1]',
Namespace: 'test-namespace',
TaskGroup: 'test-group',
CreateTime: +sampleDate * 1000000,
ModifyTime: +sampleDate * 1000000,
TaskStates: {
task: {
State: 'running',
Failed: false,
},
},
Job: {
ID: 'test-summary',
Name: 'test-summary',
TaskGroups: [
{
Name: 'fake-group',
Count: 2,
Tasks: [],
EphemeralDisk: {},
},
{
Name: 'test-group',
Count: 3,
Tasks: [],
EphemeralDisk: {},
},
],
},
},
out: {
data: {
id: 'test-allocation',
type: 'allocation',
attributes: {
taskGroupName: 'test-group',
name: 'test-summary[1]',
modifyTime: sampleDate,
createTime: sampleDate,
states: [
{
name: 'task',
state: 'running',
failed: false,
},
],
wasPreempted: false,
allocationTaskGroup: {
name: 'test-group',
count: 3,
tasks: [],
services: [],
volumes: [],
},
},
relationships: {
followUpEvaluation: {
data: null,
},
nextAllocation: {
data: null,
},
previousAllocation: {
data: null,
},
preemptedAllocations: {
data: [],
},
preemptedByAllocation: {
data: null,
},
job: {
data: {
id: '["test-summary","test-namespace"]',
type: 'job',
},
},
},
},
},
},
];

normalizationTestCases.forEach(testCase => {
Expand Down
3 changes: 3 additions & 0 deletions ui/tests/unit/serializers/volume-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ module('Unit | Serializer | Volume', function(hooks) {
taskGroupName: 'foobar',
wasPreempted: false,
states: [],
allocationTaskGroup: null,
},
relationships: {
followUpEvaluation: {
Expand Down Expand Up @@ -286,6 +287,7 @@ module('Unit | Serializer | Volume', function(hooks) {
taskGroupName: 'write-here',
wasPreempted: false,
states: [],
allocationTaskGroup: null,
},
relationships: {
followUpEvaluation: {
Expand Down Expand Up @@ -317,6 +319,7 @@ module('Unit | Serializer | Volume', function(hooks) {
taskGroupName: 'look-if-you-must',
wasPreempted: false,
states: [],
allocationTaskGroup: null,
},
relationships: {
followUpEvaluation: {
Expand Down

0 comments on commit d9aaaa0

Please sign in to comment.