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: Make allocation reference own task group instead of job's task group when job versions don't match #7855

Merged
merged 3 commits into from
May 6, 2020
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
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